From dae0b077f82939b3b2750eef9027c815324446d4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 3 Jul 2026 03:05:42 +0000 Subject: [PATCH] feat: retry token refresh before checking for clock out-of-sync - tokenHasExpiredDueToClockOutOfSync$: skip the first "token expired" event so the library's automatic refresh has a chance to run before clock drift is diagnosed as the root cause - oauthErrorEventOccurDueToClockOutOfSync$: use scan() to skip the first token_refresh_error (allowing one retry) and only check clock sync on the second occurrence; all other error types are still checked immediately - Update test for token expiry: now requires two events before logout - Replace single token_refresh_error clock-sync test with two tests: one verifying no logout on first error, one verifying logout on second --- .../auth/oidc/redirect-auth.service.spec.ts | 51 ++++++++++++++++++- .../lib/auth/oidc/redirect-auth.service.ts | 20 +++++++- 2 files changed, 68 insertions(+), 3 deletions(-) diff --git a/lib/core/src/lib/auth/oidc/redirect-auth.service.spec.ts b/lib/core/src/lib/auth/oidc/redirect-auth.service.spec.ts index 88a03bbf6e..14463265df 100644 --- a/lib/core/src/lib/auth/oidc/redirect-auth.service.spec.ts +++ b/lib/core/src/lib/auth/oidc/redirect-auth.service.spec.ts @@ -254,6 +254,11 @@ describe('RedirectAuthService', () => { timeSyncServiceSpy.getCorrectedNow.and.returnValue(mockDateNowInMilliseconds); oauthServiceSpy.getIdentityClaims.and.returnValue({ exp: tokenExpiresAtInSeconds, iat: tokenIssuedAtInSeconds }); + // First event is skipped to allow at least one token refresh attempt + oauthEvents$.next({ type: 'discovery_document_loaded' } as OAuthEvent); + expect(oauthServiceSpy.logOut).not.toHaveBeenCalled(); + + // Token still expired after the refresh attempt — clock drift is confirmed oauthEvents$.next({ type: 'discovery_document_loaded' } as OAuthEvent); expect(oauthServiceSpy.logOut).toHaveBeenCalledTimes(1); @@ -349,6 +354,31 @@ describe('RedirectAuthService', () => { expect(oauthLoggerSpy.error).not.toHaveBeenCalled(); }); + it('should NOT logout user on the first event where token has expired, to allow a refresh attempt', () => { + const mockTimeSync: TimeSync = { + outOfSync: true, + localDateTimeISO: '2024-10-10T22:00:18.621Z', + serverDateTimeISO: '2024-10-10T22:10:53.000Z' + }; + timeSyncServiceSpy.checkTimeSync.and.returnValue(of(mockTimeSync)); + + const mockDateNowInMilliseconds = 1728597618621; // GMT: Thursday, October 10, 2024 10:00:18.621 PM + + const tokenExpiresAtInSeconds = 1728598353; // GMT: Thursday, October 10, 2024 10:15:00 PM + const tokenIssuedAtInSeconds = 1728598253; // GMT: Thursday, October 10, 2024 10:10:53 PM + + oauthServiceSpy.clockSkewInSec = 120; + + timeSyncServiceSpy.getCorrectedNow.and.returnValue(mockDateNowInMilliseconds); + oauthServiceSpy.getIdentityClaims.and.returnValue({ exp: tokenExpiresAtInSeconds, iat: tokenIssuedAtInSeconds }); + + // First event is skipped (retry allowed) + oauthEvents$.next(new OAuthSuccessEvent('discovery_document_loaded')); + + expect(oauthServiceSpy.logOut).not.toHaveBeenCalled(); + expect(oauthLoggerSpy.error).not.toHaveBeenCalled(); + }); + it('should NOT logout user if token has expired but local clock sync status cannot be determined', () => { timeSyncServiceSpy.checkTimeSync.and.throwError('Error'); @@ -433,7 +463,19 @@ describe('RedirectAuthService', () => { expect(oauthLoggerSpy.error).toHaveBeenCalledWith(expectedErrorCausedBySecondTokenRefreshError); })); - it('should logout user if token_refresh_error is emitted because of clock out of sync', () => { + it('should NOT logout user on the first token_refresh_error even if clock is out of sync', () => { + timeSyncServiceSpy.checkTimeSync.and.returnValue( + of({ outOfSync: true, localDateTimeISO: '2024-10-10T22:00:18.621Z', serverDateTimeISO: '2024-10-10T22:10:53.000Z' } as TimeSync) + ); + + // First occurrence is skipped to allow a retry + oauthEvents$.next(new OAuthErrorEvent('token_refresh_error', { reason: 'first error' }, {})); + + expect(oauthServiceSpy.logOut).not.toHaveBeenCalled(); + expect(oauthLoggerSpy.error).not.toHaveBeenCalled(); + }); + + it('should logout user if token_refresh_error is emitted a second time because of clock out of sync', () => { const expectedErrorMessage = new Error( 'OAuth error occurred due to local machine clock 2024-10-10T22:00:18.621Z being out of sync with server time 2024-10-10T22:10:53.000Z' ); @@ -441,7 +483,12 @@ describe('RedirectAuthService', () => { of({ outOfSync: true, localDateTimeISO: '2024-10-10T22:00:18.621Z', serverDateTimeISO: '2024-10-10T22:10:53.000Z' } as TimeSync) ); - oauthEvents$.next(new OAuthErrorEvent('token_refresh_error', { reason: 'error' }, {})); + // First occurrence is skipped (retry allowed) + oauthEvents$.next(new OAuthErrorEvent('token_refresh_error', { reason: 'first error' }, {})); + expect(oauthServiceSpy.logOut).not.toHaveBeenCalled(); + + // Second occurrence triggers the clock-out-of-sync logout + oauthEvents$.next(new OAuthErrorEvent('token_refresh_error', { reason: 'second error' }, {})); expect(oauthServiceSpy.logOut).toHaveBeenCalledTimes(1); expect(oauthLoggerSpy.error).toHaveBeenCalledWith(expectedErrorMessage); diff --git a/lib/core/src/lib/auth/oidc/redirect-auth.service.ts b/lib/core/src/lib/auth/oidc/redirect-auth.service.ts index a6a5b6ee2b..d838edc2c8 100644 --- a/lib/core/src/lib/auth/oidc/redirect-auth.service.ts +++ b/lib/core/src/lib/auth/oidc/redirect-auth.service.ts @@ -30,7 +30,7 @@ import { } from 'angular-oauth2-oidc'; import { WebCryptoJwksValidationHandler } from './web-crypto-jwks-validation-handler'; import { from, Observable, race, ReplaySubject } from 'rxjs'; -import { distinctUntilChanged, filter, map, shareReplay, switchMap, take } from 'rxjs/operators'; +import { distinctUntilChanged, filter, map, scan, shareReplay, skip, switchMap, take } from 'rxjs/operators'; import { AuthService } from './auth.service'; import { AUTH_MODULE_CONFIG, AuthModuleConfig } from './auth-config'; import { RetryLoginService } from './retry-login.service'; @@ -176,6 +176,20 @@ export class RedirectAuthService extends AuthService { ); this.oauthErrorEventOccurDueToClockOutOfSync$ = this.oauthErrorEvent$.pipe( + // For token_refresh_error, skip the first occurrence so the library's + // built-in retry (secondTokenRefreshErrorEventOccur$) has a chance to run + // before we conclude the issue is clock drift. All other error types are + // checked immediately. + scan( + (acc, event) => ({ + event, + shouldProcess: event.type !== 'token_refresh_error' || acc.tokenRefreshErrorCount >= 1, + tokenRefreshErrorCount: event.type === 'token_refresh_error' ? acc.tokenRefreshErrorCount + 1 : acc.tokenRefreshErrorCount + }), + { event: null as OAuthErrorEvent | null, shouldProcess: false, tokenRefreshErrorCount: 0 } + ), + filter(({ shouldProcess }) => shouldProcess), + map(({ event }) => event as OAuthErrorEvent), switchMap(() => this._timeSyncService.checkTimeSync(this.oauthService.clockSkewInSec)), filter((timeSync) => timeSync?.outOfSync), map( @@ -196,6 +210,10 @@ export class RedirectAuthService extends AuthService { this.tokenHasExpiredDueToClockOutOfSync$ = this.oauthService.events.pipe( map(() => !!this.oauthService.getIdentityClaims() && this.tokenHasExpired()), filter((hasExpired) => hasExpired), + // Skip the first occurrence: the library will attempt an automatic token + // refresh. Only check for clock drift if the token is still expired after + // that refresh attempt has had a chance to run. + skip(1), switchMap(() => this._timeSyncService.checkTimeSync(this.oauthService.clockSkewInSec)), filter((timeSync) => timeSync?.outOfSync), map(