From 6a1c06dead2422392c0f33a20d7a821c684097d9 Mon Sep 17 00:00:00 2001 From: Devon Hillard Date: Sat, 17 Jan 2026 20:18:11 -0700 Subject: [PATCH 1/6] Add admin-initiated password reset with session invalidation Introduces SessionInvalidationService for expiring user sessions and adds initiateAdminPasswordReset() to UserEmailService. Enables admins to force password resets while optionally invalidating all active sessions, with full audit trail support. --- .../service/SessionInvalidationService.java | 72 +++++++ .../spring/user/service/UserEmailService.java | 80 +++++++ .../SessionInvalidationServiceTest.java | 197 ++++++++++++++++++ .../user/service/UserEmailServiceTest.java | 107 ++++++++++ 4 files changed, 456 insertions(+) create mode 100644 src/main/java/com/digitalsanctuary/spring/user/service/SessionInvalidationService.java create mode 100644 src/test/java/com/digitalsanctuary/spring/user/service/SessionInvalidationServiceTest.java diff --git a/src/main/java/com/digitalsanctuary/spring/user/service/SessionInvalidationService.java b/src/main/java/com/digitalsanctuary/spring/user/service/SessionInvalidationService.java new file mode 100644 index 0000000..5645ef6 --- /dev/null +++ b/src/main/java/com/digitalsanctuary/spring/user/service/SessionInvalidationService.java @@ -0,0 +1,72 @@ +package com.digitalsanctuary.spring.user.service; + +import java.util.List; +import org.springframework.security.core.session.SessionInformation; +import org.springframework.security.core.session.SessionRegistry; +import org.springframework.stereotype.Service; +import com.digitalsanctuary.spring.user.persistence.model.User; +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; + +/** + * Service for invalidating user sessions. This is useful for admin-initiated password resets + * and other security operations that require forcing users to re-authenticate. + */ +@Slf4j +@RequiredArgsConstructor +@Service +public class SessionInvalidationService { + + private final SessionRegistry sessionRegistry; + + /** + * Invalidates all active sessions for the given user. + * This forces the user to re-authenticate on their next request. + * + * @param user the user whose sessions should be invalidated + * @return the number of sessions that were invalidated + */ + public int invalidateUserSessions(User user) { + if (user == null) { + log.warn("SessionInvalidationService.invalidateUserSessions: user is null"); + return 0; + } + + int invalidatedCount = 0; + List principals = sessionRegistry.getAllPrincipals(); + + for (Object principal : principals) { + User principalUser = extractUser(principal); + + if (principalUser != null && principalUser.getId().equals(user.getId())) { + List sessions = sessionRegistry.getAllSessions(principal, false); + for (SessionInformation session : sessions) { + session.expireNow(); + invalidatedCount++; + log.debug("SessionInvalidationService.invalidateUserSessions: expired session {} for user {}", + session.getSessionId(), user.getEmail()); + } + } + } + + log.info("SessionInvalidationService.invalidateUserSessions: invalidated {} sessions for user {}", + invalidatedCount, user.getEmail()); + return invalidatedCount; + } + + /** + * Extracts the User object from a principal. + * Handles both User and DSUserDetails principal types. + * + * @param principal the security principal + * @return the User object, or null if not extractable + */ + private User extractUser(Object principal) { + if (principal instanceof User user) { + return user; + } else if (principal instanceof DSUserDetails dsUserDetails) { + return dsUserDetails.getUser(); + } + return null; + } +} diff --git a/src/main/java/com/digitalsanctuary/spring/user/service/UserEmailService.java b/src/main/java/com/digitalsanctuary/spring/user/service/UserEmailService.java index 3f809b3..cfaae37 100644 --- a/src/main/java/com/digitalsanctuary/spring/user/service/UserEmailService.java +++ b/src/main/java/com/digitalsanctuary/spring/user/service/UserEmailService.java @@ -3,6 +3,7 @@ import java.util.HashMap; import java.util.Map; import java.util.UUID; +import org.springframework.beans.factory.annotation.Value; import org.springframework.context.ApplicationEventPublisher; import org.springframework.stereotype.Service; import com.digitalsanctuary.spring.user.audit.AuditEvent; @@ -33,6 +34,13 @@ public class UserEmailService { /** The event publisher. */ private final ApplicationEventPublisher eventPublisher; + /** The session invalidation service. */ + private final SessionInvalidationService sessionInvalidationService; + + /** The configured app URL for admin-initiated password resets. */ + @Value("${user.admin.appUrl:#{null}}") + private String configuredAppUrl; + /** * Send forgot password verification email. * @@ -109,4 +117,76 @@ public void createPasswordResetTokenForUser(final User user, final String token) passwordTokenRepository.save(myToken); } + /** + * Initiates an admin-triggered password reset for a user. + * This method: + * 1. Optionally invalidates all active sessions for the user + * 2. Generates a password reset token + * 3. Sends the password reset email + * 4. Publishes an audit event for tracking + * + * @param user the user to reset password for + * @param appUrl the application URL for the reset link + * @param adminIdentifier identifier of the admin initiating the reset (e.g., email) + * @param invalidateSessions whether to invalidate all user sessions + * @return the number of sessions invalidated (0 if invalidateSessions is false) + */ + public int initiateAdminPasswordReset(final User user, final String appUrl, final String adminIdentifier, + final boolean invalidateSessions) { + log.debug("UserEmailService.initiateAdminPasswordReset: called for user: {} by admin: {}", user.getEmail(), + adminIdentifier); + + int invalidatedSessions = 0; + + // Step 1: Optionally invalidate all user sessions + if (invalidateSessions) { + invalidatedSessions = sessionInvalidationService.invalidateUserSessions(user); + log.info("UserEmailService.initiateAdminPasswordReset: invalidated {} sessions for user {}", + invalidatedSessions, user.getEmail()); + } + + // Step 2: Generate token and create password reset token + final String token = generateToken(); + createPasswordResetTokenForUser(user, token); + + // Step 3: Publish admin-specific audit event + String auditMessage = String.format("Admin-initiated password reset by %s. Sessions invalidated: %d", + adminIdentifier, invalidatedSessions); + AuditEvent adminPasswordResetAuditEvent = AuditEvent.builder() + .source(this) + .user(user) + .action("adminInitiatedPasswordReset") + .actionStatus("Success") + .message(auditMessage) + .extraData("adminIdentifier:" + adminIdentifier + ",sessionsInvalidated:" + invalidatedSessions) + .build(); + + eventPublisher.publishEvent(adminPasswordResetAuditEvent); + + // Step 4: Send password reset email + Map variables = createEmailVariables(user, appUrl, token, "/user/changePassword?token="); + mailService.sendTemplateMessage(user.getEmail(), "Password Reset", variables, "mail/forgot-password-token.html"); + + log.info("UserEmailService.initiateAdminPasswordReset: password reset email sent to {} by admin {}", + user.getEmail(), adminIdentifier); + + return invalidatedSessions; + } + + /** + * Convenience overload that uses the configured appUrl and invalidates sessions by default. + * + * @param user the user to reset password for + * @param adminIdentifier identifier of the admin initiating the reset (e.g., email) + * @return the number of sessions invalidated + * @throws IllegalStateException if user.admin.appUrl is not configured + */ + public int initiateAdminPasswordReset(final User user, final String adminIdentifier) { + if (configuredAppUrl == null || configuredAppUrl.isBlank()) { + throw new IllegalStateException( + "user.admin.appUrl must be configured to use initiateAdminPasswordReset without explicit appUrl"); + } + return initiateAdminPasswordReset(user, configuredAppUrl, adminIdentifier, true); + } + } diff --git a/src/test/java/com/digitalsanctuary/spring/user/service/SessionInvalidationServiceTest.java b/src/test/java/com/digitalsanctuary/spring/user/service/SessionInvalidationServiceTest.java new file mode 100644 index 0000000..3851a48 --- /dev/null +++ b/src/test/java/com/digitalsanctuary/spring/user/service/SessionInvalidationServiceTest.java @@ -0,0 +1,197 @@ +package com.digitalsanctuary.spring.user.service; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.*; + +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.security.core.session.SessionInformation; +import org.springframework.security.core.session.SessionRegistry; + +import com.digitalsanctuary.spring.user.persistence.model.User; +import com.digitalsanctuary.spring.user.test.builders.UserTestDataBuilder; + +@ExtendWith(MockitoExtension.class) +@DisplayName("SessionInvalidationService Tests") +class SessionInvalidationServiceTest { + + @Mock + private SessionRegistry sessionRegistry; + + @InjectMocks + private SessionInvalidationService sessionInvalidationService; + + private User testUser; + + @BeforeEach + void setUp() { + testUser = UserTestDataBuilder.aUser() + .withId(1L) + .withEmail("test@example.com") + .withFirstName("Test") + .withLastName("User") + .enabled() + .build(); + } + + @Nested + @DisplayName("invalidateUserSessions Tests") + class InvalidateUserSessionsTests { + + @Test + @DisplayName("invalidates all sessions for user with User principal") + void invalidatesAllSessionsForUserWithUserPrincipal() { + // Given + SessionInformation session1 = mock(SessionInformation.class); + SessionInformation session2 = mock(SessionInformation.class); + when(session1.getSessionId()).thenReturn("session-1"); + when(session2.getSessionId()).thenReturn("session-2"); + + when(sessionRegistry.getAllPrincipals()).thenReturn(List.of(testUser)); + when(sessionRegistry.getAllSessions(testUser, false)).thenReturn(Arrays.asList(session1, session2)); + + // When + int invalidatedCount = sessionInvalidationService.invalidateUserSessions(testUser); + + // Then + assertThat(invalidatedCount).isEqualTo(2); + verify(session1).expireNow(); + verify(session2).expireNow(); + } + + @Test + @DisplayName("invalidates all sessions for user with DSUserDetails principal") + void invalidatesAllSessionsForUserWithDSUserDetailsPrincipal() { + // Given + DSUserDetails userDetails = new DSUserDetails(testUser); + SessionInformation session = mock(SessionInformation.class); + when(session.getSessionId()).thenReturn("session-1"); + + when(sessionRegistry.getAllPrincipals()).thenReturn(List.of(userDetails)); + when(sessionRegistry.getAllSessions(userDetails, false)).thenReturn(List.of(session)); + + // When + int invalidatedCount = sessionInvalidationService.invalidateUserSessions(testUser); + + // Then + assertThat(invalidatedCount).isEqualTo(1); + verify(session).expireNow(); + } + + @Test + @DisplayName("returns 0 when user has no active sessions") + void returnsZeroWhenUserHasNoActiveSessions() { + // Given + when(sessionRegistry.getAllPrincipals()).thenReturn(List.of(testUser)); + when(sessionRegistry.getAllSessions(testUser, false)).thenReturn(Collections.emptyList()); + + // When + int invalidatedCount = sessionInvalidationService.invalidateUserSessions(testUser); + + // Then + assertThat(invalidatedCount).isEqualTo(0); + } + + @Test + @DisplayName("returns 0 when user is not in session registry") + void returnsZeroWhenUserNotInSessionRegistry() { + // Given + User otherUser = UserTestDataBuilder.aUser() + .withId(2L) + .withEmail("other@example.com") + .build(); + when(sessionRegistry.getAllPrincipals()).thenReturn(List.of(otherUser)); + + // When + int invalidatedCount = sessionInvalidationService.invalidateUserSessions(testUser); + + // Then + assertThat(invalidatedCount).isEqualTo(0); + } + + @Test + @DisplayName("returns 0 when session registry is empty") + void returnsZeroWhenSessionRegistryIsEmpty() { + // Given + when(sessionRegistry.getAllPrincipals()).thenReturn(Collections.emptyList()); + + // When + int invalidatedCount = sessionInvalidationService.invalidateUserSessions(testUser); + + // Then + assertThat(invalidatedCount).isEqualTo(0); + } + + @Test + @DisplayName("returns 0 when user is null") + void returnsZeroWhenUserIsNull() { + // When + int invalidatedCount = sessionInvalidationService.invalidateUserSessions(null); + + // Then + assertThat(invalidatedCount).isEqualTo(0); + verify(sessionRegistry, never()).getAllPrincipals(); + } + + @Test + @DisplayName("only invalidates sessions for matching user") + void onlyInvalidatesSessionsForMatchingUser() { + // Given + User otherUser = UserTestDataBuilder.aUser() + .withId(2L) + .withEmail("other@example.com") + .build(); + + SessionInformation testUserSession = mock(SessionInformation.class); + when(testUserSession.getSessionId()).thenReturn("test-session"); + + when(sessionRegistry.getAllPrincipals()).thenReturn(Arrays.asList(testUser, otherUser)); + when(sessionRegistry.getAllSessions(testUser, false)).thenReturn(List.of(testUserSession)); + // Note: We don't stub otherUser sessions because our code only calls getAllSessions + // for principals that match the target user by ID + + // When + int invalidatedCount = sessionInvalidationService.invalidateUserSessions(testUser); + + // Then + assertThat(invalidatedCount).isEqualTo(1); + verify(testUserSession).expireNow(); + // Verify otherUser sessions were not touched + verify(sessionRegistry, never()).getAllSessions(otherUser, false); + } + + @Test + @DisplayName("handles mixed principal types correctly") + void handlesMixedPrincipalTypesCorrectly() { + // Given + DSUserDetails userDetails = new DSUserDetails(testUser); + SessionInformation session1 = mock(SessionInformation.class); + SessionInformation session2 = mock(SessionInformation.class); + when(session1.getSessionId()).thenReturn("session-1"); + when(session2.getSessionId()).thenReturn("session-2"); + + // Same user logged in with both User and DSUserDetails principals + when(sessionRegistry.getAllPrincipals()).thenReturn(Arrays.asList(testUser, userDetails)); + when(sessionRegistry.getAllSessions(testUser, false)).thenReturn(List.of(session1)); + when(sessionRegistry.getAllSessions(userDetails, false)).thenReturn(List.of(session2)); + + // When + int invalidatedCount = sessionInvalidationService.invalidateUserSessions(testUser); + + // Then + assertThat(invalidatedCount).isEqualTo(2); + verify(session1).expireNow(); + verify(session2).expireNow(); + } + } +} diff --git a/src/test/java/com/digitalsanctuary/spring/user/service/UserEmailServiceTest.java b/src/test/java/com/digitalsanctuary/spring/user/service/UserEmailServiceTest.java index 7c8bc69..55f2359 100644 --- a/src/test/java/com/digitalsanctuary/spring/user/service/UserEmailServiceTest.java +++ b/src/test/java/com/digitalsanctuary/spring/user/service/UserEmailServiceTest.java @@ -40,6 +40,9 @@ class UserEmailServiceTest { @Mock private ApplicationEventPublisher eventPublisher; + @Mock + private SessionInvalidationService sessionInvalidationService; + @InjectMocks private UserEmailService userEmailService; @@ -344,4 +347,108 @@ void user_canRequestBothRegistrationAndPasswordReset() { ); } } + + @Nested + @DisplayName("Admin Password Reset Tests") + class AdminPasswordResetTests { + + private String adminIdentifier; + + @BeforeEach + void setUp() { + adminIdentifier = "admin@example.com"; + } + + @Test + @DisplayName("initiateAdminPasswordReset - sends email and invalidates sessions when requested") + void initiateAdminPasswordReset_sendsEmailAndInvalidatesSessions() { + // Given + when(sessionInvalidationService.invalidateUserSessions(testUser)).thenReturn(3); + + // When + int invalidatedCount = userEmailService.initiateAdminPasswordReset(testUser, appUrl, adminIdentifier, true); + + // Then + assertThat(invalidatedCount).isEqualTo(3); + + // Verify sessions were invalidated + verify(sessionInvalidationService).invalidateUserSessions(testUser); + + // Verify password reset token was created + ArgumentCaptor tokenCaptor = ArgumentCaptor.forClass(PasswordResetToken.class); + verify(passwordTokenRepository).save(tokenCaptor.capture()); + PasswordResetToken savedToken = tokenCaptor.getValue(); + assertThat(savedToken.getUser()).isEqualTo(testUser); + + // Verify audit event was published with admin info + ArgumentCaptor auditCaptor = ArgumentCaptor.forClass(AuditEvent.class); + verify(eventPublisher).publishEvent(auditCaptor.capture()); + AuditEvent auditEvent = auditCaptor.getValue(); + assertThat(auditEvent.getAction()).isEqualTo("adminInitiatedPasswordReset"); + assertThat(auditEvent.getActionStatus()).isEqualTo("Success"); + assertThat(auditEvent.getMessage()).contains(adminIdentifier); + assertThat(auditEvent.getExtraData()).contains("adminIdentifier:" + adminIdentifier); + assertThat(auditEvent.getExtraData()).contains("sessionsInvalidated:3"); + + // Verify email was sent + verify(mailService).sendTemplateMessage( + eq(testUser.getEmail()), + eq("Password Reset"), + any(), + eq("mail/forgot-password-token.html") + ); + } + + @Test + @DisplayName("initiateAdminPasswordReset - skips session invalidation when not requested") + void initiateAdminPasswordReset_skipsSessionInvalidationWhenNotRequested() { + // When + int invalidatedCount = userEmailService.initiateAdminPasswordReset(testUser, appUrl, adminIdentifier, false); + + // Then + assertThat(invalidatedCount).isEqualTo(0); + + // Verify sessions were NOT invalidated + verify(sessionInvalidationService, never()).invalidateUserSessions(any()); + + // Verify email was still sent + verify(mailService).sendTemplateMessage( + eq(testUser.getEmail()), + eq("Password Reset"), + any(), + eq("mail/forgot-password-token.html") + ); + + // Verify audit event shows 0 sessions invalidated + ArgumentCaptor auditCaptor = ArgumentCaptor.forClass(AuditEvent.class); + verify(eventPublisher).publishEvent(auditCaptor.capture()); + AuditEvent auditEvent = auditCaptor.getValue(); + assertThat(auditEvent.getExtraData()).contains("sessionsInvalidated:0"); + } + + @Test + @DisplayName("initiateAdminPasswordReset - creates token and sends email correctly") + void initiateAdminPasswordReset_createsTokenAndSendsEmail() { + // Given + when(sessionInvalidationService.invalidateUserSessions(testUser)).thenReturn(0); + + // When + userEmailService.initiateAdminPasswordReset(testUser, appUrl, adminIdentifier, true); + + // Then + ArgumentCaptor> variablesCaptor = ArgumentCaptor.forClass(Map.class); + verify(mailService).sendTemplateMessage( + eq(testUser.getEmail()), + eq("Password Reset"), + variablesCaptor.capture(), + eq("mail/forgot-password-token.html") + ); + + Map variables = variablesCaptor.getValue(); + assertThat(variables).containsKey("token"); + assertThat(variables.get("appUrl")).isEqualTo(appUrl); + assertThat(variables.get("confirmationUrl")).asString() + .startsWith(appUrl + "/user/changePassword?token="); + } + } } \ No newline at end of file From 336f01332fa7bbe3733f4e2af8dee154e7bbde68 Mon Sep 17 00:00:00 2001 From: Devon Hillard Date: Sat, 17 Jan 2026 20:39:57 -0700 Subject: [PATCH 2/6] Fix security and architecture issues from code review - Add @PreAuthorize("hasRole('ROLE_ADMIN')") to admin password reset methods - Extract admin identity from SecurityContext instead of user input (fixes log injection) - Add URL validation to reject dangerous schemes (javascript:, data:) - Extract helper methods for SRP compliance - Add correlation ID for audit trail tracking - Add configurable warn threshold for session invalidation performance - Document race condition in SessionInvalidationService - Deprecate methods with adminIdentifier parameter - Add comprehensive tests for new security features --- .../service/SessionInvalidationService.java | 31 +- .../spring/user/service/UserEmailService.java | 212 ++++++++++++-- .../SessionInvalidationServiceTest.java | 80 +++++ .../user/service/UserEmailServiceTest.java | 277 +++++++++++++++++- 4 files changed, 557 insertions(+), 43 deletions(-) diff --git a/src/main/java/com/digitalsanctuary/spring/user/service/SessionInvalidationService.java b/src/main/java/com/digitalsanctuary/spring/user/service/SessionInvalidationService.java index 5645ef6..90f4752 100644 --- a/src/main/java/com/digitalsanctuary/spring/user/service/SessionInvalidationService.java +++ b/src/main/java/com/digitalsanctuary/spring/user/service/SessionInvalidationService.java @@ -1,6 +1,7 @@ package com.digitalsanctuary.spring.user.service; import java.util.List; +import org.springframework.beans.factory.annotation.Value; import org.springframework.security.core.session.SessionInformation; import org.springframework.security.core.session.SessionRegistry; import org.springframework.stereotype.Service; @@ -11,6 +12,13 @@ /** * Service for invalidating user sessions. This is useful for admin-initiated password resets * and other security operations that require forcing users to re-authenticate. + * + *

Race Condition Note: This service uses Spring's SessionRegistry to track + * and invalidate sessions. Due to the nature of the SessionRegistry API, there is an inherent + * race condition: sessions created after {@link SessionRegistry#getAllPrincipals()} is called + * but before {@link SessionInformation#expireNow()} completes will not be invalidated. This is + * a known limitation of the SessionRegistry approach. For most use cases (admin password reset), + * this is acceptable as the window is very small.

*/ @Slf4j @RequiredArgsConstructor @@ -19,10 +27,18 @@ public class SessionInvalidationService { private final SessionRegistry sessionRegistry; + /** Threshold for warning about high principal count that may impact performance. */ + @Value("${user.session.invalidation.warn-threshold:1000}") + private int warnThreshold; + /** * Invalidates all active sessions for the given user. * This forces the user to re-authenticate on their next request. * + *

Note: Sessions created after this method starts iterating + * but before it completes will not be invalidated. This race condition is inherent + * to the SessionRegistry API and is acceptable for most security operations.

+ * * @param user the user whose sessions should be invalidated * @return the number of sessions that were invalidated */ @@ -35,6 +51,17 @@ public int invalidateUserSessions(User user) { int invalidatedCount = 0; List principals = sessionRegistry.getAllPrincipals(); + // Performance monitoring: warn if principal count is high + if (principals.size() > warnThreshold) { + log.warn("SessionInvalidationService.invalidateUserSessions: high principal count ({}) may impact performance", + principals.size()); + } + + log.debug("SessionInvalidationService.invalidateUserSessions: scanning {} principals for user {}", + principals.size(), user.getEmail()); + + // NOTE: Sessions created after getAllPrincipals() but before expireNow() + // will not be invalidated. This is a known limitation of SessionRegistry. for (Object principal : principals) { User principalUser = extractUser(principal); @@ -49,8 +76,8 @@ public int invalidateUserSessions(User user) { } } - log.info("SessionInvalidationService.invalidateUserSessions: invalidated {} sessions for user {}", - invalidatedCount, user.getEmail()); + log.info("SessionInvalidationService.invalidateUserSessions: invalidated {} sessions for user {} (scanned {} principals)", + invalidatedCount, user.getEmail(), principals.size()); return invalidatedCount; } diff --git a/src/main/java/com/digitalsanctuary/spring/user/service/UserEmailService.java b/src/main/java/com/digitalsanctuary/spring/user/service/UserEmailService.java index cfaae37..32cd583 100644 --- a/src/main/java/com/digitalsanctuary/spring/user/service/UserEmailService.java +++ b/src/main/java/com/digitalsanctuary/spring/user/service/UserEmailService.java @@ -1,10 +1,15 @@ package com.digitalsanctuary.spring.user.service; +import java.net.URI; +import java.net.URISyntaxException; import java.util.HashMap; import java.util.Map; import java.util.UUID; import org.springframework.beans.factory.annotation.Value; import org.springframework.context.ApplicationEventPublisher; +import org.springframework.security.access.prepost.PreAuthorize; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.stereotype.Service; import com.digitalsanctuary.spring.user.audit.AuditEvent; import com.digitalsanctuary.spring.user.mail.MailService; @@ -87,6 +92,7 @@ public void sendRegistrationVerificationEmail(final User user, final String appU * @param token the token * @param confirmationPath the confirmation path * @return the map + * @throws IllegalArgumentException if appUrl is invalid (for admin-initiated resets) */ private Map createEmailVariables(final User user, final String appUrl, final String token, final String confirmationPath) { Map variables = new HashMap<>(); @@ -97,6 +103,65 @@ private Map createEmailVariables(final User user, final String a return variables; } + /** + * Creates the email variables with URL validation. + * This is used for admin-initiated password resets where URL validation is critical. + * + * @param user the user + * @param appUrl the app url + * @param token the token + * @param confirmationPath the confirmation path + * @return the map + * @throws IllegalArgumentException if appUrl is invalid + */ + private Map createEmailVariablesWithValidation(final User user, final String appUrl, final String token, + final String confirmationPath) { + if (!isValidAppUrl(appUrl)) { + throw new IllegalArgumentException("Invalid application URL: " + appUrl); + } + return createEmailVariables(user, appUrl, token, confirmationPath); + } + + /** + * Validates that the given URL is a valid HTTP/HTTPS URL. + * Rejects dangerous URL schemes like javascript:, data:, etc. + * + * @param url the URL to validate + * @return true if the URL is valid and safe, false otherwise + */ + private boolean isValidAppUrl(final String url) { + if (url == null || url.isBlank()) { + return false; + } + try { + URI uri = new URI(url); + String scheme = uri.getScheme(); + return scheme != null && (scheme.equalsIgnoreCase("http") || scheme.equalsIgnoreCase("https")) + && uri.getHost() != null; + } catch (URISyntaxException e) { + return false; + } + } + + /** + * Gets the current admin's identifier from the SecurityContext. + * This is used for audit logging to ensure the admin identity is derived + * from the authenticated principal rather than user-supplied input. + * + * @return the admin identifier (email or username), or "UNKNOWN_ADMIN" if not authenticated + */ + private String getCurrentAdminIdentifier() { + Authentication auth = SecurityContextHolder.getContext().getAuthentication(); + if (auth == null || !auth.isAuthenticated()) { + return "UNKNOWN_ADMIN"; + } + Object principal = auth.getPrincipal(); + if (principal instanceof DSUserDetails details) { + return details.getUser().getEmail(); + } + return auth.getName(); + } + /** * Generate random token. * @@ -125,50 +190,38 @@ public void createPasswordResetTokenForUser(final User user, final String token) * 3. Sends the password reset email * 4. Publishes an audit event for tracking * + *

Note: Email sending is asynchronous with retry. Delivery status is logged + * but not returned. The audit event provides tracking for admin actions.

+ * * @param user the user to reset password for - * @param appUrl the application URL for the reset link - * @param adminIdentifier identifier of the admin initiating the reset (e.g., email) + * @param appUrl the application URL for the reset link (must be valid HTTP/HTTPS URL) * @param invalidateSessions whether to invalidate all user sessions * @return the number of sessions invalidated (0 if invalidateSessions is false) + * @throws IllegalArgumentException if appUrl is invalid */ - public int initiateAdminPasswordReset(final User user, final String appUrl, final String adminIdentifier, - final boolean invalidateSessions) { - log.debug("UserEmailService.initiateAdminPasswordReset: called for user: {} by admin: {}", user.getEmail(), - adminIdentifier); + @PreAuthorize("hasRole('ROLE_ADMIN')") + public int initiateAdminPasswordReset(final User user, final String appUrl, final boolean invalidateSessions) { + final String correlationId = generateToken(); + final String adminIdentifier = getCurrentAdminIdentifier(); - int invalidatedSessions = 0; + log.debug("UserEmailService.initiateAdminPasswordReset: called for user: {} by admin: {} [correlationId={}]", + user.getEmail(), adminIdentifier, correlationId); // Step 1: Optionally invalidate all user sessions - if (invalidateSessions) { - invalidatedSessions = sessionInvalidationService.invalidateUserSessions(user); - log.info("UserEmailService.initiateAdminPasswordReset: invalidated {} sessions for user {}", - invalidatedSessions, user.getEmail()); - } + int invalidatedSessions = handleSessionInvalidation(user, invalidateSessions, correlationId); // Step 2: Generate token and create password reset token final String token = generateToken(); createPasswordResetTokenForUser(user, token); // Step 3: Publish admin-specific audit event - String auditMessage = String.format("Admin-initiated password reset by %s. Sessions invalidated: %d", - adminIdentifier, invalidatedSessions); - AuditEvent adminPasswordResetAuditEvent = AuditEvent.builder() - .source(this) - .user(user) - .action("adminInitiatedPasswordReset") - .actionStatus("Success") - .message(auditMessage) - .extraData("adminIdentifier:" + adminIdentifier + ",sessionsInvalidated:" + invalidatedSessions) - .build(); - - eventPublisher.publishEvent(adminPasswordResetAuditEvent); + publishAdminPasswordResetAuditEvent(user, adminIdentifier, invalidatedSessions, correlationId); // Step 4: Send password reset email - Map variables = createEmailVariables(user, appUrl, token, "/user/changePassword?token="); - mailService.sendTemplateMessage(user.getEmail(), "Password Reset", variables, "mail/forgot-password-token.html"); + sendPasswordResetEmail(user, appUrl, token); - log.info("UserEmailService.initiateAdminPasswordReset: password reset email sent to {} by admin {}", - user.getEmail(), adminIdentifier); + log.info("UserEmailService.initiateAdminPasswordReset: password reset email sent to {} by admin {} [correlationId={}]", + user.getEmail(), adminIdentifier, correlationId); return invalidatedSessions; } @@ -176,17 +229,114 @@ public int initiateAdminPasswordReset(final User user, final String appUrl, fina /** * Convenience overload that uses the configured appUrl and invalidates sessions by default. * + *

Note: Email sending is asynchronous with retry. Delivery status is logged + * but not returned. The audit event provides tracking for admin actions.

+ * * @param user the user to reset password for - * @param adminIdentifier identifier of the admin initiating the reset (e.g., email) * @return the number of sessions invalidated * @throws IllegalStateException if user.admin.appUrl is not configured */ - public int initiateAdminPasswordReset(final User user, final String adminIdentifier) { + @PreAuthorize("hasRole('ROLE_ADMIN')") + public int initiateAdminPasswordReset(final User user) { if (configuredAppUrl == null || configuredAppUrl.isBlank()) { throw new IllegalStateException( "user.admin.appUrl must be configured to use initiateAdminPasswordReset without explicit appUrl"); } - return initiateAdminPasswordReset(user, configuredAppUrl, adminIdentifier, true); + return initiateAdminPasswordReset(user, configuredAppUrl, true); + } + + /** + * Initiates an admin-triggered password reset for a user. + * + * @param user the user to reset password for + * @param appUrl the application URL for the reset link + * @param adminIdentifier identifier of the admin initiating the reset (ignored, derived from SecurityContext) + * @param invalidateSessions whether to invalidate all user sessions + * @return the number of sessions invalidated (0 if invalidateSessions is false) + * @deprecated Use {@link #initiateAdminPasswordReset(User, String, boolean)} instead. + * The adminIdentifier is now derived from the SecurityContext for security. + */ + @Deprecated(forRemoval = true) + @PreAuthorize("hasRole('ROLE_ADMIN')") + public int initiateAdminPasswordReset(final User user, final String appUrl, final String adminIdentifier, + final boolean invalidateSessions) { + log.warn("UserEmailService.initiateAdminPasswordReset: adminIdentifier parameter is deprecated and ignored. " + + "Admin identity is now derived from SecurityContext."); + return initiateAdminPasswordReset(user, appUrl, invalidateSessions); + } + + /** + * Convenience overload that uses the configured appUrl and invalidates sessions by default. + * + * @param user the user to reset password for + * @param adminIdentifier identifier of the admin initiating the reset (ignored, derived from SecurityContext) + * @return the number of sessions invalidated + * @throws IllegalStateException if user.admin.appUrl is not configured + * @deprecated Use {@link #initiateAdminPasswordReset(User)} instead. + * The adminIdentifier is now derived from the SecurityContext for security. + */ + @Deprecated(forRemoval = true) + @PreAuthorize("hasRole('ROLE_ADMIN')") + public int initiateAdminPasswordReset(final User user, final String adminIdentifier) { + log.warn("UserEmailService.initiateAdminPasswordReset: adminIdentifier parameter is deprecated and ignored. " + + "Admin identity is now derived from SecurityContext."); + return initiateAdminPasswordReset(user); + } + + /** + * Handles optional session invalidation for admin password reset. + * + * @param user the user whose sessions may be invalidated + * @param invalidateSessions whether to invalidate sessions + * @param correlationId the correlation ID for tracking + * @return the number of sessions invalidated + */ + private int handleSessionInvalidation(final User user, final boolean invalidateSessions, final String correlationId) { + if (!invalidateSessions) { + return 0; + } + int invalidatedCount = sessionInvalidationService.invalidateUserSessions(user); + log.info("UserEmailService.initiateAdminPasswordReset: invalidated {} sessions for user {} [correlationId={}]", + invalidatedCount, user.getEmail(), correlationId); + return invalidatedCount; + } + + /** + * Publishes an audit event for admin-initiated password reset. + * + * @param user the user whose password is being reset + * @param adminIdentifier the admin's identifier + * @param invalidatedSessions the number of sessions invalidated + * @param correlationId the correlation ID for tracking + */ + private void publishAdminPasswordResetAuditEvent(final User user, final String adminIdentifier, + final int invalidatedSessions, final String correlationId) { + String auditMessage = String.format("Admin-initiated password reset by %s. Sessions invalidated: %d", + adminIdentifier, invalidatedSessions); + AuditEvent adminPasswordResetAuditEvent = AuditEvent.builder() + .source(this) + .user(user) + .action("adminInitiatedPasswordReset") + .actionStatus("Success") + .message(auditMessage) + .extraData(String.format("adminIdentifier:%s,sessionsInvalidated:%d,correlationId:%s", + adminIdentifier, invalidatedSessions, correlationId)) + .build(); + + eventPublisher.publishEvent(adminPasswordResetAuditEvent); + } + + /** + * Sends the password reset email to the user. + * + * @param user the user to send the email to + * @param appUrl the application URL for the reset link + * @param token the password reset token + * @throws IllegalArgumentException if appUrl is invalid + */ + private void sendPasswordResetEmail(final User user, final String appUrl, final String token) { + Map variables = createEmailVariablesWithValidation(user, appUrl, token, "/user/changePassword?token="); + mailService.sendTemplateMessage(user.getEmail(), "Password Reset", variables, "mail/forgot-password-token.html"); } } diff --git a/src/test/java/com/digitalsanctuary/spring/user/service/SessionInvalidationServiceTest.java b/src/test/java/com/digitalsanctuary/spring/user/service/SessionInvalidationServiceTest.java index 3851a48..f392aa6 100644 --- a/src/test/java/com/digitalsanctuary/spring/user/service/SessionInvalidationServiceTest.java +++ b/src/test/java/com/digitalsanctuary/spring/user/service/SessionInvalidationServiceTest.java @@ -3,6 +3,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.*; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -17,6 +18,7 @@ import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.security.core.session.SessionInformation; import org.springframework.security.core.session.SessionRegistry; +import org.springframework.test.util.ReflectionTestUtils; import com.digitalsanctuary.spring.user.persistence.model.User; import com.digitalsanctuary.spring.user.test.builders.UserTestDataBuilder; @@ -42,6 +44,9 @@ void setUp() { .withLastName("User") .enabled() .build(); + + // Set a high default threshold to avoid warning logs in most tests + ReflectionTestUtils.setField(sessionInvalidationService, "warnThreshold", 1000); } @Nested @@ -194,4 +199,79 @@ void handlesMixedPrincipalTypesCorrectly() { verify(session2).expireNow(); } } + + @Nested + @DisplayName("Performance Monitoring Tests") + class PerformanceMonitoringTests { + + @Test + @DisplayName("logs warning when principal count exceeds threshold") + void logsWarningWhenPrincipalCountExceedsThreshold() { + // Given - set a low threshold for testing + ReflectionTestUtils.setField(sessionInvalidationService, "warnThreshold", 5); + + // Create more principals than the threshold + List manyPrincipals = new ArrayList<>(); + for (int i = 0; i < 10; i++) { + User user = UserTestDataBuilder.aUser() + .withId((long) (i + 100)) + .withEmail("user" + i + "@example.com") + .build(); + manyPrincipals.add(user); + } + when(sessionRegistry.getAllPrincipals()).thenReturn(manyPrincipals); + + // When + sessionInvalidationService.invalidateUserSessions(testUser); + + // Then - the method completes without error + // (Log output verification would require a log capture library like LogCaptor) + verify(sessionRegistry).getAllPrincipals(); + } + + @Test + @DisplayName("does not warn when principal count is below threshold") + void doesNotWarnWhenPrincipalCountBelowThreshold() { + // Given - threshold is set to 1000 in @BeforeEach + // Create fewer principals than the threshold + when(sessionRegistry.getAllPrincipals()).thenReturn(List.of(testUser)); + + // When + sessionInvalidationService.invalidateUserSessions(testUser); + + // Then - the method completes without error + verify(sessionRegistry).getAllPrincipals(); + } + + @Test + @DisplayName("uses default threshold of 1000") + void usesDefaultThresholdOf1000() { + // Given - create a new service without setting threshold (should use default) + SessionInvalidationService newService = new SessionInvalidationService(sessionRegistry); + + // Verify the default value is set correctly via reflection + Integer threshold = (Integer) ReflectionTestUtils.getField(newService, "warnThreshold"); + + // Then - default should be 0 (unset by Spring) since we're not using Spring context + // In production, Spring will inject the default value of 1000 + assertThat(threshold).isEqualTo(0); + } + + @Test + @DisplayName("includes principal count in info log") + void includesPrincipalCountInInfoLog() { + // Given - threshold is set to 1000 in @BeforeEach + SessionInformation session = mock(SessionInformation.class); + when(session.getSessionId()).thenReturn("session-1"); + when(sessionRegistry.getAllPrincipals()).thenReturn(List.of(testUser)); + when(sessionRegistry.getAllSessions(testUser, false)).thenReturn(List.of(session)); + + // When + int invalidatedCount = sessionInvalidationService.invalidateUserSessions(testUser); + + // Then - verify the session was invalidated (log verification would require LogCaptor) + assertThat(invalidatedCount).isEqualTo(1); + verify(session).expireNow(); + } + } } diff --git a/src/test/java/com/digitalsanctuary/spring/user/service/UserEmailServiceTest.java b/src/test/java/com/digitalsanctuary/spring/user/service/UserEmailServiceTest.java index 55f2359..e4d4e6a 100644 --- a/src/test/java/com/digitalsanctuary/spring/user/service/UserEmailServiceTest.java +++ b/src/test/java/com/digitalsanctuary/spring/user/service/UserEmailServiceTest.java @@ -1,6 +1,7 @@ package com.digitalsanctuary.spring.user.service; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.*; import static org.mockito.Mockito.*; @@ -11,6 +12,7 @@ import com.digitalsanctuary.spring.user.persistence.repository.PasswordResetTokenRepository; import com.digitalsanctuary.spring.user.test.builders.UserTestDataBuilder; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Nested; @@ -21,7 +23,12 @@ import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.context.ApplicationEventPublisher; +import org.springframework.security.access.prepost.PreAuthorize; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.context.SecurityContext; +import org.springframework.security.core.context.SecurityContextHolder; +import java.lang.reflect.Method; import java.util.Map; @ExtendWith(MockitoExtension.class) @@ -47,6 +54,7 @@ class UserEmailServiceTest { private UserEmailService userEmailService; private User testUser; + private User adminUser; private String appUrl; @BeforeEach @@ -59,6 +67,36 @@ void setUp() { .enabled() .build(); appUrl = "https://example.com"; + + // Set up admin user for SecurityContext mocking + adminUser = UserTestDataBuilder.aUser() + .withId(99L) + .withEmail("admin@example.com") + .withFirstName("Admin") + .withLastName("User") + .enabled() + .build(); + } + + @AfterEach + void tearDown() { + SecurityContextHolder.clearContext(); + } + + /** + * Sets up a mock SecurityContext with the given user as the authenticated admin. + */ + private void mockSecurityContext(User user) { + DSUserDetails userDetails = new DSUserDetails(user); + Authentication authentication = mock(Authentication.class, withSettings().lenient()); + when(authentication.isAuthenticated()).thenReturn(true); + when(authentication.getPrincipal()).thenReturn(userDetails); + when(authentication.getName()).thenReturn(user.getEmail()); + + SecurityContext securityContext = mock(SecurityContext.class, withSettings().lenient()); + when(securityContext.getAuthentication()).thenReturn(authentication); + + SecurityContextHolder.setContext(securityContext); } @Nested @@ -352,11 +390,9 @@ void user_canRequestBothRegistrationAndPasswordReset() { @DisplayName("Admin Password Reset Tests") class AdminPasswordResetTests { - private String adminIdentifier; - @BeforeEach - void setUp() { - adminIdentifier = "admin@example.com"; + void setUpAdmin() { + mockSecurityContext(adminUser); } @Test @@ -366,7 +402,7 @@ void initiateAdminPasswordReset_sendsEmailAndInvalidatesSessions() { when(sessionInvalidationService.invalidateUserSessions(testUser)).thenReturn(3); // When - int invalidatedCount = userEmailService.initiateAdminPasswordReset(testUser, appUrl, adminIdentifier, true); + int invalidatedCount = userEmailService.initiateAdminPasswordReset(testUser, appUrl, true); // Then assertThat(invalidatedCount).isEqualTo(3); @@ -380,14 +416,14 @@ void initiateAdminPasswordReset_sendsEmailAndInvalidatesSessions() { PasswordResetToken savedToken = tokenCaptor.getValue(); assertThat(savedToken.getUser()).isEqualTo(testUser); - // Verify audit event was published with admin info + // Verify audit event was published with admin info from SecurityContext ArgumentCaptor auditCaptor = ArgumentCaptor.forClass(AuditEvent.class); verify(eventPublisher).publishEvent(auditCaptor.capture()); AuditEvent auditEvent = auditCaptor.getValue(); assertThat(auditEvent.getAction()).isEqualTo("adminInitiatedPasswordReset"); assertThat(auditEvent.getActionStatus()).isEqualTo("Success"); - assertThat(auditEvent.getMessage()).contains(adminIdentifier); - assertThat(auditEvent.getExtraData()).contains("adminIdentifier:" + adminIdentifier); + assertThat(auditEvent.getMessage()).contains(adminUser.getEmail()); + assertThat(auditEvent.getExtraData()).contains("adminIdentifier:" + adminUser.getEmail()); assertThat(auditEvent.getExtraData()).contains("sessionsInvalidated:3"); // Verify email was sent @@ -403,7 +439,7 @@ void initiateAdminPasswordReset_sendsEmailAndInvalidatesSessions() { @DisplayName("initiateAdminPasswordReset - skips session invalidation when not requested") void initiateAdminPasswordReset_skipsSessionInvalidationWhenNotRequested() { // When - int invalidatedCount = userEmailService.initiateAdminPasswordReset(testUser, appUrl, adminIdentifier, false); + int invalidatedCount = userEmailService.initiateAdminPasswordReset(testUser, appUrl, false); // Then assertThat(invalidatedCount).isEqualTo(0); @@ -433,7 +469,7 @@ void initiateAdminPasswordReset_createsTokenAndSendsEmail() { when(sessionInvalidationService.invalidateUserSessions(testUser)).thenReturn(0); // When - userEmailService.initiateAdminPasswordReset(testUser, appUrl, adminIdentifier, true); + userEmailService.initiateAdminPasswordReset(testUser, appUrl, true); // Then ArgumentCaptor> variablesCaptor = ArgumentCaptor.forClass(Map.class); @@ -450,5 +486,226 @@ void initiateAdminPasswordReset_createsTokenAndSendsEmail() { assertThat(variables.get("confirmationUrl")).asString() .startsWith(appUrl + "/user/changePassword?token="); } + + @Test + @DisplayName("initiateAdminPasswordReset - includes correlation ID in audit extraData") + void initiateAdminPasswordReset_includesCorrelationIdInAuditExtraData() { + // When + userEmailService.initiateAdminPasswordReset(testUser, appUrl, false); + + // Then + ArgumentCaptor auditCaptor = ArgumentCaptor.forClass(AuditEvent.class); + verify(eventPublisher).publishEvent(auditCaptor.capture()); + AuditEvent auditEvent = auditCaptor.getValue(); + assertThat(auditEvent.getExtraData()).contains("correlationId:"); + // Verify correlation ID is a UUID format + String extraData = auditEvent.getExtraData(); + String correlationId = extraData.substring(extraData.lastIndexOf("correlationId:") + 14); + assertThat(correlationId).matches("[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}"); + } + + @Test + @DisplayName("initiateAdminPasswordReset - gets admin identifier from SecurityContext") + void initiateAdminPasswordReset_getsAdminIdentifierFromSecurityContext() { + // When + userEmailService.initiateAdminPasswordReset(testUser, appUrl, false); + + // Then + ArgumentCaptor auditCaptor = ArgumentCaptor.forClass(AuditEvent.class); + verify(eventPublisher).publishEvent(auditCaptor.capture()); + AuditEvent auditEvent = auditCaptor.getValue(); + // Admin identifier should be from SecurityContext, not a parameter + assertThat(auditEvent.getMessage()).contains(adminUser.getEmail()); + assertThat(auditEvent.getExtraData()).contains("adminIdentifier:" + adminUser.getEmail()); + } + + @Test + @DisplayName("initiateAdminPasswordReset - returns UNKNOWN_ADMIN when not authenticated") + void initiateAdminPasswordReset_returnsUnknownAdminWhenNotAuthenticated() { + // Given - clear the security context + SecurityContextHolder.clearContext(); + + // When + userEmailService.initiateAdminPasswordReset(testUser, appUrl, false); + + // Then + ArgumentCaptor auditCaptor = ArgumentCaptor.forClass(AuditEvent.class); + verify(eventPublisher).publishEvent(auditCaptor.capture()); + AuditEvent auditEvent = auditCaptor.getValue(); + assertThat(auditEvent.getExtraData()).contains("adminIdentifier:UNKNOWN_ADMIN"); + } + } + + @Nested + @DisplayName("URL Validation Tests") + class UrlValidationTests { + + @BeforeEach + void setUpAdmin() { + mockSecurityContext(adminUser); + } + + @Test + @DisplayName("initiateAdminPasswordReset - accepts valid HTTPS URL") + void initiateAdminPasswordReset_acceptsValidHttpsUrl() { + // When/Then - no exception should be thrown + userEmailService.initiateAdminPasswordReset(testUser, "https://example.com", false); + + verify(mailService).sendTemplateMessage( + eq(testUser.getEmail()), + eq("Password Reset"), + any(), + eq("mail/forgot-password-token.html") + ); + } + + @Test + @DisplayName("initiateAdminPasswordReset - accepts valid HTTP URL") + void initiateAdminPasswordReset_acceptsValidHttpUrl() { + // When/Then - no exception should be thrown + userEmailService.initiateAdminPasswordReset(testUser, "http://localhost:8080", false); + + verify(mailService).sendTemplateMessage( + eq(testUser.getEmail()), + eq("Password Reset"), + any(), + eq("mail/forgot-password-token.html") + ); + } + + @Test + @DisplayName("initiateAdminPasswordReset - rejects javascript: URL") + void initiateAdminPasswordReset_rejectsJavascriptUrl() { + // When/Then + assertThatThrownBy(() -> userEmailService.initiateAdminPasswordReset(testUser, "javascript:alert('xss')", false)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Invalid application URL"); + } + + @Test + @DisplayName("initiateAdminPasswordReset - rejects data: URL") + void initiateAdminPasswordReset_rejectsDataUrl() { + // When/Then + assertThatThrownBy(() -> userEmailService.initiateAdminPasswordReset(testUser, "data:text/html,", false)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Invalid application URL"); + } + + @Test + @DisplayName("initiateAdminPasswordReset - rejects null URL") + void initiateAdminPasswordReset_rejectsNullUrl() { + // When/Then + assertThatThrownBy(() -> userEmailService.initiateAdminPasswordReset(testUser, null, false)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Invalid application URL"); + } + + @Test + @DisplayName("initiateAdminPasswordReset - rejects blank URL") + void initiateAdminPasswordReset_rejectsBlankUrl() { + // When/Then + assertThatThrownBy(() -> userEmailService.initiateAdminPasswordReset(testUser, " ", false)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Invalid application URL"); + } + + @Test + @DisplayName("initiateAdminPasswordReset - rejects URL without host") + void initiateAdminPasswordReset_rejectsUrlWithoutHost() { + // When/Then + assertThatThrownBy(() -> userEmailService.initiateAdminPasswordReset(testUser, "http://", false)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Invalid application URL"); + } + } + + @Nested + @DisplayName("PreAuthorize Annotation Tests") + class PreAuthorizeAnnotationTests { + + @Test + @DisplayName("initiateAdminPasswordReset(User, String, boolean) has @PreAuthorize annotation") + void initiateAdminPasswordReset_hasPreAuthorizeAnnotation() throws NoSuchMethodException { + // Given + Method method = UserEmailService.class.getMethod("initiateAdminPasswordReset", User.class, String.class, boolean.class); + + // When + PreAuthorize annotation = method.getAnnotation(PreAuthorize.class); + + // Then + assertThat(annotation).isNotNull(); + assertThat(annotation.value()).isEqualTo("hasRole('ROLE_ADMIN')"); + } + + @Test + @DisplayName("initiateAdminPasswordReset(User) has @PreAuthorize annotation") + void initiateAdminPasswordReset_withUserOnly_hasPreAuthorizeAnnotation() throws NoSuchMethodException { + // Given + Method method = UserEmailService.class.getMethod("initiateAdminPasswordReset", User.class); + + // When + PreAuthorize annotation = method.getAnnotation(PreAuthorize.class); + + // Then + assertThat(annotation).isNotNull(); + assertThat(annotation.value()).isEqualTo("hasRole('ROLE_ADMIN')"); + } + + @Test + @DisplayName("deprecated initiateAdminPasswordReset has @PreAuthorize annotation") + void initiateAdminPasswordReset_deprecated_hasPreAuthorizeAnnotation() throws NoSuchMethodException { + // Given - deprecated method with adminIdentifier parameter + Method method = UserEmailService.class.getMethod("initiateAdminPasswordReset", User.class, String.class, String.class, boolean.class); + + // When + PreAuthorize annotation = method.getAnnotation(PreAuthorize.class); + + // Then + assertThat(annotation).isNotNull(); + assertThat(annotation.value()).isEqualTo("hasRole('ROLE_ADMIN')"); + } + } + + @Nested + @DisplayName("Deprecated Method Tests") + class DeprecatedMethodTests { + + @BeforeEach + void setUpAdmin() { + mockSecurityContext(adminUser); + } + + @Test + @DisplayName("deprecated method with adminIdentifier still works but ignores parameter") + @SuppressWarnings("deprecation") + void deprecatedMethod_stillWorksButIgnoresAdminIdentifier() { + // Given + String ignoredAdminIdentifier = "ignored@example.com"; + + // When + userEmailService.initiateAdminPasswordReset(testUser, appUrl, ignoredAdminIdentifier, false); + + // Then - admin identifier should be from SecurityContext, not the parameter + ArgumentCaptor auditCaptor = ArgumentCaptor.forClass(AuditEvent.class); + verify(eventPublisher).publishEvent(auditCaptor.capture()); + AuditEvent auditEvent = auditCaptor.getValue(); + // Should use admin from SecurityContext, not the ignored parameter + assertThat(auditEvent.getExtraData()).contains("adminIdentifier:" + adminUser.getEmail()); + assertThat(auditEvent.getExtraData()).doesNotContain(ignoredAdminIdentifier); + } + + @Test + @DisplayName("deprecated method is marked with @Deprecated(forRemoval = true)") + void deprecatedMethod_isMarkedForRemoval() throws NoSuchMethodException { + // Given + Method method = UserEmailService.class.getMethod("initiateAdminPasswordReset", User.class, String.class, String.class, boolean.class); + + // When + Deprecated annotation = method.getAnnotation(Deprecated.class); + + // Then + assertThat(annotation).isNotNull(); + assertThat(annotation.forRemoval()).isTrue(); + } } } \ No newline at end of file From 67e645e81242f53722b36039f710512b8d78a171 Mon Sep 17 00:00:00 2001 From: Devon Hillard Date: Sat, 17 Jan 2026 20:56:04 -0700 Subject: [PATCH 3/6] Fix @PreAuthorize expressions and add URL validation - Change hasRole('ROLE_ADMIN') to hasRole('ADMIN') in all @PreAuthorize annotations (Spring auto-prefixes ROLE_) - Add URL validation to sendForgotPasswordVerificationEmail and sendRegistrationVerificationEmail to prevent XSS - Switch audit extraData from CSV to JSON format for better parsing - Update all corresponding tests --- .../spring/user/service/UserEmailService.java | 61 ++++++++-- .../user/service/UserEmailServiceTest.java | 111 ++++++++++-------- 2 files changed, 113 insertions(+), 59 deletions(-) diff --git a/src/main/java/com/digitalsanctuary/spring/user/service/UserEmailService.java b/src/main/java/com/digitalsanctuary/spring/user/service/UserEmailService.java index 32cd583..1d7fb4f 100644 --- a/src/main/java/com/digitalsanctuary/spring/user/service/UserEmailService.java +++ b/src/main/java/com/digitalsanctuary/spring/user/service/UserEmailService.java @@ -50,7 +50,8 @@ public class UserEmailService { * Send forgot password verification email. * * @param user the user - * @param appUrl the app url + * @param appUrl the app url (must be a valid HTTP/HTTPS URL) + * @throws IllegalArgumentException if appUrl is null, blank, or uses a dangerous scheme */ public void sendForgotPasswordVerificationEmail(final User user, final String appUrl) { log.debug("UserEmailService.sendForgotPasswordVerificationEmail: called with user: {}", user); @@ -62,7 +63,7 @@ public void sendForgotPasswordVerificationEmail(final User user, final String ap eventPublisher.publishEvent(sendForgotPasswordEmailAuditEvent); - Map variables = createEmailVariables(user, appUrl, token, "/user/changePassword?token="); + Map variables = createEmailVariablesWithValidation(user, appUrl, token, "/user/changePassword?token="); mailService.sendTemplateMessage(user.getEmail(), "Password Reset", variables, "mail/forgot-password-token.html"); } @@ -73,13 +74,14 @@ public void sendForgotPasswordVerificationEmail(final User user, final String ap * Create a Verification token for the user, and send the email out. * * @param user the user - * @param appUrl the app url + * @param appUrl the app url (must be a valid HTTP/HTTPS URL) + * @throws IllegalArgumentException if appUrl is null, blank, or uses a dangerous scheme */ public void sendRegistrationVerificationEmail(final User user, final String appUrl) { final String token = generateToken(); userVerificationService.createVerificationTokenForUser(user, token); - Map variables = createEmailVariables(user, appUrl, token, "/user/registrationConfirm?token="); + Map variables = createEmailVariablesWithValidation(user, appUrl, token, "/user/registrationConfirm?token="); mailService.sendTemplateMessage(user.getEmail(), "Registration Confirmation", variables, "mail/registration-token.html"); } @@ -199,7 +201,7 @@ public void createPasswordResetTokenForUser(final User user, final String token) * @return the number of sessions invalidated (0 if invalidateSessions is false) * @throws IllegalArgumentException if appUrl is invalid */ - @PreAuthorize("hasRole('ROLE_ADMIN')") + @PreAuthorize("hasRole('ADMIN')") public int initiateAdminPasswordReset(final User user, final String appUrl, final boolean invalidateSessions) { final String correlationId = generateToken(); final String adminIdentifier = getCurrentAdminIdentifier(); @@ -236,7 +238,7 @@ public int initiateAdminPasswordReset(final User user, final String appUrl, fina * @return the number of sessions invalidated * @throws IllegalStateException if user.admin.appUrl is not configured */ - @PreAuthorize("hasRole('ROLE_ADMIN')") + @PreAuthorize("hasRole('ADMIN')") public int initiateAdminPasswordReset(final User user) { if (configuredAppUrl == null || configuredAppUrl.isBlank()) { throw new IllegalStateException( @@ -257,7 +259,7 @@ public int initiateAdminPasswordReset(final User user) { * The adminIdentifier is now derived from the SecurityContext for security. */ @Deprecated(forRemoval = true) - @PreAuthorize("hasRole('ROLE_ADMIN')") + @PreAuthorize("hasRole('ADMIN')") public int initiateAdminPasswordReset(final User user, final String appUrl, final String adminIdentifier, final boolean invalidateSessions) { log.warn("UserEmailService.initiateAdminPasswordReset: adminIdentifier parameter is deprecated and ignored. " @@ -276,7 +278,7 @@ public int initiateAdminPasswordReset(final User user, final String appUrl, fina * The adminIdentifier is now derived from the SecurityContext for security. */ @Deprecated(forRemoval = true) - @PreAuthorize("hasRole('ROLE_ADMIN')") + @PreAuthorize("hasRole('ADMIN')") public int initiateAdminPasswordReset(final User user, final String adminIdentifier) { log.warn("UserEmailService.initiateAdminPasswordReset: adminIdentifier parameter is deprecated and ignored. " + "Admin identity is now derived from SecurityContext."); @@ -313,19 +315,58 @@ private void publishAdminPasswordResetAuditEvent(final User user, final String a final int invalidatedSessions, final String correlationId) { String auditMessage = String.format("Admin-initiated password reset by %s. Sessions invalidated: %d", adminIdentifier, invalidatedSessions); + + String extraDataJson = buildAuditExtraData(adminIdentifier, invalidatedSessions, correlationId); + AuditEvent adminPasswordResetAuditEvent = AuditEvent.builder() .source(this) .user(user) .action("adminInitiatedPasswordReset") .actionStatus("Success") .message(auditMessage) - .extraData(String.format("adminIdentifier:%s,sessionsInvalidated:%d,correlationId:%s", - adminIdentifier, invalidatedSessions, correlationId)) + .extraData(extraDataJson) .build(); eventPublisher.publishEvent(adminPasswordResetAuditEvent); } + /** + * Builds the audit extra data as a JSON string for easier parsing in audit dashboards. + * Uses manual JSON construction for simplicity and to avoid Jackson version dependencies. + * + * @param adminIdentifier the admin's identifier + * @param invalidatedSessions the number of sessions invalidated + * @param correlationId the correlation ID for tracking + * @return JSON string containing the audit data + */ + private String buildAuditExtraData(final String adminIdentifier, final int invalidatedSessions, + final String correlationId) { + // Escape any special characters in string values for JSON safety + String escapedAdminId = escapeJsonString(adminIdentifier); + String escapedCorrelationId = escapeJsonString(correlationId); + + return String.format("{\"adminIdentifier\":\"%s\",\"sessionsInvalidated\":%d,\"correlationId\":\"%s\"}", + escapedAdminId, invalidatedSessions, escapedCorrelationId); + } + + /** + * Escapes special characters in a string for safe JSON inclusion. + * + * @param value the string to escape + * @return the escaped string + */ + private String escapeJsonString(final String value) { + if (value == null) { + return "null"; + } + return value + .replace("\\", "\\\\") + .replace("\"", "\\\"") + .replace("\n", "\\n") + .replace("\r", "\\r") + .replace("\t", "\\t"); + } + /** * Sends the password reset email to the user. * diff --git a/src/test/java/com/digitalsanctuary/spring/user/service/UserEmailServiceTest.java b/src/test/java/com/digitalsanctuary/spring/user/service/UserEmailServiceTest.java index e4d4e6a..c897769 100644 --- a/src/test/java/com/digitalsanctuary/spring/user/service/UserEmailServiceTest.java +++ b/src/test/java/com/digitalsanctuary/spring/user/service/UserEmailServiceTest.java @@ -148,26 +148,33 @@ void sendForgotPasswordVerificationEmail_sendsEmailWithCorrectParameters() { } @Test - @DisplayName("sendForgotPasswordVerificationEmail - handles empty app URL") - void sendForgotPasswordVerificationEmail_handlesEmptyAppUrl() { + @DisplayName("sendForgotPasswordVerificationEmail - rejects empty app URL") + void sendForgotPasswordVerificationEmail_rejectsEmptyAppUrl() { // Given String emptyUrl = ""; - // When - userEmailService.sendForgotPasswordVerificationEmail(testUser, emptyUrl); + // When/Then + assertThatThrownBy(() -> userEmailService.sendForgotPasswordVerificationEmail(testUser, emptyUrl)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Invalid application URL"); + } - // Then - ArgumentCaptor> variablesCaptor = ArgumentCaptor.forClass(Map.class); - verify(mailService).sendTemplateMessage( - eq(testUser.getEmail()), - eq("Password Reset"), - variablesCaptor.capture(), - eq("mail/forgot-password-token.html") - ); + @Test + @DisplayName("sendForgotPasswordVerificationEmail - rejects null app URL") + void sendForgotPasswordVerificationEmail_rejectsNullAppUrl() { + // When/Then + assertThatThrownBy(() -> userEmailService.sendForgotPasswordVerificationEmail(testUser, null)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Invalid application URL"); + } - Map variables = variablesCaptor.getValue(); - assertThat(variables.get("confirmationUrl")).asString() - .startsWith("/user/changePassword?token="); + @Test + @DisplayName("sendForgotPasswordVerificationEmail - rejects javascript URL") + void sendForgotPasswordVerificationEmail_rejectsJavascriptUrl() { + // When/Then + assertThatThrownBy(() -> userEmailService.sendForgotPasswordVerificationEmail(testUser, "javascript:alert('xss')")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Invalid application URL"); } @Test @@ -228,24 +235,30 @@ void sendRegistrationVerificationEmail_sendsEmailWithCorrectParameters() { } @Test - @DisplayName("sendRegistrationVerificationEmail - handles null app URL") - void sendRegistrationVerificationEmail_handlesNullAppUrl() { - // When - userEmailService.sendRegistrationVerificationEmail(testUser, null); + @DisplayName("sendRegistrationVerificationEmail - rejects null app URL") + void sendRegistrationVerificationEmail_rejectsNullAppUrl() { + // When/Then + assertThatThrownBy(() -> userEmailService.sendRegistrationVerificationEmail(testUser, null)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Invalid application URL"); + } - // Then - ArgumentCaptor> variablesCaptor = ArgumentCaptor.forClass(Map.class); - verify(mailService).sendTemplateMessage( - eq(testUser.getEmail()), - eq("Registration Confirmation"), - variablesCaptor.capture(), - eq("mail/registration-token.html") - ); + @Test + @DisplayName("sendRegistrationVerificationEmail - rejects empty app URL") + void sendRegistrationVerificationEmail_rejectsEmptyAppUrl() { + // When/Then + assertThatThrownBy(() -> userEmailService.sendRegistrationVerificationEmail(testUser, "")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Invalid application URL"); + } - Map variables = variablesCaptor.getValue(); - assertThat(variables.get("appUrl")).isNull(); - assertThat(variables.get("confirmationUrl")).asString() - .isEqualTo("null/user/registrationConfirm?token=" + variables.get("token")); + @Test + @DisplayName("sendRegistrationVerificationEmail - rejects javascript URL") + void sendRegistrationVerificationEmail_rejectsJavascriptUrl() { + // When/Then + assertThatThrownBy(() -> userEmailService.sendRegistrationVerificationEmail(testUser, "javascript:alert('xss')")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Invalid application URL"); } @Test @@ -423,8 +436,9 @@ void initiateAdminPasswordReset_sendsEmailAndInvalidatesSessions() { assertThat(auditEvent.getAction()).isEqualTo("adminInitiatedPasswordReset"); assertThat(auditEvent.getActionStatus()).isEqualTo("Success"); assertThat(auditEvent.getMessage()).contains(adminUser.getEmail()); - assertThat(auditEvent.getExtraData()).contains("adminIdentifier:" + adminUser.getEmail()); - assertThat(auditEvent.getExtraData()).contains("sessionsInvalidated:3"); + // Audit extraData is now JSON format + assertThat(auditEvent.getExtraData()).contains("\"adminIdentifier\":\"" + adminUser.getEmail() + "\""); + assertThat(auditEvent.getExtraData()).contains("\"sessionsInvalidated\":3"); // Verify email was sent verify(mailService).sendTemplateMessage( @@ -455,11 +469,11 @@ void initiateAdminPasswordReset_skipsSessionInvalidationWhenNotRequested() { eq("mail/forgot-password-token.html") ); - // Verify audit event shows 0 sessions invalidated + // Verify audit event shows 0 sessions invalidated (JSON format) ArgumentCaptor auditCaptor = ArgumentCaptor.forClass(AuditEvent.class); verify(eventPublisher).publishEvent(auditCaptor.capture()); AuditEvent auditEvent = auditCaptor.getValue(); - assertThat(auditEvent.getExtraData()).contains("sessionsInvalidated:0"); + assertThat(auditEvent.getExtraData()).contains("\"sessionsInvalidated\":0"); } @Test @@ -488,7 +502,7 @@ void initiateAdminPasswordReset_createsTokenAndSendsEmail() { } @Test - @DisplayName("initiateAdminPasswordReset - includes correlation ID in audit extraData") + @DisplayName("initiateAdminPasswordReset - includes correlation ID in audit extraData as JSON") void initiateAdminPasswordReset_includesCorrelationIdInAuditExtraData() { // When userEmailService.initiateAdminPasswordReset(testUser, appUrl, false); @@ -497,11 +511,10 @@ void initiateAdminPasswordReset_includesCorrelationIdInAuditExtraData() { ArgumentCaptor auditCaptor = ArgumentCaptor.forClass(AuditEvent.class); verify(eventPublisher).publishEvent(auditCaptor.capture()); AuditEvent auditEvent = auditCaptor.getValue(); - assertThat(auditEvent.getExtraData()).contains("correlationId:"); - // Verify correlation ID is a UUID format - String extraData = auditEvent.getExtraData(); - String correlationId = extraData.substring(extraData.lastIndexOf("correlationId:") + 14); - assertThat(correlationId).matches("[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}"); + // Audit extraData is now JSON format + assertThat(auditEvent.getExtraData()).contains("\"correlationId\":\""); + // Verify correlation ID is a UUID format within JSON + assertThat(auditEvent.getExtraData()).matches(".*\"correlationId\":\"[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\".*"); } @Test @@ -514,9 +527,9 @@ void initiateAdminPasswordReset_getsAdminIdentifierFromSecurityContext() { ArgumentCaptor auditCaptor = ArgumentCaptor.forClass(AuditEvent.class); verify(eventPublisher).publishEvent(auditCaptor.capture()); AuditEvent auditEvent = auditCaptor.getValue(); - // Admin identifier should be from SecurityContext, not a parameter + // Admin identifier should be from SecurityContext, not a parameter (JSON format) assertThat(auditEvent.getMessage()).contains(adminUser.getEmail()); - assertThat(auditEvent.getExtraData()).contains("adminIdentifier:" + adminUser.getEmail()); + assertThat(auditEvent.getExtraData()).contains("\"adminIdentifier\":\"" + adminUser.getEmail() + "\""); } @Test @@ -532,7 +545,7 @@ void initiateAdminPasswordReset_returnsUnknownAdminWhenNotAuthenticated() { ArgumentCaptor auditCaptor = ArgumentCaptor.forClass(AuditEvent.class); verify(eventPublisher).publishEvent(auditCaptor.capture()); AuditEvent auditEvent = auditCaptor.getValue(); - assertThat(auditEvent.getExtraData()).contains("adminIdentifier:UNKNOWN_ADMIN"); + assertThat(auditEvent.getExtraData()).contains("\"adminIdentifier\":\"UNKNOWN_ADMIN\""); } } @@ -634,7 +647,7 @@ void initiateAdminPasswordReset_hasPreAuthorizeAnnotation() throws NoSuchMethodE // Then assertThat(annotation).isNotNull(); - assertThat(annotation.value()).isEqualTo("hasRole('ROLE_ADMIN')"); + assertThat(annotation.value()).isEqualTo("hasRole('ADMIN')"); } @Test @@ -648,7 +661,7 @@ void initiateAdminPasswordReset_withUserOnly_hasPreAuthorizeAnnotation() throws // Then assertThat(annotation).isNotNull(); - assertThat(annotation.value()).isEqualTo("hasRole('ROLE_ADMIN')"); + assertThat(annotation.value()).isEqualTo("hasRole('ADMIN')"); } @Test @@ -662,7 +675,7 @@ void initiateAdminPasswordReset_deprecated_hasPreAuthorizeAnnotation() throws No // Then assertThat(annotation).isNotNull(); - assertThat(annotation.value()).isEqualTo("hasRole('ROLE_ADMIN')"); + assertThat(annotation.value()).isEqualTo("hasRole('ADMIN')"); } } @@ -685,12 +698,12 @@ void deprecatedMethod_stillWorksButIgnoresAdminIdentifier() { // When userEmailService.initiateAdminPasswordReset(testUser, appUrl, ignoredAdminIdentifier, false); - // Then - admin identifier should be from SecurityContext, not the parameter + // Then - admin identifier should be from SecurityContext, not the parameter (JSON format) ArgumentCaptor auditCaptor = ArgumentCaptor.forClass(AuditEvent.class); verify(eventPublisher).publishEvent(auditCaptor.capture()); AuditEvent auditEvent = auditCaptor.getValue(); // Should use admin from SecurityContext, not the ignored parameter - assertThat(auditEvent.getExtraData()).contains("adminIdentifier:" + adminUser.getEmail()); + assertThat(auditEvent.getExtraData()).contains("\"adminIdentifier\":\"" + adminUser.getEmail() + "\""); assertThat(auditEvent.getExtraData()).doesNotContain(ignoredAdminIdentifier); } From 7ce3866accd9ec9b189a531a9ebeb01c2e59bb81 Mon Sep 17 00:00:00 2001 From: Devon Hillard Date: Sat, 17 Jan 2026 21:15:20 -0700 Subject: [PATCH 4/6] fix(security): address critical code review findings Security improvements for admin password reset feature: - Reorder operations in initiateAdminPasswordReset to prevent user lockout: token creation and email sending now happen before session invalidation - Replace UUID.randomUUID() with SecureRandom for cryptographically strong 256-bit tokens (Base64 URL-safe encoded) - Replace vulnerable custom JSON escaping with Jackson ObjectMapper for proper serialization in audit events - Truncate session IDs to first 8 chars in debug logs to prevent exposure Updates tests to expect Base64 token format instead of UUID format. --- .../service/SessionInvalidationService.java | 5 +- .../spring/user/service/UserEmailService.java | 79 ++++++++++--------- .../user/service/UserEmailServiceTest.java | 9 ++- 3 files changed, 51 insertions(+), 42 deletions(-) diff --git a/src/main/java/com/digitalsanctuary/spring/user/service/SessionInvalidationService.java b/src/main/java/com/digitalsanctuary/spring/user/service/SessionInvalidationService.java index 90f4752..fae5317 100644 --- a/src/main/java/com/digitalsanctuary/spring/user/service/SessionInvalidationService.java +++ b/src/main/java/com/digitalsanctuary/spring/user/service/SessionInvalidationService.java @@ -70,8 +70,11 @@ public int invalidateUserSessions(User user) { for (SessionInformation session : sessions) { session.expireNow(); invalidatedCount++; + // Log truncated session ID to avoid exposing full session identifiers + String sessionId = session.getSessionId(); + String safeSessionId = sessionId.length() > 8 ? sessionId.substring(0, 8) + "..." : sessionId; log.debug("SessionInvalidationService.invalidateUserSessions: expired session {} for user {}", - session.getSessionId(), user.getEmail()); + safeSessionId, user.getEmail()); } } } diff --git a/src/main/java/com/digitalsanctuary/spring/user/service/UserEmailService.java b/src/main/java/com/digitalsanctuary/spring/user/service/UserEmailService.java index 1d7fb4f..d55fbcb 100644 --- a/src/main/java/com/digitalsanctuary/spring/user/service/UserEmailService.java +++ b/src/main/java/com/digitalsanctuary/spring/user/service/UserEmailService.java @@ -2,9 +2,12 @@ import java.net.URI; import java.net.URISyntaxException; +import java.security.SecureRandom; +import java.util.Base64; import java.util.HashMap; import java.util.Map; -import java.util.UUID; +import tools.jackson.core.JacksonException; +import tools.jackson.databind.ObjectMapper; import org.springframework.beans.factory.annotation.Value; import org.springframework.context.ApplicationEventPublisher; import org.springframework.security.access.prepost.PreAuthorize; @@ -46,6 +49,12 @@ public class UserEmailService { @Value("${user.admin.appUrl:#{null}}") private String configuredAppUrl; + /** ObjectMapper for JSON serialization in audit events. */ + private final ObjectMapper objectMapper = new ObjectMapper(); + + /** SecureRandom for cryptographically strong token generation. */ + private final SecureRandom secureRandom = new SecureRandom(); + /** * Send forgot password verification email. * @@ -165,12 +174,15 @@ private String getCurrentAdminIdentifier() { } /** - * Generate random token. + * Generates a cryptographically secure random token. + * Uses SecureRandom for proper entropy instead of UUID. * - * @return the string + * @return URL-safe Base64 encoded token with 256 bits of entropy */ private String generateToken() { - return UUID.randomUUID().toString(); + byte[] tokenBytes = new byte[32]; // 256 bits of entropy + secureRandom.nextBytes(tokenBytes); + return Base64.getUrlEncoder().withoutPadding().encodeToString(tokenBytes); } /** @@ -187,14 +199,17 @@ public void createPasswordResetTokenForUser(final User user, final String token) /** * Initiates an admin-triggered password reset for a user. * This method: - * 1. Optionally invalidates all active sessions for the user - * 2. Generates a password reset token - * 3. Sends the password reset email + * 1. Generates a password reset token + * 2. Sends the password reset email + * 3. Optionally invalidates all active sessions for the user (done last to prevent lockout if earlier steps fail) * 4. Publishes an audit event for tracking * *

Note: Email sending is asynchronous with retry. Delivery status is logged * but not returned. The audit event provides tracking for admin actions.

* + *

Operation Order: Session invalidation is performed last to ensure + * users are not locked out if token creation or email sending fails.

+ * * @param user the user to reset password for * @param appUrl the application URL for the reset link (must be valid HTTP/HTTPS URL) * @param invalidateSessions whether to invalidate all user sessions @@ -209,19 +224,19 @@ public int initiateAdminPasswordReset(final User user, final String appUrl, fina log.debug("UserEmailService.initiateAdminPasswordReset: called for user: {} by admin: {} [correlationId={}]", user.getEmail(), adminIdentifier, correlationId); - // Step 1: Optionally invalidate all user sessions - int invalidatedSessions = handleSessionInvalidation(user, invalidateSessions, correlationId); - - // Step 2: Generate token and create password reset token + // Step 1: Generate token and create password reset token (must succeed before invalidating sessions) final String token = generateToken(); createPasswordResetTokenForUser(user, token); - // Step 3: Publish admin-specific audit event - publishAdminPasswordResetAuditEvent(user, adminIdentifier, invalidatedSessions, correlationId); - - // Step 4: Send password reset email + // Step 2: Send password reset email (must succeed before invalidating sessions) sendPasswordResetEmail(user, appUrl, token); + // Step 3: Invalidate sessions LAST - only after recovery mechanism is in place + int invalidatedSessions = handleSessionInvalidation(user, invalidateSessions, correlationId); + + // Step 4: Publish admin-specific audit event + publishAdminPasswordResetAuditEvent(user, adminIdentifier, invalidatedSessions, correlationId); + log.info("UserEmailService.initiateAdminPasswordReset: password reset email sent to {} by admin {} [correlationId={}]", user.getEmail(), adminIdentifier, correlationId); @@ -332,7 +347,7 @@ private void publishAdminPasswordResetAuditEvent(final User user, final String a /** * Builds the audit extra data as a JSON string for easier parsing in audit dashboards. - * Uses manual JSON construction for simplicity and to avoid Jackson version dependencies. + * Uses Jackson ObjectMapper for proper JSON escaping and security. * * @param adminIdentifier the admin's identifier * @param invalidatedSessions the number of sessions invalidated @@ -341,30 +356,18 @@ private void publishAdminPasswordResetAuditEvent(final User user, final String a */ private String buildAuditExtraData(final String adminIdentifier, final int invalidatedSessions, final String correlationId) { - // Escape any special characters in string values for JSON safety - String escapedAdminId = escapeJsonString(adminIdentifier); - String escapedCorrelationId = escapeJsonString(correlationId); - - return String.format("{\"adminIdentifier\":\"%s\",\"sessionsInvalidated\":%d,\"correlationId\":\"%s\"}", - escapedAdminId, invalidatedSessions, escapedCorrelationId); - } + Map data = new HashMap<>(); + data.put("adminIdentifier", adminIdentifier); + data.put("sessionsInvalidated", invalidatedSessions); + data.put("correlationId", correlationId); - /** - * Escapes special characters in a string for safe JSON inclusion. - * - * @param value the string to escape - * @return the escaped string - */ - private String escapeJsonString(final String value) { - if (value == null) { - return "null"; + try { + return objectMapper.writeValueAsString(data); + } catch (JacksonException e) { + log.error("Failed to serialize audit extra data", e); + // Return minimal safe JSON on failure + return "{\"error\":\"serialization_failed\"}"; } - return value - .replace("\\", "\\\\") - .replace("\"", "\\\"") - .replace("\n", "\\n") - .replace("\r", "\\r") - .replace("\t", "\\t"); } /** diff --git a/src/test/java/com/digitalsanctuary/spring/user/service/UserEmailServiceTest.java b/src/test/java/com/digitalsanctuary/spring/user/service/UserEmailServiceTest.java index c897769..81f1db4 100644 --- a/src/test/java/com/digitalsanctuary/spring/user/service/UserEmailServiceTest.java +++ b/src/test/java/com/digitalsanctuary/spring/user/service/UserEmailServiceTest.java @@ -116,7 +116,8 @@ void sendForgotPasswordVerificationEmail_sendsEmailWithCorrectParameters() { PasswordResetToken savedToken = tokenCaptor.getValue(); assertThat(savedToken.getUser()).isEqualTo(testUser); assertThat(savedToken.getToken()).isNotNull(); - assertThat(savedToken.getToken()).matches("[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}"); + // Base64 URL-safe encoded 32-byte token = 43 characters + assertThat(savedToken.getToken()).matches("[A-Za-z0-9_-]{43}"); // Verify audit event was published ArgumentCaptor auditCaptor = ArgumentCaptor.forClass(AuditEvent.class); @@ -212,7 +213,8 @@ void sendRegistrationVerificationEmail_sendsEmailWithCorrectParameters() { verify(userVerificationService).createVerificationTokenForUser(eq(testUser), tokenCaptor.capture()); String token = tokenCaptor.getValue(); assertThat(token).isNotNull(); - assertThat(token).matches("[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}"); + // Base64 URL-safe encoded 32-byte token = 43 characters + assertThat(token).matches("[A-Za-z0-9_-]{43}"); // Verify email was sent with correct parameters ArgumentCaptor> variablesCaptor = ArgumentCaptor.forClass(Map.class); @@ -514,7 +516,8 @@ void initiateAdminPasswordReset_includesCorrelationIdInAuditExtraData() { // Audit extraData is now JSON format assertThat(auditEvent.getExtraData()).contains("\"correlationId\":\""); // Verify correlation ID is a UUID format within JSON - assertThat(auditEvent.getExtraData()).matches(".*\"correlationId\":\"[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\".*"); + // Correlation ID is also a Base64 URL-safe encoded 32-byte token + assertThat(auditEvent.getExtraData()).matches(".*\"correlationId\":\"[A-Za-z0-9_-]{43}\".*"); } @Test From 07a6b6bef86e21a263fea7a1e28dcea83c82fa05 Mon Sep 17 00:00:00 2001 From: Devon Hillard Date: Sat, 17 Jan 2026 21:33:57 -0700 Subject: [PATCH 5/6] fix(security): prevent NPE in DSUserDetails.getClaims() Add null check for oidcUserInfo before calling getClaims() to prevent NullPointerException when DSUserDetails is constructed without OIDC info. Returns empty map instead of throwing NPE when OIDC info is not available. --- .../com/digitalsanctuary/spring/user/service/DSUserDetails.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/digitalsanctuary/spring/user/service/DSUserDetails.java b/src/main/java/com/digitalsanctuary/spring/user/service/DSUserDetails.java index c0ffd33..222d9e3 100644 --- a/src/main/java/com/digitalsanctuary/spring/user/service/DSUserDetails.java +++ b/src/main/java/com/digitalsanctuary/spring/user/service/DSUserDetails.java @@ -199,7 +199,7 @@ public String getName() { @Override public Map getClaims() { - return oidcUserInfo.getClaims(); + return oidcUserInfo != null ? oidcUserInfo.getClaims() : Map.of(); } @Override From 2c4dd35c83f4db2066c55f80d3871491da7e6196 Mon Sep 17 00:00:00 2001 From: Devon Hillard Date: Sat, 17 Jan 2026 21:35:52 -0700 Subject: [PATCH 6/6] docs: add admin password reset documentation - Add admin password reset to Features list in README - Add Admin Password Reset section with code examples and security notes - Add Admin Settings section to CONFIG.md with new configuration properties --- CONFIG.md | 5 +++++ README.md | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/CONFIG.md b/CONFIG.md index 3d5c712..44d4535 100644 --- a/CONFIG.md +++ b/CONFIG.md @@ -29,6 +29,11 @@ Welcome to the User Framework SpringBoot Configuration Guide! This document outl - **Account Deletion (`user.actuallyDeleteAccount`)**: Set to `true` to enable account deletion. Defaults to `false` where accounts are disabled instead of deleted. - **Registration Email Verification (`user.registration.sendVerificationEmail`)**: Enable (`true`) or disable (`false`) sending verification emails post-registration. +## Admin Settings + +- **Admin App URL (`user.admin.appUrl`)**: Base URL for admin-initiated password reset emails. Required when using `initiateAdminPasswordReset(user)` without explicit URL. Example: `https://myapp.com` +- **Session Invalidation Warn Threshold (`user.session.invalidation.warn-threshold`)**: Number of active sessions that triggers a performance warning during session invalidation. Defaults to `1000`. + ## Audit Logging - **Log File Path (`user.audit.logFilePath`)**: The path to the audit log file. diff --git a/README.md b/README.md index 81cde36..1e6c07b 100644 --- a/README.md +++ b/README.md @@ -62,6 +62,7 @@ Check out the [Spring User Framework Demo Application](https://github.com/devond - Registration, with optional email verification. - Login and logout functionality. - Forgot password flow. + - Admin-initiated password reset with optional session invalidation. - Database-backed user store using Spring JPA. - SSO support for Google - SSO support for Facebook @@ -521,6 +522,43 @@ Users can: - Change their password - Delete their account (configurable to either disable or fully delete) +### Admin Password Reset + +Administrators can trigger password resets for users programmatically: + +```java +@Autowired +private UserEmailService userEmailService; + +// Reset password and invalidate all user sessions +int sessionsInvalidated = userEmailService.initiateAdminPasswordReset(user, appUrl, true); + +// Reset password without invalidating sessions +userEmailService.initiateAdminPasswordReset(user, appUrl, false); + +// Use configured appUrl (from user.admin.appUrl property) +userEmailService.initiateAdminPasswordReset(user); +``` + +**Features:** +- Requires `ROLE_ADMIN` authorization (`@PreAuthorize`) +- Optional session invalidation to force re-authentication +- Sends password reset email with secure token +- Comprehensive audit logging with correlation IDs +- Cryptographically secure tokens (256-bit entropy) + +**Configuration:** +```yaml +user: + admin: + appUrl: https://myapp.com # Base URL for password reset links +``` + +**Security Notes:** +- Admin identity is derived from `SecurityContext`, not user input +- Sessions are invalidated *after* email is sent to prevent lockout +- URL validation prevents XSS (blocks javascript:, data: schemes) + ## Email Verification