Skip to content

Commit 11428da

Browse files
committed
fix(oidc): align DSOidcUserService behavior with DSOAuth2UserService
Fixes four inconsistencies identified in issue #276: - Add .toLowerCase() to email lookup in handleOidcLoginSuccess() to normalize email case before findByEmail(), matching OAuth2 behavior - Add @transactional at class level and on registerNewOidcUser() to ensure database operations are transactional, matching OAuth2 behavior - Publish AuditEvent on new OIDC user registration (\"OIDC Registration Success\"), matching the OAuth2 \"OAuth2 Registration Success\" event - Call loginHelperService.userLoginHelper() in loadUser() to update lastActivityDate and run lockout checks; set OIDC-specific tokens (idToken, userInfo) via new @Setter fields on DSUserDetails Add @Setter to DSUserDetails.oidcUserInfo and oidcUserInfo to support setting OIDC tokens after loginHelperService creates the instance. Update DSOidcUserServiceTest and DSOidcUserServiceRegistrationGuardTest to inject @mock LoginHelperService and ApplicationEventPublisher. Closes #276
1 parent 1f1c917 commit 11428da

4 files changed

Lines changed: 43 additions & 11 deletions

File tree

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

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.digitalsanctuary.spring.user.service;
22

33
import java.util.Arrays;
4+
import com.digitalsanctuary.spring.user.audit.AuditEvent;
45
import com.digitalsanctuary.spring.user.persistence.model.User;
56
import com.digitalsanctuary.spring.user.persistence.repository.RoleRepository;
67
import com.digitalsanctuary.spring.user.persistence.repository.UserRepository;
@@ -10,6 +11,7 @@
1011
import com.digitalsanctuary.spring.user.registration.RegistrationSource;
1112
import lombok.RequiredArgsConstructor;
1213
import lombok.extern.slf4j.Slf4j;
14+
import org.springframework.context.ApplicationEventPublisher;
1315
import org.springframework.security.oauth2.client.oidc.userinfo.OidcUserRequest;
1416
import org.springframework.security.oauth2.client.oidc.userinfo.OidcUserService;
1517
import org.springframework.security.oauth2.client.userinfo.OAuth2UserRequest;
@@ -19,6 +21,7 @@
1921
import org.springframework.security.oauth2.core.oidc.user.OidcUser;
2022
import org.springframework.security.oauth2.core.user.OAuth2User;
2123
import org.springframework.stereotype.Service;
24+
import org.springframework.transaction.annotation.Transactional;
2225

2326
/**
2427
* OIDC user service implementation for handling OpenID Connect authentication (Keycloak).
@@ -36,17 +39,23 @@
3639
*/
3740
@Slf4j
3841
@Service
42+
@Transactional
3943
@RequiredArgsConstructor
4044
public class DSOidcUserService implements OAuth2UserService<OidcUserRequest, OidcUser> {
4145

4246
/** The user repository. */
4347
private final UserRepository userRepository;
44-
48+
4549
/** The role repository. */
4650
private final RoleRepository roleRepository;
4751

52+
private final LoginHelperService loginHelperService;
53+
4854
private final RegistrationGuard registrationGuard;
4955

56+
/** The Event Publisher. */
57+
private final ApplicationEventPublisher eventPublisher;
58+
5059
OidcUserService defaultOidcUserService = new OidcUserService();
5160

5261
/**
@@ -79,7 +88,7 @@ public User handleOidcLoginSuccess(String registrationId, OidcUser oidcUser) {
7988
"Unable to retrieve email address from " + registrationId + ". Please ensure you have granted email permissions.");
8089
}
8190
log.debug("handleOidcLoginSuccess: looking up user with email: {}", user.getEmail());
82-
User existingUser = userRepository.findByEmail(user.getEmail());
91+
User existingUser = userRepository.findByEmail(user.getEmail().toLowerCase());
8392
log.debug("handleOidcLoginSuccess: existingUser: {}", existingUser);
8493
if (existingUser != null && registrationId != null) {
8594
log.debug("handleOidcLoginSuccess: existingUser.getProvider(): {}", existingUser.getProvider());
@@ -116,12 +125,17 @@ public User handleOidcLoginSuccess(String registrationId, OidcUser oidcUser) {
116125
* @param user The User object representing the authenticated user.
117126
* @return A User object representing the newly registered user.
118127
*/
128+
@Transactional
119129
private User registerNewOidcUser(String registrationId, User user) {
120130
User.Provider provider = User.Provider.valueOf(registrationId.toUpperCase());
121131
user.setProvider(provider);
122132
user.setRoles(Arrays.asList(roleRepository.findByName("ROLE_USER")));
123-
// We will trust OAuth2 providers to provide us with a verified email address.
133+
// We will trust OIDC providers to provide us with a verified email address.
124134
user.setEnabled(true);
135+
AuditEvent registrationAuditEvent = AuditEvent.builder().source(this).user(user).action("OIDC Registration Success").actionStatus("Success")
136+
.message("Registration Confirmed. User logged in.").build();
137+
138+
eventPublisher.publishEvent(registrationAuditEvent);
125139
return userRepository.save(user);
126140
}
127141

@@ -180,12 +194,9 @@ public OidcUser loadUser(OidcUserRequest userRequest) throws OAuth2Authenticatio
180194
String registrationId = userRequest.getClientRegistration().getRegistrationId();
181195
log.debug("registrationId: {}", registrationId);
182196
User dbUser = handleOidcLoginSuccess(registrationId, user);
183-
DSUserDetails dsUserDetails = DSUserDetails.builder()
184-
.user(dbUser)
185-
.oidcUserInfo(user.getUserInfo())
186-
.oidcIdToken(user.getIdToken())
187-
.grantedAuthorities(user.getAuthorities())
188-
.build();
197+
DSUserDetails dsUserDetails = loginHelperService.userLoginHelper(dbUser);
198+
dsUserDetails.setOidcUserInfo(user.getUserInfo());
199+
dsUserDetails.setOidcIdToken(user.getIdToken());
189200
return dsUserDetails;
190201
}
191202
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import org.springframework.security.oauth2.core.oidc.user.OidcUser;
1212
import com.digitalsanctuary.spring.user.persistence.model.User;
1313
import lombok.Builder;
14+
import lombok.Setter;
1415
import lombok.ToString;
1516

1617
/**
@@ -54,9 +55,11 @@ public class DSUserDetails implements UserDetails, OidcUser {
5455
private Map<String, Object> attributes;
5556

5657
/** The Oidc user properties. */
58+
@Setter
5759
private OidcUserInfo oidcUserInfo;
5860

5961
/** The Oidc user token. */
62+
@Setter
6063
private OidcIdToken oidcIdToken;
6164

6265
/**

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import org.mockito.InjectMocks;
1616
import org.mockito.Mock;
1717
import org.mockito.junit.jupiter.MockitoExtension;
18+
import org.springframework.context.ApplicationEventPublisher;
1819
import org.springframework.security.oauth2.core.OAuth2AuthenticationException;
1920
import org.springframework.security.oauth2.core.oidc.user.OidcUser;
2021

@@ -37,9 +38,15 @@ class DSOidcUserServiceRegistrationGuardTest {
3738
@Mock
3839
private RoleRepository roleRepository;
3940

41+
@Mock
42+
private LoginHelperService loginHelperService;
43+
4044
@Mock
4145
private RegistrationGuard registrationGuard;
4246

47+
@Mock
48+
private ApplicationEventPublisher eventPublisher;
49+
4350
@InjectMocks
4451
private DSOidcUserService service;
4552

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

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.springframework.security.oauth2.core.OAuth2AuthenticationException;
2525
import org.springframework.security.oauth2.core.oidc.user.OidcUser;
2626
import com.digitalsanctuary.spring.user.fixtures.OidcUserTestDataBuilder;
27+
import org.springframework.context.ApplicationEventPublisher;
2728
import com.digitalsanctuary.spring.user.persistence.model.Role;
2829
import com.digitalsanctuary.spring.user.persistence.model.User;
2930
import com.digitalsanctuary.spring.user.persistence.repository.RoleRepository;
@@ -45,9 +46,15 @@ class DSOidcUserServiceTest {
4546
@Mock
4647
private RoleRepository roleRepository;
4748

49+
@Mock
50+
private LoginHelperService loginHelperService;
51+
4852
@Mock
4953
private RegistrationGuard registrationGuard;
5054

55+
@Mock
56+
private ApplicationEventPublisher eventPublisher;
57+
5158
@InjectMocks
5259
private DSOidcUserService service;
5360

@@ -360,6 +367,10 @@ void shouldLoadUserThroughOidcRequestFlow() {
360367
user.setId(999L);
361368
return user;
362369
});
370+
when(loginHelperService.userLoginHelper(any(User.class))).thenAnswer(invocation -> {
371+
User user = invocation.getArgument(0);
372+
return new DSUserDetails(user, new ArrayList<>());
373+
});
363374

364375
// When
365376
OidcUser result = spyService.loadUser(userRequest);
@@ -370,9 +381,9 @@ void shouldLoadUserThroughOidcRequestFlow() {
370381
DSUserDetails dsUserDetails = (DSUserDetails) result;
371382
assertThat(dsUserDetails.getIdToken()).isNotNull();
372383
assertThat(dsUserDetails.getUserInfo()).isNotNull();
373-
assertThat(dsUserDetails.getAuthorities()).isNotEmpty();
374-
384+
375385
verify(userRepository).save(any(User.class));
386+
verify(loginHelperService).userLoginHelper(any(User.class));
376387
}
377388

378389
@Test

0 commit comments

Comments
 (0)