fix(editor): Return to the requested page after SSO sign-in (#41151)

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('');
+	});
+});

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 : '';
+}

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'));
 

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'));

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/);
+		});
 	},
 );