From 16f0feb8d5ad347ac8664f20f3d726a6999f31cf Mon Sep 17 00:00:00 2001 From: Marcel Raddatz Date: Thu, 2 Apr 2026 18:48:48 +0200 Subject: [PATCH] fix(auth): fix mock responses in tests and block open redirect in login - Add response object to mockSuccess() in login and signup tests so response.headers.get() no longer throws - Validate ?redirect= param: must start with / and not // to prevent redirecting users to external domains Co-Authored-By: Claude Sonnet 4.6 --- .../src/routes/(public)/login/+page.server.ts | 3 +- .../routes/(public)/login/page.server.test.ts | 44 +++++++++++++++++-- .../(public)/signup/page.server.test.ts | 12 ++++- 3 files changed, 53 insertions(+), 6 deletions(-) diff --git a/frontend/src/routes/(public)/login/+page.server.ts b/frontend/src/routes/(public)/login/+page.server.ts index 5b4a45b..9dc9b09 100644 --- a/frontend/src/routes/(public)/login/+page.server.ts +++ b/frontend/src/routes/(public)/login/+page.server.ts @@ -40,7 +40,8 @@ export const actions = { cookies.set('JSESSIONID', sessionId, { path: '/', httpOnly: true, sameSite: 'lax' }); } - const redirectTo = url.searchParams.get('redirect') || '/planner'; + const raw = url.searchParams.get('redirect'); + const redirectTo = raw && raw.startsWith('/') && !raw.startsWith('//') ? raw : '/planner'; throw redirect(303, redirectTo); } } satisfies Actions; diff --git a/frontend/src/routes/(public)/login/page.server.test.ts b/frontend/src/routes/(public)/login/page.server.test.ts index a3f636e..e143566 100644 --- a/frontend/src/routes/(public)/login/page.server.test.ts +++ b/frontend/src/routes/(public)/login/page.server.test.ts @@ -31,8 +31,16 @@ describe('login form action', () => { } as any; } + function mockSuccess() { + return { + data: { data: { id: '123' } }, + error: undefined, + response: { headers: { get: vi.fn().mockReturnValue(null) } } + }; + } + it('calls POST /v1/auth/login with form data', async () => { - mockPost.mockResolvedValue({ data: { data: { id: '123' } }, error: undefined }); + mockPost.mockResolvedValue(mockSuccess()); try { await actions.default(createEvent({ @@ -52,7 +60,7 @@ describe('login form action', () => { }); it('redirects to /planner on success by default', async () => { - mockPost.mockResolvedValue({ data: { data: { id: '123' } }, error: undefined }); + mockPost.mockResolvedValue(mockSuccess()); try { await actions.default(createEvent({ @@ -67,7 +75,7 @@ describe('login form action', () => { }); it('redirects to ?redirect param when present', async () => { - mockPost.mockResolvedValue({ data: { data: { id: '123' } }, error: undefined }); + mockPost.mockResolvedValue(mockSuccess()); try { await actions.default(createEvent( @@ -81,6 +89,36 @@ describe('login form action', () => { } }); + it('falls back to /planner when ?redirect= is an absolute URL', async () => { + mockPost.mockResolvedValue(mockSuccess()); + + try { + await actions.default(createEvent( + { email: 'sarah@example.com', password: 'password123' }, + '?redirect=https%3A%2F%2Fevil.com' + )); + expect.unreachable(); + } catch (e: any) { + expect(e.status).toBe(303); + expect(e.location).toBe('/planner'); + } + }); + + it('falls back to /planner when ?redirect= is a protocol-relative URL', async () => { + mockPost.mockResolvedValue(mockSuccess()); + + try { + await actions.default(createEvent( + { email: 'sarah@example.com', password: 'password123' }, + '?redirect=%2F%2Fevil.com' + )); + expect.unreachable(); + } catch (e: any) { + expect(e.status).toBe(303); + expect(e.location).toBe('/planner'); + } + }); + it('rejects empty email with validation error', async () => { const result = await actions.default(createEvent({ email: '', diff --git a/frontend/src/routes/(public)/signup/page.server.test.ts b/frontend/src/routes/(public)/signup/page.server.test.ts index defc113..b224db5 100644 --- a/frontend/src/routes/(public)/signup/page.server.test.ts +++ b/frontend/src/routes/(public)/signup/page.server.test.ts @@ -30,8 +30,16 @@ describe('signup form action', () => { } as any; } + function mockSuccess() { + return { + data: { data: { id: '123' } }, + error: undefined, + response: { headers: { get: vi.fn().mockReturnValue(null) } } + }; + } + it('calls POST /v1/auth/signup with form data', async () => { - mockPost.mockResolvedValue({ data: { data: { id: '123' } }, error: undefined }); + mockPost.mockResolvedValue(mockSuccess()); try { await actions.default(createRequest({ @@ -53,7 +61,7 @@ describe('signup form action', () => { }); it('redirects to /household/setup on success', async () => { - mockPost.mockResolvedValue({ data: { data: { id: '123' } }, error: undefined }); + mockPost.mockResolvedValue(mockSuccess()); try { await actions.default(createRequest({