From 2fa592eccb86892be6b214a5556b371f0fb87e93 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 1 Jul 2026 20:07:19 +0000 Subject: [PATCH] fix: compensate for VM/Citrix clock drift in token expiry check --- .../auth/oidc/redirect-auth.service.spec.ts | 22 +++-- .../lib/auth/oidc/redirect-auth.service.ts | 3 +- .../auth/services/time-sync.service.spec.ts | 92 +++++++++++++++++++ .../lib/auth/services/time-sync.service.ts | 49 +++++++++- 4 files changed, 158 insertions(+), 8 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 bc16799499..ff55c4cac9 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,7 @@ describe('RedirectAuthService', () => { beforeEach(() => { retryLoginServiceSpy = jasmine.createSpyObj('RetryLoginService', ['tryToLoginTimes']); - timeSyncServiceSpy = jasmine.createSpyObj('TimeSyncService', ['checkTimeSync']); + timeSyncServiceSpy = jasmine.createSpyObj('TimeSyncService', ['checkTimeSync', 'getCorrectedNow', 'syncClockOffset']); oauthLoggerSpy = jasmine.createSpyObj('OAuthLogger', ['error', 'info', 'warn']); oauthServiceSpy = jasmine.createSpyObj( 'OAuthService', @@ -87,6 +87,8 @@ describe('RedirectAuthService', () => { service = TestBed.inject(RedirectAuthService); timeSyncServiceSpy.checkTimeSync.and.returnValue(of({ outOfSync: false } as TimeSync)); + timeSyncServiceSpy.getCorrectedNow.and.callFake(() => Date.now()); + timeSyncServiceSpy.syncClockOffset.and.returnValue(of(void 0)); ensureDiscoveryDocumentSpy = spyOn(service, 'ensureDiscoveryDocument'); }); @@ -164,6 +166,14 @@ describe('RedirectAuthService', () => { expect(mockOAuthStorage.removeItem).not.toHaveBeenCalled(); }); + it('should call syncClockOffset when the discovery document has loaded', async () => { + ensureDiscoveryDocumentSpy.and.resolveTo(true); + + await service.init(); + + expect(timeSyncServiceSpy.syncClockOffset).toHaveBeenCalledTimes(1); + }); + it('should configure OAuthService with given config', async () => { const config = { sessionChecksEnabled: false } as AuthConfig; ensureDiscoveryDocumentSpy.and.resolveTo(true); @@ -235,7 +245,7 @@ describe('RedirectAuthService', () => { oauthServiceSpy.clockSkewInSec = 120; - spyOn(Date, 'now').and.returnValue(mockDateNowInMilliseconds); + timeSyncServiceSpy.getCorrectedNow.and.returnValue(mockDateNowInMilliseconds); oauthServiceSpy.getIdentityClaims.and.returnValue({ exp: tokenExpiresAtInSeconds, iat: tokenIssuedAtInSeconds }); oauthEvents$.next({ type: 'discovery_document_loaded' } as OAuthEvent); @@ -324,7 +334,7 @@ describe('RedirectAuthService', () => { oauthServiceSpy.clockSkewInSec = 120; - spyOn(Date, 'now').and.returnValue(mockDateNowInMilliseconds); + timeSyncServiceSpy.getCorrectedNow.and.returnValue(mockDateNowInMilliseconds); oauthServiceSpy.getIdentityClaims.and.returnValue({ exp: tokenExpiresAtInSeconds, iat: tokenIssuedAtInSeconds }); oauthEvents$.next(new OAuthSuccessEvent('discovery_document_loaded')); @@ -343,7 +353,7 @@ describe('RedirectAuthService', () => { oauthServiceSpy.clockSkewInSec = 120; - spyOn(Date, 'now').and.returnValue(mockDateNowInMilliseconds); + timeSyncServiceSpy.getCorrectedNow.and.returnValue(mockDateNowInMilliseconds); oauthServiceSpy.getIdentityClaims.and.returnValue({ exp: tokenExpiresAtInSeconds, iat: tokenIssuedAtInSeconds }); oauthEvents$.next(new OAuthSuccessEvent('discovery_document_loaded')); @@ -360,7 +370,7 @@ describe('RedirectAuthService', () => { oauthServiceSpy.clockSkewInSec = 120; - spyOn(Date, 'now').and.returnValue(mockDateNowInMilliseconds); + timeSyncServiceSpy.getCorrectedNow.and.returnValue(mockDateNowInMilliseconds); oauthServiceSpy.getIdentityClaims.and.returnValue({ exp: tokenExpiresAtInSeconds, iat: tokenIssuedAtInSeconds }); oauthEvents$.next(new OAuthSuccessEvent('discovery_document_loaded')); @@ -377,7 +387,7 @@ describe('RedirectAuthService', () => { oauthServiceSpy.clockSkewInSec = 120; - spyOn(Date, 'now').and.returnValue(mockDateNowInMilliseconds); + timeSyncServiceSpy.getCorrectedNow.and.returnValue(mockDateNowInMilliseconds); oauthServiceSpy.getIdentityClaims.and.returnValue({ exp: tokenExpiresAtInSeconds, iat: tokenIssuedAtInSeconds }); oauthEvents$.next(new OAuthSuccessEvent('discovery_document_loaded')); 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 2829c07cb0..3325b6d617 100644 --- a/lib/core/src/lib/auth/oidc/redirect-auth.service.ts +++ b/lib/core/src/lib/auth/oidc/redirect-auth.service.ts @@ -350,6 +350,7 @@ export class RedirectAuthService extends AuthService { .then(() => { this._isDiscoveryDocumentLoadedSubject$.next(true); this.oauthService.setupAutomaticSilentRefresh(); + this._timeSyncService.syncClockOffset().subscribe(); return void this.allowRefreshTokenAndSilentRefreshOnMultipleTabs(); }) .catch(() => { @@ -419,7 +420,7 @@ export class RedirectAuthService extends AuthService { this._oauthLogger.warn('No claims found in the token'); return false; } - const now = Date.now(); + const now = this._timeSyncService.getCorrectedNow(); const issuedAtMSec = claims.iat * 1000; const expiresAtMSec = claims.exp * 1000; const clockSkewInMSec = this.oauthService.clockSkewInSec * 1000; 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 a2c8d6b66a..9dbdf4dd39 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 @@ -181,4 +181,96 @@ describe('TimeSyncService', () => { req.flush(serverTime); }); }); + + describe('syncClockOffset', () => { + it('should store a positive offset when the local clock is behind the server', () => { + appConfigSpy.get.and.returnValue('http://fake-server-time-url'); + + const timeBeforeRequest = 1728911579000; // (GMT): Monday, October 14, 2024 1:12:59 PM + const timeResponseReceived = 1728911580000; // (GMT): Monday, October 14, 2024 1:13:00 PM + const timeAfterOffsetCalc = 1728911580000; + + // Server is 60 seconds ahead of the client + const serverTime = 1728911640000; // (GMT): Monday, October 14, 2024 1:14:00 PM + // adjustedServerTime = 1728911640000 + 1000/2 = 1728911640500 + // expectedOffset = 1728911640500 - 1728911580000 = 60500 ms + + spyOn(Date, 'now').and.returnValues(timeBeforeRequest, timeResponseReceived, timeAfterOffsetCalc); + + service.syncClockOffset().subscribe(() => { + expect(service.clockOffsetMs).toBe(60500); + }); + + const req = httpMock.expectOne('http://fake-server-time-url'); + req.flush(serverTime); + }); + + it('should store 0 offset when local clock matches the server', () => { + appConfigSpy.get.and.returnValue('http://fake-server-time-url'); + + const requestTime = 1728911580000; + const responseTime = 1728911580000; + const afterCalcTime = 1728911580000; + const serverTime = 1728911580000; // same as local + + spyOn(Date, 'now').and.returnValues(requestTime, responseTime, afterCalcTime); + + service.syncClockOffset().subscribe(() => { + // adjustedServerTime = 1728911580000 + 0/2 = 1728911580000 + // offset = 1728911580000 - 1728911580000 = 0 + expect(service.clockOffsetMs).toBe(0); + }); + + const req = httpMock.expectOne('http://fake-server-time-url'); + req.flush(serverTime); + }); + + it('should leave clockOffsetMs at 0 when serverTimeUrl is not configured', () => { + appConfigSpy.get.and.returnValue(''); + + service.syncClockOffset().subscribe(() => { + expect(service.clockOffsetMs).toBe(0); + }); + + httpMock.expectNone('http://fake-server-time-url'); + }); + + it('should leave clockOffsetMs at 0 when the server time endpoint fails', () => { + appConfigSpy.get.and.returnValue('http://fake-server-time-url'); + + service.syncClockOffset().subscribe(() => { + expect(service.clockOffsetMs).toBe(0); + }); + + const req = httpMock.expectOne('http://fake-server-time-url'); + req.error(new ProgressEvent('')); + }); + }); + + describe('getCorrectedNow', () => { + it('should return Date.now() when clockOffsetMs is 0', () => { + const fixedNow = 1728911580000; + spyOn(Date, 'now').and.returnValue(fixedNow); + + expect(service.getCorrectedNow()).toBe(fixedNow); + }); + + it('should return Date.now() plus the stored offset', () => { + const fixedNow = 1728911580000; + spyOn(Date, 'now').and.returnValue(fixedNow); + + service.clockOffsetMs = 60000; + + expect(service.getCorrectedNow()).toBe(fixedNow + 60000); + }); + + it('should return Date.now() minus the stored offset when local clock is ahead', () => { + const fixedNow = 1728911640000; + spyOn(Date, 'now').and.returnValue(fixedNow); + + service.clockOffsetMs = -60000; + + expect(service.getCorrectedNow()).toBe(fixedNow - 60000); + }); + }); }); 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 a806d17777..8ad3950fb1 100644 --- a/lib/core/src/lib/auth/services/time-sync.service.ts +++ b/lib/core/src/lib/auth/services/time-sync.service.ts @@ -18,7 +18,7 @@ import { HttpClient } from '@angular/common/http'; import { Injectable, Injector, inject } from '@angular/core'; import { AppConfigService } from '../../app-config/app-config.service'; -import { from, Observable, throwError } from 'rxjs'; +import { from, Observable, of, throwError } from 'rxjs'; import { catchError, map, timeout } from 'rxjs/operators'; export interface TimeSync { @@ -37,10 +37,57 @@ export class TimeSyncService { private readonly _http: HttpClient; + /** + * The signed offset in milliseconds between the adjusted server time and the local clock. + * Positive means the local clock is behind the server; negative means it is ahead. + * Defaults to 0 until `syncClockOffset` has successfully run. + */ + clockOffsetMs = 0; + constructor() { this._http = this._injector.get(HttpClient); } + /** + * Returns the current local time corrected by the last measured clock offset. + * Use this instead of `Date.now()` when evaluating token expiration to avoid + * false positives caused by VM / Citrix clock drift. + * + * @returns corrected timestamp in milliseconds + */ + getCorrectedNow(): number { + return Date.now() + this.clockOffsetMs; + } + + /** + * 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. + * + * @returns Observable that completes after the offset has been stored (or silently on error) + */ + syncClockOffset(): Observable { + try { + const startTime = Date.now(); + return this.getServerTime().pipe( + map((serverTimeResponse: number) => { + const endTime = Date.now(); + const roundTripTimeInMs = endTime - startTime; + + const isServerTimeResponseInMs = serverTimeResponse.toString().length === 13; + const serverTimeInMs = isServerTimeResponseInMs ? serverTimeResponse : serverTimeResponse * 1000; + const adjustedServerTimeInMs = serverTimeInMs + roundTripTimeInMs / 2; + + this.clockOffsetMs = adjustedServerTimeInMs - Date.now(); + }), + catchError(() => of(void 0)) + ); + } catch { + return of(void 0); + } + } + checkTimeSync(maxAllowedClockSkewInSec: number): Observable { const startTime = Date.now();