From 570eb01733cfcaed5696bb8ee5880e0be909ce3c Mon Sep 17 00:00:00 2001 From: Dan Zheng Date: Sat, 2 Mar 2019 12:28:22 +0800 Subject: [PATCH] review phase1 --- .../WebMvcSecurityConfiguration.java | 3 +- .../ServerHttpSecurityConfiguration.java | 3 +- .../annotation/CurrentSecurityContext.java | 26 +++----- ...urrentSecurityContextArgumentResolver.java | 8 +-- ...urrentSecurityContextArgumentResolver.java | 2 +- ...tSecurityContextArgumentResolverTests.java | 64 ++++++++++++++++--- ...tSecurityContextArgumentResolverTests.java | 35 +++++++++- 7 files changed, 106 insertions(+), 35 deletions(-) diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configuration/WebMvcSecurityConfiguration.java b/config/src/main/java/org/springframework/security/config/annotation/web/configuration/WebMvcSecurityConfiguration.java index 707e8775f7..3c49395b99 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configuration/WebMvcSecurityConfiguration.java +++ b/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 @@ import java.util.List; * {@link HandlerMethodArgumentResolver}. * * @author Rob Winch + * @author Dan Zheng * @since 3.2 */ class WebMvcSecurityConfiguration implements WebMvcConfigurer, ApplicationContextAware { diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/reactive/ServerHttpSecurityConfiguration.java b/config/src/main/java/org/springframework/security/config/annotation/web/reactive/ServerHttpSecurityConfiguration.java index ae7ed2ede0..8cc8378f09 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/reactive/ServerHttpSecurityConfiguration.java +++ b/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 @@ import org.springframework.web.reactive.result.method.annotation.ArgumentResolve /** * @author Rob Winch + * @author Dan Zheng * @since 5.0 */ @Configuration diff --git a/core/src/main/java/org/springframework/security/core/annotation/CurrentSecurityContext.java b/core/src/main/java/org/springframework/security/core/annotation/CurrentSecurityContext.java index fbbe2c95ed..da7596b559 100644 --- a/core/src/main/java/org/springframework/security/core/annotation/CurrentSecurityContext.java +++ b/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 @@ import java.lang.annotation.Target; * argument. * * @author Dan Zheng - * @since 5.2.x + * @since 5.2 * * See: + * @CurrentSecurityContext(expression = "authentication") Authentication authentication + * + * *

- * 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: + * if you want to retrieve more object from the authentcation, you can see the following the expression *

* * 
-	 * public class CustomUserUserDetails extends User {
-	 *     // ...
-	 *     public CustomUser getCustomUser() {
-	 *         return customUser;
-	 *     }
-	 * }
-	 *
- * - * Then the user can specify an annotation that looks like: - * - * 
-	 * @CurrentSecurityContext(expression = "authentication")
+	 * @CurrentSecurityContext(expression = "authentication.principal") Object principal
 	 *
* * @return the expression to use. diff --git a/web/src/main/java/org/springframework/security/web/bind/support/CurrentSecurityContextArgumentResolver.java b/web/src/main/java/org/springframework/security/web/bind/support/CurrentSecurityContextArgumentResolver.java index 634801f6f4..8a0832617c 100644 --- a/web/src/main/java/org/springframework/security/web/bind/support/CurrentSecurityContextArgumentResolver.java +++ b/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.context.request.NativeWebRequest; 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 @@ import java.lang.annotation.Annotation; *

* * @author Dan Zheng - * @since 5.2.x + * @since 5.2 */ public final class CurrentSecurityContextArgumentResolver implements HandlerMethodArgumentResolver { diff --git a/web/src/main/java/org/springframework/security/web/reactive/result/method/annotation/CurrentSecurityContextArgumentResolver.java b/web/src/main/java/org/springframework/security/web/reactive/result/method/annotation/CurrentSecurityContextArgumentResolver.java index 20a75e32bd..bb4c61b27d 100644 --- a/web/src/main/java/org/springframework/security/web/reactive/result/method/annotation/CurrentSecurityContextArgumentResolver.java +++ b/web/src/main/java/org/springframework/security/web/reactive/result/method/annotation/CurrentSecurityContextArgumentResolver.java @@ -40,7 +40,7 @@ import java.lang.annotation.Annotation; /** * Resolves the SecurityContext * @author Dan Zheng - * @since 5.2.x + * @since 5.2 */ public class CurrentSecurityContextArgumentResolver extends HandlerMethodArgumentResolverSupport { diff --git a/web/src/test/java/org/springframework/security/web/bind/support/CurrentSecurityContextArgumentResolverTests.java b/web/src/test/java/org/springframework/security/web/bind/support/CurrentSecurityContextArgumentResolverTests.java index e0256ea8fa..739880f9ad 100644 --- a/web/src/test/java/org/springframework/security/web/bind/support/CurrentSecurityContextArgumentResolverTests.java +++ b/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.context.SecurityContextHolder; 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 class CurrentSecurityContextArgumentResolverTests { 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 class CurrentSecurityContextArgumentResolverTests { 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 @@ public class CurrentSecurityContextArgumentResolverTests { 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 class CurrentSecurityContextArgumentResolverTests { 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 @@ public class CurrentSecurityContextArgumentResolverTests { "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"); diff --git a/web/src/test/java/org/springframework/security/web/reactive/result/method/annotation/CurrentSecurityContextArgumentResolverTests.java b/web/src/test/java/org/springframework/security/web/reactive/result/method/annotation/CurrentSecurityContextArgumentResolverTests.java index a34560d926..e044f6dc75 100644 --- a/web/src/test/java/org/springframework/security/web/reactive/result/method/annotation/CurrentSecurityContextArgumentResolverTests.java +++ b/web/src/test/java/org/springframework/security/web/reactive/result/method/annotation/CurrentSecurityContextArgumentResolverTests.java @@ -21,6 +21,9 @@ import org.junit.Test; 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.core.context.SecurityContext; 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 class CurrentSecurityContextArgumentResolverTests { 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 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 class CurrentSecurityContextArgumentResolverTests { void securityContext(@CurrentSecurityContext Mono monoSecurityContext) {} + void customSecurityContext(@CurrentSecurityContext Mono monoSecurityContext) {} + void securityContextWithAuthentication(@CurrentSecurityContext(expression = "authentication") Mono authentication) {} void securityContextWithDepthPropOptional(@CurrentSecurityContext(expression = "authentication?.principal") Mono principal) {} @@ -230,7 +245,21 @@ public class CurrentSecurityContextArgumentResolverTests { void errorOnInvalidTypeWhenExplicitTrue(@CurrentSecurityContext(errorOnInvalidType = true) Mono 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");