Skip to content

Commit 10eb5bd

Browse files
anthonyraymondrstoyanchev
authored andcommitted
Do not resolve Principal argument if annotated
Closes gh-25780
1 parent 52084ed commit 10eb5bd

File tree

2 files changed

+28
-1
lines changed

2 files changed

+28
-1
lines changed

spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ServletRequestMethodArgumentResolver.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import java.io.IOException;
2020
import java.io.InputStream;
2121
import java.io.Reader;
22+
import java.lang.annotation.Annotation;
2223
import java.security.Principal;
2324
import java.time.ZoneId;
2425
import java.util.Locale;
@@ -84,12 +85,13 @@ public class ServletRequestMethodArgumentResolver implements HandlerMethodArgume
8485
@Override
8586
public boolean supportsParameter(MethodParameter parameter) {
8687
Class<?> paramType = parameter.getParameterType();
88+
final Annotation[] parameterAnnotations = parameter.getParameterAnnotations();
8789
return (WebRequest.class.isAssignableFrom(paramType) ||
8890
ServletRequest.class.isAssignableFrom(paramType) ||
8991
MultipartRequest.class.isAssignableFrom(paramType) ||
9092
HttpSession.class.isAssignableFrom(paramType) ||
9193
(pushBuilder != null && pushBuilder.isAssignableFrom(paramType)) ||
92-
Principal.class.isAssignableFrom(paramType) ||
94+
(Principal.class.isAssignableFrom(paramType) && parameterAnnotations.length == 0) ||
9395
InputStream.class.isAssignableFrom(paramType) ||
9496
Reader.class.isAssignableFrom(paramType) ||
9597
HttpMethod.class == paramType ||

spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletRequestMethodArgumentResolverTests.java

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,10 @@
1818

1919
import java.io.InputStream;
2020
import java.io.Reader;
21+
import java.lang.annotation.ElementType;
22+
import java.lang.annotation.Retention;
23+
import java.lang.annotation.RetentionPolicy;
24+
import java.lang.annotation.Target;
2125
import java.lang.reflect.Method;
2226
import java.security.Principal;
2327
import java.time.ZoneId;
@@ -122,6 +126,19 @@ public void principalAsNull() throws Exception {
122126
assertThat(result).as("Invalid result").isNull();
123127
}
124128

129+
// spring-security already provides the @AuthenticationPrincipal annotation to inject the Principal taken from SecurityContext.getAuthentication.getPrincipal()
130+
// but ServletRequestMethodArgumentResolver used to take precedence over @AuthenticationPrincipal resolver org.springframework.security.web.method.annotation.AuthenticationPrincipalArgumentResolver
131+
// and we used to get the wrong Principal in methods. See https://github.com/spring-projects/spring-framework/pull/25780
132+
@Test
133+
public void annotatedPrincipal() throws Exception {
134+
Principal principal = () -> "Foo";
135+
servletRequest.setUserPrincipal(principal);
136+
Method principalMethod = getClass().getMethod("supportedParamsWithAnnotatedPrincipal", Principal.class);
137+
138+
MethodParameter principalParameter = new MethodParameter(principalMethod, 0);
139+
assertThat(resolver.supportsParameter(principalParameter)).as("Principal not supported").isFalse();
140+
}
141+
125142
@Test
126143
public void locale() throws Exception {
127144
Locale locale = Locale.ENGLISH;
@@ -245,6 +262,14 @@ public PushBuilder newPushBuilder() {
245262
assertThat(result).as("Invalid result").isSameAs(pushBuilder);
246263
}
247264

265+
@Target({ ElementType.PARAMETER })
266+
@Retention(RetentionPolicy.RUNTIME)
267+
public @interface PlaceHolder {}
268+
269+
@SuppressWarnings("unused")
270+
public void supportedParamsWithAnnotatedPrincipal(@PlaceHolder Principal p) {
271+
272+
}
248273

249274
@SuppressWarnings("unused")
250275
public void supportedParams(ServletRequest p0,

0 commit comments

Comments
 (0)