From 4e349fb905b3c2e9b1392af6bb70354daae8a984 Mon Sep 17 00:00:00 2001 From: alep85 Date: Fri, 24 Oct 2025 13:23:40 +0200 Subject: [PATCH] AAE-37746 revert previous changes, update the front-channel-logout component logic to always logout the user --- .../front-channel-logout.component.md | 52 ++---- lib/core/src/lib/auth/oidc/auth.service.ts | 14 -- .../front-channel-logout.component.spec.ts | 149 +----------------- .../oidc/front-channel-logout.component.ts | 30 +--- .../auth/oidc/redirect-auth.service.spec.ts | 38 ----- .../lib/auth/oidc/redirect-auth.service.ts | 10 -- 6 files changed, 13 insertions(+), 280 deletions(-) diff --git a/docs/core/components/front-channel-logout.component.md b/docs/core/components/front-channel-logout.component.md index 1e80f7f744..0ab5ceb2eb 100644 --- a/docs/core/components/front-channel-logout.component.md +++ b/docs/core/components/front-channel-logout.component.md @@ -7,7 +7,7 @@ Last reviewed: 2025-10-24 # [Front Channel Logout component](../../../lib/core/src/lib/auth/oidc/front-channel-logout.component.ts "Defined in front-channel-logout.component.ts") -Handles an OpenID Connect (OIDC) Front-Channel Logout request by validating issuer and session identifiers and triggering a local logout when they match. +Handles an OpenID Connect (OIDC) Front-Channel Logout request by always triggering a local logout when the route is hit. ## Contents @@ -32,13 +32,9 @@ export const routes: Routes = [ ]; ``` -When the IdP performs a front-channel logout it will iframe / redirect the user's browser to a URL like: +When the IdP performs a front-channel logout it will iframe or redirect the user's browser to the configured route (e.g. `/oidc/frontchannel_logout`). -```text -/oidc/frontchannel_logout?iss=https://issuer.example.com&sid=abc123-session-id -``` - -On initialisation the component compares those query parameters with locally stored values provided by `AuthService` and calls `logout()` if both match. +On initialisation the component always calls `logout()` via `AuthService`, regardless of any query parameters. ## Details @@ -46,48 +42,18 @@ On initialisation the component compares those query parameters with locally sto Front-Channel Logout is part of the OIDC specification. The Identity Provider notifies relying parties (your SPA) of a logout by issuing an HTTP(S) request (often via an iframe). The client application must validate the request and clear its own session. -### How matching works +### How it works -Inside `ngOnInit` the component: - -1. Reads `iss` and `sid` from `ActivatedRoute.snapshot.queryParamMap`. -2. Retrieves the stored issuer and session id via `AuthService.getStoredIssuer()` and `AuthService.getStoredSessionId()`. -3. Compares both pairs. Logout is executed only if: - - storedIssuer === issuerParam AND - - storedSessionId === sessionIdParam (and none are falsy). - -```ts -const storedIssuerMatches = storedIssuer && issuerParam && storedIssuer === issuerParam; -const storedSessionMatches = storedSessionId && sessionIdParam && storedSessionId === sessionIdParam; -if (storedIssuerMatches && storedSessionMatches) { - authService.logout(); -} -``` - -If either value is missing or does not match, nothing happens. +On `ngOnInit`, the component simply calls `authService.logout()`. There is no check for issuer or session ID; logout is unconditional. ### Security considerations -- The component performs strict equality checks; no partial matching. -- Both parameters must be present and match; a single match will not trigger logout. -- Avoid exposing sensitive data in query parameters beyond issuer (`iss`) and session identifier (`sid`). +- The component does not inspect or require any query parameters. +- No sensitive data is read from the URL. -### Logout scenarios +### Logout behavior -These scenarios outline when a logout is triggered or suppressed. - -Key scenarios: - -| Scenario | Stored Issuer | URL Issuer | Stored SID | URL SID | Outcome | -|----------|---------------|-----------|------------|---------|---------| -| Full match | A | A | 123 | 123 | logout called | -| Issuer mismatch | A | B | 123 | 123 | no logout | -| SID mismatch | A | A | 123 | 999 | no logout | -| Both mismatch | A | B | 123 | 999 | no logout | -| Missing issuer | null | A | 123 | 123 | no logout | -| Missing SID | A | A | null | 123 | no logout | -| Missing URL issuer | A | null | 123 | 123 | no logout | -| Missing URL SID | A | A | 123 | null | no logout | +Whenever this route is hit, the user is always logged out, regardless of any parameters or state. ### See also diff --git a/lib/core/src/lib/auth/oidc/auth.service.ts b/lib/core/src/lib/auth/oidc/auth.service.ts index 5d0aa7e2f7..63c105e549 100644 --- a/lib/core/src/lib/auth/oidc/auth.service.ts +++ b/lib/core/src/lib/auth/oidc/auth.service.ts @@ -71,18 +71,4 @@ export abstract class AuthService { */ abstract loginCallback(loginOptions?: LoginOptions): Promise; abstract updateIDPConfiguration(...args: any[]): void; - - /** - * Get the stored issuer URL. - * - * @returns stored issuer URL - */ - abstract getStoredIssuer(): string; - - /** - * Get the stored session ID. - * - * @returns stored session ID - */ - abstract getStoredSessionId(): string; } diff --git a/lib/core/src/lib/auth/oidc/front-channel-logout.component.spec.ts b/lib/core/src/lib/auth/oidc/front-channel-logout.component.spec.ts index 5b1be026dc..4eb69679b1 100644 --- a/lib/core/src/lib/auth/oidc/front-channel-logout.component.spec.ts +++ b/lib/core/src/lib/auth/oidc/front-channel-logout.component.spec.ts @@ -16,7 +16,6 @@ */ import { ComponentFixture, TestBed } from '@angular/core/testing'; -import { ActivatedRoute } from '@angular/router'; import { AuthService } from './auth.service'; import { FrontChannelLogoutComponent } from './front-channel-logout.component'; @@ -24,26 +23,13 @@ describe('FrontChannelLogoutComponent', () => { let component: FrontChannelLogoutComponent; let fixture: ComponentFixture; let authServiceSpy: jasmine.SpyObj; - let activatedRouteMock: any; beforeEach(async () => { - authServiceSpy = jasmine.createSpyObj('AuthService', ['logout', 'getStoredIssuer', 'getStoredSessionId']); - activatedRouteMock = { - snapshot: { - queryParamMap: { - get: jasmine.createSpy('get') - } - } - }; - + authServiceSpy = jasmine.createSpyObj('AuthService', ['logout']); await TestBed.configureTestingModule({ imports: [FrontChannelLogoutComponent], - providers: [ - { provide: AuthService, useValue: authServiceSpy }, - { provide: ActivatedRoute, useValue: activatedRouteMock } - ] + providers: [{ provide: AuthService, useValue: authServiceSpy }] }).compileComponents(); - fixture = TestBed.createComponent(FrontChannelLogoutComponent); component = fixture.componentInstance; }); @@ -53,138 +39,9 @@ describe('FrontChannelLogoutComponent', () => { }); describe('ngOnInit - logout logic', () => { - it('should call logout when both stored and URL issuer match AND both stored and URL session ID match', () => { - const testIssuer = 'test-issuer'; - const testSessionId = 'test-session-id'; - authServiceSpy.getStoredIssuer.and.returnValue(testIssuer); - authServiceSpy.getStoredSessionId.and.returnValue(testSessionId); - activatedRouteMock.snapshot.queryParamMap.get.and.callFake((param: string) => { - if (param === 'iss') return testIssuer; - if (param === 'sid') return testSessionId; - return null; - }); + it('should always call logout on init', () => { component.ngOnInit(); expect(authServiceSpy.logout).toHaveBeenCalledTimes(1); }); - - it('should not call logout when issuer matches but session ID differs between stored and URL values', () => { - const testIssuer = 'test-issuer'; - const storedSessionId = 'stored-session-id'; - const urlSessionId = 'different-session-id'; - authServiceSpy.getStoredIssuer.and.returnValue(testIssuer); - authServiceSpy.getStoredSessionId.and.returnValue(storedSessionId); - activatedRouteMock.snapshot.queryParamMap.get.and.callFake((param: string) => { - if (param === 'iss') return testIssuer; - if (param === 'sid') return urlSessionId; - return null; - }); - component.ngOnInit(); - expect(authServiceSpy.logout).not.toHaveBeenCalled(); - }); - - it('should not call logout when session ID matches but issuer differs between stored and URL values', () => { - const testSessionId = 'test-session-id'; - const storedIssuer = 'stored-issuer'; - const urlIssuer = 'different-issuer'; - authServiceSpy.getStoredIssuer.and.returnValue(storedIssuer); - authServiceSpy.getStoredSessionId.and.returnValue(testSessionId); - activatedRouteMock.snapshot.queryParamMap.get.and.callFake((param: string) => { - if (param === 'iss') return urlIssuer; - if (param === 'sid') return testSessionId; - return null; - }); - component.ngOnInit(); - expect(authServiceSpy.logout).not.toHaveBeenCalled(); - }); - - it('should not call logout when both issuer and session ID differ between stored and URL values', () => { - const storedIssuer = 'stored-issuer'; - const storedSessionId = 'stored-session-id'; - const urlIssuer = 'different-issuer'; - const urlSessionId = 'different-session-id'; - authServiceSpy.getStoredIssuer.and.returnValue(storedIssuer); - authServiceSpy.getStoredSessionId.and.returnValue(storedSessionId); - activatedRouteMock.snapshot.queryParamMap.get.and.callFake((param: string) => { - if (param === 'iss') return urlIssuer; - if (param === 'sid') return urlSessionId; - return null; - }); - component.ngOnInit(); - expect(authServiceSpy.logout).not.toHaveBeenCalled(); - }); - - it('should not call logout when stored issuer is null but URL parameters are valid', () => { - const testSessionId = 'test-session-id'; - authServiceSpy.getStoredIssuer.and.returnValue(null); - authServiceSpy.getStoredSessionId.and.returnValue(testSessionId); - activatedRouteMock.snapshot.queryParamMap.get.and.callFake((param: string) => { - if (param === 'iss') return 'test-issuer'; - if (param === 'sid') return testSessionId; - return null; - }); - component.ngOnInit(); - expect(authServiceSpy.logout).not.toHaveBeenCalled(); - }); - - it('should not call logout when stored session ID is null but URL parameters are valid', () => { - const testIssuer = 'test-issuer'; - authServiceSpy.getStoredIssuer.and.returnValue(testIssuer); - authServiceSpy.getStoredSessionId.and.returnValue(null); - activatedRouteMock.snapshot.queryParamMap.get.and.callFake((param: string) => { - if (param === 'iss') return testIssuer; - if (param === 'sid') return 'test-session-id'; - return null; - }); - component.ngOnInit(); - expect(authServiceSpy.logout).not.toHaveBeenCalled(); - }); - - it('should not call logout when URL issuer parameter is missing but stored values are valid', () => { - const testIssuer = 'test-issuer'; - const testSessionId = 'test-session-id'; - authServiceSpy.getStoredIssuer.and.returnValue(testIssuer); - authServiceSpy.getStoredSessionId.and.returnValue(testSessionId); - activatedRouteMock.snapshot.queryParamMap.get.and.callFake((param: string) => { - if (param === 'iss') return null; - if (param === 'sid') return testSessionId; - return null; - }); - component.ngOnInit(); - expect(authServiceSpy.logout).not.toHaveBeenCalled(); - }); - - it('should not call logout when URL session ID parameter is missing but stored values are valid', () => { - const testIssuer = 'test-issuer'; - const testSessionId = 'test-session-id'; - authServiceSpy.getStoredIssuer.and.returnValue(testIssuer); - authServiceSpy.getStoredSessionId.and.returnValue(testSessionId); - activatedRouteMock.snapshot.queryParamMap.get.and.callFake((param: string) => { - if (param === 'iss') return testIssuer; - if (param === 'sid') return null; - return null; - }); - component.ngOnInit(); - expect(authServiceSpy.logout).not.toHaveBeenCalled(); - }); - - it('should not call logout when both stored and URL values are empty strings', () => { - authServiceSpy.getStoredIssuer.and.returnValue(''); - authServiceSpy.getStoredSessionId.and.returnValue(''); - activatedRouteMock.snapshot.queryParamMap.get.and.returnValue(''); - component.ngOnInit(); - expect(authServiceSpy.logout).not.toHaveBeenCalled(); - }); - - it('should not call logout when AuthService returns undefined for stored issuer and session ID', () => { - authServiceSpy.getStoredIssuer.and.returnValue(undefined); - authServiceSpy.getStoredSessionId.and.returnValue(undefined); - activatedRouteMock.snapshot.queryParamMap.get.and.callFake((param: string) => { - if (param === 'iss') return 'test-issuer'; - if (param === 'sid') return 'test-session-id'; - return null; - }); - expect(() => component.ngOnInit()).not.toThrow(); - expect(authServiceSpy.logout).not.toHaveBeenCalled(); - }); }); }); diff --git a/lib/core/src/lib/auth/oidc/front-channel-logout.component.ts b/lib/core/src/lib/auth/oidc/front-channel-logout.component.ts index 7d0029dbae..df30a4c071 100644 --- a/lib/core/src/lib/auth/oidc/front-channel-logout.component.ts +++ b/lib/core/src/lib/auth/oidc/front-channel-logout.component.ts @@ -16,41 +16,13 @@ */ import { Component, inject, OnInit } from '@angular/core'; -import { ActivatedRoute } from '@angular/router'; import { AuthService } from './auth.service'; @Component({ template: '', standalone: true }) export class FrontChannelLogoutComponent implements OnInit { - private readonly activatedRoute = inject(ActivatedRoute); private readonly authService = inject(AuthService); ngOnInit() { - const { issuerParam, sessionIdParam } = this.getIssuerAndSessionIdFromRouteParams(); - - const { storedIssuer, storedSessionId } = this.getIssuerAndSessionIdFromAuthService(); - - this.logoutIfIssuerAndSessionIdMatch(storedIssuer, issuerParam, storedSessionId, sessionIdParam); - } - - private logoutIfIssuerAndSessionIdMatch(storedIssuer: string, issuerParam: string, storedSessionId: string, sessionIdParam: string) { - const storedIssuerMatchUrlIssuerParam = storedIssuer && issuerParam && storedIssuer === issuerParam; - const storedSessionIdMatchUrlSessionIdParam = storedSessionId && sessionIdParam && storedSessionId === sessionIdParam; - - if (storedIssuerMatchUrlIssuerParam && storedSessionIdMatchUrlSessionIdParam) { - this.authService.logout(); - } - } - - private getIssuerAndSessionIdFromAuthService() { - const storedIssuer = this.authService.getStoredIssuer(); - const storedSessionId = this.authService.getStoredSessionId(); - return { storedIssuer, storedSessionId }; - } - - private getIssuerAndSessionIdFromRouteParams() { - const queryParamMap = this.activatedRoute.snapshot.queryParamMap; - const issuerParam = queryParamMap.get('iss'); - const sessionIdParam = queryParamMap.get('sid'); - return { issuerParam, sessionIdParam }; + this.authService.logout(); } } 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 40c948ef15..bc16799499 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 @@ -537,42 +537,4 @@ describe('RedirectAuthService', () => { expect(expectedLogoutIsEmitted).toBeTrue(); }); - - describe('getStoredIssuer', () => { - it('should return the stored issuer from the OAuthStorage', () => { - const expectedIssuer = 'https://example.com/auth'; - oauthServiceSpy.getIdentityClaims.and.returnValue({ iss: expectedIssuer }); - - const storedIssuer = service.getStoredIssuer(); - - expect(storedIssuer).toBe(expectedIssuer); - }); - - it('should return empty string if no issuer is stored in the OAuthStorage', () => { - oauthServiceSpy.getIdentityClaims.and.returnValue({ iss: null }); - - const storedIssuer = service.getStoredIssuer(); - - expect(storedIssuer).toBe(''); - }); - }); - - describe('getStoredSessionId', () => { - it('should return the stored session id from the OAuthStorage', () => { - const expectedSessionId = '12345678910'; - oauthServiceSpy.getIdentityClaims.and.returnValue({ sid: expectedSessionId }); - - const storedSessionId = service.getStoredSessionId(); - - expect(storedSessionId).toBe(expectedSessionId); - }); - - it('should return string if no session id is stored in the OAuthStorage', () => { - oauthServiceSpy.getIdentityClaims.and.returnValue({ sid: null }); - - const storedSessionId = service.getStoredSessionId(); - - expect(storedSessionId).toBe(''); - }); - }); }); 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 271d2abdee..1f65ef0003 100644 --- a/lib/core/src/lib/auth/oidc/redirect-auth.service.ts +++ b/lib/core/src/lib/auth/oidc/redirect-auth.service.ts @@ -318,16 +318,6 @@ export class RedirectAuthService extends AuthService { .then(() => this._getRedirectUrl()); } - getStoredIssuer(): string { - const claims = this.oauthService.getIdentityClaims(); - return claims?.['iss'] || ''; - } - - getStoredSessionId(): string { - const claims = this.oauthService.getIdentityClaims(); - return claims?.['sid'] || ''; - } - private _getRedirectUrl() { const DEFAULT_REDIRECT = '/'; const stateKey = this.oauthService.state;