P0: fix(auth): show account-settings action results

.changeset/account-settings-flash-messages.md

@@ -0,0 +1,9 @@
+---
+'ePDS': patch
+---
+
+Account Settings now confirms every action with a visible banner.
+
+**Affects:** End users
+
+**End users:** when you added a backup email, removed one, changed your handle, revoked a session, or hit a validation error on any of those, the page silently bounced back to the same form with no indication that anything had changed (or what went wrong). The page now shows a green "Backup email added" / "Handle updated" / etc. banner on success, and a red "That handle is not available" / "We couldn't send the verification email" / etc. banner on error — so you know whether your action took effect.

packages/auth-service/src/__tests__/account-settings-flash.test.ts

@@ -0,0 +1,145 @@
+import { describe, it, expect } from 'vitest'
+import {
+  FLASH_SUCCESS_MESSAGES,
+  FLASH_ERROR_MESSAGES,
+  resolveAccountFlashFromQuery,
+  renderSettingsPage,
+} from '../routes/account-settings.js'
+
+describe('account-settings flash messages', () => {
+  // Every code referenced in a redirect from a POST handler needs an
+  // entry in the matching lookup, otherwise the user lands on the
+  // settings page with no acknowledgement of the action they took.
+  // These tests guard against drift — adding a new code without a
+  // corresponding entry would mean the redirect silently dropped.
+
+  it('has a success message for each code redirected to from a POST handler', () => {
+    const expectedSuccessCodes = [
+      'backup_added',
+      'backup_verified',
+      'backup_removed',
+      'handle_updated',
+      'session_revoked',
+    ]
+    for (const code of expectedSuccessCodes) {
+      expect(
+        FLASH_SUCCESS_MESSAGES[code],
+        `missing FLASH_SUCCESS_MESSAGES["${code}"]`,
+      ).toBeTruthy()
+    }
+  })
+
+  it('has an error message for each code redirected to from a POST handler', () => {
+    const expectedErrorCodes = [
+      'invalid_email',
+      'already_primary',
+      'account_not_found',
+      'send_failed',
+      'backup_remove_failed',
+      'revoke_failed',
+      'verify_failed',
+      'invalid_handle',
+      'handle_failed',
+      'handle_taken',
+      'delete_failed',
+      'confirm_delete',
+    ]
+    for (const code of expectedErrorCodes) {
+      expect(
+        FLASH_ERROR_MESSAGES[code],
+        `missing FLASH_ERROR_MESSAGES["${code}"]`,
+      ).toBeTruthy()
+    }
+  })
+
+  it('returns undefined for unknown codes — the GET handler treats this as "no banner"', () => {
+    expect(FLASH_SUCCESS_MESSAGES['attacker_injected_text']).toBeUndefined()
+    expect(FLASH_ERROR_MESSAGES['<script>alert(1)</script>']).toBeUndefined()
+  })
+
+  it('all messages are plain text, no HTML', () => {
+    // The renderer escapeHtml's the lookup result anyway (defence in
+    // depth) but the values themselves shouldn't carry markup.
+    for (const msg of Object.values(FLASH_SUCCESS_MESSAGES)) {
+      expect(msg).not.toMatch(/<[a-z]/i)
+    }
+    for (const msg of Object.values(FLASH_ERROR_MESSAGES)) {
+      expect(msg).not.toMatch(/<[a-z]/i)
+    }
+  })
+})
+
+describe('account-settings flash rendering', () => {
+  const renderPage = (messages: {
+    successMessage?: string
+    errorMessage?: string
+  }) =>
+    renderSettingsPage({
+      did: 'did:plc:test',
+      email: 'primary@example.com',
+      handleDomain: 'example.com',
+      currentHandle: null,
+      backupEmails: [],
+      sessions: [],
+      currentSessionToken: 'current-session',
+      csrfToken: 'csrf-token',
+      ...messages,
+    })
+
+  it('renders successful actions as an accessible status', () => {
+    const html = renderPage({ successMessage: 'Backup email removed.' })
+    expect(html).toContain(
+      '<div class="flash flash-success" role="status">Backup email removed.</div>',
+    )
+  })
+
+  it('renders escaped failures as an alert', () => {
+    const html = renderPage({ errorMessage: '<script>failed</script>' })
+    expect(html).toContain(
+      '<div class="flash flash-error" role="alert">&lt;script&gt;failed&lt;/script&gt;</div>',
+    )
+    expect(html).not.toContain('<script>failed</script>')
+  })
+})
+
+describe('resolveAccountFlashFromQuery', () => {
+  it('returns the success message when the success code is known', () => {
+    const result = resolveAccountFlashFromQuery({ success: 'backup_added' })
+    expect(result.successMessage).toBe(FLASH_SUCCESS_MESSAGES.backup_added)
+    expect(result.errorMessage).toBeNull()
+  })
+
+  it('returns the error message when the error code is known', () => {
+    const result = resolveAccountFlashFromQuery({
+      success: '',
+      error: 'invalid_handle',
+    })
+    expect(result.successMessage).toBeNull()
+    expect(result.errorMessage).toBe(FLASH_ERROR_MESSAGES.invalid_handle)
+  })
+
+  it('returns null on both sides when the query is empty', () => {
+    expect(resolveAccountFlashFromQuery({})).toEqual({
+      successMessage: null,
+      errorMessage: null,
+    })
+  })
+
+  it('returns null on both sides for unknown codes (safety against URL-injection of attacker text)', () => {
+    expect(
+      resolveAccountFlashFromQuery({
+        success: 'attacker_chosen_text',
+        error: '<script>alert(1)</script>',
+      }),
+    ).toEqual({ successMessage: null, errorMessage: null })
+  })
+
+  it('ignores non-string query values gracefully (e.g. ?success=foo&success=bar arrays)', () => {
+    expect(
+      resolveAccountFlashFromQuery({
+        success: ['backup_added', 'backup_removed'],
+        error: 12345,
+      }),
+    ).toEqual({ successMessage: null, errorMessage: null })
+  })
+})

packages/auth-service/src/routes/account-settings.ts

@@ -16,6 +16,71 @@ import { POWERED_BY_CSS, POWERED_BY_HTML } from '../lib/page-helpers.js'
 
 const logger = createLogger('auth:account-settings')
 
+/**
+ * User-visible flash-message lookup tables for the GET /account
+ * page. The POST handlers redirect back with `?success=<code>` or
+ * `?error=<code>` and the GET handler renders the matching message.
+ *
+ * Whitelisted lookup (not `?success=Some+raw+text`) so attackers
+ * can't inject arbitrary text into the settings page via a crafted
+ * URL.
+ *
+ * Exported for unit testing.
+ */
+export const FLASH_SUCCESS_MESSAGES: Record<string, string> = {
+  backup_added:
+    'Backup email added. Click the verification link we sent to confirm.',
+  backup_verified: 'Backup email verified.',
+  backup_removed: 'Backup email removed.',
+  handle_updated: 'Handle updated.',
+  session_revoked: 'Session revoked.',
+}
+
+export const FLASH_ERROR_MESSAGES: Record<string, string> = {
+  invalid_email: 'That email address is not valid.',
+  already_primary:
+    'That email is already your primary — you can sign in with it directly.',
+  account_not_found: "We couldn't find an account for your sign-in.",
+  send_failed:
+    "We couldn't send the verification email. Please try again in a moment.",
+  backup_remove_failed:
+    "We couldn't remove that backup email. Please try again in a moment.",
+  revoke_failed:
+    "We couldn't revoke that session. Please try again in a moment.",
+  verify_failed:
+    'That verification link is no longer valid. Add the backup email again to receive a fresh link.',
+  invalid_handle:
+    "That handle isn't valid. Use 5–20 characters: letters, numbers, or hyphens.",
+  handle_failed:
+    "We couldn't change your handle. It may be reserved or already taken.",
+  delete_failed:
+    "We couldn't delete your account right now. Please try again in a moment.",
+  handle_taken: 'That handle is not available — please choose another.',
+  confirm_delete:
+    'Type DELETE in the confirmation box to permanently delete your account.',
+}
+
+/**
+ * Resolve the success / error message pair from a /account request's
+ * query params. Whitelist the keys via the FLASH_*_MESSAGES tables so
+ * an attacker can't craft a URL like
+ * `/account?error=Some+raw+text` to inject arbitrary copy.
+ *
+ * Exported so the route handler can stay declarative AND be unit-
+ * tested without needing a fake express request.
+ */
+export function resolveAccountFlashFromQuery(query: {
+  success?: unknown
+  error?: unknown
+}): { successMessage: string | null; errorMessage: string | null } {
+  const successCode = typeof query.success === 'string' ? query.success : ''
+  const errorCode = typeof query.error === 'string' ? query.error : ''
+  return {
+    successMessage: FLASH_SUCCESS_MESSAGES[successCode] ?? null,
+    errorMessage: FLASH_ERROR_MESSAGES[errorCode] ?? null,
+  }
+}
+
 /**
  * Middleware that validates a better-auth session and injects it into res.locals.
  * If not authenticated, redirects to /account/login.
@@ -80,6 +145,14 @@ export function createAccountSettingsRouter(
       logger.warn({ err }, 'Failed to list sessions')
     }
 
+    // The POST handlers below redirect back to /account with a
+    // ?success=… or ?error=… query param to confirm the action.
+    // resolveAccountFlashFromQuery whitelists the recognised codes
+    // so an attacker can't craft a URL that injects arbitrary text.
+    const { successMessage, errorMessage } = resolveAccountFlashFromQuery(
+      req.query as { success?: unknown; error?: unknown },
+    )
+
     res.type('html').send(
       renderSettingsPage({
         did: did ?? '(unknown)',
@@ -90,6 +163,8 @@ export function createAccountSettingsRouter(
         sessions,
         currentSessionToken: session.session.token,
         csrfToken: res.locals.csrfToken,
+        successMessage,
+        errorMessage,
       }),
     )
   })
@@ -209,15 +284,26 @@ export function createAccountSettingsRouter(
     async (req: Request, res: Response) => {
       const session = res.locals.betterAuthSession
       const email = ((req.body.email as string) || '').trim().toLowerCase()
+      if (!email) {
+        res.redirect(303, '/account?error=invalid_email')
+        return
+      }
       const did = await getDidByEmail(
         session.user.email,
         pdsUrl,
         internalSecret,
       )
-      if (did && email) {
+      if (!did) {
+        res.redirect(303, '/account?error=account_not_found')
+        return
+      }
+      try {
         ctx.db.removeBackupEmail(did, email)
+        res.redirect(303, '/account?success=backup_removed')
+      } catch (err) {
+        logger.error({ err }, 'Failed to remove backup email')
+        res.redirect(303, '/account?error=backup_remove_failed')
       }
-      res.redirect(303, '/account')
     },
   )
 
@@ -227,17 +313,20 @@ export function createAccountSettingsRouter(
     requireAuth,
     async (req: Request, res: Response) => {
       const tokenToRevoke = req.body.session_token as string
-      if (tokenToRevoke) {
-        try {
-          await auth.api.revokeSession({
-            body: { token: tokenToRevoke },
-            headers: fromNodeHeaders(req.headers),
-          })
-        } catch (err) {
-          logger.warn({ err }, 'Failed to revoke session')
-        }
+      if (!tokenToRevoke) {
+        res.redirect(303, '/account?error=revoke_failed')
+        return
+      }
+      try {
+        await auth.api.revokeSession({
+          body: { token: tokenToRevoke },
+          headers: fromNodeHeaders(req.headers),
+        })
+        res.redirect(303, '/account?success=session_revoked')
+      } catch (err) {
+        logger.warn({ err }, 'Failed to revoke session')
+        res.redirect(303, '/account?error=revoke_failed')
       }
-      res.redirect(303, '/account')
     },
   )
 
@@ -413,7 +502,7 @@ export function createAccountSettingsRouter(
   return router
 }
 
-function renderSettingsPage(opts: {
+export function renderSettingsPage(opts: {
   did: string
   email: string
   handleDomain: string
@@ -427,7 +516,21 @@ function renderSettingsPage(opts: {
   }>
   currentSessionToken: string
   csrfToken: string
+  successMessage?: string | null
+  errorMessage?: string | null
 }): string {
+  // Prepend any flash banners INSIDE the existing page-wrap so they
+  // sit above the settings content. Both go through escapeHtml even
+  // though the source dictionary is whitelisted — defence-in-depth
+  // against a future maintainer accidentally widening the lookup.
+  const flashHtml = [
+    opts.successMessage
+      ? `<div class="flash flash-success" role="status">${escapeHtml(opts.successMessage)}</div>`
+      : '',
+    opts.errorMessage
+      ? `<div class="flash flash-error" role="alert">${escapeHtml(opts.errorMessage)}</div>`
+      : '',
+  ].join('')
   const backupRows = opts.backupEmails
     .map(
       (be) => `
@@ -489,6 +592,8 @@ function renderSettingsPage(opts: {
       </form>
     </div>
 
+    ${flashHtml}
+
     <section class="section">
       <h2>Account</h2>
       <div class="setting-row"><strong>DID:</strong> <code>${escapeHtml(opts.did)}</code></div>
@@ -589,6 +694,9 @@ const SETTINGS_CSS = `
   .header { display: flex; justify-content: space-between; align-items: center; margin-bottom: 24px; border-bottom: 1px solid #eee; padding-bottom: 16px; }
   h1 { font-size: 24px; color: #111; }
   h2 { font-size: 18px; color: #333; margin-bottom: 12px; }
+  .flash { padding: 12px 16px; border-radius: 8px; margin-bottom: 20px; font-size: 14px; }
+  .flash-success { background: #f0fff4; color: #28a745; border: 1px solid #c3e6cb; }
+  .flash-error { background: #fdf0f0; color: #dc3545; border: 1px solid #f5c6cb; }
   .section { margin-bottom: 28px; padding-bottom: 20px; border-bottom: 1px solid #f0f0f0; }
   .section:last-child { border-bottom: none; margin-bottom: 0; }
   .setting-row { display: flex; justify-content: space-between; align-items: center; padding: 8px 0; font-size: 14px; color: #333; }

vitest.config.ts

@@ -24,7 +24,7 @@ export default defineConfig({
       // See AGENTS.md for the ratcheting policy.
       thresholds: {
         statements: 58,
-        branches: 57,
+        branches: 58,
         functions: 71,
         lines: 57,
       },