diff --git a/CHANGELOG.md b/CHANGELOG.md
index e4e7c920..71a8e3af 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -91,7 +91,8 @@ legacy packages (`authentication/`, `authorization/`, the in-tree
never the token value). A denial answers a bare `{"active":false}`, the
same bytes as an unknown token; a policy error fails closed the same way
and reaches `OnError` as a `server_error`. Optional: without a policy, any
- authenticated confidential client may introspect any token.
+ authenticated confidential client may introspect any access token of the
+ request's issuer, and the refresh tokens issued to it.
- **OAuth2 token-family primitives**: `oauth2.ErrTokenFamilyRevoked`
(`invalid_grant`) refuses an operation on a revoked token family;
`oauth2.ErrTokenFamilyRevocationFailed`, a plain sentinel that never
@@ -346,8 +347,8 @@ legacy packages (`authentication/`, `authorization/`, the in-tree
`ErrInvalidRequestURI`, `ErrInvalidRequestObject`).
- **OAuth2 grant context on codes and tokens**: `AuthorizationCode`,
`AccessToken` and `RefreshToken` gain `Issuer` and `ExtensionData`;
- `AuthorizeRequest.Issuer` and `IntrospectedToken.Issuer` expose the realm
- (an `IntrospectionPolicy` can now enforce it, RFC 7662 §4).
+ `AuthorizeRequest.Issuer` and `IntrospectedToken.Issuer` expose the realm,
+ which `/introspect` and `/revoke` enforce (see Changed).
- **OAuth2 refresh-token gate**: `grant.Config.RefreshTokenScope` makes the
`authorization_code` and legacy `password` grants issue a refresh token
only when the granted scope holds that scope token — e.g.
@@ -469,6 +470,22 @@ legacy packages (`authentication/`, `authorization/`, the in-tree
`security.ErrUnavailable`); a
`jwtsec.RemoteJWKS.Fetch` span; fuzz targets for the verifier, the claims
decoder and NumericDate. See Security and Changed.
+- **OAuth2 request bounds**: `oauth2.ServerConfig.MaxRequestBytes` (default
+ `oauth2.DefaultMaxRequestBytes`, 64 KiB) bounds the query and the body of
+ every endpoint; `oauth2.MaxNonceLength` (512), `oauth2.MaxStateLength`
+ (4096) and `oauth2.MaxClientIDLength` (255) document the bounds of the
+ values the server keeps, echoes or looks up.
+- **`oauth2.ValidateRedirectURI`** (and `oauth2.ErrInvalidRedirectURI`)
+ checks a redirect URI at registration — absolute, no fragment, a host, no
+ userinfo and a port within 1–65535 for http(s), "." in a private-use
+ scheme (RFC 8252 §8.4), no response parameter in its query; `/authorize`
+ applies it to the URI a request resolves to.
+- **`oauth2.AuthorizeConfig.TrustedRedirect`**: for a client it does not
+ trust, the errors `/authorize` detects before the resource owner
+ authenticated — and `prompt=none`-style refusals (`login_required`,
+ `interaction_required`, `account_selection_required`, `consent_required`)
+ — are answered with a bare error page instead of a redirect (RFC 9700
+ §4.11.2). Without it, every registered client is trusted, as before.
### Changed
@@ -761,6 +778,55 @@ legacy packages (`authentication/`, `authorization/`, the in-tree
provider may release, and a grant records only those; the values of a
voluntary `acr` request join `Request.ACRValues`, which now holds each
value once. See [MIGRATION.md](MIGRATION.md).
+- **OAuth2 requests and parameters are bounded (breaking)**: a query or
+ body over `ServerConfig.MaxRequestBytes` (64 KiB by default) is refused
+ with `invalid_request` (a bare 400 at `/authorize`) before it is parsed; a
+ `nonce` over 512 bytes, not UTF-8 or holding NUL, and a `state` over 4096
+ bytes or outside VSCHAR are refused with a redirected `invalid_request` —
+ the state not echoed; a `client_id` over 255 bytes is an unknown client;
+ the PKCE parameters of an implicit request, and a `code_challenge_method`
+ without `code_challenge`, are ignored. See [MIGRATION.md](MIGRATION.md).
+- **OAuth2 `/authorize` answers a POST with 303 (breaking for clients
+ asserting 302)** and lists `Allow: GET, POST` on its 405. See
+ [MIGRATION.md](MIGRATION.md).
+- **OAuth2 registered redirect URIs are validated (breaking)**: one that
+ `oauth2.ValidateRedirectURI` refuses — relative, with a fragment, an
+ http(s) URI without host or with a port outside 1–65535, a scheme without
+ a period such as `myapp:`, `javascript:` or the out-of-band
+ `urn:ietf:wg:oauth:2.0:oob`, a query naming a response parameter — is
+ answered with a bare 400, never redirected to. See
+ [MIGRATION.md](MIGRATION.md).
+- **OAuth2 failed client authentications read alike**: every `401
+ invalid_client` of `/token`, `/revoke` and `/introspect` carries the
+ description "client authentication failed"; the reason ("unknown client",
+ "method not allowed for client"…) moved to the cause `OnError` receives.
+ See [MIGRATION.md](MIGRATION.md).
+- **OAuth2 client-store failures are outages (breaking for stores reporting
+ "not found" with an error)**: a `ClientStore` error not carrying
+ `invalid_client` is answered as a failure — a bare 500 at `/authorize`,
+ `server_error` at `/token` and `/introspect`, `temporarily_unavailable` at
+ `/revoke` — and reported to `OnError`, instead of an unknown client. See
+ [MIGRATION.md](MIGRATION.md).
+- **OAuth2 `/introspect` and `/revoke` are bound to the issuer of the
+ request (breaking for multi-issuer deployments)**: a token issued under
+ another issuer introspects `{"active":false}` and is not revoked; without
+ an `IntrospectionPolicy`, a refresh token is disclosed to its own client
+ only; a token recorded without issuer is inactive at a non-empty issuer.
+ Resource servers call `/introspect` at the issuer of the tokens they
+ accept. See [MIGRATION.md](MIGRATION.md).
+- **OAuth2 metadata answers GET and HEAD only (breaking for other
+ methods)**: any other method gets 405 with `Allow: GET, HEAD` (RFC 8414
+ §3.1). See [MIGRATION.md](MIGRATION.md).
+- **The OpenID Provider's authorization endpoint serves no implicit flow
+ (breaking for `Profile20` providers enabling it)**:
+ `oidcprovider.Provider.AuthorizeHandler` panics when
+ `AuthorizeConfig.AllowImplicit` is set — RFC 9700 §2.1.2 deprecates the
+ implicit grant — so the plain OAuth 2.0 `response_type=token` requests it
+ serves get `unsupported_response_type` too, and the RFC 8414 metadata and
+ the discovery document both advertise the code flow alone (RFC 9068 §4).
+ A handler built from `Provider.Server()` is not checked and must not
+ enable the implicit flow either: the metadata would then contradict the
+ discovery document. See [MIGRATION.md](MIGRATION.md).
### Fixed
@@ -923,6 +989,37 @@ legacy packages (`authentication/`, `authorization/`, the in-tree
`invalid_request` or, for `client_id`, `redirect_uri` and
`response_mode`, a bare `400`; it is now served like `nonce=n-1`. Two
occurrences with a value are still refused.
+- OAuth2 redirects keep the query of the registered redirect URI byte for
+ byte (RFC 6749 §3.1.2: it "MUST be retained"): it was re-encoded —
+ reordered, `;`-separated pairs dropped, `flag` turned into `flag=`. The
+ implicit flow no longer appends a second fragment.
+- OAuth2 accepts any port for a registered loopback IP redirect URI
+ (`http://127.0.0.1`, `http://[::1]`), as RFC 8252 §7.3 requires for native
+ apps: the request is otherwise matched exactly, and the code bound to the
+ URI requested.
+- The OAuth2 RFC 8414 metadata lists what the server serves: the `token`
+ response type, the `fragment` response mode and the `implicit` grant type
+ once an `AuthorizeHandler` enabling the implicit flow is built, and the
+ client authentication methods of `/revoke` and `/introspect` (without
+ `none` for the latter), whose absence meant `client_secret_basic` alone
+ (RFC 8414 §2); its grant types are sorted, no longer in map order. The
+ godoc gives the `jwks_uri` and `.well-known` locations actually built.
+- The OpenID Provider discovery document agrees with the RFC 8414 metadata
+ again (RFC 9068 §4): it publishes
+ `revocation_endpoint_auth_methods_supported` — whose absence meant
+ `client_secret_basic` alone (RFC 8414 §2) — and
+ `introspection_endpoint_auth_methods_supported` (without `none`), and
+ sorts `grant_types_supported`.
+- `oauth2.DefaultClient.SecretMatches` no longer discloses the length of
+ the registered secret through its timing: `subtle.ConstantTimeCompare`
+ answers at once when the lengths differ, so it now compares the SHA-256
+ digests of the two secrets, in constant time.
+- OAuth2 storage failures stop hiding: a token lookup that failed at
+ `/introspect` answered `{"active":false}` without reaching `OnError` (it
+ still answers so — RFC 7662 §2.1 — and is now reported), and a failing
+ `ClientStore` read as an unknown client — `invalid_client`, which tells a
+ client its credentials are wrong — although the `ClientAuthenticator`
+ contract asked to return store failures as such.
### Removed
@@ -1456,7 +1553,8 @@ legacy packages (`authentication/`, `authorization/`, the in-tree
declined offline access. `Request.OfflineAccess` is now set for every
request; `OfflineAccessPrompted` needs no `prompt=consent` for a plain
request, `OfflineAccessByPolicy` keeps the former behaviour, an implicit
- request never keeps it (Core §11), and an approval left without any
+ request never keeps it (Core §11; `Provider.AuthorizeHandler` now serves
+ no implicit flow, see Changed), and an approval left without any
scope answers `access_denied` (breaking, see
[MIGRATION.md](MIGRATION.md)).
- `examples/oidc-provider` grants offline access only on the end-user's
@@ -1475,3 +1573,53 @@ legacy packages (`authentication/`, `authorization/`, the in-tree
entry for the life of the process. It compares the password of an
unknown username with a dummy hash — the response time told whether an
account existed — and its HTTP server sets read, write and idle timeouts.
+- The in-memory OAuth2 store (`oauth2/storage/memory`) keeps tokens hashed
+ only. It kept the raw access and refresh tokens it was handed — and
+ returned them from `LookupAccessToken` and `LookupRefreshToken` — contrary
+ to the storage contract ("MUST store hashes"); codes were already stored
+ without their raw value. The `oauth2/storetest` conformance suite now
+ checks that no lookup returns a raw token and that a store leaves the
+ caller's pair untouched: a custom store keeping raw tokens fails it.
+- OAuth2 requests can no longer make the server parse, echo or persist
+ megabytes. Every endpoint read up to 10 MB of form; `/authorize` echoed any
+ `state` in its redirects and recorded any `nonce` with the code — persisted
+ by every store, re-encoded by the Redis consume script (tens of
+ milliseconds of a blocked server per redemption, about 100 such codes
+ filling 1 GB of a `noeviction` Redis, whose writes — revocations included
+ — then fail). `oidc/provider` bounded the nonce of OpenID requests only.
+ Requests are now bounded to 64 KiB, and the state and the nonce of every
+ authorization request to 4096 and 512 bytes.
+- OAuth2 `/authorize` responses can no longer be framed, cached or leak
+ their URL in `Referer`. The endpoint set no header while the
+ `ConsentFunc` rendered the login and consent pages behind it, and the
+ canonical example approved any POST carrying `decision=approve` — a
+ cross-site form could approve on the user's behalf. Every response now
+ carries `X-Frame-Options: DENY`, `Content-Security-Policy:
+ frame-ancestors 'none'`, `Cache-Control: no-store` and `Referrer-Policy:
+ same-origin` (RFC 6749 §10.13, RFC 9700 §4.16, §4.2.4); the `ConsentFunc`
+ godoc states the CSRF contract (RFC 6749 §10.12), and `examples/oauth2`
+ refuses a cross-origin decision. A redirect answering a POST is a 303
+ (RFC 9700 §4.12).
+- OAuth2 no longer redirects to registered redirect URIs that cannot be
+ safe ones: a relative URI, one with a fragment, a `javascript:` or `data:`
+ URI, or a query naming a response parameter (which then traveled twice)
+ were used as registered.
+- OAuth2 `/authorize` can stop redirecting to clients it does not trust
+ before the resource owner authenticated (RFC 9700 §4.11.2: the
+ authorization server "MUST always authenticate the user first"). Any
+ registered client could send users an erroneous or `prompt=none` request
+ and have the server redirect them — without any authentication — to its
+ redirect URI: `AuthorizeConfig.TrustedRedirect` answers such requests of
+ untrusted clients with a bare error page.
+- OAuth2 client authentication no longer enumerates clients: the
+ `invalid_client` descriptions told an unknown client ("unknown client")
+ from a known one with a wrong secret, and disclosed a client's type and
+ methods ("none method reserved to public clients", "method not allowed
+ for client"). Every failed client authentication now reads "client
+ authentication failed" (RFC 6749 §5.2); the detail goes to `OnError`.
+- OAuth2 realms no longer leak through `/introspect` and `/revoke`: behind
+ an `IssuerResolver` serving several issuers, a client registered in two
+ realms could introspect — and revoke — the tokens of one at the other,
+ while the grants refused them there (OIDC Core §16.15). And without an
+ `IntrospectionPolicy`, any confidential client — every resource server —
+ could read the claims of any client's refresh token (RFC 6749 §1.5).
diff --git a/CLAUDE.md b/CLAUDE.md
index f78c4260..796ef7a7 100644
--- a/CLAUDE.md
+++ b/CLAUDE.md
@@ -271,9 +271,20 @@ configurable via `ServerConfig.RoutePrefix`).
- Access/refresh tokens and authz codes are stored **hashed** (`HashToken`).
- Every authorization is **bound to its issuer**: every endpoint resolves it
first (the `IssuerResolver` is the realm), codes and tokens record it, and
- the `authorization_code` / `refresh_token` grants refuse it elsewhere. A
- client implementing `IssuerBoundClient` (`DefaultClient.IssuerValues`) is
- unknown at other issuers — same bytes as an unknown `client_id`.
+ the `authorization_code` / `refresh_token` grants refuse it elsewhere;
+ `/introspect` answers another issuer's token inactive and `/revoke` leaves
+ it alone. A client implementing `IssuerBoundClient`
+ (`DefaultClient.IssuerValues`) is unknown at other issuers — same bytes as
+ an unknown `client_id`.
+- **Hardened endpoints**: every request is bounded
+ (`ServerConfig.MaxRequestBytes`, 64 KiB; `state`, `nonce`, `client_id`
+ bounds); `/authorize` sets anti-framing / no-store / `Referrer-Policy:
+ same-origin` headers, answers a POST with 303, validates redirect URIs
+ (`ValidateRedirectURI`) and, with `AuthorizeConfig.TrustedRedirect`,
+ redirects no error of an untrusted client before authentication. Every
+ failed client authentication answers the same `invalid_client` ("client
+ authentication failed", detail in `OnError`); a `ClientStore` error not
+ carrying `invalid_client` is an outage (500/503), not an unknown client.
- **Extension points** (`ServerConfig.Extension`, `oauth2.Extension`): the
contract `oidc/provider` builds on — `ValidateAuthorizeRequest` after the
core checks, `CompleteAuthorization` turning `Consent.ExtensionResult` into
diff --git a/LIMITATIONS.md b/LIMITATIONS.md
index 71113ca2..d9bae009 100644
--- a/LIMITATIONS.md
+++ b/LIMITATIONS.md
@@ -11,8 +11,27 @@ future refactor phases.
- **`private_key_jwt` client authentication (RFC 7523)** — not implemented.
`client_secret_basic`, `client_secret_post`, and `none` are.
+- **No throttling of client authentication** — `/token`, `/revoke` and
+ `/introspect` verify a client secret on every request without limiting
+ attempts: RFC 6749 §2.3.1 asks for brute-force protection, which a rate
+ limiter in front of the handlers provides (per `client_id` and source
+ address). Failures reach `OnError`.
+- **No hashed-secret client** — `DefaultClient` keeps its secret in
+ cleartext (compared by SHA-256 digest in constant time); keeping only a
+ hash at rest takes a `Client` implementing `SecretMatcher` over your
+ store — see [docs/security-considerations.md](docs/security-considerations.md).
+- **Another client's token is "revoked" with 200** — `/revoke` leaves a
+ token issued to another client, or under another issuer, untouched and
+ answers it as an invalid token (RFC 7009 §2.2) where RFC 7009 §2.1 refuses
+ the request with an error: a deliberate deviation, so that the answer
+ never confirms the token is valid for someone else.
+- **A failed introspection lookup answers inactive** — when the store fails
+ to look a token up, `/introspect` answers `{"active":false}` (RFC 7662
+ §2.1) and reports the failure to `OnError`: the resource server cannot
+ tell an outage from a revoked token, and refuses the request either way.
- **`/.well-known/jwks.json` endpoint** — the `oauth2` module alone serves
- none, although its RFC 8414 metadata advertises `jwks_uri`: mount the
+ none, although its RFC 8414 metadata advertises `jwks_uri` (the issuer
+ followed by `/.well-known/jwks.json`): mount the
`oidc/provider` `JWKSHandler`, which serves the issuer's public keys, or
publish your own with `jwtsec.MarshalJWKS` (it refuses private and
symmetric keys).
@@ -48,6 +67,14 @@ future refactor phases.
PKCE verifier) can make the legitimate exchange fail with
`invalid_grant`; it obtains nothing. A deliberate trade-off, see
[docs/security-considerations.md](docs/security-considerations.md).
+- **Every registered client is trusted to receive error redirects** — an
+ error `/authorize` detects before the resource owner authenticated is
+ redirected to the client's registered redirect URI unless
+ `AuthorizeConfig.TrustedRedirect` says the client is not trusted (RFC
+ 9700 §4.11.2); set it when clients are registered by third parties. An
+ explicit denial after authentication (`access_denied`) is redirected to
+ every client — RFC 9700 §4.11.2's second attack: a `ConsentFunc` may warn
+ the resource owner about a client it does not trust.
- **RFC 9207 `iss` needs a resolved issuer** — every authorization response
names its issuer, except the `server_error` redirected when the
`IssuerResolver` itself fails: no issuer is known then, so it travels
@@ -64,17 +91,16 @@ future refactor phases.
several issuers, a client is known at every issuer unless it implements
`IssuerBoundClient` (`DefaultClient.IssuerValues`) or the store is scoped
by issuer.
-- **`/introspect` returns no `iss`** — the token's issuer reaches an
+- **`/introspect` returns no `iss`** — an active token is always of the
+ issuer the request resolved to; the token's issuer reaches an
`IntrospectionPolicy` (`IntrospectedToken.Issuer`), not the response body.
-- **The `nonce` parameter is not bounded by the core** — `/authorize` records
- it as sent; `oidc/provider` bounds it in OpenID requests (512 bytes).
- **`oauth2.Extension` scope** — it extends the `authorization_code` and
`refresh_token` exchanges; the implicit flow runs its authorization hooks
and records their context, but its fragment members are fixed (RFC 6749
§4.2.2), and the `client_credentials` and `password` grants never call it.
- Response modes: `query` for the code flow — the only one the metadata
- advertises — and `fragment` only for an enabled implicit flow; `form_post`
- is refused.
+ Response modes: `query` for the code flow, and `fragment` only for an
+ enabled implicit flow — which the metadata advertises once an
+ `AuthorizeHandler` enabling it is built; `form_post` is refused.
## OpenID Provider
@@ -83,8 +109,14 @@ future refactor phases.
matrix. Not supported, and never advertised:
- **Other flows and response modes** — the implicit and hybrid flows
- (`unsupported_response_type`), `form_post`, `fragment` and JARM response
- modes (refused by the core with a bare 400).
+ (`unsupported_response_type`, for plain OAuth 2.0 requests too:
+ `Provider.AuthorizeHandler` refuses `AuthorizeConfig.AllowImplicit`),
+ `form_post`, `fragment` and JARM response modes (refused by the core with
+ a bare 400). Nothing refuses `AllowImplicit` on a handler built from
+ `Provider.Server()`: never enable it there — the server would serve the
+ implicit flow to plain OAuth 2.0 requests and advertise it in its RFC 8414
+ metadata, which the discovery document would then contradict (RFC 9068
+ §4).
- **Request objects and registration** — `request` and `request_uri` are
refused (`request_not_supported`, `request_uri_not_supported`), and so is
the `registration` parameter; clients are static (`oauth2.ClientStore`),
diff --git a/MIGRATION.md b/MIGRATION.md
index 64a0fd53..e5cc3eaa 100644
--- a/MIGRATION.md
+++ b/MIGRATION.md
@@ -239,14 +239,16 @@ To migrate:
and `scope` from the `/token` response, or ask a confidential backend.
3. When several resource servers or tenants share the authorization server,
set `ServerConfig.IntrospectionPolicy`: without one, any authenticated
- confidential client may introspect any token. RFC 7662 §4 says protected
- resources SHOULD be specifically authorized, and when a token can be used
- only at certain resource servers, the authorization server MUST determine
- whether it can be used at the one making the call. A denial answers a bare
- `{"active":false}`, the same bytes as an unknown token; a policy error
- fails closed the same way and reaches `OnError`, so wire both. A typical
- policy discloses access tokens to the resource server they are meant for,
- and refresh tokens to none:
+ confidential client may introspect any access token of the request's
+ issuer, and the refresh tokens issued to it (see
+ [below](#oauth2-introspect-and-revoke-are-bound-to-the-issuer-of-the-request)).
+ RFC 7662 §4 says protected resources SHOULD be specifically authorized,
+ and when a token can be used only at certain resource servers, the
+ authorization server MUST determine whether it can be used at the one
+ making the call. A denial answers a bare `{"active":false}`, the same
+ bytes as an unknown token; a policy error fails closed the same way and
+ reaches `OnError`, so wire both. A typical policy discloses access tokens
+ to the resource server they are meant for, and refresh tokens to none:
```go
srv, err := oauth2.NewServer(oauth2.ServerConfig{
@@ -269,8 +271,9 @@ To migrate:
Do not reduce the check to `caller.ID() == tok.ClientID`: the resource
server introspecting a token is usually not the client it was issued to,
so that rule refuses the legitimate calls without enforcing any resource
- or tenant boundary. The policy only sees active tokens, only after the
- caller authenticated as a confidential client, and gets the request
+ or tenant boundary. The policy only sees active tokens of the request's
+ issuer, only after the caller authenticated as a confidential client,
+ and gets the request
context; see the `oauth2.IntrospectionPolicy` godoc for the full
contract.
@@ -729,7 +732,7 @@ issuer the client does not exist:
| Request at an issuer the client is not registered at | Before | After |
| ---------------------------------------------------- | ------ | ----- |
| `/authorize` | served like at its own issuer | bare `400` "unknown or invalid client", never redirected — the same bytes as for a `client_id` that exists nowhere (RFC 6749 §4.1.2.1) |
-| `/token`, `/revoke`, `/introspect` | authenticated | `401 invalid_client` "unknown client", whatever the secret — the same answer as for an unknown client (RFC 6749 §5.2) |
+| `/token`, `/revoke`, `/introspect` | authenticated | `401 invalid_client`, whatever the secret — the same answer as for an unknown client (RFC 6749 §5.2); the description read "unknown client" until every failed client authentication was answered "client authentication failed" (see below) |
| any of them | — | `OnError` gets the refusal with `oauth2.ErrClientNotAtIssuer` as its cause |
The server now resolves the issuer of a request before it loads its client:
@@ -1772,9 +1775,9 @@ has one meaning at the provider's server:
| plain OAuth 2.0, `Decision.OfflineAccess` unset | `offline_access` granted | dropped from the grant (no refresh token with `RefreshTokenScope = offline_access`) |
| plain OAuth 2.0, only `offline_access` requested, `Decision.OfflineAccess` unset | code issued | `access_denied`, nothing issued |
| plain OAuth 2.0, `OfflineAccessPrompted` or `OfflineAccessByPolicy` | granted | granted (`prompt=consent` is not required: no OAuth 2.0 parameter) |
-| plain OAuth 2.0 implicit request (`response_type=token`) | `offline_access` as approved | always dropped (Core §11: it needs a response type returning a code) |
+| plain OAuth 2.0 implicit request (`response_type=token`) | `offline_access` as approved | always dropped (Core §11: it needs a response type returning a code) — and `Provider.AuthorizeHandler` now serves no implicit flow ([below](#oidcprovider-providerauthorizehandler-serves-no-implicit-flow)) |
| OpenID request | §11 rules | unchanged |
-| plain OAuth 2.0 request approved by another `ConsentFunc` (`Server().AuthorizeHandler`) | OAuth 2.0 semantics | unchanged |
+| plain OAuth 2.0 request approved by another `ConsentFunc` (`Server().AuthorizeHandler`) | OAuth 2.0 semantics | unchanged — never enable `AllowImplicit` on that handler ([below](#oidcprovider-providerauthorizehandler-serves-no-implicit-flow)) |
Only the scope is gated: a plain OAuth 2.0 grant carries no provider context,
so its refresh tokens still follow `grant.Config.RefreshTokenScope` alone.
@@ -1810,3 +1813,187 @@ separator, and the three hint lists are bounded:
To migrate: nothing for clients sending ordinary values. A client sending
more than 2048 bytes in one of these hints sends fewer: they never fail a
request (OIDC Core §3.1.2.1, §15.1).
+### `oauth2`: request sizes and parameters are bounded
+
+The endpoints parsed requests of any size up to the 10 MB `net/http` form
+limit, echoed any `state` in their redirects and recorded any `nonce` with
+the code — persisted by every store and re-encoded by the Redis scripts.
+
+| | Before | After |
+| --- | --- | --- |
+| query or body over `ServerConfig.MaxRequestBytes` (default 64 KiB) | parsed (up to 10 MB of body) | `invalid_request` "request too large" — a bare 400 at `/authorize` — before anything is read |
+| `nonce` over 512 bytes, invalid UTF-8 or holding NUL | recorded with the code (OpenID requests through `oidc/provider` excepted) | redirected `invalid_request`, every request |
+| `state` over 4096 bytes or outside VSCHAR (`%x20-7E`) | echoed in every redirect | redirected `invalid_request`, without `state` |
+| `client_id` over 255 bytes | looked up | an unknown client, never looked up |
+| `code_challenge` / `code_challenge_method` of an implicit request | handed to the `ConsentFunc` | ignored |
+| `code_challenge_method` without `code_challenge` (`Profile20`) | recorded with the code | ignored |
+
+To migrate:
+
+1. Grants carrying large assertions (a custom SAML or JWT bearer grant) set
+ `ServerConfig.MaxRequestBytes` to what they need.
+2. Clients sending a non-ASCII `state`, or one over 4 KiB, encode a shorter
+ value — a random key to their own state, as RFC 6749 §10.12 suggests.
+3. Extensions relying on the core's `nonce` (`AuthorizeRequest.Nonce`,
+ `TokenIssuance.Nonce`) may drop their own bound: the core applies the one
+ `oidc/provider` applies.
+
+### `oauth2`: `/authorize` responses carry protective headers; POST redirects are 303
+
+| | Before | After |
+| --- | --- | --- |
+| headers of every `/authorize` response (pages, redirects, errors) | none | `X-Frame-Options: DENY`, `Content-Security-Policy: frame-ancestors 'none'` (added to an existing policy), `Cache-Control: no-store`, `Referrer-Policy: same-origin` |
+| a redirect answering a POST | `302 Found` | `303 See Other` (RFC 9700 §4.12) |
+| another method than GET and POST | `405` | `405` with `Allow: GET, POST` |
+
+To migrate:
+
+1. A `ConsentFunc` whose page must be framed by a trusted origin replaces
+ `X-Frame-Options` and `Content-Security-Policy` on `w.Header()` before
+ writing it; one setting its own `Content-Security-Policy` with `Set`
+ keeps `frame-ancestors` in it.
+2. Protect the request carrying the decision against CSRF if your
+ `ConsentFunc` does not (RFC 6749 §10.12): a session-bound token, or
+ `net/http.CrossOriginProtection.Check` on that request — not on the
+ endpoint, which clients may POST authorization requests to.
+3. Clients and tests expecting `302` after a POST to `/authorize` accept
+ `303`; user agents follow both with a GET.
+
+### `oauth2`: registered redirect URIs are validated; their query is kept; loopback ports vary
+
+The authorization endpoint redirected to whatever a client registered — a
+relative URI, one with a fragment (the implicit flow then answered
+`cb#section#access_token=…`), `javascript:` — rebuilt the registered query
+(reordered, `a=1;b=2` dropped, `flag` turned into `flag=`, a registered
+`state` or `code` overwritten), and refused a loopback redirect URI whose
+port differed from the registered one, which RFC 8252 §7.3 requires.
+
+| | Before | After |
+| --- | --- | --- |
+| registered URI that `oauth2.ValidateRedirectURI` refuses: relative, with a fragment, http(s) without host (`http://:80/cb`), with userinfo or a port outside 1–65535, a scheme without "." (`myapp:`, `javascript:`, `data:`, the out-of-band `urn:ietf:wg:oauth:2.0:oob`), invalid escapes, a query naming `code`, `state`, `iss`, `error`, `error_description` or `error_uri` | redirected to | bare `400` "oauth2: invalid redirect_uri", never redirected |
+| registered query | re-encoded, sorted, `;`-separated pairs dropped | kept byte for byte, the response parameters appended |
+| `http://127.0.0.1:{port}/…` / `http://[::1]:{port}/…` requested for a registered loopback IP URI | `400` unless the port matched exactly | accepted for any port 1–65535, the rest unchanged; the code is bound to the URI requested |
+| `http://localhost:{port}/…` | exact match | exact match (no variance, RFC 8252 §8.3) |
+
+To migrate:
+
+1. Run `oauth2.ValidateRedirectURI` over every registered redirect URI —
+ and at registration from now on — and fix what it refuses.
+2. Native apps register a reverse domain name scheme
+ (`com.example.app:/oauth2redirect`, RFC 8252 §7.1) instead of `myapp:` —
+ and instead of the out-of-band `urn:ietf:wg:oauth:2.0:oob`, which RFC
+ 8252 §7 does not keep — or a loopback IP literal instead of `localhost`
+ to get port variance.
+3. A registered query parameter named like a response parameter is
+ renamed: the response would carry it twice (RFC 6749 §3.1).
+4. A loopback client exchanging a code sends, at `/token`, the redirect URI
+ with the port it used at `/authorize`.
+
+### `oauth2`: failed client authentications read alike
+
+`/token`, `/revoke` and `/introspect` described each failed client
+authentication: "unknown client", "client authentication failed" (a wrong
+secret), "method not allowed for client", "none method reserved to public
+clients", "client cannot verify secret", "introspection requires
+confidential client authentication"… — telling whether a client exists, and
+its type and authentication methods.
+
+| | Before | After |
+| --- | --- | --- |
+| `error_description` of any `401 invalid_client` | the reason | `client authentication failed` |
+| the reason | in the body | in the cause of the `invalid_client` `OnError` receives: `errors.As` on its `Unwrap` finds the detailed `*oauth2.Error`; `oauth2.ErrClientNotAtIssuer` still matches with `errors.Is` |
+
+To migrate: clients and tests that branched on the `error_description` of an
+`invalid_client` use the `error` code only; diagnostics read the detail from
+`OnError`.
+
+### `oauth2`: client-store and token-lookup failures are reported as failures
+
+| | Before | After |
+| --- | --- | --- |
+| `ClientStore.LoadClient` error (other than one carrying `invalid_client`) at `/authorize` | bare `400` "unknown or invalid client" | bare `500` "oauth2: server error", `OnError` `server_error` |
+| … at `/token`, `/introspect` | `401 invalid_client` | `500 server_error`, `OnError` |
+| … at `/revoke` (and an `IssuerResolver` failure there) | `401 invalid_client` (`500` for the resolver) | `503 temporarily_unavailable` (RFC 7009 §2.2.1), `OnError` |
+| a failed token lookup at `/introspect` | `{"active":false}`, unreported | `{"active":false}`, reported to `OnError` |
+
+To migrate:
+
+1. A `ClientStore` reports an unknown client with `(nil, nil)` — or with an
+ error carrying `invalid_client` (`oauth2.ErrInvalidClient`): one returning
+ a plain "not found" error (`sql.ErrNoRows`…) now makes every unknown
+ `client_id` a 500 — map it.
+2. Custom `ClientAuthenticator`s return their store's failures as such,
+ not wrapped in `invalid_client` (the contract already said so).
+
+### `oauth2`: `/introspect` and `/revoke` are bound to the issuer of the request
+
+Codes and tokens record their issuer since #83, and the grants refuse them at
+another issuer — but `/introspect` and `/revoke` ignored it: behind an
+`IssuerResolver` serving several realms, a client registered in two of them
+could introspect or revoke the tokens of one at the other. And without an
+`IntrospectionPolicy`, any confidential client — every resource server —
+could read the claims of any refresh token, although refresh tokens are
+meant for the authorization server and their client (RFC 6749 §1.5).
+
+| | Before | After |
+| --- | --- | --- |
+| a token of another issuer at `/introspect` | active (subject to the policy) | `{"active":false}`, the policy not consulted |
+| a token of another issuer at `/revoke` | revoked when the caller was its client | left untouched; `200` all the same |
+| another client's refresh token at `/introspect`, no `IntrospectionPolicy` | active, claims disclosed | `{"active":false}` |
+| a token recorded without issuer (written before #83) | active at any issuer | `{"active":false}` at a non-empty issuer; still revoked by its client |
+| `IntrospectedToken.Issuer` seen by a policy | any issuer | always the request's issuer |
+
+To migrate:
+
+1. **Roll out with your resource servers.** Each resource server calls
+ `/introspect` — and each client `/revoke` — at the issuer the token was
+ issued under: the `Host` or path your `IssuerResolver` maps to that
+ realm. A resource server calling one URL for every realm now gets
+ `{"active":false}` for the tokens of the others, and a revocation sent
+ there silently does nothing: point them at each realm's endpoints before
+ upgrading.
+2. A resource server that must introspect other clients' refresh tokens
+ needs an `IntrospectionPolicy` allowing it: a policy replaces the default
+ rule, for access and refresh tokens alike.
+3. Tokens written before #83 carry no issuer: they introspect inactive at a
+ non-empty issuer until they expire.
+
+### `oauth2`: the RFC 8414 metadata lists what the server serves; GET and HEAD only
+
+| | Before | After |
+| --- | --- | --- |
+| a method other than GET or HEAD | `200`, the document | `405`, `Allow: GET, HEAD` |
+| `response_types_supported`, `response_modes_supported` once an `AuthorizeHandler` enabling the implicit flow is built | `["code"]`, `["query"]` | `["code","token"]`, `["query","fragment"]` |
+| `grant_types_supported` | map order, never `implicit` | sorted; `implicit` with the implicit flow |
+| `revocation_endpoint_auth_methods_supported` | absent — meaning `client_secret_basic` alone (RFC 8414 §2) | the `ServerConfig.ClientAuth` methods |
+| `introspection_endpoint_auth_methods_supported` | absent | the same but `none`; absent when nothing is left |
+
+To migrate: serve the document to GET (a Go `GET` route pattern serves HEAD
+too), and build the `AuthorizeHandler`s before serving it — the implicit
+members reflect the handlers built so far.
+
+### `oidc/provider`: `Provider.AuthorizeHandler` serves no implicit flow
+
+A `Profile20` provider could enable the legacy implicit flow for plain OAuth
+2.0 requests (`Provider.AuthorizeHandler(oauth2.AuthorizeConfig{AllowImplicit:
+true, …})`); the oauth2 RFC 8414 metadata then advertised `response_type=token`
+while the OpenID discovery document did not — two documents of one
+authorization server disagreeing, which RFC 9068 §4 forbids. RFC 9700 §2.1.2
+deprecates the implicit grant, and OpenID requests could never use it.
+
+| | Before | After |
+| --- | --- | --- |
+| `Provider.AuthorizeHandler` with `AllowImplicit` (`Profile20`) | the OAuth 2.0 implicit flow served for requests without `openid` | panics at construction: `oidcprovider: AuthorizeHandler: the provider serves no implicit flow (AllowImplicit)` |
+| `response_type=token` at `Provider.AuthorizeHandler` | `unsupported_response_type` for OpenID requests only | `unsupported_response_type` for every request |
+
+To migrate:
+
+1. Drop `AllowImplicit` and `ImplicitTokens` from the configuration of the
+ provider's `AuthorizeHandler`, and move the clients still using the
+ implicit flow to the authorization code flow with PKCE (RFC 9700
+ §2.1.2).
+2. Do not move the implicit flow to a handler built from
+ `Provider.Server().AuthorizeHandler` either: nothing refuses
+ `AllowImplicit` there, and the server would serve the flow to plain OAuth
+ 2.0 requests and advertise it in its RFC 8414 metadata while the
+ discovery document does not — the disagreement RFC 9068 §4 forbids.
diff --git a/docs/architecture.md b/docs/architecture.md
index fd4427e7..31a5f257 100644
--- a/docs/architecture.md
+++ b/docs/architecture.md
@@ -213,9 +213,11 @@ The `oauth2` module is an authorization server, not just a provider:
opt-in legacy `implicit` flow, with an application-supplied consent hook;
every authorization response names its issuer, RFC 9207),
`/token`, `/revoke` (RFC 7009), `/introspect` (RFC 7662, with an optional
- `IntrospectionPolicy` deciding which tokens each caller may see),
- `/.well-known/oauth-authorization-server` (RFC 8414). The endpoint path
- prefix used in the metadata document is configurable via
+ `IntrospectionPolicy` deciding which tokens each caller may see) — the
+ last two bound to the issuer of the request, like the grants —
+ `/.well-known/oauth-authorization-server` (RFC 8414, listing what the
+ server serves: grants, flows, client authentication methods). The
+ endpoint path prefix used in the metadata document is configurable via
`ServerConfig.RoutePrefix`.
- **`Storage`** — an interface with explicit atomicity contracts: a
single-use `ConsumeAuthorizationCode` — consumed codes are retained until
@@ -252,8 +254,9 @@ The `oauth2` module is an authorization server, not just a provider:
travels unchanged through every refresh — the stores refuse a rotation
that would change it.
-Tokens and authorization codes are **never stored in cleartext** — the
-store only ever sees a hash.
+Tokens and authorization codes are **never stored in cleartext** — every
+store, the in-memory one included, keeps their hash only, and the
+`storetest` conformance suite checks that no lookup returns a raw value.
## OpenID Provider
diff --git a/docs/observability.md b/docs/observability.md
index 15c46ad0..e87b2d7c 100644
--- a/docs/observability.md
+++ b/docs/observability.md
@@ -65,9 +65,15 @@ srv, err := oauth2.NewServer(oauth2.ServerConfig{
The hook fires for:
- every error serialized as an RFC 6749 §5.2 body (`/token`, `/revoke`,
- `/introspect`, metadata) and every error redirected back to the client by
+ `/introspect`, metadata) — a failed client authentication as the constant
+ `invalid_client` the wire carries, the refusal it replaced ("unknown
+ client", "method not allowed for client"…) in its cause — and every error
+ redirected back to the client by
`/authorize`, including the pre-redirect refusals answered with a bare
- 400 (unknown client, unregistered `redirect_uri`);
+ 400 (unknown client, unregistered `redirect_uri`, a registered
+ `redirect_uri` `oauth2.ValidateRedirectURI` refuses — `invalid registered
+ redirect_uri`, the `ErrInvalidRedirectURI` refusal as cause — and a
+ request too large or malformed);
- every storage failure at `/revoke`: a token lookup that failed for another
reason than an unknown token (`revoke: token lookup failed`), and a failed
family or access-token revocation (`revoke refresh family failed`,
@@ -84,6 +90,16 @@ The hook fires for:
behind a bare `{"active":false}` and reports a `server_error`
(`introspection policy failed`, the policy's error as cause). A plain
refusal by the policy is an expected outcome and is not reported;
+- a token lookup that failed at `/introspect` — not one that missed: the
+ token answers `{"active":false}` (RFC 7662 §2.1) and the failure is
+ reported as a `server_error` (`introspect: token lookup failed`, cause
+ attached);
+- a `ClientStore` that failed — an error not carrying `invalid_client`:
+ `/authorize` answers a bare 500 (`client lookup failed`), `/token` and
+ `/introspect` a 500 `server_error`, `/revoke` a 503
+ `temporarily_unavailable` (`revoke: client authentication failed`, which
+ also covers an `IssuerResolver` failure there) — one `server_error`
+ carrying the store's error;
- at `/authorize`, an unsupported `response_mode` (`invalid_request`, answered
with a bare 400), a failing `IssuerResolver`, a failing `ConsentFunc`
(`consent handler failed`, its error as cause), a `Consent.Refusal` (as
@@ -91,7 +107,9 @@ The hook fires for:
`oauth2.Extension` hook — a protocol code as returned, anything else as a
`server_error` (`authorization extension failed`) carrying it. An error
whose code the endpoint cannot send reaches the hook as a `server_error`
- whose cause is the original;
+ whose cause is the original. An error answered with a bare page because
+ `AuthorizeConfig.TrustedRedirect` does not trust the client reaches the
+ hook exactly as it would have been redirected;
- at `/token`, an `ExtendTokenResponse` failure: its `invalid_grant` as
returned, anything else as a `server_error` carrying it.
diff --git a/docs/oidc-provider.md b/docs/oidc-provider.md
index f7d78342..08b47d3d 100644
--- a/docs/oidc-provider.md
+++ b/docs/oidc-provider.md
@@ -50,13 +50,13 @@ Test names refer to `oidc/provider` unless prefixed with `interop:`
| Authorization Code flow | Core §3.1 | Supported | `TestIDTokenClaims`, interop: `TestInteropCodeFlowS256`, `TestInteropCodeReplay` |
| PKCE | RFC 7636 | As the core's `Profile` decides: `S256` required under `Profile20BCP` (default) and `Profile21Draft`; optional under `Profile20`, which also allows `plain` | `TestDiscoveryPKCEMethodsFollowTheProfile`, interop: `TestInteropPublicClient` |
| Plain OAuth 2.0 requests (no `openid`) | Core §3.1.2.1 | OAuth 2.0 semantics: no ID Token, no UserInfo; the realm rule and the `offline_access` rule only | `TestPlainOAuth2RequestKeepsOAuth2Semantics`, `TestPlainOAuth2CodeExchange`, `TestRefreshOfAPlainOAuth2Grant`, `TestPlainOAuth2OfflineAccessNeedsTheDecision` |
-| Implicit and hybrid flows | Core §3.2, §3.3 | Not supported, not advertised: an OpenID request with another `response_type` gets `unsupported_response_type` | `TestOpenIDImplicitRefused` |
+| Implicit and hybrid flows | Core §3.2, §3.3; RFC 9700 §2.1.2 | Not supported, not advertised: a request with another `response_type` — OpenID or plain OAuth 2.0 — to `Provider.AuthorizeHandler` gets `unsupported_response_type`; it panics on `AuthorizeConfig.AllowImplicit`, so the RFC 8414 metadata does not advertise the implicit flow either (RFC 9068 §4). A handler built from `Server().AuthorizeHandler` is not checked: never enable `AllowImplicit` there — the server would serve the flow to plain OAuth 2.0 requests and its RFC 8414 metadata would contradict the discovery document | `TestOpenIDImplicitRefused`, `TestAuthorizeHandlerServesNoImplicitFlow`, `TestDiscoveryConsistentWithRFC8414Metadata` |
| Response modes | Core §3.1.2.1; OAuth 2.0 Multiple Response Types | `query` only (advertised); the core refuses any other with a bare `400`, never redirected | `TestDiscoveryDocument`, oauth2: `TestAuthorizeRefusesUnsupportedResponseModes` |
-| `redirect_uri` | Core §3.1.2.1 | REQUIRED for OpenID requests: missing → `400`, never redirected — once the core accepted the request: the core's response type, scope and PKCE checks run first and redirect their errors to the client's sole registered redirect URI (OAuth 2.0 behaviour) | `TestOpenIDRequestWithoutRedirectURIIsNotRedirected` |
+| `redirect_uri` | Core §3.1.2.1 | REQUIRED for OpenID requests: missing → `400`, never redirected — once the core accepted the request: the core's state, response type, scope, PKCE and nonce checks run first and redirect their errors to the client's sole registered redirect URI (OAuth 2.0 behaviour) | `TestOpenIDRequestWithoutRedirectURIIsNotRedirected` |
| Repeated parameters | RFC 6749 §3.1 | Refused: `client_id`, `redirect_uri`, `response_mode` → `400`, never redirected — by the oauth2 core itself, before any other check; any other → `invalid_request` (the core's parameters by the core, the OpenID ones by the provider). An empty value is no occurrence, wherever it stands: it is omitted, by the provider as by the core | `TestDuplicatedParameters` |
-| `nonce` | Core §3.1.2.1, §2 | Optional (code flow); at most 512 bytes of UTF-8 without NUL, else `invalid_request`; in the ID Token of the code exchange only | `TestNonceBounds`, `TestIDTokenClaims`, `TestIDTokenWithoutNonceHasNoNonce` |
+| `nonce` | Core §3.1.2.1, §2 | Optional (code flow); at most 512 bytes of UTF-8 without NUL, else `invalid_request` — the oauth2 core enforces the same bound on every request, OpenID or not; in the ID Token of the code exchange only | `TestNonceBounds`, `TestIDTokenClaims`, `TestIDTokenWithoutNonceHasNoNonce` |
| `prompt` | Core §3.1.2.1 | `none`, `login`, `consent`, `select_account` reach the `Interaction`; unknown values ignored; `none` with another value → `invalid_request`; read whole (never truncated), without allocating per separator | `TestPromptParsing`, `TestPromptIsReadWhole` |
-| `prompt=none` | Core §3.1.2.1, §3.1.2.6 | `Interaction.AuthorizeSilently` only — it has no `ResponseWriter`; its refusals (`login_required`, `consent_required`, `interaction_required`, `account_selection_required`) are redirected. The provider's own refusals carry fixed descriptions: no session, a session of another realm, of another end-user or a stale one read the same on the wire (the reason goes to the `ErrorHook`) | `TestPromptNoneIsSilent`, `TestPromptNoneRefusals`, `TestSilentRefusalsRevealNothing`, `TestPromptNoneWithoutDecisionIsAServerError`, interop: `TestInteropPromptNone` |
+| `prompt=none` | Core §3.1.2.1, §3.1.2.6 | `Interaction.AuthorizeSilently` only — it has no `ResponseWriter`; its refusals (`login_required`, `consent_required`, `interaction_required`, `account_selection_required`) are redirected — to a client `oauth2.AuthorizeConfig.TrustedRedirect` trusts: for an untrusted one they are a bare 400, never redirected (RFC 9700 §4.11.2). The provider's own refusals carry fixed descriptions: no session, a session of another realm, of another end-user or a stale one read the same on the wire (the reason goes to the `ErrorHook`) | `TestPromptNoneIsSilent`, `TestPromptNoneRefusals`, `TestSilentRefusalsRevealNothing`, `TestPromptNoneWithoutDecisionIsAServerError`, interop: `TestInteropPromptNone` |
| `prompt=login` | Core §3.1.2.1 | Enforced by the provider: an authentication older than the request (to the second of `auth_time`) → `login_required` | `TestFreshnessIsEnforcedByTheProvider`, `TestRequestCheck`, interop: `TestInteropReauthentication` |
| `max_age` | Core §3.1.2.1, §2 | Enforced (`auth_time + max_age` ≥ request time, to the second); `1*DIGIT`, longer values clamped to 2³¹−1 s, anything else → `invalid_request`; `auth_time` always in the ID Token | `TestMaxAgeParsing`, `TestFreshnessIsEnforcedByTheProvider`, `TestIDTokenCarriesTheAuthenticationEvent`, interop: `TestInteropReauthentication` |
| `id_token_hint` | Core §3.1.2.1, §3.1.2.2 | Verified: signed by a key the issuer publishes, `iss` the issuer, the client among its audiences, a `sub`, typed `JWT` or untyped (an `at+jwt` is refused); its expiry is not checked. It designates the end-user (another one → `login_required`). Invalid → `invalid_request` | `TestIDTokenHint`, `TestIDTokenHintOfARetiredKey`, `TestIDTokenHintDesignatesTheEndUser`, interop: `TestInteropIDTokenHint` |
@@ -68,7 +68,7 @@ Test names refer to `oidc/provider` unless prefixed with `interop:`
| `registration` parameter | Core §7.2.1 | Not supported: `registration_not_supported` | `TestRequestObjectParametersRefused` |
| Realm of the authentication | Core §16.15 | `AuthenticationEvent.Issuer` must be the request's issuer: else `login_required` (OpenID) or `access_denied` (OAuth 2.0) | `TestRealmIsEnforced`, `TestRequestCheck`, stack: `TestRefusalsRevealNoSession` |
| Consent | Core §3.1.2.4 | `Decision`: approval, `Scope` narrowing (keeps `openid`), `Claims`, `Refusal`; an empty decision → `access_denied` | `TestDecisionScopeNarrows`, `TestInteractionFailures`, `TestApprovalContractViolations`, interop: `TestInteropConsentNarrowsScope` |
-| `offline_access` | Core §11 | Every request: kept only when `Decision.OfflineAccess` grants it — the consent dialog (`OfflineAccessPrompted`: `prompt=consent` required for an OpenID request, `prompt` being no OAuth 2.0 parameter) or the application's policy (`OfflineAccessByPolicy`) — else dropped from the grant — and always outside the code flow (Core §11: an implicit request keeps no `offline_access`); an approval left without any scope → `access_denied`. OpenID grants: a refresh token without that consent fails closed (`server_error` at issuance, `invalid_grant` at refresh) unless `Config.AllowRefreshWithoutOfflineAccess`. Plain OAuth 2.0 grants carry no context: only their scope is gated, their refresh tokens follow `grant.Config.RefreshTokenScope` | `TestOfflineAccessRequiresConsent`, `TestPlainOAuth2OfflineAccessNeedsTheDecision`, `TestRequestReportsOfflineAccessForEveryRequest`, `TestRefreshWithoutOfflineConsentFailsClosed`, interop: `TestInteropRefresh` |
+| `offline_access` | Core §11 | Every request: kept only when `Decision.OfflineAccess` grants it — the consent dialog (`OfflineAccessPrompted`: `prompt=consent` required for an OpenID request, `prompt` being no OAuth 2.0 parameter) or the application's policy (`OfflineAccessByPolicy`) — else dropped from the grant — and always outside the code flow (Core §11; `Provider.AuthorizeHandler` serves no implicit flow anyway); an approval left without any scope → `access_denied`. OpenID grants: a refresh token without that consent fails closed (`server_error` at issuance, `invalid_grant` at refresh) unless `Config.AllowRefreshWithoutOfflineAccess`. Plain OAuth 2.0 grants carry no context: only their scope is gated, their refresh tokens follow `grant.Config.RefreshTokenScope` | `TestOfflineAccessRequiresConsent`, `TestPlainOAuth2OfflineAccessNeedsTheDecision`, `TestRequestReportsOfflineAccessForEveryRequest`, `TestRefreshWithoutOfflineConsentFailsClosed`, interop: `TestInteropRefresh` |
### ID Tokens and refresh
@@ -208,11 +208,17 @@ travels to the client, and must not tell it what sessions the browser holds.
one; without it, `offline_access` is dropped from the grant, and an
approval left without any scope answers `access_denied`.
- The login and consent pages are the application's: a CSRF token on every
- form, no framing (`X-Frame-Options: DENY`, `frame-ancestors 'none'`), no
- caching, a return address limited to the authorization endpoint (never
- an open redirect), no `307` redirects, no page at all under
- `prompt=none`. The example's pages do all of it, and the `app-hardening`
- scenarios of `e2e/oidc` check it through HTTP and a browser.
+ form that carries a decision — not on the authorization endpoint, to
+ which relying parties may POST their requests — no framing
+ (`X-Frame-Options: DENY`, `frame-ancestors 'none'`), no caching, a
+ return address limited to the authorization endpoint (never an open
+ redirect), no `307` redirects, no page at all under `prompt=none`. The
+ oauth2 core sets those headers, and `Referrer-Policy: same-origin`, on
+ every response of the authorization endpoint — pages rendered behind it
+ included; `same-origin` rather than `no-referrer`, under which browsers
+ send `Origin: null` on the page's own POST. The example's pages do all of
+ it, and the `app-hardening` scenarios of `e2e/oidc` check it through HTTP
+ and a browser.
### Claims
diff --git a/docs/security-considerations.md b/docs/security-considerations.md
index 6123107b..c632ad6d 100644
--- a/docs/security-considerations.md
+++ b/docs/security-considerations.md
@@ -287,7 +287,36 @@ keys by `kid` against a JWKS provider (static or remote).
public a client that cannot keep its credentials confidential (RFC 6749
§2.1). `/introspect` answers `ClientConfidential` callers only: a public
client (whatever the method, `none` included) or an untyped client gets
- `401 invalid_client`.
+ `401 invalid_client`. Every failed client authentication reads the same —
+ `401 invalid_client`, "client authentication failed", the Basic
+ challenge — whether the client is unknown, of another issuer, presented a
+ wrong secret, used a method it may not use or a malformed credential: the
+ answer neither confirms a client exists nor discloses its type and
+ methods; the detail reaches `OnError` only. Two residual signals remain,
+ both acceptable since client identifiers are not secret (RFC 6749 §2.2):
+ the response time — an unknown client skips `SecretMatches`, so a slow
+ matcher (a password hash for human-chosen secrets) makes it ~100 ms
+ faster: such a matcher runs a dummy verification for unknown clients — and
+ `/authorize`, which tells an unknown `client_id` from an unregistered
+ `redirect_uri` by design (RFC 6749 §4.1.2.1 asks it to inform the resource
+ owner).
+- **Client secrets and guessing** — a client secret is a password the
+ server verifies on every `/token`, `/revoke` and `/introspect` request:
+ RFC 6749 §2.3.1 has the authorization server protect these endpoints
+ against brute force, and RFC 7662 §4 warns that an un-throttled
+ `/introspect` lets an attacker fish for valid tokens. The server does not
+ throttle — every failed client authentication answers the same `401
+ invalid_client` and reaches `OnError` — so rate-limit the three endpoints
+ per `client_id` and per source address in front of the handlers, and
+ alert on the failures. Generate secrets with at least 128 bits of entropy
+ (RFC 6749 §10.10: a guessing probability of at most 2^-128, 2^-160
+ recommended — 32 random bytes), out of reach of online guessing. At
+ rest, keep a hash: `DefaultClient` holds its secret in cleartext — for
+ tests, examples and static configurations — and compares SHA-256 digests
+ in constant time, so its timing discloses neither the secret nor its
+ length; a production `Client` implements `SecretMatcher` over the stored
+ SHA-256 of a generated secret, compared in constant time, or over a
+ password hash for a human-chosen one (with the dummy verification above).
- **Token introspection** — RFC 7662 §4 requires the authorization server
to authenticate the protected resources calling `/introspect`. A public
client cannot authenticate: its `client_id` ships inside every copy of the
@@ -297,18 +326,28 @@ keys by `kid` against a JWKS provider (static or remote).
validity and claims of a token they held — through `none`, or through
Basic/Post with the client's registered secret — so `/introspect` refuses
every caller that is not a confidential client. By default, any
- authenticated confidential client may then introspect any token.
- RFC 7662 §4 adds that protected resources SHOULD be specifically
- authorized, and that when a token can be used only at certain resource
- servers the authorization server MUST determine whether it can be used at
- the caller: that decision is `ServerConfig.IntrospectionPolicy`, which
- sees the authenticated caller and the token's metadata before anything is
- disclosed. Refuse refresh tokens to resource servers — they are meant for
- the client alone, and a resource server that ignores `token_type` could
- accept one as a bearer token. A withheld token and a failed policy answer
+ authenticated confidential client may then introspect any access token of
+ the request's issuer, and a refresh token only if it was issued to it:
+ refresh tokens are meant for the authorization server and their client
+ (RFC 6749 §1.5), and a resource server that ignores `token_type` could
+ accept one as a bearer token. RFC 7662 §4 adds that protected resources
+ SHOULD be specifically authorized, and that when a token can be used only
+ at certain resource servers the authorization server MUST determine
+ whether it can be used at the caller: that decision is
+ `ServerConfig.IntrospectionPolicy`, which sees the authenticated caller
+ and the token's metadata before anything is disclosed — and replaces the
+ default, so refuse refresh tokens to resource servers there too. A
+ withheld token and a failed policy answer
the same bytes as an unknown token (`{"active":false}`), so the response
discloses nothing (its latency may still differ); a policy failure is
reported to `OnError`.
+- **Token revocation** — `/revoke` revokes the caller's own tokens of the
+ request's issuer: a refresh token with its whole family, an access token
+ with its family too (RFC 7009 §2.1). A token issued to another client, or
+ under another issuer, is left untouched and answered with the `200` of an
+ invalid token (RFC 7009 §2.2) — a deliberate deviation from RFC 7009
+ §2.1, which refuses such a request with an error: the error would confirm
+ to whoever holds a token that it is valid for another client.
- **Issuer binding (realms)** — a subject is only meaningful within its
issuer (OIDC Core §16.15). Every endpoint resolves the issuer of the
request through the `IssuerResolver` before it loads the client;
@@ -324,8 +363,14 @@ keys by `kid` against a JWKS provider (static or remote).
boundary: it must return the same issuer for `/authorize` and `/token` of
a realm, and the `ConsentFunc` must authenticate the resource owner within
`AuthorizeRequest.Issuer` — a session of another realm is no
- authentication. `IntrospectedToken.Issuer` lets an `IntrospectionPolicy`
- enforce the same boundary.
+ authentication. `/introspect` and `/revoke` keep the same boundary: at
+ the issuer the request resolves to, a token issued under another issuer
+ answers `{"active":false}` and is not revoked (the answer is 200 all the
+ same), so a client registered in several realms can neither read nor
+ revoke another realm's tokens, and an `IntrospectionPolicy` only sees
+ tokens of the request's issuer. A token recorded without issuer — written
+ before tokens recorded one — is inactive at a non-empty issuer, but its
+ client can still revoke it: revocation only takes away.
- **Clients and realms** — one `ClientStore` behind several issuers makes
every client known at every issuer: a client of realm A could send realm
B's end-users to B's genuine login page and obtain their codes and
@@ -333,8 +378,8 @@ keys by `kid` against a JWKS provider (static or remote).
(`DefaultClient.IssuerValues`) exists at its issuers only: elsewhere
`/authorize` answers the bare 400 of an unknown `client_id`, never
redirected (RFC 6749 §4.1.2.1), and `/token`, `/revoke` and `/introspect`
- fail its authentication as an unknown client's (`401 invalid_client`,
- "unknown client"), whatever the secret — the same bytes in both cases, so
+ fail its authentication as any other (`401 invalid_client`, "client
+ authentication failed"), whatever the secret — the same bytes in both cases, so
realm B can neither be used by A's clients nor enumerate them or check
their secrets. The binding is checked before the client's secret and
redirect URI, and again on the client an authenticator returns. A
@@ -353,6 +398,83 @@ keys by `kid` against a JWKS provider (static or remote).
`ConsentFunc` error is always a `server_error`. A refusal's description
and URI travel in the client's URL — browser history, logs, `Referer` —
so they must never carry personal data.
+- **Redirect URIs** — a request's `redirect_uri` must equal a registered
+ one, byte for byte (RFC 6749 §3.1.2.3, RFC 9700 §2.1: simple string
+ comparison, no pattern), with one exception RFC 8252 §7.3 makes
+ mandatory: a registered loopback IP redirect URI (`http://127.0.0.1` or
+ `http://[::1]`) accepts any port, the rest of the URI unchanged — native
+ apps listen on an ephemeral port — and the code is bound to the URI
+ requested. `http://localhost` gets no such variance (RFC 8252 §8.3
+ recommends the IP literal). The registered URI a request resolves to must
+ pass `oauth2.ValidateRedirectURI` — absolute, no fragment, a host, no
+ userinfo and a port within 1–65535 for http(s), a private-use scheme
+ holding a "." (RFC 8252 §8.4, which also rules out `javascript:`, `data:`
+ and `urn:ietf:wg:oauth:2.0:oob`), a query naming no
+ response parameter — or the request gets a bare 400, never a redirect
+ (RFC 6749 §3.1.2.4); run the same function when registering clients.
+ Responses keep the registered query byte for byte (RFC 6749 §3.1.2),
+ and the implicit flow appends its fragment to a URI that has none.
+- **Open redirection** — RFC 9700 §4.11.2: a registered client controlling
+ its redirect URI could send users an erroneous request (an invalid scope)
+ or a `prompt=none` one, and have the authorization server redirect them
+ to its phishing page without anyone authenticating. Set
+ `AuthorizeConfig.TrustedRedirect`: for a client it does not trust, every
+ error detected before the resource owner authenticated — and a
+ `Consent.Refusal` stating the request cannot complete without a user
+ interface (`login_required`, `interaction_required`,
+ `account_selection_required`, `consent_required`) — is a bare error page,
+ never redirected. Without it, every registered client is trusted (RFC 6749
+ §4.1.2.1 redirects errors once the redirect URI is validated). Once the
+ resource owner authenticated, the code and an explicit `access_denied` are
+ redirected to every client.
+- **Authorization endpoint pages** — the `ConsentFunc` renders the login
+ and consent pages, but every response of `/authorize` carries
+ `X-Frame-Options: DENY`, `Content-Security-Policy: frame-ancestors 'none'`
+ (added to a policy an outer handler set), `Cache-Control: no-store` and
+ `Referrer-Policy: same-origin`: the authorization server MUST prevent
+ clickjacking (RFC 6749 §10.13, RFC 9700 §4.16), its pages and redirects
+ carry codes and states, and no `Referer` leaks the request URL to another
+ origin (RFC 9700 §4.2.4). `same-origin`, not `no-referrer`: under
+ `no-referrer` browsers send `Origin: null` on the consent page's own
+ POST, which Origin-based CSRF checks then refuse. A `ConsentFunc` that
+ must let a trusted origin frame its page replaces them on
+ `w.Header()`. Browsers check framing on the final response only, so
+ `prompt=none` in an iframe still follows the redirect. The authorization
+ endpoint MUST be protected against CSRF (RFC 6749 §10.12): protect the
+ request carrying the resource owner's decision — a token bound to their
+ session, or `net/http.CrossOriginProtection` as `examples/oauth2` does —
+ not the endpoint, to which clients may POST authorization requests. A
+ redirect answering a POST is a `303` (RFC 9700 §4.12: only 303
+ unambiguously turns the POST, and the credentials it may carry, into a
+ GET), never a `307`.
+- **Failures are outages, not refusals** — a `ClientStore` that fails is
+ not an unknown client: answering `invalid_client` would tell a client its
+ credentials are wrong (and push it to rotate them). `/authorize` answers a
+ bare 500, `/token` and `/introspect` a `server_error`, `/revoke` a 503
+ `temporarily_unavailable` (RFC 7009 §2.2.1), each reported to `OnError`;
+ a store reports an unknown client with `(nil, nil)` or an error carrying
+ `invalid_client`. A token lookup that fails at `/introspect` answers
+ `{"active":false}` — a resource server fails closed (RFC 7662 §2.1) — and
+ reaches `OnError`.
+- **Request bounds** — RFC 6749 bounds no parameter, and `net/http` alone
+ reads 10 MB of form. Every endpoint bounds the query and the body it
+ parses to `ServerConfig.MaxRequestBytes` (64 KiB by default) —
+ `invalid_request`, a bare 400 at `/authorize`, before any parameter is
+ read; a declared `Content-Length` over the bound is refused before the
+ body is. The values the server keeps or echoes are bounded too: the
+ `state` echoed in every authorization response (at most 4096 bytes of
+ VSCHAR, RFC 6749 Appendix A.5 — refused without echoing it), the `nonce`
+ recorded with the code (512 bytes of UTF-8 without NUL, for every request
+ — so no store is handed a multi-megabyte value to persist and re-encode),
+ the scope (100 tokens, 4096 bytes) and the `client_id` (255 bytes: a longer
+ one is unknown, never looked up). A 4 KiB `state` of unreserved
+ characters keeps the redirect URL within the usual 8 KiB request-line
+ limits; percent-encoding triples every other VSCHAR, so a state made of
+ them reaches 12 KiB of `Location` — and an authorization server behind a
+ proxy whose response-header buffer is smaller (nginx's
+ `proxy_buffer_size` defaults to 4 KiB) needs that buffer raised. A body a
+ middleware parses before the handlers (`r.ParseForm`, `r.FormValue`) is
+ read without `MaxRequestBytes`: bound that middleware's reads too.
- **Mix-up defense (RFC 9207)** — every authorization response, code and
implicit, success and redirected error, carries `iss`, the issuer the
request resolved to, and the RFC 8414 metadata sets
@@ -1356,9 +1478,26 @@ span catalog.
`ScopeValues` — an empty list allows nothing — and watch `OnError`
for `ErrNoGrantTypesDeclared` / `ErrNoScopesDeclared` after an
upgrade.
+- [ ] When third parties register clients, set
+ `AuthorizeConfig.TrustedRedirect`, so that errors before
+ authentication never redirect to a client you do not trust.
+- [ ] Validate every redirect URI at registration with
+ `oauth2.ValidateRedirectURI`; register native apps with a reverse
+ domain name scheme (`com.example.app:/cb`) or a loopback IP literal
+ (`http://127.0.0.1/cb`).
+- [ ] Protect the request carrying the consent decision against CSRF (a
+ session-bound token, or `net/http.CrossOriginProtection`); replace
+ the anti-framing headers of `/authorize` only for origins you trust.
- [ ] Register the resource servers calling `/introspect` as confidential
- clients, and set `ServerConfig.IntrospectionPolicy` when several
- resource servers or tenants share the authorization server.
+ clients, have them call it at the issuer of the tokens they accept,
+ and set `ServerConfig.IntrospectionPolicy` when several resource
+ servers or tenants share the authorization server.
+- [ ] Rate-limit `/token`, `/revoke` and `/introspect` per `client_id` and
+ per source address, and alert on the `invalid_client` failures
+ `OnError` reports.
+- [ ] Generate client secrets with at least 128 bits of entropy and keep
+ only a hash of them, behind a `SecretMatcher` (`DefaultClient` holds
+ them in cleartext).
- [ ] Give `ServerConfig.Storage` and every grant the same store, and wire
`ServerConfig.OnError` and `grant.Config.OnError` to the same sink:
failed family revocations are reported there.
diff --git a/examples/oauth2/README.md b/examples/oauth2/README.md
index 330dac86..e695dd23 100644
--- a/examples/oauth2/README.md
+++ b/examples/oauth2/README.md
@@ -83,6 +83,29 @@ curl -i -u 5cc06c3b-5755-4229-958c-a515a245aaeb:WTvuAztPD2XBauomleRzGFYuZawS07Ym
A code is single-use: running the same exchange again is refused with
`invalid_grant` and revokes the tokens the first exchange issued.
+## Consent page protections
+
+- Every `/oauth2/authorize` response carries `X-Frame-Options: DENY`,
+ `Content-Security-Policy: frame-ancestors 'none'`, `Cache-Control:
+ no-store` and `Referrer-Policy: same-origin`: the authorization server
+ sets them, so the consent page cannot be framed (clickjacking) nor cached.
+- The POST carrying the decision is refused when it comes from another site
+ (`http.CrossOriginProtection`, RFC 6749 §10.12): a page elsewhere cannot
+ approve on the user's behalf. An authorization request POSTed from
+ another site still shows the consent page — its parameters in the query
+ string or in the body: the consent form posts them back with the
+ decision. A real application also binds
+ the decision to the user's login session with a CSRF token — see
+ `examples/oidc-provider`.
+- The approval is answered with `303 See Other`, the redirect that turns a
+ POST into a GET (RFC 9700 §4.12).
+
+```sh
+curl -i -X POST -H 'Sec-Fetch-Site: cross-site' -d 'decision=approve' \
+ 'http://localhost:1337/oauth2/authorize?response_type=code&client_id=5cc06c3b-5755-4229-958c-a515a245aaeb&redirect_uri=http://localhost:1337/callback&scope=api:read&state=demo&code_challenge=E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM&code_challenge_method=S256'
+# → 403 Forbidden
+```
+
## What this example does NOT cover
- JWT-formatted access tokens (`jwt.OAuth2AccessTokenSigner` adapter wires
diff --git a/examples/oauth2/main.go b/examples/oauth2/main.go
index 36c8c126..48c8c01f 100644
--- a/examples/oauth2/main.go
+++ b/examples/oauth2/main.go
@@ -33,6 +33,7 @@ import (
"html"
"log"
"net/http"
+ "strings"
"time"
"github.com/hyperscale-stack/security"
@@ -173,7 +174,7 @@ func buildServer() (http.Handler, error) {
// "/oauth2") so the metadata document advertises the right URLs.
mux := http.NewServeMux()
// /authorize answers GET (consent page) and POST (decision).
- authorize := srv.AuthorizeHandler(oauth2.AuthorizeConfig{}, consentHandler)
+ authorize := srv.AuthorizeHandler(oauth2.AuthorizeConfig{}, consentHandler(http.NewCrossOriginProtection()))
mux.Handle("GET /oauth2/authorize", authorize)
mux.Handle("POST /oauth2/authorize", authorize)
mux.Handle("POST /oauth2/token", srv.TokenHandler())
@@ -190,32 +191,80 @@ func buildServer() (http.Handler, error) {
return mux, nil
}
-// consentHandler is the /authorize consent hook. A real application
+// consentHandler returns the /authorize consent hook. A real application
// authenticates the resource owner and renders branded UI; this demo
// renders a bare Approve / Deny form and treats every visitor as the
// fixed "demo-user".
-func consentHandler(w http.ResponseWriter, r *http.Request, ar *oauth2.AuthorizeRequest) (*oauth2.Consent, error) {
- if r.Method == http.MethodPost {
- return &oauth2.Consent{
- Approved: r.FormValue("decision") == "approve",
- Subject: "demo-user",
- }, nil
+//
+// The POST carrying the decision is what an attacker would forge from
+// another site (RFC 6749 §10.12), so it alone goes through csrf, which
+// refuses cross-origin browser requests (Fetch Metadata, Origin) — not
+// the endpoint: an authorization request may itself be POSTed from the
+// client's site (RFC 6749 §3.1, OpenID Connect Core §3.1.2.1). A real
+// application binds the decision to the resource owner's session as well,
+// with a CSRF token (session.CSRFToken, see examples/oidc-provider). The
+// page needs no protective header of its own: the authorization endpoint
+// sets X-Frame-Options, CSP frame-ancestors, Cache-Control and
+// Referrer-Policy on every response.
+func consentHandler(csrf *http.CrossOriginProtection) oauth2.ConsentFunc {
+ return func(w http.ResponseWriter, r *http.Request, ar *oauth2.AuthorizeRequest) (*oauth2.Consent, error) {
+ if decision := r.PostFormValue("decision"); r.Method == http.MethodPost && decision != "" {
+ if refusedCrossOrigin(csrf, w, r) {
+ return nil, nil // answered
+ }
+
+ return &oauth2.Consent{Approved: decision == "approve", Subject: "demo-user"}, nil
+ }
+
+ renderConsent(w, r, ar)
+
+ return nil, nil // the consent page was rendered
+ }
+}
+
+// refusedCrossOrigin answers 403 Forbidden to a cross-origin browser
+// request and reports whether it did.
+func refusedCrossOrigin(csrf *http.CrossOriginProtection, w http.ResponseWriter, r *http.Request) bool {
+ if csrf.Check(r) == nil {
+ return false
}
+ http.Error(w, "cross-origin decision refused", http.StatusForbidden)
+
+ return true
+}
+
+// renderConsent writes the consent page of ar: it shows the request as the
+// server validated it (ar), and its form posts the decision with the
+// authorization request as received — its query string, and the
+// parameters of a POSTed body as hidden fields — for the server to
+// validate it again.
+func renderConsent(w http.ResponseWriter, r *http.Request, ar *oauth2.AuthorizeRequest) {
w.Header().Set("Content-Type", "text/html; charset=utf-8")
+ var fields strings.Builder
+
+ for name, values := range r.PostForm {
+ if name == "decision" {
+ continue
+ }
+
+ for _, v := range values {
+ fmt.Fprintf(&fields, "\n", html.EscapeString(name), html.EscapeString(v))
+ }
+ }
+
//nolint:gosec // G705: every interpolated value is HTML-escaped
fmt.Fprintf(w, `Authorize
Client %s requests scope %s.
`,
html.EscapeString(ar.Client.ID()),
html.EscapeString(ar.Scope),
- html.EscapeString(r.URL.RawQuery))
-
- return nil, nil // the consent page was rendered
+ html.EscapeString(r.URL.RawQuery),
+ fields.String())
}
// showCallback stands in for the client's redirect endpoint: it just
diff --git a/examples/oauth2/main_test.go b/examples/oauth2/main_test.go
index 1be910bb..654051fa 100644
--- a/examples/oauth2/main_test.go
+++ b/examples/oauth2/main_test.go
@@ -6,10 +6,12 @@ package main
import (
"encoding/json"
+ "html"
"io"
"net/http"
"net/http/httptest"
"net/url"
+ "regexp"
"strings"
"testing"
@@ -119,11 +121,12 @@ func TestExampleOAuth2AuthorizationCodeFlow(t *testing.T) {
require.Equal(t, http.StatusOK, resp.StatusCode)
assert.Contains(t, string(page), "Approve")
- // 2. Approving redirects to the callback with an authorization code.
+ // 2. Approving redirects to the callback with an authorization code —
+ // 303 See Other: the answer to a POST (RFC 9700 §4.12).
resp, err = client.PostForm(srv.URL+"/oauth2/authorize?"+authz, url.Values{"decision": {"approve"}})
require.NoError(t, err)
_ = resp.Body.Close()
- require.Equal(t, http.StatusFound, resp.StatusCode)
+ require.Equal(t, http.StatusSeeOther, resp.StatusCode)
loc, err := url.Parse(resp.Header.Get("Location"))
require.NoError(t, err)
@@ -194,3 +197,133 @@ func refreshExchange(t *testing.T, client *http.Client, baseURL, raw string) *ht
return resp
}
+
+// TestExampleConsentIsProtected drives the consent page the way a browser
+// does: the page cannot be framed nor cached, a decision posted from another
+// site is refused (RFC 6749 §10.12) and issues nothing, while an
+// authorization request posted from another site — which RFC 6749 §3.1 and
+// OpenID Connect allow — still shows the consent page.
+func TestExampleConsentIsProtected(t *testing.T) {
+ t.Parallel()
+
+ handler, err := buildServer()
+ require.NoError(t, err)
+
+ authz := url.Values{
+ "response_type": {"code"},
+ "client_id": {demoClientID},
+ "redirect_uri": {"http://localhost:1337/callback"},
+ "scope": {"api:read"},
+ "state": {"demo-state"},
+ "code_challenge": {pkceChallenge},
+ "code_challenge_method": {"S256"},
+ }
+
+ serve := func(req *http.Request) *httptest.ResponseRecorder {
+ rec := httptest.NewRecorder()
+ handler.ServeHTTP(rec, req)
+
+ return rec
+ }
+ post := func(form url.Values, site string) *http.Request {
+ req := httptest.NewRequest(http.MethodPost, "/oauth2/authorize?"+authz.Encode(), strings.NewReader(form.Encode()))
+ req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+ req.Header.Set("Sec-Fetch-Site", site)
+
+ return req
+ }
+
+ page := serve(httptest.NewRequest(http.MethodGet, "/oauth2/authorize?"+authz.Encode(), nil))
+ require.Equal(t, http.StatusOK, page.Code)
+ assert.Equal(t, "DENY", page.Header().Get("X-Frame-Options"))
+ assert.Contains(t, page.Header().Values("Content-Security-Policy"), "frame-ancestors 'none'")
+ assert.Equal(t, "no-store", page.Header().Get("Cache-Control"))
+ assert.Equal(t, "same-origin", page.Header().Get("Referrer-Policy"))
+
+ forged := serve(post(url.Values{"decision": {"approve"}}, "cross-site"))
+ assert.Equal(t, http.StatusForbidden, forged.Code, "a cross-site decision is refused")
+ assert.Empty(t, forged.Header().Get("Location"), "no code")
+
+ request := serve(post(url.Values{}, "cross-site"))
+ assert.Equal(t, http.StatusOK, request.Code, "a cross-site authorization request shows the consent page")
+ assert.Contains(t, request.Body.String(), "Approve")
+
+ approved := serve(post(url.Values{"decision": {"approve"}}, "same-origin"))
+ require.Equal(t, http.StatusSeeOther, approved.Code)
+
+ loc, err := url.Parse(approved.Header().Get("Location"))
+ require.NoError(t, err)
+ assert.NotEmpty(t, loc.Query().Get("code"))
+}
+
+// consentForm extracts the action and the hidden fields of the consent
+// page's form, as a browser submits them.
+func consentForm(t *testing.T, page string) (string, url.Values) {
+ t.Helper()
+
+ action := regexp.MustCompile(`")
+
+ return nil, nil
+}
+
+// TestAuthorizeResponsesCarryProtectiveHeaders: every /authorize response —
+// the page a ConsentFunc renders, a redirect carrying a code or an error,
+// a bare error — carries the protective headers, set before anything else
+// runs.
+func TestAuthorizeResponsesCarryProtectiveHeaders(t *testing.T) {
+ t.Parallel()
+
+ get := func(q url.Values) *http.Request {
+ return httptest.NewRequest(http.MethodGet, authorizePath+"?"+q.Encode(), nil)
+ }
+ post := func(q url.Values) *http.Request {
+ return boundsPost(authorizePath, strings.NewReader(q.Encode()))
+ }
+ with := func(key, value string) url.Values {
+ q := boundsQuery()
+ q.Set(key, value)
+
+ return q
+ }
+
+ cases := []struct {
+ name string
+ req *http.Request
+ consent oauth2.ConsentFunc
+ status int
+ }{
+ {"the ConsentFunc's page", get(boundsQuery()), renderPage, http.StatusOK},
+ {"a code (GET)", get(boundsQuery()), approve, http.StatusFound},
+ {"a code (POST)", post(boundsQuery()), approve, http.StatusSeeOther},
+ {"an implicit token", get(with("response_type", "token")), approve, http.StatusFound},
+ {"a redirected error", get(with("scope", "admin")), approve, http.StatusFound},
+ {"an unknown client", get(with("client_id", "ghost")), approve, http.StatusBadRequest},
+ {"an unregistered redirect URI", get(with("redirect_uri", "https://evil.example/cb")), approve, http.StatusBadRequest},
+ {"a request too large", get(with("pad", strings.Repeat("x", oauth2.DefaultMaxRequestBytes))), approve, http.StatusBadRequest},
+ {"another method", httptest.NewRequest(http.MethodPut, authorizePath, nil), approve, http.StatusMethodNotAllowed},
+ }
+
+ for _, tc := range cases {
+ t.Run(tc.name, func(t *testing.T) {
+ t.Parallel()
+
+ env := newBoundsEnv(t, 0, nil, nil)
+ rec := serve(env.authorizeHandler(tc.consent), tc.req)
+
+ require.Equal(t, tc.status, rec.Code, rec.Body.String())
+ requireProtectiveHeaders(t, rec)
+ })
+ }
+}
+
+// TestAuthorizeAnswersAnotherMethodWithAllow: a 405 lists the methods the
+// endpoint serves (RFC 9110 §15.5.6).
+func TestAuthorizeAnswersAnotherMethodWithAllow(t *testing.T) {
+ t.Parallel()
+
+ env := newBoundsEnv(t, 0, nil, nil)
+ rec := serve(env.authorizeHandler(approve), httptest.NewRequest(http.MethodDelete, authorizePath, nil))
+
+ require.Equal(t, http.StatusMethodNotAllowed, rec.Code)
+ assert.Equal(t, "GET, POST", rec.Header().Get("Allow"))
+}
+
+// TestConsentFuncCanReplaceProtectiveHeaders: the ConsentFunc, which
+// renders the page, may allow framing by origins it trusts (RFC 9700 §4.16
+// lets administrators configure them per client).
+func TestConsentFuncCanReplaceProtectiveHeaders(t *testing.T) {
+ t.Parallel()
+
+ env := newBoundsEnv(t, 0, nil, nil)
+ partner := func(w http.ResponseWriter, r *http.Request, ar *oauth2.AuthorizeRequest) (*oauth2.Consent, error) {
+ w.Header().Del("X-Frame-Options")
+ w.Header().Set("Content-Security-Policy", "frame-ancestors https://partner.example")
+
+ return renderPage(w, r, ar)
+ }
+
+ rec := serve(env.authorizeHandler(partner), httptest.NewRequest(http.MethodGet, authorizePath+"?"+boundsQuery().Encode(), nil))
+ require.Equal(t, http.StatusOK, rec.Code)
+ assert.Empty(t, rec.Header().Get("X-Frame-Options"))
+ assert.Equal(t, []string{"frame-ancestors https://partner.example"}, rec.Header().Values("Content-Security-Policy"))
+}
+
+// TestAuthorizeKeepsAnApplicationCSP: a Content-Security-Policy a middleware
+// set is kept — the handler adds its frame-ancestors policy, and browsers
+// enforce both.
+func TestAuthorizeKeepsAnApplicationCSP(t *testing.T) {
+ t.Parallel()
+
+ env := newBoundsEnv(t, 0, nil, nil)
+ h := env.authorizeHandler(renderPage)
+ csp := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ w.Header().Set("Content-Security-Policy", "default-src 'self'")
+ h.ServeHTTP(w, r)
+ })
+
+ rec := serve(csp, httptest.NewRequest(http.MethodGet, authorizePath+"?"+boundsQuery().Encode(), nil))
+ require.Equal(t, http.StatusOK, rec.Code)
+ assert.Equal(t, []string{"default-src 'self'", "frame-ancestors 'none'"}, rec.Header().Values("Content-Security-Policy"))
+}
+
+// TestAuthorizePostRedirectsWithSeeOther pins RFC 9700 §4.12: a redirect
+// answering a POST — which may carry the resource owner's credentials — is
+// a 303, which every user agent follows with a GET; a GET keeps the 302 of
+// RFC 6749 §4.1.2.
+func TestAuthorizePostRedirectsWithSeeOther(t *testing.T) {
+ t.Parallel()
+
+ with := func(key, value string) url.Values {
+ q := boundsQuery()
+ q.Set(key, value)
+
+ return q
+ }
+ deny := consentWith(&oauth2.Consent{Approved: false}, nil)
+
+ cases := []struct {
+ name string
+ q url.Values
+ consent oauth2.ConsentFunc
+ }{
+ {"an approval", boundsQuery(), approve},
+ {"a refusal", boundsQuery(), deny},
+ {"an error before the ConsentFunc", with("scope", "admin"), approve},
+ {"an implicit token", with("response_type", "token"), approve},
+ }
+
+ for _, tc := range cases {
+ t.Run(tc.name, func(t *testing.T) {
+ t.Parallel()
+
+ env := newBoundsEnv(t, 0, nil, nil)
+ h := env.authorizeHandler(tc.consent)
+
+ rec := serve(h, boundsPost(authorizePath, strings.NewReader(tc.q.Encode())))
+ assert.Equal(t, http.StatusSeeOther, rec.Code, "POST")
+ assert.NotEmpty(t, rec.Header().Get("Location"))
+
+ rec = serve(h, httptest.NewRequest(http.MethodGet, authorizePath+"?"+tc.q.Encode(), nil))
+ assert.Equal(t, http.StatusFound, rec.Code, "GET")
+ })
+ }
+}
diff --git a/oauth2/client.go b/oauth2/client.go
index 92efd698..1dc6257c 100644
--- a/oauth2/client.go
+++ b/oauth2/client.go
@@ -6,6 +6,7 @@ package oauth2
import (
"context"
+ "crypto/sha256"
"crypto/subtle"
"slices"
)
@@ -57,9 +58,13 @@ type Client interface {
ID() string
// Type reports whether the client is confidential or public.
Type() ClientType
- // RedirectURIs lists the redirect URIs registered by the client.
- // Authorization code requests MUST match one of these exactly per
- // RFC 6749 §3.1.2.3 / OAuth 2.0 BCP §2.1.4.
+ // RedirectURIs lists the redirect URIs registered by the client — each
+ // one valid for [ValidateRedirectURI], which the authorization endpoint
+ // applies to the URI a request resolves to (an invalid one is answered
+ // with a bare 400, never redirected to). A request's redirect_uri MUST
+ // match one exactly (RFC 6749 §3.1.2.3, RFC 9700 §2.1), but for the port
+ // of a loopback IP redirect URI (http://127.0.0.1 or http://[::1], RFC
+ // 8252 §7.3), which the request may set freely.
RedirectURIs() []string
// GrantTypes lists the grant types the client may use, compared exactly
// with RFC 7591 §2 values — the GrantType constants
@@ -86,6 +91,15 @@ type Client interface {
// verify the registered secret without exposing it. Implementations MUST
// use constant-time comparison (or a hashed-secret scheme).
//
+// Keep a hash of the secret, not the secret. A secret the server generates
+// with at least 128 bits of entropy (RFC 6749 §10.10: a guessing
+// probability of at most 2^-128, 2^-160 recommended) — 32 random bytes,
+// for instance — is safe behind a fast hash: store its SHA-256 and compare
+// the digest of the presented secret with it in constant time. A secret
+// chosen by a human needs a password hash (the password module): it is slow
+// by design, so verify a dummy hash for unknown clients too, or their
+// faster answer tells them apart.
+//
// An empty secret is never a credential: implementations MUST NOT match
// it — not even against a hash of the empty string — nor anything when no
// secret is registered.
@@ -100,6 +114,13 @@ type SecretMatcher interface {
// for caching policy; the Server invokes LoadClient once per request that
// needs client authentication.
//
+// LoadClient returns (nil, nil) — or an error carrying invalid_client
+// ([IsCode]) — for an unknown client, which the server answers as such. Any
+// other error is a failure of the store: /authorize answers a bare 500,
+// /token and /introspect server_error, /revoke temporarily_unavailable, and
+// the error reaches the [ErrorHook] — never invalid_client, which would tell
+// the client its credentials are wrong.
+//
// One store serving an [IssuerResolver] with several issuers makes every
// client known at every issuer: a deployment where each client belongs to
// one realm (tenant) gives its clients the [IssuerBoundClient] capability,
@@ -119,9 +140,9 @@ type ClientStore interface {
// exist. /authorize answers it as it answers an unknown client_id — a bare
// 400 that is never redirected (RFC 6749 §4.1.2.1: the client identifier is
// invalid here, and so are its redirect URIs) — and /token, /revoke and
-// /introspect fail its authentication as they fail an unknown client's —
-// 401 invalid_client, "unknown client" (RFC 6749 §5.2), whatever the
-// secret presented — so another realm's clients can be neither enumerated
+// /introspect fail its authentication as they fail any other — 401
+// invalid_client, "client authentication failed" (RFC 6749 §5.2), whatever
+// the secret presented — so another realm's clients can be neither enumerated
// nor their secrets checked. unauthorized_client is not used: it would
// confirm an authenticated, existing client. The [ErrorHook] gets
// [ErrClientNotAtIssuer] as the cause. A server whose IssuerResolver fails
@@ -205,17 +226,23 @@ func (c *DefaultClient) Scopes() []string { return c.ScopeValues }
// AuthMethods implements [Client].
func (c *DefaultClient) AuthMethods() []string { return c.AuthMethodValues }
-// SecretMatches implements [SecretMatcher] using constant-time comparison.
-// An empty Secret registers no secret and matches nothing, the empty string
+// SecretMatches implements [SecretMatcher]: it compares the SHA-256
+// digests of the presented and registered secrets in constant time, so the
+// time it takes tells nothing about the registered secret — its length
+// included, which comparing the secrets themselves would leak. An empty
+// Secret registers no secret and matches nothing, the empty string
// included. The DefaultClient stores secrets in cleartext for development
// convenience; production deployments SHOULD wrap a hashed-secret store and
-// implement SecretMatches themselves.
+// implement SecretMatches themselves (see [SecretMatcher]).
func (c *DefaultClient) SecretMatches(secret string) bool {
if c.Secret == "" {
return false
}
- return subtle.ConstantTimeCompare([]byte(c.Secret), []byte(secret)) == 1
+ registered := sha256.Sum256([]byte(c.Secret))
+ presented := sha256.Sum256([]byte(secret))
+
+ return subtle.ConstantTimeCompare(registered[:], presented[:]) == 1
}
// AllowsIssuer implements [IssuerBoundClient]: the issuers listed in
diff --git a/oauth2/client_auth_failure_test.go b/oauth2/client_auth_failure_test.go
new file mode 100644
index 00000000..d3e60a2b
--- /dev/null
+++ b/oauth2/client_auth_failure_test.go
@@ -0,0 +1,200 @@
+// Copyright 2026 Hyperscale. All rights reserved.
+// Use of this source code is governed by a MIT
+// license that can be found in the LICENSE file.
+
+package oauth2_test
+
+import (
+ "errors"
+ "net/http"
+ "net/http/httptest"
+ "net/url"
+ "strings"
+ "testing"
+
+ "github.com/hyperscale-stack/security/oauth2"
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+)
+
+// unmatchableClient is a confidential client that cannot verify a secret:
+// the generated Client mock implements oauth2.Client, not
+// oauth2.SecretMatcher.
+func unmatchableClient(t *testing.T) *oauth2.MockClient {
+ t.Helper()
+
+ client := oauth2.NewMockClient(t)
+ client.EXPECT().ID().Return("unmatchable").Maybe()
+ client.EXPECT().Type().Return(oauth2.ClientConfidential).Maybe()
+ client.EXPECT().RedirectURIs().Return(nil).Maybe()
+ client.EXPECT().GrantTypes().Return([]string{oauth2.GrantTypeClientCredentials}).Maybe()
+ client.EXPECT().Scopes().Return([]string{"read"}).Maybe()
+ client.EXPECT().AuthMethods().Return(nil).Maybe()
+
+ return client
+}
+
+// clientAuthFailures are client authentications every endpoint refuses,
+// each with the detail of its refusal.
+func clientAuthFailures(t *testing.T) []struct {
+ name, detail string
+ cause error
+ req func(path string, form url.Values) *http.Request
+} {
+ t.Helper()
+
+ basicAs := func(id, secret string) func(string, url.Values) *http.Request {
+ return func(path string, form url.Values) *http.Request {
+ req := boundsPost(path, strings.NewReader(form.Encode()))
+ req.SetBasicAuth(id, secret)
+
+ return req
+ }
+ }
+ bodyAs := func(extra url.Values) func(string, url.Values) *http.Request {
+ return func(path string, form url.Values) *http.Request {
+ body := url.Values{}
+ for k, v := range form {
+ body[k] = v
+ }
+
+ for k, v := range extra {
+ body[k] = v
+ }
+
+ req := httptest.NewRequest(http.MethodPost, path, strings.NewReader(body.Encode()))
+ req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+
+ return req
+ }
+ }
+
+ return []struct {
+ name, detail string
+ cause error
+ req func(path string, form url.Values) *http.Request
+ }{
+ {name: "unknown client (basic)", detail: "unknown client", req: basicAs("ghost", "secret")},
+ {name: "unknown client (post)", detail: "unknown client", req: bodyAs(url.Values{"client_id": {"ghost"}, "client_secret": {"x"}})},
+ {name: "unknown client (none)", detail: "unknown client", req: bodyAs(url.Values{"client_id": {"ghost"}})},
+ {name: "wrong secret (basic)", detail: "client authentication failed", req: basicAs("machine", "wrong")},
+ {name: "wrong secret (post)", detail: "client authentication failed", req: bodyAs(url.Values{"client_id": {"machine"}, "client_secret": {"wrong"}})},
+ {name: "method not allowed", detail: "method not allowed for client", req: bodyAs(url.Values{"client_id": {"basiconly"}, "client_secret": {"basiconly-secret"}})},
+ {name: "none for a confidential client", detail: "none method reserved to public clients", req: bodyAs(url.Values{"client_id": {"machine"}})},
+ {name: "client unable to verify a secret", detail: "client cannot verify secret", req: basicAs("unmatchable", "secret")},
+ {name: "malformed Basic header", detail: "malformed Basic Authorization header", req: func(path string, form url.Values) *http.Request {
+ req := boundsPost(path, strings.NewReader(form.Encode()))
+ req.Header.Set("Authorization", "Basic %%%")
+
+ return req
+ }},
+ {name: "empty Basic password", detail: "missing client_id or client_secret", req: basicAs("machine", "")},
+ {name: "no credentials", detail: "no client authentication method matched", req: bodyAs(nil)},
+ {name: "client of another issuer", detail: "unknown client", cause: oauth2.ErrClientNotAtIssuer, req: basicAs("elsewhere", "elsewhere-secret")},
+ }
+}
+
+// failuresEnv is a boundsEnv also serving "basiconly" (client_secret_basic
+// only), "unmatchable" (no SecretMatcher) and "elsewhere" (registered at
+// another issuer).
+func failuresEnv(t *testing.T) *boundsEnv {
+ t.Helper()
+
+ env := newBoundsEnv(t, 0, nil, nil)
+
+ basicOnly := confidentialClient([]string{oauth2.GrantTypeClientCredentials}, []string{"read"})
+ basicOnly.IDValue, basicOnly.Secret, basicOnly.AuthMethodValues = "basiconly", "basiconly-secret", []string{"client_secret_basic"}
+ env.clients.clients["basiconly"] = basicOnly
+
+ elsewhere := confidentialClient([]string{oauth2.GrantTypeClientCredentials}, []string{"read"})
+ elsewhere.IDValue, elsewhere.Secret, elsewhere.IssuerValues = "elsewhere", "elsewhere-secret", []string{"https://other.example"}
+ env.clients.clients["elsewhere"] = elsewhere
+
+ env.clients.clients["unmatchable"] = unmatchableClient(t)
+
+ return env
+}
+
+// requireConstantInvalidClient asserts the one answer every failed client
+// authentication gets — 401 invalid_client, "client authentication
+// failed", the Basic challenge — and that the hook got the refusal it
+// replaced: its detailed description, and cause.
+func requireConstantInvalidClient(t *testing.T, env *boundsEnv, rec *httptest.ResponseRecorder, detail string, cause error) {
+ t.Helper()
+
+ require.Equal(t, http.StatusUnauthorized, rec.Code, rec.Body.String())
+ assert.JSONEq(t, `{"error":"invalid_client","error_description":"client authentication failed"}`, rec.Body.String())
+ assert.Equal(t, `Basic realm="oauth2"`, rec.Header().Get("WWW-Authenticate"))
+
+ reported := env.sink.collected()
+ require.Len(t, reported, 1)
+
+ var envelope *oauth2.Error
+
+ require.ErrorAs(t, reported[0], &envelope)
+ assert.Equal(t, oauth2.CodeInvalidClient, envelope.Code)
+
+ var detailed *oauth2.Error
+
+ require.ErrorAs(t, envelope.Unwrap(), &detailed, "the refusal it replaced is in the cause")
+ assert.Equal(t, detail, detailed.Description)
+
+ if cause != nil {
+ assert.ErrorIs(t, reported[0], cause)
+ }
+}
+
+// TestClientAuthenticationFailuresLookAlike pins RFC 6749 §5.2 without the
+// enumeration oracle: whatever made a client authentication fail — an
+// unknown client, a client of another issuer, a wrong secret, a method the
+// client may not use, a malformed credential — /token, /revoke and
+// /introspect answer the very same bytes; only the ErrorHook learns the
+// detail.
+func TestClientAuthenticationFailuresLookAlike(t *testing.T) {
+ t.Parallel()
+
+ endpoints := []struct {
+ path string
+ form url.Values
+ handler func(*oauth2.Server) http.Handler
+ }{
+ {"/oauth2/token", url.Values{"grant_type": {"client_credentials"}}, (*oauth2.Server).TokenHandler},
+ {"/oauth2/revoke", url.Values{"token": {"ghost"}}, (*oauth2.Server).RevokeHandler},
+ {"/oauth2/introspect", url.Values{"token": {"ghost"}}, (*oauth2.Server).IntrospectHandler},
+ }
+
+ for _, ep := range endpoints {
+ for _, tc := range clientAuthFailures(t) {
+ t.Run(ep.path+"/"+tc.name, func(t *testing.T) {
+ t.Parallel()
+
+ env := failuresEnv(t)
+ rec := serve(ep.handler(env.srv), tc.req(ep.path, ep.form))
+
+ requireConstantInvalidClient(t, env, rec, tc.detail, tc.cause)
+ })
+ }
+ }
+}
+
+// errPublicCaller stands for the introspection refusal of a public caller:
+// the hook gets its description.
+var errPublicCaller = errors.New("introspection requires confidential client authentication")
+
+// TestIntrospectionOfAPublicCallerLooksAlike: a public client — whose
+// client_id authenticates nothing — gets the same invalid_client bytes at
+// /introspect as any other failed caller.
+func TestIntrospectionOfAPublicCallerLooksAlike(t *testing.T) {
+ t.Parallel()
+
+ env := failuresEnv(t)
+ env.clients.clients["spa"] = &oauth2.DefaultClient{
+ IDValue: "spa", TypeValue: oauth2.ClientPublic, GrantTypeValues: []string{oauth2.GrantTypeAuthorizationCode},
+ }
+
+ req := httptest.NewRequest(http.MethodPost, "/oauth2/introspect", strings.NewReader("token=ghost&client_id=spa"))
+ req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+
+ rec := serve(env.srv.IntrospectHandler(), req)
+ requireConstantInvalidClient(t, env, rec, errPublicCaller.Error(), nil)
+}
diff --git a/oauth2/client_issuer.go b/oauth2/client_issuer.go
index ba02f430..225e846b 100644
--- a/oauth2/client_issuer.go
+++ b/oauth2/client_issuer.go
@@ -15,10 +15,14 @@ import (
// issuerClients is the [ClientStore] a request sees at its issuer: a
// client bound to other issuers ([IssuerBoundClient]) does not exist
-// there. One instance serves one request.
+// there, and neither does an identifier longer than [MaxClientIDLength],
+// which is never looked up. One instance serves one request.
type issuerClients struct {
store ClientStore
issuer string
+ // anyIssuer reports that the issuer is unknown — the IssuerResolver
+ // failed — so the binding cannot be checked: the request fails anyway.
+ anyIssuer bool
// hidden records that a client bound to other issuers was hidden: the
// cause of the refusal that follows.
hidden bool
@@ -26,8 +30,12 @@ type issuerClients struct {
// LoadClient implements [ClientStore].
func (c *issuerClients) LoadClient(ctx context.Context, id string) (Client, error) {
+ if len(id) > MaxClientIDLength {
+ return nil, nil
+ }
+
client, err := c.store.LoadClient(ctx, id)
- if err != nil || client == nil {
+ if err != nil || client == nil || c.anyIssuer {
return client, err //nolint:wrapcheck // the store's own answer, unchanged
}
@@ -54,6 +62,7 @@ func (c *issuerClients) explain(oe *Error) *Error {
// authenticateClientAt resolves the issuer of r, then authenticates its
// client at that issuer — a client bound to other issuers is unknown there
// — and returns the client, the issuer and the audience the resolver gave.
+// Every failed authentication reads the same (clientAuthFailed).
func (s *Server) authenticateClientAt(ctx context.Context, r *http.Request) (Client, string, string, error) {
issuer, audience, err := s.resolveIssuer(ctx, r)
if err != nil {
@@ -62,12 +71,27 @@ func (s *Server) authenticateClientAt(ctx context.Context, r *http.Request) (Cli
client, err := s.authenticateClient(ctx, r, issuer)
if err != nil {
- return nil, "", "", err
+ return nil, "", "", clientAuthFailed(err)
}
return client, issuer, audience, nil
}
+// clientAuthFailed answers every failed client authentication alike — 401
+// invalid_client, "client authentication failed" — whatever made it fail:
+// an unknown client, a client of another issuer, a wrong secret, a method
+// the client may not use, a malformed credential (RFC 6749 §5.2). Telling
+// them apart on the wire would confirm a client exists and disclose its type
+// and methods; the refusal it replaces stays in the cause, for the
+// [ErrorHook] only. Any other error passes through unchanged.
+func clientAuthFailed(err error) error {
+ if IsCode(err) != CodeInvalidClient {
+ return err
+ }
+
+ return ErrInvalidClient.WithCause(err)
+}
+
// authenticateClient runs the configured client-authentication methods in
// order and returns the first match, the clients of other issuers hidden.
// An authenticator that does not load the client through the store it is
diff --git a/oauth2/clientauth/basic.go b/oauth2/clientauth/basic.go
index 1edc5b06..9ec6b633 100644
--- a/oauth2/clientauth/basic.go
+++ b/oauth2/clientauth/basic.go
@@ -62,7 +62,7 @@ func (basicAuth) Authenticate(ctx context.Context, r *http.Request, store oauth2
client, err := store.LoadClient(ctx, id)
if err != nil {
- return nil, errInvalid(err)
+ return nil, loadFailure(err)
}
if client == nil {
diff --git a/oauth2/clientauth/clientauth.go b/oauth2/clientauth/clientauth.go
index 7718b346..69854a65 100644
--- a/oauth2/clientauth/clientauth.go
+++ b/oauth2/clientauth/clientauth.go
@@ -19,6 +19,11 @@
// client_secret — and refuses a body client_id naming another client than
// the one authenticated.
//
+// The descriptions of the invalid_client errors the methods return reach
+// the server's ErrorHook only: on the wire, the server answers every failed
+// client authentication alike (401 invalid_client, "client authentication
+// failed").
+//
// Adding private_key_jwt requires the JWT module; it lives behind a small
// adapter in the jwt sub-module so this package stays JOSE-free.
package clientauth
@@ -27,6 +32,7 @@ import (
"context"
"encoding/base64"
"errors"
+ "fmt"
"net/http"
"net/url"
"strings"
@@ -41,8 +47,11 @@ import (
//
// Authenticate MUST return:
// - (client, nil) on success.
-// - (nil, oauth2.ErrInvalidClient) on credential mismatch.
-// - (nil, other) on storage / unexpected errors.
+// - (nil, oauth2.ErrInvalidClient) on credential mismatch, or an unknown
+// client.
+// - (nil, other) on storage / unexpected errors: the
+// server answers server_error (temporarily_unavailable at /revoke),
+// never invalid_client.
type ClientAuthenticator interface {
// Method returns the RFC 6749 / OIDC method identifier
// ("client_secret_basic", "client_secret_post", "none",
@@ -145,6 +154,15 @@ func formValue(r *http.Request, name string) string {
return param.Value(r.PostForm, name)
}
+// loadFailure returns the error a client lookup failed with, wrapped: an
+// error carrying invalid_client keeps it — the store reports an unknown
+// client so (the [oauth2.ClientStore] contract) — and any other is a
+// failure of the store, which the server answers as an outage, never as
+// invalid_client: that would tell the client its credentials are wrong.
+func loadFailure(err error) error {
+ return fmt.Errorf("clientauth: load client: %w", err)
+}
+
// errSecretMismatch is the typed error returned by secret-matcher
// implementations on cleartext or hashed-secret mismatch. It is wrapped in
// oauth2.ErrInvalidClient before being returned to the caller.
diff --git a/oauth2/clientauth/clientauth_test.go b/oauth2/clientauth/clientauth_test.go
index e869611d..a026adc5 100644
--- a/oauth2/clientauth/clientauth_test.go
+++ b/oauth2/clientauth/clientauth_test.go
@@ -54,7 +54,7 @@ type noMatcherClient struct {
methods []string
}
-func (c noMatcherClient) ID() string { return c.id }
+func (c noMatcherClient) ID() string { return c.id }
func (c noMatcherClient) Type() oauth2.ClientType { return c.typ }
func (c noMatcherClient) RedirectURIs() []string { return nil }
func (c noMatcherClient) GrantTypes() []string { return nil }
@@ -192,8 +192,11 @@ func TestBasicAuthenticate(t *testing.T) {
t.Run("store error", func(t *testing.T) {
t.Parallel()
- boom := fakeStore{err: errors.New("db down")}
- _, err := NewBasic().Authenticate(ctx, req(basicHeader("c1", "s3cr3t")), boom)
+ down := errors.New("db down")
+ _, err := NewBasic().Authenticate(ctx, req(basicHeader("c1", "s3cr3t")), fakeStore{err: down})
+ assertStoreFailure(t, err, down)
+
+ _, err = NewBasic().Authenticate(ctx, req(basicHeader("c1", "s3cr3t")), fakeStore{err: oauth2.ErrInvalidClient})
assertInvalidClient(t, err)
})
@@ -277,9 +280,13 @@ func TestPostAuthenticate(t *testing.T) {
t.Run("store error", func(t *testing.T) {
t.Parallel()
+ down := errors.New("db down")
_, err := NewPost().Authenticate(ctx,
- postReq(url.Values{"client_id": {"c1"}, "client_secret": {"s3cr3t"}}),
- fakeStore{err: errors.New("db down")})
+ postReq(url.Values{"client_id": {"c1"}, "client_secret": {"s3cr3t"}}), fakeStore{err: down})
+ assertStoreFailure(t, err, down)
+
+ _, err = NewPost().Authenticate(ctx,
+ postReq(url.Values{"client_id": {"c1"}, "client_secret": {"s3cr3t"}}), fakeStore{err: oauth2.ErrInvalidClient})
assertInvalidClient(t, err)
})
@@ -435,8 +442,11 @@ func TestNoneAuthenticate(t *testing.T) {
t.Run("store error", func(t *testing.T) {
t.Parallel()
- _, err := NewNone().Authenticate(ctx, postReq(url.Values{"client_id": {"pub"}}),
- fakeStore{err: errors.New("db down")})
+ down := errors.New("db down")
+ _, err := NewNone().Authenticate(ctx, postReq(url.Values{"client_id": {"pub"}}), fakeStore{err: down})
+ assertStoreFailure(t, err, down)
+
+ _, err = NewNone().Authenticate(ctx, postReq(url.Values{"client_id": {"pub"}}), fakeStore{err: oauth2.ErrInvalidClient})
assertInvalidClient(t, err)
})
@@ -463,3 +473,14 @@ func TestNoneAuthenticate(t *testing.T) {
assertInvalidClient(t, err)
})
}
+
+// assertStoreFailure asserts that a client store failure is returned as the
+// failure it is — wrapping cause, never invalid_client: the server answers
+// an outage, not a wrong credential.
+func assertStoreFailure(t *testing.T, err, cause error) {
+ t.Helper()
+
+ require.Error(t, err)
+ assert.ErrorIs(t, err, cause)
+ assert.Empty(t, oauth2.IsCode(err), "not an OAuth2 error: the server answers server_error")
+}
diff --git a/oauth2/clientauth/none.go b/oauth2/clientauth/none.go
index b351d7b8..97963255 100644
--- a/oauth2/clientauth/none.go
+++ b/oauth2/clientauth/none.go
@@ -52,7 +52,7 @@ func (noneAuth) Authenticate(ctx context.Context, r *http.Request, store oauth2.
client, err := store.LoadClient(ctx, id)
if err != nil {
- return nil, errInvalid(err)
+ return nil, loadFailure(err)
}
if client == nil {
diff --git a/oauth2/clientauth/post.go b/oauth2/clientauth/post.go
index c8fa6963..8039e1f9 100644
--- a/oauth2/clientauth/post.go
+++ b/oauth2/clientauth/post.go
@@ -50,7 +50,7 @@ func (postAuth) Authenticate(ctx context.Context, r *http.Request, store oauth2.
client, err := store.LoadClient(ctx, id)
if err != nil {
- return nil, errInvalid(err)
+ return nil, loadFailure(err)
}
if client == nil {
diff --git a/oauth2/doc.go b/oauth2/doc.go
index a74a303c..932e68fc 100644
--- a/oauth2/doc.go
+++ b/oauth2/doc.go
@@ -13,12 +13,15 @@
// - Endpoints: AuthorizeHandler runs the RFC 6749 §3.1 authorization
// endpoint (authorization_code, and the opt-in legacy implicit flow);
// TokenHandler, RevokeHandler, IntrospectHandler and MetadataHandler
-// cover the remaining RFC endpoints.
+// cover the remaining RFC endpoints. Every request is bounded
+// (ServerConfig.MaxRequestBytes, 64 KiB by default), and so are the
+// state, nonce and client_id the server echoes, records or looks up.
// - IntrospectHandler answers confidential clients only (a public
// client's client_id is no credential), and the optional
// ServerConfig.IntrospectionPolicy decides which active tokens each
// caller may see, enforcing resource and tenant boundaries (RFC 7662
-// §4).
+// §4); without one, a refresh token is disclosed to its own client
+// only.
// - Clients get only what they declare: the grant types of
// Client.GrantTypes — checked at /token for every grant, custom ones
// included, before it runs — and the scopes of Client.Scopes, an empty
@@ -44,9 +47,10 @@
// failed after a refresh-token reuse or an authorization-code replay,
// and a failed IntrospectionPolicy. See ErrorHook.
// - Every authorization is bound to the issuer the IssuerResolver
-// returned at /authorize: the code and every token record it, and the
+// returned at /authorize: the code and every token record it, the
// authorization_code and refresh_token grants refuse them at another
-// issuer. Every authorization response names that issuer in its iss
+// issuer, /introspect answers them inactive and /revoke leaves them
+// untouched there. Every authorization response names that issuer in its iss
// parameter (RFC 9207), errors included. A client implementing
// IssuerBoundClient is unknown at the issuers it is not registered at.
// - ServerConfig.Extension plugs an optional protocol extension — the
diff --git a/oauth2/empty_client_secret_test.go b/oauth2/empty_client_secret_test.go
index da47814f..19b2b12e 100644
--- a/oauth2/empty_client_secret_test.go
+++ b/oauth2/empty_client_secret_test.go
@@ -19,8 +19,10 @@ import (
"github.com/stretchr/testify/require"
)
-// missingCredentials is the error_description client_secret_basic and
-// client_secret_post answer a missing client_id or an empty secret with.
+// missingCredentials is the detail the ErrorHook gets when
+// client_secret_basic or client_secret_post refuses a missing client_id or
+// an empty secret: on the wire, every failed client authentication reads
+// "client authentication failed".
const missingCredentials = "missing client_id or client_secret"
// emptySecretNow is the instant newEmptySecretServer's clock is pinned to.
@@ -126,7 +128,7 @@ func TestEmptyBasicPasswordRefusedAtEveryEndpoint(t *testing.T) {
ref := serve(h, basicForm(path, ep.form("ghost"), "ghost", ""))
require.Equal(t, http.StatusUnauthorized, ref.Code)
assert.Equal(t, `Basic realm="oauth2"`, ref.Header().Get("WWW-Authenticate"))
- assert.JSONEq(t, `{"error":"invalid_client","error_description":"`+missingCredentials+`"}`, ref.Body.String())
+ assert.JSONEq(t, `{"error":"invalid_client","error_description":"client authentication failed"}`, ref.Body.String())
desc, _ := decodeBody(t, ref)["error_description"].(string)
assert.Regexp(t, descriptionCharset, desc)
diff --git a/oauth2/extension.go b/oauth2/extension.go
index 5a377c37..558e497d 100644
--- a/oauth2/extension.go
+++ b/oauth2/extension.go
@@ -90,9 +90,10 @@ type Extension interface {
// received: the core refused it when it repeats a parameter the core
// reads, but r.Form still holds the empty occurrences — which RFC 6749
// §3.1 omits: read the core's parameters from ar, never r.FormValue,
- // which an empty occurrence can hide them from — the extension's own
- // parameters as sent, repeated or not (refuse your own repetitions), and
- // the nonce unbounded: bound it.
+ // which an empty occurrence can hide them from — and the extension's own
+ // parameters as sent, repeated or not (refuse your own repetitions),
+ // within [ServerConfig.MaxRequestBytes]. ar.State and ar.Nonce are
+ // bounded already ([MaxStateLength], [MaxNonceLength]).
ValidateAuthorizeRequest(ctx context.Context, r *http.Request, ar *AuthorizeRequest) (any, error)
// CompleteAuthorization decides what an approved authorization records.
@@ -156,8 +157,8 @@ type TokenIssuance struct {
Access AccessToken
// Nonce is the nonce the authorization code recorded: set at the code
// exchange only, always empty on a refresh (OpenID Connect Core §12.2).
- // The core does not bound it: an extension bounds it when it validates
- // the authorization request.
+ // The authorization endpoint bounded it: at most [MaxNonceLength] bytes
+ // of valid UTF-8 without NUL.
Nonce string
// RefreshTokenIssued reports whether a refresh token is persisted with
// the access token — minted at a code exchange (a RefreshTokens
diff --git a/oauth2/extension_helpers_test.go b/oauth2/extension_helpers_test.go
index 3537814a..3f138e71 100644
--- a/oauth2/extension_helpers_test.go
+++ b/oauth2/extension_helpers_test.go
@@ -275,6 +275,11 @@ func withProfile(p oauth2.Profile) envOption {
return func(s *oauth2.ServerConfig, _ *grant.Config) { s.Profile = p }
}
+// withMaxRequestBytes bounds the server's requests to n bytes.
+func withMaxRequestBytes(n int64) envOption {
+ return func(s *oauth2.ServerConfig, _ *grant.Config) { s.MaxRequestBytes = n }
+}
+
// newExtensionEnv builds a server — authorization_code, refresh_token and
// client_credentials over a memory store, a confidential client allowed
// "openid read write", StaticIssuer("https://auth.example", "api"), a
diff --git a/oauth2/hook.go b/oauth2/hook.go
index 4a89c5f0..9d814197 100644
--- a/oauth2/hook.go
+++ b/oauth2/hook.go
@@ -17,8 +17,18 @@ import "context"
// failure at /revoke, reported as a server_error — when it leaves the
// revocation unfinished, /revoke answers a bare 503
// temporarily_unavailable (RFC 7009 §2.2.1) that is not reported
-// again — or an [IntrospectionPolicy] that failed (the token is
-// withheld behind a bare {"active":false}, RFC 7662 §2.2).
+// again — a failed token lookup at /introspect, or an
+// [IntrospectionPolicy] that failed (the token is withheld behind a
+// bare {"active":false}, RFC 7662 §2.1, §2.2), and a [ClientStore]
+// that failed (a server_error carrying its error, whatever the
+// endpoint answers: 500, or 503 at /revoke).
+//
+// A failed client authentication at /token, /revoke or /introspect reaches
+// it as the invalid_client the wire carries — "client authentication
+// failed", whatever the reason — whose cause holds the refusal it replaced:
+// errors.As on its Unwrap finds the detailed [*Error] ("unknown client",
+// "method not allowed for client"…), and errors.Is finds
+// [ErrClientNotAtIssuer] for a client of another issuer.
//
// The hook is a pure observability sink: it MUST NOT influence the
// response. It runs synchronously on the request goroutine, so a slow
diff --git a/oauth2/introspect_endpoint.go b/oauth2/introspect_endpoint.go
index 8b5a7c98..12bdf16e 100644
--- a/oauth2/introspect_endpoint.go
+++ b/oauth2/introspect_endpoint.go
@@ -12,9 +12,9 @@ import (
"github.com/hyperscale-stack/security/oauth2/internal/param"
)
-// introspectionCallerRefused is the error_description of the 401 answered
-// to a caller that is not a confidential client. It stays within the
-// RFC 6749 §5.2 charset.
+// introspectionCallerRefused describes, to the [ErrorHook], the refusal of
+// a caller that is not a confidential client — answered on the wire like
+// every failed client authentication.
const introspectionCallerRefused = "introspection requires confidential client authentication"
// IntrospectHandler returns the http.Handler for RFC 7662 token
@@ -31,10 +31,20 @@ const introspectionCallerRefused = "introspection requires confidential client a
// whatever the method, "none" included — or an untyped one gets 401
// invalid_client.
//
-// An active token is disclosed — "active":true plus its claims — when the
-// [ServerConfig.IntrospectionPolicy] allows it, or when no policy is set.
-// Anything else (an unknown, expired or consumed token, or one the policy
-// withholds) answers a bare {"active":false} (RFC 7662 §2.2).
+// A token is introspected at the issuer it was issued under only (OpenID
+// Connect Core §16.15: issuers are separate realms): at the issuer the
+// request resolves to, a token of another issuer — or one recorded without
+// issuer, written before tokens recorded one — is not active. An active
+// token of that issuer is disclosed — "active":true plus its claims — when
+// the [ServerConfig.IntrospectionPolicy] allows it; without a policy, an
+// access token is disclosed to every caller, and a refresh token to the
+// client it was issued to only (RFC 6749 §1.5: refresh tokens are meant for
+// the authorization server). Anything else (an unknown, expired or consumed
+// token, one of another issuer, one withheld) answers a bare
+// {"active":false} (RFC 7662 §2.2) — so does a token whose lookup failed
+// (RFC 7662 §2.1: a server that cannot determine the state of a token
+// SHOULD answer it is not active), the failure reported to the
+// [ErrorHook].
//
// token and token_type_hint, like the client credentials, are
// single-valued: a request including one more than once with a value is
@@ -50,13 +60,13 @@ func (s *Server) serveIntrospect(w http.ResponseWriter, r *http.Request) {
return
}
- if err := r.ParseForm(); err != nil {
- s.writeOAuthError(r.Context(), w, ErrInvalidRequest.WithCause(err))
+ if err := s.parseRequestForm(w, r); err != nil {
+ s.writeOAuthError(r.Context(), w, formRefusal(err))
return
}
- caller, err := s.authenticateIntrospectionCaller(r.Context(), r)
+ caller, issuer, err := s.authenticateIntrospectionCaller(r.Context(), r)
if err != nil {
s.writeOAuthError(r.Context(), w, err)
@@ -79,7 +89,7 @@ func (s *Server) serveIntrospect(w http.ResponseWriter, r *http.Request) {
return
}
- body := s.introspect(r.Context(), caller, rawToken)
+ body := s.introspect(r.Context(), caller, issuer, rawToken)
w.Header().Set("Content-Type", "application/json")
w.Header().Set("Cache-Control", "no-store")
@@ -93,18 +103,19 @@ func (s *Server) serveIntrospect(w http.ResponseWriter, r *http.Request) {
// authenticateIntrospectionCaller authenticates the caller through the
// shared client-authentication chain, then admits confidential clients
// only: a public client's credentials prove nothing (RFC 6749 §2.2, §2.3),
-// and an untyped client cannot be told apart from one.
-func (s *Server) authenticateIntrospectionCaller(ctx context.Context, r *http.Request) (Client, error) {
- caller, _, _, err := s.authenticateClientAt(ctx, r)
+// and an untyped client cannot be told apart from one. It returns the
+// caller and the issuer the request resolved to.
+func (s *Server) authenticateIntrospectionCaller(ctx context.Context, r *http.Request) (Client, string, error) {
+ caller, issuer, _, err := s.authenticateClientAt(ctx, r)
if err != nil {
- return nil, err
+ return nil, "", err
}
if caller.Type() != ClientConfidential {
- return nil, ErrInvalidClient.WithDescription(introspectionCallerRefused)
+ return nil, "", clientAuthFailed(ErrInvalidClient.WithDescription(introspectionCallerRefused))
}
- return caller, nil
+ return caller, issuer, nil
}
// introspectResponse is the RFC 7662 §2.2 JSON envelope. We populate the
@@ -121,12 +132,12 @@ type introspectResponse struct {
Audience string `json:"aud,omitempty"`
}
-// introspect builds the response for rawToken as seen by caller: the
-// claims of an active token the policy discloses, a bare inactive answer
-// for anything else.
-func (s *Server) introspect(ctx context.Context, caller Client, rawToken string) introspectResponse {
+// introspect builds the response for rawToken as seen by caller at issuer:
+// the claims of an active token it may learn, a bare inactive answer for
+// anything else.
+func (s *Server) introspect(ctx context.Context, caller Client, issuer, rawToken string) introspectResponse {
tok, ok := s.lookupActiveToken(ctx, rawToken)
- if !ok || !s.introspectionAllowed(ctx, caller, tok) {
+ if !ok || !s.introspectionAllowed(ctx, caller, issuer, tok) {
return introspectResponse{Active: false}
}
@@ -135,12 +146,17 @@ func (s *Server) introspect(ctx context.Context, caller Client, rawToken string)
// lookupActiveToken resolves rawToken as an access token, then as a
// refresh token, and reports whether it is active: an expired token or a
-// consumed refresh token is not.
+// consumed refresh token is not, and neither is one whose lookup failed —
+// RFC 7662 §2.1: a server unable to determine the state of a token SHOULD
+// answer it is not active — the failure reported to the [ErrorHook].
func (s *Server) lookupActiveToken(ctx context.Context, rawToken string) (IntrospectedToken, bool) {
+ const what = "introspect: token lookup failed"
+
hash := HashToken(nil, rawToken)
now := s.cfg.Now()
- if at, err := s.cfg.Storage.LookupAccessToken(ctx, hash); err == nil {
+ at, err := s.cfg.Storage.LookupAccessToken(ctx, hash)
+ if err == nil {
if at.IsExpired(now) {
return IntrospectedToken{}, false
}
@@ -157,7 +173,12 @@ func (s *Server) lookupActiveToken(ctx context.Context, rawToken string) (Intros
}, true
}
- if rt, err := s.cfg.Storage.LookupRefreshToken(ctx, hash); err == nil {
+ if !s.lookupMissed(ctx, err, what) {
+ return IntrospectedToken{}, false
+ }
+
+ rt, err := s.cfg.Storage.LookupRefreshToken(ctx, hash)
+ if err == nil {
if rt.IsExpired(now) || rt.Consumed {
return IntrospectedToken{}, false
}
@@ -173,16 +194,27 @@ func (s *Server) lookupActiveToken(ctx context.Context, rawToken string) (Intros
}, true
}
+ s.lookupMissed(ctx, err, what)
+
return IntrospectedToken{}, false
}
-// introspectionAllowed runs the configured [IntrospectionPolicy]; without
-// one, every confidential caller may see every token. A failed policy
-// withholds the token — fail closed — and is reported to the [ErrorHook],
-// since the response must not tell it apart from an unknown token.
-func (s *Server) introspectionAllowed(ctx context.Context, caller Client, tok IntrospectedToken) bool {
+// introspectionAllowed reports whether caller may learn tok at issuer. A
+// token of another issuer never: issuers are compared exactly, a token
+// recorded without one matching an empty issuer only (OpenID Connect Core
+// §16.15, RFC 7662 §4). Then the configured [IntrospectionPolicy] decides;
+// without one, every access token is disclosed, and a refresh token to its
+// own client only — refresh tokens are meant for the authorization server
+// (RFC 6749 §1.5). A failed policy withholds the token — fail closed — and
+// is reported to the [ErrorHook], since the response must not tell it apart
+// from an unknown token.
+func (s *Server) introspectionAllowed(ctx context.Context, caller Client, issuer string, tok IntrospectedToken) bool {
+ if tok.Issuer != issuer {
+ return false
+ }
+
if s.cfg.IntrospectionPolicy == nil {
- return true
+ return tok.Kind != TokenKindRefresh || tok.ClientID == caller.ID()
}
allowed, err := s.cfg.IntrospectionPolicy(ctx, caller, tok)
diff --git a/oauth2/introspect_endpoint_test.go b/oauth2/introspect_endpoint_test.go
index f75e12f8..7de2a4a4 100644
--- a/oauth2/introspect_endpoint_test.go
+++ b/oauth2/introspect_endpoint_test.go
@@ -42,8 +42,9 @@ const (
)
const (
- // introspectionRefused is the error_description /introspect answers
- // every caller that is not a confidential client with.
+ // introspectionRefused is the detail the ErrorHook gets when
+ // /introspect refuses a caller that is not a confidential client: on
+ // the wire, every failed client authentication reads the same.
introspectionRefused = "introspection requires confidential client authentication"
// inactiveBody is the bare RFC 7662 §2.2 answer for a token the caller
// may not see.
@@ -284,7 +285,7 @@ func TestIntrospectCallerAuthentication(t *testing.T) {
name string
req *http.Request
accepted bool
- wantDesc string // the pinned error_description, if any
+ wantDesc string // the detail the ErrorHook gets, if pinned
}{
{
name: "client_secret_basic confidential client",
@@ -375,11 +376,12 @@ func TestIntrospectCallerAuthentication(t *testing.T) {
assert.NotEmpty(t, rec.Header().Get("WWW-Authenticate"))
assert.Equal(t, oauth2.CodeInvalidClient, body["error"])
- desc, _ := body["error_description"].(string)
- assert.Regexp(t, descriptionCharset, desc)
+ // ...whose description is the same for every refused
+ // caller (RFC 6749 §5.2), the detail going to the hook only.
+ assert.Equal(t, "client authentication failed", body["error_description"])
if tc.wantDesc != "" {
- assert.Equal(t, tc.wantDesc, desc)
+ assert.Equal(t, tc.wantDesc, hookDetail(t, sink))
}
// ...that tells nothing about the token, never handed to the
@@ -395,8 +397,10 @@ func TestIntrospectCallerAuthentication(t *testing.T) {
// TestIntrospectDefaultDisclosesToConfidentialResourceServer pins the
// default: with no policy, any authenticated confidential client sees any
-// token — including one issued to another client, since the resource
-// server asking is usually not the client the token was issued to.
+// access token of the request's issuer — including one issued to another
+// client, since the resource server asking is usually not the client the
+// token was issued to — but no refresh token of another client: they are
+// meant for the authorization server (RFC 6749 §1.5).
func TestIntrospectDefaultDisclosesToConfidentialResourceServer(t *testing.T) {
t.Parallel()
@@ -410,9 +414,7 @@ func TestIntrospectDefaultDisclosesToConfidentialResourceServer(t *testing.T) {
rec = serve(srv.IntrospectHandler(), introspectRequest(rsBillingClientID, rsBillingSecret, "rt-app"))
require.Equal(t, http.StatusOK, rec.Code)
- assert.JSONEq(t, fmt.Sprintf(
- `{"active":true,"scope":"read","client_id":"app","sub":"alice","exp":%d,"iat":%d,"token_type":"refresh_token"}`,
- seededRefreshExp.Unix(), seededIssuedAt.Unix()), rec.Body.String())
+ assert.JSONEq(t, inactiveBody, rec.Body.String(), "another client's refresh token")
}
// tenantKey carries a request-scoped tenant, as a middleware would set it.
@@ -704,3 +706,19 @@ func TestPublicClientAuthorizationCodeFlowUnaffected(t *testing.T) {
require.Equal(t, http.StatusOK, rec.Code)
assert.JSONEq(t, inactiveBody, rec.Body.String())
}
+
+// hookDetail returns the description of the refusal the constant
+// invalid_client the sink received replaced.
+func hookDetail(t *testing.T, sink *errorSink) string {
+ t.Helper()
+
+ reported := sink.collected()
+ require.Len(t, reported, 1)
+
+ var envelope, detailed *oauth2.Error
+
+ require.ErrorAs(t, reported[0], &envelope)
+ require.ErrorAs(t, envelope.Unwrap(), &detailed)
+
+ return detailed.Description
+}
diff --git a/oauth2/introspect_policy.go b/oauth2/introspect_policy.go
index 340d221d..872f856d 100644
--- a/oauth2/introspect_policy.go
+++ b/oauth2/introspect_policy.go
@@ -39,8 +39,10 @@ type IntrospectedToken struct {
// IssuerResolver returned no audience.
Audience string
// Issuer is the issuer the token was issued under — the realm of its
- // authorization; empty for a token written before tokens recorded it.
- // A policy enforcing realm boundaries compares it with the caller's.
+ // authorization. A policy only sees the tokens of the issuer the
+ // request resolved to, so it is always that issuer: a token of another
+ // issuer, or recorded without issuer (written before tokens recorded
+ // one), is inactive there without consulting the policy.
Issuer string
// IssuedAt is the issuance time.
IssuedAt time.Time
@@ -68,9 +70,10 @@ type IntrospectedToken struct {
// ("introspection policy failed"). Set [ServerConfig.OnError] along with
// a policy, or its failures go unseen.
//
-// The policy only runs for an active token — an unknown, expired or
-// consumed one answers {"active":false} without consulting it — and only
-// once the caller has authenticated, so caller.Type() is always
+// The policy only runs for an active token of the issuer the request
+// resolved to — an unknown, expired or consumed token, or one of another
+// issuer, answers {"active":false} without consulting it — and only once
+// the caller has authenticated, so caller.Type() is always
// [ClientConfidential]. caller is the client the authentication returned —
// the [ClientStore] record for the built-in methods — so it can be
// type-asserted to your own [Client] implementation to read tenant
@@ -80,7 +83,8 @@ type IntrospectedToken struct {
// Do not reduce the check to caller.ID() == token.ClientID: the resource
// server introspecting a token is usually not the client it was issued to.
// Refresh tokens carry no audience and are meant for their client alone:
-// refuse them to resource servers.
+// without a policy, only that client sees one; a policy decides for them
+// too, so refuse them to resource servers.
//
// The policy runs synchronously on the request goroutine, before any byte
// of the response is written, and may be called concurrently. It MUST NOT
diff --git a/oauth2/issuer_isolation_test.go b/oauth2/issuer_isolation_test.go
new file mode 100644
index 00000000..806f586c
--- /dev/null
+++ b/oauth2/issuer_isolation_test.go
@@ -0,0 +1,213 @@
+// Copyright 2026 Hyperscale. All rights reserved.
+// Use of this source code is governed by a MIT
+// license that can be found in the LICENSE file.
+
+package oauth2_test
+
+import (
+ "context"
+ "net/http"
+ "net/http/httptest"
+ "net/url"
+ "strings"
+ "sync/atomic"
+ "testing"
+
+ "github.com/hyperscale-stack/security/oauth2"
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+)
+
+// atHost posts form to path on host as the test client, through h.
+func atHost(h http.Handler, host, path string, form url.Values) *httptest.ResponseRecorder {
+ req := formRequest(path, form, true)
+ req.Host = host
+
+ return serve(h, req)
+}
+
+// realmTokens runs the code flow at a.example on a realm server and
+// returns the access and refresh tokens its test client obtained.
+func realmTokens(t *testing.T, srv *oauth2.Server) (string, string) {
+ t.Helper()
+
+ rec := tokenAt(srv, "a.example", codeExchangeForm(codeAt(t, srv, "a.example")))
+ require.Equal(t, http.StatusOK, rec.Code, rec.Body.String())
+
+ body := jsonBody(t, rec)
+ at, _ := body["access_token"].(string)
+ rt, _ := body["refresh_token"].(string)
+ require.NotEmpty(t, at)
+ require.NotEmpty(t, rt)
+
+ return at, rt
+}
+
+// introspectedActive reports what /introspect at host says of raw.
+func introspectedActive(t *testing.T, srv *oauth2.Server, host, raw string) bool {
+ t.Helper()
+
+ rec := atHost(srv.IntrospectHandler(), host, "/oauth2/introspect", url.Values{"token": {raw}})
+ require.Equal(t, http.StatusOK, rec.Code, rec.Body.String())
+
+ active, _ := jsonBody(t, rec)["active"].(bool)
+ if !active {
+ assert.JSONEq(t, inactiveBody, rec.Body.String(), "a bare inactive answer")
+ }
+
+ return active
+}
+
+// TestIntrospectionIsBoundToTheRequestIssuer pins issuer isolation at
+// /introspect (OIDC Core §16.15, RFC 7662 §4): a token of another issuer is
+// inactive at this one — for the client it was issued to too — even though
+// the client is registered at both.
+func TestIntrospectionIsBoundToTheRequestIssuer(t *testing.T) {
+ t.Parallel()
+
+ srv, _ := newRealmServer(t, hostIssuer{})
+ at, rt := realmTokens(t, srv)
+
+ assert.True(t, introspectedActive(t, srv, "a.example", at))
+ assert.True(t, introspectedActive(t, srv, "a.example", rt))
+ assert.False(t, introspectedActive(t, srv, "b.example", at), "the access token of another issuer")
+ assert.False(t, introspectedActive(t, srv, "b.example", rt), "the refresh token of another issuer")
+}
+
+// TestRevocationIsBoundToTheRequestIssuer pins issuer isolation at /revoke
+// (RFC 7009 §2.1): a token of another issuer is not revoked — the answer
+// is 200 all the same — while a token recorded without issuer, written
+// before tokens recorded one, is still revoked by its client: revocation
+// only takes away.
+func TestRevocationIsBoundToTheRequestIssuer(t *testing.T) {
+ t.Parallel()
+
+ srv, store := newRealmServer(t, hostIssuer{})
+ at, rt := realmTokens(t, srv)
+
+ for _, raw := range []string{at, rt} {
+ rec := atHost(srv.RevokeHandler(), "b.example", "/oauth2/revoke", url.Values{"token": {raw}})
+ require.Equal(t, http.StatusOK, rec.Code)
+ }
+
+ assert.True(t, introspectedActive(t, srv, "a.example", at), "untouched by a revocation at another issuer")
+ assert.True(t, introspectedActive(t, srv, "a.example", rt), "untouched by a revocation at another issuer")
+
+ rec := atHost(srv.RevokeHandler(), "a.example", "/oauth2/revoke", url.Values{"token": {at}})
+ require.Equal(t, http.StatusOK, rec.Code)
+ assert.False(t, introspectedActive(t, srv, "a.example", at))
+ assert.False(t, introspectedActive(t, srv, "a.example", rt), "the family went with it")
+
+ legacy := newClientPair("legacy-rt", "fam-legacy")
+ legacy.Access.Issuer, legacy.Refresh.Issuer = "", ""
+ seedPair(t, store, legacy)
+
+ rec = atHost(srv.RevokeHandler(), "a.example", "/oauth2/revoke", url.Values{"token": {legacy.Access.Token}})
+ require.Equal(t, http.StatusOK, rec.Code)
+
+ _, err := store.LookupAccessToken(t.Context(), legacy.Access.TokenHash)
+ assert.Error(t, err, "an issuer-less token is revoked by its client")
+}
+
+// isolationEnv is a boundsEnv whose "machine" holds an access and a refresh
+// token ("iso-rt"), and which also serves the confidential resource server
+// "rs" (secret "rs-secret"), behind policy.
+func isolationEnv(t *testing.T, policy oauth2.IntrospectionPolicy, pairs ...*oauth2.TokenPair) *boundsEnv {
+ t.Helper()
+
+ env := newBoundsEnv(t, 0, nil, nil)
+ env.clients.clients["rs"] = &oauth2.DefaultClient{IDValue: "rs", Secret: "rs-secret", TypeValue: oauth2.ClientConfidential}
+
+ cfg := env.srv.Config()
+ cfg.IntrospectionPolicy = policy
+
+ srv, err := oauth2.NewServer(cfg)
+ require.NoError(t, err)
+
+ env.srv = srv
+
+ for _, p := range pairs {
+ require.NoError(t, env.store.Store.SaveTokenPair(t.Context(), p))
+ }
+
+ return env
+}
+
+// machinePair is a pair of "machine", issued under issuer.
+func machinePair(name, issuer string) *oauth2.TokenPair {
+ pair := newClientPair(name, "fam-"+name)
+ pair.Access.ClientID, pair.Refresh.ClientID = "machine", "machine"
+ pair.Access.Issuer, pair.Refresh.Issuer = issuer, issuer
+
+ return pair
+}
+
+// introspectAs introspects raw at env as the client id.
+func introspectAs(t *testing.T, env *boundsEnv, id, secret, raw string) bool {
+ t.Helper()
+
+ req := httptest.NewRequest(http.MethodPost, "/oauth2/introspect", strings.NewReader(url.Values{"token": {raw}}.Encode()))
+ req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+ req.SetBasicAuth(id, secret)
+
+ rec := serve(env.srv.IntrospectHandler(), req)
+ require.Equal(t, http.StatusOK, rec.Code, rec.Body.String())
+
+ active, _ := jsonBody(t, rec)["active"].(bool)
+
+ return active
+}
+
+// TestRefreshTokensIntrospectOnlyForTheirClientByDefault: refresh tokens
+// are meant for the authorization server and their client (RFC 6749 §1.5):
+// without an IntrospectionPolicy, /introspect discloses one to its own
+// client only — a resource server still sees access tokens. A policy, when
+// set, decides.
+func TestRefreshTokensIntrospectOnlyForTheirClientByDefault(t *testing.T) {
+ t.Parallel()
+
+ pair := machinePair("iso", "https://auth.example")
+
+ env := isolationEnv(t, nil, pair)
+ assert.False(t, introspectAs(t, env, "rs", "rs-secret", pair.Refresh.Token), "another client's refresh token")
+ assert.True(t, introspectAs(t, env, "machine", "machine-secret", pair.Refresh.Token), "its own refresh token")
+ assert.True(t, introspectAs(t, env, "rs", "rs-secret", pair.Access.Token), "a resource server sees access tokens")
+
+ allowAll := func(context.Context, oauth2.Client, oauth2.IntrospectedToken) (bool, error) { return true, nil }
+ env = isolationEnv(t, allowAll, pair)
+ assert.True(t, introspectAs(t, env, "rs", "rs-secret", pair.Refresh.Token), "a policy decides")
+}
+
+// TestPolicySeesOnlyTokensOfTheRequestIssuer: the IntrospectionPolicy runs
+// for tokens of the issuer the request resolved to only — so the token's
+// Issuer it gets is the request's — and a token of another issuer, or
+// recorded without issuer, is inactive without consulting it.
+func TestPolicySeesOnlyTokensOfTheRequestIssuer(t *testing.T) {
+ t.Parallel()
+
+ var (
+ calls atomic.Int32
+ issuer atomic.Value
+ )
+
+ spy := func(_ context.Context, _ oauth2.Client, tok oauth2.IntrospectedToken) (bool, error) {
+ calls.Add(1)
+ issuer.Store(tok.Issuer)
+
+ return true, nil
+ }
+
+ own := machinePair("own", "https://auth.example")
+ other := machinePair("other", "https://other.example")
+ legacy := machinePair("legacy", "")
+
+ env := isolationEnv(t, spy, own, other, legacy)
+
+ assert.False(t, introspectAs(t, env, "rs", "rs-secret", other.Access.Token))
+ assert.False(t, introspectAs(t, env, "rs", "rs-secret", legacy.Access.Token), "an issuer-less token at an issuer")
+ assert.Zero(t, calls.Load(), "the policy never sees another issuer's token")
+
+ assert.True(t, introspectAs(t, env, "rs", "rs-secret", own.Access.Token))
+ assert.Equal(t, int32(1), calls.Load())
+ assert.Equal(t, "https://auth.example", issuer.Load())
+}
diff --git a/oauth2/metadata_endpoint.go b/oauth2/metadata_endpoint.go
index 6122adcf..330c8eb6 100644
--- a/oauth2/metadata_endpoint.go
+++ b/oauth2/metadata_endpoint.go
@@ -6,25 +6,51 @@ package oauth2
import (
"encoding/json"
+ "maps"
"net/http"
+ "slices"
"strings"
"github.com/hyperscale-stack/security/oauth2/pkce"
)
-// MetadataHandler returns the http.Handler for RFC 8414's
-// /.well-known/oauth-authorization-server discovery document. The
-// payload is derived from the active ServerConfig so adding a grant or
-// changing the issuer is automatically reflected.
+// MetadataHandler returns the http.Handler for the RFC 8414 authorization
+// server metadata document, to mount at
+// /.well-known/oauth-authorization-server followed by the path of the
+// issuer, if any (RFC 8414 §3). It answers GET and HEAD (RFC 8414 §3.1);
+// any other method gets 405 with Allow: GET, HEAD.
//
-// Endpoint URLs are built as issuer + ServerConfig.RoutePrefix + "/",
-// so the document stays consistent with wherever the handlers are mounted.
-// The jwks_uri keeps the host-root .well-known location per RFC 8615.
+// The document is built for the issuer the request resolves to, from the
+// server itself, so adding a grant or changing the issuer is reflected:
+//
+// - endpoint URLs are the issuer + [ServerConfig.RoutePrefix] +
+// "/", consistent with wherever the handlers are mounted;
+// - jwks_uri is the issuer, a trailing "/" removed, followed by
+// "/.well-known/jwks.json" — where oidc/provider mounts its key set;
+// the oauth2 module alone serves none;
+// - grant_types_supported lists the registered grants, sorted;
+// - response_types_supported and response_modes_supported are "code" and
+// "query" — plus "token", "fragment" and the "implicit" grant type once
+// a [Server.AuthorizeHandler] enabling [AuthorizeConfig.AllowImplicit]
+// was built: they reflect the handlers built so far;
+// - token_endpoint_auth_methods_supported and
+// revocation_endpoint_auth_methods_supported list the
+// [ServerConfig.ClientAuth] methods, and
+// introspection_endpoint_auth_methods_supported the same but "none" —
+// /introspect answers confidential clients only — omitted when no
+// method is left.
func (s *Server) MetadataHandler() http.Handler {
return http.HandlerFunc(s.serveMetadata)
}
func (s *Server) serveMetadata(w http.ResponseWriter, r *http.Request) {
+ if r.Method != http.MethodGet && r.Method != http.MethodHead {
+ w.Header().Set("Allow", "GET, HEAD")
+ http.Error(w, "oauth2: metadata requires GET or HEAD", http.StatusMethodNotAllowed)
+
+ return
+ }
+
issuer, _, err := s.resolveIssuer(r.Context(), r)
if err != nil {
s.writeOAuthError(r.Context(), w, err)
@@ -34,10 +60,11 @@ func (s *Server) serveMetadata(w http.ResponseWriter, r *http.Request) {
base := strings.TrimSuffix(issuer, "/")
routes := base + s.cfg.RoutePrefix
+ methods := s.clientAuthMethods()
- // response_modes_supported names the mode /authorize answers the code
- // response type in: omitting it would mean ["query", "fragment"] (RFC
- // 8414 §2), and the handler refuses fragment for the code flow.
+ // response_modes_supported names the modes /authorize answers in:
+ // omitting it would mean ["query", "fragment"] (RFC 8414 §2), and the
+ // handler refuses fragment for the code flow.
doc := metadataDoc{
Issuer: issuer,
AuthorizationEndpoint: routes + "/authorize",
@@ -48,12 +75,23 @@ func (s *Server) serveMetadata(w http.ResponseWriter, r *http.Request) {
GrantTypesSupported: s.grantTypes(),
ResponseTypesSupported: []string{responseTypeCode},
ResponseModesSupported: []string{responseModeQuery},
- TokenEndpointAuthMethodsSupported: s.clientAuthMethods(),
- CodeChallengeMethodsSupported: s.pkceMethods(),
+ TokenEndpointAuthMethodsSupported: methods,
+ // Omitted, the revocation list would mean client_secret_basic alone
+ // (RFC 8414 §2).
+ RevocationEndpointAuthMethodsSupported: methods,
+ IntrospectionEndpointAuthMethodsSupported: confidentialMethods(methods),
+ CodeChallengeMethodsSupported: s.pkceMethods(),
// Every authorization response names the issuer (RFC 9207 §2.3, §3).
AuthorizationResponseISSParameterSupported: true,
}
+ if s.implicit.Load() {
+ doc.ResponseTypesSupported = append(doc.ResponseTypesSupported, responseTypeToken)
+ doc.ResponseModesSupported = append(doc.ResponseModesSupported, responseModeFragment)
+ doc.GrantTypesSupported = append(doc.GrantTypesSupported, GrantTypeImplicit)
+ slices.Sort(doc.GrantTypesSupported)
+ }
+
w.Header().Set("Content-Type", "application/json")
w.Header().Set("Cache-Control", "max-age=300")
w.WriteHeader(http.StatusOK)
@@ -76,19 +114,20 @@ type metadataDoc struct {
ResponseTypesSupported []string `json:"response_types_supported"`
ResponseModesSupported []string `json:"response_modes_supported"`
TokenEndpointAuthMethodsSupported []string `json:"token_endpoint_auth_methods_supported"`
- CodeChallengeMethodsSupported []string `json:"code_challenge_methods_supported,omitempty"`
+ // RevocationEndpointAuthMethodsSupported is never empty: NewServer
+ // requires a ClientAuthenticator.
+ RevocationEndpointAuthMethodsSupported []string `json:"revocation_endpoint_auth_methods_supported"`
+ IntrospectionEndpointAuthMethodsSupported []string `json:"introspection_endpoint_auth_methods_supported,omitempty"`
+ CodeChallengeMethodsSupported []string `json:"code_challenge_methods_supported,omitempty"`
// AuthorizationResponseISSParameterSupported is RFC 9207 §3; omitted, it
// would mean false.
AuthorizationResponseISSParameterSupported bool `json:"authorization_response_iss_parameter_supported"`
}
+// grantTypes lists the registered grant types, sorted: map order would
+// change from one response to the next.
func (s *Server) grantTypes() []string {
- out := make([]string, 0, len(s.dispatch))
- for t := range s.dispatch {
- out = append(out, t)
- }
-
- return out
+ return slices.Sorted(maps.Keys(s.dispatch))
}
func (s *Server) clientAuthMethods() []string {
@@ -100,6 +139,24 @@ func (s *Server) clientAuthMethods() []string {
return out
}
+// clientAuthMethodNone is the client authentication method of a public
+// client, which authenticates nothing (RFC 7591 §2).
+const clientAuthMethodNone = "none"
+
+// confidentialMethods is methods without "none": the methods by which a
+// confidential client may authenticate.
+func confidentialMethods(methods []string) []string {
+ out := make([]string, 0, len(methods))
+
+ for _, m := range methods {
+ if m != clientAuthMethodNone {
+ out = append(out, m)
+ }
+ }
+
+ return out
+}
+
func (s *Server) pkceMethods() []string {
if s.cfg.Profile.AllowsPKCEPlain() {
return []string{pkce.MethodS256.String(), pkce.MethodPlain.String()}
diff --git a/oauth2/metadata_test.go b/oauth2/metadata_test.go
new file mode 100644
index 00000000..a2f9dcd4
--- /dev/null
+++ b/oauth2/metadata_test.go
@@ -0,0 +1,120 @@
+// Copyright 2026 Hyperscale. All rights reserved.
+// Use of this source code is governed by a MIT
+// license that can be found in the LICENSE file.
+
+package oauth2_test
+
+import (
+ "encoding/json"
+ "net/http"
+ "net/http/httptest"
+ "testing"
+
+ "github.com/hyperscale-stack/security/oauth2"
+ "github.com/hyperscale-stack/security/oauth2/clientauth"
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+)
+
+// metadataPath is where the tests request the RFC 8414 document.
+const metadataPath = "/.well-known/oauth-authorization-server"
+
+// metadataOf returns the RFC 8414 document srv serves.
+func metadataOf(t *testing.T, srv *oauth2.Server) map[string]any {
+ t.Helper()
+
+ rec := serve(srv.MetadataHandler(), httptest.NewRequest(http.MethodGet, metadataPath, nil))
+ require.Equal(t, http.StatusOK, rec.Code, rec.Body.String())
+
+ var doc map[string]any
+ require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &doc))
+
+ return doc
+}
+
+// TestMetadataReflectsTheServer pins RFC 8414 §2 against what the server
+// serves: the response types and modes of the flows its AuthorizeHandlers
+// run, the grant types it registers — sorted, "implicit" (RFC 7591 §2)
+// once the implicit flow is enabled — and the client authentication
+// methods of /revoke and /introspect, whose omission would mean
+// client_secret_basic and nothing respectively; /introspect answers
+// confidential clients only, so "none" is not one of its methods.
+func TestMetadataReflectsTheServer(t *testing.T) {
+ t.Parallel()
+
+ allMethods := []any{"client_secret_basic", "client_secret_post", "none"}
+
+ t.Run("code flow", func(t *testing.T) {
+ t.Parallel()
+
+ env := newBoundsEnv(t, 0, nil, nil)
+ _ = env.srv.AuthorizeHandler(oauth2.AuthorizeConfig{}, approve)
+
+ doc := metadataOf(t, env.srv)
+ assert.Equal(t, []any{"code"}, doc["response_types_supported"])
+ assert.Equal(t, []any{"query"}, doc["response_modes_supported"])
+ assert.Equal(t, []any{"authorization_code", "client_credentials", "refresh_token"}, doc["grant_types_supported"])
+ assert.Equal(t, allMethods, doc["token_endpoint_auth_methods_supported"])
+ assert.Equal(t, allMethods, doc["revocation_endpoint_auth_methods_supported"])
+ assert.Equal(t, []any{"client_secret_basic", "client_secret_post"}, doc["introspection_endpoint_auth_methods_supported"])
+ })
+
+ t.Run("implicit flow enabled", func(t *testing.T) {
+ t.Parallel()
+
+ env := newBoundsEnv(t, 0, nil, nil)
+ _ = env.authorizeHandler(approve)
+
+ doc := metadataOf(t, env.srv)
+ assert.Equal(t, []any{"code", "token"}, doc["response_types_supported"])
+ assert.Equal(t, []any{"query", "fragment"}, doc["response_modes_supported"])
+ assert.Equal(t, []any{"authorization_code", "client_credentials", "implicit", "refresh_token"}, doc["grant_types_supported"])
+ })
+
+ t.Run("grant types are sorted", func(t *testing.T) {
+ t.Parallel()
+
+ env := newBoundsEnv(t, 0, nil, nil)
+
+ // Map iteration order varies from one call to the next: ask often.
+ for range 32 {
+ assert.Equal(t, []any{"authorization_code", "client_credentials", "refresh_token"},
+ metadataOf(t, env.srv)["grant_types_supported"])
+ }
+ })
+
+ t.Run("public clients only", func(t *testing.T) {
+ t.Parallel()
+
+ cfg := newBoundsEnv(t, 0, nil, nil).srv.Config()
+ cfg.ClientAuth = []oauth2.ClientAuthenticator{clientauth.NewNone()}
+
+ srv, err := oauth2.NewServer(cfg)
+ require.NoError(t, err)
+
+ doc := metadataOf(t, srv)
+ assert.Equal(t, []any{"none"}, doc["revocation_endpoint_auth_methods_supported"])
+ assert.NotContains(t, doc, "introspection_endpoint_auth_methods_supported",
+ "no method authenticates a confidential client")
+ })
+}
+
+// TestMetadataAnswersGETAndHEADOnly: the document is retrieved with GET
+// (RFC 8414 §3.1); HEAD gets its headers, any other method 405 listing the
+// two in Allow (RFC 9110 §15.5.6).
+func TestMetadataAnswersGETAndHEADOnly(t *testing.T) {
+ t.Parallel()
+
+ env := newBoundsEnv(t, 0, nil, nil)
+
+ rec := serve(env.srv.MetadataHandler(), httptest.NewRequest(http.MethodHead, metadataPath, nil))
+ assert.Equal(t, http.StatusOK, rec.Code)
+ assert.Equal(t, "application/json", rec.Header().Get("Content-Type"))
+
+ for _, method := range []string{http.MethodPost, http.MethodPut, http.MethodPatch, http.MethodDelete, http.MethodOptions} {
+ rec := serve(env.srv.MetadataHandler(), httptest.NewRequest(method, metadataPath, nil))
+ assert.Equal(t, http.StatusMethodNotAllowed, rec.Code, method)
+ assert.Equal(t, "GET, HEAD", rec.Header().Get("Allow"), method)
+ assert.NotContains(t, rec.Body.String(), "issuer", method)
+ }
+}
diff --git a/oauth2/models.go b/oauth2/models.go
index 5fef19db..88000a7b 100644
--- a/oauth2/models.go
+++ b/oauth2/models.go
@@ -46,7 +46,8 @@ type AuthorizationCode struct {
// CodeChallengeMethod is the PKCE method ("S256" or "plain").
CodeChallengeMethod string
// Nonce echoes the OIDC nonce parameter for replay protection in id
- // tokens. Empty for plain OAuth2 flows.
+ // tokens. Empty for plain OAuth2 flows. At most [MaxNonceLength] bytes
+ // of valid UTF-8 without NUL, which the authorization endpoint enforces.
Nonce string
// IssuedAt is the wall-clock issuance time.
IssuedAt time.Time
diff --git a/oauth2/params.go b/oauth2/params.go
index d12dd515..0e7427bf 100644
--- a/oauth2/params.go
+++ b/oauth2/params.go
@@ -11,6 +11,8 @@ const (
paramRedirectURI = "redirect_uri"
paramResponseMode = "response_mode"
paramScope = "scope"
+ paramState = "state"
+ paramCode = "code"
)
// The request parameters each endpoint reads itself (RFC 6749 §3.1, §3.2):
@@ -26,7 +28,7 @@ var (
// order, so a request repeating one of them is refused with a bare 400
// whatever else it repeats.
authorizeParameters = []string{
- paramClientID, paramRedirectURI, paramResponseMode, "response_type", paramScope, "state",
+ paramClientID, paramRedirectURI, paramResponseMode, "response_type", paramScope, paramState,
"code_challenge", "code_challenge_method", "nonce",
}
// unredirectableParameters, repeated, leave the client, the redirection
@@ -36,7 +38,7 @@ var (
// tokenParameters are the parameters of an access-token request (RFC
// 6749 §2.3.1, §4.1.3, §4.3.2, §4.4.2, §6; RFC 7636 §4.5).
tokenParameters = []string{
- "grant_type", "code", paramRedirectURI, "code_verifier", "refresh_token",
+ "grant_type", paramCode, paramRedirectURI, "code_verifier", "refresh_token",
paramScope, "username", "password", paramClientID, paramClientSecret,
}
// clientAuthParameters are the client credentials a request body may
diff --git a/oauth2/redirect_uri.go b/oauth2/redirect_uri.go
new file mode 100644
index 00000000..4fe2443d
--- /dev/null
+++ b/oauth2/redirect_uri.go
@@ -0,0 +1,257 @@
+// Copyright 2026 Hyperscale. All rights reserved.
+// Use of this source code is governed by a MIT
+// license that can be found in the LICENSE file.
+
+package oauth2
+
+import (
+ "errors"
+ "fmt"
+ "net/url"
+ "slices"
+ "strconv"
+ "strings"
+)
+
+// ErrInvalidRedirectURI is wrapped by every [ValidateRedirectURI] refusal.
+// It is the cause the [ErrorHook] gets when /authorize meets a registered
+// redirect URI the rules refuse: the request is then answered with a bare
+// 400, never redirected (RFC 6749 §3.1.2.4).
+var ErrInvalidRedirectURI = errors.New("oauth2: invalid redirect URI")
+
+// reservedRedirectParameters are the authorization response parameters the
+// server adds to the query of a redirect URI (RFC 6749 §4.1.2, §4.1.2.1;
+// RFC 9207 §2): a registered query naming one would carry it twice (RFC
+// 6749 §3.1).
+var reservedRedirectParameters = []string{paramCode, paramState, "iss", "error", "error_description", "error_uri"}
+
+// ValidateRedirectURI reports whether uri may be registered as a redirect
+// URI, wrapping [ErrInvalidRedirectURI] with the reason when it may not.
+// Call it when registering a client; [Server.AuthorizeHandler] applies it to
+// the registered URI a request resolves to as well. The rules:
+//
+// - an absolute URI (RFC 6749 §3.1.2: "MUST be an absolute URI", RFC 3986
+// §4.3) made of RFC 3986 characters, its percent-encodings valid;
+// - no fragment, not even an empty one (RFC 6749 §3.1.2: "MUST NOT include
+// a fragment component");
+// - for http and https, a host, no userinfo and, when present, a port
+// within 1–65535 — TLS is not required (RFC 6749 §3.1.2.1 SHOULD;
+// http://localhost serves development);
+// - for any other scheme — the private-use scheme of a native app — a
+// scheme holding a "." (RFC 8252 §7.1: a reverse domain name; §8.4:
+// "any private-use URI scheme that doesn't contain a period character
+// SHOULD be rejected"), which also refuses javascript:, data:,
+// vbscript:, file: and a bare myapp:;
+// - a query, kept as registered when the response is added (RFC 6749
+// §3.1.2), naming — split on "&" and ";" — none of the parameters the
+// server adds: code, state, iss, error, error_description, error_uri
+// (RFC 6749 §3.1: no parameter twice).
+func ValidateRedirectURI(uri string) error {
+ if reason := redirectURISyntax(uri); reason != "" {
+ return fmt.Errorf("%w: %s", ErrInvalidRedirectURI, reason)
+ }
+
+ u, err := url.Parse(uri)
+ if err != nil {
+ return fmt.Errorf("%w: %w", ErrInvalidRedirectURI, err)
+ }
+
+ if reason := redirectURIParts(u); reason != "" {
+ return fmt.Errorf("%w: %s", ErrInvalidRedirectURI, reason)
+ }
+
+ return nil
+}
+
+// redirectURISyntax returns why uri is not made of RFC 3986 characters
+// without fragment, or "".
+func redirectURISyntax(uri string) string {
+ switch {
+ case uri == "":
+ return "empty"
+ case strings.Contains(uri, "#"):
+ return "it has a fragment"
+ }
+
+ for i := 0; i < len(uri); i++ {
+ switch c := uri[i]; {
+ case c == '%':
+ if i+2 >= len(uri) || !isHexDigit(uri[i+1]) || !isHexDigit(uri[i+2]) {
+ return "an invalid percent-encoding"
+ }
+
+ i += 2
+ case !isURIChar(c):
+ return "a character outside RFC 3986"
+ }
+ }
+
+ return ""
+}
+
+// redirectURIParts returns why the parsed URI u breaks the scheme, host or
+// query rules, or "".
+func redirectURIParts(u *url.URL) string {
+ switch u.Scheme {
+ case "":
+ return "not an absolute URI"
+ case "http", "https":
+ // Hostname, not Host: "http://:80/cb" has an authority, no host
+ // (RFC 9110 §4.2.1: such a URI MUST be rejected as invalid).
+ if u.Hostname() == "" {
+ return "no host"
+ }
+
+ if u.User != nil {
+ return "userinfo"
+ }
+
+ if port := u.Port(); port != "" && !inPortRange(port) {
+ return "a port outside 1-65535"
+ }
+ default:
+ if !strings.Contains(u.Scheme, ".") {
+ return "a private-use scheme without a period (RFC 8252 §7.1)"
+ }
+ }
+
+ for field := range strings.FieldsFuncSeq(u.RawQuery, func(r rune) bool { return r == '&' || r == ';' }) {
+ name, _, _ := strings.Cut(field, "=")
+
+ if name, err := url.QueryUnescape(name); err == nil && slices.Contains(reservedRedirectParameters, name) {
+ return "its query names an authorization response parameter"
+ }
+ }
+
+ return ""
+}
+
+// inPortRange reports whether the decimal port is a TCP port a URI may name:
+// 1 to 65535 (0 is reserved, RFC 6335 §6).
+func inPortRange(port string) bool {
+ n, err := strconv.Atoi(port)
+
+ return err == nil && n >= 1 && n <= 65535
+}
+
+// isURIChar reports whether c may appear, unencoded, in an RFC 3986 URI:
+// unreserved (ALPHA DIGIT - . _ ~) or reserved (gen-delims and sub-delims).
+func isURIChar(c byte) bool {
+ switch {
+ case 'a' <= c && c <= 'z', 'A' <= c && c <= 'Z', '0' <= c && c <= '9':
+ return true
+ default:
+ return strings.IndexByte("-._~:/?#[]@!$&'()*+,;=", c) >= 0
+ }
+}
+
+// isHexDigit reports whether c is an RFC 3986 HEXDIG.
+func isHexDigit(c byte) bool {
+ return '0' <= c && c <= '9' || 'a' <= c && c <= 'f' || 'A' <= c && c <= 'F'
+}
+
+// errUnregisteredRedirectURI is the cause of a request naming no redirect
+// URI the client registered — or none at all, for a client registering
+// several.
+var errUnregisteredRedirectURI = errors.New("oauth2: missing or unregistered redirect_uri")
+
+// resolveRedirectURI returns the redirect URI of a request: the requested
+// one when it exactly matches a registered URI (RFC 6749 §3.1.2.3: simple
+// string comparison) or a registered loopback IP redirect URI but for the
+// port (RFC 8252 §7.3), or the sole registered URI when the request omitted
+// the parameter. The registered URI it rests on must pass
+// [ValidateRedirectURI]: an invalid one is never redirected to.
+func resolveRedirectURI(client Client, requested string) (string, error) {
+ registered := client.RedirectURIs()
+
+ var used, resolved string
+
+ switch {
+ case requested == "":
+ if len(registered) != 1 {
+ return "", errUnregisteredRedirectURI
+ }
+
+ used, resolved = registered[0], registered[0]
+ case slices.Contains(registered, requested):
+ used, resolved = requested, requested
+ default:
+ i := slices.IndexFunc(registered, func(r string) bool { return loopbackRedirect(r, requested) })
+ if i < 0 {
+ return "", errUnregisteredRedirectURI
+ }
+
+ used, resolved = registered[i], requested
+ }
+
+ if err := ValidateRedirectURI(used); err != nil {
+ return "", err
+ }
+
+ return resolved, nil
+}
+
+// loopbackRedirect reports whether requested is the loopback IP redirect
+// URI registered but for its port: RFC 8252 §7.3 — "The authorization
+// server MUST allow any port to be specified at the time of the request for
+// loopback IP redirect URIs" — and RFC 9700 §2.1, the one exception to
+// exact matching. Both URIs use http and the IP literal 127.0.0.1 or [::1]
+// (localhost gets no variance, RFC 8252 §8.3), any port being a decimal
+// number from 1 to 65535 without a leading zero, and they are byte-for-byte
+// equal once the port of each authority is removed — path, query and
+// fragment included.
+func loopbackRedirect(registered, requested string) bool {
+ regHost, regRest, ok := loopbackParts(registered)
+ if !ok {
+ return false
+ }
+
+ reqHost, reqRest, ok := loopbackParts(requested)
+
+ return ok && regHost == reqHost && regRest == reqRest
+}
+
+// loopbackParts splits a loopback IP redirect URI — "http://", 127.0.0.1 or
+// [::1], an optional valid port, then the rest — into its host and the rest
+// that follows the authority, and reports whether uri is one.
+func loopbackParts(uri string) (string, string, bool) {
+ const scheme = "http://"
+
+ if !strings.HasPrefix(uri, scheme) {
+ return "", "", false
+ }
+
+ authority, rest := uri[len(scheme):], ""
+ if i := strings.IndexAny(authority, "/?#"); i >= 0 {
+ authority, rest = authority[:i], authority[i:]
+ }
+
+ for _, host := range []string{"127.0.0.1", "[::1]"} {
+ if port, ok := strings.CutPrefix(authority, host); ok && (port == "" || validPort(port)) {
+ return host, rest, true
+ }
+ }
+
+ return "", "", false
+}
+
+// validPort reports whether p is ":" and a port from 1 to 65535 written
+// without a leading zero.
+func validPort(p string) bool {
+ digits, ok := strings.CutPrefix(p, ":")
+ if !ok || digits == "" || len(digits) > 5 || digits[0] == '0' {
+ return false
+ }
+
+ n := 0
+
+ for i := 0; i < len(digits); i++ {
+ if digits[i] < '0' || digits[i] > '9' {
+ return false
+ }
+
+ n = n*10 + int(digits[i]-'0')
+ }
+
+ return n <= 65535
+}
diff --git a/oauth2/redirect_uri_internal_test.go b/oauth2/redirect_uri_internal_test.go
new file mode 100644
index 00000000..fe8eba16
--- /dev/null
+++ b/oauth2/redirect_uri_internal_test.go
@@ -0,0 +1,134 @@
+// Copyright 2026 Hyperscale. All rights reserved.
+// Use of this source code is governed by a MIT
+// license that can be found in the LICENSE file.
+
+package oauth2
+
+import (
+ "net/url"
+ "regexp"
+ "strings"
+ "testing"
+
+ "github.com/stretchr/testify/assert"
+)
+
+// TestLoopbackRedirect pins the comparison of RFC 8252 §7.3: two loopback
+// IP redirect URIs match when they are byte-for-byte equal once the port of
+// each authority is removed.
+func TestLoopbackRedirect(t *testing.T) {
+ t.Parallel()
+
+ cases := []struct {
+ registered, requested string
+ match bool
+ }{
+ {"http://127.0.0.1/cb", "http://127.0.0.1:1/cb", true},
+ {"http://127.0.0.1:8080/cb", "http://127.0.0.1:9090/cb", true},
+ {"http://127.0.0.1:8080/cb", "http://127.0.0.1/cb", true},
+ {"http://[::1]/cb", "http://[::1]:65535/cb", true},
+ {"http://127.0.0.1/cb?x=1", "http://127.0.0.1:2/cb?x=1", true},
+ {"http://127.0.0.1", "http://127.0.0.1:2", true},
+ {"http://127.0.0.1/cb", "http://127.0.0.1:2/cb?", false},
+ {"http://127.0.0.1/cb", "http://127.0.0.1:2/cb#", false},
+ {"http://127.0.0.1/cb", "http://[::1]:2/cb", false},
+ {"http://127.0.0.1/cb", "http://127.0.0.1:0/cb", false},
+ {"http://127.0.0.1/cb", "http://127.0.0.1:00001/cb", false},
+ {"http://127.0.0.1/cb", "http://127.0.0.1:123456/cb", false},
+ {"http://127.0.0.1/cb", "http://127.0.0.1:65536/cb", false},
+ {"http://127.0.0.1/cb", "http://127.0.0.1:-1/cb", false},
+ {"http://127.0.0.1/cb", "http://127.0.0.1:1:2/cb", false},
+ {"http://localhost/cb", "http://localhost:2/cb", false},
+ {"https://127.0.0.1/cb", "https://127.0.0.1:2/cb", false},
+ {"http://127.0.0.1:0/cb", "http://127.0.0.1:2/cb", false},
+ {"http://user@127.0.0.1/cb", "http://user@127.0.0.1:2/cb", false},
+ }
+
+ for _, tc := range cases {
+ assert.Equal(t, tc.match, loopbackRedirect(tc.registered, tc.requested), "%s vs %s", tc.registered, tc.requested)
+ }
+}
+
+// loopbackPort matches the ":port" right after a loopback IP literal.
+var loopbackPort = regexp.MustCompile(`^http://(127\.0\.0\.1|\[::1\])(:[0-9]*)?`)
+
+// FuzzLoopbackRedirect checks the property behind loopbackRedirect: a match
+// means both URIs are http loopback IP redirect URIs with a valid port, and
+// identical once that port is removed — the requested one parses with the
+// same host, path, query and fragment as the registered one.
+func FuzzLoopbackRedirect(f *testing.F) {
+ for _, seed := range [][2]string{
+ {"http://127.0.0.1/cb", "http://127.0.0.1:51004/cb"},
+ {"http://[::1]/cb", "http://[::1]:1@evil.example/cb"},
+ {"http://127.0.0.1/cb", "http://127.0.0.1:51004/cb#x"},
+ {"http://127.0.0.1/cb?a=%2F", "http://127.0.0.1:2/cb?a=%2F"},
+ {"http://127.0.0.1/cb", "http://127.0.0.1:2/cb?"},
+ } {
+ f.Add(seed[0], seed[1])
+ }
+
+ f.Fuzz(func(t *testing.T, registered, requested string) {
+ if !loopbackRedirect(registered, requested) {
+ return
+ }
+
+ strip := func(s string) string { return loopbackPort.ReplaceAllString(s, "http://$1") }
+ if strip(registered) != strip(requested) {
+ t.Fatalf("%q matches %q but differs beyond the port", requested, registered)
+ }
+
+ u, err := url.Parse(requested)
+ if err != nil {
+ return // not a URI at all: ValidateRedirectURI refuses the registered twin
+ }
+
+ if u.Scheme != "http" || (u.Hostname() != "127.0.0.1" && u.Hostname() != "::1") || u.User != nil {
+ t.Fatalf("%q matched but is no loopback IP redirect URI", requested)
+ }
+
+ if p := u.Port(); p != "" && (strings.HasPrefix(p, "0") || len(p) > 5) {
+ t.Fatalf("%q matched with port %q", requested, p)
+ }
+ })
+}
+
+// FuzzAuthorizeRedirectTarget pins RFC 6749 §3.1.2 and §4.1.2 together:
+// whatever redirect URI ValidateRedirectURI accepts, the response
+// parameters appended to its query come back exactly once, with their
+// values — none glued to a registered value, none duplicated.
+func FuzzAuthorizeRedirectTarget(f *testing.F) {
+ for _, seed := range []string{
+ "https://app.example/cb", "https://app.example/cb?", "https://app.example/cb?&", "https://app.example/cb?a=1&",
+ "https://app.example/cb?a=1;b=2", "https://app.example/cb?flag", "https://app.example/cb?x=%26code%3D1",
+ "https://app.example/cb?next=/home?", "https://app.example/cb?next=https://partner.example/landing?",
+ "com.example.app:/cb??", "com.example.app:/cb?y=1", "http://127.0.0.1:5/cb?z", "https://app.example/cb?a=b=c",
+ } {
+ f.Add(seed)
+ }
+
+ want := url.Values{paramCode: {"C"}, paramState: {"S"}, "iss": {"I"}}
+
+ f.Fuzz(func(t *testing.T, uri string) {
+ if ValidateRedirectURI(uri) != nil {
+ return
+ }
+
+ target := authorizeRedirectTarget(uri, want, false)
+ if !strings.HasPrefix(target, uri) {
+ t.Fatalf("%q -> %q: the registered URI is not kept", uri, target)
+ }
+
+ u, err := url.Parse(target)
+ if err != nil {
+ t.Fatalf("%q -> unparsable %q: %v", uri, target, err)
+ }
+
+ // The error reports a registered ";" pair, skipped: not one of ours.
+ got, _ := url.ParseQuery(u.RawQuery)
+ for name, values := range want {
+ if len(got[name]) != 1 || got[name][0] != values[0] {
+ t.Fatalf("%q -> %q: %s = %q, want %q once", uri, target, name, got[name], values[0])
+ }
+ }
+ })
+}
diff --git a/oauth2/redirect_uri_test.go b/oauth2/redirect_uri_test.go
new file mode 100644
index 00000000..d4362ffe
--- /dev/null
+++ b/oauth2/redirect_uri_test.go
@@ -0,0 +1,360 @@
+// Copyright 2026 Hyperscale. All rights reserved.
+// Use of this source code is governed by a MIT
+// license that can be found in the LICENSE file.
+
+package oauth2_test
+
+import (
+ "net/http"
+ "net/http/httptest"
+ "net/url"
+ "strings"
+ "testing"
+
+ "github.com/hyperscale-stack/security/oauth2"
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+)
+
+// TestValidateRedirectURI pins the registration rules: an absolute URI (RFC
+// 6749 §3.1.2) of RFC 3986 characters, without fragment; a host and no
+// userinfo for http(s); a private-use scheme holding a "." for native apps
+// (RFC 8252 §7.1, §8.4) — which refuses javascript:, data:, file: and the
+// like; and a query naming none of the authorization response parameters,
+// which RFC 6749 §3.1 forbids twice (§3.1.2 keeps the registered query).
+func TestValidateRedirectURI(t *testing.T) {
+ t.Parallel()
+
+ valid := []string{
+ "https://app.example/cb",
+ "https://app.example/cb?b=2&a=1",
+ "https://app.example/cb?a=1;b=2",
+ "https://app.example/cb?flag",
+ "https://app.example/cb?sig=abc%2Fdef%3D",
+ "https://app.example/cb?State=x",
+ "https://app.example/cb?",
+ "https://app.example:8443/cb",
+ "https://app.example:1/cb",
+ "https://app.example:65535/cb",
+ "https://app.example:/cb",
+ "http://localhost:1337/callback",
+ "http://127.0.0.1:51004/cb",
+ "http://[::1]/cb",
+ "com.example.app:/oauth2redirect/example-provider",
+ "com.example.app:cb",
+ "HTTPS://App.Example/CB",
+ }
+
+ for _, uri := range valid {
+ assert.NoError(t, oauth2.ValidateRedirectURI(uri), uri)
+ }
+
+ const (
+ empty = "empty"
+ fragment = "fragment"
+ chars = "outside RFC 3986"
+ escape = "percent-encoding"
+ relative = "not an absolute URI"
+ period = "without a period"
+ host = "no host"
+ port = "a port outside 1-65535"
+ userinfo = "userinfo"
+ query = "authorization response parameter"
+ )
+
+ invalid := []struct{ uri, reason string }{
+ {"", empty},
+ {"/relative/cb", relative},
+ {"cb", relative},
+ {"//app.example/cb", relative},
+ {"https://app.example/cb#frag", fragment},
+ {"https://app.example/cb#", fragment},
+ {"javascript:alert(1)", period},
+ {"JavaScript:alert(1)", period},
+ {"data:text/html,x", period},
+ {"vbscript:x", period},
+ {"file:///etc/passwd", period},
+ {"myapp://cb", period},
+ {"urn:ietf:wg:oauth:2.0:oob", period},
+ {"http:/cb", host},
+ {"https://", host},
+ {"http://:80/cb", host},
+ {"https://:443/cb", host},
+ {"http://:/cb", host},
+ {"http://[]/cb", "invalid host"},
+ {"http://app.example:0/cb", port},
+ {"http://app.example:65536/cb", port},
+ {"http://app.example:99999/cb", port},
+ {"http://app.example:99999999999999999999/cb", port},
+ {"https://user@app.example/cb", userinfo},
+ {"https://user:pass@app.example/cb", userinfo},
+ {"https://app.example/a b", chars},
+ {"https://app.example/a\tb", chars},
+ {"https://app.example/é", chars},
+ {"https://app.example/a 0 {
+ return s.cfg.MaxRequestBytes
+ }
+
+ return DefaultMaxRequestBytes
+}
+
+// parseRequestForm parses the query and the body of r (http.Request
+// ParseForm) within s.maxRequestBytes(): a query — or a body whose declared
+// length is — longer than the bound is refused before anything is read, and
+// a body of unknown length is read through http.MaxBytesReader. A refusal
+// for size wraps errRequestTooLarge (and the *http.MaxBytesError of a body
+// found too long while reading it). A request whose form a middleware
+// already parsed keeps it: only its query is checked.
+func (s *Server) parseRequestForm(w http.ResponseWriter, r *http.Request) error {
+ limit := s.maxRequestBytes()
+ if int64(len(r.URL.RawQuery)) > limit || r.ContentLength > limit {
+ return errRequestTooLarge
+ }
+
+ if r.Body != nil {
+ r.Body = http.MaxBytesReader(w, r.Body, limit)
+ }
+
+ if err := r.ParseForm(); err != nil {
+ if tooLarge := (*http.MaxBytesError)(nil); errors.As(err, &tooLarge) {
+ return errors.Join(errRequestTooLarge, err)
+ }
+
+ return fmt.Errorf("oauth2: parse request: %w", err)
+ }
+
+ return nil
+}
+
+// formRefusal is the invalid_request answering a request parseRequestForm
+// refused: "request too large", or the sentinel's "the request is
+// malformed"; the cause reaches the [ErrorHook].
+func formRefusal(err error) *Error {
+ if errors.Is(err, errRequestTooLarge) {
+ return errTooLarge.WithCause(err)
+ }
+
+ return ErrInvalidRequest.WithCause(err)
+}
+
+// validState reports whether state may be echoed: at most MaxStateLength
+// bytes of VSCHAR, %x20-7E (RFC 6749 Appendix A.5). An empty state is an
+// omitted one (RFC 6749 §3.1).
+func validState(state string) bool {
+ if len(state) > MaxStateLength {
+ return false
+ }
+
+ for i := range len(state) {
+ if state[i] < 0x20 || state[i] > 0x7E {
+ return false
+ }
+ }
+
+ return true
+}
+
+// validNonce reports whether nonce may be recorded: at most MaxNonceLength
+// bytes of valid UTF-8 without NUL — what every store keeps byte-exact.
+func validNonce(nonce string) bool {
+ return len(nonce) <= MaxNonceLength && utf8.ValidString(nonce) && strings.IndexByte(nonce, 0) < 0
+}
+
+// The invalid_request answers of an invalid state or nonce. They never echo
+// the request.
+var (
+ errMalformedState = ErrInvalidRequest.WithDescription("the state is too long or malformed")
+ errMalformedNonce = ErrInvalidRequest.WithDescription("the nonce is too long or not valid UTF-8")
+)
diff --git a/oauth2/revoke_endpoint.go b/oauth2/revoke_endpoint.go
index d82d8386..a4b53543 100644
--- a/oauth2/revoke_endpoint.go
+++ b/oauth2/revoke_endpoint.go
@@ -23,11 +23,22 @@ var errRevocationUnavailable = newCoded(CodeTemporarilyUnavailable, "revocation
// turn. Revoking a refresh token revokes its whole family, and
// revoking an access token revokes its family too (RFC 7009 §2.1).
//
+// A client revokes its own tokens of that issuer only: a token issued to
+// another client, or under another issuer (OpenID Connect Core §16.15:
+// issuers are separate realms), is left untouched. RFC 7009 §2.1 has such a
+// request refused with an error; the server answers it as an invalid token
+// instead (§2.2), so that /revoke never confirms to whoever holds a token
+// that it is valid for another client. A token recorded without issuer —
+// written before tokens recorded one — is revoked by its client at any
+// issuer: revocation only takes away.
+//
// The answer is 200 OK whether or not the token existed or belonged to the
-// caller (RFC 7009 §2.2), and 503 temporarily_unavailable when a storage
-// failure leaves the revocation unfinished (RFC 7009 §2.2.1): the token's
-// lookup failed, its family could not be revoked, or — for a token outside
-// any family — it could not be deleted. The client then retries, which
+// caller (RFC 7009 §2.2), and 503 temporarily_unavailable when a failure
+// leaves the revocation undone or unfinished (RFC 7009 §2.2.1): the client
+// could not be authenticated for another reason than a refused credential
+// (the IssuerResolver or the [ClientStore] failed), the token's lookup
+// failed, its family could not be revoked, or — for a token outside any
+// family — it could not be deleted. The client then retries, which
// redoes the whole revocation. Each storage failure reaches the
// [ErrorHook] once, as a server_error carrying its cause; the 503 is not
// reported again. A 503 hints that the token exists only when the backend
@@ -49,15 +60,15 @@ func (s *Server) serveRevoke(w http.ResponseWriter, r *http.Request) {
return
}
- if err := r.ParseForm(); err != nil {
- s.writeOAuthError(r.Context(), w, ErrInvalidRequest.WithCause(err))
+ if err := s.parseRequestForm(w, r); err != nil {
+ s.writeOAuthError(r.Context(), w, formRefusal(err))
return
}
- client, _, _, err := s.authenticateClientAt(r.Context(), r)
+ client, issuer, _, err := s.authenticateClientAt(r.Context(), r)
if err != nil {
- s.writeOAuthError(r.Context(), w, err)
+ s.refuseRevocation(r.Context(), w, err)
return
}
@@ -80,7 +91,7 @@ func (s *Server) serveRevoke(w http.ResponseWriter, r *http.Request) {
// RFC 7009 §2.1: the hint is optional. We try access then refresh
// regardless so the caller's hint is treated as advisory.
- if !s.revoke(r.Context(), client, rawToken) {
+ if !s.revoke(r.Context(), client, issuer, rawToken) {
writeErrorResponse(w, errRevocationUnavailable)
return
@@ -91,45 +102,72 @@ func (s *Server) serveRevoke(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
}
+// refuseRevocation answers a revocation whose client could not be
+// authenticated: as the protocol refusal it is (invalid_client,
+// invalid_request), or — for a failure of the IssuerResolver, the
+// ClientStore or an authenticator — temporarily_unavailable (RFC 7009
+// §2.2.1: the revocation did not take place; the client must assume the
+// token still exists, and may retry), the failure reported once.
+func (s *Server) refuseRevocation(ctx context.Context, w http.ResponseWriter, err error) {
+ if code := IsCode(err); code != "" && code != CodeServerError {
+ s.writeOAuthError(ctx, w, err)
+
+ return
+ }
+
+ s.notifyError(ctx, ErrServerError.WithDescription("revoke: client authentication failed").WithCause(err))
+ writeErrorResponse(w, errRevocationUnavailable)
+}
+
// revoke revokes rawToken when it is an access or a refresh token of
-// client, and reports whether the token can no longer be used: true when it
-// was revoked, is unknown, or belongs to another client; false when a
-// lookup or a revocation failed and the token may still be usable. Each
-// failure is reported to the [ErrorHook]; an unknown token is not.
-func (s *Server) revoke(ctx context.Context, client Client, rawToken string) bool {
+// client at issuer (ownedAt), and reports whether the token can no longer
+// be used — as far as the caller is concerned: true when it was revoked, is
+// unknown, or belongs to another client or issuer; false when a lookup or a
+// revocation failed and the token may still be usable. Each failure is
+// reported to the [ErrorHook]; an unknown token is not.
+func (s *Server) revoke(ctx context.Context, client Client, issuer, rawToken string) bool {
hash := HashToken(nil, rawToken)
at, err := s.cfg.Storage.LookupAccessToken(ctx, hash)
if err == nil {
- return s.revokeAccess(ctx, client, at, hash)
+ return s.revokeAccess(ctx, client, issuer, at, hash)
}
- if !s.lookupMissed(ctx, err) {
+ if !s.lookupMissed(ctx, err, "revoke: token lookup failed") {
return false
}
rt, err := s.cfg.Storage.LookupRefreshToken(ctx, hash)
if err != nil {
- return s.lookupMissed(ctx, err)
+ return s.lookupMissed(ctx, err, "revoke: token lookup failed")
}
- if rt.ClientID != client.ID() || rt.FamilyID == "" {
+ if !ownedAt(client, issuer, rt.ClientID, rt.Issuer) || rt.FamilyID == "" {
return true
}
return s.revokeFamily(ctx, rt.FamilyID)
}
+// ownedAt reports whether a token issued to clientID under tokenIssuer is
+// client's at issuer: its own, and of this issuer (RFC 7009 §2.1; OpenID
+// Connect Core §16.15: a client of one realm cannot revoke another
+// realm's grants). A token recorded without issuer — written before tokens
+// recorded one — is its client's at any issuer: revocation only takes away.
+func ownedAt(client Client, issuer, clientID, tokenIssuer string) bool {
+ return clientID == client.ID() && (tokenIssuer == "" || tokenIssuer == issuer)
+}
+
// lookupMissed reports whether a failed lookup merely missed the token:
// invalid_grant, for a token that is unknown, expired from the store, or of
-// a revoked family. Any other failure leaves the token possibly usable and
-// is reported to the [ErrorHook].
-func (s *Server) lookupMissed(ctx context.Context, err error) bool {
+// a revoked family. Any other failure leaves the token's state unknown and
+// is reported to the [ErrorHook] as a server_error described by what.
+func (s *Server) lookupMissed(ctx context.Context, err error, what string) bool {
if IsCode(err) == CodeInvalidGrant {
return true
}
- s.notifyError(ctx, ErrServerError.WithDescription("revoke: token lookup failed").WithCause(err))
+ s.notifyError(ctx, ErrServerError.WithDescription(what).WithCause(err))
return false
}
@@ -144,8 +182,8 @@ func (s *Server) lookupMissed(ctx context.Context, err error) bool {
// is revoked no token of it is usable, and deleting the access token is
// mere cleanup: a failed delete is reported but leaves the answer
// unchanged. A token outside any family only goes away when it is deleted.
-func (s *Server) revokeAccess(ctx context.Context, client Client, at *AccessToken, hash string) bool {
- if at.ClientID != client.ID() {
+func (s *Server) revokeAccess(ctx context.Context, client Client, issuer string, at *AccessToken, hash string) bool {
+ if !ownedAt(client, issuer, at.ClientID, at.Issuer) {
return true
}
diff --git a/oauth2/revoke_failure_test.go b/oauth2/revoke_failure_test.go
index fbb53fee..a5ad930a 100644
--- a/oauth2/revoke_failure_test.go
+++ b/oauth2/revoke_failure_test.go
@@ -231,6 +231,11 @@ func TestRevokeAccessTokenAnswersByTheTokenOutcome(t *testing.T) {
})
}
+// TestRevokeLeavesOtherClientsTokensAlone pins a deliberate deviation from
+// RFC 7009 §2.1, which refuses the revocation of a token issued to another
+// client: the server answers it as an invalid token (§2.2) — 200, the very
+// answer of an unknown token — and leaves the token alone, so /revoke tells
+// no caller whether a token it holds is valid for another client.
func TestRevokeLeavesOtherClientsTokensAlone(t *testing.T) {
t.Parallel()
@@ -242,15 +247,23 @@ func TestRevokeLeavesOtherClientsTokensAlone(t *testing.T) {
other.Access.ClientID, other.Refresh.ClientID = "someone-else", "someone-else"
seedPair(t, store, other)
+ unknown := revoke(srv, "never-issued")
+ require.Equal(t, http.StatusOK, unknown.Code)
+
for _, raw := range []string{other.Access.Token, other.Refresh.Token} {
rec := revoke(srv, raw)
assert.Equal(t, http.StatusOK, rec.Code, "RFC 7009 §2.2: no hint the token exists")
+ assert.Equal(t, unknown.Body.String(), rec.Body.String(), "the answer of an unknown token")
+ assert.Equal(t, unknown.Header(), rec.Header(), "the answer of an unknown token")
}
assert.Empty(t, sink.collected())
_, err := store.LookupAccessToken(t.Context(), other.Access.TokenHash)
require.NoError(t, err, "a client cannot revoke another client's token")
+
+ _, err = store.LookupRefreshToken(t.Context(), other.Refresh.TokenHash)
+ require.NoError(t, err, "nor its refresh token family")
}
func TestRevokeIsQuietForAlreadyRevokedFamilies(t *testing.T) {
diff --git a/oauth2/scope_syntax_test.go b/oauth2/scope_syntax_test.go
index 24704f42..6e1d2738 100644
--- a/oauth2/scope_syntax_test.go
+++ b/oauth2/scope_syntax_test.go
@@ -61,8 +61,11 @@ func TestAuthorizeRefusesMalformedScope(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
+ // 1 MiB requests: the scope bounds, not the request-size bound
+ // (64 KiB by default, which alone refuses the 40 000 tokens),
+ // answer every case.
ext := &fakeExtension{}
- env := newExtensionEnv(t, ext)
+ env := newExtensionEnv(t, ext, withMaxRequestBytes(1<<20))
q := authorizeQuery()
q.Set("scope", tc.scope)
diff --git a/oauth2/server.go b/oauth2/server.go
index af132b16..fcf195d6 100644
--- a/oauth2/server.go
+++ b/oauth2/server.go
@@ -9,6 +9,7 @@ import (
"errors"
"fmt"
"net/http"
+ "sync/atomic"
"time"
)
@@ -60,8 +61,11 @@ type ServerConfig struct {
//
// The value is normalized at construction: an empty prefix defaults to
// "/oauth2", a missing leading slash is added, and a trailing slash is
- // trimmed ("/" yields a root mount). The .well-known endpoints are not
- // affected — they live at the host root per RFC 8615.
+ // trimmed ("/" yields a root mount). The .well-known documents are not
+ // affected: the metadata document lives at
+ // /.well-known/oauth-authorization-server followed by the issuer's path
+ // (RFC 8414 §3), and the jwks_uri it advertises is the issuer followed
+ // by /.well-known/jwks.json (see [Server.MetadataHandler]).
RoutePrefix string
// Now is the clock used to stamp issuance / expiry, and to compute the
// expires_in the /token response advertises. Defaults to time.Now
@@ -70,16 +74,29 @@ type ServerConfig struct {
// OnError, when set, observes every error the server turns into an
// RFC 6749 §5.2 response — including the cause of a server_error, which
// never reaches the wire — plus the errors behind an answer that cannot
- // carry them (the storage failures of /revoke, a failed
+ // carry them (the storage failures of /revoke and /introspect, a failed
// [IntrospectionPolicy]). Optional; see [ErrorHook].
OnError ErrorHook
- // IntrospectionPolicy, when set, decides which active tokens each
- // confidential caller of /introspect may see — the place to enforce
- // resource and tenant boundaries (RFC 7662 §4). A withheld token
- // answers {"active":false}, exactly like an unknown one. Optional; nil
- // lets any authenticated confidential client introspect any token. See
+ // IntrospectionPolicy, when set, decides which active tokens of the
+ // request's issuer each confidential caller of /introspect may see —
+ // the place to enforce resource and tenant boundaries (RFC 7662 §4). A
+ // withheld token answers {"active":false}, exactly like an unknown one.
+ // Optional; nil discloses every access token to any authenticated
+ // confidential client, and a refresh token to its own client only. See
// [IntrospectionPolicy].
IntrospectionPolicy IntrospectionPolicy
+ // MaxRequestBytes bounds every request the server parses: the query
+ // string and the body of /authorize, /token, /revoke and /introspect —
+ // net/http alone reads up to 10 MB of form. A longer request is refused
+ // with invalid_request (a bare 400 at /authorize) before any parameter
+ // is read, its declared length checked before the body is. Zero or
+ // negative selects [DefaultMaxRequestBytes] (64 KiB); raise it only for
+ // custom grants carrying large assertions. A ConsentFunc reading the
+ // body of a POST /authorize itself reads it within the same bound. A
+ // body a middleware parsed before the handler (r.ParseForm,
+ // r.FormValue) was read without it — only its declared length is
+ // checked: bound such a middleware's reads too (http.MaxBytesReader).
+ MaxRequestBytes int64
// Extension, when set, plugs a protocol extension — an OpenID Connect
// provider — into /authorize and into the authorization_code and
// refresh_token exchanges of /token: see [Extension]. Optional: without
@@ -97,6 +114,10 @@ type Server struct {
// dispatch maps Grant.Type() to the Grant instance for O(1) lookup
// on /token.
dispatch map[string]Grant
+
+ // implicit records that an AuthorizeHandler serving the implicit flow
+ // was built, for the metadata document.
+ implicit atomic.Bool
}
// NewServer validates cfg and returns a ready-to-mount [Server]. It
diff --git a/oauth2/server_test.go b/oauth2/server_test.go
index f78c039f..2ae43104 100644
--- a/oauth2/server_test.go
+++ b/oauth2/server_test.go
@@ -393,6 +393,7 @@ func TestIntrospectEndpoint(t *testing.T) {
Scope: "api:read",
IssuedAt: time.Now().Add(-time.Minute),
ExpiresAt: time.Now().Add(time.Hour),
+ Issuer: "https://auth.example",
}}))
body := introspect(t, srv, "live-at")
diff --git a/oauth2/storage/memory/memory.go b/oauth2/storage/memory/memory.go
index dc499fac..183ec30c 100644
--- a/oauth2/storage/memory/memory.go
+++ b/oauth2/storage/memory/memory.go
@@ -7,7 +7,9 @@
// implementations instead — the in-memory store loses all state on
// restart, and never purges anything: it grows with every code and token
// it is given, since consumed and expired codes, expired tokens and
-// revoked families stay in memory for the process lifetime.
+// revoked families stay in memory for the process lifetime. Like every
+// store it keeps codes and tokens hashed only: the raw code or token of a
+// record it is handed is never kept, nor returned by a lookup.
//
// All operations are guarded by a single sync.Mutex; the resulting
// throughput is fine for tens of thousands of req/s but the structure is
@@ -231,12 +233,18 @@ func (s *Store) isRevoked(familyID string) bool {
return revoked
}
-// persist writes the tokens of pair. The caller holds the mutex.
+// persist writes the tokens of pair — copies without their raw value: the
+// store keeps hashes only, and leaves the caller's pair untouched. The
+// caller holds the mutex.
func (s *Store) persist(pair *oauth2.TokenPair) {
- s.access[pair.Access.TokenHash] = pair.Access
+ access := pair.Access
+ access.Token = ""
+ s.access[access.TokenHash] = access
if pair.Refresh != nil {
- s.refresh[pair.Refresh.TokenHash] = *pair.Refresh
+ refresh := *pair.Refresh
+ refresh.Token = ""
+ s.refresh[refresh.TokenHash] = refresh
}
}
diff --git a/oauth2/storage/memory/memory_test.go b/oauth2/storage/memory/memory_test.go
new file mode 100644
index 00000000..ced159bd
--- /dev/null
+++ b/oauth2/storage/memory/memory_test.go
@@ -0,0 +1,56 @@
+// Copyright 2026 Hyperscale. All rights reserved.
+// Use of this source code is governed by a MIT
+// license that can be found in the LICENSE file.
+
+package memory_test
+
+import (
+ "testing"
+ "time"
+
+ "github.com/hyperscale-stack/security/oauth2"
+ "github.com/hyperscale-stack/security/oauth2/storage/memory"
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+)
+
+// TestMemoryKeepsNoRawToken: like every store, the in-memory one keeps
+// tokens hashed only — SaveTokenPair and RotateRefreshToken store copies
+// without the raw value, and the lookups never return it — while the pairs
+// the caller handed over keep theirs.
+func TestMemoryKeepsNoRawToken(t *testing.T) {
+ t.Parallel()
+
+ now := time.Now()
+ pair := func(raw string) *oauth2.TokenPair {
+ return &oauth2.TokenPair{
+ Access: oauth2.AccessToken{
+ Token: raw + "-access", TokenHash: oauth2.HashToken(nil, raw+"-access"),
+ FamilyID: "fam", ExpiresAt: now.Add(time.Hour),
+ },
+ Refresh: &oauth2.RefreshToken{
+ Token: raw, TokenHash: oauth2.HashToken(nil, raw),
+ FamilyID: "fam", ExpiresAt: now.Add(time.Hour),
+ },
+ }
+ }
+
+ s := memory.New()
+ first, next := pair("RAW-REFRESH-SECRET"), pair("RAW-REFRESH-2")
+
+ require.NoError(t, s.SaveTokenPair(t.Context(), first))
+ require.NoError(t, s.RotateRefreshToken(t.Context(), first.Refresh.TokenHash, next))
+
+ for _, p := range []*oauth2.TokenPair{first, next} {
+ at, err := s.LookupAccessToken(t.Context(), p.Access.TokenHash)
+ require.NoError(t, err)
+ assert.Empty(t, at.Token, "the raw access token is never kept")
+
+ rt, err := s.LookupRefreshToken(t.Context(), p.Refresh.TokenHash)
+ require.NoError(t, err)
+ assert.Empty(t, rt.Token, "the raw refresh token is never kept")
+ }
+
+ assert.Equal(t, "RAW-REFRESH-SECRET-access", first.Access.Token, "the caller's pair is untouched")
+ assert.Equal(t, "RAW-REFRESH-2", next.Refresh.Token, "the caller's pair is untouched")
+}
diff --git a/oauth2/store_failures_test.go b/oauth2/store_failures_test.go
new file mode 100644
index 00000000..599d1054
--- /dev/null
+++ b/oauth2/store_failures_test.go
@@ -0,0 +1,236 @@
+// Copyright 2026 Hyperscale. All rights reserved.
+// Use of this source code is governed by a MIT
+// license that can be found in the LICENSE file.
+
+package oauth2_test
+
+import (
+ "context"
+ "errors"
+ "net/http"
+ "net/http/httptest"
+ "net/url"
+ "strings"
+ "testing"
+
+ "github.com/hyperscale-stack/security/oauth2"
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/mock"
+ "github.com/stretchr/testify/require"
+)
+
+// errClientStoreDown is the failure of a client store backend.
+var errClientStoreDown = errors.New("client store: connection refused")
+
+// clientsAnswering is a ClientStore answering every lookup with err — a
+// failing backend, or an unknown client reported with an error.
+func clientsAnswering(t *testing.T, err error) *oauth2.MockClientStore {
+ t.Helper()
+
+ clients := oauth2.NewMockClientStore(t)
+ clients.EXPECT().LoadClient(mock.Anything, mock.Anything).Return(nil, err)
+
+ return clients
+}
+
+// failingClientsEnv is a boundsEnv whose ClientStore is store.
+func failingClientsEnv(t *testing.T, store oauth2.ClientStore, resolver oauth2.IssuerResolver) *boundsEnv {
+ t.Helper()
+
+ env := newBoundsEnv(t, 0, resolver, nil)
+ cfg := env.srv.Config()
+ cfg.ClientStore = store
+
+ srv, err := oauth2.NewServer(cfg)
+ require.NoError(t, err)
+
+ env.srv = srv
+
+ return env
+}
+
+// requireReportedServerError asserts the one server_error the hook got,
+// carrying cause.
+func requireReportedServerError(t *testing.T, env *boundsEnv, cause error) {
+ t.Helper()
+
+ reported := env.sink.collected()
+ require.Len(t, reported, 1)
+ assert.Equal(t, oauth2.CodeServerError, oauth2.IsCode(reported[0]))
+ assert.ErrorIs(t, reported[0], cause)
+}
+
+// TestClientStoreFailureIsAServerError: a ClientStore that fails is an
+// outage, not an unknown client — answering invalid_client would tell a
+// client its credentials are wrong. /authorize answers a bare 500 (no
+// redirect URI can be trusted without the client), /token and /introspect
+// a 500 server_error, /revoke a 503 temporarily_unavailable (RFC 7009
+// §2.2.1: the client must assume the token still exists, and may retry);
+// the failure reaches the hook once.
+func TestClientStoreFailureIsAServerError(t *testing.T) {
+ t.Parallel()
+
+ t.Run("authorize", func(t *testing.T) {
+ t.Parallel()
+
+ env := failingClientsEnv(t, clientsAnswering(t, errClientStoreDown), nil)
+ rec := serve(env.authorizeHandler(approve), httptest.NewRequest(http.MethodGet, authorizePath+"?"+boundsQuery().Encode(), nil))
+
+ require.Equal(t, http.StatusInternalServerError, rec.Code)
+ assert.Equal(t, "oauth2: server error\n", rec.Body.String())
+ assert.Empty(t, rec.Header().Get("Location"))
+ requireReportedServerError(t, env, errClientStoreDown)
+ })
+
+ requests := map[string]func(path string, form url.Values) *http.Request{
+ "client_secret_basic": func(path string, form url.Values) *http.Request {
+ return boundsPost(path, strings.NewReader(form.Encode()))
+ },
+ "client_secret_post": func(path string, form url.Values) *http.Request {
+ form.Set("client_id", "machine")
+ form.Set("client_secret", "machine-secret")
+
+ return formRequest(path, form, false)
+ },
+ "none": func(path string, form url.Values) *http.Request {
+ form.Set("client_id", "spa")
+
+ return formRequest(path, form, false)
+ },
+ }
+
+ endpoints := []struct {
+ path string
+ form url.Values
+ handler func(*oauth2.Server) http.Handler
+ status int
+ body string
+ }{
+ {"/oauth2/token", url.Values{"grant_type": {"client_credentials"}}, (*oauth2.Server).TokenHandler,
+ http.StatusInternalServerError, `{"error":"server_error","error_description":"internal server error"}`},
+ {"/oauth2/introspect", url.Values{"token": {"ghost"}}, (*oauth2.Server).IntrospectHandler,
+ http.StatusInternalServerError, `{"error":"server_error","error_description":"internal server error"}`},
+ {"/oauth2/revoke", url.Values{"token": {"ghost"}}, (*oauth2.Server).RevokeHandler,
+ http.StatusServiceUnavailable, `{"error":"temporarily_unavailable","error_description":"revocation temporarily unavailable"}`},
+ }
+
+ for _, ep := range endpoints {
+ for method, req := range requests {
+ t.Run(ep.path+"/"+method, func(t *testing.T) {
+ t.Parallel()
+
+ env := failingClientsEnv(t, clientsAnswering(t, errClientStoreDown), nil)
+ form := url.Values{}
+ for k, v := range ep.form {
+ form[k] = v
+ }
+
+ rec := serve(ep.handler(env.srv), req(ep.path, form))
+ require.Equal(t, ep.status, rec.Code, rec.Body.String())
+ assert.JSONEq(t, ep.body, rec.Body.String())
+ requireReportedServerError(t, env, errClientStoreDown)
+ })
+ }
+ }
+}
+
+// TestClientStoreInvalidClientMeansUnknown pins the ClientStore contract:
+// (nil, nil) — or an error carrying invalid_client — reports an unknown
+// client, answered as such.
+func TestClientStoreInvalidClientMeansUnknown(t *testing.T) {
+ t.Parallel()
+
+ env := failingClientsEnv(t, clientsAnswering(t, oauth2.ErrInvalidClient), nil)
+
+ rec := serve(env.authorizeHandler(approve), httptest.NewRequest(http.MethodGet, authorizePath+"?"+boundsQuery().Encode(), nil))
+ assert.Equal(t, http.StatusBadRequest, rec.Code)
+ assert.Equal(t, "oauth2: unknown or invalid client\n", rec.Body.String())
+
+ rec = serve(env.srv.TokenHandler(), boundsPost("/oauth2/token", strings.NewReader("grant_type=client_credentials")))
+ assert.Equal(t, http.StatusUnauthorized, rec.Code)
+ assert.JSONEq(t, `{"error":"invalid_client","error_description":"client authentication failed"}`, rec.Body.String())
+}
+
+// TestRevokeAnswersUnavailableWhenTheResolverFails: at /revoke, an
+// IssuerResolver failure leaves the revocation undone like a store failure:
+// 503 temporarily_unavailable (RFC 7009 §2.2.1), reported once.
+func TestRevokeAnswersUnavailableWhenTheResolverFails(t *testing.T) {
+ t.Parallel()
+
+ env := newBoundsEnv(t, 0, failingIssuer{}, nil)
+
+ rec := serve(env.srv.RevokeHandler(), boundsPost("/oauth2/revoke", strings.NewReader("token=ghost")))
+ require.Equal(t, http.StatusServiceUnavailable, rec.Code, rec.Body.String())
+ assert.Equal(t, []string{oauth2.CodeServerError}, env.sink.codes())
+}
+
+// lookupFailingStore fails the token lookups it is told to.
+type lookupFailingStore struct {
+ *writeCountingStore
+
+ access, refresh error
+}
+
+func (s *lookupFailingStore) LookupAccessToken(ctx context.Context, hash string) (*oauth2.AccessToken, error) {
+ if s.access != nil {
+ return nil, s.access
+ }
+
+ return s.writeCountingStore.LookupAccessToken(ctx, hash) //nolint:wrapcheck // test double
+}
+
+func (s *lookupFailingStore) LookupRefreshToken(ctx context.Context, hash string) (*oauth2.RefreshToken, error) {
+ if s.refresh != nil {
+ return nil, s.refresh
+ }
+
+ return s.writeCountingStore.LookupRefreshToken(ctx, hash) //nolint:wrapcheck // test double
+}
+
+// errTokenStoreDown is the failure of a token store backend.
+var errTokenStoreDown = errors.New("token store: connection refused")
+
+// TestIntrospectionReportsStorageFailures: a token lookup that fails — not
+// one that merely misses — answers {"active":false} (RFC 7662 §2.1: a server
+// unable to determine the state of a token SHOULD answer it is not active)
+// and reaches the hook as a server_error; a missing token, or one of a
+// revoked family, is no failure.
+func TestIntrospectionReportsStorageFailures(t *testing.T) {
+ t.Parallel()
+
+ cases := []struct {
+ name string
+ access, refresh error
+ reported bool
+ }{
+ {"access lookup fails", errTokenStoreDown, nil, true},
+ {"refresh lookup fails", oauth2.ErrInvalidGrant, errTokenStoreDown, true},
+ {"unknown token", nil, nil, false},
+ {"revoked family", oauth2.ErrTokenFamilyRevoked, oauth2.ErrTokenFamilyRevoked, false},
+ }
+
+ for _, tc := range cases {
+ t.Run(tc.name, func(t *testing.T) {
+ t.Parallel()
+
+ env := newBoundsEnv(t, 0, nil, nil)
+ cfg := env.srv.Config()
+ cfg.Storage = &lookupFailingStore{writeCountingStore: env.store, access: tc.access, refresh: tc.refresh}
+
+ srv, err := oauth2.NewServer(cfg)
+ require.NoError(t, err)
+
+ rec := serve(srv.IntrospectHandler(), boundsPost("/oauth2/introspect", strings.NewReader("token=ghost")))
+ require.Equal(t, http.StatusOK, rec.Code)
+ assert.JSONEq(t, `{"active":false}`, rec.Body.String())
+
+ if !tc.reported {
+ assert.Empty(t, env.sink.collected())
+
+ return
+ }
+
+ requireReportedServerError(t, env, errTokenStoreDown)
+ })
+ }
+}
diff --git a/oauth2/storetest/conformance.go b/oauth2/storetest/conformance.go
index a678513f..70c72b21 100644
--- a/oauth2/storetest/conformance.go
+++ b/oauth2/storetest/conformance.go
@@ -58,6 +58,7 @@ func RunConformance(t *testing.T, newStore Factory) {
{"AuthorizationCodeReplayRevokesIssuedTokens", testCodeReplayRevokesIssuedTokens},
{"AccessTokenSaveLookupRevoke", testAccessLifecycle},
{"AccessTokenLookupUnknown", testAccessUnknown},
+ {"TokensAreNeverStoredRaw", testTokensAreNeverStoredRaw},
{"RefreshTokenSaveLookup", testRefreshSaveLookup},
{"RefreshTokenRotation", testRefreshRotation},
{"RefreshTokenReuseRevokesFamily", testRefreshReuse},
@@ -348,6 +349,57 @@ func testAccessUnknown(t *testing.T, s oauth2.Storage) {
mustCode(t, err, oauth2.CodeInvalidGrant, "lookup of an unknown access token")
}
+// testTokensAreNeverStoredRaw checks that a store keeps tokens hashed only
+// ([oauth2.AccessTokenStore]: "MUST store hashes"): no lookup returns the
+// raw value of a token — after SaveTokenPair, after RotateRefreshToken, for
+// the consumed ancestor, for an access token outside any family — and the
+// pairs the caller handed over are left as they were: the server answers
+// the client from them once they are persisted.
+func testTokensAreNeverStoredRaw(t *testing.T, s oauth2.Storage) {
+ first := newPair("raw-0", "fam-raw")
+ mustSave(t, s, first)
+ requireCallerTokens(t, first, "raw-0")
+ requireNoRawToken(t, s, first)
+
+ next := newPair("raw-1", "fam-raw")
+ mustRotate(t, s, first, next)
+ requireCallerTokens(t, next, "raw-1")
+ requireNoRawToken(t, s, next)
+ requireNoRawToken(t, s, first)
+
+ cc := accessOnly("raw-cc", "")
+ mustSave(t, s, cc)
+ mustEqual(t, cc.Access.Token, "raw-at-raw-cc", "the caller's access token")
+ requireNoRawToken(t, s, cc)
+}
+
+// requireCallerTokens asserts that the store left the raw tokens of the
+// pair it was handed — newPair(name, …) — untouched.
+func requireCallerTokens(t *testing.T, pair *oauth2.TokenPair, name string) {
+ t.Helper()
+
+ mustEqual(t, pair.Access.Token, "raw-at-"+name, "the caller's access token")
+ mustEqual(t, pair.Refresh.Token, "raw-rt-"+name, "the caller's refresh token")
+}
+
+// requireNoRawToken asserts that the lookups of pair's tokens — consumed
+// or not — return no raw token value.
+func requireNoRawToken(t *testing.T, s oauth2.Storage, pair *oauth2.TokenPair) {
+ t.Helper()
+
+ at, err := s.LookupAccessToken(ctx(), pair.Access.TokenHash)
+ mustNoError(t, err, "lookup access token "+pair.Access.TokenHash)
+ mustEqual(t, at.Token, "", "lookup of access token "+pair.Access.TokenHash+" returns the raw token")
+
+ if pair.Refresh == nil {
+ return
+ }
+
+ rt, err := s.LookupRefreshToken(ctx(), pair.Refresh.TokenHash)
+ mustNoError(t, err, "lookup refresh token "+pair.Refresh.TokenHash)
+ mustEqual(t, rt.Token, "", "lookup of refresh token "+pair.Refresh.TokenHash+" returns the raw token")
+}
+
// --- refresh tokens ------------------------------------------------------
func testRefreshSaveLookup(t *testing.T, s oauth2.Storage) {
diff --git a/oauth2/storetest/family.go b/oauth2/storetest/family.go
index 5269f8b3..ef96eecb 100644
--- a/oauth2/storetest/family.go
+++ b/oauth2/storetest/family.go
@@ -76,6 +76,7 @@ func requireContext(t *testing.T, s oauth2.Storage, pair *oauth2.TokenPair) {
mustNoError(t, err, "lookup access token "+pair.Access.TokenHash)
mustEqual(t, at.Issuer, pair.Access.Issuer, "access token issuer")
mustEqual(t, at.ExtensionData, pair.Access.ExtensionData, "access token extension data")
+ mustEqual(t, at.Token, "", "the raw access token is never kept")
if pair.Refresh == nil {
return
@@ -85,6 +86,7 @@ func requireContext(t *testing.T, s oauth2.Storage, pair *oauth2.TokenPair) {
mustNoError(t, err, "lookup refresh token "+pair.Refresh.TokenHash)
mustEqual(t, rt.Issuer, pair.Refresh.Issuer, "refresh token issuer")
mustEqual(t, rt.ExtensionData, pair.Refresh.ExtensionData, "refresh token extension data")
+ mustEqual(t, rt.Token, "", "the raw refresh token is never kept")
requireFamilyIssuedAt(t, rt, pair.Refresh.FamilyIssuedAt)
}
diff --git a/oauth2/token_endpoint.go b/oauth2/token_endpoint.go
index 684f02cd..441b94dd 100644
--- a/oauth2/token_endpoint.go
+++ b/oauth2/token_endpoint.go
@@ -32,6 +32,9 @@ import (
// id_token an [Extension] adds (OpenID Connect Core §3.1.3.3) — or §5.2
// (error).
//
+// Its query and body are bounded by [ServerConfig.MaxRequestBytes]
+// (invalid_request beyond), like those of /revoke and /introspect.
+//
// The parameters of RFC 6749 and RFC 7636 the endpoint and the shipped
// grants read — grant_type, code, redirect_uri, code_verifier,
// refresh_token, scope, username, password, client_id, client_secret — are
@@ -55,8 +58,8 @@ func (s *Server) serveToken(w http.ResponseWriter, r *http.Request) {
return
}
- if err := r.ParseForm(); err != nil {
- s.writeOAuthError(r.Context(), w, ErrInvalidRequest.WithCause(err))
+ if err := s.parseRequestForm(w, r); err != nil {
+ s.writeOAuthError(r.Context(), w, formRefusal(err))
return
}
diff --git a/oauth2/trusted_redirect_test.go b/oauth2/trusted_redirect_test.go
new file mode 100644
index 00000000..fa831941
--- /dev/null
+++ b/oauth2/trusted_redirect_test.go
@@ -0,0 +1,298 @@
+// Copyright 2026 Hyperscale. All rights reserved.
+// Use of this source code is governed by a MIT
+// license that can be found in the LICENSE file.
+
+package oauth2_test
+
+import (
+ "context"
+ "errors"
+ "net/http"
+ "net/http/httptest"
+ "net/url"
+ "strings"
+ "sync/atomic"
+ "testing"
+
+ "github.com/hyperscale-stack/security/oauth2"
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+)
+
+// trustHook is a TrustedRedirect answering trusted, counting its calls.
+type trustHook struct {
+ trusted bool
+ calls atomic.Int32
+}
+
+func (h *trustHook) decide(context.Context, oauth2.Client, string) bool {
+ h.calls.Add(1)
+
+ return h.trusted
+}
+
+// trustEnv is a boundsEnv also serving "implicitonly" (confidential, the
+// implicit grant only) and "spa" (public, authorization_code).
+func trustEnv(t *testing.T, resolver oauth2.IssuerResolver, ext oauth2.Extension) *boundsEnv {
+ t.Helper()
+
+ env := newBoundsEnv(t, 0, resolver, ext)
+
+ implicitOnly := confidentialClient([]string{oauth2.GrantTypeImplicit}, []string{"read"})
+ implicitOnly.IDValue = "implicitonly"
+ env.clients.clients["implicitonly"] = implicitOnly
+ env.clients.clients["spa"] = &oauth2.DefaultClient{
+ IDValue: "spa", TypeValue: oauth2.ClientPublic, RedirectURIValues: []string{redirectURI},
+ GrantTypeValues: []string{oauth2.GrantTypeAuthorizationCode}, ScopeValues: []string{"read"},
+ }
+
+ return env
+}
+
+// authorizeTrusting runs GET /authorize?q on env, the implicit flow
+// enabled, trust deciding the redirects of errors before authentication.
+func authorizeTrusting(env *boundsEnv, hook func(context.Context, oauth2.Client, string) bool, consent oauth2.ConsentFunc, q url.Values) *httptest.ResponseRecorder {
+ cfg := oauth2.AuthorizeConfig{AllowImplicit: true, ImplicitTokens: implicitTokens(), TrustedRedirect: hook}
+
+ return serve(env.srv.AuthorizeHandler(cfg, consent), httptest.NewRequest(http.MethodGet, authorizePath+"?"+q.Encode(), nil))
+}
+
+// preAuthenticationFailure is a request failing before the resource owner
+// authenticated, with the code and the status of its bare page.
+type preAuthenticationFailure struct {
+ name string
+ resolver oauth2.IssuerResolver
+ ext oauth2.Extension
+ consent oauth2.ConsentFunc
+ query func() url.Values
+ code string
+ status int
+}
+
+func preAuthenticationFailures() []preAuthenticationFailure {
+ with := func(pairs ...string) func() url.Values {
+ return func() url.Values {
+ q := boundsQuery()
+ for i := 0; i < len(pairs); i += 2 {
+ q.Set(pairs[i], pairs[i+1])
+ }
+
+ return q
+ }
+ }
+
+ return []preAuthenticationFailure{
+ {name: "failing issuer resolver", resolver: failingIssuer{}, query: boundsQuery, code: oauth2.CodeServerError, status: http.StatusInternalServerError},
+ {name: "invalid state", query: with("state", "a\x01b"), code: oauth2.CodeInvalidRequest, status: http.StatusBadRequest},
+ {name: "unsupported response type", query: with("response_type", "magic"), code: oauth2.CodeUnsupportedResponseType, status: http.StatusBadRequest},
+ {name: "repeated parameter", query: func() url.Values {
+ q := boundsQuery()
+ q["scope"] = []string{"read", "write"}
+
+ return q
+ }, code: oauth2.CodeInvalidRequest, status: http.StatusBadRequest},
+ {name: "grant type not declared", query: with("client_id", "implicitonly"), code: oauth2.CodeUnauthorizedClient, status: http.StatusBadRequest},
+ {name: "scope outside the registration", query: with("scope", "admin"), code: oauth2.CodeInvalidScope, status: http.StatusBadRequest},
+ {name: "public client without PKCE", query: with("client_id", "spa"), code: oauth2.CodeInvalidRequest, status: http.StatusBadRequest},
+ {name: "nonce too long", query: with("nonce", strings.Repeat("n", oauth2.MaxNonceLength+1)), code: oauth2.CodeInvalidRequest, status: http.StatusBadRequest},
+ {name: "extension refusal", ext: &fakeExtension{validate: func(context.Context, *http.Request, *oauth2.AuthorizeRequest) (any, error) {
+ return nil, oauth2.ErrInvalidRequest.WithDescription("bad prompt")
+ }}, query: boundsQuery, code: oauth2.CodeInvalidRequest, status: http.StatusBadRequest},
+ // A code the authorization endpoint does not define is answered as
+ // server_error, bare page and redirect alike (RFC 6749 §4.1.2.1).
+ {name: "extension refusal with a code /authorize does not define", ext: &fakeExtension{validate: func(context.Context, *http.Request, *oauth2.AuthorizeRequest) (any, error) {
+ return nil, oauth2.ErrInvalidClient.WithDescription("not for this endpoint")
+ }}, query: boundsQuery, code: oauth2.CodeServerError, status: http.StatusInternalServerError},
+ {name: "extension modifying the request", ext: &fakeExtension{validate: func(_ context.Context, _ *http.Request, ar *oauth2.AuthorizeRequest) (any, error) {
+ ar.Scope = "write"
+
+ return nil, nil
+ }}, query: boundsQuery, code: oauth2.CodeServerError, status: http.StatusInternalServerError},
+ {name: "ConsentFunc failure", consent: consentWith(nil, errors.New("session store down")), query: boundsQuery, code: oauth2.CodeServerError, status: http.StatusInternalServerError},
+ }
+}
+
+// TestUntrustedClientErrorsAreNotRedirected pins decision 12 and RFC 9700
+// §4.11.2 ("The authorization server MUST always authenticate the user
+// first … before redirecting the user"): for a client TrustedRedirect does
+// not trust, an error detected before the resource owner authenticated is a
+// bare page — the status of its code — never a redirect; without the hook,
+// or for a trusted client, it is redirected as RFC 6749 §4.1.2.1 says.
+func TestUntrustedClientErrorsAreNotRedirected(t *testing.T) {
+ t.Parallel()
+
+ for _, tc := range preAuthenticationFailures() {
+ t.Run(tc.name+"/untrusted", func(t *testing.T) {
+ t.Parallel()
+
+ env := trustEnv(t, tc.resolver, tc.ext)
+ hook := &trustHook{}
+
+ consented := false
+ consent := func(w http.ResponseWriter, r *http.Request, ar *oauth2.AuthorizeRequest) (*oauth2.Consent, error) {
+ consented = true
+
+ if tc.consent != nil {
+ return tc.consent(w, r, ar)
+ }
+
+ return &oauth2.Consent{Approved: true, Subject: "alice"}, nil
+ }
+
+ rec := authorizeTrusting(env, hook.decide, consent, tc.query())
+
+ require.Equal(t, tc.status, rec.Code, rec.Body.String())
+ assert.Empty(t, rec.Header().Get("Location"), "never redirected")
+ assert.Equal(t, "oauth2: authorization request refused: "+tc.code+"\n", rec.Body.String())
+ requireProtectiveHeaders(t, rec)
+ assert.Equal(t, int32(1), hook.calls.Load(), "the hook is asked once")
+ assert.Equal(t, []string{tc.code}, env.sink.codes(), "the envelope reaches the hook")
+ assert.Equal(t, tc.consent != nil, consented, "the ConsentFunc runs only for its own failure")
+ assert.Zero(t, env.store.writes.Load(), "nothing stored")
+ })
+
+ for name, hook := range map[string]func(context.Context, oauth2.Client, string) bool{
+ "no hook": nil, "trusted": (&trustHook{trusted: true}).decide,
+ } {
+ t.Run(tc.name+"/"+name, func(t *testing.T) {
+ t.Parallel()
+
+ env := trustEnv(t, tc.resolver, tc.ext)
+ consent := tc.consent
+ if consent == nil {
+ consent = approve
+ }
+
+ rec := authorizeTrusting(env, hook, consent, tc.query())
+ require.Equal(t, http.StatusFound, rec.Code, rec.Body.String())
+ assert.Equal(t, tc.code, redirectParams(t, rec, false).Get("error"))
+ })
+ }
+ }
+}
+
+// TestUntrustedClientInteractionRefusalsAreNotRedirected: a ConsentFunc
+// refusing because the request cannot complete without showing a user
+// interface — the OpenID Connect Core §3.1.2.6 codes prompt=none answers
+// with — has authenticated nobody, so an untrusted client gets the bare page
+// (RFC 9700 §4.11.2, attack 3); a trusted one the redirect.
+func TestUntrustedClientInteractionRefusalsAreNotRedirected(t *testing.T) {
+ t.Parallel()
+
+ for _, refusal := range []*oauth2.Error{
+ oauth2.ErrLoginRequired, oauth2.ErrInteractionRequired, oauth2.ErrAccountSelectionRequired, oauth2.ErrConsentRequired,
+ } {
+ t.Run(refusal.Code, func(t *testing.T) {
+ t.Parallel()
+
+ consent := consentWith(&oauth2.Consent{Refusal: refusal}, nil)
+
+ env := trustEnv(t, nil, nil)
+ rec := authorizeTrusting(env, (&trustHook{}).decide, consent, boundsQuery())
+ require.Equal(t, http.StatusBadRequest, rec.Code, rec.Body.String())
+ assert.Empty(t, rec.Header().Get("Location"))
+ assert.Equal(t, "oauth2: authorization request refused: "+refusal.Code+"\n", rec.Body.String())
+
+ env = trustEnv(t, nil, nil)
+ rec = authorizeTrusting(env, (&trustHook{trusted: true}).decide, consent, boundsQuery())
+ require.Equal(t, http.StatusFound, rec.Code)
+ assert.Equal(t, refusal.Code, redirectParams(t, rec, false).Get("error"))
+ })
+ }
+}
+
+// TestUntrustedClientIsRedirectedOnceAuthenticated: once the ConsentFunc
+// decided — the resource owner authenticated — every answer is redirected,
+// to an untrusted client too: the code, a denial, another refusal, a later
+// failure.
+func TestUntrustedClientIsRedirectedOnceAuthenticated(t *testing.T) {
+ t.Parallel()
+
+ cases := []struct {
+ name string
+ consent oauth2.ConsentFunc
+ want string // the redirected error, "" for a code
+ }{
+ {name: "an approval", consent: approve},
+ {name: "a denial", consent: consentWith(&oauth2.Consent{}, nil), want: oauth2.CodeAccessDenied},
+ {name: "another refusal", consent: consentWith(&oauth2.Consent{Refusal: oauth2.ErrInvalidScope}, nil), want: oauth2.CodeInvalidScope},
+ {name: "an approval the server cannot honor", consent: consentWith(&oauth2.Consent{
+ Approved: true, Subject: strings.Repeat("s", 256),
+ }, nil), want: oauth2.CodeServerError},
+ {name: "a decision on a request the ConsentFunc modified", consent: func(_ http.ResponseWriter, _ *http.Request, ar *oauth2.AuthorizeRequest) (*oauth2.Consent, error) {
+ ar.Scope = "write"
+
+ return &oauth2.Consent{Approved: true, Subject: "alice"}, nil
+ }, want: oauth2.CodeServerError},
+ }
+
+ for _, tc := range cases {
+ t.Run(tc.name, func(t *testing.T) {
+ t.Parallel()
+
+ env := trustEnv(t, nil, nil)
+ hook := &trustHook{}
+ rec := authorizeTrusting(env, hook.decide, tc.consent, boundsQuery())
+ require.Equal(t, http.StatusFound, rec.Code, rec.Body.String())
+
+ params := redirectParams(t, rec, false)
+ assert.Equal(t, tc.want, params.Get("error"))
+
+ if tc.want == "" {
+ assert.NotEmpty(t, params.Get("code"))
+ }
+
+ assert.Zero(t, hook.calls.Load(), "the hook is never asked after authentication")
+ })
+ }
+}
+
+// trustContextKey carries a request-scoped value a middleware sets.
+type trustContextKey struct{}
+
+// TestTrustedRedirectArguments: the hook gets the request context, the
+// client and the redirect URI the request resolved to — a loopback URI with
+// the port the request set — and runs only when a request fails before
+// authentication.
+func TestTrustedRedirectArguments(t *testing.T) {
+ t.Parallel()
+
+ env := trustEnv(t, nil, nil)
+ env.clients.clients["machine"].(*oauth2.DefaultClient).RedirectURIValues = []string{"http://127.0.0.1/cb"}
+
+ var (
+ calls atomic.Int32
+ gotValue any
+ gotClient string
+ gotRedirect string
+ )
+
+ hook := func(ctx context.Context, c oauth2.Client, redirectURI string) bool {
+ calls.Add(1)
+
+ gotValue, gotClient, gotRedirect = ctx.Value(trustContextKey{}), c.ID(), redirectURI
+
+ return true
+ }
+
+ cfg := oauth2.AuthorizeConfig{TrustedRedirect: hook}
+ h := env.srv.AuthorizeHandler(cfg, approve)
+ run := func(q url.Values) *httptest.ResponseRecorder {
+ req := httptest.NewRequest(http.MethodGet, authorizePath+"?"+q.Encode(), nil)
+
+ return serve(h, req.WithContext(context.WithValue(req.Context(), trustContextKey{}, "tenant-a")))
+ }
+
+ q := boundsQuery()
+ q.Set("redirect_uri", "http://127.0.0.1:51004/cb")
+
+ require.Equal(t, http.StatusFound, run(q).Code)
+ assert.Zero(t, calls.Load(), "not asked for a request that succeeds")
+
+ q.Set("scope", "admin")
+ require.Equal(t, http.StatusFound, run(q).Code)
+ assert.Equal(t, int32(1), calls.Load())
+ assert.Equal(t, "tenant-a", gotValue)
+ assert.Equal(t, "machine", gotClient)
+ assert.Equal(t, "http://127.0.0.1:51004/cb", gotRedirect)
+}
diff --git a/oauth2/values_test.go b/oauth2/values_test.go
index fe187d76..337d5bd9 100644
--- a/oauth2/values_test.go
+++ b/oauth2/values_test.go
@@ -13,6 +13,7 @@ import (
"fmt"
"net/http"
"net/http/httptest"
+ "strings"
"testing"
"time"
@@ -211,6 +212,41 @@ func TestDefaultClient(t *testing.T) {
assert.False(t, secretless.SecretMatches("s3cr3t"))
}
+// TestDefaultClientSecretMatches pins what DefaultClient.SecretMatches
+// accepts — the registered secret, byte for byte, nothing else — now that
+// it compares digests, which hide the registered secret's length.
+func TestDefaultClientSecretMatches(t *testing.T) {
+ t.Parallel()
+
+ cases := []struct {
+ name, registered, presented string
+ match bool
+ }{
+ {"equal", "s3cr3t", "s3cr3t", true},
+ {"different, same length", "s3cr3t", "s3cr3T", false},
+ {"prefix", "s3cr3t", "s3cr3", false},
+ {"longer", "s3cr3t", "s3cr3t!", false},
+ {"trailing NUL", "s3cr3t", "s3cr3t\x00", false},
+ {"empty presented", "s3cr3t", "", false},
+ {"empty registered", "", "", false},
+ {"empty registered, any presented", "", "s3cr3t", false},
+ {"long secret", strings.Repeat("k", 1024), strings.Repeat("k", 1024), true},
+ {"long secret, last byte differs", strings.Repeat("k", 1024), strings.Repeat("k", 1023) + "K", false},
+ // Their SHA-256 digests share their first four bytes (b65abac7):
+ // the whole digest is compared.
+ {"digests sharing a prefix", "secret-36533", "secret-103289", false},
+ }
+
+ for _, tc := range cases {
+ t.Run(tc.name, func(t *testing.T) {
+ t.Parallel()
+
+ c := &oauth2.DefaultClient{IDValue: "client", Secret: tc.registered}
+ assert.Equal(t, tc.match, c.SecretMatches(tc.presented))
+ })
+ }
+}
+
func TestStaticIssuer(t *testing.T) {
t.Parallel()
diff --git a/oidc/provider/authorize_test.go b/oidc/provider/authorize_test.go
index 0eb2d17a..1e32acd8 100644
--- a/oidc/provider/authorize_test.go
+++ b/oidc/provider/authorize_test.go
@@ -6,14 +6,12 @@ package oidcprovider_test
import (
"net/http"
- "net/url"
"slices"
"strings"
"testing"
"time"
"github.com/hyperscale-stack/security/oauth2"
- "github.com/hyperscale-stack/security/oauth2/token"
oidcprovider "github.com/hyperscale-stack/security/oidc/provider"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@@ -311,26 +309,6 @@ func TestPlainOAuth2OfflineAccessNeedsTheDecision(t *testing.T) {
})
}
- t.Run("implicit flow", func(t *testing.T) {
- t.Parallel()
-
- e := newEnv(t, withProfile(oauth2.Profile20))
- e.decide(nil, func(d *oidcprovider.Decision) { d.OfflineAccess = oidcprovider.OfflineAccessPrompted })
- cfg := oauth2.AuthorizeConfig{AllowImplicit: true, ImplicitTokens: token.OpaqueRefreshAdapter{Opaque: token.NewOpaque(32)}}
-
- implicit := func(scope string) url.Values {
- t.Helper()
-
- return redirect(t, e.authorizeWith(cfg, with(authorizeQuery(), "response_type", "token", "scope", scope, "nonce", "")))
- }
-
- params := implicit("read offline_access")
- assert.Equal(t, "read", params.Get("scope"), "Core §11: offline_access needs a response type returning a code")
- assert.NotEmpty(t, params.Get("access_token"))
-
- assert.Equal(t, oauth2.CodeAccessDenied, implicit("offline_access").Get("error"))
- })
-
t.Run("another ConsentFunc", func(t *testing.T) {
t.Parallel()
diff --git a/oidc/provider/discovery.go b/oidc/provider/discovery.go
index 2a4cfa0c..c0c26ee8 100644
--- a/oidc/provider/discovery.go
+++ b/oidc/provider/discovery.go
@@ -8,6 +8,7 @@ import (
"encoding/json"
"fmt"
"net/http"
+ "slices"
"strings"
"github.com/hyperscale-stack/security/oauth2"
@@ -34,6 +35,12 @@ var protocolClaims = []string{
// true, authorization_response_iss_parameter_supported true (RFC 9207);
// - offline_access among the scopes only when a refresh_token grant is
// registered; acr_values_supported only when configured;
+// - grant_types_supported sorted; token_endpoint_auth_methods_supported
+// and revocation_endpoint_auth_methods_supported (omitted, it would
+// mean client_secret_basic alone, RFC 8414 §2) the server's ClientAuth
+// methods, introspection_endpoint_auth_methods_supported the same but
+// "none" — /introspect answers confidential clients only — omitted when
+// no method is left;
// - no registration_endpoint, no encryption, request-object, signed
// UserInfo or session-management metadata.
//
@@ -101,6 +108,8 @@ type discoveryDocument struct {
SubjectTypesSupported []string `json:"subject_types_supported"`
IDTokenSigningAlgValuesSupported []string `json:"id_token_signing_alg_values_supported"`
TokenEndpointAuthMethodsSupported []string `json:"token_endpoint_auth_methods_supported"`
+ RevocationEndpointAuthMethodsSupported []string `json:"revocation_endpoint_auth_methods_supported"`
+ IntrospectionEndpointAuthMethodsSupported []string `json:"introspection_endpoint_auth_methods_supported,omitempty"`
CodeChallengeMethodsSupported []string `json:"code_challenge_methods_supported"`
ClaimsSupported []string `json:"claims_supported"`
ACRValuesSupported []string `json:"acr_values_supported,omitempty"`
@@ -114,6 +123,7 @@ func (p *Provider) discoveryDocument(issuer string, snap *keySnapshot) discovery
sc := p.server.Config()
base := strings.TrimSuffix(issuer, "/")
routes := base + sc.RoutePrefix
+ methods := authMethods(sc.ClientAuth)
return discoveryDocument{
Issuer: issuer,
@@ -129,7 +139,9 @@ func (p *Provider) discoveryDocument(issuer string, snap *keySnapshot) discovery
GrantTypesSupported: grantTypes(sc.Grants),
SubjectTypesSupported: []string{"public"},
IDTokenSigningAlgValuesSupported: algorithmNames(snap),
- TokenEndpointAuthMethodsSupported: authMethods(sc.ClientAuth),
+ TokenEndpointAuthMethodsSupported: methods,
+ RevocationEndpointAuthMethodsSupported: methods,
+ IntrospectionEndpointAuthMethodsSupported: confidentialAuthMethods(methods),
CodeChallengeMethodsSupported: pkceMethods(sc.Profile),
ClaimsSupported: p.claimsSupported,
ACRValuesSupported: p.cfg.ACRValuesSupported,
@@ -151,12 +163,16 @@ func (p *Provider) scopesSupported() []string {
return scopes
}
+// grantTypes lists the registered grant types, sorted — the oauth2
+// metadata's order.
func grantTypes(grants []oauth2.Grant) []string {
out := make([]string, 0, len(grants))
for _, g := range grants {
out = appendUnique(out, g.Type())
}
+ slices.Sort(out)
+
return out
}
@@ -169,6 +185,25 @@ func authMethods(auths []oauth2.ClientAuthenticator) []string {
return out
}
+// authMethodNone is the client authentication method of a public client,
+// which authenticates nothing (RFC 7591 §2).
+const authMethodNone = "none"
+
+// confidentialAuthMethods is methods without "none" — /introspect answers
+// confidential clients only — as the oauth2 metadata lists them; empty, the
+// member is omitted.
+func confidentialAuthMethods(methods []string) []string {
+ out := make([]string, 0, len(methods))
+
+ for _, m := range methods {
+ if m != authMethodNone {
+ out = append(out, m)
+ }
+ }
+
+ return out
+}
+
func algorithmNames(snap *keySnapshot) []string {
out := make([]string, 0, len(snap.algorithms))
for _, a := range snap.algorithms {
diff --git a/oidc/provider/discovery_test.go b/oidc/provider/discovery_test.go
index b5060a4b..17dd2416 100644
--- a/oidc/provider/discovery_test.go
+++ b/oidc/provider/discovery_test.go
@@ -13,6 +13,9 @@ import (
jwtsec "github.com/hyperscale-stack/security/jwt"
"github.com/hyperscale-stack/security/oauth2"
+ "github.com/hyperscale-stack/security/oauth2/clientauth"
+ "github.com/hyperscale-stack/security/oauth2/grant"
+ "github.com/hyperscale-stack/security/oauth2/token"
oidcprovider "github.com/hyperscale-stack/security/oidc/provider"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@@ -56,10 +59,12 @@ func TestDiscoveryDocument(t *testing.T) {
"scopes_supported": ["openid", "profile", "email", "address", "phone", "offline_access"],
"response_types_supported": ["code"],
"response_modes_supported": ["query"],
- "grant_types_supported": ["authorization_code", "refresh_token", "client_credentials"],
+ "grant_types_supported": ["authorization_code", "client_credentials", "refresh_token"],
"subject_types_supported": ["public"],
"id_token_signing_alg_values_supported": ["RS256", "ES256"],
"token_endpoint_auth_methods_supported": ["client_secret_basic", "client_secret_post", "none"],
+ "revocation_endpoint_auth_methods_supported": ["client_secret_basic", "client_secret_post", "none"],
+ "introspection_endpoint_auth_methods_supported": ["client_secret_basic", "client_secret_post"],
"code_challenge_methods_supported": ["S256"],
"claims_supported": ["sub", "iss", "aud", "exp", "iat", "auth_time", "nonce", "acr", "amr", "at_hash"],
"claims_parameter_supported": true,
@@ -195,31 +200,64 @@ func TestDiscoveryFailsClosed(t *testing.T) {
})
}
+// withClientAuth replaces the server's client authentication methods.
+func withClientAuth(auths ...oauth2.ClientAuthenticator) envOption {
+ return func(s *oauth2.ServerConfig, _ *grant.Config, _ *oidcprovider.Config) { s.ClientAuth = auths }
+}
+
// TestDiscoveryConsistentWithRFC8414Metadata pins RFC 9068 §4: values
// published through RFC 8414 and OpenID Connect Discovery MUST be
-// consistent.
+// consistent. Every member of the RFC 8414 document is published alike;
+// the authentication methods of /revoke and /introspect are also omitted
+// alike, since their omission has a meaning of its own (RFC 8414 §2).
func TestDiscoveryConsistentWithRFC8414Metadata(t *testing.T) {
t.Parallel()
- for _, prefix := range []string{"", "/connect"} {
- e := newEnv(t, withRoutePrefix(prefix), withResolver(fixedIssuer{issuer: "https://op.example/tenant-a"}))
+ // attemptImplicit tries to enable the implicit flow, which the provider
+ // refuses (UD-1): the documents must stay consistent all the same.
+ attemptImplicit := func(e *env) {
+ defer func() { _ = recover() }()
+
+ e.op.AuthorizeHandler(oauth2.AuthorizeConfig{
+ AllowImplicit: true, ImplicitTokens: token.OpaqueRefreshAdapter{Opaque: token.NewOpaque(32)},
+ })
+ }
+
+ cases := map[string]struct {
+ opts []envOption
+ setup func(*env)
+ }{
+ "default routes": {},
+ "route prefix": {opts: []envOption{withRoutePrefix("/connect")}},
+ "public clients only": {opts: []envOption{withClientAuth(clientauth.NewNone())}},
+ "implicit flow attempted (Profile20)": {
+ opts: []envOption{withProfile(oauth2.Profile20)}, setup: attemptImplicit,
+ },
+ }
+
+ for name, tc := range cases {
+ e := newEnv(t, append(tc.opts, withResolver(fixedIssuer{issuer: "https://op.example/tenant-a"}))...)
+
+ if tc.setup != nil {
+ tc.setup(e)
+ }
oidc := discovery(t, e, get("/tenant-a/.well-known/openid-configuration"))
rec := serve(e.op.Server().MetadataHandler(), get("/.well-known/oauth-authorization-server/tenant-a"))
- require.Equal(t, http.StatusOK, rec.Code)
+ require.Equal(t, http.StatusOK, rec.Code, name)
var rfc8414 map[string]any
require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &rfc8414))
for member, v := range rfc8414 {
- if member == "grant_types_supported" {
- assert.ElementsMatch(t, v, oidc[member], member)
-
- continue
- }
+ assert.Equal(t, v, oidc[member], "%s: %s", name, member)
+ }
- assert.Equal(t, v, oidc[member], member)
+ for _, member := range []string{"revocation_endpoint_auth_methods_supported", "introspection_endpoint_auth_methods_supported"} {
+ _, inOIDC := oidc[member]
+ _, inRFC8414 := rfc8414[member]
+ assert.Equal(t, inRFC8414, inOIDC, "%s: %s published alike", name, member)
}
}
}
diff --git a/oidc/provider/extension_internal_test.go b/oidc/provider/extension_internal_test.go
index a7905bcd..e9de8cd5 100644
--- a/oidc/provider/extension_internal_test.go
+++ b/oidc/provider/extension_internal_test.go
@@ -6,6 +6,8 @@ package oidcprovider
import (
"context"
+ "net/url"
+ "strings"
"testing"
"github.com/hyperscale-stack/security/oauth2"
@@ -32,3 +34,53 @@ func TestTokenResponseRefusesAnotherClientsGrant(t *testing.T) {
require.ErrorIs(t, err, errClientMismatch)
}
}
+
+// TestProtocolGuardsBehindTheCore: the provider re-checks the response type
+// and the nonce of an OpenID request although the core refuses such requests
+// first — no OpenID request gets another response type through (its
+// AuthorizeHandler refuses AllowImplicit, and a handler built from Server()
+// is not mounted for OpenID requests), and the core bounds the nonce of every
+// request — so only a direct call reaches these guards.
+func TestProtocolGuardsBehindTheCore(t *testing.T) {
+ t.Parallel()
+
+ require.NoError(t, checkProtocol(url.Values{}, &oauth2.AuthorizeRequest{ResponseType: responseTypeCode}))
+
+ err := checkProtocol(url.Values{}, &oauth2.AuthorizeRequest{ResponseType: "token"})
+ require.Equal(t, oauth2.CodeUnsupportedResponseType, oauth2.IsCode(err))
+
+ err = checkProtocol(url.Values{}, &oauth2.AuthorizeRequest{ResponseType: responseTypeCode, Nonce: "a\x00b"})
+ require.Equal(t, oauth2.CodeInvalidRequest, oauth2.IsCode(err))
+
+ require.NoError(t, checkNonce(strings.Repeat("n", MaxNonceLength)))
+
+ for _, nonce := range []string{strings.Repeat("n", MaxNonceLength+1), "a\x00b", "\xff"} {
+ require.Equal(t, oauth2.CodeInvalidRequest, oauth2.IsCode(checkNonce(nonce)), "%q", nonce)
+ }
+}
+
+// TestOfflineAccessNeedsTheCodeFlow pins OpenID Connect Core §11: the
+// provider ignores offline_access outside the authorization code flow,
+// whatever the decision. Its AuthorizeHandler serves no implicit flow — the
+// core refuses any other response type first — and a request approved by
+// another ConsentFunc gets no provider rule, so only a direct call reaches
+// it.
+func TestOfflineAccessNeedsTheCodeFlow(t *testing.T) {
+ t.Parallel()
+
+ view := &authRequest{scopes: []string{"read", ScopeOfflineAccess}}
+
+ scope, offline, err := view.offlineScope("read "+ScopeOfflineAccess, OfflineAccessPrompted)
+ require.NoError(t, err)
+ require.Equal(t, "read", scope)
+ require.False(t, offline)
+
+ _, _, err = view.offlineScope(ScopeOfflineAccess, OfflineAccessByPolicy)
+ require.Equal(t, oauth2.CodeAccessDenied, oauth2.IsCode(err))
+
+ view.codeFlow = true
+
+ _, offline, err = view.offlineScope("read "+ScopeOfflineAccess, OfflineAccessByPolicy)
+ require.NoError(t, err)
+ require.True(t, offline, "kept in the code flow")
+}
diff --git a/oidc/provider/interaction.go b/oidc/provider/interaction.go
index ef9e2b57..616a4477 100644
--- a/oidc/provider/interaction.go
+++ b/oidc/provider/interaction.go
@@ -157,9 +157,9 @@ type AuthenticationEvent struct {
// OfflineAccessConsent says why a requested offline_access is granted (Core
// §11, §17.4); its zero value refuses it. It applies to every request the
-// provider serves — offline_access has one meaning at its server — but
-// one that returns no authorization code (the implicit flow), whose
-// offline_access is always ignored (§11).
+// provider serves — offline_access has one meaning at its server. A request
+// returning no authorization code would always have its offline_access
+// ignored (§11); [Provider.AuthorizeHandler] serves none.
type OfflineAccessConsent int
const (
@@ -265,7 +265,19 @@ type authResult struct {
// one served by Server().AuthorizeHandler with another ConsentFunc is
// refused (server_error) before that ConsentFunc runs. cfg configures the
// endpoint as for oauth2.Server.AuthorizeHandler.
+//
+// The provider serves the authorization code flow only — RFC 9700 §2.1.2
+// deprecates the implicit grant, and the discovery document advertises
+// response_type code alone: AuthorizeHandler panics when cfg enables the
+// implicit flow (AllowImplicit), before the server records it, as the
+// server panics on its own misconfigurations. A handler built from
+// Server().AuthorizeHandler is not checked: never enable the implicit flow
+// there either (see [Provider.Server]).
func (p *Provider) AuthorizeHandler(cfg oauth2.AuthorizeConfig) http.Handler {
+ if cfg.AllowImplicit {
+ panic("oidcprovider: AuthorizeHandler: the provider serves no implicit flow (AllowImplicit)")
+ }
+
h := p.server.AuthorizeHandler(cfg, p.consent)
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
diff --git a/oidc/provider/interaction_test.go b/oidc/provider/interaction_test.go
index 94ee558e..bd96c398 100644
--- a/oidc/provider/interaction_test.go
+++ b/oidc/provider/interaction_test.go
@@ -12,6 +12,7 @@ import (
"time"
"github.com/hyperscale-stack/security/oauth2"
+ "github.com/hyperscale-stack/security/oauth2/token"
oidcprovider "github.com/hyperscale-stack/security/oidc/provider"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@@ -273,6 +274,27 @@ func TestForeignConsentFuncNeverRunsForOpenIDRequests(t *testing.T) {
assert.Empty(t, tokens(t, e.exchange(code)).IDToken)
}
+// TestAuthorizeHandlerServesNoImplicitFlow pins UD-1: the provider serves
+// the authorization code flow only — RFC 9700 §2.1.2 deprecates the
+// implicit grant — so AuthorizeHandler refuses a configuration enabling it,
+// as the server refuses its own misconfigurations, before the server
+// records the flow: the RFC 8414 metadata keeps advertising the code flow
+// only, like the discovery document (RFC 9068 §4).
+func TestAuthorizeHandlerServesNoImplicitFlow(t *testing.T) {
+ t.Parallel()
+
+ e := newEnv(t, withProfile(oauth2.Profile20))
+ cfg := oauth2.AuthorizeConfig{AllowImplicit: true, ImplicitTokens: token.OpaqueRefreshAdapter{Opaque: token.NewOpaque(32)}}
+
+ assert.PanicsWithValue(t, "oidcprovider: AuthorizeHandler: the provider serves no implicit flow (AllowImplicit)", func() {
+ e.op.AuthorizeHandler(cfg)
+ })
+
+ rec := serve(e.op.Server().MetadataHandler(), get("/.well-known/oauth-authorization-server"))
+ require.Equal(t, http.StatusOK, rec.Code)
+ assert.Contains(t, rec.Body.String(), `"response_types_supported":["code"]`)
+}
+
// TestRequestCheck pins the freshness, subject and acr rules the provider
// applies to every OpenID approval (Core §3.1.2.1, §3.1.2.2, §3.1.2.3,
// §5.5.1, §5.5.1.1), as the application can apply them beforehand.
diff --git a/oidc/provider/provider.go b/oidc/provider/provider.go
index cff44f27..d596a2ca 100644
--- a/oidc/provider/provider.go
+++ b/oidc/provider/provider.go
@@ -238,6 +238,13 @@ func New(server oauth2.ServerConfig, cfg Config) (*Provider, error) {
// authorization endpoint is served by [Provider.AuthorizeHandler], never by
// Server().AuthorizeHandler — the provider refuses OpenID requests that do
// not come through it.
+//
+// A handler built from Server().AuthorizeHandler — for plain OAuth 2.0
+// requests approved by another ConsentFunc — MUST NOT enable
+// AuthorizeConfig.AllowImplicit: nothing refuses it there, the server would
+// serve the implicit flow to those requests, and its RFC 8414 metadata would
+// advertise a flow the discovery document does not — the two publications
+// would disagree (RFC 9068 §4). RFC 9700 §2.1.2 deprecates the flow.
func (p *Provider) Server() *oauth2.Server { return p.server }
// Paths are the request paths, for one issuer, at which the application
diff --git a/oidc/provider/request_test.go b/oidc/provider/request_test.go
index c96732c5..b3fcd118 100644
--- a/oidc/provider/request_test.go
+++ b/oidc/provider/request_test.go
@@ -12,7 +12,7 @@ import (
"time"
"github.com/hyperscale-stack/security/oauth2"
- "github.com/hyperscale-stack/security/oauth2/token"
+ "github.com/hyperscale-stack/security/oauth2/grant"
oidcprovider "github.com/hyperscale-stack/security/oidc/provider"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@@ -134,19 +134,22 @@ func TestDuplicatedParameters(t *testing.T) {
}
}
-// TestOpenIDImplicitRefused pins OIDC Core §3: OpenID Connect does not use
-// response_type=token, which returns no ID Token.
+// TestOpenIDImplicitRefused pins OIDC Core §3 — OpenID Connect does not use
+// response_type=token, which returns no ID Token — and UD-1: the provider's
+// AuthorizeHandler serves no implicit flow at all
+// (TestAuthorizeHandlerServesNoImplicitFlow), so a plain OAuth 2.0
+// response_type=token is unsupported too, even under Profile20.
func TestOpenIDImplicitRefused(t *testing.T) {
t.Parallel()
e := newEnv(t, withProfile(oauth2.Profile20))
- cfg := oauth2.AuthorizeConfig{AllowImplicit: true, ImplicitTokens: token.OpaqueRefreshAdapter{Opaque: token.NewOpaque(32)}}
- params := redirect(t, e.authorizeWith(cfg, with(authorizeQuery(), "response_type", "token")))
+ params := redirect(t, e.authorize(with(authorizeQuery(), "response_type", "token")))
assert.Equal(t, oauth2.CodeUnsupportedResponseType, params.Get("error"))
- plain := redirect(t, e.authorizeWith(cfg, with(authorizeQuery(), "response_type", "token", "scope", "read", "nonce", "")))
- assert.NotEmpty(t, plain.Get("access_token"), "the OAuth 2.0 implicit flow is unaffected")
+ plain := redirect(t, e.authorize(with(authorizeQuery(), "response_type", "token", "scope", "read", "nonce", "")))
+ assert.Equal(t, oauth2.CodeUnsupportedResponseType, plain.Get("error"), "no OAuth 2.0 implicit flow either")
+ assert.Empty(t, plain.Get("access_token"))
}
// TestRequestObjectParametersRefused pins OIDC Core §3.1.2.6, §6, §7.2.1.
@@ -369,20 +372,27 @@ func TestOversizedListParametersAreIgnored(t *testing.T) {
}
}
-// TestPromptIsReadWhole: prompt has no length bound — truncating or
-// ignoring it could turn "none … login" into a valid prompt=none, or skip a
-// required re-authentication — and is read whole, without allocating per
-// separator.
+// withMaxRequestBytes sets the server's request bound
+// (oauth2.ServerConfig.MaxRequestBytes).
+func withMaxRequestBytes(n int64) envOption {
+ return func(s *oauth2.ServerConfig, _ *grant.Config, _ *oidcprovider.Config) { s.MaxRequestBytes = n }
+}
+
+// TestPromptIsReadWhole: prompt has no length bound of its own — only the
+// request's (oauth2.ServerConfig.MaxRequestBytes, raised here above the
+// value) — since truncating or ignoring it could turn "none … login" into a
+// valid prompt=none, or skip a required re-authentication; and it is read
+// whole, without allocating per separator.
func TestPromptIsReadWhole(t *testing.T) {
t.Parallel()
spaces := strings.Repeat(" ", 1<<20)
- e := newEnv(t)
+ e := newEnv(t, withMaxRequestBytes(4<<20))
require.NotEmpty(t, redirect(t, e.authorize(with(authorizeQuery(), "prompt", spaces+"login"))).Get("code"))
assert.Equal(t, oidcprovider.Prompt{Login: true}, e.lastRequest().Prompt)
- refused := newEnv(t)
+ refused := newEnv(t, withMaxRequestBytes(4<<20))
params := redirect(t, refused.authorize(with(authorizeQuery(), "prompt", "none"+spaces+"login")))
assert.Equal(t, oauth2.CodeInvalidRequest, params.Get("error"), "none with another value, however far")
@@ -427,12 +437,14 @@ func TestRequestReportsOfflineAccessForEveryRequest(t *testing.T) {
// TestPlainOAuth2RequestKeepsOAuth2Semantics: without openid the request
// is an OAuth 2.0 one: OpenID parameters are not parsed, the interaction
-// is always interactive, and the token response carries no id_token.
+// is always interactive, and the token response carries no id_token. (The
+// nonce is a parameter of the oauth2 core, which bounds it in every
+// request.)
func TestPlainOAuth2RequestKeepsOAuth2Semantics(t *testing.T) {
t.Parallel()
e := newEnv(t)
- q := with(authorizeQuery(), "scope", "read", "prompt", "none login", "nonce", strings.Repeat("n", 600), "request", "x")
+ q := with(authorizeQuery(), "scope", "read", "prompt", "none login", "request", "x")
resp := e.flow(q)
assert.Empty(t, resp.IDToken)
diff --git a/oidc/provider/trusted_redirect_test.go b/oidc/provider/trusted_redirect_test.go
new file mode 100644
index 00000000..20e87abf
--- /dev/null
+++ b/oidc/provider/trusted_redirect_test.go
@@ -0,0 +1,46 @@
+// Copyright 2026 Hyperscale. All rights reserved.
+// Use of this source code is governed by a MIT
+// license that can be found in the LICENSE file.
+
+package oidcprovider_test
+
+import (
+ "context"
+ "net/http"
+ "testing"
+
+ "github.com/hyperscale-stack/security/oauth2"
+ oidcprovider "github.com/hyperscale-stack/security/oidc/provider"
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+)
+
+// TestPromptNoneRefusalOfAnUntrustedClientIsNotRedirected pins RFC 9700
+// §4.11.2, attack 3, through the provider: a silent authentication request
+// (prompt=none) the application refuses with login_required authenticated
+// nobody, so it is answered with a bare page when
+// oauth2.AuthorizeConfig.TrustedRedirect does not trust the client, and
+// redirected when it does.
+func TestPromptNoneRefusalOfAnUntrustedClientIsNotRedirected(t *testing.T) {
+ t.Parallel()
+
+ for _, trusted := range []bool{false, true} {
+ e := newEnv(t)
+ e.interaction.silent = func(*http.Request, *oidcprovider.Request) (*oidcprovider.Decision, error) {
+ return &oidcprovider.Decision{Refusal: oauth2.ErrLoginRequired}, nil
+ }
+
+ cfg := oauth2.AuthorizeConfig{TrustedRedirect: func(context.Context, oauth2.Client, string) bool { return trusted }}
+ rec := e.authorizeWith(cfg, with(authorizeQuery(), "prompt", "none"))
+
+ if trusted {
+ assert.Equal(t, oauth2.CodeLoginRequired, redirect(t, rec).Get("error"))
+
+ continue
+ }
+
+ require.Equal(t, http.StatusBadRequest, rec.Code, rec.Body.String())
+ assert.Empty(t, rec.Header().Get("Location"), "never redirected")
+ assert.Equal(t, "oauth2: authorization request refused: login_required\n", rec.Body.String())
+ }
+}