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 a10e161a83..5449d54413 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 @@ -184,30 +184,28 @@ public class UserLicenseSettingsService { long oauthUsersCount = userService.countOAuthUsers(); long grandfatheredCount = userService.countGrandfatheredOAuthUsers(); - if (oauthUsersCount > 0 && grandfatheredCount < oauthUsersCount) { - // We have OAuth users but not all have been grandfathered - this is first run after - // upgrade + if (oauthUsersCount > 0 && grandfatheredCount == 0) { + // We have OAuth users but none are grandfathered - this is first run after upgrade int updated = userService.grandfatherAllOAuthUsers(); log.warn( "OAuth GRANDFATHERING: Marked {} existing OAuth/SAML users as grandfathered. " + "They will retain OAuth access even without a paid license. " + "New users will require a paid license for OAuth.", updated); + + // Also grandfather pending users (invited but never logged in) at the same time + int pendingUpdated = userService.grandfatherPendingSsoUsersWithoutSession(); + if (pendingUpdated > 0) { + log.warn( + "OAuth GRANDFATHERING: Marked {} pending SSO users (no prior sessions) as" + + " grandfathered.", + pendingUpdated); + } } else if (grandfatheredCount > 0) { log.debug( "OAuth grandfathering already completed: {} users grandfathered", grandfatheredCount); } - - int pendingUpdated = userService.grandfatherPendingSsoUsersWithoutSession(); - if (pendingUpdated > 0) { - log.warn( - "OAuth GRANDFATHERING: Marked {} pending SSO users (no prior sessions) as" - + " grandfathered.", - pendingUpdated); - } else { - log.debug("No pending SSO users required grandfathering"); - } } } 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 03a40fde98..692bf560c4 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 @@ -203,14 +203,29 @@ class UserLicenseSettingsServiceTest { } @Test - void grandfatherExistingOAuthUsers_runsWhenSomeUsersNotGrandfathered() { + void grandfatherExistingOAuthUsers_runsOnlyWhenNoneGrandfathered() { + // With grandfatheredCount == 0, should run grandfathering when(userService.countOAuthUsers()).thenReturn(10L); - when(userService.countGrandfatheredOAuthUsers()).thenReturn(4L); - when(userService.grandfatherAllOAuthUsers()).thenReturn(6); + 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_skipsWhenSomeAlreadyGrandfathered() { + // If any users are already grandfathered, skip (grandfathering already happened) + when(userService.countOAuthUsers()).thenReturn(10L); + when(userService.countGrandfatheredOAuthUsers()).thenReturn(4L); + + service.grandfatherExistingOAuthUsers(); + + verify(userService, never()).grandfatherAllOAuthUsers(); + verify(userService, never()).grandfatherPendingSsoUsersWithoutSession(); } @Test @@ -221,6 +236,7 @@ class UserLicenseSettingsServiceTest { service.grandfatherExistingOAuthUsers(); verify(userService, never()).grandfatherAllOAuthUsers(); + verify(userService, never()).grandfatherPendingSsoUsersWithoutSession(); } @Test @@ -231,28 +247,21 @@ class UserLicenseSettingsServiceTest { service.grandfatherExistingOAuthUsers(); verify(userService, never()).grandfatherAllOAuthUsers(); + verify(userService, never()).grandfatherPendingSsoUsersWithoutSession(); } @Test - void grandfatherExistingOAuthUsers_handlesPendingUsersWithoutSessions() { + void grandfatherExistingOAuthUsers_grandfathersPendingUsersOnFirstRun() { + // Pending users (invited but never logged in) should be grandfathered + // only during the initial grandfathering run (when grandfatheredCount == 0) when(userService.countOAuthUsers()).thenReturn(5L); - when(userService.countGrandfatheredOAuthUsers()).thenReturn(5L); + when(userService.countGrandfatheredOAuthUsers()).thenReturn(0L); + when(userService.grandfatherAllOAuthUsers()).thenReturn(5); when(userService.grandfatherPendingSsoUsersWithoutSession()).thenReturn(3); service.grandfatherExistingOAuthUsers(); - verify(userService, never()).grandfatherAllOAuthUsers(); - verify(userService, times(1)).grandfatherPendingSsoUsersWithoutSession(); - } - - @Test - void grandfatherExistingOAuthUsers_checksPendingUsersEvenWhenNoneExist() { - when(userService.countOAuthUsers()).thenReturn(0L); - when(userService.countGrandfatheredOAuthUsers()).thenReturn(0L); - when(userService.grandfatherPendingSsoUsersWithoutSession()).thenReturn(0); - - service.grandfatherExistingOAuthUsers(); - + verify(userService, times(1)).grandfatherAllOAuthUsers(); verify(userService, times(1)).grandfatherPendingSsoUsersWithoutSession(); } }