Copilot commented on code in PR #843:
URL: https://github.com/apache/unomi/pull/843#discussion_r3740306329
##########
graphql/cxs-impl/src/main/java/org/apache/unomi/graphql/servlet/websocket/SubscriptionWebSocket.java:
##########
@@ -106,33 +125,51 @@ private void unsubscribe(GraphQLMessage message) {
private void subscribe(GraphQLMessage message) {
final Map<String, Object> payload = message.getPayload();
- ExecutionInput executionInput = ExecutionInput.newExecutionInput()
- .query((String) payload.get("query"))
- .variables((Map<String, Object>) payload.get("variables"))
- .operationName((String) payload.get("operationName"))
- .context(serviceManager)
- .build();
-
- ExecutionResult executionResult = this.graphQL.execute(executionInput);
- if (executionResult.getErrors() != null &&
!executionResult.getErrors().isEmpty()) {
- sendMessage(GraphQLMessage.create(message.getId())
- .errors(executionResult.getErrors())
- .build());
- closeConnection(message, "Error executing graphQL query");
- return;
- } else if (!(executionResult.getData() instanceof Publisher)) {
- final String error = "Fetched value should be instance of
Publisher, was: " + executionResult.getClass().getName();
- sendMessage(GraphQLMessage.create(message.getId())
- .errors(Collections.singletonList(error))
- .build());
- closeConnection(message, error);
- return;
+ try {
+ securityService.setCurrentSubject(subject);
+ executionContextManager.setCurrentContext(executionContext);
+
+ Map<String, Object> variables = (Map<String, Object>)
payload.get("variables");
+ if (variables == null) {
+ variables = new HashMap<>();
+ }
+
+ ExecutionInput executionInput = ExecutionInput.newExecutionInput()
+ .query((String) payload.get("query"))
+ .variables(variables)
+ .operationName((String) payload.get("operationName"))
+ .context(serviceManager)
+ .build();
+
+ ExecutionResult executionResult =
this.graphQL.execute(executionInput);
+ if (executionResult.getErrors() != null &&
!executionResult.getErrors().isEmpty()) {
+ sendMessage(GraphQLMessage.create(message.getId())
+ .errors(executionResult.getErrors())
+ .build());
+ closeConnection(message, "Error executing graphQL query");
+ return;
+ } else if (!(executionResult.getData() instanceof Publisher)) {
+ Object data = executionResult.getData();
+ final String error = "Fetched value should be instance of
Publisher, was: " + (data == null ? "null" : data.getClass().getName());
+ sendMessage(GraphQLMessage.create(message.getId())
+ .errors(Collections.singletonList(error))
+ .build());
+ closeConnection(message, error);
+ return;
+ }
+
+ Publisher<ExecutionResult> publisher = executionResult.getData();
+ ExecutionResultSubscriber subscriber = new
ExecutionResultSubscriber(message.getId(), getRemote());
+ publisher.subscribe(subscriber);
+
+ subscriptions.put(message.getId(), subscriber);
+ } finally {
+ try {
+ securityService.clearCurrentSubject();
+ executionContextManager.setCurrentContext(null);
Review Comment:
The authenticated context only covers subscription registration. GraphQL
executes a subscription's selection set when each upstream event is emitted,
after this `finally` has cleared the thread-locals. For example, the tested
`cdp_profile` field can call `ProfileService.load` during event delivery
(`CDPEventInterface.java:77-85`), so it runs with the producer thread's context
rather than this socket's subject/context. That can apply the wrong
tenant/permissions. Propagate the captured context around each subscription
event execution and clear it after each signal, not just around `subscribe()`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
