Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Comment on lines +70 to +74
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

attributes is stored as a mutable HashMap and getAttributes() returns the backing map. That makes the authentication principal externally mutable (callers can do getAttributes().put(...)), which contradicts the “immutable” intent and can lead to hard-to-debug security/context issues. Consider storing an unmodifiable map (e.g., Map.copyOf(...)) and/or returning an unmodifiable/copy from getAttributes().

Copilot uses AI. Check for mistakes.

/**
* 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);
}

/**
Expand All @@ -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;
}

/**
Expand Down Expand Up @@ -189,7 +258,7 @@ public User getUser() {

@Override
public Map<String, Object> getAttributes() {
return attributes;
return Collections.unmodifiableMap(attributes);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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));

Expand All @@ -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));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
Loading
Loading