From bc4be559da7d47dc9c8120840f64e230dc439874 Mon Sep 17 00:00:00 2001 From: Devon Hillard Date: Sun, 22 Mar 2026 14:58:11 -0600 Subject: [PATCH 1/2] fix(oauth2): populate DSUserDetails.getAttributes() from provider for OAuth2/OIDC users Fixes #284. DSUserDetails implemented OAuth2User/OidcUser but getAttributes() always returned an empty map for OAuth2 users and null for OIDC users, silently breaking the standard getAttribute("email") access pattern. Changes: - DSUserDetails: all constructors now initialize attributes correctly. New 3-arg simple constructor accepts provider attributes. OIDC constructor falls back to idToken claims, then User entity fields when no attributes are provided. Added buildFallbackAttributes() helper that maps User entity fields to standard OAuth2 claim names (email, given_name, family_name, name) so getAttributes() is never null on any code path. - LoginHelperService: added userLoginHelper(User, Map) and userLoginHelper(User, OidcUserInfo, OidcIdToken, Map) overloads that forward provider attributes to DSUserDetails. Original overloads kept for backward compatibility (local/password login path). - DSOAuth2UserService.loadUser(): passes original OAuth2User.getAttributes() through to loginHelperService. - DSOidcUserService.loadUser(): passes original OidcUser.getAttributes() through to loginHelperService. Tests: new DSUserDetailsTest (13 tests) covers all constructor paths, fallback behavior, defensive copying, builder, and the getAttribute("email") pattern. LoginHelperServiceTest and DSOAuth2/OidcUserServiceTest updated. --- .../user/service/DSOAuth2UserService.java | 2 +- .../user/service/DSOidcUserService.java | 2 +- .../spring/user/service/DSUserDetails.java | 76 +++++- .../user/service/LoginHelperService.java | 40 ++- .../user/service/DSOAuth2UserServiceTest.java | 4 +- .../user/service/DSOidcUserServiceTest.java | 9 +- .../user/service/DSUserDetailsTest.java | 244 ++++++++++++++++++ .../user/service/LoginHelperServiceTest.java | 100 +++++++ 8 files changed, 455 insertions(+), 22 deletions(-) create mode 100644 src/test/java/com/digitalsanctuary/spring/user/service/DSUserDetailsTest.java diff --git a/src/main/java/com/digitalsanctuary/spring/user/service/DSOAuth2UserService.java b/src/main/java/com/digitalsanctuary/spring/user/service/DSOAuth2UserService.java index ce0ca20..6831f36 100644 --- a/src/main/java/com/digitalsanctuary/spring/user/service/DSOAuth2UserService.java +++ b/src/main/java/com/digitalsanctuary/spring/user/service/DSOAuth2UserService.java @@ -219,7 +219,7 @@ public OAuth2User loadUser(OAuth2UserRequest userRequest) throws OAuth2Authentic String registrationId = userRequest.getClientRegistration().getRegistrationId(); log.debug("registrationId: {}", registrationId); User dbUser = handleOAuthLoginSuccess(registrationId, user); - DSUserDetails userDetails = loginHelperService.userLoginHelper(dbUser); + DSUserDetails userDetails = loginHelperService.userLoginHelper(dbUser, user.getAttributes()); return userDetails; } diff --git a/src/main/java/com/digitalsanctuary/spring/user/service/DSOidcUserService.java b/src/main/java/com/digitalsanctuary/spring/user/service/DSOidcUserService.java index ad1bd12..2c7b0d6 100644 --- a/src/main/java/com/digitalsanctuary/spring/user/service/DSOidcUserService.java +++ b/src/main/java/com/digitalsanctuary/spring/user/service/DSOidcUserService.java @@ -198,6 +198,6 @@ public OidcUser loadUser(OidcUserRequest userRequest) throws OAuth2Authenticatio String registrationId = userRequest.getClientRegistration().getRegistrationId(); log.debug("registrationId: {}", registrationId); User dbUser = handleOidcLoginSuccess(registrationId, user); - return loginHelperService.userLoginHelper(dbUser, user.getUserInfo(), user.getIdToken()); + return loginHelperService.userLoginHelper(dbUser, user.getUserInfo(), user.getIdToken(), user.getAttributes()); } } diff --git a/src/main/java/com/digitalsanctuary/spring/user/service/DSUserDetails.java b/src/main/java/com/digitalsanctuary/spring/user/service/DSUserDetails.java index 222d9e3..436b99d 100644 --- a/src/main/java/com/digitalsanctuary/spring/user/service/DSUserDetails.java +++ b/src/main/java/com/digitalsanctuary/spring/user/service/DSUserDetails.java @@ -60,15 +60,27 @@ public class DSUserDetails implements UserDetails, OidcUser { private OidcIdToken oidcIdToken; /** - * Instantiates a new DS user details. + * Instantiates a new DS user details with OAuth2 provider attributes. * * @param user the user * @param grantedAuthorities the granted authorities (optional, default = empty list) + * @param attributes the OAuth2 provider attributes (optional, falls back to User entity fields) */ - public DSUserDetails(User user, Collection grantedAuthorities) { + public DSUserDetails(User user, Collection grantedAuthorities, Map attributes) { this.user = user; this.grantedAuthorities = grantedAuthorities != null ? grantedAuthorities : new ArrayList<>(); - this.attributes = new HashMap<>(); + this.attributes = attributes != null ? new HashMap<>(attributes) : buildFallbackAttributes(user); + } + + /** + * Instantiates a new DS user details without provider attributes. Attributes will be populated + * from the {@link User} entity fields as a fallback. + * + * @param user the user + * @param grantedAuthorities the granted authorities (optional, default = empty list) + */ + public DSUserDetails(User user, Collection grantedAuthorities) { + this(user, grantedAuthorities, (Map) null); } /** @@ -77,35 +89,81 @@ public DSUserDetails(User user, Collection grantedAu * @param user the user */ public DSUserDetails(User user) { - this(user, null); + this(user, null, (Map) null); } /** - * Instantiates a new DS user details. + * Instantiates a new DS user details with OIDC tokens and provider attributes. * * @param user the user * @param oidcUserInfo containing claims about the user * @param oidcIdToken containing claims about the user * @param grantedAuthorities the granted authorities (optional, default = empty list) + * @param attributes the OAuth2/OIDC provider attributes (optional, falls back to idToken claims or User entity) */ @Builder - public DSUserDetails(User user, OidcUserInfo oidcUserInfo, OidcIdToken oidcIdToken, Collection grantedAuthorities) { + public DSUserDetails(User user, OidcUserInfo oidcUserInfo, OidcIdToken oidcIdToken, + Collection grantedAuthorities, Map attributes) { this.user = user; this.oidcUserInfo = oidcUserInfo; this.oidcIdToken = oidcIdToken; this.grantedAuthorities = grantedAuthorities != null ? grantedAuthorities : new ArrayList<>(); + if (attributes != null) { + this.attributes = new HashMap<>(attributes); + } else if (oidcIdToken != null) { + this.attributes = new HashMap<>(oidcIdToken.getClaims()); + } else { + this.attributes = buildFallbackAttributes(user); + } } /** - * Instantiates a new DS user details. + * Instantiates a new DS user details with OIDC tokens. Attributes will be populated from the + * OIDC ID token claims or {@link User} entity as a fallback. + * + * @param user the user + * @param oidcUserInfo containing claims about the user + * @param oidcIdToken containing claims about the user + * @param grantedAuthorities the granted authorities (optional, default = empty list) + */ + public DSUserDetails(User user, OidcUserInfo oidcUserInfo, OidcIdToken oidcIdToken, + Collection grantedAuthorities) { + this(user, oidcUserInfo, oidcIdToken, grantedAuthorities, null); + } + + /** + * Instantiates a new DS user details with OIDC tokens and no authorities. * * @param user the user * @param oidcUserInfo containing claims about the user * @param oidcIdToken containing claims about the user */ - @Builder public DSUserDetails(User user, OidcUserInfo oidcUserInfo, OidcIdToken oidcIdToken) { - this(user, oidcUserInfo, oidcIdToken, null); + this(user, oidcUserInfo, oidcIdToken, null, null); + } + + /** + * Builds a fallback attributes map from the {@link User} entity fields. Used when no provider + * attributes are available (e.g., local/password login). + * + * @param user the user entity + * @return a map containing available user fields using standard OAuth2/OIDC claim names + */ + private static Map buildFallbackAttributes(User user) { + Map attrs = new HashMap<>(); + if (user.getEmail() != null) { + attrs.put("email", user.getEmail()); + } + if (user.getFirstName() != null) { + attrs.put("given_name", user.getFirstName()); + } + if (user.getLastName() != null) { + attrs.put("family_name", user.getLastName()); + } + if (user.getFirstName() != null || user.getLastName() != null) { + attrs.put("name", user.getFullName()); + } + return attrs; } /** diff --git a/src/main/java/com/digitalsanctuary/spring/user/service/LoginHelperService.java b/src/main/java/com/digitalsanctuary/spring/user/service/LoginHelperService.java index 89420f0..c85c809 100644 --- a/src/main/java/com/digitalsanctuary/spring/user/service/LoginHelperService.java +++ b/src/main/java/com/digitalsanctuary/spring/user/service/LoginHelperService.java @@ -2,6 +2,7 @@ import java.util.Collection; import java.util.Date; +import java.util.Map; import org.springframework.security.core.GrantedAuthority; import org.springframework.security.oauth2.core.oidc.OidcIdToken; import org.springframework.security.oauth2.core.oidc.OidcUserInfo; @@ -34,13 +35,25 @@ public class LoginHelperService { private final AuthorityService authorityService; /** - * Helper method to authenticate a user after login. This method is called from the DSUserDetailsService and DSOAuth2UserService classes after a - * user has been successfully authenticated. + * Helper method to authenticate a user after login. This method is called from the DSUserDetailsService after a user has been successfully + * authenticated via local/password login. Attributes are populated from the {@link User} entity as a fallback. * * @param dbUser The user to authenticate. * @return The user details object. */ public DSUserDetails userLoginHelper(User dbUser) { + return userLoginHelper(dbUser, (Map) null); + } + + /** + * Helper method to authenticate a user after OAuth2 login, preserving the original provider attributes so that + * {@link DSUserDetails#getAttributes()} returns the full set of attributes from the OAuth2 provider. + * + * @param dbUser The user to authenticate. + * @param attributes The OAuth2 provider attributes (may be null for local login fallback). + * @return The user details object with provider attributes set. + */ + public DSUserDetails userLoginHelper(User dbUser, Map attributes) { // Updating lastActivity date for this login dbUser.setLastActivityDate(new Date()); @@ -48,13 +61,13 @@ public DSUserDetails userLoginHelper(User dbUser) { dbUser = loginAttemptService.checkIfUserShouldBeUnlocked(dbUser); Collection authorities = authorityService.getAuthoritiesFromUser(dbUser); - DSUserDetails userDetails = new DSUserDetails(dbUser, authorities); - return userDetails; + return new DSUserDetails(dbUser, authorities, attributes); } /** * Helper method to authenticate an OIDC user after login, attaching the OIDC-specific tokens - * and claims to the principal while keeping {@link DSUserDetails} immutable. + * and claims to the principal while keeping {@link DSUserDetails} immutable. Attributes are + * populated from the OIDC ID token claims as a fallback. * * @param dbUser The user to authenticate. * @param oidcUserInfo The OIDC user info claims. @@ -62,6 +75,21 @@ public DSUserDetails userLoginHelper(User dbUser) { * @return The user details object with OIDC tokens set. */ public DSUserDetails userLoginHelper(User dbUser, OidcUserInfo oidcUserInfo, OidcIdToken oidcIdToken) { + return userLoginHelper(dbUser, oidcUserInfo, oidcIdToken, null); + } + + /** + * Helper method to authenticate an OIDC user after login, preserving the original provider attributes and + * attaching the OIDC-specific tokens and claims to the principal. + * + * @param dbUser The user to authenticate. + * @param oidcUserInfo The OIDC user info claims. + * @param oidcIdToken The OIDC ID token. + * @param attributes The OIDC provider attributes (may be null to fall back to idToken claims). + * @return The user details object with OIDC tokens and provider attributes set. + */ + public DSUserDetails userLoginHelper(User dbUser, OidcUserInfo oidcUserInfo, OidcIdToken oidcIdToken, + Map attributes) { // Updating lastActivity date for this login dbUser.setLastActivityDate(new Date()); @@ -69,6 +97,6 @@ public DSUserDetails userLoginHelper(User dbUser, OidcUserInfo oidcUserInfo, Oid dbUser = loginAttemptService.checkIfUserShouldBeUnlocked(dbUser); Collection authorities = authorityService.getAuthoritiesFromUser(dbUser); - return new DSUserDetails(dbUser, oidcUserInfo, oidcIdToken, authorities); + return new DSUserDetails(dbUser, oidcUserInfo, oidcIdToken, authorities, attributes); } } diff --git a/src/test/java/com/digitalsanctuary/spring/user/service/DSOAuth2UserServiceTest.java b/src/test/java/com/digitalsanctuary/spring/user/service/DSOAuth2UserServiceTest.java index 1c839eb..09f4dfe 100644 --- a/src/test/java/com/digitalsanctuary/spring/user/service/DSOAuth2UserServiceTest.java +++ b/src/test/java/com/digitalsanctuary/spring/user/service/DSOAuth2UserServiceTest.java @@ -416,7 +416,7 @@ void shouldLoadUserThroughOAuth2RequestFlow() { when(spyService.defaultOAuth2UserService.loadUser(userRequest)).thenReturn(googleUser); DSUserDetails mockUserDetails = mock(DSUserDetails.class); - when(loginHelperService.userLoginHelper(any(User.class))).thenReturn(mockUserDetails); + when(loginHelperService.userLoginHelper(any(User.class), any(Map.class))).thenReturn(mockUserDetails); when(userRepository.findByEmail("loadtest@gmail.com")).thenReturn(null); when(userRepository.save(any(User.class))).thenAnswer(invocation -> invocation.getArgument(0)); @@ -425,7 +425,7 @@ void shouldLoadUserThroughOAuth2RequestFlow() { // Then assertThat(result).isEqualTo(mockUserDetails); - verify(loginHelperService).userLoginHelper(any(User.class)); + verify(loginHelperService).userLoginHelper(any(User.class), any(Map.class)); verify(userRepository).save(any(User.class)); } } diff --git a/src/test/java/com/digitalsanctuary/spring/user/service/DSOidcUserServiceTest.java b/src/test/java/com/digitalsanctuary/spring/user/service/DSOidcUserServiceTest.java index 1b9ee2b..c81c107 100644 --- a/src/test/java/com/digitalsanctuary/spring/user/service/DSOidcUserServiceTest.java +++ b/src/test/java/com/digitalsanctuary/spring/user/service/DSOidcUserServiceTest.java @@ -7,6 +7,7 @@ import java.util.Arrays; import java.util.List; +import java.util.Map; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Nested; @@ -372,13 +373,14 @@ void shouldLoadUserThroughOidcRequestFlow() { user.setId(999L); return user; }); - when(loginHelperService.userLoginHelper(any(User.class), any(OidcUserInfo.class), any(OidcIdToken.class))) + when(loginHelperService.userLoginHelper(any(User.class), any(OidcUserInfo.class), any(OidcIdToken.class), any(Map.class))) .thenAnswer(invocation -> { User user = invocation.getArgument(0); OidcUserInfo oidcUserInfo = invocation.getArgument(1); OidcIdToken oidcIdToken = invocation.getArgument(2); + Map attributes = invocation.getArgument(3); return new DSUserDetails(user, oidcUserInfo, oidcIdToken, - List.of(new SimpleGrantedAuthority("ROLE_USER"))); + List.of(new SimpleGrantedAuthority("ROLE_USER")), attributes); }); // When @@ -391,9 +393,10 @@ void shouldLoadUserThroughOidcRequestFlow() { assertThat(dsUserDetails.getIdToken()).isNotNull(); assertThat(dsUserDetails.getUserInfo()).isNotNull(); assertThat(dsUserDetails.getAuthorities()).isNotEmpty(); + assertThat(dsUserDetails.getAttributes()).isNotEmpty(); verify(userRepository).save(any(User.class)); - verify(loginHelperService).userLoginHelper(any(User.class), any(OidcUserInfo.class), any(OidcIdToken.class)); + verify(loginHelperService).userLoginHelper(any(User.class), any(OidcUserInfo.class), any(OidcIdToken.class), any(Map.class)); } @Test diff --git a/src/test/java/com/digitalsanctuary/spring/user/service/DSUserDetailsTest.java b/src/test/java/com/digitalsanctuary/spring/user/service/DSUserDetailsTest.java new file mode 100644 index 0000000..0ef0cf1 --- /dev/null +++ b/src/test/java/com/digitalsanctuary/spring/user/service/DSUserDetailsTest.java @@ -0,0 +1,244 @@ +package com.digitalsanctuary.spring.user.service; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.time.Instant; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.springframework.security.core.authority.SimpleGrantedAuthority; +import org.springframework.security.oauth2.core.oidc.OidcIdToken; +import org.springframework.security.oauth2.core.oidc.OidcUserInfo; + +import com.digitalsanctuary.spring.user.persistence.model.User; + +/** + * Unit tests for {@link DSUserDetails} attribute handling. + * + *

Verifies that {@code getAttributes()} satisfies the {@code OAuth2User} contract across all + * constructor paths: OAuth2, OIDC, and local/password login.

+ */ +@DisplayName("DSUserDetails Tests") +class DSUserDetailsTest { + + private User testUser; + + @BeforeEach + void setUp() { + testUser = new User(); + testUser.setEmail("test@example.com"); + testUser.setFirstName("Test"); + testUser.setLastName("User"); + } + + @Nested + @DisplayName("Simple Constructor (OAuth2 / Local Login)") + class SimpleConstructorTests { + + @Test + @DisplayName("Should return provider attributes when provided") + void shouldReturnProviderAttributesWhenProvided() { + Map providerAttrs = new HashMap<>(); + providerAttrs.put("email", "test@example.com"); + providerAttrs.put("given_name", "Test"); + providerAttrs.put("family_name", "User"); + providerAttrs.put("picture", "https://example.com/photo.jpg"); + providerAttrs.put("sub", "123456789"); + + DSUserDetails details = new DSUserDetails(testUser, + List.of(new SimpleGrantedAuthority("ROLE_USER")), providerAttrs); + + assertThat(details.getAttributes()).isNotNull(); + assertThat(details.getAttributes()).containsEntry("email", "test@example.com"); + assertThat(details.getAttributes()).containsEntry("picture", "https://example.com/photo.jpg"); + assertThat(details.getAttributes()).containsEntry("sub", "123456789"); + } + + @Test + @DisplayName("Should build fallback attributes from User entity when attributes are null") + void shouldBuildFallbackAttributesWhenNull() { + DSUserDetails details = new DSUserDetails(testUser, + List.of(new SimpleGrantedAuthority("ROLE_USER"))); + + assertThat(details.getAttributes()).isNotNull(); + assertThat(details.getAttributes()).containsEntry("email", "test@example.com"); + assertThat(details.getAttributes()).containsEntry("given_name", "Test"); + assertThat(details.getAttributes()).containsEntry("family_name", "User"); + assertThat(details.getAttributes()).containsEntry("name", "Test User"); + } + + @Test + @DisplayName("Should return empty map for User with no fields set") + void shouldReturnEmptyMapForMinimalUser() { + User minimalUser = new User(); + + DSUserDetails details = new DSUserDetails(minimalUser); + + assertThat(details.getAttributes()).isNotNull(); + assertThat(details.getAttributes()).isEmpty(); + } + + @Test + @DisplayName("getAttribute('email') should return email for OAuth2 user") + void shouldSupportGetAttributePattern() { + Map providerAttrs = Map.of( + "email", "oauth@example.com", + "sub", "abc123" + ); + + DSUserDetails details = new DSUserDetails(testUser, + List.of(new SimpleGrantedAuthority("ROLE_USER")), providerAttrs); + + // This is the standard OAuth2User pattern that was broken before the fix + assertThat((String) details.getAttribute("email")).isEqualTo("oauth@example.com"); + } + + @Test + @DisplayName("Should defensively copy provided attributes") + void shouldDefensivelyCopyAttributes() { + Map providerAttrs = new HashMap<>(); + providerAttrs.put("email", "test@example.com"); + + DSUserDetails details = new DSUserDetails(testUser, null, providerAttrs); + + // Modifying the original map should not affect DSUserDetails + providerAttrs.put("injected", "value"); + assertThat(details.getAttributes()).doesNotContainKey("injected"); + } + } + + @Nested + @DisplayName("OIDC Constructor") + class OidcConstructorTests { + + private OidcIdToken idToken; + private OidcUserInfo userInfo; + + @BeforeEach + void setUp() { + Map tokenClaims = new HashMap<>(); + tokenClaims.put("sub", "f:12345678:testuser"); + tokenClaims.put("email", "oidc@example.com"); + tokenClaims.put("email_verified", true); + tokenClaims.put("given_name", "Test"); + tokenClaims.put("family_name", "User"); + tokenClaims.put("iss", "https://idp.example.com"); + + idToken = new OidcIdToken("token-value", Instant.now(), Instant.now().plusSeconds(3600), tokenClaims); + + Map infoClaims = new HashMap<>(); + infoClaims.put("sub", "f:12345678:testuser"); + infoClaims.put("email", "oidc@example.com"); + userInfo = new OidcUserInfo(infoClaims); + } + + @Test + @DisplayName("Should return provider attributes when provided to OIDC constructor") + void shouldReturnProviderAttributesWhenProvided() { + Map providerAttrs = new HashMap<>(); + providerAttrs.put("email", "oidc@example.com"); + providerAttrs.put("custom_claim", "custom_value"); + + DSUserDetails details = new DSUserDetails(testUser, userInfo, idToken, + List.of(new SimpleGrantedAuthority("ROLE_USER")), providerAttrs); + + assertThat(details.getAttributes()).isNotNull(); + assertThat(details.getAttributes()).containsEntry("email", "oidc@example.com"); + assertThat(details.getAttributes()).containsEntry("custom_claim", "custom_value"); + } + + @Test + @DisplayName("Should fall back to idToken claims when attributes are null") + void shouldFallBackToIdTokenClaimsWhenAttributesNull() { + DSUserDetails details = new DSUserDetails(testUser, userInfo, idToken, + List.of(new SimpleGrantedAuthority("ROLE_USER"))); + + assertThat(details.getAttributes()).isNotNull(); + assertThat(details.getAttributes()).containsEntry("sub", "f:12345678:testuser"); + assertThat(details.getAttributes()).containsEntry("email", "oidc@example.com"); + assertThat(details.getAttributes()).containsEntry("email_verified", true); + assertThat(details.getAttributes()).containsEntry("iss", "https://idp.example.com"); + } + + @Test + @DisplayName("Should fall back to User entity when both attributes and idToken are null") + void shouldFallBackToUserEntityWhenBothNull() { + DSUserDetails details = new DSUserDetails(testUser, userInfo, null, + List.of(new SimpleGrantedAuthority("ROLE_USER"))); + + assertThat(details.getAttributes()).isNotNull(); + assertThat(details.getAttributes()).containsEntry("email", "test@example.com"); + assertThat(details.getAttributes()).containsEntry("given_name", "Test"); + } + + @Test + @DisplayName("Should never return null from getAttributes() for OIDC path") + void shouldNeverReturnNullForOidcPath() { + // All-null OIDC constructor — was previously returning null (NPE risk) + DSUserDetails details = new DSUserDetails(testUser, (OidcUserInfo) null, (OidcIdToken) null); + + assertThat(details.getAttributes()).isNotNull(); + } + + @Test + @DisplayName("getAttribute('email') should work for OIDC user") + void shouldSupportGetAttributePatternForOidc() { + Map providerAttrs = Map.of("email", "oidc@example.com", "sub", "test-sub"); + + DSUserDetails details = new DSUserDetails(testUser, userInfo, idToken, + List.of(new SimpleGrantedAuthority("ROLE_USER")), providerAttrs); + + assertThat((String) details.getAttribute("email")).isEqualTo("oidc@example.com"); + } + + @Test + @DisplayName("Should preserve OIDC tokens alongside attributes") + void shouldPreserveOidcTokensAlongsideAttributes() { + Map providerAttrs = Map.of("email", "oidc@example.com"); + + DSUserDetails details = new DSUserDetails(testUser, userInfo, idToken, + List.of(new SimpleGrantedAuthority("ROLE_USER")), providerAttrs); + + assertThat(details.getIdToken()).isEqualTo(idToken); + assertThat(details.getUserInfo()).isEqualTo(userInfo); + assertThat(details.getAttributes()).containsEntry("email", "oidc@example.com"); + } + } + + @Nested + @DisplayName("Builder") + class BuilderTests { + + @Test + @DisplayName("Builder should accept attributes parameter") + void shouldAcceptAttributesViaBuilder() { + Map providerAttrs = Map.of("email", "builder@example.com", "sub", "builder-sub"); + + DSUserDetails details = DSUserDetails.builder() + .user(testUser) + .attributes(providerAttrs) + .grantedAuthorities(List.of(new SimpleGrantedAuthority("ROLE_USER"))) + .build(); + + assertThat(details.getAttributes()).containsEntry("email", "builder@example.com"); + assertThat(details.getAttributes()).containsEntry("sub", "builder-sub"); + } + + @Test + @DisplayName("Builder without attributes should use fallback") + void shouldUseFallbackWhenBuilderOmitsAttributes() { + DSUserDetails details = DSUserDetails.builder() + .user(testUser) + .grantedAuthorities(List.of(new SimpleGrantedAuthority("ROLE_USER"))) + .build(); + + assertThat(details.getAttributes()).isNotNull(); + assertThat(details.getAttributes()).containsEntry("email", "test@example.com"); + } + } +} diff --git a/src/test/java/com/digitalsanctuary/spring/user/service/LoginHelperServiceTest.java b/src/test/java/com/digitalsanctuary/spring/user/service/LoginHelperServiceTest.java index 919f454..beee09a 100644 --- a/src/test/java/com/digitalsanctuary/spring/user/service/LoginHelperServiceTest.java +++ b/src/test/java/com/digitalsanctuary/spring/user/service/LoginHelperServiceTest.java @@ -4,10 +4,13 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import java.time.Instant; import java.util.Collection; import java.util.Collections; import java.util.Date; +import java.util.HashMap; import java.util.HashSet; +import java.util.Map; import java.util.Set; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; @@ -19,6 +22,8 @@ import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.authority.SimpleGrantedAuthority; +import org.springframework.security.oauth2.core.oidc.OidcIdToken; +import org.springframework.security.oauth2.core.oidc.OidcUserInfo; import com.digitalsanctuary.spring.user.persistence.model.Privilege; import com.digitalsanctuary.spring.user.persistence.model.Role; import com.digitalsanctuary.spring.user.persistence.model.User; @@ -439,4 +444,99 @@ void shouldHandleRapidSuccessiveLogins() { assertThat(secondLoginTime).isAfter(firstLoginTime); } } + + @Nested + @DisplayName("Attribute Passthrough Tests") + class AttributePassthroughTests { + + @Test + @DisplayName("Should pass OAuth2 provider attributes to DSUserDetails") + void shouldPassOAuth2AttributesToDSUserDetails() { + // Given + Map providerAttrs = new HashMap<>(); + providerAttrs.put("email", "test@example.com"); + providerAttrs.put("sub", "123456789"); + providerAttrs.put("picture", "https://example.com/photo.jpg"); + + when(loginAttemptService.checkIfUserShouldBeUnlocked(testUser)).thenReturn(testUser); + when(authorityService.getAuthoritiesFromUser(testUser)).thenReturn((Collection) testAuthorities); + + // When + DSUserDetails result = loginHelperService.userLoginHelper(testUser, providerAttrs); + + // Then + assertThat(result.getAttributes()).isNotNull(); + assertThat(result.getAttributes()).containsEntry("email", "test@example.com"); + assertThat(result.getAttributes()).containsEntry("sub", "123456789"); + assertThat(result.getAttributes()).containsEntry("picture", "https://example.com/photo.jpg"); + assertThat((String) result.getAttribute("email")).isEqualTo("test@example.com"); + } + + @Test + @DisplayName("Should fall back to User entity attributes when OAuth2 attributes are null") + void shouldFallBackWhenOAuth2AttributesNull() { + // Given + when(loginAttemptService.checkIfUserShouldBeUnlocked(testUser)).thenReturn(testUser); + when(authorityService.getAuthoritiesFromUser(testUser)).thenReturn((Collection) testAuthorities); + + // When + DSUserDetails result = loginHelperService.userLoginHelper(testUser); + + // Then + assertThat(result.getAttributes()).isNotNull(); + assertThat(result.getAttributes()).containsEntry("email", "test@example.com"); + } + + @Test + @DisplayName("Should pass OIDC provider attributes to DSUserDetails") + void shouldPassOidcAttributesToDSUserDetails() { + // Given + Map tokenClaims = new HashMap<>(); + tokenClaims.put("sub", "oidc-sub-123"); + tokenClaims.put("email", "oidc@example.com"); + tokenClaims.put("iss", "https://idp.example.com"); + + OidcIdToken idToken = new OidcIdToken("token", Instant.now(), Instant.now().plusSeconds(3600), tokenClaims); + OidcUserInfo userInfo = new OidcUserInfo(Map.of("sub", "oidc-sub-123", "email", "oidc@example.com")); + + Map providerAttrs = new HashMap<>(tokenClaims); + providerAttrs.put("extra_claim", "extra_value"); + + when(loginAttemptService.checkIfUserShouldBeUnlocked(testUser)).thenReturn(testUser); + when(authorityService.getAuthoritiesFromUser(testUser)).thenReturn((Collection) testAuthorities); + + // When + DSUserDetails result = loginHelperService.userLoginHelper(testUser, userInfo, idToken, providerAttrs); + + // Then + assertThat(result.getAttributes()).isNotNull(); + assertThat(result.getAttributes()).containsEntry("email", "oidc@example.com"); + assertThat(result.getAttributes()).containsEntry("extra_claim", "extra_value"); + assertThat(result.getIdToken()).isEqualTo(idToken); + assertThat(result.getUserInfo()).isEqualTo(userInfo); + } + + @Test + @DisplayName("Should fall back to idToken claims when OIDC attributes are null") + void shouldFallBackToIdTokenClaimsWhenOidcAttributesNull() { + // Given + Map tokenClaims = new HashMap<>(); + tokenClaims.put("sub", "oidc-sub-123"); + tokenClaims.put("email", "oidc@example.com"); + + OidcIdToken idToken = new OidcIdToken("token", Instant.now(), Instant.now().plusSeconds(3600), tokenClaims); + OidcUserInfo userInfo = new OidcUserInfo(Map.of("sub", "oidc-sub-123")); + + when(loginAttemptService.checkIfUserShouldBeUnlocked(testUser)).thenReturn(testUser); + when(authorityService.getAuthoritiesFromUser(testUser)).thenReturn((Collection) testAuthorities); + + // When + DSUserDetails result = loginHelperService.userLoginHelper(testUser, userInfo, idToken); + + // Then + assertThat(result.getAttributes()).isNotNull(); + assertThat(result.getAttributes()).containsEntry("sub", "oidc-sub-123"); + assertThat(result.getAttributes()).containsEntry("email", "oidc@example.com"); + } + } } From 7c372cfbf26c0e062859e28b87cdda61a5821309 Mon Sep 17 00:00:00 2001 From: Devon Hillard Date: Sun, 22 Mar 2026 15:12:58 -0600 Subject: [PATCH 2/2] fix: address PR #285 review feedback - DSUserDetails.getAttributes(): return Collections.unmodifiableMap so callers cannot mutate the internal map, preventing hard-to-debug security issues - buildFallbackAttributes(): build 'name' claim without calling getFullName() to avoid "Test null" / "null User" values when only one name part is set; now safely skips missing parts and only adds the claim when non-empty - DSUserDetailsTest: add @ExtendWith(MockitoExtension.class) per project conventions; add tests for first-name-only, last-name-only, and unmodifiable-map scenarios - LoginHelperServiceTest: replace raw (Collection) cast with typed field declaration (Collection) and doReturn() stubs to eliminate unchecked warnings - DSOAuth2UserServiceTest / DSOidcUserServiceTest: replace raw any(Map.class) matchers with typed ArgumentMatchers.>any() --- .../spring/user/service/DSUserDetails.java | 17 ++++++-- .../user/service/DSOAuth2UserServiceTest.java | 6 ++- .../user/service/DSOidcUserServiceTest.java | 5 ++- .../user/service/DSUserDetailsTest.java | 41 +++++++++++++++++++ .../user/service/LoginHelperServiceTest.java | 33 +++++++-------- 5 files changed, 79 insertions(+), 23 deletions(-) diff --git a/src/main/java/com/digitalsanctuary/spring/user/service/DSUserDetails.java b/src/main/java/com/digitalsanctuary/spring/user/service/DSUserDetails.java index 436b99d..eb3efec 100644 --- a/src/main/java/com/digitalsanctuary/spring/user/service/DSUserDetails.java +++ b/src/main/java/com/digitalsanctuary/spring/user/service/DSUserDetails.java @@ -2,6 +2,7 @@ import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.Map; import org.springframework.security.core.GrantedAuthority; @@ -160,8 +161,18 @@ private static Map buildFallbackAttributes(User user) { if (user.getLastName() != null) { attrs.put("family_name", user.getLastName()); } - if (user.getFirstName() != null || user.getLastName() != null) { - attrs.put("name", user.getFullName()); + StringBuilder name = new StringBuilder(); + if (user.getFirstName() != null && !user.getFirstName().trim().isEmpty()) { + name.append(user.getFirstName().trim()); + } + if (user.getLastName() != null && !user.getLastName().trim().isEmpty()) { + if (name.length() > 0) { + name.append(' '); + } + name.append(user.getLastName().trim()); + } + if (name.length() > 0) { + attrs.put("name", name.toString()); } return attrs; } @@ -247,7 +258,7 @@ public User getUser() { @Override public Map getAttributes() { - return attributes; + return Collections.unmodifiableMap(attributes); } @Override diff --git a/src/test/java/com/digitalsanctuary/spring/user/service/DSOAuth2UserServiceTest.java b/src/test/java/com/digitalsanctuary/spring/user/service/DSOAuth2UserServiceTest.java index 09f4dfe..85dcf51 100644 --- a/src/test/java/com/digitalsanctuary/spring/user/service/DSOAuth2UserServiceTest.java +++ b/src/test/java/com/digitalsanctuary/spring/user/service/DSOAuth2UserServiceTest.java @@ -5,6 +5,8 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; +import org.mockito.ArgumentMatchers; + import static org.mockito.Mockito.*; import java.util.Arrays; @@ -416,7 +418,7 @@ void shouldLoadUserThroughOAuth2RequestFlow() { when(spyService.defaultOAuth2UserService.loadUser(userRequest)).thenReturn(googleUser); DSUserDetails mockUserDetails = mock(DSUserDetails.class); - when(loginHelperService.userLoginHelper(any(User.class), any(Map.class))).thenReturn(mockUserDetails); + when(loginHelperService.userLoginHelper(any(User.class), ArgumentMatchers.>any())).thenReturn(mockUserDetails); when(userRepository.findByEmail("loadtest@gmail.com")).thenReturn(null); when(userRepository.save(any(User.class))).thenAnswer(invocation -> invocation.getArgument(0)); @@ -425,7 +427,7 @@ void shouldLoadUserThroughOAuth2RequestFlow() { // Then assertThat(result).isEqualTo(mockUserDetails); - verify(loginHelperService).userLoginHelper(any(User.class), any(Map.class)); + verify(loginHelperService).userLoginHelper(any(User.class), ArgumentMatchers.>any()); verify(userRepository).save(any(User.class)); } } diff --git a/src/test/java/com/digitalsanctuary/spring/user/service/DSOidcUserServiceTest.java b/src/test/java/com/digitalsanctuary/spring/user/service/DSOidcUserServiceTest.java index c81c107..65f90f5 100644 --- a/src/test/java/com/digitalsanctuary/spring/user/service/DSOidcUserServiceTest.java +++ b/src/test/java/com/digitalsanctuary/spring/user/service/DSOidcUserServiceTest.java @@ -4,6 +4,7 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.*; +import org.mockito.ArgumentMatchers; import java.util.Arrays; import java.util.List; @@ -373,7 +374,7 @@ void shouldLoadUserThroughOidcRequestFlow() { user.setId(999L); return user; }); - when(loginHelperService.userLoginHelper(any(User.class), any(OidcUserInfo.class), any(OidcIdToken.class), any(Map.class))) + when(loginHelperService.userLoginHelper(any(User.class), any(OidcUserInfo.class), any(OidcIdToken.class), ArgumentMatchers.>any())) .thenAnswer(invocation -> { User user = invocation.getArgument(0); OidcUserInfo oidcUserInfo = invocation.getArgument(1); @@ -396,7 +397,7 @@ void shouldLoadUserThroughOidcRequestFlow() { assertThat(dsUserDetails.getAttributes()).isNotEmpty(); verify(userRepository).save(any(User.class)); - verify(loginHelperService).userLoginHelper(any(User.class), any(OidcUserInfo.class), any(OidcIdToken.class), any(Map.class)); + verify(loginHelperService).userLoginHelper(any(User.class), any(OidcUserInfo.class), any(OidcIdToken.class), ArgumentMatchers.>any()); } @Test diff --git a/src/test/java/com/digitalsanctuary/spring/user/service/DSUserDetailsTest.java b/src/test/java/com/digitalsanctuary/spring/user/service/DSUserDetailsTest.java index 0ef0cf1..df125df 100644 --- a/src/test/java/com/digitalsanctuary/spring/user/service/DSUserDetailsTest.java +++ b/src/test/java/com/digitalsanctuary/spring/user/service/DSUserDetailsTest.java @@ -11,6 +11,8 @@ import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.security.core.authority.SimpleGrantedAuthority; import org.springframework.security.oauth2.core.oidc.OidcIdToken; import org.springframework.security.oauth2.core.oidc.OidcUserInfo; @@ -23,6 +25,7 @@ *

Verifies that {@code getAttributes()} satisfies the {@code OAuth2User} contract across all * constructor paths: OAuth2, OIDC, and local/password login.

*/ +@ExtendWith(MockitoExtension.class) @DisplayName("DSUserDetails Tests") class DSUserDetailsTest { @@ -83,6 +86,44 @@ void shouldReturnEmptyMapForMinimalUser() { assertThat(details.getAttributes()).isEmpty(); } + @Test + @DisplayName("Should build name from first name only without 'null' suffix") + void shouldBuildNameFromFirstNameOnly() { + User user = new User(); + user.setFirstName("Test"); + + DSUserDetails details = new DSUserDetails(user); + + assertThat(details.getAttributes()).containsEntry("given_name", "Test"); + assertThat(details.getAttributes()).containsEntry("name", "Test"); + assertThat((String) details.getAttributes().get("name")).doesNotContain("null"); + } + + @Test + @DisplayName("Should build name from last name only without 'null' prefix") + void shouldBuildNameFromLastNameOnly() { + User user = new User(); + user.setLastName("User"); + + DSUserDetails details = new DSUserDetails(user); + + assertThat(details.getAttributes()).containsEntry("family_name", "User"); + assertThat(details.getAttributes()).containsEntry("name", "User"); + assertThat((String) details.getAttributes().get("name")).doesNotContain("null"); + } + + @Test + @DisplayName("Should return an unmodifiable map from getAttributes()") + void shouldReturnUnmodifiableAttributes() { + Map providerAttrs = new HashMap<>(); + providerAttrs.put("email", "test@example.com"); + + DSUserDetails details = new DSUserDetails(testUser, + List.of(new SimpleGrantedAuthority("ROLE_USER")), providerAttrs); + + assertThat(details.getAttributes()).isUnmodifiable(); + } + @Test @DisplayName("getAttribute('email') should return email for OAuth2 user") void shouldSupportGetAttributePattern() { diff --git a/src/test/java/com/digitalsanctuary/spring/user/service/LoginHelperServiceTest.java b/src/test/java/com/digitalsanctuary/spring/user/service/LoginHelperServiceTest.java index beee09a..73de706 100644 --- a/src/test/java/com/digitalsanctuary/spring/user/service/LoginHelperServiceTest.java +++ b/src/test/java/com/digitalsanctuary/spring/user/service/LoginHelperServiceTest.java @@ -2,6 +2,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import java.time.Instant; @@ -96,7 +97,7 @@ void shouldUpdateLastActivityDate() { // Given Date beforeLogin = testUser.getLastActivityDate(); when(loginAttemptService.checkIfUserShouldBeUnlocked(any(User.class))).thenAnswer(invocation -> invocation.getArgument(0)); - when(authorityService.getAuthoritiesFromUser(testUser)).thenReturn((Collection) testAuthorities); + doReturn(testAuthorities).when(authorityService).getAuthoritiesFromUser(testUser); // When DSUserDetails result = loginHelperService.userLoginHelper(testUser); @@ -122,7 +123,7 @@ void shouldCheckUserUnlockStatus() { unlockedUser.setLockedDate(null); when(loginAttemptService.checkIfUserShouldBeUnlocked(testUser)).thenReturn(unlockedUser); - when(authorityService.getAuthoritiesFromUser(unlockedUser)).thenReturn((Collection) testAuthorities); + doReturn(testAuthorities).when(authorityService).getAuthoritiesFromUser(unlockedUser); // When DSUserDetails result = loginHelperService.userLoginHelper(testUser); @@ -138,7 +139,7 @@ void shouldCheckUserUnlockStatus() { void shouldCreateUserDetailsWithAuthorities() { // Given when(loginAttemptService.checkIfUserShouldBeUnlocked(testUser)).thenReturn(testUser); - when(authorityService.getAuthoritiesFromUser(testUser)).thenReturn((Collection) testAuthorities); + doReturn(testAuthorities).when(authorityService).getAuthoritiesFromUser(testUser); // When DSUserDetails result = loginHelperService.userLoginHelper(testUser); @@ -178,7 +179,7 @@ void shouldHandleLockedUserThatRemainsLocked() { testUser.setLockedDate(lockedDate); when(loginAttemptService.checkIfUserShouldBeUnlocked(testUser)).thenReturn(testUser); // User remains locked - when(authorityService.getAuthoritiesFromUser(testUser)).thenReturn((Collection) testAuthorities); + doReturn(testAuthorities).when(authorityService).getAuthoritiesFromUser(testUser); // When DSUserDetails result = loginHelperService.userLoginHelper(testUser); @@ -196,7 +197,7 @@ void shouldHandleDisabledUser() { // Given testUser.setEnabled(false); when(loginAttemptService.checkIfUserShouldBeUnlocked(testUser)).thenReturn(testUser); - when(authorityService.getAuthoritiesFromUser(testUser)).thenReturn((Collection) testAuthorities); + doReturn(testAuthorities).when(authorityService).getAuthoritiesFromUser(testUser); // When DSUserDetails result = loginHelperService.userLoginHelper(testUser); @@ -217,7 +218,7 @@ void shouldPreserveUserStateDuringProcess() { // Note: imageUrl and usingMfa fields don't exist in User class when(loginAttemptService.checkIfUserShouldBeUnlocked(testUser)).thenReturn(testUser); - when(authorityService.getAuthoritiesFromUser(testUser)).thenReturn((Collection) testAuthorities); + doReturn(testAuthorities).when(authorityService).getAuthoritiesFromUser(testUser); // When DSUserDetails result = loginHelperService.userLoginHelper(testUser); @@ -253,7 +254,7 @@ void shouldAutomaticallyUnlockUserAfterDuration() { unlockedUser.setEnabled(true); when(loginAttemptService.checkIfUserShouldBeUnlocked(testUser)).thenReturn(unlockedUser); - when(authorityService.getAuthoritiesFromUser(unlockedUser)).thenReturn((Collection) testAuthorities); + doReturn(testAuthorities).when(authorityService).getAuthoritiesFromUser(unlockedUser); // When DSUserDetails result = loginHelperService.userLoginHelper(testUser); @@ -271,7 +272,7 @@ void shouldTrackTimingOfLastActivityUpdate() { // Given Date testStartTime = new Date(); when(loginAttemptService.checkIfUserShouldBeUnlocked(testUser)).thenReturn(testUser); - when(authorityService.getAuthoritiesFromUser(testUser)).thenReturn((Collection) testAuthorities); + doReturn(testAuthorities).when(authorityService).getAuthoritiesFromUser(testUser); // When DSUserDetails result = loginHelperService.userLoginHelper(testUser); @@ -359,7 +360,7 @@ void shouldCreateCompleteUserDetails() { testUser.setLastName("User"); when(loginAttemptService.checkIfUserShouldBeUnlocked(testUser)).thenReturn(testUser); - when(authorityService.getAuthoritiesFromUser(testUser)).thenReturn((Collection) testAuthorities); + doReturn(testAuthorities).when(authorityService).getAuthoritiesFromUser(testUser); // When DSUserDetails result = loginHelperService.userLoginHelper(testUser); @@ -383,7 +384,7 @@ void shouldHandleOAuth2User() { testUser.setPassword(null); // OAuth2 users don't have passwords when(loginAttemptService.checkIfUserShouldBeUnlocked(testUser)).thenReturn(testUser); - when(authorityService.getAuthoritiesFromUser(testUser)).thenReturn((Collection) testAuthorities); + doReturn(testAuthorities).when(authorityService).getAuthoritiesFromUser(testUser); // When DSUserDetails result = loginHelperService.userLoginHelper(testUser); @@ -407,7 +408,7 @@ void shouldHandleNullLastActivityDate() { testUser.setLastActivityDate(null); when(loginAttemptService.checkIfUserShouldBeUnlocked(testUser)).thenReturn(testUser); - when(authorityService.getAuthoritiesFromUser(testUser)).thenReturn((Collection) testAuthorities); + doReturn(testAuthorities).when(authorityService).getAuthoritiesFromUser(testUser); // When DSUserDetails result = loginHelperService.userLoginHelper(testUser); @@ -422,7 +423,7 @@ void shouldHandleNullLastActivityDate() { void shouldHandleRapidSuccessiveLogins() { // Given when(loginAttemptService.checkIfUserShouldBeUnlocked(testUser)).thenReturn(testUser); - when(authorityService.getAuthoritiesFromUser(testUser)).thenReturn((Collection) testAuthorities); + doReturn(testAuthorities).when(authorityService).getAuthoritiesFromUser(testUser); // When - Simulate rapid successive logins DSUserDetails result1 = loginHelperService.userLoginHelper(testUser); @@ -459,7 +460,7 @@ void shouldPassOAuth2AttributesToDSUserDetails() { providerAttrs.put("picture", "https://example.com/photo.jpg"); when(loginAttemptService.checkIfUserShouldBeUnlocked(testUser)).thenReturn(testUser); - when(authorityService.getAuthoritiesFromUser(testUser)).thenReturn((Collection) testAuthorities); + doReturn(testAuthorities).when(authorityService).getAuthoritiesFromUser(testUser); // When DSUserDetails result = loginHelperService.userLoginHelper(testUser, providerAttrs); @@ -477,7 +478,7 @@ void shouldPassOAuth2AttributesToDSUserDetails() { void shouldFallBackWhenOAuth2AttributesNull() { // Given when(loginAttemptService.checkIfUserShouldBeUnlocked(testUser)).thenReturn(testUser); - when(authorityService.getAuthoritiesFromUser(testUser)).thenReturn((Collection) testAuthorities); + doReturn(testAuthorities).when(authorityService).getAuthoritiesFromUser(testUser); // When DSUserDetails result = loginHelperService.userLoginHelper(testUser); @@ -503,7 +504,7 @@ void shouldPassOidcAttributesToDSUserDetails() { providerAttrs.put("extra_claim", "extra_value"); when(loginAttemptService.checkIfUserShouldBeUnlocked(testUser)).thenReturn(testUser); - when(authorityService.getAuthoritiesFromUser(testUser)).thenReturn((Collection) testAuthorities); + doReturn(testAuthorities).when(authorityService).getAuthoritiesFromUser(testUser); // When DSUserDetails result = loginHelperService.userLoginHelper(testUser, userInfo, idToken, providerAttrs); @@ -528,7 +529,7 @@ void shouldFallBackToIdTokenClaimsWhenOidcAttributesNull() { OidcUserInfo userInfo = new OidcUserInfo(Map.of("sub", "oidc-sub-123")); when(loginAttemptService.checkIfUserShouldBeUnlocked(testUser)).thenReturn(testUser); - when(authorityService.getAuthoritiesFromUser(testUser)).thenReturn((Collection) testAuthorities); + doReturn(testAuthorities).when(authorityService).getAuthoritiesFromUser(testUser); // When DSUserDetails result = loginHelperService.userLoginHelper(testUser, userInfo, idToken);