fix(auth-service): stop "Invalid OTP" flash on successful sign-in

.changeset/otp-verify-double-submit-flash.md

@@ -0,0 +1,15 @@
+---
+'ePDS': patch
+---
+
+A smoother sign-in code experience: no false error flash on a successful sign-in, no rapid-fire failures when correcting a wrong code, and tidier-looking banners.
+
+**Affects:** End users, Client app developers
+
+**End users:**
+
+- A successful sign-in no longer briefly shows a red "Invalid OTP" message on its way to signing you in.
+- After entering a wrong code, the boxes clear and focus jumps back to the first one, so retyping doesn't immediately resubmit the still-wrong code on every keystroke (which previously could lock you out for spamming the server).
+- The red "Invalid OTP" and green "Code resent" banners are centred inside their coloured container instead of sitting in the corner of an empty wide box.
+
+**Client app developers:** the sign-in page's flash-message container now uses a stable `flash-msg` base class with `error` / `success` modifier classes, so custom client CSS can restyle either variant cleanly via `.flash-msg`, `.flash-msg.error`, and `.flash-msg.success`.

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

@@ -485,3 +485,92 @@ describe('renderLoginPage handle login button', () => {
     expect(html).not.toContain(BUTTON_HTML)
   })
 })
+
+// Regression: the segmented OTP input auto-submits the verify form when the
+// last digit lands (paste handler at the same site). If a second submit
+// fires while the first is in flight — Enter after typing, OTP autofill
+// dispatching input on every box, paste+input pair on some browsers — the
+// second call hits /sign-in/email-otp with a now-consumed code, the
+// response is "Invalid OTP", and that error renders briefly before the
+// success-path redirect to /auth/complete unloads the page. The visible
+// symptom is a red "Invalid OTP" flash followed by a successful login.
+//
+// The fix is an in-flight latch in the verify-form submit handler. These
+// tests pin its structure so accidental refactors (removing the guard,
+// moving it after the fetch, resetting the flag unconditionally on
+// success) fail loudly.
+function renderDefault(): string {
+  return renderLoginPage({
+    flowId: 'flow-1',
+    clientId: 'https://example.com/client-metadata.json',
+    clientName: 'Example',
+    branding: {},
+    customCss: null,
+    customFaviconUrl: null,
+    customFaviconUrlDark: null,
+    loginHint: '',
+    initialStep: 'email',
+    otpAlreadySent: false,
+    csrfToken: 'csrf',
+    authBasePath: '/api/auth',
+    pdsPublicUrl: 'https://pds.example.com',
+    otpLength: 6,
+    otpCharset: 'numeric',
+  })
+}
+
+describe('renderLoginPage OTP verify-form double-submit latch (regression)', () => {
+  it('declares the verifying flag at IIFE scope so input/paste/submit handlers share it', () => {
+    const html = renderDefault()
+    expect(html).toContain('var verifying = false;')
+    // Exactly one declaration — a second one would shadow the shared flag.
+    expect(html.match(/var verifying =/g)).toHaveLength(1)
+  })
+
+  it('guards the verify-form submit handler before any in-flight state is touched', () => {
+    const html = renderDefault()
+    // Order matters: the guard must short-circuit BEFORE we set
+    // verifying=true and BEFORE the verifyOtp() call. A guard placed
+    // after the fetch would not prevent a second request.
+    const guardIdx = html.indexOf('if (verifying) return;')
+    const setTrueIdx = html.indexOf('verifying = true;')
+    const verifyCallIdx = html.indexOf('await verifyOtp(currentEmail, otp)')
+    expect(guardIdx).toBeGreaterThan(0)
+    expect(setTrueIdx).toBeGreaterThan(guardIdx)
+    expect(verifyCallIdx).toBeGreaterThan(setTrueIdx)
+  })
+
+  it('resets the latch only on the error path, not on success', () => {
+    const html = renderDefault()
+    // The reset is wrapped in `if (!result || result.error) { ... }`. An
+    // unconditional reset would re-open the form during the post-success
+    // navigation and let a late input/Enter event fire a second verify
+    // on the consumed OTP — exactly the bug being prevented.
+    expect(html).toMatch(
+      /if \(!result \|\| result\.error\)\s*\{\s*verifying = false;/,
+    )
+    // And there is exactly one place that sets the flag back to false (in
+    // the error branch). A second `verifying = false` somewhere else
+    // would defeat the latch.
+    const resetCount = html.split('verifying = false;').length - 1
+    expect(resetCount).toBe(2) // initial declaration + single reset
+  })
+
+  it('clears the OTP boxes on verify error so re-entry does not auto-spam', () => {
+    const html = renderDefault()
+    // Without clearing, the boxes stay full at length 6 after an invalid
+    // code. The auto-submit handler fires whenever total length === 6, so
+    // the next keystroke (replacing one wrong digit) would immediately
+    // trigger another verify, again with a still-wrong code, on every
+    // edit — easily tripping the per-IP rate limiter.
+    const branchStart = html.indexOf('if (result && result.error) {')
+    expect(branchStart).toBeGreaterThan(0)
+    // The next `verifying = false;` (in the `finally` block) bounds the
+    // error branch — bounded slice, no unbounded regex backtracking.
+    const branchEnd = html.indexOf('verifying = false;', branchStart)
+    expect(branchEnd).toBeGreaterThan(branchStart)
+    const branch = html.slice(branchStart, branchEnd)
+    expect(branch).toContain('showError(result.error);')
+    expect(branch).toContain('clearOtpBoxes();')
+  })
+})

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

@@ -571,7 +571,9 @@ export function renderLoginPage(opts: {
     .btn-atproto { margin-top: 12px; margin-bottom: 0; color: #1A130F !important; background: var(--input-bg) !important; border-color: var(--input-border) !important; }
     .divider { display: flex; align-items: center; gap: 12px; margin: 20px 0; color: #999; font-size: 13px; }
     .divider::before, .divider::after { content: ''; flex: 1; height: 1px; background: #ececec; }
-    .error { color: #dc3545; background: #fdf0f0; padding: 12px; border-radius: 10px; margin: 12px 0; font-size: 14px; text-align: left; }
+    .flash-msg { padding: 12px; border-radius: 10px; margin: 12px 0; font-size: 14px; text-align: center; }
+    .flash-msg.error { color: #dc3545; background: #fdf0f0; }
+    .flash-msg.success { color: #28a745; background: #f0fff4; }
     .step-otp { display: none; }
     .step-otp.active { display: block; }
     .step-email.hidden { display: none; }
@@ -593,7 +595,7 @@ export function renderLoginPage(opts: {
     ${logoHtml}
     <h1 id="heading">${opts.initialStep === 'otp' ? 'Enter your code' : 'Sign in'}</h1>
 
-    <div id="error-msg" class="error" style="display:none;"></div>
+    <div id="error-msg" class="flash-msg" style="display:none;"></div>
 
     ${socialButtonsHtml}
 
@@ -659,6 +661,7 @@ export function renderLoginPage(opts: {
       var requestUri = ${JSON.stringify('')};  // not needed client-side; flow_id is in cookie
       var currentEmail = '';
       var loginMode = 'email'; // 'email' | 'handle'
+      var verifying = false;
       var errorEl = document.getElementById('error-msg');
       var stepEmail = document.getElementById('step-email');
       var stepOtp = document.getElementById('step-otp');
@@ -723,14 +726,20 @@ export function renderLoginPage(opts: {
         box.addEventListener('focus', function() { box.select(); });
       });
 
-      function showError(msg) {
+      function showFlash(msg, kind) {
         errorEl.textContent = msg;
+        errorEl.classList.remove('error', 'success');
+        errorEl.classList.add(kind);
         errorEl.style.display = 'block';
       }
 
+      function showError(msg) { showFlash(msg, 'error'); }
+      function showSuccess(msg) { showFlash(msg, 'success'); }
+
       function clearError() {
         errorEl.style.display = 'none';
         errorEl.textContent = '';
+        errorEl.classList.remove('error', 'success');
       }
 
       function setLoginMode(mode) {
@@ -867,18 +876,36 @@ export function renderLoginPage(opts: {
       // Form: verify OTP
       document.getElementById('form-verify-otp').addEventListener('submit', async function(e) {
         e.preventDefault();
+        // Collapse duplicate submits (auto-submit on 6th digit + Enter, OTP
+        // autofill firing input on every box, paste+input pair, etc.). The
+        // first call consumes the code; a second one races the redirect and
+        // flashes "Invalid OTP" before the page unloads.
+        if (verifying) return;
+        verifying = true;
         clearError();
         var otp = document.getElementById('code').value.trim();
         var btn = this.querySelector('button[type=submit]');
         btn.disabled = true;
         btn.textContent = 'Verifying...';
 
-        var result = await verifyOtp(currentEmail, otp);
-        btn.disabled = false;
-        btn.textContent = 'Verify';
-
-        if (result && result.error) {
-          showError(result.error);
+        try {
+          var result = await verifyOtp(currentEmail, otp);
+          if (result && result.error) {
+            showError(result.error);
+            // Clear the boxes so the user re-enters all 6 digits. Editing
+            // a still-full grid would auto-submit on the first keystroke
+            // (length stays at 6) and spam the rate limiter.
+            clearOtpBoxes();
+            if (otpBoxes.length) otpBoxes[0].focus();
+          }
+        } finally {
+          // Leave the latch set on success: verifyOtp triggers a redirect,
+          // and we don't want late events to re-open the form mid-navigation.
+          if (!result || result.error) {
+            verifying = false;
+            btn.disabled = false;
+            btn.textContent = 'Verify';
+          }
         }
       });
 
@@ -893,9 +920,7 @@ export function renderLoginPage(opts: {
         if (result.error) {
           showError(result.error);
         } else {
-          showError('Code resent!');
-          errorEl.style.color = '#28a745';
-          errorEl.style.background = '#f0fff4';
+          showSuccess('Code resent!');
         }
       });