Skip to content

Commit

Permalink
Use new DefaultOidcUser constructors
Browse files Browse the repository at this point in the history 
OidcUserRequestUtils.getUser remains; requires usage of inaccessible OidcUserAuthority.collectClaims.
  • Loading branch information
@andreblanke
andreblanke committed Dec 8, 2024
1 parent efbc5fe commit 4848c6b
Show file tree
Hide file tree
Showing 10 changed files with 32 additions and 32 deletions.
2 changes: 1 addition & 1 deletion .../security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurerTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -717,7 +717,7 @@ private static OAuth2UserService<OAuth2UserRequest, OAuth2User> createOauth2User

private static OAuth2UserService<OidcUserRequest, OidcUser> createOidcUserService() {
OidcIdToken idToken = TestOidcIdTokens.idToken().build();
return (request) -> new DefaultOidcUser(Collections.singleton(new OidcUserAuthority(idToken)), idToken);
return (request) -> new DefaultOidcUser(idToken, Collections.singleton(new OidcUserAuthority(idToken)));
}

private static GrantedAuthoritiesMapper createGrantedAuthoritiesMapper() {
Expand Down
2 changes: 1 addition & 1 deletion ...n/java/org/springframework/security/oauth2/client/oidc/userinfo/OidcUserRequestUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ static OidcUser getUser(OidcUserRequest userRequest, OidcUserInfo userInfo) {
if (StringUtils.hasText(userNameAttributeName)) {
return new DefaultOidcUser(authorities, userRequest.getIdToken(), userInfo, userNameAttributeName);
}
return new DefaultOidcUser(authorities, userRequest.getIdToken(), userInfo);
return new DefaultOidcUser(userRequest.getIdToken(), userInfo, authorities);
}

private OidcUserRequestUtils() {
Expand Down
4 changes: 2 additions & 2 deletions .../springframework/security/oauth2/client/jackson2/OAuth2AuthenticationTokenMixinTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ public void serializeWhenMixinRegisteredThenSerializes() throws Exception {
@Test
public void serializeWhenRequiredAttributesOnlyThenSerializes() throws Exception {
DefaultOidcUser principal = TestOidcUsers.create();
principal = new DefaultOidcUser(principal.getAuthorities(), principal.getIdToken());
principal = new DefaultOidcUser(principal.getIdToken(), principal.getAuthorities());
OAuth2AuthenticationToken authentication = new OAuth2AuthenticationToken(principal, Collections.emptyList(),
"registration-id");
String expectedJson = asJson(authentication);
Expand Down Expand Up @@ -147,7 +147,7 @@ public void deserializeWhenMixinRegisteredThenDeserializes() throws Exception {
@Test
public void deserializeWhenRequiredAttributesOnlyThenDeserializes() throws Exception {
DefaultOidcUser expectedPrincipal = TestOidcUsers.create();
expectedPrincipal = new DefaultOidcUser(expectedPrincipal.getAuthorities(), expectedPrincipal.getIdToken());
expectedPrincipal = new DefaultOidcUser(expectedPrincipal.getIdToken(), expectedPrincipal.getAuthorities());
OAuth2AuthenticationToken expectedAuthentication = new OAuth2AuthenticationToken(expectedPrincipal,
Collections.emptyList(), "registration-id");
String json = asJson(expectedAuthentication);
Expand Down
8 changes: 4 additions & 4 deletions ...2/client/oidc/authentication/OidcAuthorizationCodeReactiveAuthenticationManagerTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ public void authenticationWhenOAuth2UserFoundThenSuccess() {
claims.put(IdTokenClaimNames.NONCE, this.nonceHash);
Jwt idToken = TestJwts.jwt().claims((c) -> c.putAll(claims)).build();
given(this.accessTokenResponseClient.getTokenResponse(any())).willReturn(Mono.just(accessTokenResponse));
DefaultOidcUser user = new DefaultOidcUser(AuthorityUtils.createAuthorityList("ROLE_USER"), this.idToken);
DefaultOidcUser user = new DefaultOidcUser(this.idToken, AuthorityUtils.createAuthorityList("ROLE_USER"));
given(this.userService.loadUser(any())).willReturn(Mono.just(user));
given(this.jwtDecoder.decode(any())).willReturn(Mono.just(idToken));
this.manager.setJwtDecoderFactory((c) -> this.jwtDecoder);
Expand Down Expand Up @@ -285,7 +285,7 @@ public void authenticationWhenRefreshTokenThenRefreshTokenInAuthorizedClient() {
claims.put(IdTokenClaimNames.NONCE, this.nonceHash);
Jwt idToken = TestJwts.jwt().claims((c) -> c.putAll(claims)).build();
given(this.accessTokenResponseClient.getTokenResponse(any())).willReturn(Mono.just(accessTokenResponse));
DefaultOidcUser user = new DefaultOidcUser(AuthorityUtils.createAuthorityList("ROLE_USER"), this.idToken);
DefaultOidcUser user = new DefaultOidcUser(this.idToken, AuthorityUtils.createAuthorityList("ROLE_USER"));
given(this.userService.loadUser(any())).willReturn(Mono.just(user));
given(this.jwtDecoder.decode(any())).willReturn(Mono.just(idToken));
this.manager.setJwtDecoderFactory((c) -> this.jwtDecoder);
Expand Down Expand Up @@ -321,7 +321,7 @@ public void authenticateWhenTokenSuccessResponseThenAdditionalParametersAddedToU
claims.put(IdTokenClaimNames.NONCE, this.nonceHash);
Jwt idToken = TestJwts.jwt().claims((c) -> c.putAll(claims)).build();
given(this.accessTokenResponseClient.getTokenResponse(any())).willReturn(Mono.just(accessTokenResponse));
DefaultOidcUser user = new DefaultOidcUser(AuthorityUtils.createAuthorityList("ROLE_USER"), this.idToken);
DefaultOidcUser user = new DefaultOidcUser(this.idToken, AuthorityUtils.createAuthorityList("ROLE_USER"));
ArgumentCaptor<OidcUserRequest> userRequestArgCaptor = ArgumentCaptor.forClass(OidcUserRequest.class);
given(this.userService.loadUser(userRequestArgCaptor.capture())).willReturn(Mono.just(user));
given(this.jwtDecoder.decode(any())).willReturn(Mono.just(idToken));
Expand Down Expand Up @@ -349,7 +349,7 @@ public void authenticateWhenAuthoritiesMapperSetThenReturnMappedAuthorities() {
claims.put(IdTokenClaimNames.NONCE, this.nonceHash);
Jwt idToken = TestJwts.jwt().claims((c) -> c.putAll(claims)).build();
given(this.accessTokenResponseClient.getTokenResponse(any())).willReturn(Mono.just(accessTokenResponse));
DefaultOidcUser user = new DefaultOidcUser(AuthorityUtils.createAuthorityList("ROLE_USER"), this.idToken);
DefaultOidcUser user = new DefaultOidcUser(this.idToken, AuthorityUtils.createAuthorityList("ROLE_USER"));
ArgumentCaptor<OidcUserRequest> userRequestArgCaptor = ArgumentCaptor.forClass(OidcUserRequest.class);
given(this.userService.loadUser(userRequestArgCaptor.capture())).willReturn(Mono.just(user));
List<GrantedAuthority> mappedAuthorities = AuthorityUtils.createAuthorityList("ROLE_OIDC_USER");
Expand Down
10 changes: 5 additions & 5 deletions ...springframework/security/oauth2/client/oidc/session/InMemoryOidcSessionRegistryTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public void registerWhenDefaultsThenStoresSessionInformation() {
public void registerWhenIdTokenHasSessionIdThenStoresSessionInformation() {
InMemoryOidcSessionRegistry sessionRegistry = new InMemoryOidcSessionRegistry();
OidcIdToken idToken = TestOidcIdTokens.idToken().claim("sid", "provider").build();
OidcUser user = new DefaultOidcUser(AuthorityUtils.NO_AUTHORITIES, idToken);
OidcUser user = new DefaultOidcUser(idToken, AuthorityUtils.NO_AUTHORITIES);
OidcSessionInformation info = TestOidcSessionInformations.create("client", user);
sessionRegistry.saveSessionInformation(info);
OidcLogoutToken logoutToken = TestOidcLogoutTokens.withSessionId(idToken.getIssuer().toString(), "provider")
Expand All @@ -61,15 +61,15 @@ public void registerWhenIdTokenHasSessionIdThenStoresSessionInformation() {
public void unregisterWhenMultipleSessionsThenRemovesAllMatching() {
InMemoryOidcSessionRegistry sessionRegistry = new InMemoryOidcSessionRegistry();
OidcIdToken idToken = TestOidcIdTokens.idToken().claim("sid", "providerOne").subject("otheruser").build();
OidcUser user = new DefaultOidcUser(AuthorityUtils.NO_AUTHORITIES, idToken);
OidcUser user = new DefaultOidcUser(idToken, AuthorityUtils.NO_AUTHORITIES);
OidcSessionInformation oneSession = TestOidcSessionInformations.create("clientOne", user);
sessionRegistry.saveSessionInformation(oneSession);
idToken = TestOidcIdTokens.idToken().claim("sid", "providerTwo").build();
user = new DefaultOidcUser(AuthorityUtils.NO_AUTHORITIES, idToken);
user = new DefaultOidcUser(idToken, AuthorityUtils.NO_AUTHORITIES);
OidcSessionInformation twoSession = TestOidcSessionInformations.create("clientTwo", user);
sessionRegistry.saveSessionInformation(twoSession);
idToken = TestOidcIdTokens.idToken().claim("sid", "providerThree").build();
user = new DefaultOidcUser(AuthorityUtils.NO_AUTHORITIES, idToken);
user = new DefaultOidcUser(idToken, AuthorityUtils.NO_AUTHORITIES);
OidcSessionInformation threeSession = TestOidcSessionInformations.create("clientThree", user);
sessionRegistry.saveSessionInformation(threeSession);
OidcLogoutToken logoutToken = TestOidcLogoutTokens
Expand All @@ -86,7 +86,7 @@ public void unregisterWhenMultipleSessionsThenRemovesAllMatching() {
public void unregisterWhenNoSessionsThenEmptyList() {
InMemoryOidcSessionRegistry sessionRegistry = new InMemoryOidcSessionRegistry();
OidcIdToken idToken = TestOidcIdTokens.idToken().claim("sid", "provider").build();
OidcUser user = new DefaultOidcUser(AuthorityUtils.NO_AUTHORITIES, idToken);
OidcUser user = new DefaultOidcUser(idToken, AuthorityUtils.NO_AUTHORITIES);
OidcSessionInformation info = TestOidcSessionInformations.create("client", user);
sessionRegistry.saveSessionInformation(info);
OidcLogoutToken logoutToken = TestOidcLogoutTokens.withSessionId(idToken.getIssuer().toString(), "wrong")
Expand Down
6 changes: 2 additions & 4 deletions ...ingframework/security/oauth2/client/oidc/userinfo/OidcReactiveOAuth2UserServiceTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,8 +250,7 @@ public void loadUserWhenCustomOidcUserMapperSetThenUsed() {
AuthorityUtils.createAuthorityList("ROLE_USER"));
given(this.oauth2UserService.loadUser(any(OidcUserRequest.class))).willReturn(Mono.just(oauth2User));
BiFunction<OidcUserRequest, OidcUserInfo, Mono<OidcUser>> customOidcUserMapper = mock(BiFunction.class);
OidcUser actualUser = new DefaultOidcUser(AuthorityUtils.createAuthorityList("a", "b"), this.idToken,
IdTokenClaimNames.SUB);
OidcUser actualUser = new DefaultOidcUser(this.idToken, AuthorityUtils.createAuthorityList("a", "b"));
given(customOidcUserMapper.apply(any(OidcUserRequest.class), any(OidcUserInfo.class)))
.willReturn(Mono.just(actualUser));
this.userService.setOidcUserMapper(customOidcUserMapper);
Expand All @@ -277,8 +276,7 @@ public void loadUserWhenCustomOidcUserMapperSetAndUserInfoNotRetrievedThenUsed()
Collections.emptySet());
// @formatter:on
BiFunction<OidcUserRequest, OidcUserInfo, Mono<OidcUser>> customOidcUserMapper = mock(BiFunction.class);
OidcUser actualUser = new DefaultOidcUser(AuthorityUtils.createAuthorityList("a", "b"), this.idToken,
IdTokenClaimNames.SUB);
OidcUser actualUser = new DefaultOidcUser(this.idToken, AuthorityUtils.createAuthorityList("a", "b"));
given(customOidcUserMapper.apply(any(OidcUserRequest.class), isNull())).willReturn(Mono.just(actualUser));
this.userService.setOidcUserMapper(customOidcUserMapper);
OidcUserRequest userRequest = userRequest();
Expand Down
3 changes: 1 addition & 2 deletions ...t/java/org/springframework/security/oauth2/client/oidc/userinfo/OidcUserServiceTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -285,8 +285,7 @@ public void loadUserWhenCustomOidcUserMapperSetThenUsed() {
ClientRegistration clientRegistration = this.clientRegistrationBuilder.userInfoUri(userInfoUri).build();
this.accessToken = TestOAuth2AccessTokens.noScopes();
BiFunction<OidcUserRequest, OidcUserInfo, OidcUser> customOidcUserMapper = mock(BiFunction.class);
OidcUser actualUser = new DefaultOidcUser(AuthorityUtils.createAuthorityList("a", "b"), this.idToken,
IdTokenClaimNames.SUB);
OidcUser actualUser = new DefaultOidcUser(this.idToken, AuthorityUtils.createAuthorityList("a", "b"));
given(customOidcUserMapper.apply(any(OidcUserRequest.class), any(OidcUserInfo.class))).willReturn(actualUser);
this.userService.setOidcUserMapper(customOidcUserMapper);
OidcUserRequest userRequest = new OidcUserRequest(clientRegistration, this.accessToken, this.idToken);
Expand Down
13 changes: 8 additions & 5 deletions ...rc/test/java/org/springframework/security/oauth2/core/oidc/user/DefaultOidcUserTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,17 +69,18 @@ public class DefaultOidcUserTests {

@Test
public void constructorWhenIdTokenIsNullThenThrowIllegalArgumentException() {
assertThatIllegalArgumentException().isThrownBy(() -> new DefaultOidcUser(AUTHORITIES, null));
assertThatIllegalArgumentException().isThrownBy(() -> new DefaultOidcUser(null, AUTHORITIES));
}

@Test
@Deprecated
public void constructorWhenNameAttributeKeyInvalidThenThrowIllegalArgumentException() {
assertThatIllegalArgumentException().isThrownBy(() -> new DefaultOidcUser(AUTHORITIES, ID_TOKEN, "invalid"));
}

@Test
public void constructorWhenAuthoritiesIsNullThenCreatedWithEmptyAuthorities() {
DefaultOidcUser user = new DefaultOidcUser(null, ID_TOKEN);
DefaultOidcUser user = new DefaultOidcUser(ID_TOKEN, null);
assertThat(user.getClaims()).containsOnlyKeys(IdTokenClaimNames.ISS, IdTokenClaimNames.SUB);
assertThat(user.getIdToken()).isEqualTo(ID_TOKEN);
assertThat(user.getName()).isEqualTo(SUBJECT);
Expand All @@ -89,7 +90,7 @@ public void constructorWhenAuthoritiesIsNullThenCreatedWithEmptyAuthorities() {

@Test
public void constructorWhenAuthoritiesIsEmptyThenCreated() {
DefaultOidcUser user = new DefaultOidcUser(AuthorityUtils.NO_AUTHORITIES, ID_TOKEN);
DefaultOidcUser user = new DefaultOidcUser(ID_TOKEN, AuthorityUtils.NO_AUTHORITIES);
assertThat(user.getClaims()).containsOnlyKeys(IdTokenClaimNames.ISS, IdTokenClaimNames.SUB);
assertThat(user.getIdToken()).isEqualTo(ID_TOKEN);
assertThat(user.getName()).isEqualTo(SUBJECT);
Expand All @@ -99,7 +100,7 @@ public void constructorWhenAuthoritiesIsEmptyThenCreated() {

@Test
public void constructorWhenAuthoritiesIdTokenProvidedThenCreated() {
DefaultOidcUser user = new DefaultOidcUser(AUTHORITIES, ID_TOKEN);
DefaultOidcUser user = new DefaultOidcUser(ID_TOKEN, AUTHORITIES);
assertThat(user.getClaims()).containsOnlyKeys(IdTokenClaimNames.ISS, IdTokenClaimNames.SUB);
assertThat(user.getIdToken()).isEqualTo(ID_TOKEN);
assertThat(user.getName()).isEqualTo(SUBJECT);
Expand All @@ -109,6 +110,7 @@ public void constructorWhenAuthoritiesIdTokenProvidedThenCreated() {
}

@Test
@Deprecated
public void constructorWhenAuthoritiesIdTokenNameAttributeKeyProvidedThenCreated() {
DefaultOidcUser user = new DefaultOidcUser(AUTHORITIES, ID_TOKEN, IdTokenClaimNames.SUB);
assertThat(user.getClaims()).containsOnlyKeys(IdTokenClaimNames.ISS, IdTokenClaimNames.SUB);
Expand All @@ -121,7 +123,7 @@ public void constructorWhenAuthoritiesIdTokenNameAttributeKeyProvidedThenCreated

@Test
public void constructorWhenAuthoritiesIdTokenUserInfoProvidedThenCreated() {
DefaultOidcUser user = new DefaultOidcUser(AUTHORITIES, ID_TOKEN, USER_INFO);
DefaultOidcUser user = new DefaultOidcUser(ID_TOKEN, USER_INFO, AUTHORITIES);
assertThat(user.getClaims()).containsOnlyKeys(IdTokenClaimNames.ISS, IdTokenClaimNames.SUB,
StandardClaimNames.NAME, StandardClaimNames.EMAIL);
assertThat(user.getIdToken()).isEqualTo(ID_TOKEN);
Expand All @@ -134,6 +136,7 @@ public void constructorWhenAuthoritiesIdTokenUserInfoProvidedThenCreated() {
}

@Test
@Deprecated
public void constructorWhenAllParametersProvidedAndValidThenCreated() {
DefaultOidcUser user = new DefaultOidcUser(AUTHORITIES, ID_TOKEN, USER_INFO, StandardClaimNames.EMAIL);
assertThat(user.getClaims()).containsOnlyKeys(IdTokenClaimNames.ISS, IdTokenClaimNames.SUB,
Expand Down
8 changes: 4 additions & 4 deletions ...mework/security/test/web/reactive/server/SecurityMockServerConfigurersOidcLoginTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,9 +148,9 @@ public void oidcLoginWhenUserInfoSpecifiedThenUserHasClaims() throws Exception {

@Test
public void oidcUserWhenNameSpecifiedThenUserHasName() throws Exception {
OidcUser oidcUser = new DefaultOidcUser(AuthorityUtils.commaSeparatedStringToAuthorityList("SCOPE_read"),
OidcUser oidcUser = new DefaultOidcUser("test-subject",
OidcIdToken.withTokenValue("id-token").claim("custom-attribute", "test-subject").build(),
"custom-attribute");
AuthorityUtils.commaSeparatedStringToAuthorityList("SCOPE_read"));
this.client.mutateWith(SecurityMockServerConfigurers.mockOAuth2Login().oauth2User(oidcUser))
.get()
.uri("/token")
Expand All @@ -172,8 +172,8 @@ public void oidcUserWhenNameSpecifiedThenUserHasName() throws Exception {
// gh-7794
@Test
public void oidcLoginWhenOidcUserSpecifiedThenLastCalledTakesPrecedence() throws Exception {
OidcUser oidcUser = new DefaultOidcUser(AuthorityUtils.createAuthorityList("SCOPE_read"),
TestOidcIdTokens.idToken().build());
OidcUser oidcUser = new DefaultOidcUser(TestOidcIdTokens.idToken().build(),
AuthorityUtils.createAuthorityList("SCOPE_read"));
this.client
.mutateWith(
SecurityMockServerConfigurers.mockOidcLogin().idToken((i) -> i.subject("foo")).oidcUser(oidcUser))
Expand Down
8 changes: 4 additions & 4 deletions ...security/test/web/servlet/request/SecurityMockMvcRequestPostProcessorsOidcLoginTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,9 @@ public void oidcLoginWhenUserInfoSpecifiedThenUserHasClaims() throws Exception {

@Test
public void oidcLoginWhenNameSpecifiedThenUserHasName() throws Exception {
OidcUser oidcUser = new DefaultOidcUser(AuthorityUtils.commaSeparatedStringToAuthorityList("SCOPE_read"),
OidcUser oidcUser = new DefaultOidcUser("test-subject",
OidcIdToken.withTokenValue("id-token").claim("custom-attribute", "test-subject").build(),
"custom-attribute");
AuthorityUtils.commaSeparatedStringToAuthorityList("SCOPE_read"));
this.mvc.perform(get("/id-token/custom-attribute").with(oidcLogin().oidcUser(oidcUser)))
.andExpect(content().string("test-subject"));
this.mvc.perform(get("/name").with(oidcLogin().oidcUser(oidcUser))).andExpect(content().string("test-subject"));
Expand All @@ -137,8 +137,8 @@ public void oidcLoginWhenNameSpecifiedThenUserHasName() throws Exception {
// gh-7794
@Test
public void oidcLoginWhenOidcUserSpecifiedThenLastCalledTakesPrecedence() throws Exception {
OidcUser oidcUser = new DefaultOidcUser(AuthorityUtils.createAuthorityList("SCOPE_read"),
TestOidcIdTokens.idToken().build());
OidcUser oidcUser = new DefaultOidcUser(TestOidcIdTokens.idToken().build(),
AuthorityUtils.createAuthorityList("SCOPE_read"));
this.mvc.perform(get("/id-token/sub").with(oidcLogin().idToken((i) -> i.subject("foo")).oidcUser(oidcUser)))
.andExpect(status().isOk())
.andExpect(content().string("subject"));
Expand Down

0 comments on commit 4848c6b

Please sign in to comment.