diff --git a/src/app/auth.service.spec.ts b/src/app/auth.service.spec.ts index 764ea7d3e..f9621c2f1 100644 --- a/src/app/auth.service.spec.ts +++ b/src/app/auth.service.spec.ts @@ -13,8 +13,21 @@ describe('AuthService', () => { let routerSpy: jasmine.SpyObj const mockEnvironment = { mpsServer: 'https://test-mps', rpsServer: 'https://test-rps' } + const createJwtWithExp = (expSeconds: number): string => { + const header = btoa(JSON.stringify({ alg: 'HS256', typ: 'JWT' })) + .replace(/\+/g, '-') + .replace(/\//g, '_') + .replace(/=+$/g, '') + const payload = btoa(JSON.stringify({ exp: expSeconds })) + .replace(/\+/g, '-') + .replace(/\//g, '_') + .replace(/=+$/g, '') + return `${header}.${payload}.signature` + } beforeEach(() => { + localStorage.clear() + routerSpy = jasmine.createSpyObj('Router', ['navigate']) environment.mpsServer = mockEnvironment.mpsServer environment.rpsServer = mockEnvironment.rpsServer @@ -35,6 +48,7 @@ describe('AuthService', () => { afterEach(() => { httpMock.verify() + localStorage.clear() }) it('should be created', () => { @@ -200,4 +214,138 @@ describe('AuthService', () => { expect(service.compareSemver('1.0.0', '1.0.0')).toBe(0) }) }) + + describe('constructor token validation', () => { + it('should accept a non-expired token from localStorage', () => { + const validJwt = createJwtWithExp(Math.floor(Date.now() / 1000) + 300) + localStorage.setItem('loggedInUser', JSON.stringify({ token: validJwt })) + + TestBed.resetTestingModule() + TestBed.configureTestingModule({ + providers: [ + provideTranslateService(), + AuthService, + { provide: Router, useValue: routerSpy }, + provideHttpClient(), + provideHttpClientTesting() + ] + }) + + const newService = TestBed.inject(AuthService) + const newHttpMock = TestBed.inject(HttpTestingController) + + // Should not make any HTTP calls during construction + newHttpMock.expectNone(`${mockEnvironment.mpsServer}/api/v1/devices/stats`) + + expect(newService.isLoggedIn).toBeTrue() + expect(localStorage.getItem('loggedInUser')).not.toBeNull() + newHttpMock.verify() + }) + + it('should accept a recently expired token within the clock-skew tolerance', () => { + // Token expired 3 minutes ago (within 5-minute tolerance) + const toleratedJwt = createJwtWithExp(Math.floor(Date.now() / 1000) - 3 * 60) + localStorage.setItem('loggedInUser', JSON.stringify({ token: toleratedJwt })) + + TestBed.resetTestingModule() + TestBed.configureTestingModule({ + providers: [ + provideTranslateService(), + AuthService, + { provide: Router, useValue: routerSpy }, + provideHttpClient(), + provideHttpClientTesting() + ] + }) + + const newService = TestBed.inject(AuthService) + const newHttpMock = TestBed.inject(HttpTestingController) + + // Should not make a server call for tokens still valid within skew tolerance + newHttpMock.expectNone(`${mockEnvironment.mpsServer}/api/v1/devices/stats`) + + expect(newService.isLoggedIn).toBeTrue() + expect(localStorage.getItem('loggedInUser')).not.toBeNull() + newHttpMock.verify() + }) + + it('should clear expired token from localStorage', () => { + // Token expired 10 minutes ago (beyond 5-minute tolerance) + const expiredJwt = createJwtWithExp(Math.floor(Date.now() / 1000) - 10 * 60) + localStorage.setItem('loggedInUser', JSON.stringify({ token: expiredJwt })) + + TestBed.resetTestingModule() + TestBed.configureTestingModule({ + providers: [ + provideTranslateService(), + AuthService, + { provide: Router, useValue: routerSpy }, + provideHttpClient(), + provideHttpClientTesting() + ] + }) + + const newService = TestBed.inject(AuthService) + const newHttpMock = TestBed.inject(HttpTestingController) + + // Should not make a server call for expired tokens + newHttpMock.expectNone(`${mockEnvironment.mpsServer}/api/v1/devices/stats`) + + expect(newService.isLoggedIn).toBeFalse() + expect(localStorage.getItem('loggedInUser')).toBeNull() + newHttpMock.verify() + }) + + it('should clear malformed token from localStorage', () => { + localStorage.setItem('loggedInUser', JSON.stringify({ token: 'bad.jwt' })) + + TestBed.resetTestingModule() + TestBed.configureTestingModule({ + providers: [ + provideTranslateService(), + AuthService, + { provide: Router, useValue: routerSpy }, + provideHttpClient(), + provideHttpClientTesting() + ] + }) + + const newService = TestBed.inject(AuthService) + const newHttpMock = TestBed.inject(HttpTestingController) + + // Should not make a server call for malformed tokens + newHttpMock.expectNone(`${mockEnvironment.mpsServer}/api/v1/devices/stats`) + + expect(newService.isLoggedIn).toBeFalse() + expect(localStorage.getItem('loggedInUser')).toBeNull() + newHttpMock.verify() + }) + + it('should handle corrupted localStorage JSON gracefully', () => { + // Simulate corrupted localStorage (invalid JSON) + localStorage.setItem('loggedInUser', 'not-valid-json{]') + + TestBed.resetTestingModule() + TestBed.configureTestingModule({ + providers: [ + provideTranslateService(), + AuthService, + { provide: Router, useValue: routerSpy }, + provideHttpClient(), + provideHttpClientTesting() + ] + }) + + const newService = TestBed.inject(AuthService) + const newHttpMock = TestBed.inject(HttpTestingController) + + // Should not crash app startup + newHttpMock.expectNone(`${mockEnvironment.mpsServer}/api/v1/devices/stats`) + + // Should clear corrupted data and not be logged in + expect(newService.isLoggedIn).toBeFalse() + expect(localStorage.getItem('loggedInUser')).toBeNull() + newHttpMock.verify() + }) + }) }) diff --git a/src/app/auth.service.ts b/src/app/auth.service.ts index 2432cbe46..6d0ceba18 100644 --- a/src/app/auth.service.ts +++ b/src/app/auth.service.ts @@ -16,14 +16,23 @@ import { OAuthService } from 'angular-oauth2-oidc' providedIn: 'root' }) export class AuthService { + // Clock skew tolerance for distributed systems + // Allows 5 minutes for clock differences between client, edge nodes, and backend + // This handles timezone/NTP drift without allowing truly expired tokens through + private static readonly clockSkewToleranceMs = 5 * 60 * 1000 private readonly http = inject(HttpClient) private oauthService router = inject(Router) loggedInSubject$: BehaviorSubject = new BehaviorSubject(false) + private authStateInitialized$: BehaviorSubject = new BehaviorSubject(false) public canActivateProtectedRoutes$: Observable = combineLatest([ - this.loggedInSubject$ - ]).pipe(map((values) => values.every((b) => b))) + this.loggedInSubject$, + this.authStateInitialized$ + ]).pipe( + filter(([, initialized]) => initialized), + map(([isLoggedIn]) => isLoggedIn) + ) isLoggedIn = false url = `${environment.mpsServer}/api/v1/authorize` @@ -32,10 +41,15 @@ export class AuthService { if (environment.useOAuth) { this.oauthService = inject(OAuthService) } - if (localStorage.loggedInUser != null) { - this.isLoggedIn = true - this.loggedInSubject$.next(this.isLoggedIn) + // Only restore from localStorage for JWT-based auth, not OAuth + if (!environment.useOAuth) { + if (localStorage.getItem('loggedInUser') != null) { + this.restoreSessionFromStorage() + } else { + this.authStateInitialized$.next(true) + } } + // OAuth initialization happens below after access token validity is checked if (environment.mpsServer.includes('/mps')) { // handles kong route this.url = `${environment.mpsServer}/login/api/v1/authorize` @@ -46,6 +60,7 @@ export class AuthService { }) this.loggedInSubject$.next(this.oauthService.hasValidAccessToken()) + this.authStateInitialized$.next(true) this.oauthService.events .pipe(filter((e) => ['session_terminated', 'session_error'].includes(e.type))) @@ -108,12 +123,83 @@ export class AuthService { getLoggedUserToken(): string { const loggedInUser: string = localStorage.getItem('loggedInUser') ?? '' if (loggedInUser !== '') { - const token: string = JSON.parse(loggedInUser).token - return token + try { + const token: string = JSON.parse(loggedInUser).token + return token + } catch { + // Corrupted localStorage - clear it to allow app recovery + localStorage.removeItem('loggedInUser') + return '' + } } return '' } + private restoreSessionFromStorage(): void { + const token = this.getLoggedUserToken() + if (!token) { + this.clearSessionAndMarkInitialized() + return + } + + // Validate token client-side + if (!this.isTokenValidClientSide(token)) { + this.clearSessionAndMarkInitialized() + return + } + + // Token is valid client-side, allow login but mark for validation + // The first API call will verify if the token is actually valid server-side + // If it gets a 401, the error interceptor will handle logout + this.isLoggedIn = true + this.loggedInSubject$.next(true) + this.authStateInitialized$.next(true) + } + + private isTokenValidClientSide(token: string): boolean { + try { + const payloadPart = token.split('.')[1] + if (!payloadPart) { + return false + } + + const payload = JSON.parse(this.decodeBase64Url(payloadPart)) as { exp?: number } + if (typeof payload.exp !== 'number') { + return false + } + + // Check if token is expired, with tolerance for clock skew + // In distributed systems, client/edge/backend clocks may differ + // Allow tokens that expired within the last 5 minutes (clock skew tolerance) + // but reject anything older than that + const expirationMs = payload.exp * 1000 + const now = Date.now() + + // Token is valid if: expiration time + tolerance > current time + // This means tokens expired >5 minutes ago are rejected + return expirationMs + AuthService.clockSkewToleranceMs > now + } catch { + return false + } + } + + private clearSession(): void { + localStorage.removeItem('loggedInUser') + this.isLoggedIn = false + this.loggedInSubject$.next(false) + } + + private clearSessionAndMarkInitialized(): void { + this.clearSession() + this.authStateInitialized$.next(true) + } + + private decodeBase64Url(value: string): string { + const base64 = value.replace(/-/g, '+').replace(/_/g, '/') + const padded = base64.padEnd(base64.length + ((4 - (base64.length % 4)) % 4), '=') + return atob(padded) + } + login(username: string, password: string): Observable { return this.http.post(this.url, { username, password }).pipe( map((data: any) => { @@ -121,6 +207,7 @@ export class AuthService { this.isLoggedIn = true localStorage.loggedInUser = JSON.stringify(data) this.loggedInSubject$.next(this.isLoggedIn) + this.authStateInitialized$.next(true) } return data }), @@ -133,6 +220,7 @@ export class AuthService { logout(): void { this.isLoggedIn = false this.loggedInSubject$.next(this.isLoggedIn) + this.authStateInitialized$.next(true) localStorage.removeItem('loggedInUser') if (environment.useOAuth) { this.oauthService?.logOut() diff --git a/src/app/authorize.interceptor.spec.ts b/src/app/authorize.interceptor.spec.ts index 822db1e43..0e17cf843 100644 --- a/src/app/authorize.interceptor.spec.ts +++ b/src/app/authorize.interceptor.spec.ts @@ -49,12 +49,21 @@ describe('AuthorizeInterceptor', () => { }) it('should not add Authorization header for /authorize endpoint', () => { - httpClient.get('/authorize').subscribe() + httpClient.get('/api/v1/authorize').subscribe() - const req = httpTestingController.expectOne('/authorize') + const req = httpTestingController.expectOne('/api/v1/authorize') expect(req.request.headers.has('Authorization')).toBeFalse() }) + it('should add Authorization header for /authorize/validate endpoint', () => { + authServiceSpy.getLoggedUserToken.and.returnValue('test-token') + + httpClient.get('/api/v1/authorize/validate').subscribe() + + const req = httpTestingController.expectOne('/api/v1/authorize/validate') + expect(req.request.headers.get('Authorization')).toBe('Bearer test-token') + }) + it('should add if-match header if body contains version', () => { authServiceSpy.getLoggedUserToken.and.returnValue('test-token') diff --git a/src/app/authorize.interceptor.ts b/src/app/authorize.interceptor.ts index 1e486d4bc..46abb5a0d 100644 --- a/src/app/authorize.interceptor.ts +++ b/src/app/authorize.interceptor.ts @@ -9,9 +9,11 @@ import { AuthService } from './auth.service' export const authorizationInterceptor: HttpInterceptorFn = (request, next) => { const authService = inject(AuthService) + const url = request.url.toString() + const isLoginEndpoint = /\/(login\/)?api\/v1\/authorize$/.test(url) - if (request.url.toString().includes('/authorize') && !request.url.toString().includes('/authorize/redirection')) { - // Skip adding authorization headers for specific routes + if (isLoginEndpoint) { + // Login endpoint should not include bearer token. return next(request) }