Skip to content

Commit 64e9d2c

Browse files
EdamijuedaEdamijueda
committed
fix: apply requested changes
1 parent 9c9fcd4 commit 64e9d2c

3 files changed

Lines changed: 43 additions & 54 deletions

File tree

‎src/main/java/com/digitalsanctuary/spring/user/service/UserService.java‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -220,8 +220,6 @@ public String getValue() {
220220

221221
private final PasswordHistoryRepository passwordHistoryRepository;
222222

223-
// private final PasswordEncoder passwordEncoder;
224-
225223
/** The send registration verification email flag. */
226224
@Value("${user.registration.sendVerificationEmail:false}")
227225
private boolean sendRegistrationVerificationEmail;
@@ -243,16 +241,17 @@ public String getValue() {
243241
public User registerNewUserAccount(final UserDto newUserDto) {
244242
TimeLogger timeLogger = new TimeLogger(log, "UserService.registerNewUserAccount");
245243
log.debug("UserService.registerNewUserAccount: called with userDto: {}", newUserDto);
246-
244+
247245
// Validate password match only if both are provided
248-
if (newUserDto.getPassword() != null && newUserDto.getMatchingPassword() != null
246+
if (newUserDto.getPassword() != null && newUserDto.getMatchingPassword() != null
249247
&& !newUserDto.getPassword().equals(newUserDto.getMatchingPassword())) {
250248
throw new IllegalArgumentException("Passwords do not match");
251249
}
252-
250+
253251
if (emailExists(newUserDto.getEmail())) {
254252
log.debug("UserService.registerNewUserAccount: email already exists: {}", newUserDto.getEmail());
255-
throw new UserAlreadyExistException("There is an account with that email address: " + newUserDto.getEmail());
253+
throw new UserAlreadyExistException(
254+
"There is an account with that email address: " + newUserDto.getEmail());
256255
}
257256

258257
// Create a new User entity
@@ -287,7 +286,6 @@ public User saveRegisteredUser(final User user) {
287286
}
288287

289288
private void savePasswordHistory(User user, String encodedPassword) {
290-
log.info("landed in savePasswordHistory");
291289
if (user == null || !StringUtils.hasText(encodedPassword)) {
292290
log.warn("Cannot save password history: user or password is null/empty.");
293291
return;

‎src/main/resources/META-INF/additional-spring-configuration-metadata.json‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -128,57 +128,57 @@
128128
{
129129
"name": "user.security.password.enabled",
130130
"type": "java.lang.String",
131-
"description": "A description for 'user.security.password.enabled'"
131+
"description": "Enable/disable password policy enforcement"
132132
},
133133
{
134134
"name": "user.security.password.min-length",
135135
"type": "java.lang.String",
136-
"description": "A description for 'user.security.password.min-length'"
136+
"description": "Minimum password length"
137137
},
138138
{
139139
"name": "user.security.password.max-length",
140140
"type": "java.lang.String",
141-
"description": "A description for 'user.security.password.max-length'"
141+
"description": "Maximum password length"
142142
},
143143
{
144144
"name": "user.security.password.require-uppercase",
145145
"type": "java.lang.String",
146-
"description": "A description for 'user.security.password.require-uppercase'"
146+
"description": "Require at least one uppercase character"
147147
},
148148
{
149149
"name": "user.security.password.require-lowercase",
150150
"type": "java.lang.String",
151-
"description": "A description for 'user.security.password.require-lowercase'"
151+
"description": "Require at least one lowercase character"
152152
},
153153
{
154154
"name": "user.security.password.require-digit",
155155
"type": "java.lang.String",
156-
"description": "A description for 'user.security.password.require-digit'"
156+
"description": "Require at least one digit"
157157
},
158158
{
159159
"name": "user.security.password.require-special",
160160
"type": "java.lang.String",
161-
"description": "A description for 'user.security.password.require-special'"
161+
"description": "Require at least one special character"
162162
},
163163
{
164164
"name": "user.security.password.special-chars",
165165
"type": "java.lang.String",
166-
"description": "A description for 'user.security.password.special-chars'"
166+
"description": "Allowed special characters"
167167
},
168168
{
169169
"name": "user.security.password.prevent-common-passwords",
170170
"type": "java.lang.String",
171-
"description": "A description for 'user.security.password.prevent-common-passwords'"
171+
"description": "Prevent use of common passwords (dictionary check)"
172172
},
173173
{
174174
"name": "user.security.password.history-count",
175175
"type": "java.lang.String",
176-
"description": "A description for 'user.security.password.history-count'"
176+
"description": "Number of previous passwords to prevent reuse"
177177
},
178178
{
179179
"name": "user.security.password.similarity-threshold",
180180
"type": "java.lang.String",
181-
"description": "A description for 'user.security.password.similarity-threshold'"
181+
"description": "Percentage of similarity allowed with username/email"
182182
},
183183
{
184184
"name": "hibernate.globally_quoted_identifiers",

‎src/test/java/com/digitalsanctuary/spring/user/service/UserServiceTest.java‎

Lines changed: 27 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -80,42 +80,20 @@ public class UserServiceTest {
8080
private DSUserDetailsService dsUserDetailsService;
8181
@Mock
8282
private ApplicationEventPublisher eventPublisher;
83-
8483
@Mock
8584
private AuthorityService authorityService;
86-
87-
@InjectMocks
8885
@Mock
8986
private PasswordHistoryRepository passwordHistoryRepository;
87+
@InjectMocks
9088
private UserService userService;
9189
private User testUser;
9290
private UserDto testUserDto;
9391

9492
@BeforeEach
9593
void setUp() {
96-
testUser = new User();
97-
testUser.setEmail("test@example.com");
98-
testUser.setFirstName("testFirstName");
99-
testUser.setLastName("testLastName");
100-
testUser.setPassword("testPassword");
101-
testUser.setRoles(Collections.singletonList(new Role("ROLE_USER")));
102-
testUser.setEnabled(true);
103-
104-
testUserDto = new UserDto();
105-
testUserDto.setEmail("test@example.com");
106-
testUserDto.setFirstName("testFirstName");
107-
testUserDto.setLastName("testLastName");
108-
testUserDto.setPassword("testPassword");
109-
testUserDto.setRole(1);
110-
11194
// Use centralized test fixtures for consistent test data
11295
testUser = TestFixtures.Users.standardUser();
11396
testUserDto = TestFixtures.DTOs.validUserRegistration();
114-
115-
userService = new UserService(userRepository, tokenRepository, passwordTokenRepository, passwordEncoder,
116-
roleRepository, sessionRegistry,
117-
userEmailService, userVerificationService, authorityService, dsUserDetailsService, eventPublisher,
118-
passwordHistoryRepository);
11997
}
12098

12199
@Test
@@ -147,7 +125,8 @@ void registerNewUserAccount_throwsExceptionWhenUserExist() {
147125
when(userRepository.findByEmail(testUser.getEmail())).thenReturn(testUser);
148126

149127
// When & Then
150-
assertThatThrownBy(() -> userService.registerNewUserAccount(testUserDto)).isInstanceOf(UserAlreadyExistException.class)
128+
assertThatThrownBy(() -> userService.registerNewUserAccount(testUserDto))
129+
.isInstanceOf(UserAlreadyExistException.class)
151130
.hasMessageContaining("There is an account with that email address");
152131
}
153132

@@ -307,7 +286,8 @@ class PasswordResetTokenTests {
307286
void getPasswordResetToken_returnsTokenWhenExists() {
308287
// Given
309288
String tokenString = "test-token-123";
310-
PasswordResetToken token = TokenTestDataBuilder.aPasswordResetToken().withToken(tokenString).forUser(testUser).build();
289+
PasswordResetToken token = TokenTestDataBuilder.aPasswordResetToken().withToken(tokenString)
290+
.forUser(testUser).build();
311291
when(passwordTokenRepository.findByToken(tokenString)).thenReturn(token);
312292

313293
// When
@@ -324,7 +304,8 @@ void getPasswordResetToken_returnsTokenWhenExists() {
324304
void getUserByPasswordResetToken_returnsUserWhenTokenValid() {
325305
// Given
326306
String tokenString = "valid-token";
327-
PasswordResetToken token = TokenTestDataBuilder.aPasswordResetToken().withToken(tokenString).forUser(testUser).build();
307+
PasswordResetToken token = TokenTestDataBuilder.aPasswordResetToken().withToken(tokenString)
308+
.forUser(testUser).build();
328309
when(passwordTokenRepository.findByToken(tokenString)).thenReturn(token);
329310

330311
// When
@@ -340,7 +321,8 @@ void getUserByPasswordResetToken_returnsUserWhenTokenValid() {
340321
void validatePasswordResetToken_returnsValidForFreshToken() {
341322
// Given
342323
String tokenString = "fresh-token";
343-
PasswordResetToken token = TokenTestDataBuilder.aValidPasswordResetToken().withToken(tokenString).expiringInHours(1).build();
324+
PasswordResetToken token = TokenTestDataBuilder.aValidPasswordResetToken().withToken(tokenString)
325+
.expiringInHours(1).build();
344326
when(passwordTokenRepository.findByToken(tokenString)).thenReturn(token);
345327

346328
// When
@@ -370,7 +352,8 @@ void validatePasswordResetToken_returnsInvalidForNullToken() {
370352
void validatePasswordResetToken_returnsExpiredForOldToken() {
371353
// Given
372354
String tokenString = "expired-token";
373-
PasswordResetToken token = TokenTestDataBuilder.anExpiredPasswordResetToken().withToken(tokenString).expiredMinutesAgo(120).build();
355+
PasswordResetToken token = TokenTestDataBuilder.anExpiredPasswordResetToken().withToken(tokenString)
356+
.expiredMinutesAgo(120).build();
374357
when(passwordTokenRepository.findByToken(tokenString)).thenReturn(token);
375358

376359
// When
@@ -431,9 +414,12 @@ void getUsersFromSessionRegistry_returnsActiveUserEmails() {
431414
List<Object> principals = Arrays.asList(user1, user2, stringPrincipal);
432415

433416
when(sessionRegistry.getAllPrincipals()).thenReturn(principals);
434-
when(sessionRegistry.getAllSessions(user1, false)).thenReturn(Arrays.asList(mock(SessionInformation.class)));
435-
when(sessionRegistry.getAllSessions(user2, false)).thenReturn(Arrays.asList(mock(SessionInformation.class)));
436-
when(sessionRegistry.getAllSessions(stringPrincipal, false)).thenReturn(Arrays.asList(mock(SessionInformation.class)));
417+
when(sessionRegistry.getAllSessions(user1, false))
418+
.thenReturn(Arrays.asList(mock(SessionInformation.class)));
419+
when(sessionRegistry.getAllSessions(user2, false))
420+
.thenReturn(Arrays.asList(mock(SessionInformation.class)));
421+
when(sessionRegistry.getAllSessions(stringPrincipal, false))
422+
.thenReturn(Arrays.asList(mock(SessionInformation.class)));
437423

438424
// When
439425
List<String> result = userService.getUsersFromSessionRegistry();
@@ -450,7 +436,8 @@ void getUsersFromSessionRegistry_filtersOutInactiveSessions() {
450436
User inactiveUser = UserTestDataBuilder.aUser().withEmail("inactive@example.com").build();
451437

452438
when(sessionRegistry.getAllPrincipals()).thenReturn(Arrays.asList(activeUser, inactiveUser));
453-
when(sessionRegistry.getAllSessions(activeUser, false)).thenReturn(Arrays.asList(mock(SessionInformation.class)));
439+
when(sessionRegistry.getAllSessions(activeUser, false))
440+
.thenReturn(Arrays.asList(mock(SessionInformation.class)));
454441
when(sessionRegistry.getAllSessions(inactiveUser, false)).thenReturn(Collections.emptyList());
455442

456443
// When
@@ -483,7 +470,8 @@ void authWithoutPassword_authenticatesValidUser() {
483470
when(mockRequest.getSession(true)).thenReturn(mockSession);
484471

485472
try (MockedStatic<RequestContextHolder> mockedHolder = mockStatic(RequestContextHolder.class);
486-
MockedStatic<SecurityContextHolder> mockedSecurityHolder = mockStatic(SecurityContextHolder.class)) {
473+
MockedStatic<SecurityContextHolder> mockedSecurityHolder = mockStatic(
474+
SecurityContextHolder.class)) {
487475

488476
mockedHolder.when(RequestContextHolder::getRequestAttributes).thenReturn(attrs);
489477
SecurityContext securityContext = mock(SecurityContext.class);
@@ -494,7 +482,8 @@ void authWithoutPassword_authenticatesValidUser() {
494482

495483
// Then
496484
verify(securityContext)
497-
.setAuthentication(argThat(auth -> auth.getPrincipal().equals(userDetails) && auth.getAuthorities().equals(authorities)));
485+
.setAuthentication(argThat(auth -> auth.getPrincipal().equals(userDetails)
486+
&& auth.getAuthorities().equals(authorities)));
498487
verify(mockSession).setAttribute(eq("SPRING_SECURITY_CONTEXT"), eq(securityContext));
499488
}
500489
}
@@ -528,7 +517,8 @@ void authWithoutPassword_handlesUserWithNullEmail() {
528517
@DisplayName("authWithoutPassword - handles user not found")
529518
void authWithoutPassword_handlesUserNotFound() {
530519
// Given
531-
when(dsUserDetailsService.loadUserByUsername(testUser.getEmail())).thenThrow(new UsernameNotFoundException("User not found"));
520+
when(dsUserDetailsService.loadUserByUsername(testUser.getEmail()))
521+
.thenThrow(new UsernameNotFoundException("User not found"));
532522

533523
// When
534524
userService.authWithoutPassword(testUser);
@@ -548,7 +538,8 @@ void authWithoutPassword_handlesNoRequestContext() {
548538
when(authorityService.getAuthoritiesFromUser(testUser)).thenReturn((Collection) authorities);
549539

550540
try (MockedStatic<RequestContextHolder> mockedHolder = mockStatic(RequestContextHolder.class);
551-
MockedStatic<SecurityContextHolder> mockedSecurityHolder = mockStatic(SecurityContextHolder.class)) {
541+
MockedStatic<SecurityContextHolder> mockedSecurityHolder = mockStatic(
542+
SecurityContextHolder.class)) {
552543

553544
mockedHolder.when(RequestContextHolder::getRequestAttributes).thenReturn(null);
554545
SecurityContext securityContext = mock(SecurityContext.class);

0 commit comments

Comments
 (0)