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 ff55c4cac9..88a03bbf6e 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 @@ -52,7 +52,13 @@ describe('RedirectAuthService', () => { beforeEach(() => { retryLoginServiceSpy = jasmine.createSpyObj('RetryLoginService', ['tryToLoginTimes']); - timeSyncServiceSpy = jasmine.createSpyObj('TimeSyncService', ['checkTimeSync', 'getCorrectedNow', 'syncClockOffset']); + timeSyncServiceSpy = jasmine.createSpyObj('TimeSyncService', [ + 'checkTimeSync', + 'getCorrectedNow', + 'syncClockOffset', + 'startPeriodicSync', + 'stopPeriodicSync' + ]); oauthLoggerSpy = jasmine.createSpyObj('OAuthLogger', ['error', 'info', 'warn']); oauthServiceSpy = jasmine.createSpyObj( 'OAuthService', 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 3325b6d617..a6a5b6ee2b 100644 --- a/lib/core/src/lib/auth/oidc/redirect-auth.service.ts +++ b/lib/core/src/lib/auth/oidc/redirect-auth.service.ts @@ -261,6 +261,7 @@ export class RedirectAuthService extends AuthService { } logout() { + this._timeSyncService.stopPeriodicSync(); this.oauthService.logOut(); } @@ -351,6 +352,7 @@ export class RedirectAuthService extends AuthService { this._isDiscoveryDocumentLoadedSubject$.next(true); this.oauthService.setupAutomaticSilentRefresh(); this._timeSyncService.syncClockOffset().subscribe(); + this._timeSyncService.startPeriodicSync(undefined, this.oauthService.clockSkewInSec * 1000); return void this.allowRefreshTokenAndSilentRefreshOnMultipleTabs(); }) .catch(() => { diff --git a/lib/core/src/lib/auth/services/time-sync.service.spec.ts b/lib/core/src/lib/auth/services/time-sync.service.spec.ts index 9dbdf4dd39..c6eae59543 100644 --- a/lib/core/src/lib/auth/services/time-sync.service.spec.ts +++ b/lib/core/src/lib/auth/services/time-sync.service.spec.ts @@ -39,6 +39,7 @@ describe('TimeSyncService', () => { }); afterEach(() => { + service.stopPeriodicSync(); httpMock.verify(); }); @@ -235,16 +236,62 @@ describe('TimeSyncService', () => { httpMock.expectNone('http://fake-server-time-url'); }); - it('should leave clockOffsetMs at 0 when the server time endpoint fails', () => { + it('should leave clockOffsetMs unchanged when the server time endpoint fails', () => { appConfigSpy.get.and.returnValue('http://fake-server-time-url'); + service.clockOffsetMs = 5000; service.syncClockOffset().subscribe(() => { - expect(service.clockOffsetMs).toBe(0); + expect(service.clockOffsetMs).toBe(5000); }); const req = httpMock.expectOne('http://fake-server-time-url'); req.error(new ProgressEvent('')); }); + + it('should not update offset when it exceeds maxAllowedOffsetMs', () => { + appConfigSpy.get.and.returnValue('http://fake-server-time-url'); + + const timeBeforeRequest = 1728911579000; + const timeResponseReceived = 1728911580000; + + // Server is 600 seconds ahead (way beyond our cap) + const serverTime = 1728912180000; + + spyOn(Date, 'now').and.returnValues(timeBeforeRequest, timeResponseReceived); + + service.clockOffsetMs = 1000; + + // Cap at 60 seconds (60000 ms) + service.syncClockOffset(60000).subscribe(() => { + // Offset should remain unchanged because computed offset exceeds cap + expect(service.clockOffsetMs).toBe(1000); + }); + + const req = httpMock.expectOne('http://fake-server-time-url'); + req.flush(serverTime); + }); + + it('should update offset when it is within maxAllowedOffsetMs', () => { + appConfigSpy.get.and.returnValue('http://fake-server-time-url'); + + const timeBeforeRequest = 1728911579000; + const timeResponseReceived = 1728911580000; + + // Server is 30 seconds ahead (within our cap) + const serverTime = 1728911610000; + // adjustedServerTime = 1728911610000 + 1000/2 = 1728911610500 + // offset = 1728911610500 - 1728911580000 = 30500 ms + + spyOn(Date, 'now').and.returnValues(timeBeforeRequest, timeResponseReceived); + + // Cap at 60 seconds (60000 ms) + service.syncClockOffset(60000).subscribe(() => { + expect(service.clockOffsetMs).toBe(30500); + }); + + const req = httpMock.expectOne('http://fake-server-time-url'); + req.flush(serverTime); + }); }); describe('getCorrectedNow', () => { @@ -273,4 +320,63 @@ describe('TimeSyncService', () => { expect(service.getCorrectedNow()).toBe(fixedNow - 60000); }); }); + + describe('startPeriodicSync', () => { + it('should re-sync on visibility change', () => { + appConfigSpy.get.and.returnValue('http://fake-server-time-url'); + + const timeBeforeRequest = 1728911579000; + const timeResponseReceived = 1728911580000; + const serverTime = 1728911610000; + + spyOn(Date, 'now').and.returnValues(timeBeforeRequest, timeResponseReceived); + + service.startPeriodicSync(60000); + + Object.defineProperty(document, 'visibilityState', { value: 'visible', writable: true, configurable: true }); + document.dispatchEvent(new Event('visibilitychange')); + + const req = httpMock.expectOne('http://fake-server-time-url'); + req.flush(serverTime); + + expect(service.clockOffsetMs).toBe(30500); + }); + + it('should apply maxAllowedOffsetMs cap during visibility re-sync', () => { + appConfigSpy.get.and.returnValue('http://fake-server-time-url'); + + const timeBeforeRequest = 1728911579000; + const timeResponseReceived = 1728911580000; + // Server is 600 seconds ahead — exceeds cap + const serverTime = 1728912180000; + + spyOn(Date, 'now').and.returnValues(timeBeforeRequest, timeResponseReceived); + + service.clockOffsetMs = 1000; + service.startPeriodicSync(60000, 60000); + + Object.defineProperty(document, 'visibilityState', { value: 'visible', writable: true, configurable: true }); + document.dispatchEvent(new Event('visibilitychange')); + + const req = httpMock.expectOne('http://fake-server-time-url'); + req.flush(serverTime); + + // Offset should remain unchanged + expect(service.clockOffsetMs).toBe(1000); + }); + }); + + describe('stopPeriodicSync', () => { + it('should remove visibility change listener', () => { + appConfigSpy.get.and.returnValue('http://fake-server-time-url'); + + service.startPeriodicSync(60000); + service.stopPeriodicSync(); + + Object.defineProperty(document, 'visibilityState', { value: 'visible', writable: true, configurable: true }); + document.dispatchEvent(new Event('visibilitychange')); + + httpMock.expectNone('http://fake-server-time-url'); + }); + }); }); diff --git a/lib/core/src/lib/auth/services/time-sync.service.ts b/lib/core/src/lib/auth/services/time-sync.service.ts index 39451c211c..6cd59a1346 100644 --- a/lib/core/src/lib/auth/services/time-sync.service.ts +++ b/lib/core/src/lib/auth/services/time-sync.service.ts @@ -16,10 +16,10 @@ */ import { HttpClient } from '@angular/common/http'; -import { Injectable, Injector, inject } from '@angular/core'; +import { Injectable, Injector, NgZone, inject } from '@angular/core'; import { AppConfigService } from '../../app-config/app-config.service'; -import { from, Observable, of, throwError } from 'rxjs'; -import { catchError, map, timeout } from 'rxjs/operators'; +import { from, interval, Observable, of, Subscription, throwError } from 'rxjs'; +import { catchError, map, switchMap, timeout } from 'rxjs/operators'; export interface TimeSync { outOfSync: boolean; @@ -28,12 +28,16 @@ export interface TimeSync { serverDateTimeISO: string; } +/** Default interval for periodic clock re-sync (5 minutes). */ +const DEFAULT_PERIODIC_SYNC_INTERVAL_MS = 5 * 60 * 1000; + @Injectable({ providedIn: 'root' }) export class TimeSyncService { private readonly _injector = inject(Injector); private readonly _appConfigService = inject(AppConfigService); + private readonly _ngZone = inject(NgZone); private readonly _http: HttpClient; @@ -44,6 +48,9 @@ export class TimeSyncService { */ clockOffsetMs = 0; + private _periodicSyncSubscription: Subscription | null = null; + private _visibilityChangeHandler: (() => void) | null = null; + constructor() { this._http = this._injector.get(HttpClient); } @@ -63,11 +70,15 @@ export class TimeSyncService { * Fetches the server time once and stores the signed clock offset in `clockOffsetMs`. * Call this at application start-up (fire-and-forget) so subsequent calls to * `getCorrectedNow` compensate for any VM / Citrix clock drift. - * If `serverTimeUrl` is not configured or the request fails, the offset is left at 0. + * If `serverTimeUrl` is not configured or the request fails, the offset is left unchanged + * (or at 0 if this is the first call). * + * @param maxAllowedOffsetMs Optional safety cap. If the computed offset exceeds this value, + * it is ignored to prevent a compromised time endpoint from + * tricking the client into accepting expired tokens. * @returns Observable that completes after the offset has been stored (or silently on error) */ - syncClockOffset(): Observable { + syncClockOffset(maxAllowedOffsetMs?: number): Observable { try { const startTime = Date.now(); return this.getServerTime().pipe( @@ -79,15 +90,17 @@ export class TimeSyncService { const serverTimeInMs = isServerTimeResponseInMs ? serverTimeResponse : serverTimeResponse * 1000; const adjustedServerTimeInMs = serverTimeInMs + roundTripTimeInMs / 2; - this.clockOffsetMs = adjustedServerTimeInMs - endTime; + const newOffset = adjustedServerTimeInMs - endTime; + + if (maxAllowedOffsetMs != null && Math.abs(newOffset) > maxAllowedOffsetMs) { + return; + } + + this.clockOffsetMs = newOffset; }), - catchError(() => { - this.clockOffsetMs = 0; - return of(void 0); - }) + catchError(() => of(void 0)) ); } catch { - this.clockOffsetMs = 0; return of(void 0); } } @@ -135,6 +148,50 @@ export class TimeSyncService { return this.checkTimeSync(maxAllowedClockSkewInSec).pipe(map((sync) => sync.outOfSync)); } + /** + * Starts periodic re-synchronization of the clock offset to protect against + * progressive clock drift during a user session (common in Citrix/VM environments). + * + * Re-sync is triggered: + * - On a regular interval (default: every 5 minutes) + * - When the document becomes visible again (e.g., Citrix session resumes after idle) + * + * @param intervalMs How often to re-sync in milliseconds (default: 5 minutes) + * @param maxAllowedOffsetMs Safety cap for the offset. If exceeded, the new offset is ignored. + */ + startPeriodicSync(intervalMs: number = DEFAULT_PERIODIC_SYNC_INTERVAL_MS, maxAllowedOffsetMs?: number): void { + this.stopPeriodicSync(); + + this._ngZone.runOutsideAngular(() => { + this._periodicSyncSubscription = interval(intervalMs) + .pipe(switchMap(() => this.syncClockOffset(maxAllowedOffsetMs))) + .subscribe(); + + this._visibilityChangeHandler = () => { + if (typeof document !== 'undefined' && document.visibilityState === 'visible') { + this.syncClockOffset(maxAllowedOffsetMs).subscribe(); + } + }; + + if (typeof document !== 'undefined') { + document.addEventListener('visibilitychange', this._visibilityChangeHandler); + } + }); + } + + /** + * Stops the periodic clock re-synchronization and removes the visibility change listener. + */ + stopPeriodicSync(): void { + this._periodicSyncSubscription?.unsubscribe(); + this._periodicSyncSubscription = null; + + if (this._visibilityChangeHandler && typeof document !== 'undefined') { + document.removeEventListener('visibilitychange', this._visibilityChangeHandler); + this._visibilityChangeHandler = null; + } + } + private getServerTime(): Observable { return from(this._http.get(this.getServerTimeUrl())).pipe( timeout(5000),