Repository navigation
fix(oauth2)!: harden the authorization server endpoints - #119
Merged
Merged
Conversation
The storage contract says stores keep tokens hashed only ("MUST store
hashes"; refresh tokens are "ALWAYS stored hashed"), and the SQL and Redis
stores persist DTOs without the raw value. The in-memory store saved the
AccessToken and RefreshToken structs it was handed as they were, so
LookupAccessToken and LookupRefreshToken returned the raw tokens — after a
rotation too — while SaveAuthorizationCode already cleared the raw code.
persist now stores copies without the raw value and leaves the caller's
pair untouched (the server writes the token response from it). The
storetest conformance suite gains TokensAreNeverStoredRaw — no lookup,
after saving, rotating, for the consumed ancestor or an access token outside
any family, returns a raw token, and the caller's pair keeps its tokens —
and requireContext asserts the same on every lookup it makes: a custom
store keeping raw tokens now fails the suite.
Audit: STORE-03.
Every endpoint parsed its form with net/http's defaults — up to 10 MB of body — and /authorize echoed any state in its redirects and recorded any nonce with the code, which every store persists and the Redis consume script decodes and re-encodes: a 9 MB nonce cost tens of milliseconds of a blocked Redis per redemption, and about 100 such codes filled 1 GB of a noeviction Redis, whose writes — family revocations included — then fail. oidc/provider bounded the nonce of OpenID requests only. - ServerConfig.MaxRequestBytes (DefaultMaxRequestBytes, 64 KiB, when zero or negative) bounds the query and the body of /authorize, /token, /revoke and /introspect: a declared Content-Length over it is refused before the body is read, a body of unknown length through http.MaxBytesReader; the answer is invalid_request "request too large" (RFC 6749 §5.2), a bare 400 at /authorize. - The nonce of every authorization request is at most MaxNonceLength (512) bytes of valid UTF-8 without NUL — the rule and text of oidc/provider — else a redirected invalid_request: no store is handed more. - The state is at most MaxStateLength (4096) bytes of VSCHAR (RFC 6749 App. A.5, §10.14), else a redirected invalid_request that does not echo it. - A client_id over MaxClientIDLength (255) bytes is an unknown client, never looked up — at /authorize whatever the resolver did, and by every client-authentication method (RFC 6749 §2.2). - An implicit request records no PKCE parameter, and a code_challenge_method without code_challenge is not recorded (RFC 7636 §4.3): neither reached a store or a callback unbounded any more. Test fixtures: the scope-syntax test raises the bound so that its 40 000 tokens still reach the scope bounds; the OP's plain-OAuth test no longer sends a 600-byte nonce, which the core now refuses everywhere. Audit: OAUTH-11, STORE-01.
The authorization endpoint renders the application's login and consent pages through the ConsentFunc but set no protective header: the pages could be framed (clickjacking, RFC 6749 §10.13: the server MUST prevent it, RFC 9700 §4.16) and cached, and the godoc said nothing about CSRF — while the canonical example approved any POST carrying decision=approve, so a cross-site form could approve on the user's behalf (RFC 6749 §10.12: the server MUST protect its authorization endpoint against CSRF). A POST was also answered with a 302, which user agents may replay as a POST carrying the resource owner's credentials (RFC 9700 §4.12 asks for 303). - Every /authorize response — the ConsentFunc's page, the redirects, the bare errors — carries X-Frame-Options: DENY, Content-Security-Policy: frame-ancestors 'none' (added, so a policy an outer handler set is kept), Cache-Control: no-store and Referrer-Policy: same-origin (RFC 9700 §4.2.4). same-origin rather than no-referrer: under no-referrer browsers send Origin: null on the consent page's own POST, which Origin-based CSRF checks refuse. The ConsentFunc may replace them to let a trusted origin frame its page. Browsers check framing on the final response only: prompt=none in an iframe still follows the redirect (checked in headless Chrome). - A redirect answering a POST is a 303 See Other, a GET keeps 302; a 405 lists Allow: GET, POST (RFC 9110 §15.5.6). - The ConsentFunc godoc states the CSRF contract: protect the request carrying the decision — a session-bound token or net/http.CrossOriginProtection — not the endpoint, to which clients may POST authorization requests (RFC 6749 §3.1). - examples/oauth2 refuses a cross-origin decision with http.CrossOriginProtection while a cross-site authorization request still gets the consent page. Audit: OAUTH-04, OAUTH-I5.
The authorization endpoint redirected to whatever a client registered:
relative URIs, URIs with a fragment (the implicit flow then answered
cb#section#access_token=…), javascript:, a query naming the response
parameters. RFC 6749 §3.1.2 requires an absolute URI without fragment
whose query "MUST be retained when adding additional query parameters";
the endpoint instead re-encoded it — reordered, a=1;b=2 dropped (Go
refuses ";"), flag turned into flag=, a registered state or code
overwritten. And exact matching refused a loopback redirect URI whose port
differed from the registered one, which RFC 8252 §7.3 requires ("The
authorization server MUST allow any port to be specified at the time of
the request for loopback IP redirect URIs").
- oauth2.ValidateRedirectURI (wrapping ErrInvalidRedirectURI) checks a
redirect URI at registration: an absolute URI of RFC 3986 characters with
valid escapes and no fragment; a host and no userinfo for http(s); for
any other scheme a "." in it (RFC 8252 §7.1, §8.4), which also refuses
javascript:, data:, file: and myapp:; a query naming none of code,
state, iss, error, error_description, error_uri — split on "&" and ";",
percent-decoded (RFC 6749 §3.1: no parameter twice). /authorize applies
it to the registered URI a request resolves to: a refusal is a bare 400,
never redirected (RFC 6749 §3.1.2.4), the cause in OnError.
- A registered http://127.0.0.1 or http://[::1] redirect URI accepts any
port from 1 to 65535 without leading zero, the URIs byte-for-byte equal
once the port of each authority is removed (fragment, query, userinfo
and path included); localhost gets no variance (RFC 8252 §8.3). The code
is bound to the URI requested, which /token must present.
- Redirects append the response parameters to the registered URI as is.
Audit: OAUTH-I3.
/authorize redirected every protocol error to the client's registered redirect URI as soon as it was validated — a bad scope, response type or PKCE parameter, a failing resolver or extension — before the ConsentFunc ran, so before anyone authenticated. RFC 9700 §4.11.2: a registered client controlling its redirect URI can thus use the authorization server as a redirector to a phishing page; "The authorization server MUST always authenticate the user first … before redirecting the user" and "SHOULD only automatically redirect the user agent if it trusts the redirection URI". AuthorizeConfig.TrustedRedirect(ctx, client, redirectURI) decides, for the errors detected before the resource owner authenticated — every check of the request, the extension's, a ConsentFunc failure — and for a Consent.Refusal stating the request cannot complete without a user interface (login_required, interaction_required, account_selection_required, consent_required: OpenID Connect Core §3.1.2.6, the answers to prompt=none, which authenticate nobody — RFC 9700 §4.11.2's third attack). For a client it does not trust they are a bare error page (Error.HTTPStatus: 400, 500 for server_error…), reported to OnError like a redirect; once the ConsentFunc returned any other decision, the code, access_denied and later failures are redirected for every client. Without the hook every registered client is trusted, as before. It flows through oidc/provider, which takes the AuthorizeConfig from the application. Audit: OAUTH-10.
/token, /revoke and /introspect described why a client authentication failed: "unknown client" against "client authentication failed" for a known client with a wrong secret, "none method reserved to public clients", "method not allowed for client", "client cannot verify secret", "introspection requires confidential client authentication"… Client identifiers are not secret (RFC 6749 §2.2), but the answers confirmed which clients exist and disclosed their type and authentication methods — while the realm design wanted a client of another issuer to read exactly like an unknown one. clientAuthFailed turns every invalid_client of the client authentication — the shipped methods', a custom authenticator's, the realm check's, the introspection caller check's — into the same answer: 401 invalid_client, "client authentication failed", the Basic challenge (RFC 6749 §5.2). The refusal it replaces is the cause: OnError reads its description with errors.As, and ErrClientNotAtIssuer still matches. Other errors — invalid_request for repeated or several credentials, server errors — pass through unchanged. Audit: OAUTH-06.
The ClientAuthenticator contract returns "(nil, other) on storage /
unexpected errors", yet the shipped methods wrapped a ClientStore failure
in invalid_client: a database outage answered 401 invalid_client — telling
every client its credentials were wrong — and /authorize answered it as an
unknown client. A token lookup that failed at /introspect answered
{"active":false} without reaching OnError, so an outage looked like a
stream of invalid tokens.
- The ClientStore contract is explicit: (nil, nil) — or an error carrying
invalid_client — is an unknown client; any other error is a failure.
clientauth returns it wrapped; /authorize answers a bare 500, /token and
/introspect a 500 server_error, each reported to OnError.
- /revoke answers 503 temporarily_unavailable (RFC 7009 §2.2.1: the
revocation did not take place, the client retries) for any failure of
its client authentication that is not a refused credential — a failing
ClientStore or IssuerResolver — reported once.
- /introspect still answers {"active":false} for a token whose lookup
failed (RFC 7662 §2.1: a server unable to determine the state of a token
SHOULD answer it is not active) and now reports the failure; a lookup
that merely misses (invalid_grant: unknown, revoked family) is not one.
Audit: OAUTH-I8.
/introspect and /revoke resolved the issuer of the request to
authenticate the client, then ignored it: behind an IssuerResolver
serving several realms, a client registered in two of them could
introspect, and revoke, the tokens of one realm at the other, while the
grants refused them there (OpenID Connect Core §16.15: issuers are
separate realms). Without an IntrospectionPolicy, any confidential
client could also read the claims of any client's refresh token,
although refresh tokens are meant for the authorization server and
their client (RFC 6749 §1.5).
A token whose recorded issuer differs from the request's, compared
exactly as the grants do, now introspects {"active":false} without
consulting the policy, which therefore only sees tokens of the
request's issuer (RFC 7662 §4), and is left untouched by /revoke,
answered 200 as before (RFC 7009 §2.1, §2.2). Without a policy, a
refresh token is disclosed to its own client only; a policy, when set,
decides for every token. A token recorded without issuer is inactive at
a non-empty issuer, but its client can still revoke it: revocation only
takes away.
Breaking for multi-issuer deployments: resource servers call
/introspect, and clients /revoke, at the issuer of the token
(MIGRATION.md).
Audit: OAUTH-05.
The RFC 8414 document disagreed with what the server serves. With the implicit flow enabled it still listed response_types_supported ["code"] and response_modes_supported ["query"], and never the implicit grant type. It omitted revocation_endpoint_auth_methods_supported, whose absence means client_secret_basic alone (RFC 8414 §2), while /revoke accepts every ClientAuth method, and introspection_endpoint_auth_methods_supported. grant_types_supported came in map order, changing from one response to the next, and any method was answered 200 although the document is retrieved with GET (RFC 8414 §3.1). An AuthorizeHandler enabling the implicit flow now records it on the server, and the document then adds the token response type, the fragment response mode and the implicit grant type (RFC 7591 §2). The revocation list is the ClientAuth methods, the introspection list the same without "none" (/introspect answers confidential clients only), omitted when nothing is left; grant types are sorted; methods other than GET and HEAD get 405 with Allow (RFC 9110 §15.5.6). The godoc gives the jwks_uri and .well-known locations actually built (RFC 8414 §3). The oidc/provider consistency test skips the two auth-method lists its discovery document does not publish yet. Audit: OAUTH-I4.
…cation DefaultClient.SecretMatches compared the secrets themselves with subtle.ConstantTimeCompare, which answers at once when their lengths differ: the timing disclosed the length of the registered secret. It now compares their SHA-256 digests, in constant time; what matches is unchanged. The rest is documentation of what the server leaves to its deployment: - Client secrets at rest: DefaultClient keeps the cleartext; production clients implement SecretMatcher over a hash, the SHA-256 of a generated secret of at least 128 bits (RFC 6749 §10.10) or a password hash for human-chosen secrets, with a dummy verification for unknown clients. - Brute force: the server does not throttle client authentication. RFC 6749 §2.3.1 asks for brute-force protection and RFC 7662 §4 warns against an un-throttled /introspect: rate-limit /token, /revoke and /introspect per client_id and source address, and alert on OnError. - Foreign-token revocation: /revoke answers a token of another client, or of another issuer, with the 200 of an invalid token (RFC 7009 §2.2) and leaves it alone, where RFC 7009 §2.1 refuses the request with an error, which would confirm the token is valid for someone else. The test pins that the answer is the one of an unknown token. LIMITATIONS also lists that a failed introspection lookup answers inactive (RFC 7662 §2.1), and CLAUDE.md summarizes the endpoint hardening of this series. Audit: OAUTH-I9, OAUTH-I10, OAUTH-I11, OAUTH-I8.
…iscovery The OpenID Provider discovery document, also servable as the RFC 8414 document of its issuer, omitted revocation_endpoint_auth_methods_supported, whose absence means client_secret_basic alone (RFC 8414 §2) while /revoke accepts every ClientAuth method, and introspection_endpoint_auth_methods_supported, and listed its grant types in configuration order. The oauth2 metadata now publishes both lists and sorts its grant types: the two documents disagreed, which RFC 9068 §4 forbids, and their consistency test skipped the two lists. The discovery document lists the ClientAuth methods for /revoke, the same without "none" for /introspect (confidential clients only; omitted when nothing is left), and sorts grant_types_supported, as the oauth2 metadata does. The consistency test compares every member strictly again, for a server of public clients too, and checks that the two lists are published or omitted alike. Audit: OAUTH-I4.
authorizeRedirectTarget chose its separator from the whole redirect URI: a registered query whose last value ends with "?" (cb?next=/home?) was taken for an empty query, and the response parameters were glued to that value. The client read no code and next="/home?code=...", and with a next URL the code traveled inside a third-party URL. RFC 6749 §4.1.2 adds code to the query component, and §3.1.2 retains the registered query as it is. The separator now depends on the query component alone. A fuzz target pins the property: for every redirect URI ValidateRedirectURI accepts, code, state and iss come back exactly once, with their values. Audit: OAUTH-I3.
ValidateRedirectURI tested the authority, not the host: http://:80/cb, https://:443/cb and http://:/cb passed, and /authorize redirected to them, although RFC 9110 §4.2.1 and §4.2.2 forbid generating an http(s) URI with an empty host and require rejecting one. A port outside 1-65535 (http://app.example:99999/cb) passed as well. The validator now requires a host name and, when a port is present, one within 1-65535; an empty port stays valid (RFC 3986 §3.2.3). The docs of the redirect URI rules also name the out-of-band urn:ietf:wg:oauth:2.0:oob among the registrations the period rule refuses. Audit: OAUTH-I3.
Two behaviours of the TrustedRedirect hook were untested, so mutants changing them survived. An extension refusal carrying a code the authorization endpoint does not define (invalid_client) must reach an untrusted client as the bare 500 server_error page the godoc promises, and the trusted path as a redirected server_error, not as a 401 invalid_client (RFC 6749 §4.1.2.1). A decision a ConsentFunc returns on a request it modified must count as an authentication: the server_error is redirected and the hook is not asked. The ConsentFunc godoc now states the premise the hook relies on: a decision means the resource owner authenticated; before that, render the login page again or refuse with an interaction code, since any other decision, a denial included, is redirected even to an untrusted client (RFC 9700 §4.11.2). Audit: OAUTH-10.
A Profile20 provider could enable the legacy implicit flow for plain OAuth 2.0 requests. The server then advertised response_type token, the fragment response mode and the implicit grant in its RFC 8414 metadata, while the OpenID discovery document of the same issuer advertised the code flow alone: RFC 9068 §4 requires the two publications to be consistent. RFC 9700 §2.1.2 deprecates the implicit grant, and OpenID requests could never use it (OIDC Core §3). Provider.AuthorizeHandler now panics when AuthorizeConfig.AllowImplicit is set, before the server records the flow, as the server panics on its own misconfigurations; every response_type=token request then gets unsupported_response_type. The consistency test covers a Profile20 provider attempting it. BREAKING CHANGE: a provider configured with AllowImplicit panics at construction; drop the option and move implicit clients to the authorization code flow with PKCE (MIGRATION.md). Audit: OAUTH-I4.
The tests added by this series stubbed oauth2 interfaces by hand, while generated mockery mocks now serve every interface double: a ClientStore answering every lookup with an error, a Client unable to verify a secret, and the bounds environment's client store counting its lookups, which now records them on a MockClientStore answering from the test registry, as the client-policy tests already do. The decorators delegating to a real store (the write-counting and lookup-failing stores, the code recorder) stay hand-written: they are not plain doubles. The touched clientauth test file is gofmt-clean again. Audit: OAUTH-11, OAUTH-06, OAUTH-I8.
…t page The example's consent form posted the decision to /oauth2/authorize with the query string it was served at. An authorization request POSTed with its parameters in the body, which RFC 6749 §3.1 and OpenID Connect Core §3.1.2.1 allow and the README advertises, rendered a form that posted no request at all, and the approval failed as an unknown client. The form now also re-posts the body parameters as hidden fields, HTML-escaped, except an empty decision, which would precede the button's. Docs: ServerConfig.MaxRequestBytes says that a body a middleware parsed before the handler was read without the bound, and the security considerations qualify the size of a 4 KiB state once percent-encoded. Audit: OAUTH-04, OAUTH-11.
The provider re-checks the response type and the nonce of every OpenID request. The core now refuses those requests first: it bounds the nonce of every request, and the provider serves no implicit flow, while a handler built from Server() is not mounted for OpenID requests. So no request reaches the two refusals any more. They stay as the provider's own defense in depth and are now tested directly, as the package already does for its token-response client check. Audit: STORE-01, OAUTH-I4.
euskadi31
force-pushed
the
fix/audit-oauth2-endpoints
branch
from
October 13, 2026 06:30
bff323a to
6636822
Compare
The docs said the OpenID Provider never serves the implicit flow, and that its RFC 8414 metadata never advertises it. But only Provider.AuthorizeHandler refuses AllowImplicit: a handler built from Provider.Server().AuthorizeHandler, the path MIGRATION documents for plain OAuth 2.0 requests approved by another ConsentFunc, still accepts it. The server then serves the flow to those requests and advertises it in its RFC 8414 metadata, while the discovery document does not, which RFC 9068 §4 forbids. The CHANGELOG, docs/oidc-provider.md, LIMITATIONS and MIGRATION statements now apply to Provider.AuthorizeHandler. The Provider.Server and AuthorizeHandler godoc, the provider docs, LIMITATIONS and the MIGRATION path warn never to enable AllowImplicit on a handler built from Server(): nothing refuses it there, and RFC 9700 §2.1.2 deprecates the flow. No code changes, as decided (UD-1): a guard would need a new oauth2 hook. Audit: OAUTH-I4.
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
1 participant
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.
Fixes the following findings of the internal security audit of 2026-10-12 (commit c5c2ac3): OAUTH-04, OAUTH-05, OAUTH-06, OAUTH-10, OAUTH-11, STORE-01, STORE-03, OAUTH-I3, OAUTH-I4, OAUTH-I5, OAUTH-I8, OAUTH-I10. It documents OAUTH-I9 and OAUTH-I11, deliberate design choices.
It completes the OAuth2 part of the campaign started by #114 (the fail-closed client policy). #114 covered the client policy; this PR hardens the endpoints themselves:
/authorize,/token,/revoke,/introspectand the RFC 8414 metadata. It also fixes the in-memory store. It applies maintainer decision 12 (an opt-in trust hook for error redirects) and the decisions taken for this PR, listed below.Goal / Problem
/authorizeresponses set no anti-framing, caching or referrer header, and these are the login and consent pages theConsentFuncrenders. The canonical example also approved any POST carryingdecision=approve. So a framed page or a cross-site form could approve on the user's behalf (RFC 6749 §10.12, §10.13; RFC 9700 §4.16)./introspectand/revokeignored the issuer of the request. Behind anIssuerResolverserving several realms, a client registered in two of them could introspect and revoke one realm's tokens at the other. Without anIntrospectionPolicy, any confidential client (every resource server) could also read the claims of any client's refresh token (RFC 6749 §1.5; RFC 7662 §4; OIDC Core §16.15).invalid_clientdescriptions told an unknown client from one with a wrong secret, and revealed a client's type and authentication methods. That made/token,/revokeand/introspecta client-enumeration oracle.prompt=nonerequest. The server then redirected the user to the client's redirect URI before anyone authenticated: an open redirector (RFC 9700 §4.11.2)./authorizeechoed anystatein its redirects and stored anynoncewith the code, which every store persists and the Redis consume script re-encodes. About 100 codes with a 10 MB nonce fill 1 GB of anoevictionRedis, whose writes, revocations included, then fail.javascript:. Their query was re-encoded, and a loopback redirect URI requested with another port was refused, although RFC 8252 §7.3 requires accepting it.tokenresponse type orfragmentmode with the implicit flow, no revocation or introspection auth-method lists (an absent revocation list meansclient_secret_basicalone), grant types in random map order. It also answered every HTTP method.ClientStorewas answered as an unknown client (invalid_client, which tells a client its credentials are wrong), and a failed token lookup at/introspectreached no hook.DefaultClient.SecretMatchesrevealed the length of the registered secret through its timing./revokeanswers 200 for another client's token.Design
/authorizeresponse carriesX-Frame-Options: DENY,Content-Security-Policy: frame-ancestors 'none'(added to an existing policy, never replacing it),Cache-Control: no-storeandReferrer-Policy: same-origin. A redirect answering a POST is a 303, and a 405 listsAllow: GET, POST. TheConsentFuncgodoc states the CSRF contract: protect the request carrying the decision, not the endpoint.examples/oauth2refuses cross-origin decisions withhttp.CrossOriginProtection, and its consent form now also re-posts a request sent in the body.{"active":false}without consulting the policy, and/revokeleaves it untouched (still 200). Without a policy, a refresh token is disclosed to its own client only. A token recorded without issuer is inactive at a non-empty issuer, but its client can still revoke it.invalid_client, "client authentication failed", the Basic challenge. The detailed refusal is the causeOnErrorreceives.AuthorizeConfig.TrustedRedirect(ctx, Client, redirectURI) bool. For a client it does not trust, an error detected before the resource owner authenticated, or aprompt=none-style refusal (login_required,interaction_required,account_selection_required,consent_required), is a bare page with the error's status, never a redirect. Without the hook, every client is trusted, as before (decision 12).ServerConfig.MaxRequestBytes(DefaultMaxRequestBytes= 64 KiB) bounds the query and the body of the four endpoints; the declared length is checked before the body is read. Over the bound:invalid_request(a bare 400 at/authorize). Every request'snonceis limited to 512 bytes of UTF-8 without NUL (the OP's rule). Thestateis limited to 4096 bytes of VSCHAR; an invalid one is refused and never echoed. Aclient_idover 255 bytes is an unknown client and is never looked up. The PKCE parameters of an implicit request are ignored.storetestconformance suite now fails any store that returns a raw token or changes the caller's pair.ValidateRedirectURI/ErrInvalidRedirectURI, enforced at/authorizewith a bare 400. Its rules: absolute; no fragment; for http(s), a host, no userinfo and a port within 1–65535; a "." in a private-use scheme (RFC 8252 §8.4), which also refusesjavascript:,data:andurn:ietf:wg:oauth:2.0:oob; no response parameter in the query. The registered query is kept byte for byte, and the implicit flow no longer adds a second fragment. A loopback IP redirect URI accepts any port (RFC 8252 §7.3), and the code is bound to the URI requested.AuthorizeHandleris built, grant types are sorted, the revocation list holds theClientAuthmethods and the introspection list the same withoutnone. It answers GET and HEAD only. The OpenID Provider's discovery document publishes the same lists (RFC 9068 §4).ClientStoreerror that does not carryinvalid_clientis an outage: a bare 500 at/authorize, 500server_errorat/tokenand/introspect, 503temporarily_unavailableat/revoke(RFC 7009 §2.2.1), reported toOnError. A failed introspection lookup answers{"active":false}(RFC 7662 §2.1) and is now reported too.DefaultClient.SecretMatchescompares the SHA-256 digests of the two secrets in constant time. TheSecretMatchergodoc explains how to keep only a hash at rest.Decisions applied
Referrer-Policy: same-origin, notno-referrer: withno-referrer, Chrome sendsOrigin: nullon the consent POST), Q6 (metadata), Q7 (I9 and I10 documentation, plus the length-leak fix), Q8 (303).oidcprovider.Provider.AuthorizeHandlerpanics whenAuthorizeConfig.AllowImplicitis set, before the server records the flow, like the server's own misconfiguration panics. RFC 9700 §2.1.2 deprecates the implicit grant. The discovery document and the RFC 8414 metadata therefore stay consistent (RFC 9068 §4). A handler built fromProvider.Server()is not checked: the godoc and docs warn never to enable the flow there. A guard would need a newoauth2hook, which the decision left out.Breaking changes
Each has a MIGRATION.md section (before/after table and steps):
stateornonceover its bound or of an invalid form is refused, and a request over 64 KiB is refused./authorizeanswers a POST with 303 (for clients asserting 302).ValidateRedirectURIrefuses get a bare 400.invalid_clientdescriptions are constant.ClientStorereporting "not found" with a plain error now causes a 500; it should return(nil, nil)orErrInvalidClient./introspectand/revokeare bound to the request's issuer. Rollout: resource servers must call/introspect, and clients/revoke, at the issuer of the token.Provider.AuthorizeHandlerwithAllowImplicitpanics at construction.Tests
TDD, with the red phase recorded from the audit probes, rewritten in repository style. Every audit probe of this PR stops reproducing.
Mutation (run in place, always restored):
decisionre-posted by the example.Fuzz:
FuzzLoopbackRedirectandFuzzAuthorizeRedirectTarget. The latter checks that, for every URIValidateRedirectURIaccepts,code,stateandisscome back exactly once; 13.3 M executions were clean.The new test doubles are the generated mockery mocks (
MockClientStore,MockClient). Hand-written decorators delegating to a real store remain (writeCountingStore,savedCodes,lookupFailingStore): they are not plain doubles.The independent adversarial code review found 0 blocker, 0 major, 5 minor and 7 nits, all resolved in
55c717d..5e98bd3:?swallowed the code;A focused re-review of the fixes found one more docs-only minor point, RR1: it scoped the implicit-flow statements to
Provider.AuthorizeHandlerand added theServer()warning. It is fixed in5e98bd3.Opt-in runs by the reviewer, before the post-review fixes (which change no store code): real Redis 7.4 storage conformance PASS; Tales OIDC browser suite 21 scenarios PASS.
Checks
make generate sync build test lint check-moduleson Go 1.27.2 with golangci-lint v2.14.0. All green:check-modules: ok;make sync: no changes;-db file://on a freshvuln.go.devsnapshot; the CI workflow runs it online.oauth2oauth2/clientauthoauth2/grantoauth2/storage/memoryoauth2/storetestoauth2/tokenoauth2/store/sqloauth2/store/redisoidc/providerexamples/oauth2Every new or changed function is at 100 %, except error paths that cannot be reached: the
json.Encoderwrite error afterWriteHeader(serveIntrospect,serveMetadata) and thecrypto/randfailure.Follow-ups (out of scope)
oidcprovider.MaxNonceLengthduplicatesoauth2.MaxNonceLength(both 512). Aliasing it is a nit left for later.Notes for the maintainer
Two tests from PR G (merged meanwhile) were adapted.
TestPromptIsReadWholesends a 1 MiBprompt, so it now raisesMaxRequestBytes: the prompt has no bound of its own, but the request does.offline_accesssubtest of an implicit request needed an implicit-enabled provider, which UD-1 refuses. The provider's code-flow rule is now unit-tested directly instead.G's docs on that rule now note that the provider serves no implicit flow.
OAUTH-I11.
/revokeanswers a token of another client, or another issuer, exactly like an unknown token: 200, nothing revoked. RFC 7009 §2.1 says to refuse such a request with an error, but an error would confirm the token is valid for someone else. The gain against probing is small: the caller already holds the token, and/introspectis closed to public clients. Revisit if you prefer the RFC's error.Deploying OAUTH-05. Point every resource server at its realm's
/introspectbefore deploying. A single shared URL now answers{"active":false}for the other realms' tokens.TrustedRedirectis opt-in: set it when third parties register clients.Under Fixed.
8760cde, typeddocs(oauth2), also changesDefaultClient.SecretMatches(timing) and adds a test; CHANGELOG lists it under Fixed. Three intermediate commit subjects exceed 72 characters, which the squash makes moot.