fix(security): keep-alive Agent pinned to the Authentik origin — last documented mitigation for the sign-in timeout - #494
Merged
Conversation
…ver-side login Every hop in the server-brokered password-login chain (flow-executor stages, authorize hops, admin-API set_password) lands on the same in-cluster Authentik origin, several times per request. The default fetch dispatcher's keep-alive window is tuned for general traffic and can lapse between hops when CoreDNS's documented multi-second stalls (see the recordHop comments in authentikPassword.ts) space them out, forcing a fresh connection — and a fresh DNS lookup — per hop. A dedicated undici Agent with a longer keep-alive holds one socket open across the whole chain, so only the first hop pays for DNS+connect. This was the last documented, unapplied mitigation in that file's own incident notes (#362, #371) for the intermittent 16-30s+ sign-in stalls that show up as an outright client-side timeout for some accounts.
Contributor
Automated code review (gate-code-review)Credit balance is too low Report-only — this check never blocks merge. |
Contributor
Automated code review (gate-code-review)Credit balance is too low Report-only — this check never blocks merge. |
Contributor
Automated code review (gate-code-review)Credit balance is too low Report-only — this check never blocks merge. |
Contributor
Automated code review (gate-code-review)Credit balance is too low Report-only — this check never blocks merge. |
Contributor
Automated code review (gate-code-review)Credit balance is too low Report-only — this check never blocks merge. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📋 Description
Follow-up to #455 and #459 on the same sign-in-timeout thread. A fresh production report on this thread confirmed the stall is still live post-#459: a Google sign-in redirect took ~10-15s (inside the documented 16-30s slow-path range), and a password sign-in for one account timed out outright with no session established.
Both #455 and #459 already fixed the symptoms — the client/server timeout budgets, the misleading 401, the redundant userinfo hop, and instrumented the cause (
recordHopWARN logging) without confirming it. #455's own "Future Work" section named the one mitigation it did not apply:This PR applies exactly that, unconfirmed-hypothesis caveat intact — I don't have production log access to verify
elapsedMsfromrecordHop, so this is the documented next step, not a proven cure.Root cause (documented in
authentikPassword.ts, unconfirmed)Every hop of one login/signup attempt (2-3 flow-executor stages, the authorize→code chain, admin-API
set_password) lands on the same in-cluster Authentik origin, several times per request. This pod's owndnsConfigdocuments CoreDNS "intermittently stalls lookups in 5s/10s retry multiples." DNS resolves per new connection. #455 already fixed one multiplier (leaked response bodies pinning sockets, forcing a fresh connection per hop) viadrainBody. What's left: Node's default fetch dispatcher still uses a keep-alive window sized for general-purpose traffic, which can lapse between hops when a DNS stall spaces them out further than expected — forcing a reconnect (and a fresh lookup) anyway.Fix
A dedicated
undici.Agent(keepAliveTimeout: 30s,keepAliveMaxTimeout: 60s,connections: 32), created once at module scope inauthentikPassword.tsand passed asfetch'sdispatcheroption from the singlefetchWithTimeouthelper all four Authentik-origin call sites already funnel through. Holds one socket open across an entire login chain, so only the first hop of a request should pay for DNS + connect.undiciwas already resolving transitively (hoisted from another dependency); it's now declared directly inbackend/security/package.jsonrather than relying on hoisting.🔄 Type of Change
🧪 Testing
app.fuzefront.com); same limitation noted on fix(login): un-break sign-in — right-size the timeouts, 503 for outages, and make the slow hop visible #455/fix(login): Google button disappearing + drop redundant userinfo round-trip #459Test Configuration:
enginestargets ≥24; CI covers 24.x)Test Instructions
npm install npm run type-check -w backend/security npm test -w backend/securityResults:
type-check -w backend/security: cleanbackend/securityfull suite: identical pass/fail set before and after (14 failed suites / 3 failed tests / 312 passed — all pre-existing, verified viagit stashA/B on the unmodified tree; none touchauthentikPassword.ts)authentik-password-login.test.ts,authentik-signup.test.ts) fully pass — they stubglobal.fetchdirectly, so the addeddispatcheroption is inert in tests🔧 Implementation Details
Changes Made
backend/security/src/services/authentikPassword.ts— module-scopedundici.Agent, wired intofetchWithTimeout's singlefetchcallbackend/security/package.json— declaresundicias a direct dependencyCode Quality
Documentation
🚨 Breaking Changes
None. Purely additive connection-pooling change to an internal HTTP client; no API surface, request shape, or response shape changes.
📋 Checklist
Pre-submission
Code Quality
eslintcan't resolve a config here (same note as fix(login): un-break sign-in — right-size the timeouts, 503 for outages, and make the slow hop visible #455/fix(login): Google button disappearing + drop redundant userinfo round-trip #459). CI'sgate-lintcovers it.Security Checklist
📝 Additional Notes
Deployment Notes
masteris deploy-on-push — merge in a deploy window per the repo's hardening convention.Future Work
authentikPassword: SLOW hopWARN lines after the next slow sign-in. IfelapsedMsfor early hops is still dominated by connect/DNS after this change, the dispatcher didn't fully address it and CoreDNS replica capacity is the remaining lever — that's FuzeInfra's, delegated via@claudeper repo policy, not this repo.Questions for Reviewers
🌐 Browser Compatibility
N/A — backend-only change.
🔄 Backwards Compatibility
Generated by Claude Code