Merge pull request #285 from devondragon/fix/284-oauth2-attributes-empty

fix(oauth2): populate DSUserDetails.getAttributes() with provider attributes

Author: devondragon

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;
     }
 

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());
     }
 }

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;
@@ -60,15 +61,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<? extends GrantedAuthority> grantedAuthorities) {
+	public DSUserDetails(User user, Collection<? extends GrantedAuthority> grantedAuthorities, Map<String, Object> 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<? extends GrantedAuthority> grantedAuthorities) {
+		this(user, grantedAuthorities, (Map<String, Object>) null);
 	}
 
 	/**
@@ -77,35 +90,91 @@ public DSUserDetails(User user, Collection<? extends GrantedAuthority> grantedAu
 	 * @param user the user
 	 */
 	public DSUserDetails(User user) {
-		this(user, null);
+		this(user, null, (Map<String, Object>) 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<? extends GrantedAuthority> grantedAuthorities) {
+	public DSUserDetails(User user, OidcUserInfo oidcUserInfo, OidcIdToken oidcIdToken,
+			Collection<? extends GrantedAuthority> grantedAuthorities, Map<String, Object> 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<? extends GrantedAuthority> 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<String, Object> buildFallbackAttributes(User user) {
+		Map<String, Object> 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());
+		}
+		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;
 	}
 
 	/**
@@ -189,7 +258,7 @@ public User getUser() {
 
 	@Override
 	public Map<String, Object> getAttributes() {
-		return attributes;
+		return Collections.unmodifiableMap(attributes);
 	}
 
 	@Override

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,41 +35,68 @@ 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<String, Object>) 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<String, Object> attributes) {
         // Updating lastActivity date for this login
         dbUser.setLastActivityDate(new Date());
 
         // Check if the user account is locked, but should be unlocked now, and unlock it
         dbUser = loginAttemptService.checkIfUserShouldBeUnlocked(dbUser);
 
         Collection<? extends GrantedAuthority> 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.
      * @param oidcIdToken  The OIDC ID token.
      * @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<String, Object> attributes) {
         // Updating lastActivity date for this login
         dbUser.setLastActivityDate(new Date());
 
         // Check if the user account is locked, but should be unlocked now, and unlock it
         dbUser = loginAttemptService.checkIfUserShouldBeUnlocked(dbUser);
 
         Collection<? extends GrantedAuthority> authorities = authorityService.getAuthoritiesFromUser(dbUser);
-        return new DSUserDetails(dbUser, oidcUserInfo, oidcIdToken, authorities);
+        return new DSUserDetails(dbUser, oidcUserInfo, oidcIdToken, authorities, attributes);
     }
 }

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))).thenReturn(mockUserDetails);
+            when(loginHelperService.userLoginHelper(any(User.class), ArgumentMatchers.<Map<String, Object>>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));
+            verify(loginHelperService).userLoginHelper(any(User.class), ArgumentMatchers.<Map<String, Object>>any());
             verify(userRepository).save(any(User.class));
         }
     }

src/test/java/com/digitalsanctuary/spring/user/service/DSOidcUserServiceTest.java

@@ -4,9 +4,11 @@
 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;
+import java.util.Map;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Nested;
@@ -372,13 +374,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), ArgumentMatchers.<Map<String, Object>>any()))
                     .thenAnswer(invocation -> {
                         User user = invocation.getArgument(0);
                         OidcUserInfo oidcUserInfo = invocation.getArgument(1);
                         OidcIdToken oidcIdToken = invocation.getArgument(2);
+                        Map<String, Object> 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 +394,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), ArgumentMatchers.<Map<String, Object>>any());
         }
 
         @Test