Skip to content

fix(login): un-break sign-in — right-size the timeouts, 503 for outages, and make the slow hop visible - #455

Merged
izzywdev merged 5 commits into
masterfrom
claude/signin-timeout-error-2avs7q
Jul 29, 2026
Merged

fix(login): un-break sign-in — right-size the timeouts, 503 for outages, and make the slow hop visible#455
izzywdev merged 5 commits into
masterfrom
claude/signin-timeout-error-2avs7q

Conversation

@izzywdev

@izzywdev izzywdev commented Jul 29, 2026

Copy link
Copy Markdown
Owner

📋 Description

Sign-in at https://app.fuzefront.com/login fails with a bare AxiosError: timeout of 15000ms exceeded — no HTTP status, no message, nothing actionable. Four commits: un-break it, then fix the two design faults that made it break and made it undiagnosable, then instrument the underlying slowness so it can actually be found.

1. The timeouts were sized to the ideal path, not the real one — this is the outage

The client bound was 15s, added in #371 to fail fast rather than leave the button on "Signing in…". That traded the wrong way. The documented slow path is 16–30s, so a 15s bound did not fail slow sign-ins fast — it failed sign-ins that would otherwise have succeeded. A bound below the known worst case is not a safety net, it is an outage.

Client 15s → 45s, server budget → 40s. LoginPage already shows a "still working…" hint at 8s. Both env-overridable so they can be tightened again, without a rebuild, once the slow hop is fixed.

2. Nothing bounded the chain — only each individual link

A login is many sequential hops: 2–3 flow-executor stages (each following up to 10 redirects), the authorize→code chain (up to 10 more), then openid-client's token exchange + userinfo.

Bound Scope Was
AUTHENTIK_FLOW_TIMEOUT_MS per individual fetch, fresh each hop 10s
OIDC_HTTP_TIMEOUT_MS per openid-client call, ×2 15s
server total none
LOGIN_TIMEOUT_MS (browser) the whole call 15s

Every hop got a fresh full allowance, so the server's budget exceeded the client's many times over and the client always aborted first. That is why the failure carries no status and no message: the server was still working, and its labelled diagnostics never got produced. Undiagnosable by construction.

Adds a Deadline shared by every hop of one attempt — each hop gets min(per-hop cap, time remaining), and the token exchange is held to what's left, so the server now answers inside the browser's window with a stage-labelled error.

Also fixes a connection leak. undici keeps a socket checked out until the body is consumed or cancelled. Redirect hops read only location, and authorize hops never read a body at all — so each hop pinned its connection and later hops queued behind their own predecessors. Bodies are now cancelled on every hop whose payload is discarded.

3. A provider outage returned 401 — a lie about the user's credentials

401 is a statement about the caller's credentials; the API used it for "the identity provider is unreachable or too slow". The login UI therefore had to hedge — "Incorrect email or password, OR the sign-in service is temporarily unavailable" — telling every user during an incident that their password might be wrong. Some will change a password that was never the problem. And the outage hides inside auth-failure metrics instead of showing as unavailability.

The API was the one that was wrong; three things had already drifted apart:

  • the contract's own status map declares PROVIDER_UNAVAILABLE: 503 (tests/security-api/referenceApp.ts)
  • the legacy /api/auth/* route already returns 503 for both errors (routes/auth.ts)
  • routes/security.tsthe route the login form actually calls — was the sole outlier at 401

Now 503 + Retry-After, declared in the frozen contract as a ServiceUnavailable response on createSession/signup, and the UI's hedged message split in two so each case says one true thing.

4. The slow hop has never been visible in production

Per-hop timings were instrumented — at logger.debug. LOG_LEVEL defaults to info and is not set anywhere in the chart, so in prod that detail has always been off. Finding the slow hop required a config change or redeploy, during an incident, on an intermittent path. That is why this module has two timeout band-aids and no diagnosis.

Adds the slow-query-log pattern: a hop over AUTHENTIK_SLOW_HOP_WARN_MS (1s) logs at WARN with its stage label; the fast path stays quiet. A login that succeeds over AUTHENTIK_LOGIN_WARN_MS (8s) also warns — a 25s success is exactly the failure mode here: it never errors, so nothing alerts, until a client bound trips underneath it.

Leading hypothesis, written into the code so the logs can refute it rather than confirm a guess: every hop targets the same in-cluster origin, and this pod's own dnsConfig documents CoreDNS "intermittently stalls lookups in 5s/10s retry multiples" (capped ~2s) with the note that this service "resolves authentik-server on every auth flow". DNS resolves per new connection, and the leaked bodies forced a new connection per hop — so the stall was multiplied by hop count. ~6 hops × ~2s accounts for most of the observed 16–30s. Draining bodies should let undici reuse one keep-alive socket, collapsing that to at most one. If the labelled elapsedMs still shows connect/DNS dominating, the next step is an explicit keep-alive dispatcher pinned to the Authentik origin.

Stated honestly: #4 is instrumentation, not a proven cure. It is what turns the next slow sign-in into a named stage instead of another guess. #1 is what gets users signed in today.

🔄 Type of Change

  • 🐛 Bug fix (non-breaking change which fixes an issue)
  • 🧪 Test addition or improvement

PROVIDER_UNAVAILABLE moving 401 → 503 is a status-code change on POST /v1/security/session. Not marked breaking: it is an error path only, it aligns the implementation with the contract it already declared, and the sibling /api/auth/* route has always returned 503. Any client treating it as "credentials rejected" was already acting on a wrong signal.

🧪 Testing

  • Unit tests
  • Integration tests
  • E2E tests
  • Manual testing — not possible from this environment; see Questions for Reviewers

Test Configuration:

  • Node.js version: 22.22.2 (repo engines targets ≥24; CI covers 24.x)
  • npm version: 10.9.7
  • OS: Linux
  • Browser (if applicable): n/a — jsdom via vitest

Test Instructions

npm install
npm run type-check -w backend/security
PERMIT_API_KEY=test-key npm test -w backend/security
cd frontend && npm install && npx vitest run
npm run lint:spec -w packages/security

Results, measured by diffing full-suite results with and without this change:

  • backend/security — identical to master except security-routes.test.ts FAIL → PASS. That suite failed to compile on master (res.headers['set-cookie'] is string | string[], so the unguarded .join was a type error), which took the whole file down — all 34 of its tests have been silently not running. Normalising the header brings them back, plus 2 added here.
  • frontend — identical file-level results, with one additional passing test (67 vs 66).
  • spectralNo results with a severity of 'error' found!
  • 8 new tests total. The 13 other pre-existing failing suites (unrelated referenceApp.ts type errors) are untouched and neither grew nor shrank.

🔧 Implementation Details

Changes Made

  • Frontend Changes:
    • services/api.tsLOGIN_TIMEOUT_MS 15s → 45s
    • pages/LoginPage.tsx — handle 503 PROVIDER_UNAVAILABLE with an outage message that never mentions credentials; 401 becomes a plain rejected-credential message (still not naming which field — that would leak account existence)
  • Backend Changes:
    • services/authentikPassword.tsDeadline (whole-request budget, min(cap, remaining) per hop); withDeadline bounds the token exchange, with a .catch on the in-flight promise so a late rejection can't become an unhandled rejection; drainBody releases discarded bodies; recordHop slow-hop WARN; slow-success WARN
    • routes/security.tsPROVIDER_UNAVAILABLE → 503 + Retry-After
    • packages/security/openapi.yaml — new ServiceUnavailable response, referenced from createSession + signup
  • SDK Changes: none. packages/security/src/generated.ts was already 570 lines stale on master (verified by regenerating on a clean tree); it is rebuilt by prebuild, so it is deliberately left out rather than sweeping unrelated drift into this PR. Flagging it as separate cleanup.

Code Quality

  • Code follows the project's coding standards
  • Self-review of code completed
  • Code is commented, particularly in hard-to-understand areas
  • No console.log or debugging statements left in code

Prettier reports the two largest touched files as non-conformant — pre-existing on master (verified on a clean tree). --write would reformat whole files and bury the diff, so formatting is left as found.

Documentation

  • Documentation has been updated — the reasoning, the budget ordering invariant, and the DNS hypothesis are captured in doc-comments beside the code
  • API documentation updated — 503 declared in the frozen contract
  • README updated (if applicable)
  • Migration guide provided (for breaking changes)

📋 Checklist

Pre-submission

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings or errors
  • I have added tests that prove my fix is effective
  • New and existing unit tests pass locally with my changes

Code Quality

  • Code follows conventional commit format
  • TypeScript strict mode passes
  • ESLint passes without errors — not run: the repo's flat-config migration is incomplete, so eslint cannot resolve a config here. CI's gate-lint covers it.
  • No security vulnerabilities introduced
  • Performance impact considered and documented

Security Checklist

  • No sensitive data exposed — a test asserts the provider's raw message (which can carry internal hosts / flow slugs) is not echoed to an unauthenticated caller
  • Input validation implemented — unchanged
  • Authentication/authorization properly handled — fail-closed posture preserved: every new path ends in AuthentikUnavailableError, so a budget-exhausted sign-in is rejected, never allowed through. Off-origin redirect guards untouched. The 401 message still declines to say which field was wrong.
  • No SQL injection vulnerabilities
  • XSS prevention measures in place

📝 Additional Notes

Deployment Notes

  • Requires database migration
  • Requires environment variable changes — none; every new knob has a working default
  • Requires dependency updates
  • Requires configuration changes

master is deploy-on-push — merge in a deploy window per the repo's hardening convention.

New knobs, all optional: AUTHENTIK_LOGIN_DEADLINE_MS (40s), AUTHENTIK_SLOW_HOP_WARN_MS (1s), AUTHENTIK_LOGIN_WARN_MS (8s), VITE_LOGIN_TIMEOUT_MS (45s).

Future Work

  • Read the WARN lines after the next slow sign-inauthentikPassword: SLOW hop names the stage. That is the input to the real fix.
  • If connect/DNS time dominates: an explicit keep-alive dispatcher pinned to the Authentik origin, and/or CoreDNS replica capacity (FuzeInfra's — delegate via @claude, per repo policy).
  • packages/security/src/generated.ts is 570 lines stale against its spec.
  • Once the slow hop is fixed, tighten the 45s/40s pair back down — they are sized to a bug, not to a target.
  • The 13 remaining failing backend/security suites (referenceApp.ts vs IdentityProvider) deserve their own PR; one more like security-routes may be hiding tests that aren't running.

Questions for Reviewers

  • I could not verify against prod. This environment's network policy blocks app.fuzefront.com (403 on CONNECT), so everything here is derived from the code and the repo's own recorded history, not a live reproduction. The DNS hypothesis in particular is unconfirmed — please treat the instrumentation as the way to settle it.
  • 45s/40s are sized to the documented 16–30s worst case. If your real p99 differs, retune the pair together — the invariant is server budget < client bound.
  • withDeadline stops waiting on the token exchange but cannot cancel it; the in-flight call completes in the background. Bounding the response was the goal — flagging the trade-off.

📊 Performance Impact

Bundle Size Impact:

  • No significant impact — the frontend delta is one numeric constant and one error branch

Runtime Performance:

  • Improves performance — releasing redirect-hop connections removes head-of-line blocking and should collapse repeated DNS lookups to one per login. Suggestive but not conclusive: two suites jest annotated as slow (~5.05s) dropped below the annotation threshold. Real confirmation comes from the new elapsedMs fields in prod.

🌐 Browser Compatibility

  • Chrome ✅
  • Firefox ✅
  • Safari ✅
  • Edge ✅
  • Mobile browsers ✅

No new browser APIs — an error-branch and a constant.

🔄 Backwards Compatibility

  • Fully backwards compatible — except the deliberate 401 → 503 correction on the provider-outage error path described above.

Sign-in fails in prod with a bare `timeout of 15000ms exceeded` in the
browser console — no HTTP status, no message, nothing in the UI. Two
defects on the server-brokered login chain combine to produce that.

1. Nothing bounded the CHAIN, only each link.

   A login is many hops: 2-3 flow-executor stages (each following up to 10
   redirects) then the authorize->code chain (up to 10 more), then the
   openid-client token exchange + userinfo. AUTHENTIK_FLOW_TIMEOUT_MS
   (10s) capped each fetch INDIVIDUALLY and every hop got a fresh full
   allowance, while OIDC_HTTP_TIMEOUT_MS (15s) applies per openid-client
   call, twice. The browser gives the whole call 15s (LOGIN_TIMEOUT_MS).
   So the server's budget exceeded the client's by a wide margin and the
   client always aborted first — which is why the failure carries no
   status and no message, and why the labelled server-side diagnostics
   ("hop timed out", naming the exact stage) never got produced. The
   symptom was undiagnosable by construction.

   Adds a Deadline shared by every hop of one attempt. Each hop's
   allowance is min(per-hop cap, time actually remaining), and the token
   exchange is held to what is left, so the server now answers INSIDE the
   browser's window with a stage-labelled error instead of being raced.

2. Redirect hops leaked their connections.

   undici keeps a socket checked out until the body is consumed or
   cancelled. Every redirect hop reads only `location`, and authorize hops
   never read a body at all, so each one pinned its connection for the
   rest of the request and later hops queued behind their own
   predecessors. That is the shape of the intermittent multi-second stalls
   this module has been given timeout band-aids for twice (#362, #371) —
   not one slow hop, but self-inflicted head-of-line blocking.

   Cancels the body on every hop whose payload is discarded.

The deadline defaults to 12s, comfortably under the client's 15s, and is
tunable via AUTHENTIK_LOGIN_DEADLINE_MS without a rebuild.

Note this makes a stalled sign-in FAIL FAST AND LOUDLY rather than making
a slow hop fast: the remaining question is which hop is slow in prod, and
these changes are what put that in the logs. AuthentikUnavailableError
still maps to 401 PROVIDER_UNAVAILABLE, which the frontend does not
distinguish — worth a follow-up, left alone here as a contract change.

Tests: 4 new cases pin the chain-level bound (budget exhaustion names the
stage; a hop is clamped to remaining time, not the 10s cap; the token
exchange is bounded; bodies are released). Full backend/security suite is
unchanged against master — the 13 pre-existing suite failures (unrelated
referenceApp.ts type errors) neither grew nor shrank.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QhfKNASYBD9aS3Wu6RUPfZ
@izzywdev izzywdev added the auto-merge Enable squash auto-merge once CI passes label Jul 29, 2026 — with Claude
@github-actions
github-actions Bot enabled auto-merge (squash) July 29, 2026 17:11
@github-actions

Copy link
Copy Markdown
Contributor

Automated code review (gate-code-review)

Credit balance is too low

Report-only — this check never blocks merge.

…ideal one

The client bound was 15s, chosen (#371) to fail FAST on a slow attempt
rather than leave the submit button stuck on "Signing in…". That traded
the wrong way. The documented slow path is 16-30s, so a 15s bound did not
fail slow sign-ins fast — it failed sign-ins that would otherwise have
SUCCEEDED. A bound below the known worst case is not a safety net, it is
an outage, and that is what users are hitting.

Client 15s -> 45s (VITE_LOGIN_TIMEOUT_MS), server budget 12s -> 40s
(AUTHENTIK_LOGIN_DEADLINE_MS). 45s covers the documented worst case with
real margin, and the server budget stays UNDER the client's so the server
still answers first with a labelled error rather than being raced to an
anonymous abort — that ordering is the invariant to preserve if either is
retuned later.

The wait is not pleasant. LoginPage already shows a "still working…" hint
at 8s, so a slow-but-succeeding attempt does not look frozen. Both values
stay env-overridable so the pair can be tightened again — without a
rebuild — once the underlying slow hop is actually fixed, which is the
real cure this is buying time for.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QhfKNASYBD9aS3Wu6RUPfZ
@github-actions

Copy link
Copy Markdown
Contributor

Automated code review (gate-code-review)

Credit balance is too low

Report-only — this check never blocks merge.

claude added 2 commits July 29, 2026 17:40
A 401 is a statement about the caller's credentials. The Security API was
using it for "the identity provider is unreachable, too slow, or handed
us a flow we cannot drive server-side" — a SERVICE condition that says
nothing about what the user typed.

The cost is not academic. The login UI had no way to tell a real
rejection from a provider stall, so its 401 message had to hedge across
both ("Incorrect email or password, OR the sign-in service is temporarily
unavailable"). During an incident every user is told their password might
be wrong; some will change a password that was never the problem. And
because the outage is reported as an auth failure, it hides inside
auth-failure metrics instead of showing up as unavailability.

This aligns three things that had already drifted apart — the API is the
one that was wrong:

  - the contract's own status map declares PROVIDER_UNAVAILABLE -> 503
    (tests/security-api/referenceApp.ts)
  - the legacy /api/auth/* route already returns 503 for both of these
    errors (routes/auth.ts)
  - routes/security.ts — the route the login form actually calls — was
    the sole outlier at 401

Also adds Retry-After (both errors are transient from the caller's point
of view), declares 503 on createSession/signup in the frozen contract as
a documented ServiceUnavailable response, and splits the frontend's
hedged message in two so each case now says one true thing: 401 is a
plain rejected-credential message, 503 explicitly reassures the user
their details are fine.

Unblocks tests/security-routes.test.ts as a side effect. It failed to
COMPILE on master (`res.headers['set-cookie']` is `string | string[]`, so
the unguarded `.join` was a type error), which took the whole file down —
all 34 tests in it have been silently not running. Normalising the header
brings them back, plus the 2 added here.

Tests: 2 new API cases (both provider errors -> 503 + Retry-After, and
the provider's raw message is not echoed to an unauthenticated caller), 1
new UI case (a 503 never blames the user's credentials), and the existing
ambiguous-401 UI assertion updated to the now-unambiguous wording.
backend/security is unchanged against master except security-routes
FAIL -> PASS; frontend is unchanged with one additional passing test.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QhfKNASYBD9aS3Wu6RUPfZ
Task 3 of the sign-in investigation: find the slowness. The blocker was
that the evidence has never existed when it mattered.

Per-hop timings were already instrumented — at `logger.debug`. LOG_LEVEL
defaults to `info` and is not set anywhere in the chart, so in production
that detail has always been OFF. Answering "which hop is slow?" needed a
config change or a redeploy, during an incident, on a path that only
misbehaves intermittently. That is why this module has collected two
timeout band-aids (#362, #371) and no diagnosis.

Adds a slow-hop threshold (the slow-query-log pattern): every hop is
still debug on the fast path, but a hop over AUTHENTIK_SLOW_HOP_WARN_MS
(1s) is logged at WARN with its stage label, status and elapsed time. No
LOG_LEVEL change, no redeploy, no spam on healthy logins. The
token-exchange stage is now timed the same way — it is two openid-client
round-trips and just as able to be the slow one, so it must not be the
one stage missing from the breakdown.

Also reports a slow SUCCESS. A login that succeeds at 25s is exactly the
failure mode that broke sign-in: it never errors, so nothing alerts, and
it only became visible once a client bound tripped underneath it. Over
AUTHENTIK_LOGIN_WARN_MS (8s, above the ~5.5s fast path) that now logs at
WARN, and the per-hop WARNs say which stage owned the time.

Leading hypothesis, written into the code so the logs can refute it
rather than confirm a guess: every hop targets the same in-cluster
origin, and this pod's own dnsConfig documents CoreDNS "intermittently
stalls lookups in 5s/10s retry multiples" (capped ~2s by timeout:1/
attempts:2) with the note that this service "resolves authentik-server on
every auth flow". DNS resolves per NEW CONNECTION, and the leaked
response bodies fixed earlier in this PR forced a new connection per hop
— so the stall was multiplied by hop count. ~6 hops x ~2s accounts for
most of the observed 16-30s, and draining bodies lets undici reuse one
keep-alive socket per origin, collapsing it to at most one. If the
labelled elapsedMs still shows connect/DNS dominating, the next step is
an explicit keep-alive dispatcher pinned to the Authentik origin.

This is instrumentation, not a proven cure: it is what turns the next
slow sign-in into a named stage instead of another guess.

Tests: 1 new case — a slow hop on a login that SUCCEEDS emits exactly one
WARN carrying the stage label and elapsed time.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QhfKNASYBD9aS3Wu6RUPfZ
@github-actions

Copy link
Copy Markdown
Contributor

Automated code review (gate-code-review)

Credit balance is too low

Report-only — this check never blocks merge.

@izzywdev izzywdev changed the title fix(security): bound server-brokered sign-in by a whole-request budget fix(login): un-break sign-in — right-size the timeouts, 503 for outages, and make the slow hop visible Jul 29, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Automated code review (gate-code-review)

Credit balance is too low

Report-only — this check never blocks merge.

@izzywdev
izzywdev merged commit 753032e into master Jul 29, 2026
54 checks passed
@izzywdev
izzywdev deleted the claude/signin-timeout-error-2avs7q branch July 29, 2026 21:46
izzywdev pushed a commit that referenced this pull request Jul 29, 2026
#455 ("un-break sign-in — right-size the timeouts, 503 for outages") rewrote
authentikPassword.ts and its test heavily enough that git could not align them
at all — both came through as whole-file conflicts (line 1 to EOF). Rather than
hand-merge two ~900-line files, I took master's versions wholesale, so ALL of
#455's login fix is preserved verbatim, then re-applied my tenant edits on top:

  - authentikBaseUrl(), redirectUri(), enrollmentFlowSlug() and the admin token
    resolve from the current tenant instead of process.env
  - oidcService -> getOidcService()
  - the test's oidc mock exposes getOidcService() backed by the same object

#455's three NEW timeout knobs (AUTHENTIK_LOGIN_DEADLINE_MS,
AUTHENTIK_SLOW_HOP_WARN_MS, AUTHENTIK_LOGIN_WARN_MS) are deliberately left on
the environment — like AUTHENTIK_FLOW_TIMEOUT_MS they are tuning values, not
tenant-identifying configuration.

Verified after the merge: tsc --noEmit clean; 129 passed / 2 failed across the
tenant registry plus the 8 Authentik/OIDC suites. The 2 are the same
pre-existing failures that reproduce on clean master. The count rose from 124
to 129 because #455 added 5 tests to the password-login suite.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session-Id: 55a394fa-d5ba-4da4-b41f-b0ef56fa6ddf
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-merge Enable squash auto-merge once CI passes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants