feat(authentication): preserve intended URL after re-login

app/features/authentication/routes/login.tsx

@@ -3,6 +3,10 @@ import { parseWithZod } from '@conform-to/zod'
 import { data, Form, Link, redirect } from 'react-router'
 import { loginSchema } from '~/features/authentication/schemas/login.schema'
 import { needSetupProcess } from '~/features/authentication/server/need-setup-process.server'
+import {
+  buildLoginRedirectUrl,
+  resolvePostLoginRedirect,
+} from '~/features/authentication/server/post-login-redirect.server'
 import {
   checkLoginRateLimit,
   clearLoginAttempts,
@@ -19,6 +23,7 @@ import { Button } from '~/shared/ui/button'
 import { Card, CardContent, CardFooter, CardHeader } from '~/shared/ui/card'
 import { Input } from '~/shared/ui/input'
 import { Label } from '~/shared/ui/label'
+import { safeRedirectUrl } from '~/shared/utils/safe-redirect.server'
 import type { Route } from './+types/login'
 
 export const meta: Route.MetaFunction = () => {
@@ -29,6 +34,8 @@ export async function loader({ request }: Route.LoaderArgs) {
   // Verify that the subdomain matches an existing congregation
   await resolveCongregationFromRequest(request)
 
+  const redirectTo = safeRedirectUrl(new URL(request.url).searchParams.get('redirectTo'), '/')
+
   const shouldStartSetup = await needSetupProcess()
   if (shouldStartSetup) {
     return redirect(process.env.UNITAE_MULTI_TENANT === 'true' ? '/register' : '/setup')
@@ -39,7 +46,7 @@ export async function loader({ request }: Route.LoaderArgs) {
     const uid = Number(session.get('userId'))
     if (Number.isNaN(uid) || uid <= 0) {
       return data(
-        { error: undefined, brandingName: await getBrandingName(request) },
+        { error: undefined, brandingName: await getBrandingName(request), redirectTo },
         { headers: { 'Set-Cookie': await destroySession(session) } },
       )
     }
@@ -53,18 +60,18 @@ export async function loader({ request }: Route.LoaderArgs) {
       })
       if (!user || user.congregationId !== urlCongregation.id) {
         return data(
-          { error: undefined, brandingName: await getBrandingName(request) },
+          { error: undefined, brandingName: await getBrandingName(request), redirectTo },
           { headers: { 'Set-Cookie': await destroySession(session) } },
         )
       }
     }
-    throw redirect('/')
+    throw redirect(redirectTo)
   }
 
   const brandingName = await getBrandingName(request)
 
   return data(
-    { error: session.get('error'), brandingName },
+    { error: session.get('error'), brandingName, redirectTo },
     {
       headers: {
         'Set-Cookie': await commitSession(session),
@@ -74,7 +81,7 @@ export async function loader({ request }: Route.LoaderArgs) {
 }
 
 export default function LoginPage({ loaderData, actionData }: Route.ComponentProps) {
-  const { error, brandingName } = loaderData
+  const { error, brandingName, redirectTo } = loaderData
 
   const [form, fields] = useForm({
     lastResult: actionData,
@@ -97,6 +104,7 @@ export default function LoginPage({ loaderData, actionData }: Route.ComponentPro
             </Alert>
           )}
           <Form method="post" className="flex flex-col gap-4" {...getFormProps(form)}>
+            <input type="hidden" name="redirectTo" value={redirectTo} />
             <div className="flex flex-col gap-2">
               <Label htmlFor={fields.email.id}>{m.auth_login_email()}</Label>
               <Input {...getInputProps(fields.email, { type: 'email' })} autoFocus={true} autoComplete="username" />
@@ -127,7 +135,10 @@ export default function LoginPage({ loaderData, actionData }: Route.ComponentPro
 
 export async function action({ request }: Route.ActionArgs) {
   const session = await getSession(request.headers.get('Cookie'))
-  const submission = parseWithZod(await request.formData(), { schema: loginSchema })
+  const formData = await request.formData()
+  const postLoginRedirect = resolvePostLoginRedirect(request, formData)
+  const loginUrl = buildLoginRedirectUrl(postLoginRedirect)
+  const submission = parseWithZod(formData, { schema: loginSchema })
 
   if (submission.status !== 'success') {
     return data(submission.reply(), { status: 400 })
@@ -139,7 +150,7 @@ export async function action({ request }: Route.ActionArgs) {
   if (!allowed) {
     session.flash('error', m.auth_login_rate_limit_error())
 
-    return redirect('/login', {
+    return redirect(loginUrl, {
       headers: { 'Set-Cookie': await commitSession(session) },
     })
   }
@@ -158,7 +169,7 @@ export async function action({ request }: Route.ActionArgs) {
     }
     session.flash('error', m.auth_login_invalid_credentials_error())
 
-    return redirect('/login', {
+    return redirect(loginUrl, {
       headers: { 'Set-Cookie': await commitSession(session) },
     })
   }
@@ -177,7 +188,7 @@ export async function action({ request }: Route.ActionArgs) {
     })
   }
 
-  return redirect('/', {
+  return redirect(postLoginRedirect, {
     headers: { 'Set-Cookie': await commitSession(session) },
   })
 }

app/features/authentication/server/post-login-redirect.server.test.ts

@@ -0,0 +1,103 @@
+import { describe, expect, it } from 'vitest'
+import { buildLoginRedirectUrl, resolvePostLoginRedirect } from './post-login-redirect.server'
+
+function makeFormData(entries: Record<string, string>): FormData {
+  const formData = new FormData()
+  for (const [key, value] of Object.entries(entries)) {
+    formData.set(key, value)
+  }
+  return formData
+}
+
+describe('resolvePostLoginRedirect', () => {
+  it("préfère la valeur du form à celle de l'URL", () => {
+    const request = new Request('http://localhost/login?redirectTo=%2Ffrom-url')
+    const formData = makeFormData({ redirectTo: '/from-form' })
+
+    expect(resolvePostLoginRedirect(request, formData)).toBe('/from-form')
+  })
+
+  it("utilise le paramètre d'URL quand le champ de formulaire est absent", () => {
+    const request = new Request('http://localhost/login?redirectTo=%2Fterritories%2F1')
+    const formData = makeFormData({})
+
+    expect(resolvePostLoginRedirect(request, formData)).toBe('/territories/1')
+  })
+
+  it("utilise le paramètre d'URL quand le champ de formulaire est vide", () => {
+    const request = new Request('http://localhost/login?redirectTo=%2Fterritories%2F1')
+    const formData = makeFormData({ redirectTo: '' })
+
+    expect(resolvePostLoginRedirect(request, formData)).toBe('/territories/1')
+  })
+
+  it("retourne '/' quand ni le form ni l'URL ne fournissent de redirectTo", () => {
+    const request = new Request('http://localhost/login')
+    const formData = makeFormData({})
+
+    expect(resolvePostLoginRedirect(request, formData)).toBe('/')
+  })
+
+  it("retourne '/' quand le paramètre d'URL redirectTo est présent mais vide", () => {
+    const request = new Request('http://localhost/login?redirectTo=')
+    const formData = makeFormData({})
+
+    expect(resolvePostLoginRedirect(request, formData)).toBe('/')
+  })
+
+  it("retourne '/' quand le champ de formulaire et le paramètre d'URL sont tous les deux vides", () => {
+    const request = new Request('http://localhost/login?redirectTo=')
+    const formData = makeFormData({ redirectTo: '' })
+
+    expect(resolvePostLoginRedirect(request, formData)).toBe('/')
+  })
+
+  it("rejette une URL protocol-relative (//evil.com) et retombe sur '/'", () => {
+    const request = new Request('http://localhost/login')
+    const formData = makeFormData({ redirectTo: '//evil.com/path' })
+
+    expect(resolvePostLoginRedirect(request, formData)).toBe('/')
+  })
+
+  it("rejette une URL absolue (https://evil.com) et retombe sur '/'", () => {
+    const request = new Request('http://localhost/login')
+    const formData = makeFormData({ redirectTo: 'https://evil.com/path' })
+
+    expect(resolvePostLoginRedirect(request, formData)).toBe('/')
+  })
+
+  it("rejette un chemin ne commençant pas par '/' et retombe sur '/'", () => {
+    const request = new Request('http://localhost/login')
+    const formData = makeFormData({ redirectTo: 'javascript:alert(1)' })
+
+    expect(resolvePostLoginRedirect(request, formData)).toBe('/')
+  })
+
+  it("retombe sur '/' quand request.url est mal formée (ne parvient pas à parser)", () => {
+    const badRequest = { url: 'not-a-valid-url' } as unknown as Request
+    const formData = makeFormData({})
+
+    expect(resolvePostLoginRedirect(badRequest, formData)).toBe('/')
+  })
+
+  it('privilégie la valeur du form même si request.url est mal formée', () => {
+    const badRequest = { url: 'not-a-valid-url' } as unknown as Request
+    const formData = makeFormData({ redirectTo: '/from-form' })
+
+    expect(resolvePostLoginRedirect(badRequest, formData)).toBe('/from-form')
+  })
+})
+
+describe('buildLoginRedirectUrl', () => {
+  it("retourne '/login' sans query quand la cible est '/'", () => {
+    expect(buildLoginRedirectUrl('/')).toBe('/login')
+  })
+
+  it('encode la cible dans le paramètre redirectTo', () => {
+    expect(buildLoginRedirectUrl('/territories/1?x=2')).toBe('/login?redirectTo=%2Fterritories%2F1%3Fx%3D2')
+  })
+
+  it('encode les caractères spéciaux (unicode, espaces)', () => {
+    expect(buildLoginRedirectUrl('/territoires/été 42')).toBe('/login?redirectTo=%2Fterritoires%2F%C3%A9t%C3%A9%2042')
+  })
+})

app/features/authentication/server/post-login-redirect.server.ts

@@ -0,0 +1,23 @@
+import logger from '~/shared/infra/logger.server'
+import { safeRedirectUrl } from '~/shared/utils/safe-redirect.server'
+
+export function resolvePostLoginRedirect(request: Request, formData: FormData): string {
+  const formValue = formData.get('redirectTo')
+  if (formValue != null && typeof formValue !== 'string') {
+    logger.debug(`resolvePostLoginRedirect: ignoring non-string redirectTo form value (${typeof formValue})`)
+  }
+  const fromForm = typeof formValue === 'string' && formValue.length > 0 ? formValue : null
+
+  let fromUrl: string | null = null
+  try {
+    fromUrl = new URL(request.url).searchParams.get('redirectTo')
+  } catch {
+    logger.warn(`resolvePostLoginRedirect: unable to parse request.url (${request.url})`)
+  }
+
+  return safeRedirectUrl(fromForm ?? fromUrl, '/')
+}
+
+export function buildLoginRedirectUrl(target: string): string {
+  return target === '/' ? '/login' : `/login?redirectTo=${encodeURIComponent(target)}`
+}

app/features/authentication/server/session.server.test.ts

@@ -97,43 +97,44 @@ describe('verifySession', () => {
     expect(result.session).toBe(mockSession)
   })
 
-  it('redirige vers /login si le userId est absent de la session', async () => {
+  it('redirige vers /login avec redirectTo si le userId est absent de la session', async () => {
     mockSession.get.mockReturnValue(undefined)
 
-    const response = await getRedirectResponse(() => verifySession(makeRequest()))
-    expect(response.headers.get('Location')).toBe('/login')
+    const response = await getRedirectResponse(() => verifySession(makeRequest('http://localhost/territories/1?x=2')))
+    expect(response.headers.get('Location')).toBe('/login?redirectTo=%2Fterritories%2F1%3Fx%3D2')
   })
 
-  it('redirige vers /login si le userId est NaN', async () => {
+  it('redirige vers /login avec redirectTo si le userId est NaN', async () => {
     mockSession.get.mockReturnValue('invalid')
 
-    const response = await getRedirectResponse(() => verifySession(makeRequest()))
-    expect(response.headers.get('Location')).toBe('/login')
+    const response = await getRedirectResponse(() => verifySession(makeRequest('http://localhost/territories/1?x=2')))
+    expect(response.headers.get('Location')).toBe('/login?redirectTo=%2Fterritories%2F1%3Fx%3D2')
   })
 
-  it("redirige vers /login si l'utilisateur n'existe pas", async () => {
+  it("redirige vers /login avec redirectTo si l'utilisateur n'existe pas", async () => {
     mockSession.get.mockReturnValue('42')
     vi.mocked(db.userAccount.findUnique).mockResolvedValue(null as never)
 
-    const response = await getRedirectResponse(() => verifySession(makeRequest()))
-    expect(response.headers.get('Location')).toBe('/login')
+    const response = await getRedirectResponse(() => verifySession(makeRequest('http://localhost/territories/1?x=2')))
+    expect(response.headers.get('Location')).toBe('/login?redirectTo=%2Fterritories%2F1%3Fx%3D2')
   })
 
-  it("redirige vers /login si l'utilisateur est inactif", async () => {
+  it("redirige vers /login avec redirectTo si l'utilisateur est inactif", async () => {
     mockSession.get.mockReturnValue('42')
     vi.mocked(db.userAccount.findUnique).mockResolvedValue({ ...fakeUser, active: false } as never)
 
-    const response = await getRedirectResponse(() => verifySession(makeRequest()))
-    expect(response.headers.get('Location')).toBe('/login')
+    const response = await getRedirectResponse(() => verifySession(makeRequest('http://localhost/territories/1?x=2')))
+    expect(response.headers.get('Location')).toBe('/login?redirectTo=%2Fterritories%2F1%3Fx%3D2')
   })
 
-  it("redirige vers /login si le subdomain ne correspond pas à l'assemblée de l'utilisateur", async () => {
+  it("redirige vers /login sans redirectTo si le subdomain ne correspond pas à l'assemblée de l'utilisateur", async () => {
     mockSession.get.mockReturnValue('42')
     vi.mocked(db.userAccount.findUnique).mockResolvedValue(fakeUser as never)
     vi.mocked(resolveCongregationFromRequest).mockResolvedValue({ id: 999 } as never)
 
-    const response = await getRedirectResponse(() => verifySession(makeRequest()))
+    const response = await getRedirectResponse(() => verifySession(makeRequest('http://localhost/territories/1?x=2')))
     expect(response.headers.get('Location')).toBe('/login')
+    expect(response.headers.get('Location')).not.toContain('redirectTo')
   })
 
   it("redirige vers /suspended si l'assemblée est suspendue", async () => {
@@ -165,13 +166,13 @@ describe('verifySession', () => {
     expect(response.headers.get('Location')).toBe('/trial-expired')
   })
 
-  it('redirige vers /login si findUnique échoue avec P2007', async () => {
+  it('redirige vers /login avec redirectTo si findUnique échoue avec P2007', async () => {
     mockSession.get.mockReturnValue('42')
     const p2007Error = Object.assign(new Error('Type mismatch'), { code: 'P2007' })
     vi.mocked(db.userAccount.findUnique).mockRejectedValue(p2007Error)
 
-    const response = await getRedirectResponse(() => verifySession(makeRequest()))
-    expect(response.headers.get('Location')).toBe('/login')
+    const response = await getRedirectResponse(() => verifySession(makeRequest('http://localhost/territories/1?x=2')))
+    expect(response.headers.get('Location')).toBe('/login?redirectTo=%2Fterritories%2F1%3Fx%3D2')
   })
 
   it("redirige vers /verify-email si l'email n'est pas vérifié", async () => {
@@ -183,6 +184,14 @@ describe('verifySession', () => {
     const response = await getRedirectResponse(() => verifySession(makeRequest()))
     expect(response.headers.get('Location')).toBe('/verify-email')
   })
+
+  it('redirige vers /login sans redirectTo quand request.url est mal formée', async () => {
+    mockSession.get.mockReturnValue(undefined)
+    const badRequest = { url: 'not-a-valid-url', headers: { get: () => null } } as unknown as Request
+
+    const response = await getRedirectResponse(() => verifySession(badRequest))
+    expect(response.headers.get('Location')).toBe('/login')
+  })
 })
 
 // verifySession throws redirect responses — this helper catches them