review phase1

Author: clevertension

config/src/main/java/org/springframework/security/config/annotation/web/configuration/WebMvcSecurityConfiguration.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2019 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -40,6 +40,7 @@
  * {@link HandlerMethodArgumentResolver}.
  *
  * @author Rob Winch
+ * @author Dan Zheng
  * @since 3.2
  */
 class WebMvcSecurityConfiguration implements WebMvcConfigurer, ApplicationContextAware {

config/src/main/java/org/springframework/security/config/annotation/web/reactive/ServerHttpSecurityConfiguration.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2018 the original author or authors.
+ * Copyright 2002-2019 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -39,6 +39,7 @@
 
 /**
  * @author Rob Winch
+ * @author Dan Zheng
  * @since 5.0
  */
 @Configuration

core/src/main/java/org/springframework/security/core/annotation/CurrentSecurityContext.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2019 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -26,7 +26,7 @@
  * argument.
  *
  * @author Dan Zheng
- * @since 5.2.x
+ * @since 5.2
  *
  * See: <a href=
  * "{@docRoot}/org/springframework/security/web/bind/support/CurrentSecurityContextArgumentResolver.html"
@@ -43,29 +43,21 @@
 	 * @return
 	 */
 	boolean errorOnInvalidType() default false;
+
 	/**
 	 * If specified will use the provided SpEL expression to resolve the security context. This
 	 * is convenient if users need to transform the result.
 	 *
-	 * <p>
-	 * For example, perhaps the user wants to resolve a CustomUser object that is final
-	 * and is leveraging a UserDetailsService. This can be handled by returning an object
-	 * that looks like:
-	 * </p>
-	 *
 	 * <pre>
-	 * public class CustomUserUserDetails extends User {
-	 *     // ...
-	 *     public CustomUser getCustomUser() {
-	 *         return customUser;
-	 *     }
-	 * }
+	 * &#64;CurrentSecurityContext(expression = "authentication") Authentication authentication
 	 * </pre>
 	 *
-	 * Then the user can specify an annotation that looks like:
+	 * <p>
+	 *    if you want to retrieve more object from the authentcation, you can see the following the expression
+	 * </p>
 	 *
 	 * <pre>
-	 * &#64;CurrentSecurityContext(expression = "authentication")
+	 * &#64;CurrentSecurityContext(expression = "authentication.principal") Object principal
 	 * </pre>
 	 *
 	 * @return the expression to use.

web/src/main/java/org/springframework/security/web/bind/support/CurrentSecurityContextArgumentResolver.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2019 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -15,6 +15,8 @@
  */
 package org.springframework.security.web.bind.support;
 
+import java.lang.annotation.Annotation;
+
 import org.springframework.core.MethodParameter;
 import org.springframework.core.annotation.AnnotationUtils;
 import org.springframework.expression.BeanResolver;
@@ -32,8 +34,6 @@
 import org.springframework.web.method.support.HandlerMethodArgumentResolver;
 import org.springframework.web.method.support.ModelAndViewContainer;
 
-import java.lang.annotation.Annotation;
-
 /**
  * Allows resolving the {@link SecurityContext} using the
  * {@link CurrentSecurityContext} annotation. For example, the following
@@ -69,7 +69,7 @@
  * </p>
  *
  * @author Dan Zheng
- * @since 5.2.x
+ * @since 5.2
  */
 public final class CurrentSecurityContextArgumentResolver
 		implements HandlerMethodArgumentResolver {

web/src/main/java/org/springframework/security/web/reactive/result/method/annotation/CurrentSecurityContextArgumentResolver.java

@@ -40,7 +40,7 @@
 /**
  * Resolves the SecurityContext
  * @author Dan Zheng
- * @since 5.2.x
+ * @since 5.2
  */
 public class CurrentSecurityContextArgumentResolver extends HandlerMethodArgumentResolverSupport {
 

web/src/test/java/org/springframework/security/web/bind/support/CurrentSecurityContextArgumentResolverTests.java

@@ -15,9 +15,12 @@
  */
 package org.springframework.security.web.bind.support;
 
+import java.lang.reflect.Method;
+
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
+
 import org.springframework.core.MethodParameter;
 import org.springframework.expression.spel.SpelEvaluationException;
 import org.springframework.security.authentication.TestingAuthenticationToken;
@@ -29,15 +32,12 @@
 import org.springframework.security.core.userdetails.User;
 import org.springframework.util.ReflectionUtils;
 
-import java.lang.reflect.Method;
-
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
-import static org.junit.Assert.fail;
 
 /**
  * @author Dan Zheng
- * @since 5.2.x
+ * @since 5.2
  *
  */
 public class CurrentSecurityContextArgumentResolverTests {
@@ -64,6 +64,22 @@ public void supportsParameterAnnotation() throws Exception {
 		assertThat(resolver.supportsParameter(showSecurityContextAnnotation())).isTrue();
 	}
 
+	@Test
+	public void resolveArgumentWithCustomSecurityContext() throws Exception {
+		String principal = "custom_security_context";
+		setAuthenticationPrincipalWithCustomSecurityContext(principal);
+		CustomSecurityContext customSecurityContext = (CustomSecurityContext) resolver.resolveArgument(showAnnotationWithCustomSecurityContext(), null, null, null);
+		assertThat(customSecurityContext.getAuthentication().getPrincipal()).isEqualTo(principal);
+	}
+
+	@Test
+	public void resolveArgumentWithCustomSecurityContextTypeMatch() throws Exception {
+		String principal = "custom_security_context_type_match";
+		setAuthenticationPrincipalWithCustomSecurityContext(principal);
+		CustomSecurityContext customSecurityContext = (CustomSecurityContext) resolver.resolveArgument(showAnnotationWithCustomSecurityContext(), null, null, null);
+		assertThat(customSecurityContext.getAuthentication().getPrincipal()).isEqualTo(principal);
+	}
+
 	@Test
 	public void resolveArgumentNullAuthentication() throws Exception {
 		SecurityContext context = SecurityContextHolder.getContext();
@@ -142,10 +158,8 @@ public void resolveArgumentSecurityContextErrorOnInvalidTypeFalse() throws Excep
 	public void resolveArgumentSecurityContextErrorOnInvalidTypeTrue() throws Exception {
 		String principal = "invalid_type_true";
 		setAuthenticationPrincipal(principal);
-		try {
-			resolver.resolveArgument(showSecurityContextErrorOnInvalidTypeTrue(), null, null, null);
-			fail("should not reach here");
-		} catch(ClassCastException ex) {}
+		assertThatExceptionOfType(ClassCastException.class).isThrownBy(() -> resolver.resolveArgument(showSecurityContextErrorOnInvalidTypeTrue(), null,
+				null, null));
 	}
 
 	private MethodParameter showSecurityContextNoAnnotation() {
@@ -156,6 +170,14 @@ private MethodParameter showSecurityContextAnnotation() {
 		return getMethodParameter("showSecurityContextAnnotation", SecurityContext.class);
 	}
 
+	private MethodParameter showAnnotationWithCustomSecurityContext() {
+		return getMethodParameter("showAnnotationWithCustomSecurityContext", CustomSecurityContext.class);
+	}
+
+	private MethodParameter showAnnotationWithCustomSecurityContextTypeMatch() {
+		return getMethodParameter("showAnnotationWithCustomSecurityContextTypeMatch", SecurityContext.class);
+	}
+
 	private MethodParameter showSecurityContextAuthenticationAnnotation() {
 		return getMethodParameter("showSecurityContextAuthenticationAnnotation", Authentication.class);
 	}
@@ -197,6 +219,12 @@ public void showSecurityContextNoAnnotation(String user) {
 		public void showSecurityContextAnnotation(@CurrentSecurityContext SecurityContext context) {
 		}
 
+		public void showAnnotationWithCustomSecurityContext(@CurrentSecurityContext CustomSecurityContext context) {
+		}
+
+		public void showAnnotationWithCustomSecurityContextTypeMatch(@CurrentSecurityContext(errorOnInvalidType = true) SecurityContext context) {
+		}
+
 		public void showSecurityContextAuthenticationAnnotation(@CurrentSecurityContext(expression = "authentication") Authentication authentication) {
 		}
 
@@ -229,6 +257,26 @@ private void setAuthenticationPrincipal(Object principal) {
 								"ROLE_USER"));
 	}
 
+	private void setAuthenticationPrincipalWithCustomSecurityContext(Object principal) {
+		CustomSecurityContext csc = new CustomSecurityContext();
+		csc.setAuthentication(new TestingAuthenticationToken(principal, "password",
+				"ROLE_USER"));
+		SecurityContextHolder.setContext(csc);
+	}
+
+	static class CustomSecurityContext implements SecurityContext {
+		private Authentication authentication;
+		@Override
+		public Authentication getAuthentication() {
+			return authentication;
+		}
+
+		@Override
+		public void setAuthentication(Authentication authentication) {
+			this.authentication = authentication;
+		}
+	}
+
 	private void setAuthenticationDetail(Object detail) {
 		TestingAuthenticationToken tat = new TestingAuthenticationToken("user", "password",
 				"ROLE_USER");

web/src/test/java/org/springframework/security/web/reactive/result/method/annotation/CurrentSecurityContextArgumentResolverTests.java

@@ -21,6 +21,9 @@
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
 import org.mockito.junit.MockitoJUnitRunner;
+import reactor.core.publisher.Mono;
+import reactor.util.context.Context;
+
 import org.springframework.core.MethodParameter;
 import org.springframework.core.ReactiveAdapterRegistry;
 import org.springframework.expression.BeanResolver;
@@ -33,15 +36,14 @@
 import org.springframework.security.web.method.ResolvableMethod;
 import org.springframework.web.reactive.BindingContext;
 import org.springframework.web.server.ServerWebExchange;
-import reactor.core.publisher.Mono;
-import reactor.util.context.Context;
+
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.Assert.fail;
 
 
 /**
  * @author Dan Zheng
- * @since 5.2.x
+ * @since 5.2
  */
 @RunWith(MockitoJUnitRunner.class)
 public class CurrentSecurityContextArgumentResolverTests {
@@ -98,6 +100,17 @@ public void resolveArgumentWithSecurityContext() throws Exception {
 		ReactiveSecurityContextHolder.clearContext();
 	}
 
+	@Test
+	public void resolveArgumentWithCustomSecurityContext() throws Exception {
+		MethodParameter parameter = ResolvableMethod.on(getClass()).named("customSecurityContext").build().arg(Mono.class, SecurityContext.class);
+		Authentication auth = buildAuthenticationWithPrincipal("hello");
+		Context context = ReactiveSecurityContextHolder.withSecurityContext(Mono.just(new CustomSecurityContext(auth)));
+		Mono<Object> argument = resolver.resolveArgument(parameter, bindingContext, exchange);
+		CustomSecurityContext securityContext = (CustomSecurityContext) argument.subscriberContext(context).cast(Mono.class).block().block();
+		assertThat(securityContext.getAuthentication()).isSameAs(auth);
+		ReactiveSecurityContextHolder.clearContext();
+	}
+
 	@Test
 	public void resolveArgumentWithNullAuthentication1() throws Exception {
 		MethodParameter parameter = ResolvableMethod.on(getClass()).named("securityContext").build().arg(Mono.class, SecurityContext.class);
@@ -216,6 +229,8 @@ public void resolveArgumentErrorOnInvalidTypeWhenExplicitTrue() throws Exception
 
 	void securityContext(@CurrentSecurityContext Mono<SecurityContext> monoSecurityContext) {}
 
+	void customSecurityContext(@CurrentSecurityContext Mono<SecurityContext> monoSecurityContext) {}
+
 	void securityContextWithAuthentication(@CurrentSecurityContext(expression = "authentication") Mono<Authentication> authentication) {}
 
 	void securityContextWithDepthPropOptional(@CurrentSecurityContext(expression = "authentication?.principal") Mono<Object> principal) {}
@@ -230,7 +245,21 @@ void errorOnInvalidTypeWhenExplicitFalse(@CurrentSecurityContext(errorOnInvalidT
 
 	void errorOnInvalidTypeWhenExplicitTrue(@CurrentSecurityContext(errorOnInvalidType = true) Mono<String> implicit) {}
 
+	static class CustomSecurityContext implements SecurityContext {
+		private Authentication authentication;
+		public CustomSecurityContext(Authentication authentication) {
+			this.authentication = authentication;
+		}
+		@Override
+		public Authentication getAuthentication() {
+			return authentication;
+		}
 
+		@Override
+		public void setAuthentication(Authentication authentication) {
+			this.authentication = authentication;
+		}
+	}
 	private Authentication buildAuthenticationWithPrincipal(Object principal) {
 		return new TestingAuthenticationToken(principal, "password",
 				"ROLE_USER");