OAuth phishing-resistance: consent screen, HMAC nonce, redirect policy, client cap#106
Open
jpr5 wants to merge 6 commits into
Open
OAuth phishing-resistance: consent screen, HMAC nonce, redirect policy, client cap#106jpr5 wants to merge 6 commits into
jpr5 wants to merge 6 commits into
Conversation
Introduce a shared OAuth observability log-wrapper (src/oauth/observability.ts) and a setter-injected trusted-client-IP resolver (src/oauth/trusted-client-ip.ts) with an assertion that fails fast if the resolver is not wired before serving requests. Extend src/config.ts with oauthConsentHmacKeys (parsed list with rotation support) and update cross-cutting test mocks for the new Config field.
Add src/oauth/consent-nonce.ts: HMAC-SHA256 over a canonical length-prefixed encoding of (clientId, redirectUri, scope, codeChallenge, codeChallengeMethod, state) with rotation-key support and constant-time verification. Add src/oauth/redirect-uri-policy.ts: enforce https-only except loopback, and reject 0.0.0.0/8, RFC1918, link-local, ULA, IPv4-mapped, and other non-public address ranges. Both modules covered by dedicated test files.
Extend src/oauth/store.ts: add a registered-client cap with lazy TTL-based eviction, a touch() entrypoint that bumps lastSeen on successful token exchange, and a ClientCapError raised when the cap is exhausted with no evictable entries. Add a consentLimiter (30/min) to src/oauth/rate-limiter.ts for the consent POST endpoint. Tests extended accordingly.
Add src/oauth/consent-template.ts: server-rendered HTML consent screen with every interpolation HTML-escaped and no inline JavaScript. Add src/oauth/consent-handler.ts: the POST /authorize/consent handler implementing the 8-step approval pipeline (rate-limit, parse, verify HMAC nonce, reload client, recheck redirect_uri policy, mint code, persist, redirect) where the redirect URI comes from the nonce-bound payload rather than the form body.
registerHandler now enforces the redirect_uri policy and client cap. authorizeHandler mints an HMAC-bound consent nonce and renders the consent screen with CSP and X-Frame-Options DENY. tokenHandler touches the client on successful exchange. originOf gates X-Forwarded-Proto on the configured trust-proxy boundary. Mount POST /authorize/consent in src/server.ts, wire setTrustingProxy + assertOauthIpResolverInjected, and update the OAuth section banner. Test surface rewritten in oauth-handlers.test.ts; oauth-e2e.test.ts covers the full consent flow including cap exhaustion and XFP spoofing.
Reformat OAuth files to match repo prettier config (CI Static Quality gate).
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.
Summary
Adds a server-rendered consent screen between
GET /authorizeand code issuance, replacing auto-approval. This makes the OAuth flow phishing-resistant — the consent screen, not DCR authentication, is the security boundary.Commits (5, logical)
OAuth observability, config plumbing, trusted-IP resolver —
[oauth]log vocabulary,oauthConsentHmacKeysconfig (fail-loud in prod, dev ephemeral key, multi-key rotation), trusted-IP setter-injection seam with bootstrap assertion.HMAC consent-nonce + redirect_uri policy — SHA-256 over canonical length-prefixed encoding of {client_id, redirect_uri, state, code_challenge, code_challenge_method, response_type, scope, resource, exp}; multi-key verify; MAC-then-exp ordering. Policy rejects 0.0.0.0/8, RFC1918, link-local (fe80::/10 numeric prefix), ULA, IPv4-mapped private; treats 127.0.0.0/8 +
localhost+[::1]as loopback (http permitted only here).Client cap with TTL eviction — 10,000 total / 100 per-IP; lazy sweep on
register()(>30d unused / >7d never-used-after-registration);touch()bumpslastUsedAton consent-approve and token-grant success.Consent screen template + POST
/authorize/consenthandler — server-rendered HTML withCSP default-src 'none'; frame-ancestors 'none',X-Frame-Options: DENY,Referrer-Policy: no-referrer; every interpolation HTML-escaped (5 chars). POST handler runs an 8-step pipeline (rate-limit, nonce HMAC verify, field-by-field equality, client lookup, policy + exact-match re-check, scope/response_type/PKCE re-check, decision branch). Final redirect URL is built from the nonce-bound payload, never the form body — that's the phishing-resistance invariant, positive- and negative-proven in tests.Handlers + server wiring —
registerHandlerenforces policy + cap (429 per-IP / 503 total +Retry-After);authorizeHandlermints HMAC nonce and renders consent (no longer touches the client on GET);tokenHandlertouches the client on both grant branches;originOf()gatesX-Forwarded-Protoon the trust-proxy boundary;server.tsmounts the consent route, wiressetTrustingProxy, and asserts the OAuth IP resolver was injected at boot.Tests
tsc --noEmitcleanOut of scope — follow-up PRs
Real load-bearing issues with subjects distinct from this PR's:
audrejects RFC 8707 resource-bound tokens (issueTokenPairmintsaud = resource || origin; refresh verifies{aud: origin}strict — incompatible)resource_metadatadiscovery hint (deadunauthorizedWithDiscoveryhelper exists, never wired)bucketsMap has no eviction (memory growth under IP rotation)/revokeis a no-op stub (no revocation list; leaked refresh tokens survive ~30 days)audaccept-set +client_secretenforcement against advertisedtoken_endpoint_auth_methodTest plan
/authorize200 response