From 341adaa07ddd892c6bb1475c4821e5b8f99c2267 Mon Sep 17 00:00:00 2001 From: Dario Ghunney Ware Date: Tue, 2 Dec 2025 12:34:38 +0000 Subject: [PATCH] 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 1a8b51bca..312c19964 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 d131eb2bd..d24e4722a 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 352ce0ed2..1cddefde3 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 c819055e4..139146d70 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(); + } }