Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@SubscriptionMapping missing AuthenticationPrincipal #861

Closed
madwind opened this issue Nov 27, 2023 · 3 comments
Closed

@SubscriptionMapping missing AuthenticationPrincipal #861

madwind opened this issue Nov 27, 2023 · 3 comments
Assignees
Labels
in: core Issues related to config and core support type: bug A general bug
Milestone

Comments

@madwind
Copy link

madwind commented Nov 27, 2023

When I upgrade from springboot 3.1.4 to 3.2 @SubscriptionMapping seemed to be missing AuthenticationPrincipal

    @SubscriptionMapping
    public Flux<Video> recentVideos(@AuthenticationPrincipal Account account) {
        return userService.listRecentVideos(account.getId());
    }
org.springframework.security.authentication.AuthenticationCredentialsNotFoundException: No Authentication
	at org.springframework.graphql.data.method.annotation.support.PrincipalMethodArgumentResolver.resolveAuthentication(PrincipalMethodArgumentResolver.java:68) ~[spring-graphql-1.2.4.jar:1.2.4]

@QueryMapping works fine.

    @QueryMapping
    public Mono<User> me(@AuthenticationPrincipal Account account) {
        return userRepository.findByAccountId(account.getId());
    }

but in 3.1.4 it works fine.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Nov 27, 2023
@jma-9code
Copy link

jma-9code commented Nov 29, 2023

same here.

Server-side :

  @PreAuthorize("isAuthenticated()")
  @SubscriptionMapping(value = "xxxx")
  public Flux<xxx> xxx() {
    return Flux.empty();
  }

When a client call :

{"errors":[{"message":"Subscription error","locations":[],"extensions":{"classification":"INTERNAL_ERROR"}}]}

"EDIT"
Temporary workaround: simply update the graphql-java version to the one specified in spring-graphql 1.2.4 (the latest version embedded in Spring 3.2)
<graphql-java.version>20.7</graphql-java.version>

Spring boot 3.2 : dependencies depends on java-graphql v21.3
Spring Graphql 1.2.4 : dependencies depends on java-graphql v20.7

@bclozel bclozel self-assigned this Dec 1, 2023
@bclozel
Copy link
Member

bclozel commented Dec 1, 2023

Hello @madwind, @jma-9code

Thanks for reporting this issue. I had a look at this problem and found out why this case is not working with Spring Boot 3.2.0.

Description

Right now the @AuthenticationPrincipal cannot be injected in the method invocation because it cannot be found in the context. Here is the detailed description of what happens:

  1. the schema is parsed and fed to registered GraphQLTypeVisitor implementations
  2. Spring for GraphQL registers two visitors: the ConnectionFieldTypeVisitor that augments the schema for pagination support and the ContextTypeVisitor that decorates data fetchers to set up the context for their execution (reactor context or threadlocals)
  3. The ConnectionFieldTypeVisitor runs first and returns TraversalControl.ABORT when the parent of a field is of type Subscription because we cannot support pagination under this part of the schema. Skipping everything under this part of the graph makes sense for this visitor.
  4. Previously, returning such a control would have no incidence on other visitors and the ContextTypeVisitor would visit this part of the schema and decorate the Subscription controller method. Since this commit in GraphQL Java, returning such a control completely aborts the execution and prevents other visitors from visiting this part of the schema. As a result, the context propagation is not available for subscription mappings: the security principal cannot be injected and the current observation is not available

The problem here comes from a behavior combination between Spring for GraphQL's ConnectionFieldTypeVisitor and a behavior change in GrpahQL Java's SchemaTraverser. This might be a visitor bug in this project, or a regression in GraphQL Java.

Workaround

As noted by @jma-9code, reverting to an older GraphQL Java version works around this problem. For now, downgrading your application to graphql-java 20.7 is the best course of action.

Follow up

I'm not entirely sure about the expected behavior here for GraphQLTypeVisitor and how TraversalControl values are supposed to influence the entire traversal process. I'll create a discussion on the graphql-java repository to clarify the expected behavior here.

EDIT: here's a link to the discussion.

@bclozel bclozel added in: core Issues related to config and core support type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Dec 1, 2023
@bclozel bclozel added this to the 1.2.5 milestone Dec 4, 2023
@bclozel bclozel closed this as completed in 9fee079 Dec 4, 2023
@bclozel
Copy link
Member

bclozel commented Dec 4, 2023

We have fixed this issue in ConnectionFieldTypeVisitor and you can verify with the latest 1.2.5-SNAPSHOT version. In the meantime, downgrading graphql-java version is recommended.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues related to config and core support type: bug A general bug
Projects
None yet
Development

No branches or pull requests

4 participants