From 4a5fdd5b68f676945e1e012963cafca64521058f Mon Sep 17 00:00:00 2001 From: Francesca Sargent Date: Fri, 16 Oct 2026 11:42:25 +0000 Subject: [PATCH] fix(editor): Return to the requested page after SSO sign-in (#41151) --- .../editor-ui/src/app/utils/urlUtils.test.ts | 32 ++++++++++++++++++- .../editor-ui/src/app/utils/urlUtils.ts | 26 +++++++++++++++ .../core/auth/views/SigninView.test.ts | 29 +++++++++++++++++ .../features/core/auth/views/SigninView.vue | 5 ++- .../playwright/tests/e2e/auth/oidc.spec.ts | 23 ++++++++++--- 5 files changed, 106 insertions(+), 9 deletions(-) diff --git a/packages/frontend/editor-ui/src/app/utils/urlUtils.test.ts b/packages/frontend/editor-ui/src/app/utils/urlUtils.test.ts index 6e9630cf097..33ac787b5e3 100644 --- a/packages/frontend/editor-ui/src/app/utils/urlUtils.test.ts +++ b/packages/frontend/editor-ui/src/app/utils/urlUtils.test.ts @@ -1,4 +1,4 @@ -import { getSanitizedCurrentPath } from '@/app/utils/urlUtils'; +import { getSanitizedCurrentPath, getSsoRedirectPath } from '@/app/utils/urlUtils'; describe('getSanitizedCurrentPath', () => { it('returns the path and query as-is when there is no redirect param', () => { @@ -19,3 +19,33 @@ describe('getSanitizedCurrentPath', () => { expect(getSanitizedCurrentPath({ fullPath: '/workflow/1' })).toBe('/workflow/1'); }); }); + +describe('getSsoRedirectPath', () => { + it.each([ + ['%2Fhome%2Fcredentials', '/home/credentials'], + ['%2Fworkflow%2Fabc%3Ffoo%3Da%2526b', '/workflow/abc?foo=a%26b'], + ['/home/workflows', '/home/workflows'], + ['/workflow/abc?foo=a%26b', '/workflow/abc?foo=a%26b'], + ])('returns the in-app path for %s', (redirect, expected) => { + expect(getSsoRedirectPath(redirect)).toBe(expected); + }); + + it('returns a path at the size limit', () => { + const path = `/${'a'.repeat(2047)}`; + expect(getSsoRedirectPath(path)).toBe(path); + }); + + it.each([ + ['an absolute URL', 'https://n8n.local/form/x'], + ['a protocol-relative URL', '//example.com'], + ['an encoded protocol-relative URL', '%2F%2Fexample.com'], + ['a malformed encoding', '%E0%A4%A'], + ['an empty value', ''], + ['a missing value', undefined], + ['a repeated query parameter', ['/a', '/b']], + ['a path over the size limit', `/${'a'.repeat(2048)}`], + ['a path over the size limit in UTF-8 bytes', `/${'é'.repeat(1024)}`], + ])('returns an empty string for %s', (_, redirect) => { + expect(getSsoRedirectPath(redirect)).toBe(''); + }); +}); diff --git a/packages/frontend/editor-ui/src/app/utils/urlUtils.ts b/packages/frontend/editor-ui/src/app/utils/urlUtils.ts index b2c70fae8cb..147b2d33841 100644 --- a/packages/frontend/editor-ui/src/app/utils/urlUtils.ts +++ b/packages/frontend/editor-ui/src/app/utils/urlUtils.ts @@ -4,3 +4,29 @@ export function getSanitizedCurrentPath(route: { fullPath: string }): string { url.searchParams.delete('redirect'); return `${url.pathname}${url.search}`; } + +// The backend stores the return path in the OIDC state cookie. Browsers drop a cookie that is +// larger than 4 KB, and the sign-in then fails. 2 KB leaves room for the rest of the cookie. +const MAX_SSO_REDIRECT_BYTES = 2048; + +// The route guards encode the path, and vue-router encodes the query again, so a +// `redirect` value can still be percent-encoded once. Returns '' when there is no in-app path. +export function getSsoRedirectPath(redirect: unknown): string { + if (typeof redirect !== 'string') return ''; + + let path = redirect; + // A value that starts with `/` is already decoded. A second decode would corrupt its query. + if (!path.startsWith('/')) { + try { + path = decodeURIComponent(path); + } catch { + return ''; + } + } + + // The backend validates the path again. This check only drops values that are clearly + // not in-app paths. + if (!path.startsWith('/') || path.startsWith('//')) return ''; + + return new TextEncoder().encode(path).length <= MAX_SSO_REDIRECT_BYTES ? path : ''; +} diff --git a/packages/frontend/editor-ui/src/features/core/auth/views/SigninView.test.ts b/packages/frontend/editor-ui/src/features/core/auth/views/SigninView.test.ts index a1fe093eae8..9cac31143f9 100644 --- a/packages/frontend/editor-ui/src/features/core/auth/views/SigninView.test.ts +++ b/packages/frontend/editor-ui/src/features/core/auth/views/SigninView.test.ts @@ -271,6 +271,19 @@ describe('SigninView', () => { expect(router.push).toHaveBeenCalledWith(validLocalRedirectUrl); }); + it('should decode a percent-encoded local path before redirecting', async () => { + vi.spyOn(route, 'query', 'get').mockReturnValue({ + redirect: '%2Fhome%2Fcredentials', + }); + + const hrefSpy = vi.spyOn(window.location, 'href', 'set'); + + await signInWithValidUser(); + + expect(hrefSpy).not.toHaveBeenCalled(); + expect(router.push).toHaveBeenCalledWith('/home/credentials'); + }); + it('should redirect to homepage with router if redirect url is empty', async () => { vi.spyOn(route, 'query', 'get').mockReturnValue({ redirect: '', @@ -345,6 +358,22 @@ describe('SigninView', () => { expect(hrefSpy).toHaveBeenCalledWith('https://idp.example.com/saml'); }); + it.each([ + ['%2Fhome%2Fcredentials', '/home/credentials'], + ['%2Foauth%2Fconsent', '/oauth/consent'], + ])( + 'should request the SSO login URL with the decoded destination for %s', + async (redirect, destination) => { + vi.spyOn(route, 'query', 'get').mockReturnValue({ redirect }); + ssoStore.getSsoLoginUrl.mockResolvedValue('https://idp.example.com/oidc'); + + const { getByRole } = renderComponent(); + await userEvent.click(getByRole('button', { name: 'Continue with SSO' })); + + expect(ssoStore.getSsoLoginUrl).toHaveBeenCalledWith(destination); + }, + ); + it('should show a toast when the SSO login URL cannot be resolved', async () => { ssoStore.getSsoLoginUrl.mockRejectedValue(new Error('SAML is not configured')); diff --git a/packages/frontend/editor-ui/src/features/core/auth/views/SigninView.vue b/packages/frontend/editor-ui/src/features/core/auth/views/SigninView.vue index dc221aa55b5..4e0df23df8b 100644 --- a/packages/frontend/editor-ui/src/features/core/auth/views/SigninView.vue +++ b/packages/frontend/editor-ui/src/features/core/auth/views/SigninView.vue @@ -14,6 +14,7 @@ import { useNotificationsStore } from '@n8n/stores/notifications.store'; import { useUsersStore } from '@n8n/stores/users.store'; import { useSettingsStore } from '@n8n/stores/settings.store'; import { useSSOStore } from '@/features/settings/sso/sso.store'; +import { getSsoRedirectPath } from '@/app/utils/urlUtils'; import type { IFormBoxConfig } from '@/Interface'; import { @@ -178,9 +179,7 @@ const onSsoLogin = async () => { notificationsStore.setNotificationsSuppressed(false); ssoLoading.value = true; try { - window.location.href = await ssoStore.getSsoLoginUrl( - typeof route.query?.redirect === 'string' ? route.query.redirect : '', - ); + window.location.href = await ssoStore.getSsoLoginUrl(getSsoRedirectPath(route.query?.redirect)); } catch (error) { ssoLoading.value = false; toast.showError(error, locale.baseText('auth.signin.error')); diff --git a/packages/quality/testing/playwright/tests/e2e/auth/oidc.spec.ts b/packages/quality/testing/playwright/tests/e2e/auth/oidc.spec.ts index 621d1a3d7ea..ff458889e04 100644 --- a/packages/quality/testing/playwright/tests/e2e/auth/oidc.spec.ts +++ b/packages/quality/testing/playwright/tests/e2e/auth/oidc.spec.ts @@ -11,11 +11,7 @@ test.describe( annotation: [{ type: 'owner', description: 'Identity & Access' }], }, () => { - test('should configure OIDC and login with Keycloak @auth:owner', async ({ - n8n, - api, - services, - }) => { + test.beforeEach(async ({ n8n, api, services }) => { const keycloak = services.keycloak; await api.enableFeature('oidc'); await n8n.oidcComposer.configureOidc( @@ -26,12 +22,29 @@ test.describe( await n8n.sideBar.signOutFromWorkflows(); await n8n.page.waitForURL('/signin'); + }); + test('should configure OIDC and login with Keycloak @auth:owner', async ({ n8n, services }) => { + const keycloak = services.keycloak; await n8n.signIn.getSsoButton().click(); await n8n.keycloakLogin.login(keycloak.testUser.email, keycloak.testUser.password); await expect(n8n.page).toHaveURL(/\/(workflow|home)/); await expect(n8n.sideBar.getSettings()).toBeVisible(); }); + + test('should return to the requested page after SSO login @auth:owner', async ({ + n8n, + services, + }) => { + const keycloak = services.keycloak; + await n8n.navigate.toCredentials(); + await expect(n8n.page).toHaveURL(/\/signin\?redirect=/); + + await n8n.signIn.getSsoButton().click(); + await n8n.keycloakLogin.login(keycloak.testUser.email, keycloak.testUser.password); + + await expect(n8n.page).toHaveURL(/\/home\/credentials/); + }); }, );