From e0fc4024469f0aa6d6cf5c3d3ba5ec0d73c226e5 Mon Sep 17 00:00:00 2001 From: Nena Raab Date: Tue, 5 May 2020 17:52:00 +0200 Subject: [PATCH 1/6] spring-security-test @WithMockOidcuser gh-8459 --- .../context/support/WithMockOidcUser.java | 103 +++++++++++++++++ ...ithMockOidcUserSecurityContextFactory.java | 92 ++++++++++++++++ ...hSecurityContextTestExecutionListener.java | 2 +- .../WithMockOidcUserSecurityContextTests.java | 104 ++++++++++++++++++ .../support/WithMockOidcUserTests.java | 71 ++++++++++++ 5 files changed, 371 insertions(+), 1 deletion(-) create mode 100644 test/src/main/java/org/springframework/security/test/context/support/WithMockOidcUser.java create mode 100644 test/src/main/java/org/springframework/security/test/context/support/WithMockOidcUserSecurityContextFactory.java create mode 100644 test/src/test/java/org/springframework/security/test/context/support/WithMockOidcUserSecurityContextTests.java create mode 100644 test/src/test/java/org/springframework/security/test/context/support/WithMockOidcUserTests.java diff --git a/test/src/main/java/org/springframework/security/test/context/support/WithMockOidcUser.java b/test/src/main/java/org/springframework/security/test/context/support/WithMockOidcUser.java new file mode 100644 index 00000000000..20ff7496ff1 --- /dev/null +++ b/test/src/main/java/org/springframework/security/test/context/support/WithMockOidcUser.java @@ -0,0 +1,103 @@ +/* + * Copyright 2002-2020 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.security.test.context.support; + +import org.springframework.core.annotation.AliasFor; +import org.springframework.security.core.GrantedAuthority; +import org.springframework.security.core.context.SecurityContext; +import org.springframework.security.oauth2.client.authentication.OAuth2AuthenticationToken; +import org.springframework.security.test.context.support.*; +import org.springframework.test.context.TestContext; +import org.springframework.test.web.servlet.MockMvc; + +import java.lang.annotation.*; + +/** + * When used with {@link WithSecurityContextTestExecutionListener} this annotation can be + * added to a test method to emulate running with a mocked user. In order to work with + * {@link MockMvc} The {@link SecurityContext} that is used will have the following + * properties: + * + * + * + * @author Nena Raab + * @see WithUserDetails + * @since 5.3 + */ +@Target({ ElementType.METHOD, ElementType.TYPE }) +@Retention(RetentionPolicy.RUNTIME) +@Inherited +@Documented +@WithSecurityContext(factory = WithMockOidcUserSecurityContextFactory.class) +public @interface WithMockOidcUser { + /** + * Convenience mechanism for specifying the username. The default is "user". If + * {@link #name()} is specified it will be used instead of {@link #value()} + * + * @return + */ + String value() default "user"; + + /** + * The user name oder user id (subject) to be used. Note that {@link #value()} is a synonym for + * {@link #name()}, but if {@link #name()} is specified it will take + * precedence. + * + * @return + */ + String name() default ""; + + /** + *

+ * The authorities to use. The default is "openid". A {@link GrantedAuthority} will be created for each value. + *

+ * * + * + * @return + */ + String[] authorities() default { "openid" }; + + /** + * The name of the OIDC token claim that contains the subject identifier that identifies the End-User. + * The default is "sub". + * + * @return + */ + String nameTokenClaim() default "sub"; + + /** + * The password to be used. The default is "clientId". + * + * @return + */ + String clientId() default "clientId"; + + /** + * Determines when the {@link SecurityContext} is setup. The default is before + * {@link TestExecutionEvent#TEST_METHOD} which occurs during + * {@link org.springframework.test.context.TestExecutionListener#beforeTestMethod(TestContext)} + * + * @return the {@link TestExecutionEvent} to initialize before + * @since 5.1 + */ + @AliasFor(annotation = WithSecurityContext.class) + TestExecutionEvent setupBefore() default TestExecutionEvent.TEST_METHOD; +} diff --git a/test/src/main/java/org/springframework/security/test/context/support/WithMockOidcUserSecurityContextFactory.java b/test/src/main/java/org/springframework/security/test/context/support/WithMockOidcUserSecurityContextFactory.java new file mode 100644 index 00000000000..7798f7df164 --- /dev/null +++ b/test/src/main/java/org/springframework/security/test/context/support/WithMockOidcUserSecurityContextFactory.java @@ -0,0 +1,92 @@ +/* + * Copyright 2002-2020 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.security.test.context.support; + +import org.springframework.security.core.Authentication; +import org.springframework.security.core.GrantedAuthority; +import org.springframework.security.core.authority.SimpleGrantedAuthority; +import org.springframework.security.core.context.SecurityContext; +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.security.oauth2.client.authentication.OAuth2AuthenticationToken; +import org.springframework.security.oauth2.core.oidc.OidcIdToken; +import org.springframework.security.oauth2.core.oidc.user.DefaultOidcUser; +import org.springframework.security.oauth2.core.oidc.user.OidcUser; +import org.springframework.util.StringUtils; + +import java.time.Instant; +import java.util.*; + +/** + * A {@link WithMockOidcUserSecurityContextFactory} that works with {@link WithMockOidcUser}. + *

+ * Initializes the Spring Security Context with a OAuth2AuthenticationToken instance, which comes with + * an encoded oidc token with some default claims but without header and signature. + * + * @author Nena Raab + * @see WithMockOidcUser + * @since 5.3 + */ +final class WithMockOidcUserSecurityContextFactory implements + WithSecurityContextFactory { + + public SecurityContext createSecurityContext(WithMockOidcUser withUser) { + String userId = StringUtils.hasLength(withUser.name()) ? withUser + .name() : withUser.value(); + if (userId == null) { + throw new IllegalArgumentException(withUser + + " cannot have null user name/id on both name and value properties"); + } + + List grantedAuthorities = new ArrayList<>(); + for (String authority : withUser.authorities()) { + grantedAuthorities.add(new SimpleGrantedAuthority(authority)); + } + + OidcUser principal = new DefaultOidcUser(grantedAuthorities, + new OidcIdTokenFactory(userId, withUser.clientId(), withUser.nameTokenClaim()).build(), + withUser.nameTokenClaim()); + + Authentication authentication = new OAuth2AuthenticationToken( + principal, principal.getAuthorities(), withUser.clientId()); + + SecurityContext context = SecurityContextHolder.createEmptyContext(); + context.setAuthentication(authentication); + return context; + } + + private class OidcIdTokenFactory { + private Map claims = new HashMap<>(); + final Instant expiredAt = new GregorianCalendar().toInstant().plusSeconds(600); + final Instant issuedAt = new GregorianCalendar().toInstant().minusSeconds(3); + + OidcIdTokenFactory(String userId, String clientId, String userIdClaimName) { + claims.put("client_id", clientId); // mandatory + claims.put("iat", issuedAt.getEpochSecond()); + claims.put("exp", expiredAt.getEpochSecond()); + claims.put(userIdClaimName, userId); + claims.put("email", userId + "@test.org"); + } + + public OidcIdToken build() { + return new OidcIdToken(emptyTokenBase64Encode(), issuedAt, expiredAt, claims); + } + + private String emptyTokenBase64Encode() { + byte[] emptyToken = "{}".getBytes(); + return Base64.getUrlEncoder().withoutPadding().encodeToString(emptyToken); + } + } +} diff --git a/test/src/main/java/org/springframework/security/test/context/support/WithSecurityContextTestExecutionListener.java b/test/src/main/java/org/springframework/security/test/context/support/WithSecurityContextTestExecutionListener.java index 72259a549bc..9221dec2ba5 100644 --- a/test/src/main/java/org/springframework/security/test/context/support/WithSecurityContextTestExecutionListener.java +++ b/test/src/main/java/org/springframework/security/test/context/support/WithSecurityContextTestExecutionListener.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2020 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. diff --git a/test/src/test/java/org/springframework/security/test/context/support/WithMockOidcUserSecurityContextTests.java b/test/src/test/java/org/springframework/security/test/context/support/WithMockOidcUserSecurityContextTests.java new file mode 100644 index 00000000000..26b5953a18b --- /dev/null +++ b/test/src/test/java/org/springframework/security/test/context/support/WithMockOidcUserSecurityContextTests.java @@ -0,0 +1,104 @@ +/* + * Copyright 2002-2020 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.security.test.context.support; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.*; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; +import org.springframework.security.oauth2.core.oidc.user.OidcUser; + +import java.time.Instant; + +@RunWith(MockitoJUnitRunner.class) +public class WithMockOidcUserSecurityContextTests { + + @Mock + private WithMockOidcUser withUser; + + private WithMockOidcUserSecurityContextFactory factory; + + @Before + public void setup() { + factory = new WithMockOidcUserSecurityContextFactory(); + when(withUser.value()).thenReturn("valueUser"); + when(withUser.clientId()).thenReturn("clientid"); + when(withUser.authorities()).thenReturn(new String[] { "openid" }); + when(withUser.nameTokenClaim()).thenReturn("sub"); + when(withUser.name()).thenReturn(""); + } + + @Test(expected = IllegalArgumentException.class) + public void userNameValueNullRaiseException() { + when(withUser.value()).thenReturn(null); + factory.createSecurityContext(withUser); + } + + public void valueDefaultsUserIdWhenUserNameIsNull() { + when(withUser.name()).thenReturn(null); + assertThat(factory.createSecurityContext(withUser).getAuthentication().getName()) + .isEqualTo("valueUser"); + } + + @Test + public void valueDefaultsUserIdWhenUserNameIsNotSet() { + assertThat(factory.createSecurityContext(withUser).getAuthentication().getName()) + .isEqualTo("valueUser"); + } + + @Test + public void userNamePrioritizedOverValue() { + when(withUser.name()).thenReturn("customUser"); + + assertThat(factory.createSecurityContext(withUser).getAuthentication().getName()) + .isEqualTo("customUser"); + } + + @Test + public void overwriteAuthorities() { + when(withUser.authorities()).thenReturn(new String[] { "USER", "CUSTOM" }); + + assertThat( + factory.createSecurityContext(withUser).getAuthentication() + .getAuthorities()).extracting("authority").containsOnly( + "USER", "CUSTOM"); + } + + @Test + public void overwriteNameTokenClaim() { + when(withUser.nameTokenClaim()).thenReturn("userNameClaim"); + + Object authn = factory.createSecurityContext(withUser).getAuthentication().getPrincipal(); + assertThat(authn).isInstanceOf(OidcUser.class); + assertThat(((OidcUser) authn).getClaims()).containsKey("userNameClaim"); + assertThat(((OidcUser) authn).getClaims()).doesNotContainKey("sub"); + assertThat(((OidcUser) authn).getName()).isEqualTo("valueUser"); + } + + @Test + public void claimNotExpired() { + when(withUser.nameTokenClaim()).thenReturn("userNameClaim"); + + OidcUser oidcUser = (OidcUser) factory.createSecurityContext(withUser).getAuthentication().getPrincipal(); + assertThat(oidcUser.getIssuedAt().compareTo(Instant.now())).isNegative(); + assertThat(oidcUser.getExpiresAt().compareTo(Instant.now())).isPositive(); + } + +} diff --git a/test/src/test/java/org/springframework/security/test/context/support/WithMockOidcUserTests.java b/test/src/test/java/org/springframework/security/test/context/support/WithMockOidcUserTests.java new file mode 100644 index 00000000000..d7014d12ec8 --- /dev/null +++ b/test/src/test/java/org/springframework/security/test/context/support/WithMockOidcUserTests.java @@ -0,0 +1,71 @@ +/* + * Copyright 2002-2020 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.security.test.context.support; + +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.Test; +import org.springframework.core.annotation.AnnotatedElementUtils; +import org.springframework.security.test.context.support.TestExecutionEvent; +import org.springframework.security.test.context.support.WithSecurityContext; + +public class WithMockOidcUserTests { + + @Test + public void defaults() { + WithMockOidcUser mockUser = AnnotatedElementUtils.findMergedAnnotation(Annotated.class, + WithMockOidcUser.class); + assertThat(mockUser.value()).isEqualTo("user"); + assertThat(mockUser.name()).isEmpty(); + assertThat(mockUser.authorities()).containsOnly("openid"); + assertThat(mockUser.clientId()).isEqualTo("clientId"); + assertThat(mockUser.setupBefore()).isEqualByComparingTo(TestExecutionEvent.TEST_METHOD); + + WithSecurityContext context = AnnotatedElementUtils.findMergedAnnotation(Annotated.class, + WithSecurityContext.class); + + assertThat(context.setupBefore()).isEqualTo(TestExecutionEvent.TEST_METHOD); + } + + @WithMockOidcUser + private class Annotated { + } + + @Test + public void findMergedAnnotationWhenSetupExplicitThenOverridden() { + WithSecurityContext context = AnnotatedElementUtils + .findMergedAnnotation(SetupExplicit.class, + WithSecurityContext.class); + + assertThat(context.setupBefore()).isEqualTo(TestExecutionEvent.TEST_METHOD); + } + + @WithMockOidcUser(setupBefore = TestExecutionEvent.TEST_METHOD) + private class SetupExplicit { + } + + @Test + public void findMergedAnnotationWhenSetupOverriddenThenOverridden() { + WithSecurityContext context = AnnotatedElementUtils.findMergedAnnotation(SetupOverridden.class, + WithSecurityContext.class); + + assertThat(context.setupBefore()).isEqualTo(TestExecutionEvent.TEST_EXECUTION); + } + + @WithMockOidcUser(setupBefore = TestExecutionEvent.TEST_EXECUTION) + private class SetupOverridden { + } +} From 75adac77595a19c6205a136017d4cc88967cae5c Mon Sep 17 00:00:00 2001 From: Nena Raab Date: Tue, 5 May 2020 18:41:07 +0200 Subject: [PATCH 2/6] undo unwanted change --- .../support/WithSecurityContextTestExecutionListener.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/src/main/java/org/springframework/security/test/context/support/WithSecurityContextTestExecutionListener.java b/test/src/main/java/org/springframework/security/test/context/support/WithSecurityContextTestExecutionListener.java index 9221dec2ba5..72259a549bc 100644 --- a/test/src/main/java/org/springframework/security/test/context/support/WithSecurityContextTestExecutionListener.java +++ b/test/src/main/java/org/springframework/security/test/context/support/WithSecurityContextTestExecutionListener.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2018 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. From 9e6091f74cade0bcf6317160307b1431e0956501 Mon Sep 17 00:00:00 2001 From: Nena Raab Date: Wed, 6 May 2020 10:55:29 +0200 Subject: [PATCH 3/6] cleanup imports, add test, remove email claim --- .../security/test/context/support/WithMockOidcUser.java | 1 - .../support/WithMockOidcUserSecurityContextFactory.java | 1 - .../security/test/context/support/WithMockOidcUserTests.java | 3 +-- 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/test/src/main/java/org/springframework/security/test/context/support/WithMockOidcUser.java b/test/src/main/java/org/springframework/security/test/context/support/WithMockOidcUser.java index 20ff7496ff1..3cf450bebf3 100644 --- a/test/src/main/java/org/springframework/security/test/context/support/WithMockOidcUser.java +++ b/test/src/main/java/org/springframework/security/test/context/support/WithMockOidcUser.java @@ -19,7 +19,6 @@ import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.context.SecurityContext; import org.springframework.security.oauth2.client.authentication.OAuth2AuthenticationToken; -import org.springframework.security.test.context.support.*; import org.springframework.test.context.TestContext; import org.springframework.test.web.servlet.MockMvc; diff --git a/test/src/main/java/org/springframework/security/test/context/support/WithMockOidcUserSecurityContextFactory.java b/test/src/main/java/org/springframework/security/test/context/support/WithMockOidcUserSecurityContextFactory.java index 7798f7df164..8387d74b102 100644 --- a/test/src/main/java/org/springframework/security/test/context/support/WithMockOidcUserSecurityContextFactory.java +++ b/test/src/main/java/org/springframework/security/test/context/support/WithMockOidcUserSecurityContextFactory.java @@ -77,7 +77,6 @@ private class OidcIdTokenFactory { claims.put("iat", issuedAt.getEpochSecond()); claims.put("exp", expiredAt.getEpochSecond()); claims.put(userIdClaimName, userId); - claims.put("email", userId + "@test.org"); } public OidcIdToken build() { diff --git a/test/src/test/java/org/springframework/security/test/context/support/WithMockOidcUserTests.java b/test/src/test/java/org/springframework/security/test/context/support/WithMockOidcUserTests.java index d7014d12ec8..cd7c3e3ce96 100644 --- a/test/src/test/java/org/springframework/security/test/context/support/WithMockOidcUserTests.java +++ b/test/src/test/java/org/springframework/security/test/context/support/WithMockOidcUserTests.java @@ -19,8 +19,6 @@ import org.junit.Test; import org.springframework.core.annotation.AnnotatedElementUtils; -import org.springframework.security.test.context.support.TestExecutionEvent; -import org.springframework.security.test.context.support.WithSecurityContext; public class WithMockOidcUserTests { @@ -32,6 +30,7 @@ public void defaults() { assertThat(mockUser.name()).isEmpty(); assertThat(mockUser.authorities()).containsOnly("openid"); assertThat(mockUser.clientId()).isEqualTo("clientId"); + assertThat(mockUser.nameTokenClaim()).isEqualTo("sub"); assertThat(mockUser.setupBefore()).isEqualByComparingTo(TestExecutionEvent.TEST_METHOD); WithSecurityContext context = AnnotatedElementUtils.findMergedAnnotation(Annotated.class, From 6056ee302fb2c20b61ddcee392a63e198603d37c Mon Sep 17 00:00:00 2001 From: Nena Raab Date: Fri, 8 May 2020 14:53:12 +0200 Subject: [PATCH 4/6] the OidcUser should have by default SCOPE_openid as authority --- .../test/context/support/WithMockOidcUser.java | 15 ++++++--------- .../WithMockOidcUserSecurityContextFactory.java | 2 +- .../WithMockOidcUserSecurityContextTests.java | 8 ++++---- .../context/support/WithMockOidcUserTests.java | 2 +- 4 files changed, 12 insertions(+), 15 deletions(-) diff --git a/test/src/main/java/org/springframework/security/test/context/support/WithMockOidcUser.java b/test/src/main/java/org/springframework/security/test/context/support/WithMockOidcUser.java index 3cf450bebf3..2595ee25801 100644 --- a/test/src/main/java/org/springframework/security/test/context/support/WithMockOidcUser.java +++ b/test/src/main/java/org/springframework/security/test/context/support/WithMockOidcUser.java @@ -47,10 +47,11 @@ @Documented @WithSecurityContext(factory = WithMockOidcUserSecurityContextFactory.class) public @interface WithMockOidcUser { + String DEFAULT_SCOPE = "SCOPE_openid"; + /** * Convenience mechanism for specifying the username. The default is "user". If * {@link #name()} is specified it will be used instead of {@link #value()} - * * @return */ String value() default "user"; @@ -59,32 +60,29 @@ * The user name oder user id (subject) to be used. Note that {@link #value()} is a synonym for * {@link #name()}, but if {@link #name()} is specified it will take * precedence. - * * @return */ String name() default ""; /** *

- * The authorities to use. The default is "openid". A {@link GrantedAuthority} will be created for each value. + * The authorities, that should be mapped to {@code GrantedAuthority}. + * The default is "SCOPE_openid". A {@link GrantedAuthority} will be created for each value. *

- * * - * + ** * @return */ - String[] authorities() default { "openid" }; + String[] authorities() default { "SCOPE_openid" }; /** * The name of the OIDC token claim that contains the subject identifier that identifies the End-User. * The default is "sub". - * * @return */ String nameTokenClaim() default "sub"; /** * The password to be used. The default is "clientId". - * * @return */ String clientId() default "clientId"; @@ -93,7 +91,6 @@ * Determines when the {@link SecurityContext} is setup. The default is before * {@link TestExecutionEvent#TEST_METHOD} which occurs during * {@link org.springframework.test.context.TestExecutionListener#beforeTestMethod(TestContext)} - * * @return the {@link TestExecutionEvent} to initialize before * @since 5.1 */ diff --git a/test/src/main/java/org/springframework/security/test/context/support/WithMockOidcUserSecurityContextFactory.java b/test/src/main/java/org/springframework/security/test/context/support/WithMockOidcUserSecurityContextFactory.java index 8387d74b102..a6d43901e20 100644 --- a/test/src/main/java/org/springframework/security/test/context/support/WithMockOidcUserSecurityContextFactory.java +++ b/test/src/main/java/org/springframework/security/test/context/support/WithMockOidcUserSecurityContextFactory.java @@ -47,7 +47,7 @@ public SecurityContext createSecurityContext(WithMockOidcUser withUser) { .name() : withUser.value(); if (userId == null) { throw new IllegalArgumentException(withUser - + " cannot have null user name/id on both name and value properties"); + + " cannot have null userId on both userId and value properties"); } List grantedAuthorities = new ArrayList<>(); diff --git a/test/src/test/java/org/springframework/security/test/context/support/WithMockOidcUserSecurityContextTests.java b/test/src/test/java/org/springframework/security/test/context/support/WithMockOidcUserSecurityContextTests.java index 26b5953a18b..66cbf9cc870 100644 --- a/test/src/test/java/org/springframework/security/test/context/support/WithMockOidcUserSecurityContextTests.java +++ b/test/src/test/java/org/springframework/security/test/context/support/WithMockOidcUserSecurityContextTests.java @@ -39,8 +39,8 @@ public class WithMockOidcUserSecurityContextTests { public void setup() { factory = new WithMockOidcUserSecurityContextFactory(); when(withUser.value()).thenReturn("valueUser"); - when(withUser.clientId()).thenReturn("clientid"); - when(withUser.authorities()).thenReturn(new String[] { "openid" }); + when(withUser.clientId()).thenReturn("clientId"); + when(withUser.authorities()).thenReturn(new String[] { WithMockOidcUser.DEFAULT_SCOPE }); when(withUser.nameTokenClaim()).thenReturn("sub"); when(withUser.name()).thenReturn(""); } @@ -87,8 +87,8 @@ public void overwriteNameTokenClaim() { Object authn = factory.createSecurityContext(withUser).getAuthentication().getPrincipal(); assertThat(authn).isInstanceOf(OidcUser.class); - assertThat(((OidcUser) authn).getClaims()).containsKey("userNameClaim"); - assertThat(((OidcUser) authn).getClaims()).doesNotContainKey("sub"); + assertThat(((OidcUser)authn).getClaims()).containsKey("userNameClaim"); + assertThat(((OidcUser)authn).getClaims()).doesNotContainKey("sub"); assertThat(((OidcUser) authn).getName()).isEqualTo("valueUser"); } diff --git a/test/src/test/java/org/springframework/security/test/context/support/WithMockOidcUserTests.java b/test/src/test/java/org/springframework/security/test/context/support/WithMockOidcUserTests.java index cd7c3e3ce96..837f0a4b8aa 100644 --- a/test/src/test/java/org/springframework/security/test/context/support/WithMockOidcUserTests.java +++ b/test/src/test/java/org/springframework/security/test/context/support/WithMockOidcUserTests.java @@ -28,7 +28,7 @@ public void defaults() { WithMockOidcUser.class); assertThat(mockUser.value()).isEqualTo("user"); assertThat(mockUser.name()).isEmpty(); - assertThat(mockUser.authorities()).containsOnly("openid"); + assertThat(mockUser.authorities()).containsOnly(WithMockOidcUser.DEFAULT_SCOPE); assertThat(mockUser.clientId()).isEqualTo("clientId"); assertThat(mockUser.nameTokenClaim()).isEqualTo("sub"); assertThat(mockUser.setupBefore()).isEqualByComparingTo(TestExecutionEvent.TEST_METHOD); From 1f043cfdc780a420b4d1c6cf8be963fd021b2321 Mon Sep 17 00:00:00 2001 From: Nena Raab Date: Fri, 8 May 2020 14:54:36 +0200 Subject: [PATCH 5/6] checkstyle issue --- .../support/WithMockOidcUserSecurityContextTests.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/src/test/java/org/springframework/security/test/context/support/WithMockOidcUserSecurityContextTests.java b/test/src/test/java/org/springframework/security/test/context/support/WithMockOidcUserSecurityContextTests.java index 66cbf9cc870..0e6faf7a93e 100644 --- a/test/src/test/java/org/springframework/security/test/context/support/WithMockOidcUserSecurityContextTests.java +++ b/test/src/test/java/org/springframework/security/test/context/support/WithMockOidcUserSecurityContextTests.java @@ -81,14 +81,15 @@ public void overwriteAuthorities() { "USER", "CUSTOM"); } + @SuppressWarnings("checkstyle:WhitespaceAfter") @Test public void overwriteNameTokenClaim() { when(withUser.nameTokenClaim()).thenReturn("userNameClaim"); Object authn = factory.createSecurityContext(withUser).getAuthentication().getPrincipal(); assertThat(authn).isInstanceOf(OidcUser.class); - assertThat(((OidcUser)authn).getClaims()).containsKey("userNameClaim"); - assertThat(((OidcUser)authn).getClaims()).doesNotContainKey("sub"); + assertThat(((OidcUser) authn).getClaims()).containsKey("userNameClaim"); + assertThat(((OidcUser) authn).getClaims()).doesNotContainKey("sub"); assertThat(((OidcUser) authn).getName()).isEqualTo("valueUser"); } From 9fe2069d564ef034359cf3a12fce7b7975cf5df3 Mon Sep 17 00:00:00 2001 From: Nena Raab Date: Thu, 28 May 2020 17:04:07 +0200 Subject: [PATCH 6/6] incorporated findings from review (Josh Cummings), final cleanup thanks for the detailed and valuable review! --- .../context/support/WithMockOidcUser.java | 41 +++++++------ ...ithMockOidcUserSecurityContextFactory.java | 56 ++++++++--------- .../WithMockOidcUserSecurityContextTests.java | 60 ++++++++++--------- .../support/WithMockOidcUserTests.java | 5 +- 4 files changed, 82 insertions(+), 80 deletions(-) diff --git a/test/src/main/java/org/springframework/security/test/context/support/WithMockOidcUser.java b/test/src/main/java/org/springframework/security/test/context/support/WithMockOidcUser.java index 2595ee25801..5dff228da52 100644 --- a/test/src/main/java/org/springframework/security/test/context/support/WithMockOidcUser.java +++ b/test/src/main/java/org/springframework/security/test/context/support/WithMockOidcUser.java @@ -33,13 +33,14 @@ *
    *
  • The Authentication that is populated in the {@link SecurityContext} is of type {@link OAuth2AuthenticationToken}.
    • *
  • The principal on the Authentication is Spring Security’s User object of type {@code OidcUser}.
    • - *
  • The default User has the username of "user", {@link #value()} or {@link #name()} and does not have to exist.
    • - *
  • The default User has and a single GrantedAuthority {@link GrantedAuthority} or those that are specified by {@link #authorities()}.
    • + *
  • The default User has the user name "user". You can overwrite it the name with {@link #value()} or {@link #name()}.
    • + *
  • The default User has "ROLE_USER" and "SCOPE_openid" as {@link GrantedAuthority}. + * You can overwrite them with {@link #scopes()} or {@link #authorities()}.
    • *
* * @author Nena Raab * @see WithUserDetails - * @since 5.3 + * @since 5.4 */ @Target({ ElementType.METHOD, ElementType.TYPE }) @Retention(RetentionPolicy.RUNTIME) @@ -47,7 +48,6 @@ @Documented @WithSecurityContext(factory = WithMockOidcUserSecurityContextFactory.class) public @interface WithMockOidcUser { - String DEFAULT_SCOPE = "SCOPE_openid"; /** * Convenience mechanism for specifying the username. The default is "user". If @@ -57,7 +57,7 @@ String value() default "user"; /** - * The user name oder user id (subject) to be used. Note that {@link #value()} is a synonym for + * The user name or user id (subject) to be used. Note that {@link #value()} is a synonym for * {@link #name()}, but if {@link #name()} is specified it will take * precedence. * @return @@ -66,33 +66,36 @@ /** *

- * The authorities, that should be mapped to {@code GrantedAuthority}. - * The default is "SCOPE_openid". A {@link GrantedAuthority} will be created for each value. + * The scopes that should be mapped to {@code GrantedAuthority}. + * The default is "openid". Each value in scopes gets prefixed with "SCOPE_" + * and added to the list of {@link GrantedAuthority}. *

- ** - * @return - */ - String[] authorities() default { "SCOPE_openid" }; - - /** - * The name of the OIDC token claim that contains the subject identifier that identifies the End-User. - * The default is "sub". + *

+ * If {@link #authorities()} is specified this property cannot be changed from the default. + *

+ * * @return */ - String nameTokenClaim() default "sub"; + String[] scopes() default { "openid" }; /** - * The password to be used. The default is "clientId". + *

+ * The authorities that should be mapped to {@code GrantedAuthority}. + *

+ * + *

+ * If this property is specified then {@link #scopes()} is not used. This differs from + * {@link #scopes()} in that it does not prefix the values passed in automatically. + *

* @return */ - String clientId() default "clientId"; + String[] authorities() default { }; /** * Determines when the {@link SecurityContext} is setup. The default is before * {@link TestExecutionEvent#TEST_METHOD} which occurs during * {@link org.springframework.test.context.TestExecutionListener#beforeTestMethod(TestContext)} * @return the {@link TestExecutionEvent} to initialize before - * @since 5.1 */ @AliasFor(annotation = WithSecurityContext.class) TestExecutionEvent setupBefore() default TestExecutionEvent.TEST_METHOD; diff --git a/test/src/main/java/org/springframework/security/test/context/support/WithMockOidcUserSecurityContextFactory.java b/test/src/main/java/org/springframework/security/test/context/support/WithMockOidcUserSecurityContextFactory.java index a6d43901e20..1d39fb717f6 100644 --- a/test/src/main/java/org/springframework/security/test/context/support/WithMockOidcUserSecurityContextFactory.java +++ b/test/src/main/java/org/springframework/security/test/context/support/WithMockOidcUserSecurityContextFactory.java @@ -24,20 +24,20 @@ import org.springframework.security.oauth2.core.oidc.OidcIdToken; import org.springframework.security.oauth2.core.oidc.user.DefaultOidcUser; import org.springframework.security.oauth2.core.oidc.user.OidcUser; +import org.springframework.security.oauth2.core.oidc.user.OidcUserAuthority; +import org.springframework.util.Assert; import org.springframework.util.StringUtils; import java.time.Instant; import java.util.*; +import static org.springframework.security.oauth2.core.oidc.IdTokenClaimNames.*; + /** - * A {@link WithMockOidcUserSecurityContextFactory} that works with {@link WithMockOidcUser}. - *

- * Initializes the Spring Security Context with a OAuth2AuthenticationToken instance, which comes with - * an encoded oidc token with some default claims but without header and signature. + * Initializes the Spring Security Context with a OAuth2AuthenticationToken instance. * * @author Nena Raab * @see WithMockOidcUser - * @since 5.3 */ final class WithMockOidcUserSecurityContextFactory implements WithSecurityContextFactory { @@ -46,46 +46,42 @@ public SecurityContext createSecurityContext(WithMockOidcUser withUser) { String userId = StringUtils.hasLength(withUser.name()) ? withUser .name() : withUser.value(); if (userId == null) { - throw new IllegalArgumentException(withUser - + " cannot have null userId on both userId and value properties"); + Assert.notNull(userId, "@WithMockOidcUser cannot have null name on both name and value properties"); } - List grantedAuthorities = new ArrayList<>(); + Set grantedAuthorities = new HashSet<>(); for (String authority : withUser.authorities()) { grantedAuthorities.add(new SimpleGrantedAuthority(authority)); } - OidcUser principal = new DefaultOidcUser(grantedAuthorities, - new OidcIdTokenFactory(userId, withUser.clientId(), withUser.nameTokenClaim()).build(), - withUser.nameTokenClaim()); + if (grantedAuthorities.isEmpty()) { + for (String scope : withUser.scopes()) { + Assert.isTrue(!scope.startsWith("SCOPE_"), "scopes cannot start with SCOPE_ got " + scope); + grantedAuthorities.add(new SimpleGrantedAuthority("SCOPE_" + scope)); + } + } + // To align this with the OidcUser that OidcUserService creates, this adds a ROLE_USER + grantedAuthorities.add(new OidcUserAuthority(getOidcTokenForUser(userId))); + + OidcUser principal = new DefaultOidcUser(grantedAuthorities, getOidcTokenForUser(userId)); Authentication authentication = new OAuth2AuthenticationToken( - principal, principal.getAuthorities(), withUser.clientId()); + principal, principal.getAuthorities(), "client-id"); SecurityContext context = SecurityContextHolder.createEmptyContext(); context.setAuthentication(authentication); return context; } - private class OidcIdTokenFactory { - private Map claims = new HashMap<>(); - final Instant expiredAt = new GregorianCalendar().toInstant().plusSeconds(600); - final Instant issuedAt = new GregorianCalendar().toInstant().minusSeconds(3); + private static OidcIdToken getOidcTokenForUser(String userId) { + Map claims = new HashMap<>(); + final Instant issuedAt = Instant.now().minusSeconds(3); + final Instant expiredAt = Instant.now().plusSeconds(600); - OidcIdTokenFactory(String userId, String clientId, String userIdClaimName) { - claims.put("client_id", clientId); // mandatory - claims.put("iat", issuedAt.getEpochSecond()); - claims.put("exp", expiredAt.getEpochSecond()); - claims.put(userIdClaimName, userId); - } + claims.put(IAT, issuedAt.getEpochSecond()); + claims.put(EXP, expiredAt.getEpochSecond()); + claims.put(SUB, userId); - public OidcIdToken build() { - return new OidcIdToken(emptyTokenBase64Encode(), issuedAt, expiredAt, claims); - } - - private String emptyTokenBase64Encode() { - byte[] emptyToken = "{}".getBytes(); - return Base64.getUrlEncoder().withoutPadding().encodeToString(emptyToken); - } + return new OidcIdToken("id-token", issuedAt, expiredAt, claims); } } diff --git a/test/src/test/java/org/springframework/security/test/context/support/WithMockOidcUserSecurityContextTests.java b/test/src/test/java/org/springframework/security/test/context/support/WithMockOidcUserSecurityContextTests.java index 0e6faf7a93e..87319afcc15 100644 --- a/test/src/test/java/org/springframework/security/test/context/support/WithMockOidcUserSecurityContextTests.java +++ b/test/src/test/java/org/springframework/security/test/context/support/WithMockOidcUserSecurityContextTests.java @@ -29,6 +29,7 @@ @RunWith(MockitoJUnitRunner.class) public class WithMockOidcUserSecurityContextTests { + private final static String USER_VALUE = "valueUser"; @Mock private WithMockOidcUser withUser; @@ -38,68 +39,71 @@ public class WithMockOidcUserSecurityContextTests { @Before public void setup() { factory = new WithMockOidcUserSecurityContextFactory(); - when(withUser.value()).thenReturn("valueUser"); - when(withUser.clientId()).thenReturn("clientId"); - when(withUser.authorities()).thenReturn(new String[] { WithMockOidcUser.DEFAULT_SCOPE }); - when(withUser.nameTokenClaim()).thenReturn("sub"); + when(withUser.value()).thenReturn(USER_VALUE); + when(withUser.authorities()).thenReturn(new String[]{}); + when(withUser.scopes()).thenReturn(new String[]{"openid"}); when(withUser.name()).thenReturn(""); } @Test(expected = IllegalArgumentException.class) - public void userNameValueNullRaiseException() { + public void createSecurityContextWhenValueIsNullThenRaiseException() { when(withUser.value()).thenReturn(null); factory.createSecurityContext(withUser); } - public void valueDefaultsUserIdWhenUserNameIsNull() { + @Test + public void createSecurityContextWhenUserNameIsNullThenUseDefaultValue() { when(withUser.name()).thenReturn(null); assertThat(factory.createSecurityContext(withUser).getAuthentication().getName()) - .isEqualTo("valueUser"); + .isEqualTo(USER_VALUE); } @Test - public void valueDefaultsUserIdWhenUserNameIsNotSet() { + public void createSecurityContextWhenUserNameIsEmptyThenUseDefaultValue() { assertThat(factory.createSecurityContext(withUser).getAuthentication().getName()) - .isEqualTo("valueUser"); + .isEqualTo(USER_VALUE); } @Test - public void userNamePrioritizedOverValue() { - when(withUser.name()).thenReturn("customUser"); + public void createSecurityContextWhenUserNameIsSetThenUseUserName() { + when(withUser.name()).thenReturn(USER_VALUE); assertThat(factory.createSecurityContext(withUser).getAuthentication().getName()) - .isEqualTo("customUser"); + .isEqualTo(USER_VALUE); } @Test - public void overwriteAuthorities() { - when(withUser.authorities()).thenReturn(new String[] { "USER", "CUSTOM" }); + public void createSecurityContextWhenAuthoritiesSetThenUseAuthorities() { + when(withUser.authorities()).thenReturn(new String[]{"USER", "CUSTOM", "ROLE_USER"}); assertThat( factory.createSecurityContext(withUser).getAuthentication() - .getAuthorities()).extracting("authority").containsOnly( - "USER", "CUSTOM"); + .getAuthorities()).extracting("authority").containsExactlyInAnyOrder( + "USER", "CUSTOM", "ROLE_USER"); } - @SuppressWarnings("checkstyle:WhitespaceAfter") @Test - public void overwriteNameTokenClaim() { - when(withUser.nameTokenClaim()).thenReturn("userNameClaim"); - - Object authn = factory.createSecurityContext(withUser).getAuthentication().getPrincipal(); - assertThat(authn).isInstanceOf(OidcUser.class); - assertThat(((OidcUser) authn).getClaims()).containsKey("userNameClaim"); - assertThat(((OidcUser) authn).getClaims()).doesNotContainKey("sub"); - assertThat(((OidcUser) authn).getName()).isEqualTo("valueUser"); + public void createSecurityContextWhenNoScopesAndAuthoritiesSetThenUseDefaultScope() { + assertThat( + factory.createSecurityContext(withUser).getAuthentication() + .getAuthorities()).extracting("authority").containsExactlyInAnyOrder( + "SCOPE_openid", "ROLE_USER"); } @Test - public void claimNotExpired() { - when(withUser.nameTokenClaim()).thenReturn("userNameClaim"); + public void createSecurityContextWhenScopesSetThenUseScopes() { + when(withUser.scopes()).thenReturn(new String[]{"DISPLAY", "ADMIN"}); + assertThat( + factory.createSecurityContext(withUser).getAuthentication() + .getAuthorities()).extracting("authority").containsExactlyInAnyOrder( + "SCOPE_DISPLAY", "SCOPE_ADMIN", "ROLE_USER"); + } + + @Test + public void createSecurityContextThenOidcNotYetExpired() { OidcUser oidcUser = (OidcUser) factory.createSecurityContext(withUser).getAuthentication().getPrincipal(); assertThat(oidcUser.getIssuedAt().compareTo(Instant.now())).isNegative(); assertThat(oidcUser.getExpiresAt().compareTo(Instant.now())).isPositive(); } - } diff --git a/test/src/test/java/org/springframework/security/test/context/support/WithMockOidcUserTests.java b/test/src/test/java/org/springframework/security/test/context/support/WithMockOidcUserTests.java index 837f0a4b8aa..f3d002b5c5c 100644 --- a/test/src/test/java/org/springframework/security/test/context/support/WithMockOidcUserTests.java +++ b/test/src/test/java/org/springframework/security/test/context/support/WithMockOidcUserTests.java @@ -28,9 +28,8 @@ public void defaults() { WithMockOidcUser.class); assertThat(mockUser.value()).isEqualTo("user"); assertThat(mockUser.name()).isEmpty(); - assertThat(mockUser.authorities()).containsOnly(WithMockOidcUser.DEFAULT_SCOPE); - assertThat(mockUser.clientId()).isEqualTo("clientId"); - assertThat(mockUser.nameTokenClaim()).isEqualTo("sub"); + assertThat(mockUser.scopes()).containsOnly("openid"); + assertThat(mockUser.authorities()).isEmpty(); assertThat(mockUser.setupBefore()).isEqualByComparingTo(TestExecutionEvent.TEST_METHOD); WithSecurityContext context = AnnotatedElementUtils.findMergedAnnotation(Annotated.class,