From 543770d0fde76814f1660a37c4e76f3d1d0501bf Mon Sep 17 00:00:00 2001 From: Diogo Bastos Date: Mon, 6 Jul 2026 16:33:54 +0100 Subject: [PATCH] Add Flag --- .../src/lib/app-config/app-config.service.ts | 1 + .../auth/services/time-sync.service.spec.ts | 77 +++++++++++++++++++ .../lib/auth/services/time-sync.service.ts | 53 ++++++++++++- 3 files changed, 130 insertions(+), 1 deletion(-) diff --git a/lib/core/src/lib/app-config/app-config.service.ts b/lib/core/src/lib/app-config/app-config.service.ts index 4be1c40ae6..60c37d8868 100644 --- a/lib/core/src/lib/app-config/app-config.service.ts +++ b/lib/core/src/lib/app-config/app-config.service.ts @@ -44,6 +44,7 @@ export const AppConfigValues = { LOGIN_ROUTE: 'loginRoute', DISABLECSRF: 'disableCSRF', AUTH_WITH_CREDENTIALS: 'auth.withCredentials', + AUTH_TIME_SYNC_ENABLED: 'auth.timeSync.enabled', APPLICATION: 'application', STORAGE_PREFIX: 'application.storagePrefix', NOTIFY_DURATION: 'notificationDefaultDuration', 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 e350ced10f..f4dc432e61 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 @@ -20,6 +20,7 @@ import { HttpTestingController, provideHttpClientTesting } from '@angular/common import { TestBed } from '@angular/core/testing'; import { TimeSyncService } from './time-sync.service'; import { LogService } from '../../common/services/log.service'; +import { AppConfigService } from '../../app-config/app-config.service'; import { firstValueFrom } from 'rxjs'; // A fixed reference instant (Wed, 15 Jan 2025 12:00:00 GMT), aligned to a whole second. @@ -31,6 +32,7 @@ const toHttpDate = (epochMs: number): string => new Date(epochMs).toUTCString(); describe('TimeSyncService', () => { let service: TimeSyncService; let httpMock: HttpTestingController; + let appConfigGetSpy: jasmine.Spy; beforeEach(() => { TestBed.configureTestingModule({ @@ -39,6 +41,12 @@ describe('TimeSyncService', () => { service = TestBed.inject(TimeSyncService); httpMock = TestBed.inject(HttpTestingController); + + // Enable clock-skew correction for the behavioural suites so they exercise the new + // functionality (the behaviour that ships with this feature). The production default is + // opt-in (disabled); the 'auth.timeSync.enabled configuration' block below overrides this + // spy to assert both the disabled-by-default path and the old raw-clock behaviour. + appConfigGetSpy = spyOn(TestBed.inject(AppConfigService), 'get').and.returnValue(true); }); afterEach(() => { @@ -358,6 +366,24 @@ describe('TimeSyncService', () => { expect(service.clockOffsetMs).toBe(4000); }); + + it('should emit implausibleOffsetDetected$ with the measured offset and bound when rejected', async () => { + const events: { measuredOffsetMs: number; maxAllowedOffsetMs: number }[] = []; + service.implausibleOffsetDetected$.subscribe((event) => events.push(event)); + + await syncWithDrift(BASE, BASE + 660_000); + + expect(events).toEqual([{ measuredOffsetMs: 660_000, maxAllowedOffsetMs: 600_000 }]); + }); + + it('should not emit implausibleOffsetDetected$ when the offset is within the bound', async () => { + const events: { measuredOffsetMs: number; maxAllowedOffsetMs: number }[] = []; + service.implausibleOffsetDetected$.subscribe((event) => events.push(event)); + + await syncWithDrift(BASE, BASE + 60_000); + + expect(events).toEqual([]); + }); }); describe('time zone independence', () => { @@ -382,6 +408,57 @@ describe('TimeSyncService', () => { }); }); + describe('legacy raw-clock behaviour when disabled (auth.timeSync.enabled = false)', () => { + it('should be disabled by default (opt-in) and behave like the raw local clock', async () => { + // Fall back to the real AppConfigService: the flag is absent, so it returns the + // opt-in default (false) — i.e. the old behaviour from before clock-skew correction. + appConfigGetSpy.and.callThrough(); + service.clockOffsetMs = 60_000; // a stored offset that must be ignored while disabled + + spyOn(Date, 'now').and.returnValue(BASE); + expect(service.getCorrectedNow()).toBe(BASE); + + await firstValueFrom(service.syncClockOffset()); + httpMock.expectNone(() => true); + }); + + it('should return the raw local time from getCorrectedNow when disabled', () => { + appConfigGetSpy.and.returnValue(false); + service.clockOffsetMs = 60_000; // a stored offset that must be ignored while disabled + + spyOn(Date, 'now').and.returnValue(BASE); + + expect(service.getCorrectedNow()).toBe(BASE); + }); + + it('should not issue any HTTP request from syncClockOffset when disabled', async () => { + appConfigGetSpy.and.returnValue(false); + + await firstValueFrom(service.syncClockOffset()); + + httpMock.expectNone(() => true); + expect(service.clockOffsetMs).toBe(0); + }); + + it('should not register a periodic sync or visibility listener when disabled', () => { + appConfigGetSpy.and.returnValue(false); + const addEventListenerSpy = spyOn(document, 'addEventListener'); + + service.startPeriodicSync(1000); + + expect(addEventListenerSpy).not.toHaveBeenCalled(); + httpMock.expectNone(() => true); + }); + + it('should restore the new clock-skew correction when explicitly enabled', async () => { + appConfigGetSpy.and.returnValue(true); // explicit opt-in + + await syncWithDrift(BASE, BASE + 60_000); + + expect(service.clockOffsetMs).toBe(60_000); + }); + }); + describe('observability', () => { let warnSpy: jasmine.Spy; let debugSpy: jasmine.Spy; 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 5d15076fe7..58be1d940d 100644 --- a/lib/core/src/lib/auth/services/time-sync.service.ts +++ b/lib/core/src/lib/auth/services/time-sync.service.ts @@ -17,9 +17,10 @@ import { HttpClient } from '@angular/common/http'; import { Injectable, Injector, NgZone, inject } from '@angular/core'; -import { interval, Observable, of, Subscription } from 'rxjs'; +import { interval, Observable, of, Subject, Subscription } from 'rxjs'; import { catchError, map, switchMap, timeout } from 'rxjs/operators'; import { LogService } from '../../common/services/log.service'; +import { AppConfigService, AppConfigValues } from '../../app-config/app-config.service'; export interface TimeSync { outOfSync: boolean; @@ -28,6 +29,16 @@ export interface TimeSync { serverDateTimeISO: string; } +/** + * Emitted when a measured clock offset is rejected for exceeding `maxAllowedOffsetMs`. + * Consumers can subscribe to `implausibleOffsetDetected$` and forward this to central + * telemetry to detect misconfigured proxies or `Date`-header tampering across the fleet. + */ +export interface ImplausibleClockOffsetEvent { + measuredOffsetMs: number; + maxAllowedOffsetMs: number; +} + /** Default interval for periodic clock re-sync (5 minutes). */ const DEFAULT_PERIODIC_SYNC_INTERVAL_MS = 5 * 60 * 1000; @@ -51,6 +62,7 @@ export class TimeSyncService { private readonly _injector = inject(Injector); private readonly _ngZone = inject(NgZone); private readonly _logService = inject(LogService); + private readonly _appConfig = inject(AppConfigService); private readonly _http: HttpClient; @@ -70,6 +82,15 @@ export class TimeSyncService { */ maxAllowedOffsetMs = DEFAULT_MAX_ALLOWED_OFFSET_MS; + private readonly _implausibleOffsetDetected = new Subject(); + + /** + * Emits whenever a measured offset is rejected for exceeding `maxAllowedOffsetMs`. + * Surface this to monitoring/telemetry to detect potential `Date`-header tampering or a + * misconfigured time source; the client console alone is not a reliable security signal. + */ + readonly implausibleOffsetDetected$ = this._implausibleOffsetDetected.asObservable(); + private _periodicSyncSubscription: Subscription | null = null; private _visibilityChangeHandler: (() => void) | null = null; private _lastSyncAtMs = 0; @@ -83,9 +104,15 @@ export class TimeSyncService { * Use this instead of `Date.now()` when evaluating token expiration to avoid * false positives caused by VM / Citrix clock drift. * + * When the feature is disabled via AppConfig, this returns the raw local time so the + * consuming application behaves exactly as it did before clock-skew correction existed. + * * @returns corrected timestamp in milliseconds */ getCorrectedNow(): number { + if (!this.isEnabled()) { + return Date.now(); + } return Date.now() + this.clockOffsetMs; } @@ -105,6 +132,10 @@ export class TimeSyncService { * @returns Observable that completes after the offset has been stored (or silently on error) */ syncClockOffset(): Observable { + if (!this.isEnabled()) { + return of(void 0); + } + const appRootUrl = this.getAppRootUrl(); try { @@ -149,6 +180,10 @@ export class TimeSyncService { `TimeSyncService: ignoring implausible clock offset of ${Math.round(newOffset)} ms ` + `(exceeds the maximum allowed ${this.maxAllowedOffsetMs} ms). Falling back to the local clock.` ); + this._implausibleOffsetDetected.next({ + measuredOffsetMs: Math.round(newOffset), + maxAllowedOffsetMs: this.maxAllowedOffsetMs + }); return; } @@ -206,6 +241,10 @@ export class TimeSyncService { * @param intervalMs How often to re-sync in milliseconds (default: 5 minutes) */ startPeriodicSync(intervalMs: number = DEFAULT_PERIODIC_SYNC_INTERVAL_MS): void { + if (!this.isEnabled()) { + return; + } + this.stopPeriodicSync(); this._ngZone.runOutsideAngular(() => { @@ -259,6 +298,18 @@ export class TimeSyncService { return '/'; } + /** + * Whether clock-skew correction is enabled. Controlled by the `auth.timeSync.enabled` + * AppConfig flag so a consuming application can turn the feature on without code changes. + * The feature is opt-in: it defaults to `false` when the flag is absent, so an application + * behaves exactly as it did before clock-skew correction existed until it explicitly enables it. + * + * @returns true when the feature is enabled + */ + private isEnabled(): boolean { + return this._appConfig.get(AppConfigValues.AUTH_TIME_SYNC_ENABLED, false); + } + /** * Sanitizes an untrusted, in-path-controllable value (e.g. the server `Date` header) before * it is written to a log. Strips control characters (including CR/LF) to prevent log forging