Skip to content

Commit a960132

Browse files
committed
Polishing contribution
See gh-790
1 parent c244332 commit a960132

File tree

4 files changed

+27
-40
lines changed

4 files changed

+27
-40
lines changed

spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AuthenticationPrincipalArgumentResolver.java

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2022 the original author or authors.
2+
* Copyright 2002-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -96,26 +96,20 @@ private static AuthenticationPrincipal findMethodAnnotation(MethodParameter para
9696

9797
@Override
9898
public Object resolveArgument(MethodParameter parameter, DataFetchingEnvironment environment) throws Exception {
99-
return getCurrentAuthentication(parameter.isOptional())
100-
.flatMap(auth -> Mono.justOrEmpty(resolvePrincipal(parameter, auth.getPrincipal())))
101-
.transform((argument) -> isParameterMonoAssignable(parameter) ? Mono.just(argument) : argument);
99+
return getCurrentAuthentication(parameter)
100+
.mapNotNull(auth -> resolvePrincipal(parameter, auth.getPrincipal()))
101+
.transform((argument) -> isPublisherOrMono(parameter) ? Mono.just(argument) : argument);
102102
}
103103

104-
private static boolean isParameterMonoAssignable(MethodParameter parameter) {
104+
private static boolean isPublisherOrMono(MethodParameter parameter) {
105105
Class<?> type = parameter.getParameterType();
106106
return (Publisher.class.equals(type) || Mono.class.equals(type));
107107
}
108108

109109
@SuppressWarnings("unchecked")
110-
private Mono<Authentication> getCurrentAuthentication(boolean optional) {
111-
Object principal = PrincipalMethodArgumentResolver.doResolve(optional);
112-
if (principal instanceof Authentication) {
113-
return Mono.just((Authentication) principal);
114-
}
115-
else if (principal instanceof Mono) {
116-
return (Mono<Authentication>) principal;
117-
}
118-
return Mono.error(new IllegalStateException("Unexpected return value: " + principal));
110+
private Mono<Authentication> getCurrentAuthentication(MethodParameter parameter) {
111+
Object value = PrincipalMethodArgumentResolver.resolveAuthentication(parameter);
112+
return (value instanceof Authentication auth ? Mono.just(auth) : (Mono<Authentication>) value);
119113
}
120114

121115
@Nullable
@@ -144,7 +138,7 @@ private boolean isInvalidType(MethodParameter parameter, @Nullable Object princi
144138
return false;
145139
}
146140
Class<?> typeToCheck = parameter.getParameterType();
147-
if (isParameterMonoAssignable(parameter)) {
141+
if (isPublisherOrMono(parameter)) {
148142
Class<?> genericType = parameter.nested().getNestedParameterType();
149143
if (genericType.equals(Object.class)) {
150144
return false;

spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/BatchLoaderHandlerMethod.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2022 the original author or authors.
2+
* Copyright 2002-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -145,7 +145,7 @@ else if ("kotlin.coroutines.Continuation".equals(parameterType.getName())) {
145145
return null;
146146
}
147147
else if (springSecurityPresent && Principal.class.isAssignableFrom(parameter.getParameterType())) {
148-
return PrincipalMethodArgumentResolver.doResolve(parameter.isOptional());
148+
return PrincipalMethodArgumentResolver.resolveAuthentication(parameter);
149149
}
150150
else {
151151
throw new IllegalStateException(formatArgumentError(parameter, "Unexpected argument type."));

spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/PrincipalMethodArgumentResolver.java

Lines changed: 14 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,13 @@
1616
package org.springframework.graphql.data.method.annotation.support;
1717

1818
import java.security.Principal;
19-
import java.util.function.Function;
2019

2120
import graphql.schema.DataFetchingEnvironment;
2221

2322
import org.springframework.core.MethodParameter;
2423
import org.springframework.graphql.data.method.HandlerMethodArgumentResolver;
2524
import org.springframework.security.authentication.AuthenticationCredentialsNotFoundException;
2625
import org.springframework.security.core.Authentication;
27-
import org.springframework.security.core.AuthenticationException;
2826
import org.springframework.security.core.context.ReactiveSecurityContextHolder;
2927
import org.springframework.security.core.context.SecurityContext;
3028
import org.springframework.security.core.context.SecurityContextHolder;
@@ -44,7 +42,7 @@
4442
public class PrincipalMethodArgumentResolver implements HandlerMethodArgumentResolver {
4543

4644
/**
47-
* Return "true" if the argument is {@link Principal} or a sub-type.
45+
* Return "true" if the argument is {@link Principal} or a subtype.
4846
*/
4947
@Override
5048
public boolean supportsParameter(MethodParameter parameter) {
@@ -54,29 +52,24 @@ public boolean supportsParameter(MethodParameter parameter) {
5452

5553
@Override
5654
public Object resolveArgument(MethodParameter parameter, DataFetchingEnvironment environment) {
57-
return doResolve(parameter.isOptional());
55+
return resolveAuthentication(parameter);
5856
}
5957

60-
static Object doResolve(boolean optional) {
61-
Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
62-
63-
if (authentication != null) {
64-
return authentication;
58+
static Object resolveAuthentication(MethodParameter parameter) {
59+
Authentication auth = SecurityContextHolder.getContext().getAuthentication();
60+
if (auth != null) {
61+
return auth;
6562
}
6663

67-
return ReactiveSecurityContextHolder.getContext()
68-
.switchIfEmpty(optional ? Mono.empty() : Mono.error(new AuthenticationCredentialsNotFoundException("SecurityContext not available")))
69-
.handle((context, sink) -> {
70-
Authentication auth = context.getAuthentication();
64+
Mono<Authentication> authMono =
65+
ReactiveSecurityContextHolder.getContext().mapNotNull(SecurityContext::getAuthentication);
66+
67+
if (!parameter.isOptional()) {
68+
authMono = authMono.switchIfEmpty(
69+
Mono.error(new AuthenticationCredentialsNotFoundException("No Authentication")));
70+
}
7171

72-
if (auth != null) {
73-
sink.next(auth);
74-
} else if (!optional) {
75-
sink.error(new AuthenticationCredentialsNotFoundException("An Authentication object was not found in the SecurityContext"));
76-
} else {
77-
sink.complete();
78-
}
79-
});
72+
return authMono;
8073
}
8174

8275
}

spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/SchemaMappingPrincipalMethodArgumentResolverTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ void nonNullPrincipalRequiresSecurityContext() {
134134

135135
assertThat(responseHelper.errorCount()).isEqualTo(1);
136136
assertThat(responseHelper.error(0).errorType()).isEqualTo("UNAUTHORIZED");
137-
assertThat(responseHelper.error(0).message()).isEqualTo("Resolved error: SecurityContext not available");
137+
assertThat(responseHelper.error(0).message()).isEqualTo("Resolved error: No Authentication");
138138
}
139139

140140
@Test
@@ -154,7 +154,7 @@ void nonNullPrincipalRequiresAuthentication() {
154154

155155
assertThat(responseHelper.errorCount()).isEqualTo(1);
156156
assertThat(responseHelper.error(0).errorType()).isEqualTo("UNAUTHORIZED");
157-
assertThat(responseHelper.error(0).message()).isEqualTo("Resolved error: An Authentication object was not found in the SecurityContext");
157+
assertThat(responseHelper.error(0).message()).isEqualTo("Resolved error: No Authentication");
158158
}
159159

160160
@Test

0 commit comments

Comments
 (0)