From d4a6a73b1d027e267aa2275ab352373902a51360 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Tue, 18 Jun 2024 07:18:31 +0100 Subject: [PATCH] Correct order of authentication resolvers Closes gh-982 --- .../AnnotatedControllerConfigurer.java | 6 +- ...gPrincipalMethodArgumentResolverTests.java | 61 ++++++++++++++++--- 2 files changed, 57 insertions(+), 10 deletions(-) diff --git a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerConfigurer.java b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerConfigurer.java index 720d949c..58ead114 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerConfigurer.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerConfigurer.java @@ -277,6 +277,10 @@ private HandlerMethodArgumentResolverComposite initArgumentResolvers() { resolvers.addResolver(new ArgumentsMethodArgumentResolver(argumentBinder)); resolvers.addResolver(new ContextValueMethodArgumentResolver()); resolvers.addResolver(new LocalContextValueMethodArgumentResolver()); + if (springSecurityPresent) { + ApplicationContext context = obtainApplicationContext(); + resolvers.addResolver(new AuthenticationPrincipalArgumentResolver(new BeanFactoryResolver(context))); + } // Type based resolvers.addResolver(new DataFetchingEnvironmentMethodArgumentResolver()); @@ -284,9 +288,7 @@ private HandlerMethodArgumentResolverComposite initArgumentResolvers() { addSubrangeMethodArgumentResolver(resolvers); addSortMethodArgumentResolver(resolvers); if (springSecurityPresent) { - ApplicationContext context = obtainApplicationContext(); resolvers.addResolver(new PrincipalMethodArgumentResolver()); - resolvers.addResolver(new AuthenticationPrincipalArgumentResolver(new BeanFactoryResolver(context))); } if (KotlinDetector.isKotlinPresent()) { resolvers.addResolver(new ContinuationHandlerMethodArgumentResolver()); diff --git a/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/SchemaMappingPrincipalMethodArgumentResolverTests.java b/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/SchemaMappingPrincipalMethodArgumentResolverTests.java index 49e4e949..bc44dad2 100644 --- a/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/SchemaMappingPrincipalMethodArgumentResolverTests.java +++ b/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/SchemaMappingPrincipalMethodArgumentResolverTests.java @@ -44,7 +44,9 @@ import org.springframework.graphql.execution.ErrorType; import org.springframework.lang.Nullable; import org.springframework.security.authentication.TestingAuthenticationToken; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.core.Authentication; +import org.springframework.security.core.annotation.AuthenticationPrincipal; import org.springframework.security.core.context.ReactiveSecurityContextHolder; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.core.context.SecurityContextImpl; @@ -79,7 +81,7 @@ public class SchemaMappingPrincipalMethodArgumentResolverTests { @Test void supportsParameter() { - Method method = ClassUtils.getMethod(SchemaMappingPrincipalMethodArgumentResolverTests.class, "handle", (Class[]) null); + Method method = ClassUtils.getMethod(getClass(), "handle", (Class[]) null); assertThat(this.resolver.supportsParameter(new MethodParameter(method, 0))).isTrue(); assertThat(this.resolver.supportsParameter(new MethodParameter(method, 1))).isTrue(); assertThat(this.resolver.supportsParameter(new MethodParameter(method, 2))).isFalse(); @@ -121,10 +123,10 @@ void nullablePrincipalDoesntRequireSecurityContext() { @Test void nonNullPrincipalRequiresSecurityContext() { DataFetcherExceptionResolver exceptionResolver = - DataFetcherExceptionResolver.forSingleError((ex, env) -> GraphqlErrorBuilder.newError(env) - .message("Resolved error: " + ex.getMessage()) - .errorType(ErrorType.UNAUTHORIZED) - .build()); + DataFetcherExceptionResolver.forSingleError((ex, env) -> GraphqlErrorBuilder.newError(env) + .message("Resolved error: " + ex.getMessage()) + .errorType(ErrorType.UNAUTHORIZED) + .build()); Mono responseMono = executeAsync( "type Query { greetingMono: String }", "{ greetingMono }", @@ -217,20 +219,47 @@ private void testSubscription(Function contextModifier) { } + + @Nested + class AuthenticationPrincipalTests { + + @Test // gh-982 + void query() { + Authentication authentication = new UsernamePasswordAuthenticationToken(new GraphQlPrincipal(), null); + SecurityContextHolder.setContext(new SecurityContextImpl(authentication)); + try { + String field = "greetingAuthenticationPrincipal"; + Mono responseMono = executeAsync( + "type Query { " + field + " : String }", "{ " + field + " }", threadLocalContextWriter); + + String greeting = ResponseHelper.forResponse(responseMono).toEntity(field, String.class); + assertThat(greeting).isEqualTo("Hello"); + assertThat(greetingController.principal()).isSameAs(authentication.getPrincipal()); + } + finally { + SecurityContextHolder.clearContext(); + } + } + + } + + private Mono executeAsync( String schema, String document, Function contextWriter) { + return executeAsync(schema, document, contextWriter, null); } private Mono executeAsync( - String schema, String document, Function contextWriter, @Nullable DataFetcherExceptionResolver exceptionResolver) { + String schema, String document, Function contextWriter, + @Nullable DataFetcherExceptionResolver exceptionResolver) { AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(); context.registerBean(GreetingController.class, () -> greetingController); context.refresh(); - GraphQlSetup graphQlSetup = GraphQlSetup.schemaContent(schema) - .runtimeWiringForAnnotatedControllers(context); + GraphQlSetup graphQlSetup = + GraphQlSetup.schemaContent(schema).runtimeWiringForAnnotatedControllers(context); if (exceptionResolver != null) { graphQlSetup.exceptionResolver(exceptionResolver); @@ -288,6 +317,22 @@ Flux greetingSubscription(Principal principal) { return Flux.just("Hello", "Hi"); } + @QueryMapping + String greetingAuthenticationPrincipal(@AuthenticationPrincipal GraphQlPrincipal principal) { + this.principal = principal; + return "Hello"; + } + + } + + + private static final class GraphQlPrincipal implements Principal { + + @Override + public String getName() { + return ""; + } + } }