fix(editor): Stop nefarious redirects during sign in (#16034)

packages/frontend/editor-ui/src/views/SigninView.test.ts

@@ -2,11 +2,12 @@ import { createComponentRenderer } from '@/__tests__/render';
 import { mockedStore } from '@/__tests__/utils';
 import { createTestingPinia } from '@pinia/testing';
 import userEvent from '@testing-library/user-event';
-import { useRouter } from 'vue-router';
+import { useRouter, useRoute } from 'vue-router';
 import SigninView from '@/views/SigninView.vue';
 import { useUsersStore } from '@/stores/users.store';
 import { useSettingsStore } from '@/stores/settings.store';
 import { useTelemetry } from '@/composables/useTelemetry';
+import { VIEWS } from '@/constants';
 
 vi.mock('vue-router', () => {
 	const push = vi.fn();
@@ -14,7 +15,7 @@ vi.mock('vue-router', () => {
 		useRouter: () => ({
 			push,
 		}),
-		useRoute: () => ({
+		useRoute: vi.fn().mockReturnValue({
 			query: {
 				redirect: '/home/workflows',
 			},
@@ -43,20 +44,7 @@ let router: ReturnType<typeof useRouter>;
 let telemetry: ReturnType<typeof useTelemetry>;
 
 describe('SigninView', () => {
-	beforeEach(() => {
+	const signInWithValidUser = async () => {
-		createTestingPinia();
-		usersStore = mockedStore(useUsersStore);
-		settingsStore = mockedStore(useSettingsStore);
-
-		router = useRouter();
-		telemetry = useTelemetry();
-	});
-
-	it('should not throw error when opened', () => {
-		expect(() => renderComponent()).not.toThrow();
-	});
-
-	it('should show and submit email/password form (happy path)', async () => {
 		settingsStore.isCloudDeployment = false;
 		usersStore.loginWithCreds.mockResolvedValueOnce();
 
@@ -83,6 +71,27 @@ describe('SigninView', () => {
 		await userEvent.type(passwordInput, 'password');
 
 		await userEvent.click(submitButton);
+	};
+
+	beforeEach(() => {
+		createTestingPinia();
+		usersStore = mockedStore(useUsersStore);
+		settingsStore = mockedStore(useSettingsStore);
+
+		router = useRouter();
+		telemetry = useTelemetry();
+	});
+
+	afterEach(() => {
+		vi.clearAllMocks();
+	});
+
+	it('should not throw error when opened', () => {
+		expect(() => renderComponent()).not.toThrow();
+	});
+
+	it('should show and submit email/password form (happy path)', async () => {
+		await signInWithValidUser();
 
 		expect(usersStore.loginWithCreds).toHaveBeenCalledWith({
 			emailOrLdapLoginId: 'test@n8n.io',
@@ -97,4 +106,89 @@ describe('SigninView', () => {
 
 		expect(router.push).toHaveBeenCalledWith('/home/workflows');
 	});
+
+	describe('when redirect query parameter is set', () => {
+		const ORIGIN_URL = 'https://n8n.local';
+		let route: ReturnType<typeof useRoute>;
+
+		beforeEach(() => {
+			route = useRoute();
+			global.window = Object.create(window);
+
+			Object.defineProperty(window, 'location', {
+				value: {
+					href: '',
+					origin: ORIGIN_URL,
+				},
+				writable: true,
+			});
+		});
+
+		it('should redirect to homepage with router if redirect url does not contain the origin domain', async () => {
+			vi.spyOn(route, 'query', 'get').mockReturnValue({
+				redirect: 'https://n8n.local.evil.com',
+			});
+
+			const hrefSpy = vi.spyOn(window.location, 'href', 'set');
+
+			await signInWithValidUser();
+
+			expect(hrefSpy).not.toHaveBeenCalled();
+			expect(router.push).toHaveBeenCalledWith({ name: VIEWS.HOMEPAGE });
+		});
+
+		it('should redirect to homepage with router if redirect url does not contain a valid URL', async () => {
+			vi.spyOn(route, 'query', 'get').mockReturnValue({
+				redirect: 'not-a-valid-url',
+			});
+
+			const hrefSpy = vi.spyOn(window.location, 'href', 'set');
+
+			await signInWithValidUser();
+
+			expect(hrefSpy).not.toHaveBeenCalled();
+			expect(router.push).toHaveBeenCalledWith({ name: VIEWS.HOMEPAGE });
+		});
+
+		it('should redirect to given route if redirect url contains the origin domain', async () => {
+			const validRedirectUrl = 'https://n8n.local/valid-redirect';
+			vi.spyOn(route, 'query', 'get').mockReturnValue({
+				redirect: validRedirectUrl,
+			});
+
+			const hrefSpy = vi.spyOn(window.location, 'href', 'set');
+
+			await signInWithValidUser();
+
+			expect(hrefSpy).toHaveBeenCalledWith(validRedirectUrl);
+			expect(router.push).not.toHaveBeenCalled();
+		});
+
+		it('should redirect with router to given route if redirect url is a local path', async () => {
+			const validLocalRedirectUrl = '/valid-redirect';
+			vi.spyOn(route, 'query', 'get').mockReturnValue({
+				redirect: validLocalRedirectUrl,
+			});
+
+			const hrefSpy = vi.spyOn(window.location, 'href', 'set');
+
+			await signInWithValidUser();
+
+			expect(hrefSpy).not.toHaveBeenCalled();
+			expect(router.push).toHaveBeenCalledWith(validLocalRedirectUrl);
+		});
+
+		it('should redirect to homepage with router if redirect url is empty', async () => {
+			vi.spyOn(route, 'query', 'get').mockReturnValue({
+				redirect: '',
+			});
+
+			const hrefSpy = vi.spyOn(window.location, 'href', 'set');
+
+			await signInWithValidUser();
+
+			expect(hrefSpy).not.toHaveBeenCalled();
+			expect(router.push).toHaveBeenCalledWith({ name: VIEWS.HOMEPAGE });
+		});
+	});
 });

packages/frontend/editor-ui/src/views/SigninView.vue

@@ -101,7 +101,19 @@ const onEmailPasswordSubmitted = async (form: EmailOrLdapLoginIdAndPassword) =>
 
 const isRedirectSafe = () => {
 	const redirect = getRedirectQueryParameter();
-	return redirect.startsWith('/') || redirect.startsWith(window.location.origin);
+
+	// Allow local redirects
+	if (redirect.startsWith('/')) {
+		return true;
+	}
+
+	try {
+		// Only allow origin domain redirects
+		const url = new URL(redirect);
+		return url.origin === window.location.origin;
+	} catch {
+		return false;
+	}
 };
 
 const getRedirectQueryParameter = () => {