From 78f69b873652c7c587bdf5b73a0cf33a21e99414 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 23 Apr 2026 02:41:44 -0400 Subject: [PATCH] feat(web): add login page with returnTo support and typed error UX Inline errors for invalid creds vs. server-side failures. Validates returnTo to block open-redirect vectors (//, http://, /login, parent-traversal). --- web/package.json | 5 +- web/src/routes/login/+page.svelte | 81 +++++++++++++++++++ web/src/routes/login/+page.test.ts | 122 +++++++++++++++++++++++++++++ 3 files changed, 206 insertions(+), 2 deletions(-) create mode 100644 web/src/routes/login/+page.svelte create mode 100644 web/src/routes/login/+page.test.ts diff --git a/web/package.json b/web/package.json index 3c8559f9..691d545f 100644 --- a/web/package.json +++ b/web/package.json @@ -7,8 +7,9 @@ "dev": "vite dev", "build": "vite build", "preview": "vite preview", - "check": "svelte-kit sync && svelte-check --tsconfig ./tsconfig.json", - "test": "svelte-kit sync && vitest run" + "sync": "svelte-kit sync", + "check": "svelte-check --tsconfig ./tsconfig.json", + "test": "vitest run" }, "devDependencies": { "@sveltejs/adapter-static": "^3.0.6", diff --git a/web/src/routes/login/+page.svelte b/web/src/routes/login/+page.svelte new file mode 100644 index 00000000..a76d7337 --- /dev/null +++ b/web/src/routes/login/+page.svelte @@ -0,0 +1,81 @@ + + +
+
+

Minstrel

+
+ + + + {#if error} + + {/if} +
+
+
diff --git a/web/src/routes/login/+page.test.ts b/web/src/routes/login/+page.test.ts new file mode 100644 index 00000000..38ec3987 --- /dev/null +++ b/web/src/routes/login/+page.test.ts @@ -0,0 +1,122 @@ +import { afterEach, describe, expect, test, vi } from 'vitest'; +import { render, screen, fireEvent, waitFor } from '@testing-library/svelte'; + +// vi.hoisted avoids the temporal-dead-zone hazard: mock factories are lazy +// and can run before top-level `let` is initialized, since imports are +// hoisted above variable declarations. +const state = vi.hoisted(() => ({ pageUrl: new URL('http://localhost/login') })); + +vi.mock('$app/state', () => ({ + page: { + get url() { return state.pageUrl; } + } +})); + +vi.mock('$app/navigation', () => ({ + goto: vi.fn() +})); + +vi.mock('$lib/auth/store.svelte', () => ({ + login: vi.fn(), + user: { value: null } +})); + +import LoginPage from './+page.svelte'; +import { login } from '$lib/auth/store.svelte'; +import { goto } from '$app/navigation'; + +afterEach(() => { + vi.clearAllMocks(); + state.pageUrl = new URL('http://localhost/login'); +}); + +async function submit(username = 'alice', password = 'pw') { + const u = screen.getByLabelText(/username/i) as HTMLInputElement; + const p = screen.getByLabelText(/password/i) as HTMLInputElement; + await fireEvent.input(u, { target: { value: username } }); + await fireEvent.input(p, { target: { value: password } }); + await fireEvent.click(screen.getByRole('button', { name: /sign in/i })); +} + +describe('login page', () => { + test('renders username, password, and sign-in button', () => { + render(LoginPage); + expect(screen.getByLabelText(/username/i)).toBeInTheDocument(); + expect(screen.getByLabelText(/password/i)).toBeInTheDocument(); + expect(screen.getByRole('button', { name: /sign in/i })).toBeInTheDocument(); + }); + + test('invalid credentials show inline error and clear password', async () => { + (login as ReturnType).mockRejectedValue({ + code: 'invalid_credentials', message: 'bad creds', status: 401 + }); + render(LoginPage); + await submit(); + await waitFor(() => { + expect(screen.getByRole('alert')).toHaveTextContent(/invalid username or password/i); + }); + expect((screen.getByLabelText(/password/i) as HTMLInputElement).value).toBe(''); + expect((screen.getByLabelText(/username/i) as HTMLInputElement).value).toBe('alice'); + }); + + test('server error shows generic message and preserves both fields', async () => { + (login as ReturnType).mockRejectedValue({ + code: 'server_error', message: 'boom', status: 500 + }); + render(LoginPage); + await submit('alice', 'pw'); + await waitFor(() => { + expect(screen.getByRole('alert')).toHaveTextContent(/try again in a moment/i); + }); + expect((screen.getByLabelText(/password/i) as HTMLInputElement).value).toBe('pw'); + expect((screen.getByLabelText(/username/i) as HTMLInputElement).value).toBe('alice'); + }); + + test('success navigates to returnTo when safe', async () => { + state.pageUrl = new URL('http://localhost/login?returnTo=/artists/abc'); + (login as ReturnType).mockResolvedValue(undefined); + render(LoginPage); + await submit(); + await waitFor(() => { + expect(goto).toHaveBeenCalledWith('/artists/abc', { replaceState: true }); + }); + }); + + test('success falls back to "/" when returnTo is missing', async () => { + (login as ReturnType).mockResolvedValue(undefined); + render(LoginPage); + await submit(); + await waitFor(() => { + expect(goto).toHaveBeenCalledWith('/', { replaceState: true }); + }); + }); + + test.each([ + ['//evil.com', 'protocol-relative'], + ['http://evil.com', 'absolute URL'], + ['/login', 'self-redirect'], + ['../admin', 'parent traversal'] + ])('rejects unsafe returnTo %s (%s) and falls back to "/"', async (value) => { + state.pageUrl = new URL(`http://localhost/login?returnTo=${encodeURIComponent(value)}`); + (login as ReturnType).mockResolvedValue(undefined); + render(LoginPage); + await submit(); + await waitFor(() => { + expect(goto).toHaveBeenCalledWith('/', { replaceState: true }); + }); + }); + + test('submit button disables and marks aria-busy while pending', async () => { + let release!: () => void; + (login as ReturnType).mockReturnValue( + new Promise((resolve) => { release = () => resolve(); }) + ); + render(LoginPage); + await submit(); + const btn = screen.getByRole('button', { name: /sign in/i }); + expect(btn).toBeDisabled(); + expect(btn).toHaveAttribute('aria-busy', 'true'); + release(); + await waitFor(() => expect(btn).not.toBeDisabled()); + }); +});