Unbundle #165: hide Resend when sign-in can't recover + demo cookie outlasts OTP wait

.changeset/demo-cookie-outlasts-otp-wait.md

@@ -0,0 +1,11 @@
+---
+'ePDS': patch
+---
+
+Signing in through the demo no longer silently fails if you take a while to enter your emailed code.
+
+**Affects:** End users, Operators
+
+**End users:** if you requested a sign-in code, then took several minutes to fetch it from your email before entering it, the demo could fail at the last step with a generic "Authentication failed" message — even though your code was correct. That happened because the demo forgot the details of your in-progress sign-in after 10 minutes, which was shorter than the code itself stays valid. The demo now remembers your sign-in for up to an hour, and if it genuinely does time out you now see "Your sign-in took too long to finish. Please sign in again." instead of a message that made it look like you typed the code wrong.
+
+**Operators:** the demo client's `oauth_state` cookie `maxAge` is raised from `600` (10 min) to `60 * 60` (1 hour), matching the auth service's `auth_flow` row TTL — so as long as the auth service can still recover the flow, the demo can complete the token exchange. The OAuth callback now distinguishes a missing/expired state cookie (and an `invalid_grant` from the token endpoint) from a genuine auth error, redirecting to `/?error=session_expired` rather than `/?error=auth_failed`; `session_expired` renders as "Your sign-in took too long to finish. Please sign in again."

.changeset/hide-resend-when-sign-in-cannot-recover.md

@@ -0,0 +1,13 @@
+---
+'ePDS': patch
+---
+
+Sign-in no longer offers "Resend code" when the new code wouldn't have worked anyway.
+
+**Affects:** End users
+
+**End users:** Previously, if you sat on the email-code step long enough that the underlying sign-in had silently timed out (most often: leaving the tab in the background while reading email on your phone, or coming back after an interruption), the page would still show **Resend code**. Clicking it sent you a fresh email, but the moment you typed the new code you'd see "Sign in failed" — the code was issued for a sign-in that could no longer complete, so it never had a chance.
+
+The page now hides the Resend button as soon as it knows the sign-in can't be recovered, and shows **Start over** in its place. Clicking Start over takes you back to the app you came from to begin again, instead of letting you waste time on a code that couldn't work.
+
+If you're actively using the page (the tab in the foreground), nothing changes: Resend stays available and works the same way it always has.

e2e/step-definitions/auth.steps.ts

@@ -842,3 +842,92 @@
     }
   },
 )
+
+// ---------------------------------------------------------------------------
+// Resend-button visibility (fix for the "fresh OTP wasted on dead PAR" UX)
+// ---------------------------------------------------------------------------
+//
+// The page never offers an action that cannot complete the flow. When the
+// PAR is dead, the standalone Resend button is removed from view and a
+// Start over button takes its place — clicking it bails to /auth/abort
+// rather than issuing an OTP that would only fail downstream. The steps
+// below trigger the page's reactive ping (via the visibilitychange
+// handler that fires on tab-foreground) so it can observe the dead PAR
+// and reconcile the UI without a 5-minute wall-clock wait.
+
+When('the OTP form re-checks PAR liveness', async function (this: EpdsWorld) {
+  const page = getPage(this)
+  // Drive the page's reactive ping via a string-source script.
+  // Using page.evaluate(() => ...) inlines esbuild's __name helper,
+  // which then fails in Playwright's evaluation context with
+  // "ReferenceError: __name is not defined". Passing a string
+  // bypasses the bundler.
+  await page.evaluate(`(function () {
+      Object.defineProperty(document, 'visibilityState', {
+        configurable: true,
+        get: function () { return 'visible' },
+      })
+      document.dispatchEvent(new Event('visibilitychange'))
+    })()`)
+})
+
+Then(
+  'the Resend code button is no longer offered',
+  async function (this: EpdsWorld) {
+    const page = getPage(this)
+    await expect(page.locator('#btn-resend')).toBeHidden({ timeout: 5_000 })
+  },
+)
+
+Then(
+  'a Start over button is offered instead',
+  async function (this: EpdsWorld) {
+    const page = getPage(this)
+    await expect(page.locator('#btn-start-over')).toBeVisible({
+      timeout: 5_000,
+    })
+  },
+)
+
+// ---------------------------------------------------------------------------
+// Demo client cookie expiry simulation
+// ---------------------------------------------------------------------------
+//
+// The demo client stores OAuth state (state value, codeVerifier, token
+// endpoint, issuer) in a signed cookie called `oauth_state` with
+// `maxAge: 600` (see packages/demo/src/app/api/oauth/login/route.ts).
+// If the user spends longer than 10 minutes between starting the OAuth
+// flow and the callback firing — most realistic cause: dawdling on the
+// OTP form, then clicking Resend after the 10-minute mark — the cookie
+// expires before the callback runs, so the callback handler can't find
+// the OAuth state and silently bounces to /?error=auth_failed.
+//
+// This step deletes the cookie programmatically so we can exercise the
+// post-cookie-expiry callback path without a 10-minute wall-clock wait.
+
+When(
+  "the demo client's OAuth state cookie has expired",
+  async function (this: EpdsWorld) {
+    const page = getPage(this)
+    const ctx = page.context()
+    const before = await ctx.cookies()
+    const target = before.find((c) => c.name === 'oauth_state')
+    if (!target) {
+      throw new Error(
+        `Expected to find an oauth_state cookie set by the demo client but only saw: ${before.map((c) => c.name).join(', ')}`,
+      )
+    }
+    await ctx.clearCookies({ name: 'oauth_state' })
+  },
+)
+
+Then(
+  'the demo client surfaces a session-expired error',
+  async function (this: EpdsWorld) {
+    const origin = new URL(testEnv.demoUrl).origin
+    const page = getPage(this)
+    await page.waitForURL(`${origin}/?error=session_expired*`, {
+      timeout: 30_000,
+    })
+  },
+)

features/passwordless-authentication.feature

@@ -343,6 +343,59 @@ Feature: Passwordless authentication via email OTP
     And the user requests a new OTP via the resend button
     Then the browser lands back at the demo client with an auth error
 
+  # The page never offers actions that cannot complete the flow. When
+  # the upstream PAR has died (silent timeout, suspended tab,
+  # heartbeat throttling), the standalone Resend button is removed
+  # from view and replaced with a Start over button — so the user
+  # never wastes time typing a fresh OTP that could not have worked.
+  # This is the proactive complement to @resend-after-par-dead's
+  # reactive abort gate: rather than letting the click happen and
+  # bouncing it server-side, we surface only forward paths that can
+  # actually succeed.
+  @email @otp-and-par-expiry @resend-hidden-when-par-dead
+  Scenario: Resend button is hidden when the PAR has died — Start over is offered instead
+    When the demo client initiates an OAuth login
+    Then the browser is redirected to the auth service login page
+    And the login page displays an email input form
+    When the user enters a unique test email and submits
+    Then an OTP email arrives in the mail trap for the test email
+    And the login page shows an OTP verification form
+    When the PAR request_uri has expired before the bridge fires
+    And the OTP form re-checks PAR liveness
+    Then the Resend code button is no longer offered
+    And a Start over button is offered instead
+
+  # The demo OAuth client stores its OAuth state (state value, code
+  # verifier, token endpoint, issuer) in a signed cookie that has its
+  # own lifetime. If that cookie expires before the auth-service
+  # bridges the user back via /oauth/epds-callback (e.g. user
+  # dawdled on the OTP form long enough that the cookie aged out
+  # mid-flow), the demo's callback handler can't find the OAuth state
+  # and silently bounces to the demo home page with
+  # `?error=auth_failed`. The user has just typed a fresh OTP
+  # successfully, so this is genuinely time-wasting and misleading:
+  # the auth-service did everything right but the demo dropped the
+  # ball.
+  #
+  # The contract the demo MUST satisfy: as long as the OAuth flow is
+  # still recoverable from the auth-service side (auth_flow row
+  # alive, PAR alive or refreshable), the demo's session cookie must
+  # also be alive. This scenario reproduces the failure mode by
+  # programmatically clearing the demo's `oauth_state` cookie just
+  # before the OTP submission, which is equivalent to the cookie
+  # having lapsed by wall-clock.
+  @email @demo-cookie-expiry @bug-report
+  Scenario: Demo client's OAuth cookie has expired by the time of callback — useful error, not generic auth_failed
+    When the demo client starts a new OAuth flow with random handle mode
+    Then the browser is redirected to the auth service login page
+    And the login page displays an email input form
+    When the user enters a unique test email and submits
+    Then an OTP email arrives in the mail trap for the test email
+    And the login page shows an OTP verification form
+    When the demo client's OAuth state cookie has expired
+    And the user enters the OTP code
+    Then the demo client surfaces a session-expired error
+
   @email @otp-and-par-expiry @prompt-login
   Scenario: prompt=login + expired PAR — clean exit back to the OAuth client
     Given a returning user has a PDS account

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

@@ -743,17 +743,38 @@ export function renderLoginPage(opts: {
       var heartbeatEnabled = ${JSON.stringify(opts.heartbeatEnabled)};
       var heartbeatHandle = null;
       var heartbeatIntervalMs = 3 * 60 * 1000;
+      // Upstream's AUTHORIZATION_INACTIVITY_TIMEOUT — once this much
+      // wall-clock time has elapsed since our last successful PAR
+      // refresh, the upstream row is guaranteed to be dead. Used by
+      // parLikelyDead() to hide Resend before the user can click it.
+      var parInactivityTimeoutMs = 5 * 60 * 1000;
+      // Page load is the implicit first PAR refresh — atproto's
+      // PAR_EXPIRES_IN gives a fresh row 5 min on creation, and the
+      // user just hit /oauth/authorize seconds ago. Treat now as
+      // last-known-alive until the first ping confirms otherwise.
+      var lastSuccessfulHeartbeatAt = Date.now();
       // Set to true the moment we know the flow can no longer
       // complete (PAR or auth_flow gone). Resend / Verify gates
       // check this so a click that races the proactive notice
       // still bails to /auth/abort instead of issuing a fresh OTP
       // that would only fail.
       var flowAborted = false;
+      // True iff we have proof the PAR is still alive (last ping
+      // was ok:true and was recent enough to fall inside the
+      // upstream inactivity window). Used to gate every "offer the
+      // user a Resend" decision so they only ever see actions that
+      // can actually complete the flow.
+      function parLikelyDead() {
+        if (flowAborted) return true;
+        return Date.now() - lastSuccessfulHeartbeatAt >= parInactivityTimeoutMs;
+      }
       function pingHeartbeat() {
         return fetch('/auth/ping', { credentials: 'include', cache: 'no-store' })
           .then(function(r) { return r.json(); })
           .then(function(body) {
-            if (body && body.ok === false && body.reason !== 'transient') {
+            if (body && body.ok === true) {
+              lastSuccessfulHeartbeatAt = Date.now();
+            } else if (body && body.ok === false && body.reason !== 'transient') {
               // Auth flow / PAR genuinely dead — no point pinging again,
               // and no point letting the user keep typing. 'transient'
               // (5xx / network blip) does NOT stop the interval; the
@@ -763,7 +784,13 @@ export function renderLoginPage(opts: {
             }
             return body;
           })
-          .catch(function() { return null; /* network blip — caller may retry */ });
+          .catch(function() { return null; /* network blip — caller may retry */ })
+          .finally(function() {
+            // Always reconcile visibility — a 'transient' tick that
+            // pushes us past the inactivity window must hide Resend
+            // even though we never got a definitive 'par_expired'.
+            refreshResendVisibility();
+          });
       }
       function startHeartbeat() {
         if (!heartbeatEnabled) return;
@@ -777,6 +804,16 @@ export function renderLoginPage(opts: {
         }
       }
       window.addEventListener('beforeunload', stopHeartbeat);
+      // When the tab returns to the foreground after being hidden,
+      // setInterval may have been throttled enough that PAR has
+      // silently lapsed. Re-ping immediately so the UI reflects
+      // reality before the user clicks anything.
+      document.addEventListener('visibilitychange', function() {
+        if (document.visibilityState === 'visible' && heartbeatEnabled) {
+          pingHeartbeat();
+          refreshResendVisibility();
+        }
+      });
 
       // Show the proactive "this won't work — start over" notice when
       // the flow is unrecoverable. Disables the OTP boxes, the verify
@@ -819,6 +856,41 @@ export function renderLoginPage(opts: {
         errorEl.appendChild(startOverBtn);
       }
 
+      /**
+       * Toggle the standalone Resend button between visible and
+       * hidden based on whether the PAR is still alive. The button
+       * is removed from view (display:none) rather than just
+       * disabled — a button the user cannot productively click
+       * shouldn't be on the page at all. When hidden, a "Start over"
+       * link is shown in its place so the user always has a forward
+       * path. Idempotent — safe to call from heartbeat ticks,
+       * visibility change handlers, and inline render paths.
+       */
+      function refreshResendVisibility() {
+        var resendBtn = document.getElementById('btn-resend');
+        var startOverLink = document.getElementById('btn-start-over');
+        if (!resendBtn) return;
+        if (parLikelyDead()) {
+          resendBtn.style.display = 'none';
+          if (!startOverLink) {
+            startOverLink = document.createElement('button');
+            startOverLink.type = 'button';
+            startOverLink.id = 'btn-start-over';
+            startOverLink.className = 'btn-secondary';
+            startOverLink.textContent = 'Start over';
+            startOverLink.addEventListener('click', function() {
+              window.location.href = '/auth/abort';
+            });
+            resendBtn.parentNode.insertBefore(startOverLink, resendBtn);
+          }
+        } else {
+          resendBtn.style.display = '';
+          if (startOverLink && startOverLink.parentNode) {
+            startOverLink.parentNode.removeChild(startOverLink);
+          }
+        }
+      }
+
       /**
        * Reactive gate used by the Resend and Verify click handlers.
        * Pings /auth/ping synchronously; if the result indicates the
@@ -983,6 +1055,7 @@ export function renderLoginPage(opts: {
         if (otpBoxes.length) otpBoxes[0].focus();
         clearError();
         startHeartbeat();
+        refreshResendVisibility();
       }
 
       function showEmailStep() {
@@ -1104,15 +1177,21 @@ export function renderLoginPage(opts: {
             // expired") plus generic "expir"/"too long" variants.
             var isExpired = /expir|too long/i.test(result.error);
             if (isExpired) {
-              // The inline action triggers the same Resend handler,
-              // which itself runs abortIfFlowDead() before issuing
-              // a new code. So even if the PAR is dead the user
-              // gets the spec-compliant bounce rather than a fresh
-              // OTP that wouldn't work — no need to gate the
-              // action's visibility separately here.
-              showErrorWithAction(result.error, 'Send a new code', function() {
-                document.getElementById('btn-resend').click();
-              });
+              // Only offer "Send a new code" when the PAR is still
+              // alive. If it isn't, a fresh OTP would issue but
+              // never complete — wasting the user's time on a code
+              // that can't work. Show "Start over" instead so the
+              // only forward path we surface is one that will
+              // actually succeed.
+              if (parLikelyDead()) {
+                showErrorWithAction(result.error, 'Start over', function() {
+                  window.location.href = '/auth/abort';
+                });
+              } else {
+                showErrorWithAction(result.error, 'Send a new code', function() {
+                  document.getElementById('btn-resend').click();
+                });
+              }
             } else {
               showError(result.error);
             }
@@ -1185,8 +1264,10 @@ export function renderLoginPage(opts: {
           });
         }
         // OTP form is already visible server-side; showOtpStep() never
-        // ran, so kick off the heartbeat ourselves.
+        // ran, so kick off the heartbeat ourselves and reflect the
+        // current PAR-liveness state in the Resend button visibility.
         startHeartbeat();
+        refreshResendVisibility();
       }
     })();
   </script>

packages/demo/src/__tests__/session.test.ts

@@ -5,7 +5,9 @@ import {
   createUserSessionCookie,
   getUserSessionFromCookie,
   getSessionFromCookie,
+  resolveCallbackErrorCode,
   OAUTH_COOKIE,
+  OAUTH_COOKIE_MAX_AGE_SECONDS,
   SESSION_COOKIE,
 } from '../lib/session'
 import type { OAuthSession, UserSession } from '../lib/session'
@@ -92,3 +94,27 @@ describe('getSessionFromCookie', () => {
     expect(getSessionFromCookie(store)).toEqual(sampleUserSession)
   })
 })
+
+describe('OAUTH_COOKIE_MAX_AGE_SECONDS', () => {
+  it('is one hour, long enough to outlast a realistic email-code wait', () => {
+    expect(OAUTH_COOKIE_MAX_AGE_SECONDS).toBe(60 * 60)
+  })
+
+  it('outlasts the previous 10-minute value that could expire mid-flow', () => {
+    expect(OAUTH_COOKIE_MAX_AGE_SECONDS).toBeGreaterThan(600)
+  })
+})
+
+describe('resolveCallbackErrorCode', () => {
+  it('maps a missing oauth_state cookie to session_expired', () => {
+    expect(resolveCallbackErrorCode({ oauthCookiePresent: false })).toBe(
+      'session_expired',
+    )
+  })
+
+  it('maps a present cookie (any other failure) to auth_failed', () => {
+    expect(resolveCallbackErrorCode({ oauthCookiePresent: true })).toBe(
+      'auth_failed',
+    )
+  })
+})