P2: fix(auth): filter unsupported OTP characters

.changeset/charset-filter-on-otp-forms.md

@@ -0,0 +1,9 @@
+---
+'ePDS': patch
+---
+
+Sign-in code boxes now ignore stray characters pasted or typed alongside your code.
+
+**Affects:** End users
+
+**End users:** every screen that asks for a sign-in code — the main sign-in screen, Account Settings sign-in, and account recovery — now drops characters your code can't contain, so a code pasted with surrounding quotes, brackets, or a trailing full stop is accepted instead of rejected. The main sign-in screen previously ignored only spaces and line breaks. Where the operator has turned on codes that mix letters and numbers, typing them in lowercase now works too.

e2e/step-definitions/otp-character-filtering.steps.ts

@@ -0,0 +1,30 @@
+import { Then, When } from '@cucumber/cucumber'
+import { expect } from '@playwright/test'
+import { testEnv } from '../support/env.js'
+import { getPage } from '../support/utils.js'
+import type { EpdsWorld } from '../support/world.js'
+
+When(
+  'the recovery OTP preview uses the {string} character policy',
+  async function (this: EpdsWorld, charset: string) {
+    await getPage(this).goto(
+      `${testEnv.authUrl}/preview/recovery-otp?otp_charset=${encodeURIComponent(charset)}`,
+    )
+  },
+)
+
+When(
+  'the user types {string} into the recovery OTP input',
+  async function (this: EpdsWorld, value: string) {
+    await getPage(this).getByLabel('One-time code').fill(value)
+  },
+)
+
+Then(
+  'the recovery OTP input contains {string}',
+  async function (this: EpdsWorld, expected: string) {
+    await expect(getPage(this).getByLabel('One-time code')).toHaveValue(
+      expected,
+    )
+  },
+)

features/otp-character-filtering.feature

@@ -0,0 +1,16 @@
+Feature: OTP character filtering
+  Every OTP form — the segmented sign-in grid, account-login, and recovery —
+  must apply the configured OTP character policy while the user types.
+
+  Background:
+    Given the ePDS test environment is running
+
+  Scenario Outline: Recovery OTP input applies the configured character policy
+    When the recovery OTP preview uses the "<charset>" character policy
+    And the user types "<typed>" into the recovery OTP input
+    Then the recovery OTP input contains "<expected>"
+
+    Examples:
+      | charset      | typed | expected |
+      | numeric      | a1-!  | 1        |
+      | alphanumeric | a1-!  | A1       |

packages/auth-service/src/__tests__/login-page.test.ts

@@ -827,3 +827,64 @@ describe('renderLoginPage flow-aborted notice + reactive abort gates', () => {
     expect(verifyIdx).toBeGreaterThan(gateIdx)
   })
 })
+
+// The segmented OTP grid used to strip whitespace only, so a code copied
+// with surrounding punctuation, or a letter typed into a digits-only code,
+// reached the server verbatim and failed verification. It also never
+// upper-cased, while alphanumeric codes are generated as A-Z0-9 and the
+// other two OTP forms upper-case on the way in — so a desktop user typing
+// lowercase (where `autocapitalize` does nothing) submitted a code the
+// server would always reject. These pin the shared charset filter.
+describe('renderLoginPage OTP grid charset filter', () => {
+  it('emits the numeric filter from the shared helper', () => {
+    const html = renderDefault({ otpCharset: 'numeric' })
+    expect(html).toContain(`var otpCharFilter = ${/\D/g.toString()};`)
+  })
+
+  it('emits the alphanumeric filter from the shared helper', () => {
+    const html = renderDefault({ otpCharset: 'alphanumeric' })
+    expect(html).toContain(`var otpCharFilter = ${/[^A-Za-z0-9]/g.toString()};`)
+  })
+
+  it('upper-cases only under the alphanumeric policy', () => {
+    expect(renderDefault({ otpCharset: 'alphanumeric' })).toContain(
+      "otpCharset === 'alphanumeric' ? cleaned.toUpperCase() : cleaned",
+    )
+  })
+
+  it('routes the input handler through the filter, not a whitespace-only strip', () => {
+    const html = renderDefault()
+    expect(html).toContain('var v = filterOtpChars(box.value);')
+    // The old whitespace-only strip is a strict subset of every charset
+    // filter; leaving one behind would mean a handler was missed.
+    expect(html).not.toMatch(/replace\(\/\\s\/g, ''\)/)
+  })
+
+  it('routes the paste handler through the filter before slicing to the free boxes', () => {
+    const html = renderDefault()
+    expect(html).toContain(
+      'var cleaned = filterOtpChars(data).slice(0, otpBoxes.length - idx);',
+    )
+  })
+
+  it('defines the filter before the box handlers that call it', () => {
+    const html = renderDefault()
+    const defIdx = html.indexOf('function filterOtpChars(s)')
+    const inputIdx = html.indexOf('var v = filterOtpChars(box.value);')
+    const pasteIdx = html.indexOf('var cleaned = filterOtpChars(data)')
+    expect(defIdx).toBeGreaterThan(0)
+    expect(inputIdx).toBeGreaterThan(defIdx)
+    expect(pasteIdx).toBeGreaterThan(defIdx)
+  })
+
+  it('declares otpCharset once, above the filter that reads it', () => {
+    const html = renderDefault()
+    expect(html.match(/var otpCharset =/g)).toHaveLength(1)
+    expect(html.match(/var otpLength =/g)).toHaveLength(1)
+    // filterOtpChars is only ever called from event handlers, but keeping
+    // the declaration above it removes any reliance on var hoisting.
+    expect(html.indexOf('var otpCharset =')).toBeLessThan(
+      html.indexOf('function filterOtpChars(s)'),
+    )
+  })
+})

packages/auth-service/src/__tests__/otp-input.test.ts

@@ -9,7 +9,11 @@
  * 5. Alphanumeric pattern accepts both letters and digits
  */
 import { describe, it, expect } from 'vitest'
-import { buildOtpInputProps } from '../otp-input.js'
+import {
+  buildOtpInputFilter,
+  buildOtpInputProps,
+  resolvePreviewOtpCharset,
+} from '../otp-input.js'
 
 describe('Recovery flow: OTP input props', () => {
   it('numeric charset produces digit-only pattern and zero placeholder', () => {
@@ -63,3 +67,29 @@ describe('Recovery flow: OTP input props', () => {
     expect(re.test('A1B2C3D')).toBe(false) // one short
   })
 })
+
+describe('Server-rendered OTP character filter', () => {
+  it('removes unsupported characters under the numeric policy', () => {
+    expect('a1-!'.replace(buildOtpInputFilter('numeric'), '')).toBe('1')
+  })
+
+  it('preserves letters and digits under the alphanumeric policy', () => {
+    expect('a1-!'.replace(buildOtpInputFilter('alphanumeric'), '')).toBe('a1')
+  })
+})
+
+describe('Recovery preview: OTP charset override', () => {
+  it.each(['numeric', 'alphanumeric'] as const)(
+    'uses a supported %s override',
+    (requested) => {
+      expect(resolvePreviewOtpCharset(requested, 'numeric')).toBe(requested)
+    },
+  )
+
+  it('falls back to the configured policy for unsupported values', () => {
+    expect(resolvePreviewOtpCharset('unsupported', 'alphanumeric')).toBe(
+      'alphanumeric',
+    )
+    expect(resolvePreviewOtpCharset(undefined, 'numeric')).toBe('numeric')
+  })
+})

packages/auth-service/src/otp-input.ts

@@ -5,6 +5,8 @@
  * logic, and so the logic can be unit-tested without rendering or parsing HTML.
  */
 
+export type OtpCharset = 'numeric' | 'alphanumeric'
+
 export interface OtpInputProps {
   pattern: string
   placeholder: string
@@ -14,7 +16,7 @@ export interface OtpInputProps {
 
 export function buildOtpInputProps(
   otpLength: number,
-  otpCharset: 'numeric' | 'alphanumeric',
+  otpCharset: OtpCharset,
 ): OtpInputProps {
   if (otpCharset === 'alphanumeric') {
     return {
@@ -31,3 +33,18 @@ export function buildOtpInputProps(
     autocapitalize: 'off',
   }
 }
+
+/** Build the character-removal pattern shared by server-rendered OTP inputs. */
+export function buildOtpInputFilter(otpCharset: OtpCharset): RegExp {
+  return otpCharset === 'alphanumeric' ? /[^A-Za-z0-9]/g : /\D/g
+}
+
+/** Accept a supported preview override, otherwise preserve the configured policy. */
+export function resolvePreviewOtpCharset(
+  requested: string | undefined,
+  configured: OtpCharset,
+): OtpCharset {
+  return requested === 'numeric' || requested === 'alphanumeric'
+    ? requested
+    : configured
+}

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

@@ -17,7 +17,7 @@ import { Router, type Request, type Response } from 'express'
 import { escapeHtml, maskEmail, createLogger } from '@certified-app/shared'
 import { fromNodeHeaders } from 'better-auth/node'
 import type { AuthServiceContext } from '../context.js'
-import { buildOtpInputProps } from '../otp-input.js'
+import { buildOtpInputFilter, buildOtpInputProps } from '../otp-input.js'
 import type { BetterAuthInstance } from '../better-auth.js'
 import { POWERED_BY_CSS, POWERED_BY_HTML } from '../lib/page-helpers.js'
 import {
@@ -185,6 +185,7 @@ function renderOtpForm(opts: {
 }): string {
   const maskedEmail = maskEmail(opts.email)
   const inputProps = buildOtpInputProps(opts.otpLength, opts.otpCharset)
+  const inputFilter = buildOtpInputFilter(opts.otpCharset)
 
   return `<!DOCTYPE html>
 <html lang="en">
@@ -216,6 +217,7 @@ function renderOtpForm(opts: {
                  autocapitalize="${inputProps.autocapitalize}"
                  placeholder="${inputProps.placeholder}"
                  class="otp-input"
+                 oninput="this.value=this.value.replace(${inputFilter.toString()},'')${opts.otpCharset === 'alphanumeric' ? '.toUpperCase()' : ''}"
                  style="letter-spacing: ${Math.max(2, Math.round(32 / opts.otpLength))}px">
         </div>
         <button type="submit" class="btn-primary">Verify</button>

packages/auth-service/src/routes/login-page.ts

@@ -42,7 +42,7 @@ import {
   type HandleMode,
 } from '@certified-app/shared'
 import { socialProviders } from '../better-auth.js'
-import { buildOtpInputProps } from '../otp-input.js'
+import { buildOtpInputFilter, buildOtpInputProps } from '../otp-input.js'
 import {
   resolveLoginHint,
   fetchParLoginHint,
@@ -514,6 +514,7 @@ export function renderLoginPage(opts: {
     : `<img src="/static/certified-brandmark.svg" alt="Certified" class="client-logo">`
 
   const inputProps = buildOtpInputProps(opts.otpLength, opts.otpCharset)
+  const inputFilter = buildOtpInputFilter(opts.otpCharset)
 
   // ATProto/Bluesky handle login button.
   //
@@ -763,6 +764,8 @@ export function renderLoginPage(opts: {
       var termsEl = document.getElementById('terms');
       var otpBoxes = Array.prototype.slice.call(document.querySelectorAll('.otp-box'));
       var hiddenCode = document.getElementById('code');
+      var otpLength = ${opts.otpLength};
+      var otpCharset = ${JSON.stringify(opts.otpCharset)};
 
       // PAR heartbeat — slides the upstream request_uri inactivity
       // timer (atproto's AUTHORIZATION_INACTIVITY_TIMEOUT, 5 min) so
@@ -970,6 +973,19 @@ export function renderLoginPage(opts: {
         return false;
       }
 
+      // Drop characters the configured code alphabet can't contain, so a
+      // code copied out of prose (punctuation, line breaks, a stray letter
+      // in a digits-only code) still lands in the boxes as a valid code
+      // instead of silently failing verification. Alphanumeric codes are
+      // generated uppercase, and the browser only auto-capitalises on soft
+      // keyboards, so normalise here too — otherwise a desktop user typing
+      // lowercase submits a code the server will reject.
+      var otpCharFilter = ${inputFilter.toString()};
+      function filterOtpChars(s) {
+        var cleaned = s.replace(otpCharFilter, '');
+        return otpCharset === 'alphanumeric' ? cleaned.toUpperCase() : cleaned;
+      }
+
       function updateHiddenCode() {
         var v = '';
         for (var i = 0; i < otpBoxes.length; i++) v += otpBoxes[i].value;
@@ -984,7 +1000,7 @@ export function renderLoginPage(opts: {
       otpBoxes.forEach(function(box, idx) {
         box.addEventListener('input', function() {
           // keep only the last typed char (handles paste into a single box)
-          var v = box.value.replace(/\\s/g, '');
+          var v = filterOtpChars(box.value);
           if (v.length > 1) v = v.slice(-1);
           box.value = v;
           updateHiddenCode();
@@ -1008,7 +1024,7 @@ export function renderLoginPage(opts: {
         box.addEventListener('paste', function(e) {
           e.preventDefault();
           var data = (e.clipboardData || window.clipboardData).getData('text') || '';
-          var cleaned = data.replace(/\\s/g, '').slice(0, otpBoxes.length - idx);
+          var cleaned = filterOtpChars(data).slice(0, otpBoxes.length - idx);
           for (var i = 0; i < cleaned.length; i++) otpBoxes[idx + i].value = cleaned[i];
           updateHiddenCode();
           var nextIdx = Math.min(idx + cleaned.length, otpBoxes.length - 1);
@@ -1161,8 +1177,6 @@ export function renderLoginPage(opts: {
         });
       }
 
-      var otpLength = ${opts.otpLength};
-      var otpCharset = ${JSON.stringify(opts.otpCharset)};
       function showOtpStep(email) {
         currentEmail = email;
         otpEmailInput.value = email;

packages/auth-service/src/routes/preview.ts

@@ -28,6 +28,7 @@
 import { Router, type Request, type Response } from 'express'
 import { randomBytes } from 'node:crypto'
 import type { AuthServiceContext } from '../context.js'
+import { resolvePreviewOtpCharset } from '../otp-input.js'
 import {
   resolveClientMetadata,
   getClientCss,
@@ -307,14 +308,18 @@ export function createPreviewRouter(ctx: AuthServiceContext): Router {
 
   router.get('/preview/recovery-otp', async (req: Request, res: Response) => {
     const { css, faviconUrl, faviconUrlDark } = await getBranding(req)
+    const otpCharset = resolvePreviewOtpCharset(
+      queryString(req, 'otp_charset'),
+      ctx.config.otpCharset,
+    )
     sendHtml(
       res,
       renderRecoveryOtpForm({
         email: FAKE_EMAIL,
         csrfToken: fakeCsrfToken(),
         requestUri: FAKE_REQUEST_URI,
         otpLength: ctx.config.otpLength,
-        otpCharset: ctx.config.otpCharset,
+        otpCharset,
         error: queryString(req, 'error'),
         customCss: css,
         customFaviconUrl: faviconUrl,

packages/auth-service/src/routes/recovery.ts

@@ -17,7 +17,7 @@
 import { Router, type Request, type Response } from 'express'
 import type { AuthServiceContext } from '../context.js'
 import { createLogger, escapeHtml, maskEmail } from '@certified-app/shared'
-import { buildOtpInputProps } from '../otp-input.js'
+import { buildOtpInputFilter, buildOtpInputProps } from '../otp-input.js'
 import { resolveClientBranding } from '../lib/client-metadata.js'
 import {
   renderOptionalStyleTag,
@@ -412,6 +412,7 @@ export function renderRecoveryOtpForm(opts: {
     ? `/oauth/authorize?request_uri=${encodeURIComponent(requestUriForBack)}`
     : '/oauth/authorize'
   const inputProps = buildOtpInputProps(opts.otpLength, opts.otpCharset)
+  const inputFilter = buildOtpInputFilter(opts.otpCharset)
   // Forward the heartbeat-disabled flag through Resend (POST
   // /auth/recover) so the re-rendered OTP form keeps it disabled.
   // Verify (POST /auth/recover/verify) doesn't re-render this form,
@@ -452,7 +453,7 @@ export function renderRecoveryOtpForm(opts: {
                  autocapitalize="${inputProps.autocapitalize}"
                  placeholder="${inputProps.placeholder}"
                  class="otp-input"
-                  oninput="this.value=this.value.replace(/[\\s-]/g,'')"
+                 oninput="this.value=this.value.replace(${inputFilter.toString()},'')${opts.otpCharset === 'alphanumeric' ? '.toUpperCase()' : ''}"
                  style="letter-spacing: ${Math.max(2, Math.round(32 / opts.otpLength))}px">
         </div>
         <button type="submit" class="btn-primary">Verify</button>