fix: compensate for VM/Citrix clock drift in token expiry check

This commit is contained in:
copilot-swe-agent[bot]
2026-07-01 20:07:19 +00:00
committed by GitHub
parent 699f45e486
commit 2fa592eccb
4 changed files with 158 additions and 8 deletions
@@ -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'));
@@ -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;
@@ -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);
});
});
});
@@ -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<void> {
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<TimeSync> {
const startTime = Date.now();