From e26c6f39a081d067bfbb554abef12bdae59591bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire?= Date: Tue, 28 Jul 2026 14:25:18 +0200 Subject: [PATCH] feat(editor): Add OIDC logout support (#32756) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Grégoire Bellon-Gervais --- .../__tests__/oidc.controller.ee.test.ts | 176 +++++++++++++++++- .../__tests__/oidc.service.ee.test.ts | 6 +- .../cli/src/modules/sso-oidc/constants.ts | 14 ++ .../modules/sso-oidc/oidc.controller.ee.ts | 74 +++++++- .../src/modules/sso-oidc/oidc.service.ee.ts | 65 ++++++- .../integration/oidc/oidc.service.ee.test.ts | 20 +- .../@n8n/rest-api-client/src/api/sso.ts | 13 ++ .../@n8n/stores/src/users.store.test.ts | 75 +++++++- .../frontend/@n8n/stores/src/users.store.ts | 20 +- .../core/auth/views/SignoutView.test.ts | 106 +++++++++++ .../features/core/auth/views/SignoutView.vue | 15 +- 11 files changed, 555 insertions(+), 29 deletions(-) create mode 100644 packages/frontend/editor-ui/src/features/core/auth/views/SignoutView.test.ts diff --git a/packages/cli/src/modules/sso-oidc/__tests__/oidc.controller.ee.test.ts b/packages/cli/src/modules/sso-oidc/__tests__/oidc.controller.ee.test.ts index 7c94a339acd..2dabd66bd64 100644 --- a/packages/cli/src/modules/sso-oidc/__tests__/oidc.controller.ee.test.ts +++ b/packages/cli/src/modules/sso-oidc/__tests__/oidc.controller.ee.test.ts @@ -11,14 +11,23 @@ import type { EventService } from '@/events/event.service'; import type { AuthlessRequest } from '@/requests'; import type { UrlService } from '@/services/url.service'; +import { isOidcCurrentAuthenticationMethod } from '@/sso.ee/sso-helpers'; + +import { OIDC_ID_TOKEN_COOKIE_NAME } from '../constants'; import { OidcController } from '../oidc.controller.ee'; import type { OidcService } from '../oidc.service.ee'; -const authService = mock(); +vi.mock('@/sso.ee/sso-helpers', () => ({ + isOidcCurrentAuthenticationMethod: vi.fn().mockReturnValue(true), +})); + +const authService = mock({ jwtExpiration: 604800 }); const eventService = mock(); const oidcService = mock(); const urlService = mock(); -const globalConfig = mock(); +const globalConfig = mock({ + auth: { cookie: { samesite: 'lax', secure: true } }, +}); const logger = mock(); const instanceSettingsLoaderConfig = mock({ ssoManagedByEnv: false, @@ -73,7 +82,7 @@ describe('OidcController', () => { ); // Mock successful OIDC login - oidcService.loginUser.mockResolvedValueOnce(user); + oidcService.loginUser.mockResolvedValueOnce({ user }); await controller.callbackHandler(req, res); @@ -111,7 +120,7 @@ describe('OidcController', () => { 'http://localhost:5678/sso/oidc/callback?code=different_code&state=different_state&session_state=session123', ); - oidcService.loginUser.mockResolvedValueOnce(user); + oidcService.loginUser.mockResolvedValueOnce({ user }); await controller.callbackHandler(req, res); @@ -137,7 +146,7 @@ describe('OidcController', () => { const expectedCallbackUrl = new URL('http://localhost:5678/sso/oidc/callback'); - oidcService.loginUser.mockResolvedValueOnce(user); + oidcService.loginUser.mockResolvedValueOnce({ user }); await controller.callbackHandler(req, res); @@ -170,6 +179,62 @@ describe('OidcController', () => { expect(res.redirect).not.toHaveBeenCalled(); }); + test('Should store the encrypted ID token in a cookie for RP-initiated logout', async () => { + const req = mock({ + originalUrl: '/sso/oidc/callback?code=auth_code&state=state_value', + browserId: 'browser-id-123', + cookies: { + [OIDC_STATE_COOKIE_NAME]: 'state_value', + [OIDC_NONCE_COOKIE_NAME]: 'nonce_value', + }, + }); + const res = mock(); + + oidcService.loginUser.mockResolvedValueOnce({ user, idToken: 'raw-id-token' }); + oidcService.encryptIdToken.mockResolvedValueOnce('encrypted-id-token'); + + await controller.callbackHandler(req, res); + + expect(oidcService.encryptIdToken).toHaveBeenCalledWith('raw-id-token'); + expect(res.cookie).toHaveBeenCalledWith( + OIDC_ID_TOKEN_COOKIE_NAME, + 'encrypted-id-token', + expect.objectContaining({ + maxAge: 604800 * Time.seconds.toMilliseconds, + httpOnly: true, + sameSite: 'lax', + secure: true, + }), + ); + }); + + test('Should not store an oversized ID token in a cookie', async () => { + const req = mock({ + originalUrl: '/sso/oidc/callback?code=auth_code&state=state_value', + browserId: 'browser-id-123', + cookies: { + [OIDC_STATE_COOKIE_NAME]: 'state_value', + [OIDC_NONCE_COOKIE_NAME]: 'nonce_value', + }, + }); + const res = mock(); + + oidcService.loginUser.mockResolvedValueOnce({ user, idToken: 'raw-id-token' }); + oidcService.encryptIdToken.mockResolvedValueOnce('x'.repeat(5000)); + + await controller.callbackHandler(req, res); + + expect(res.cookie).not.toHaveBeenCalledWith( + OIDC_ID_TOKEN_COOKIE_NAME, + expect.any(String), + expect.any(Object), + ); + expect(logger.warn).toHaveBeenCalled(); + // Login itself must still succeed + expect(authService.issueCookie).toHaveBeenCalledWith(res, user, true, req.browserId); + expect(res.redirect).toHaveBeenCalledWith('/'); + }); + test('Should render success page in test mode without creating session', async () => { const req = mock({ originalUrl: '/sso/oidc/callback?code=auth_code&state=state_value', @@ -341,4 +406,105 @@ describe('OidcController', () => { expect(res.redirect).not.toHaveBeenCalled(); }); }); + + describe('logout', () => { + const makeLogoutReq = (cookies: Record) => + mock({ cookies }); + + beforeEach(() => { + vi.mocked(isOidcCurrentAuthenticationMethod).mockReturnValue(true); + }); + + test('Should always invalidate the n8n session and clear the auth and ID token cookies', async () => { + const req = makeLogoutReq({ [OIDC_ID_TOKEN_COOKIE_NAME]: 'encrypted-id-token' }); + const res = mock(); + oidcService.decryptIdToken.mockResolvedValueOnce('raw-id-token'); + oidcService.generateEndSessionUrl.mockResolvedValueOnce( + new URL('https://idp.example.com/logout?id_token_hint=raw-id-token'), + ); + + await controller.logout(req, res); + + expect(authService.invalidateToken).toHaveBeenCalledWith(req); + expect(authService.clearCookie).toHaveBeenCalledWith(res); + expect(res.clearCookie).toHaveBeenCalledWith(OIDC_ID_TOKEN_COOKIE_NAME); + }); + + test('Should return the RP-initiated logout URL for an OIDC-established session', async () => { + const req = makeLogoutReq({ [OIDC_ID_TOKEN_COOKIE_NAME]: 'encrypted-id-token' }); + const res = mock(); + oidcService.decryptIdToken.mockResolvedValueOnce('raw-id-token'); + oidcService.generateEndSessionUrl.mockResolvedValueOnce( + new URL('https://idp.example.com/logout?id_token_hint=raw-id-token'), + ); + + const result = await controller.logout(req, res); + + expect(oidcService.decryptIdToken).toHaveBeenCalledWith('encrypted-id-token'); + expect(oidcService.generateEndSessionUrl).toHaveBeenCalledWith('raw-id-token'); + expect(result).toEqual({ + redirectUrl: 'https://idp.example.com/logout?id_token_hint=raw-id-token', + }); + }); + + test('Should return a null redirect URL when the session was not established through OIDC', async () => { + const req = makeLogoutReq({}); + const res = mock(); + + const result = await controller.logout(req, res); + + expect(result).toEqual({ redirectUrl: null }); + expect(oidcService.generateEndSessionUrl).not.toHaveBeenCalled(); + // The n8n session is still terminated + expect(authService.invalidateToken).toHaveBeenCalledWith(req); + expect(authService.clearCookie).toHaveBeenCalledWith(res); + }); + + test('Should return a null redirect URL when OIDC is no longer the authentication method', async () => { + vi.mocked(isOidcCurrentAuthenticationMethod).mockReturnValue(false); + const req = makeLogoutReq({ [OIDC_ID_TOKEN_COOKIE_NAME]: 'encrypted-id-token' }); + const res = mock(); + + const result = await controller.logout(req, res); + + expect(result).toEqual({ redirectUrl: null }); + expect(oidcService.generateEndSessionUrl).not.toHaveBeenCalled(); + }); + + test('Should return a null redirect URL when the ID token cannot be decrypted', async () => { + const req = makeLogoutReq({ [OIDC_ID_TOKEN_COOKIE_NAME]: 'tampered' }); + const res = mock(); + oidcService.decryptIdToken.mockResolvedValueOnce(undefined); + + const result = await controller.logout(req, res); + + expect(result).toEqual({ redirectUrl: null }); + expect(oidcService.generateEndSessionUrl).not.toHaveBeenCalled(); + }); + + test('Should return a null redirect URL when the provider has no end_session_endpoint', async () => { + const req = makeLogoutReq({ [OIDC_ID_TOKEN_COOKIE_NAME]: 'encrypted-id-token' }); + const res = mock(); + oidcService.decryptIdToken.mockResolvedValueOnce('raw-id-token'); + oidcService.generateEndSessionUrl.mockResolvedValueOnce(undefined); + + const result = await controller.logout(req, res); + + expect(result).toEqual({ redirectUrl: null }); + }); + + test('Should not fail the sign-out when building the logout URL throws', async () => { + const req = makeLogoutReq({ [OIDC_ID_TOKEN_COOKIE_NAME]: 'encrypted-id-token' }); + const res = mock(); + oidcService.decryptIdToken.mockResolvedValueOnce('raw-id-token'); + oidcService.generateEndSessionUrl.mockRejectedValueOnce(new Error('discovery unavailable')); + + const result = await controller.logout(req, res); + + expect(result).toEqual({ redirectUrl: null }); + expect(authService.invalidateToken).toHaveBeenCalledWith(req); + expect(authService.clearCookie).toHaveBeenCalledWith(res); + expect(logger.warn).toHaveBeenCalled(); + }); + }); }); diff --git a/packages/cli/src/modules/sso-oidc/__tests__/oidc.service.ee.test.ts b/packages/cli/src/modules/sso-oidc/__tests__/oidc.service.ee.test.ts index f62eca4ccff..4d3c9e63bcf 100644 --- a/packages/cli/src/modules/sso-oidc/__tests__/oidc.service.ee.test.ts +++ b/packages/cli/src/modules/sso-oidc/__tests__/oidc.service.ee.test.ts @@ -557,7 +557,7 @@ describe('OidcService', () => { const storedState = oidcService.generateState().signed; const storedNonce = oidcService.generateNonce().signed; - const user = await oidcService.loginUser(callbackUrl, storedState, storedNonce); + const { user } = await oidcService.loginUser(callbackUrl, storedState, storedNonce); expect(user).toBeDefined(); expect(user.email).toEqual('john.doe@test.com'); // @ts-expect-error - applySsoProvisioning is private and only accessible within class 'OidcService' @@ -595,7 +595,7 @@ describe('OidcService', () => { const storedState = oidcService.generateState().signed; const storedNonce = oidcService.generateNonce().signed; - const user = await oidcService.loginUser(callbackUrl, storedState, storedNonce); + const { user } = await oidcService.loginUser(callbackUrl, storedState, storedNonce); expect(user).toBeDefined(); expect(user.email).toEqual('john.doe@test.com'); // @ts-expect-error - applySsoProvisioning is private and only accessible within class 'OidcService' @@ -635,7 +635,7 @@ describe('OidcService', () => { const storedState = oidcService.generateState().signed; const storedNonce = oidcService.generateNonce().signed; - const user = await oidcService.loginUser(callbackUrl, storedState, storedNonce); + const { user } = await oidcService.loginUser(callbackUrl, storedState, storedNonce); expect(user).toBeDefined(); expect(user.email).toEqual('john.doe@test.com'); }); diff --git a/packages/cli/src/modules/sso-oidc/constants.ts b/packages/cli/src/modules/sso-oidc/constants.ts index 44d07a2d243..1069cc621b1 100644 --- a/packages/cli/src/modules/sso-oidc/constants.ts +++ b/packages/cli/src/modules/sso-oidc/constants.ts @@ -2,3 +2,17 @@ export const OIDC_PREFERENCES_DB_KEY = 'features.oidc'; export const OIDC_LOGIN_ENABLED = 'sso.oidc.loginEnabled'; export const OIDC_CLIENT_SECRET_REDACTED_VALUE = '__n8n_CLIENT_SECRET_VALUE_e5362baf-c777-4d57-a609-6eaf1f9e87f6'; + +/** + * Cookie holding the encrypted OIDC ID token of the current session. Its + * presence marks the session as OIDC-established and provides the + * `id_token_hint` required for OIDC RP-Initiated Logout. + */ +export const OIDC_ID_TOKEN_COOKIE_NAME = 'n8n-oidc-id-token'; + +/** + * Browsers reject cookies above ~4096 bytes (name + value + attributes), + * so leave a safety margin. Oversized ID tokens simply skip the cookie and + * sign-out degrades to a local (n8n-only) logout. + */ +export const OIDC_ID_TOKEN_COOKIE_MAX_BYTES = 3800; diff --git a/packages/cli/src/modules/sso-oidc/oidc.controller.ee.ts b/packages/cli/src/modules/sso-oidc/oidc.controller.ee.ts index 45c25b90319..3720969827e 100644 --- a/packages/cli/src/modules/sso-oidc/oidc.controller.ee.ts +++ b/packages/cli/src/modules/sso-oidc/oidc.controller.ee.ts @@ -13,8 +13,13 @@ import { ForbiddenError } from '@/errors/response-errors/forbidden.error'; import { EventService } from '@/events/event.service'; import { AuthlessRequest } from '@/requests'; import { UrlService } from '@/services/url.service'; +import { isOidcCurrentAuthenticationMethod } from '@/sso.ee/sso-helpers'; -import { OIDC_CLIENT_SECRET_REDACTED_VALUE } from './constants'; +import { + OIDC_CLIENT_SECRET_REDACTED_VALUE, + OIDC_ID_TOKEN_COOKIE_MAX_BYTES, + OIDC_ID_TOKEN_COOKIE_NAME, +} from './constants'; import { OidcService } from './oidc.service.ee'; import { renderOidcTestFailure, renderOidcTestSuccess } from './views/oidc-test-result'; @@ -136,9 +141,30 @@ export class OidcController { } } - const user = await this.oidcService.loginUser(callbackUrl, state, nonce); + const { user, idToken } = await this.oidcService.loginUser(callbackUrl, state, nonce); this.authService.issueCookie(res, user, true, req.browserId); + + // Persist the encrypted ID token so a later sign-out can perform OIDC + // RP-Initiated Logout with the required `id_token_hint`. The cookie's + // presence also marks this session as OIDC-established, as opposed to + // e.g. an email session of the instance owner. + if (idToken) { + const encryptedIdToken = await this.oidcService.encryptIdToken(idToken); + if (Buffer.byteLength(encryptedIdToken, 'utf8') <= OIDC_ID_TOKEN_COOKIE_MAX_BYTES) { + const { samesite, secure } = this.globalConfig.auth.cookie; + res.cookie(OIDC_ID_TOKEN_COOKIE_NAME, encryptedIdToken, { + maxAge: this.authService.jwtExpiration * Time.seconds.toMilliseconds, + httpOnly: true, + sameSite: samesite, + secure, + }); + } else { + this.logger.warn( + 'The OIDC ID token is too large to be stored in a cookie. Signing out will terminate the n8n session but not the OIDC provider session. Consider reducing the claims included in the ID token.', + ); + } + } this.eventService.emit('user-logged-in', { user, authenticationMethod: 'oidc', @@ -146,4 +172,48 @@ export class OidcController { return res.redirect('/'); } + + /** + * Signs the user out of n8n and, when the session was established through + * OIDC, returns the provider's RP-Initiated Logout URL (built from the + * discovered `end_session_endpoint`, including the `id_token_hint`) for + * the client to redirect to. The n8n session is always invalidated first, + * so whatever happens at the provider afterwards cannot leave a valid n8n + * session behind. + */ + @Post('/logout') + @Licensed('feat:oidc') + async logout(req: AuthenticatedRequest, res: Response) { + await this.authService.invalidateToken(req); + this.authService.clearCookie(res); + + const encryptedIdToken: unknown = req.cookies[OIDC_ID_TOKEN_COOKIE_NAME]; + res.clearCookie(OIDC_ID_TOKEN_COOKIE_NAME); + + // Only sessions established through OIDC carry the ID token cookie: + // email or LDAP sessions must never trigger a logout at the provider. + if ( + typeof encryptedIdToken !== 'string' || + encryptedIdToken === '' || + !isOidcCurrentAuthenticationMethod() + ) { + return { redirectUrl: null }; + } + + const idToken = await this.oidcService.decryptIdToken(encryptedIdToken); + if (!idToken) { + return { redirectUrl: null }; + } + + try { + const endSessionUrl = await this.oidcService.generateEndSessionUrl(idToken); + return { redirectUrl: endSessionUrl?.toString() ?? null }; + } catch (error) { + // The n8n session is already terminated at this point; a failure to + // reach the provider (e.g. discovery endpoint unavailable) must not + // fail the sign-out itself. + this.logger.warn('Failed to build the OIDC RP-initiated logout URL', { error }); + return { redirectUrl: null }; + } + } } diff --git a/packages/cli/src/modules/sso-oidc/oidc.service.ee.ts b/packages/cli/src/modules/sso-oidc/oidc.service.ee.ts index 3e468073077..999527a9752 100644 --- a/packages/cli/src/modules/sso-oidc/oidc.service.ee.ts +++ b/packages/cli/src/modules/sso-oidc/oidc.service.ee.ts @@ -254,7 +254,16 @@ export class OidcService { return { url: authorizationURL, state: state.signed, nonce: nonce.signed }; } - async loginUser(callbackUrl: URL, storedState: string, storedNonce: string): Promise { + /** + * Completes the authorization code flow and resolves the n8n user. Also + * returns the raw ID token so the controller can persist it for OIDC + * RP-Initiated Logout (`id_token_hint`). + */ + async loginUser( + callbackUrl: URL, + storedState: string, + storedNonce: string, + ): Promise<{ user: User; idToken?: string }> { await this.loadOpenIdClient(); const configuration = await this.getOidcConfiguration(); @@ -325,7 +334,7 @@ export class OidcService { userInfo as Record, ); - return openidUser.user; + return { user: openidUser.user, idToken: tokens.id_token }; } const foundUser = await this.userRepository.findOne({ @@ -351,7 +360,7 @@ export class OidcService { userInfo as Record, ); - return foundUser; + return { user: foundUser, idToken: tokens.id_token }; } const user = await this.userRepository.manager.transaction(async (trx) => { @@ -384,7 +393,55 @@ export class OidcService { userInfo as Record, ); - return user; + return { user, idToken: tokens.id_token }; + } + + /** + * Encrypts the OIDC ID token with the instance encryption key so it can + * be stored in an httpOnly cookie without exposing its claims. + */ + async encryptIdToken(idToken: string): Promise { + return await this.cipher.encryptV2(idToken); + } + + /** + * Decrypts a previously stored OIDC ID token. Returns `undefined` when + * the value cannot be decrypted (e.g. tampered cookie or rotated + * encryption key), in which case sign-out degrades to a local logout. + */ + async decryptIdToken(encryptedIdToken: string): Promise { + try { + const idToken = await this.cipher.decryptV2(encryptedIdToken); + return idToken === '' ? undefined : idToken; + } catch (error) { + this.logger.warn('Failed to decrypt the stored OIDC ID token', { + cause: safeStringify(error), + }); + return undefined; + } + } + + /** + * Builds the OIDC RP-Initiated Logout URL from the provider's discovered + * metadata, including the `id_token_hint` required by the specification. + * Returns `undefined` when the provider does not advertise an + * `end_session_endpoint`, in which case sign-out is local to n8n only. + */ + async generateEndSessionUrl(idToken: string): Promise { + await this.loadOpenIdClient(); + const configuration = await this.getOidcConfiguration(); + + if (!configuration.serverMetadata().end_session_endpoint) { + this.logger.debug( + 'The OIDC provider does not advertise an end_session_endpoint, skipping RP-initiated logout', + ); + return undefined; + } + + return this.openidClient.buildEndSessionUrl(configuration, { + id_token_hint: idToken, + post_logout_redirect_uri: `${this.urlService.getInstanceBaseUrl()}/signin`, + }); } async generateTestLoginUrl(): Promise<{ url: URL; state: string; nonce: string }> { diff --git a/packages/cli/test/integration/oidc/oidc.service.ee.test.ts b/packages/cli/test/integration/oidc/oidc.service.ee.test.ts index 06c9d045484..03f13ddd377 100644 --- a/packages/cli/test/integration/oidc/oidc.service.ee.test.ts +++ b/packages/cli/test/integration/oidc/oidc.service.ee.test.ts @@ -618,7 +618,7 @@ describe('OIDC service', () => { email: 'user2@example.com', }); - const user = await oidcService.loginUser(callbackUrl, state.signed, nonce.signed); + const { user } = await oidcService.loginUser(callbackUrl, state.signed, nonce.signed); expect(user).toBeDefined(); expect(user.email).toEqual('user2@example.com'); @@ -664,7 +664,7 @@ describe('OIDC service', () => { email: 'user2@example.com', }); - const user = await oidcService.loginUser(callbackUrl, state.signed, nonce.signed); + const { user } = await oidcService.loginUser(callbackUrl, state.signed, nonce.signed); expect(user).toBeDefined(); expect(user.email).toEqual('user2@example.com'); expect(user.id).toEqual(createdUser.id); @@ -703,7 +703,7 @@ describe('OIDC service', () => { email: 'user1@example.com', }); - const user = await oidcService.loginUser(callbackUrl, state.signed, nonce.signed); + const { user } = await oidcService.loginUser(callbackUrl, state.signed, nonce.signed); expect(user).toBeDefined(); expect(user.email).toEqual('user1@example.com'); }); @@ -741,7 +741,7 @@ describe('OIDC service', () => { email: 'user3@example.com', }); - const user = await oidcService.loginUser(callbackUrl, state.signed, nonce.signed); + const { user } = await oidcService.loginUser(callbackUrl, state.signed, nonce.signed); expect(user).toBeDefined(); expect(user.email).toEqual('user3@example.com'); }); @@ -1021,7 +1021,7 @@ describe('OIDC service', () => { email: 'new-instance-role-user@example.com', }); - const user = await oidcService.loginUser(callbackUrl, state.signed, nonce.signed); + const { user } = await oidcService.loginUser(callbackUrl, state.signed, nonce.signed); expect(user).toBeDefined(); expect(user.email).toEqual('new-instance-role-user@example.com'); @@ -1059,7 +1059,7 @@ describe('OIDC service', () => { email: 'new-project-role-user@example.com', }); - const user = await oidcService.loginUser(callbackUrl, state.signed, nonce.signed); + const { user } = await oidcService.loginUser(callbackUrl, state.signed, nonce.signed); expect(user).toBeDefined(); expect(user.email).toEqual('new-project-role-user@example.com'); @@ -1096,7 +1096,7 @@ describe('OIDC service', () => { email: 'new-both-provisioning-user@example.com', }); - const user = await oidcService.loginUser(callbackUrl, state.signed, nonce.signed); + const { user } = await oidcService.loginUser(callbackUrl, state.signed, nonce.signed); expect(user).toBeDefined(); expect(user.email).toEqual('new-both-provisioning-user@example.com'); @@ -1156,7 +1156,7 @@ describe('OIDC service', () => { email: 'oidc-expr-instance-role@example.com', }); - const user = await oidcService.loginUser(callbackUrl, state.signed, nonce.signed); + const { user } = await oidcService.loginUser(callbackUrl, state.signed, nonce.signed); expect(user).toBeDefined(); const userFromDB = await userRepository.findOne({ @@ -1209,7 +1209,7 @@ describe('OIDC service', () => { email: 'oidc-expr-custom-role@example.com', }); - const user = await oidcService.loginUser(callbackUrl, state.signed, nonce.signed); + const { user } = await oidcService.loginUser(callbackUrl, state.signed, nonce.signed); expect(user).toBeDefined(); const userFromDB = await userRepository.findOne({ @@ -1256,7 +1256,7 @@ describe('OIDC service', () => { email: 'oidc-expr-project-role@example.com', }); - const user = await oidcService.loginUser(callbackUrl, state.signed, nonce.signed); + const { user } = await oidcService.loginUser(callbackUrl, state.signed, nonce.signed); expect(user).toBeDefined(); const projectRole = await getProjectRoleForUser(project.id, user.id); diff --git a/packages/frontend/@n8n/rest-api-client/src/api/sso.ts b/packages/frontend/@n8n/rest-api-client/src/api/sso.ts index e727b158dbc..897ba5ab78e 100644 --- a/packages/frontend/@n8n/rest-api-client/src/api/sso.ts +++ b/packages/frontend/@n8n/rest-api-client/src/api/sso.ts @@ -68,3 +68,16 @@ export const testOidcConfig = async ( export const initOidcLogin = async (context: IRestApiContext): Promise => { return await makeRestApiRequest(context, 'GET', '/sso/oidc/login'); }; + +export type OidcLogoutResponse = { + /** + * OIDC RP-Initiated Logout URL to redirect the browser to, or `null` when + * the session was not established through OIDC or the provider does not + * support RP-initiated logout. The n8n session is terminated either way. + */ + redirectUrl: string | null; +}; + +export const oidcLogout = async (context: IRestApiContext): Promise => { + return await makeRestApiRequest(context, 'POST', '/sso/oidc/logout'); +}; diff --git a/packages/frontend/@n8n/stores/src/users.store.test.ts b/packages/frontend/@n8n/stores/src/users.store.test.ts index 32746b62484..191be267fb6 100644 --- a/packages/frontend/@n8n/stores/src/users.store.test.ts +++ b/packages/frontend/@n8n/stores/src/users.store.test.ts @@ -5,7 +5,7 @@ import { createPinia, setActivePinia } from 'pinia'; import { useSettingsStore } from './settings.store'; import { useUsersStore } from './users.store'; -const { loginCurrentUser, inviteUsers, login, logout, getUsers } = vi.hoisted(() => { +const { loginCurrentUser, inviteUsers, login, logout, getUsers, oidcLogout } = vi.hoisted(() => { return { loginCurrentUser: vi.fn(), identify: vi.fn(), @@ -13,6 +13,7 @@ const { loginCurrentUser, inviteUsers, login, logout, getUsers } = vi.hoisted(() login: vi.fn(), logout: vi.fn(), getUsers: vi.fn(), + oidcLogout: vi.fn(), }; }); @@ -23,6 +24,10 @@ vi.mock('@n8n/rest-api-client/api/users', () => ({ getUsers, })); +vi.mock('@n8n/rest-api-client/api/sso', () => ({ + oidcLogout, +})); + vi.mock('./invitation.api', () => ({ inviteUsers, })); @@ -43,6 +48,10 @@ const mockUser: CurrentUserResponse = { describe('users.store', () => { beforeEach(() => { vi.restoreAllMocks(); + // `restoreMocks`/`vi.restoreAllMocks()` only restore spies created via + // `vi.spyOn()`; the plain `vi.fn()` mocks created via `vi.hoisted` keep + // their call history. Clear it explicitly so counts don't leak between tests. + vi.clearAllMocks(); setActivePinia(createPinia()); }); @@ -263,6 +272,70 @@ describe('users.store', () => { }); }); + describe('logout', () => { + it('should call the standard logout API by default and return no redirect URL', async () => { + const usersStore = useUsersStore(); + + const result = await usersStore.logout(); + + expect(logout).toHaveBeenCalledTimes(1); + expect(oidcLogout).not.toHaveBeenCalled(); + expect(result).toEqual({ redirectUrl: null }); + }); + + it('should call the OIDC logout API and return its redirect URL when signing out via OIDC', async () => { + const usersStore = useUsersStore(); + const redirectUrl = 'https://idp.example.com/logout?id_token_hint=abc'; + oidcLogout.mockResolvedValueOnce({ redirectUrl }); + + const result = await usersStore.logout({ viaOidc: true }); + + expect(oidcLogout).toHaveBeenCalledTimes(1); + expect(logout).not.toHaveBeenCalled(); + expect(result).toEqual({ redirectUrl }); + }); + + it('should return a null redirect URL when the session was not established through OIDC', async () => { + const usersStore = useUsersStore(); + oidcLogout.mockResolvedValueOnce({ redirectUrl: null }); + + const result = await usersStore.logout({ viaOidc: true }); + + expect(result).toEqual({ redirectUrl: null }); + }); + + it('should fall back to the standard logout API when the OIDC logout API fails', async () => { + const usersStore = useUsersStore(); + oidcLogout.mockRejectedValueOnce(new Error('license expired')); + + const result = await usersStore.logout({ viaOidc: true }); + + expect(oidcLogout).toHaveBeenCalledTimes(1); + expect(logout).toHaveBeenCalledTimes(1); + expect(result).toEqual({ redirectUrl: null }); + }); + + it('should clear the current user and still run logout hooks when signing out via OIDC', async () => { + const usersStore = useUsersStore(); + usersStore.usersById['1'] = { + ...mockUser, + isDefaultUser: false, + isPendingUser: false, + mfaEnabled: false, + }; + usersStore.currentUserId = '1'; + oidcLogout.mockResolvedValueOnce({ redirectUrl: null }); + + const hook = vi.fn(); + usersStore.registerLogoutHook(hook); + + await usersStore.logout({ viaOidc: true }); + + expect(usersStore.currentUser).toBeNull(); + expect(hook).toHaveBeenCalled(); + }); + }); + describe('logoutHooks', () => { it('should run all registered logoutHooks', async () => { const usersStore = useUsersStore(); diff --git a/packages/frontend/@n8n/stores/src/users.store.ts b/packages/frontend/@n8n/stores/src/users.store.ts index df17f191a8a..098c3213e84 100644 --- a/packages/frontend/@n8n/stores/src/users.store.ts +++ b/packages/frontend/@n8n/stores/src/users.store.ts @@ -12,6 +12,7 @@ import { BROWSER_ID_STORAGE_KEY } from '@n8n/constants'; import type { AssignableGlobalRole } from '@n8n/permissions'; import * as cloudApi from '@n8n/rest-api-client/api/cloudPlans'; import * as mfaApi from '@n8n/rest-api-client/api/mfa'; +import * as ssoApi from '@n8n/rest-api-client/api/sso'; import type { UpdateGlobalRolePayload, IUserResponse, @@ -245,8 +246,21 @@ export const useUsersStore = defineStore(STORES.USERS, () => { logoutHooks.value.push(hook); }; - const logout = async () => { - await usersApi.logout(rootStore.restApiContext); + const logout = async (options?: { viaOidc?: boolean }) => { + let redirectUrl: string | null = null; + + if (options?.viaOidc) { + try { + ({ redirectUrl } = await ssoApi.oidcLogout(rootStore.restApiContext)); + } catch { + // The OIDC logout endpoint may be unavailable (e.g. the license + // lapsed since login). Fall back to the standard logout so the + // n8n session is terminated in any case. + await usersApi.logout(rootStore.restApiContext); + } + } else { + await usersApi.logout(rootStore.restApiContext); + } unsetCurrentUser(); @@ -259,6 +273,8 @@ export const useUsersStore = defineStore(STORES.USERS, () => { } localStorage.removeItem(BROWSER_ID_STORAGE_KEY); + + return { redirectUrl }; }; const createOwner = async (params: { diff --git a/packages/frontend/editor-ui/src/features/core/auth/views/SignoutView.test.ts b/packages/frontend/editor-ui/src/features/core/auth/views/SignoutView.test.ts new file mode 100644 index 00000000000..c2acdabb9ae --- /dev/null +++ b/packages/frontend/editor-ui/src/features/core/auth/views/SignoutView.test.ts @@ -0,0 +1,106 @@ +import { createComponentRenderer } from '@/__tests__/render'; +import { mockedStore } from '@/__tests__/utils'; +import { createTestingPinia } from '@pinia/testing'; +import { flushPromises } from '@vue/test-utils'; +import { useRouter } from 'vue-router'; +import SignoutView from './SignoutView.vue'; +import { useUsersStore } from '@/features/settings/users/users.store'; +import { useSSOStore } from '@/features/settings/sso/sso.store'; + +const SIGNIN_HREF = '/signin'; + +vi.mock('vue-router', () => { + const resolve = vi.fn(() => ({ href: SIGNIN_HREF })); + return { + useRouter: () => ({ resolve }), + useRoute: vi.fn(), + RouterLink: { + template: '', + }, + }; +}); + +const { showError } = vi.hoisted(() => ({ showError: vi.fn() })); + +vi.mock('@/app/composables/useToast', () => ({ + useToast: () => ({ showError }), +})); + +const renderComponent = createComponentRenderer(SignoutView); + +let usersStore: ReturnType>; +let ssoStore: ReturnType>; +let router: ReturnType; + +describe('SignoutView', () => { + beforeEach(() => { + createTestingPinia(); + usersStore = mockedStore(useUsersStore); + ssoStore = mockedStore(useSSOStore); + router = useRouter(); + + usersStore.logout.mockResolvedValue({ redirectUrl: null }); + + Object.defineProperty(window, 'location', { + value: { href: '' }, + writable: true, + }); + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + it('should not throw error when opened', () => { + expect(() => renderComponent()).not.toThrow(); + }); + + it('should perform a standard logout and redirect to signin when OIDC is not the active authentication method', async () => { + ssoStore.isDefaultAuthenticationOidc = false; + const hrefSpy = vi.spyOn(window.location, 'href', 'set'); + + renderComponent(); + await flushPromises(); + + expect(usersStore.logout).toHaveBeenCalledWith({ viaOidc: false }); + expect(router.resolve).toHaveBeenCalled(); + expect(hrefSpy).toHaveBeenCalledWith(SIGNIN_HREF); + }); + + it('should sign out via OIDC and redirect to the RP-initiated logout URL when the backend returns one', async () => { + const redirectUrl = 'https://idp.example.com/logout?id_token_hint=abc'; + ssoStore.isDefaultAuthenticationOidc = true; + usersStore.logout.mockResolvedValue({ redirectUrl }); + const hrefSpy = vi.spyOn(window.location, 'href', 'set'); + + renderComponent(); + await flushPromises(); + + expect(usersStore.logout).toHaveBeenCalledWith({ viaOidc: true }); + expect(hrefSpy).toHaveBeenCalledWith(redirectUrl); + // Should redirect to the IdP, not resolve the local signin route. + expect(router.resolve).not.toHaveBeenCalled(); + }); + + it('should redirect to signin when signing out via OIDC but the session was not OIDC-established', async () => { + ssoStore.isDefaultAuthenticationOidc = true; + usersStore.logout.mockResolvedValue({ redirectUrl: null }); + const hrefSpy = vi.spyOn(window.location, 'href', 'set'); + + renderComponent(); + await flushPromises(); + + expect(usersStore.logout).toHaveBeenCalledWith({ viaOidc: true }); + expect(hrefSpy).toHaveBeenCalledWith(SIGNIN_HREF); + }); + + it('should show an error toast when logout fails', async () => { + const error = new Error('logout failed'); + usersStore.logout.mockRejectedValueOnce(error); + + renderComponent(); + await flushPromises(); + + expect(showError).toHaveBeenCalledWith(error, expect.any(String)); + }); +}); diff --git a/packages/frontend/editor-ui/src/features/core/auth/views/SignoutView.vue b/packages/frontend/editor-ui/src/features/core/auth/views/SignoutView.vue index 44115a55aaa..b23aa55899c 100644 --- a/packages/frontend/editor-ui/src/features/core/auth/views/SignoutView.vue +++ b/packages/frontend/editor-ui/src/features/core/auth/views/SignoutView.vue @@ -1,20 +1,31 @@