mirror of
https://github.com/Alfresco/alfresco-ng2-components.git
synced 2026-09-09 18:03:21 +00:00
feat(auth): add periodic clock re-sync to protect against mid-session drift in Citrix/VM environments
- Add startPeriodicSync() and stopPeriodicSync() to TimeSyncService - Re-sync clock offset every 5 minutes and on document visibility change - Add security cap (maxAllowedOffsetMs) to reject unreasonably large offsets - Preserve existing offset on sync failures instead of resetting to 0 - Wire periodic sync start in RedirectAuthService.configureAuth() - Wire periodic sync stop in RedirectAuthService.logout() - Add unit tests for periodic sync and offset cap behavior
This commit is contained in:
@@ -52,7 +52,13 @@ describe('RedirectAuthService', () => {
|
|||||||
|
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
retryLoginServiceSpy = jasmine.createSpyObj('RetryLoginService', ['tryToLoginTimes']);
|
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']);
|
oauthLoggerSpy = jasmine.createSpyObj('OAuthLogger', ['error', 'info', 'warn']);
|
||||||
oauthServiceSpy = jasmine.createSpyObj(
|
oauthServiceSpy = jasmine.createSpyObj(
|
||||||
'OAuthService',
|
'OAuthService',
|
||||||
|
|||||||
@@ -261,6 +261,7 @@ export class RedirectAuthService extends AuthService {
|
|||||||
}
|
}
|
||||||
|
|
||||||
logout() {
|
logout() {
|
||||||
|
this._timeSyncService.stopPeriodicSync();
|
||||||
this.oauthService.logOut();
|
this.oauthService.logOut();
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -351,6 +352,7 @@ export class RedirectAuthService extends AuthService {
|
|||||||
this._isDiscoveryDocumentLoadedSubject$.next(true);
|
this._isDiscoveryDocumentLoadedSubject$.next(true);
|
||||||
this.oauthService.setupAutomaticSilentRefresh();
|
this.oauthService.setupAutomaticSilentRefresh();
|
||||||
this._timeSyncService.syncClockOffset().subscribe();
|
this._timeSyncService.syncClockOffset().subscribe();
|
||||||
|
this._timeSyncService.startPeriodicSync(undefined, this.oauthService.clockSkewInSec * 1000);
|
||||||
return void this.allowRefreshTokenAndSilentRefreshOnMultipleTabs();
|
return void this.allowRefreshTokenAndSilentRefreshOnMultipleTabs();
|
||||||
})
|
})
|
||||||
.catch(() => {
|
.catch(() => {
|
||||||
|
|||||||
@@ -39,6 +39,7 @@ describe('TimeSyncService', () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
afterEach(() => {
|
afterEach(() => {
|
||||||
|
service.stopPeriodicSync();
|
||||||
httpMock.verify();
|
httpMock.verify();
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -235,16 +236,62 @@ describe('TimeSyncService', () => {
|
|||||||
httpMock.expectNone('http://fake-server-time-url');
|
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');
|
appConfigSpy.get.and.returnValue('http://fake-server-time-url');
|
||||||
|
service.clockOffsetMs = 5000;
|
||||||
|
|
||||||
service.syncClockOffset().subscribe(() => {
|
service.syncClockOffset().subscribe(() => {
|
||||||
expect(service.clockOffsetMs).toBe(0);
|
expect(service.clockOffsetMs).toBe(5000);
|
||||||
});
|
});
|
||||||
|
|
||||||
const req = httpMock.expectOne('http://fake-server-time-url');
|
const req = httpMock.expectOne('http://fake-server-time-url');
|
||||||
req.error(new ProgressEvent(''));
|
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', () => {
|
describe('getCorrectedNow', () => {
|
||||||
@@ -273,4 +320,63 @@ describe('TimeSyncService', () => {
|
|||||||
expect(service.getCorrectedNow()).toBe(fixedNow - 60000);
|
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');
|
||||||
|
});
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -16,10 +16,10 @@
|
|||||||
*/
|
*/
|
||||||
|
|
||||||
import { HttpClient } from '@angular/common/http';
|
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 { AppConfigService } from '../../app-config/app-config.service';
|
||||||
import { from, Observable, of, throwError } from 'rxjs';
|
import { from, interval, Observable, of, Subscription, throwError } from 'rxjs';
|
||||||
import { catchError, map, timeout } from 'rxjs/operators';
|
import { catchError, map, switchMap, timeout } from 'rxjs/operators';
|
||||||
|
|
||||||
export interface TimeSync {
|
export interface TimeSync {
|
||||||
outOfSync: boolean;
|
outOfSync: boolean;
|
||||||
@@ -28,12 +28,16 @@ export interface TimeSync {
|
|||||||
serverDateTimeISO: string;
|
serverDateTimeISO: string;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** Default interval for periodic clock re-sync (5 minutes). */
|
||||||
|
const DEFAULT_PERIODIC_SYNC_INTERVAL_MS = 5 * 60 * 1000;
|
||||||
|
|
||||||
@Injectable({
|
@Injectable({
|
||||||
providedIn: 'root'
|
providedIn: 'root'
|
||||||
})
|
})
|
||||||
export class TimeSyncService {
|
export class TimeSyncService {
|
||||||
private readonly _injector = inject(Injector);
|
private readonly _injector = inject(Injector);
|
||||||
private readonly _appConfigService = inject(AppConfigService);
|
private readonly _appConfigService = inject(AppConfigService);
|
||||||
|
private readonly _ngZone = inject(NgZone);
|
||||||
|
|
||||||
private readonly _http: HttpClient;
|
private readonly _http: HttpClient;
|
||||||
|
|
||||||
@@ -44,6 +48,9 @@ export class TimeSyncService {
|
|||||||
*/
|
*/
|
||||||
clockOffsetMs = 0;
|
clockOffsetMs = 0;
|
||||||
|
|
||||||
|
private _periodicSyncSubscription: Subscription | null = null;
|
||||||
|
private _visibilityChangeHandler: (() => void) | null = null;
|
||||||
|
|
||||||
constructor() {
|
constructor() {
|
||||||
this._http = this._injector.get(HttpClient);
|
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`.
|
* 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
|
* Call this at application start-up (fire-and-forget) so subsequent calls to
|
||||||
* `getCorrectedNow` compensate for any VM / Citrix clock drift.
|
* `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)
|
* @returns Observable that completes after the offset has been stored (or silently on error)
|
||||||
*/
|
*/
|
||||||
syncClockOffset(): Observable<void> {
|
syncClockOffset(maxAllowedOffsetMs?: number): Observable<void> {
|
||||||
try {
|
try {
|
||||||
const startTime = Date.now();
|
const startTime = Date.now();
|
||||||
return this.getServerTime().pipe(
|
return this.getServerTime().pipe(
|
||||||
@@ -79,15 +90,17 @@ export class TimeSyncService {
|
|||||||
const serverTimeInMs = isServerTimeResponseInMs ? serverTimeResponse : serverTimeResponse * 1000;
|
const serverTimeInMs = isServerTimeResponseInMs ? serverTimeResponse : serverTimeResponse * 1000;
|
||||||
const adjustedServerTimeInMs = serverTimeInMs + roundTripTimeInMs / 2;
|
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(() => {
|
catchError(() => of(void 0))
|
||||||
this.clockOffsetMs = 0;
|
|
||||||
return of(void 0);
|
|
||||||
})
|
|
||||||
);
|
);
|
||||||
} catch {
|
} catch {
|
||||||
this.clockOffsetMs = 0;
|
|
||||||
return of(void 0);
|
return of(void 0);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -135,6 +148,50 @@ export class TimeSyncService {
|
|||||||
return this.checkTimeSync(maxAllowedClockSkewInSec).pipe(map((sync) => sync.outOfSync));
|
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<number> {
|
private getServerTime(): Observable<number> {
|
||||||
return from(this._http.get<number>(this.getServerTimeUrl())).pipe(
|
return from(this._http.get<number>(this.getServerTimeUrl())).pipe(
|
||||||
timeout(5000),
|
timeout(5000),
|
||||||
|
|||||||
Reference in New Issue
Block a user