Skip to content

Commit bc4be55

Browse files
committed
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.
1 parent 91822e2 commit bc4be55

8 files changed

Lines changed: 455 additions & 22 deletions

File tree

src/main/java/com/digitalsanctuary/spring/user/service/DSOAuth2UserService.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ public OAuth2User loadUser(OAuth2UserRequest userRequest) throws OAuth2Authentic
219219
String registrationId = userRequest.getClientRegistration().getRegistrationId();
220220
log.debug("registrationId: {}", registrationId);
221221
User dbUser = handleOAuthLoginSuccess(registrationId, user);
222-
DSUserDetails userDetails = loginHelperService.userLoginHelper(dbUser);
222+
DSUserDetails userDetails = loginHelperService.userLoginHelper(dbUser, user.getAttributes());
223223
return userDetails;
224224
}
225225

src/main/java/com/digitalsanctuary/spring/user/service/DSOidcUserService.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,6 @@ public OidcUser loadUser(OidcUserRequest userRequest) throws OAuth2Authenticatio
198198
String registrationId = userRequest.getClientRegistration().getRegistrationId();
199199
log.debug("registrationId: {}", registrationId);
200200
User dbUser = handleOidcLoginSuccess(registrationId, user);
201-
return loginHelperService.userLoginHelper(dbUser, user.getUserInfo(), user.getIdToken());
201+
return loginHelperService.userLoginHelper(dbUser, user.getUserInfo(), user.getIdToken(), user.getAttributes());
202202
}
203203
}

src/main/java/com/digitalsanctuary/spring/user/service/DSUserDetails.java

Lines changed: 67 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -60,15 +60,27 @@ public class DSUserDetails implements UserDetails, OidcUser {
6060
private OidcIdToken oidcIdToken;
6161

6262
/**
63-
* Instantiates a new DS user details.
63+
* Instantiates a new DS user details with OAuth2 provider attributes.
6464
*
6565
* @param user the user
6666
* @param grantedAuthorities the granted authorities (optional, default = empty list)
67+
* @param attributes the OAuth2 provider attributes (optional, falls back to User entity fields)
6768
*/
68-
public DSUserDetails(User user, Collection<? extends GrantedAuthority> grantedAuthorities) {
69+
public DSUserDetails(User user, Collection<? extends GrantedAuthority> grantedAuthorities, Map<String, Object> attributes) {
6970
this.user = user;
7071
this.grantedAuthorities = grantedAuthorities != null ? grantedAuthorities : new ArrayList<>();
71-
this.attributes = new HashMap<>();
72+
this.attributes = attributes != null ? new HashMap<>(attributes) : buildFallbackAttributes(user);
73+
}
74+
75+
/**
76+
* Instantiates a new DS user details without provider attributes. Attributes will be populated
77+
* from the {@link User} entity fields as a fallback.
78+
*
79+
* @param user the user
80+
* @param grantedAuthorities the granted authorities (optional, default = empty list)
81+
*/
82+
public DSUserDetails(User user, Collection<? extends GrantedAuthority> grantedAuthorities) {
83+
this(user, grantedAuthorities, (Map<String, Object>) null);
7284
}
7385

7486
/**
@@ -77,35 +89,81 @@ public DSUserDetails(User user, Collection<? extends GrantedAuthority> grantedAu
7789
* @param user the user
7890
*/
7991
public DSUserDetails(User user) {
80-
this(user, null);
92+
this(user, null, (Map<String, Object>) null);
8193
}
8294

8395
/**
84-
* Instantiates a new DS user details.
96+
* Instantiates a new DS user details with OIDC tokens and provider attributes.
8597
*
8698
* @param user the user
8799
* @param oidcUserInfo containing claims about the user
88100
* @param oidcIdToken containing claims about the user
89101
* @param grantedAuthorities the granted authorities (optional, default = empty list)
102+
* @param attributes the OAuth2/OIDC provider attributes (optional, falls back to idToken claims or User entity)
90103
*/
91104
@Builder
92-
public DSUserDetails(User user, OidcUserInfo oidcUserInfo, OidcIdToken oidcIdToken, Collection<? extends GrantedAuthority> grantedAuthorities) {
105+
public DSUserDetails(User user, OidcUserInfo oidcUserInfo, OidcIdToken oidcIdToken,
106+
Collection<? extends GrantedAuthority> grantedAuthorities, Map<String, Object> attributes) {
93107
this.user = user;
94108
this.oidcUserInfo = oidcUserInfo;
95109
this.oidcIdToken = oidcIdToken;
96110
this.grantedAuthorities = grantedAuthorities != null ? grantedAuthorities : new ArrayList<>();
111+
if (attributes != null) {
112+
this.attributes = new HashMap<>(attributes);
113+
} else if (oidcIdToken != null) {
114+
this.attributes = new HashMap<>(oidcIdToken.getClaims());
115+
} else {
116+
this.attributes = buildFallbackAttributes(user);
117+
}
97118
}
98119

99120
/**
100-
* Instantiates a new DS user details.
121+
* Instantiates a new DS user details with OIDC tokens. Attributes will be populated from the
122+
* OIDC ID token claims or {@link User} entity as a fallback.
123+
*
124+
* @param user the user
125+
* @param oidcUserInfo containing claims about the user
126+
* @param oidcIdToken containing claims about the user
127+
* @param grantedAuthorities the granted authorities (optional, default = empty list)
128+
*/
129+
public DSUserDetails(User user, OidcUserInfo oidcUserInfo, OidcIdToken oidcIdToken,
130+
Collection<? extends GrantedAuthority> grantedAuthorities) {
131+
this(user, oidcUserInfo, oidcIdToken, grantedAuthorities, null);
132+
}
133+
134+
/**
135+
* Instantiates a new DS user details with OIDC tokens and no authorities.
101136
*
102137
* @param user the user
103138
* @param oidcUserInfo containing claims about the user
104139
* @param oidcIdToken containing claims about the user
105140
*/
106-
@Builder
107141
public DSUserDetails(User user, OidcUserInfo oidcUserInfo, OidcIdToken oidcIdToken) {
108-
this(user, oidcUserInfo, oidcIdToken, null);
142+
this(user, oidcUserInfo, oidcIdToken, null, null);
143+
}
144+
145+
/**
146+
* Builds a fallback attributes map from the {@link User} entity fields. Used when no provider
147+
* attributes are available (e.g., local/password login).
148+
*
149+
* @param user the user entity
150+
* @return a map containing available user fields using standard OAuth2/OIDC claim names
151+
*/
152+
private static Map<String, Object> buildFallbackAttributes(User user) {
153+
Map<String, Object> attrs = new HashMap<>();
154+
if (user.getEmail() != null) {
155+
attrs.put("email", user.getEmail());
156+
}
157+
if (user.getFirstName() != null) {
158+
attrs.put("given_name", user.getFirstName());
159+
}
160+
if (user.getLastName() != null) {
161+
attrs.put("family_name", user.getLastName());
162+
}
163+
if (user.getFirstName() != null || user.getLastName() != null) {
164+
attrs.put("name", user.getFullName());
165+
}
166+
return attrs;
109167
}
110168

111169
/**

src/main/java/com/digitalsanctuary/spring/user/service/LoginHelperService.java

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import java.util.Collection;
44
import java.util.Date;
5+
import java.util.Map;
56
import org.springframework.security.core.GrantedAuthority;
67
import org.springframework.security.oauth2.core.oidc.OidcIdToken;
78
import org.springframework.security.oauth2.core.oidc.OidcUserInfo;
@@ -34,41 +35,68 @@ public class LoginHelperService {
3435
private final AuthorityService authorityService;
3536

3637
/**
37-
* Helper method to authenticate a user after login. This method is called from the DSUserDetailsService and DSOAuth2UserService classes after a
38-
* user has been successfully authenticated.
38+
* Helper method to authenticate a user after login. This method is called from the DSUserDetailsService after a user has been successfully
39+
* authenticated via local/password login. Attributes are populated from the {@link User} entity as a fallback.
3940
*
4041
* @param dbUser The user to authenticate.
4142
* @return The user details object.
4243
*/
4344
public DSUserDetails userLoginHelper(User dbUser) {
45+
return userLoginHelper(dbUser, (Map<String, Object>) null);
46+
}
47+
48+
/**
49+
* Helper method to authenticate a user after OAuth2 login, preserving the original provider attributes so that
50+
* {@link DSUserDetails#getAttributes()} returns the full set of attributes from the OAuth2 provider.
51+
*
52+
* @param dbUser The user to authenticate.
53+
* @param attributes The OAuth2 provider attributes (may be null for local login fallback).
54+
* @return The user details object with provider attributes set.
55+
*/
56+
public DSUserDetails userLoginHelper(User dbUser, Map<String, Object> attributes) {
4457
// Updating lastActivity date for this login
4558
dbUser.setLastActivityDate(new Date());
4659

4760
// Check if the user account is locked, but should be unlocked now, and unlock it
4861
dbUser = loginAttemptService.checkIfUserShouldBeUnlocked(dbUser);
4962

5063
Collection<? extends GrantedAuthority> authorities = authorityService.getAuthoritiesFromUser(dbUser);
51-
DSUserDetails userDetails = new DSUserDetails(dbUser, authorities);
52-
return userDetails;
64+
return new DSUserDetails(dbUser, authorities, attributes);
5365
}
5466

5567
/**
5668
* Helper method to authenticate an OIDC user after login, attaching the OIDC-specific tokens
57-
* and claims to the principal while keeping {@link DSUserDetails} immutable.
69+
* and claims to the principal while keeping {@link DSUserDetails} immutable. Attributes are
70+
* populated from the OIDC ID token claims as a fallback.
5871
*
5972
* @param dbUser The user to authenticate.
6073
* @param oidcUserInfo The OIDC user info claims.
6174
* @param oidcIdToken The OIDC ID token.
6275
* @return The user details object with OIDC tokens set.
6376
*/
6477
public DSUserDetails userLoginHelper(User dbUser, OidcUserInfo oidcUserInfo, OidcIdToken oidcIdToken) {
78+
return userLoginHelper(dbUser, oidcUserInfo, oidcIdToken, null);
79+
}
80+
81+
/**
82+
* Helper method to authenticate an OIDC user after login, preserving the original provider attributes and
83+
* attaching the OIDC-specific tokens and claims to the principal.
84+
*
85+
* @param dbUser The user to authenticate.
86+
* @param oidcUserInfo The OIDC user info claims.
87+
* @param oidcIdToken The OIDC ID token.
88+
* @param attributes The OIDC provider attributes (may be null to fall back to idToken claims).
89+
* @return The user details object with OIDC tokens and provider attributes set.
90+
*/
91+
public DSUserDetails userLoginHelper(User dbUser, OidcUserInfo oidcUserInfo, OidcIdToken oidcIdToken,
92+
Map<String, Object> attributes) {
6593
// Updating lastActivity date for this login
6694
dbUser.setLastActivityDate(new Date());
6795

6896
// Check if the user account is locked, but should be unlocked now, and unlock it
6997
dbUser = loginAttemptService.checkIfUserShouldBeUnlocked(dbUser);
7098

7199
Collection<? extends GrantedAuthority> authorities = authorityService.getAuthoritiesFromUser(dbUser);
72-
return new DSUserDetails(dbUser, oidcUserInfo, oidcIdToken, authorities);
100+
return new DSUserDetails(dbUser, oidcUserInfo, oidcIdToken, authorities, attributes);
73101
}
74102
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -416,7 +416,7 @@ void shouldLoadUserThroughOAuth2RequestFlow() {
416416
when(spyService.defaultOAuth2UserService.loadUser(userRequest)).thenReturn(googleUser);
417417

418418
DSUserDetails mockUserDetails = mock(DSUserDetails.class);
419-
when(loginHelperService.userLoginHelper(any(User.class))).thenReturn(mockUserDetails);
419+
when(loginHelperService.userLoginHelper(any(User.class), any(Map.class))).thenReturn(mockUserDetails);
420420
when(userRepository.findByEmail("loadtest@gmail.com")).thenReturn(null);
421421
when(userRepository.save(any(User.class))).thenAnswer(invocation -> invocation.getArgument(0));
422422

@@ -425,7 +425,7 @@ void shouldLoadUserThroughOAuth2RequestFlow() {
425425

426426
// Then
427427
assertThat(result).isEqualTo(mockUserDetails);
428-
verify(loginHelperService).userLoginHelper(any(User.class));
428+
verify(loginHelperService).userLoginHelper(any(User.class), any(Map.class));
429429
verify(userRepository).save(any(User.class));
430430
}
431431
}

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
import java.util.Arrays;
99
import java.util.List;
10+
import java.util.Map;
1011
import org.junit.jupiter.api.BeforeEach;
1112
import org.junit.jupiter.api.DisplayName;
1213
import org.junit.jupiter.api.Nested;
@@ -372,13 +373,14 @@ void shouldLoadUserThroughOidcRequestFlow() {
372373
user.setId(999L);
373374
return user;
374375
});
375-
when(loginHelperService.userLoginHelper(any(User.class), any(OidcUserInfo.class), any(OidcIdToken.class)))
376+
when(loginHelperService.userLoginHelper(any(User.class), any(OidcUserInfo.class), any(OidcIdToken.class), any(Map.class)))
376377
.thenAnswer(invocation -> {
377378
User user = invocation.getArgument(0);
378379
OidcUserInfo oidcUserInfo = invocation.getArgument(1);
379380
OidcIdToken oidcIdToken = invocation.getArgument(2);
381+
Map<String, Object> attributes = invocation.getArgument(3);
380382
return new DSUserDetails(user, oidcUserInfo, oidcIdToken,
381-
List.of(new SimpleGrantedAuthority("ROLE_USER")));
383+
List.of(new SimpleGrantedAuthority("ROLE_USER")), attributes);
382384
});
383385

384386
// When
@@ -391,9 +393,10 @@ void shouldLoadUserThroughOidcRequestFlow() {
391393
assertThat(dsUserDetails.getIdToken()).isNotNull();
392394
assertThat(dsUserDetails.getUserInfo()).isNotNull();
393395
assertThat(dsUserDetails.getAuthorities()).isNotEmpty();
396+
assertThat(dsUserDetails.getAttributes()).isNotEmpty();
394397

395398
verify(userRepository).save(any(User.class));
396-
verify(loginHelperService).userLoginHelper(any(User.class), any(OidcUserInfo.class), any(OidcIdToken.class));
399+
verify(loginHelperService).userLoginHelper(any(User.class), any(OidcUserInfo.class), any(OidcIdToken.class), any(Map.class));
397400
}
398401

399402
@Test

0 commit comments

Comments
 (0)