diff --git a/web/app/(app)/(auth)/(components)/login-form.test.tsx b/web/app/(app)/(auth)/(components)/login-form.test.tsx new file mode 100644 index 00000000..900ca56f --- /dev/null +++ b/web/app/(app)/(auth)/(components)/login-form.test.tsx @@ -0,0 +1,92 @@ +import { describe, expect, it, vi, beforeEach } from 'vitest' +import { render, screen, waitFor } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import LoginForm from './login-form' + +const push = vi.fn() +const refresh = vi.fn() +vi.mock('next/navigation', () => ({ + useRouter: () => ({ push, refresh }), +})) + +const signIn = vi.fn() +vi.mock('next-auth/react', () => ({ signIn: (...a: unknown[]) => signIn(...a) })) + +// signIn reflects next-auth's real redirect semantics: with redirect:true it +// navigates and never resolves a usable result; with redirect:false it resolves +// { error, ok }. A component that reads the result while passing redirect:true +// can never see the error, which is the bug under test. +const redirectAwareSignIn = (_provider: string, opts: { redirect?: boolean }) => + opts?.redirect + ? undefined + : { error: 'CredentialsSignin', ok: false, status: 401, url: null } + +// Turnstile needs a live Cloudflare widget it cannot get in jsdom, so the hook +// is mocked to hand the form a token immediately, making the form submittable. +vi.mock('@/lib/turnstile', () => ({ + useTurnstile: (opts: { onToken?: (t: string) => void }) => { + opts.onToken?.('test-turnstile-token') + return { + containerRef: { current: null }, + token: 'test-turnstile-token', + error: null, + isReady: true, + } + }, +})) + +async function fillAndSubmit() { + await userEvent.type(screen.getByLabelText('Email'), 'user@example.com') + await userEvent.type(screen.getByLabelText('Password'), 'secret123') + await userEvent.click(screen.getByRole('button', { name: /sign in/i })) +} + +describe('LoginForm', () => { + beforeEach(() => { + vi.clearAllMocks() + signIn.mockImplementation(redirectAwareSignIn) + }) + + // The reported bug: wrong credentials showed no message and reloaded, because + // redirect:true made the error branch dead code. + it('shows an error and does not navigate on wrong credentials', async () => { + // Uses the redirect-aware default: only redirect:false yields a readable + // error. Against the redirect:true code this returns undefined, so the + // message never renders and this test fails, which is the point. + render() + await fillAndSubmit() + + expect( + await screen.findByText(/invalid email or password/i) + ).toBeInTheDocument() + expect(push).not.toHaveBeenCalled() + }) + + it('calls signIn without a redirect so the result can be read', async () => { + render() + await fillAndSubmit() + + await waitFor(() => expect(signIn).toHaveBeenCalled()) + expect(signIn).toHaveBeenCalledWith( + 'email-password-login', + expect.objectContaining({ redirect: false }) + ) + }) + + it('navigates to the dashboard on success', async () => { + signIn.mockResolvedValueOnce({ + error: null, + ok: true, + status: 200, + url: null, + }) + + render() + await fillAndSubmit() + + await waitFor(() => expect(push).toHaveBeenCalledWith('/dashboard')) + expect( + screen.queryByText(/invalid email or password/i) + ).not.toBeInTheDocument() + }) +}) diff --git a/web/app/(app)/(auth)/(components)/login-form.tsx b/web/app/(app)/(auth)/(components)/login-form.tsx index 2b13832d..1d0d5131 100644 --- a/web/app/(app)/(auth)/(components)/login-form.tsx +++ b/web/app/(app)/(auth)/(components)/login-form.tsx @@ -80,9 +80,11 @@ export default function LoginForm() { } try { + // redirect:false so the result is returned here. With redirect:true + // next-auth navigates on both success and failure, which reloaded the + // page and made the error branch below dead code. const result = await signIn('email-password-login', { - redirect: true, - callbackUrl: Routes.dashboard, + redirect: false, email: data.email, password: data.password, turnstileToken: data.turnstileToken, @@ -93,7 +95,11 @@ export default function LoginForm() { type: 'manual', message: 'Invalid email or password', }) + return } + + router.push(Routes.dashboard) + router.refresh() } catch (error) { console.error('login error:', error) form.setError('root', { diff --git a/web/app/(app)/(auth)/(components)/register-form.test.tsx b/web/app/(app)/(auth)/(components)/register-form.test.tsx new file mode 100644 index 00000000..c45730bc --- /dev/null +++ b/web/app/(app)/(auth)/(components)/register-form.test.tsx @@ -0,0 +1,62 @@ +import { describe, expect, it, vi, beforeEach } from 'vitest' +import { render, screen, waitFor } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import RegisterForm from './register-form' + +const push = vi.fn() +vi.mock('next/navigation', () => ({ useRouter: () => ({ push, refresh: vi.fn() }) })) + +const signIn = vi.fn() +vi.mock('next-auth/react', () => ({ signIn: (...a: unknown[]) => signIn(...a) })) + +vi.mock('@/lib/turnstile', () => ({ + useTurnstile: (opts: { onToken?: (t: string) => void }) => { + opts.onToken?.('test-turnstile-token') + return { containerRef: { current: null }, token: 'test-turnstile-token', error: null, isReady: true } + }, +})) + +async function fillAndSubmit() { + await userEvent.type(screen.getByLabelText('Full Name'), 'Ada Lovelace') + await userEvent.type(screen.getByLabelText('Email'), 'ada@example.com') + await userEvent.type(screen.getByLabelText('Password'), 'password123') + await userEvent.click(screen.getByRole('button', { name: /create account|sign up|register/i })) +} + +// register-form already handles errors correctly (redirect:false). These lock +// that so the login bug (dead error branch under redirect:true) cannot be +// reintroduced here unnoticed. +describe('RegisterForm', () => { + beforeEach(() => vi.clearAllMocks()) + + it('shows an error and does not navigate when registration fails', async () => { + signIn.mockResolvedValueOnce({ error: 'CredentialsSignin', ok: false }) + render() + await fillAndSubmit() + + expect(await screen.findByText(/failed to create account/i)).toBeInTheDocument() + expect(push).not.toHaveBeenCalled() + }) + + it('submits with redirect:false so the result is readable', async () => { + signIn.mockResolvedValueOnce({ ok: true, error: null }) + render() + await fillAndSubmit() + + await waitFor(() => expect(signIn).toHaveBeenCalled()) + expect(signIn).toHaveBeenCalledWith( + 'email-password-register', + expect.objectContaining({ redirect: false }) + ) + }) + + it('navigates to verify-email on success', async () => { + signIn.mockResolvedValueOnce({ ok: true, error: null }) + render() + await fillAndSubmit() + + await waitFor(() => + expect(push).toHaveBeenCalledWith('/verify-email?verificationEmailSent=1') + ) + }) +}) diff --git a/web/app/(app)/(auth)/(components)/reset-password-form.test.tsx b/web/app/(app)/(auth)/(components)/reset-password-form.test.tsx new file mode 100644 index 00000000..9f5d60d4 --- /dev/null +++ b/web/app/(app)/(auth)/(components)/reset-password-form.test.tsx @@ -0,0 +1,58 @@ +import { describe, expect, it, vi, beforeEach } from 'vitest' +import { render, screen, waitFor } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import ResetPasswordForm from './reset-password-form' + +const post = vi.fn() +vi.mock('@/lib/httpBrowserClient', () => ({ default: { post: (...a: unknown[]) => post(...a) } })) + +async function fillAndSubmit() { + await userEvent.type(screen.getByLabelText('New Password'), 'newpassword123') + await userEvent.type(screen.getByLabelText('Confirm Password'), 'newpassword123') + await userEvent.click(screen.getByRole('button', { name: /reset password/i })) +} + +describe('ResetPasswordForm', () => { + beforeEach(() => vi.clearAllMocks()) + + const renderForm = () => + render() + + // The bug: the catch set root.serverError but the JSX read errors.root.message, + // so a failed reset showed an empty paragraph and no message. + it('shows an error message when the reset fails', async () => { + post.mockRejectedValueOnce(new Error('boom')) + renderForm() + await fillAndSubmit() + + expect( + await screen.findByText(/failed to reset password/i) + ).toBeInTheDocument() + }) + + // The second bug: the handler swallows the error, so isSubmitSuccessful stays + // true and the success alert rendered even on failure. + it('does not claim success when the reset fails', async () => { + post.mockRejectedValueOnce(new Error('boom')) + renderForm() + await fillAndSubmit() + + await screen.findByText(/failed to reset password/i) + expect( + screen.queryByText(/password reset successful/i) + ).not.toBeInTheDocument() + }) + + it('shows the success alert when the reset succeeds', async () => { + post.mockResolvedValueOnce({ data: {} }) + renderForm() + await fillAndSubmit() + + expect( + await screen.findByText(/password reset successful/i) + ).toBeInTheDocument() + expect( + screen.queryByText(/failed to reset password/i) + ).not.toBeInTheDocument() + }) +}) diff --git a/web/app/(app)/(auth)/(components)/reset-password-form.tsx b/web/app/(app)/(auth)/(components)/reset-password-form.tsx index 22ca41e5..9636293e 100644 --- a/web/app/(app)/(auth)/(components)/reset-password-form.tsx +++ b/web/app/(app)/(auth)/(components)/reset-password-form.tsx @@ -70,11 +70,14 @@ export default function ResetPasswordForm({ }) const onResetPassword = async (data: ResetPasswordFormValues) => { + form.clearErrors() try { await httpBrowserClient.post(ApiEndpoints.auth.resetPassword(), data) } catch (error) { console.error(error) - form.setError('root.serverError', { + // 'root', not 'root.serverError': the JSX renders errors.root.message, + // and a nested key leaves that undefined so no message showed. + form.setError('root', { message: 'Failed to reset password', }) } @@ -174,7 +177,8 @@ export default function ResetPasswordForm({ - {form.formState.isSubmitted && form.formState.isSubmitSuccessful && ( + {form.formState.isSubmitSuccessful && + !form.formState.errors.root && ( {/* */} Password reset successful