From feebfe82faa7e711d7e2ab6c05935b9f09014a41 Mon Sep 17 00:00:00 2001 From: Dario Ghunney Ware Date: Tue, 2 Dec 2025 12:34:17 +0000 Subject: [PATCH 1/2] Reduce JWT Logs (#5108) Removed logging in some areas and changed level from `WARN` -> `DEBUG` to reduce verbosity Closes #5089 --------- Signed-off-by: dependabot[bot] Signed-off-by: stirlingbot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Ludy Co-authored-by: EthanHealy01 <80844253+EthanHealy01@users.noreply.github.com> Co-authored-by: Ethan Co-authored-by: Anthony Stirling <77850077+Frooodle@users.noreply.github.com> Co-authored-by: stirlingbot[bot] <195170888+stirlingbot[bot]@users.noreply.github.com> --- .../SPDF/controller/web/SignatureImageController.java | 6 +++--- .../security/controller/api/AdminLicenseController.java | 7 ++++++- .../security/filter/JwtAuthenticationFilter.java | 6 +----- .../software/proprietary/security/service/JwtService.java | 6 +----- .../security/service/KeyPairCleanupService.java | 1 - 5 files changed, 11 insertions(+), 15 deletions(-) diff --git a/app/core/src/main/java/stirling/software/SPDF/controller/web/SignatureImageController.java b/app/core/src/main/java/stirling/software/SPDF/controller/web/SignatureImageController.java index 90313af29b..5d69d60c8c 100644 --- a/app/core/src/main/java/stirling/software/SPDF/controller/web/SignatureImageController.java +++ b/app/core/src/main/java/stirling/software/SPDF/controller/web/SignatureImageController.java @@ -19,9 +19,9 @@ import stirling.software.common.service.UserServiceInterface; /** * Unified signature image controller that works for both authenticated and unauthenticated users. - * Uses composition pattern: - Core SharedSignatureService (always available): reads shared signatures - - * PersonalSignatureService (proprietary, optional): reads personal signatures For authenticated - * signature management (save/delete), see proprietary SignatureController. + * Uses composition pattern: - Core SharedSignatureService (always available): reads shared + * signatures - PersonalSignatureService (proprietary, optional): reads personal signatures For + * authenticated signature management (save/delete), see proprietary SignatureController. */ @Slf4j @RestController diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminLicenseController.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminLicenseController.java index c75b4d23f8..018607e4db 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminLicenseController.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/controller/api/AdminLicenseController.java @@ -283,7 +283,12 @@ public class AdminLicenseController { // Prevent path traversal and enforce single filename component if (filename.contains("..") || filename.contains("/") || filename.contains("\\")) { return ResponseEntity.badRequest() - .body(Map.of("success", false, "error", "Filename must not contain path separators or '..'")); + .body( + Map.of( + "success", + false, + "error", + "Filename must not contain path separators or '..'")); } // Validate file extension diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/filter/JwtAuthenticationFilter.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/filter/JwtAuthenticationFilter.java index ace7d33187..b481da51cf 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/filter/JwtAuthenticationFilter.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/filter/JwtAuthenticationFilter.java @@ -105,22 +105,18 @@ public class JwtAuthenticationFilter extends OncePerRequestFilter { } try { - log.debug("Validating JWT token"); jwtService.validateToken(jwtToken); - log.debug("JWT token validated successfully"); } catch (AuthenticationFailureException e) { - log.warn("JWT validation failed: {}", e.getMessage()); + log.debug("JWT validation failed: {}", e.getMessage()); handleAuthenticationFailure(request, response, e); return; } Map claims = jwtService.extractClaims(jwtToken); String tokenUsername = claims.get("sub").toString(); - log.debug("JWT token username: {}", tokenUsername); try { authenticate(request, claims); - log.debug("Authentication successful for user: {}", tokenUsername); } catch (SQLException | UnsupportedProviderException e) { log.error("Error processing user authentication for user: {}", tokenUsername, e); handleAuthenticationFailure( diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/service/JwtService.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/service/JwtService.java index 061b063aab..60472fef42 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/service/JwtService.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/service/JwtService.java @@ -50,7 +50,6 @@ public class JwtService implements JwtServiceInterface { KeyPersistenceServiceInterface keyPersistenceService) { this.v2Enabled = v2Enabled; this.keyPersistenceService = keyPersistenceService; - log.info("JwtService initialized"); } @Override @@ -256,11 +255,9 @@ public class JwtService implements JwtServiceInterface { String authHeader = request.getHeader("Authorization"); if (authHeader != null && authHeader.startsWith("Bearer ")) { String token = authHeader.substring(7); // Remove "Bearer " prefix - log.debug("JWT token extracted from Authorization header"); return token; } - log.debug("No JWT token found in Authorization header"); return null; } @@ -283,10 +280,9 @@ public class JwtService implements JwtServiceInterface { .parse(token) .getHeader() .get("kid"); - log.debug("Extracted key ID from token: {}", keyId); return keyId; } catch (Exception e) { - log.warn("Failed to extract key ID from token header: {}", e.getMessage()); + log.debug("Failed to extract key ID from token header: {}", e.getMessage()); return null; } } diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/service/KeyPairCleanupService.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/service/KeyPairCleanupService.java index b419f78fe2..aec455a929 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/service/KeyPairCleanupService.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/service/KeyPairCleanupService.java @@ -55,7 +55,6 @@ public class KeyPairCleanupService { return; } - log.info("Removing keys older than retention period"); removeKeys(eligibleKeys); keyPersistenceService.refreshActiveKeyPair(); } From 341adaa07ddd892c6bb1475c4821e5b8f99c2267 Mon Sep 17 00:00:00 2001 From: Dario Ghunney Ware Date: Tue, 2 Dec 2025 12:34:38 +0000 Subject: [PATCH 2/2] Grandpa Fix (#5030) PR to address inactive accounts (invited/pending activation) not being grandfathered during migration --------- Signed-off-by: dependabot[bot] Signed-off-by: stirlingbot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Ludy Co-authored-by: EthanHealy01 <80844253+EthanHealy01@users.noreply.github.com> Co-authored-by: Ethan Co-authored-by: Anthony Stirling <77850077+Frooodle@users.noreply.github.com> Co-authored-by: stirlingbot[bot] <195170888+stirlingbot[bot]@users.noreply.github.com> --- .../database/repository/UserRepository.java | 13 ++++ .../security/service/UserService.java | 26 +++++++ .../service/UserLicenseSettingsService.java | 16 +++-- .../UserLicenseSettingsServiceTest.java | 69 +++++++++++++++++++ 4 files changed, 120 insertions(+), 4 deletions(-) diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/database/repository/UserRepository.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/database/repository/UserRepository.java index 1a8b51bca1..312c19964d 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/database/repository/UserRepository.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/database/repository/UserRepository.java @@ -56,6 +56,19 @@ public interface UserRepository extends JpaRepository { + "OR LOWER(u.authenticationType) IN ('sso', 'oauth2', 'saml2')") List findAllSsoUsers(); + /** + * Finds SSO users who have never created a session (pending activation) and are not yet + * grandfathered. + */ + @Query( + "SELECT u FROM User u " + + "LEFT JOIN SessionEntity s ON u.username = s.principalName " + + "WHERE (u.ssoProvider IS NOT NULL " + + "OR LOWER(u.authenticationType) IN ('sso', 'oauth2', 'saml2')) " + + "AND (u.oauthGrandfathered IS NULL OR u.oauthGrandfathered = false) " + + "AND s.sessionId IS NULL") + List findPendingSsoUsersWithoutSession(); + /** * Counts all SSO users - those with sso_provider set OR authenticationType is sso/oauth2/saml2. */ diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/service/UserService.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/service/UserService.java index d131eb2bdc..d24e4722a2 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/service/UserService.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/service/UserService.java @@ -778,4 +778,30 @@ public class UserService implements UserServiceInterface { return updated; } + + /** + * Grandfathers SSO users who have never created a session (invited/pending accounts). These + * users would otherwise be blocked when SSO requires a paid license despite existing before the + * policy change. + * + * @return Number of pending users updated + */ + @Transactional + public int grandfatherPendingSsoUsersWithoutSession() { + List pendingUsers = userRepository.findPendingSsoUsersWithoutSession(); + int updated = 0; + + for (User user : pendingUsers) { + if (!user.isOauthGrandfathered()) { + user.setOauthGrandfathered(true); + updated++; + } + } + + if (updated > 0) { + userRepository.saveAll(pendingUsers); + } + + return updated; + } } diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/service/UserLicenseSettingsService.java b/app/proprietary/src/main/java/stirling/software/proprietary/service/UserLicenseSettingsService.java index 352ce0ed29..1cddefde3b 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/service/UserLicenseSettingsService.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/service/UserLicenseSettingsService.java @@ -192,10 +192,18 @@ public class UserLicenseSettingsService { + "They will retain OAuth access even without a paid license. " + "New users will require a paid license for OAuth.", updated); - } else if (grandfatheredCount > 0) { - log.debug( - "OAuth grandfathering already completed: {} users grandfathered", - grandfatheredCount); + } + + // Grandfather pending users (invited but never logged in) + // The query filters to non-grandfathered users only, so this is idempotent + if (grandfatheredCount > 0 || oauthUsersCount > 0) { + int pendingUpdated = userService.grandfatherPendingSsoUsersWithoutSession(); + if (pendingUpdated > 0) { + log.warn( + "OAuth GRANDFATHERING: Marked {} pending SSO users (no prior sessions) as" + + " grandfathered.", + pendingUpdated); + } } } } diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/service/UserLicenseSettingsServiceTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/service/UserLicenseSettingsServiceTest.java index c819055e47..139146d707 100644 --- a/app/proprietary/src/test/java/stirling/software/proprietary/service/UserLicenseSettingsServiceTest.java +++ b/app/proprietary/src/test/java/stirling/software/proprietary/service/UserLicenseSettingsServiceTest.java @@ -2,6 +2,9 @@ package stirling.software.proprietary.service; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import java.util.Optional; @@ -198,4 +201,70 @@ class UserLicenseSettingsServiceTest { assertEquals(5, result, "Should fall back to default 5 users if grandfathered is 0"); } + + @Test + void grandfatherExistingOAuthUsers_runsOnlyWhenNoneGrandfathered() { + // With grandfatheredCount == 0, should run grandfathering for all users + when(userService.countOAuthUsers()).thenReturn(10L); + when(userService.countGrandfatheredOAuthUsers()).thenReturn(0L); + when(userService.grandfatherAllOAuthUsers()).thenReturn(10); + when(userService.grandfatherPendingSsoUsersWithoutSession()).thenReturn(0); + + service.grandfatherExistingOAuthUsers(); + + verify(userService, times(1)).grandfatherAllOAuthUsers(); + verify(userService, times(1)).grandfatherPendingSsoUsersWithoutSession(); + } + + @Test + void grandfatherExistingOAuthUsers_skipsMainButRunsPendingWhenSomeAlreadyGrandfathered() { + // V2→V2.1 upgrade: some users already grandfathered, but pending users need to be checked + when(userService.countOAuthUsers()).thenReturn(10L); + when(userService.countGrandfatheredOAuthUsers()).thenReturn(4L); + when(userService.grandfatherPendingSsoUsersWithoutSession()).thenReturn(2); + + service.grandfatherExistingOAuthUsers(); + + verify(userService, never()).grandfatherAllOAuthUsers(); + verify(userService, times(1)).grandfatherPendingSsoUsersWithoutSession(); + } + + @Test + void grandfatherExistingOAuthUsers_stillChecksPendingWhenAllUsersGrandfathered() { + // All active users grandfathered, but still check for pending users + when(userService.countOAuthUsers()).thenReturn(10L); + when(userService.countGrandfatheredOAuthUsers()).thenReturn(10L); + when(userService.grandfatherPendingSsoUsersWithoutSession()).thenReturn(0); + + service.grandfatherExistingOAuthUsers(); + + verify(userService, never()).grandfatherAllOAuthUsers(); + verify(userService, times(1)).grandfatherPendingSsoUsersWithoutSession(); + } + + @Test + void grandfatherExistingOAuthUsers_skipsWhenNoOAuthUsers() { + when(userService.countOAuthUsers()).thenReturn(0L); + when(userService.countGrandfatheredOAuthUsers()).thenReturn(0L); + + service.grandfatherExistingOAuthUsers(); + + verify(userService, never()).grandfatherAllOAuthUsers(); + verify(userService, never()).grandfatherPendingSsoUsersWithoutSession(); + } + + @Test + void grandfatherExistingOAuthUsers_grandfathersPendingUsersOnFirstRun() { + // Pending users (invited but never logged in) should be grandfathered + // during the initial grandfathering run (when grandfatheredCount == 0) + when(userService.countOAuthUsers()).thenReturn(5L); + when(userService.countGrandfatheredOAuthUsers()).thenReturn(0L); + when(userService.grandfatherAllOAuthUsers()).thenReturn(5); + when(userService.grandfatherPendingSsoUsersWithoutSession()).thenReturn(3); + + service.grandfatherExistingOAuthUsers(); + + verify(userService, times(1)).grandfatherAllOAuthUsers(); + verify(userService, times(1)).grandfatherPendingSsoUsersWithoutSession(); + } }