Skip to content

feat(oauth): Client ID Metadata Documents, consent screen, and opt-in DCR - #758

Merged
lakhansamani merged 17 commits into
mainfrom
feat/cimd-and-consent
Aug 13, 2026
Merged

feat(oauth): Client ID Metadata Documents, consent screen, and opt-in DCR#758
lakhansamani merged 17 commits into
mainfrom
feat/cimd-and-consent

Conversation

@lakhansamani

@lakhansamani lakhansamani commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Makes the OAuth path to /mcp reachable. Follow-up to #757.

Why

Verified against Claude Code 2.1.226, the OAuth path did not work at all:

authorizer-local: http://localhost:8099/mcp (HTTP)
  - ✘ Failed to connect — Incompatible auth server: does not support dynamic client registration

Everything #757 built — PKCE, discovery, audience binding, loopback redirects — was unreachable by the flagship client. With this branch:

authorizer-cimd: http://localhost:8099/mcp (HTTP) - ! Needs authentication

and the server log shows the client walking the whole chain: POST /mcp/.well-known/oauth-protected-resource/mcp/.well-known/oauth-authorization-server (twice).

That is the blocker removed, not the full flow proven. Completing OAuth needs a browser for login and consent, which a headless check cannot drive. See the testing note below — I am deliberately not claiming more than was observed.

Update: CIMD and opt-in DCR

The section below argued for CIMD over DCR. That reasoning still holds for what
should be preferred — it was wrong only about what shipping clients can do.
Claude Code reads client_id_metadata_document_supported: true from our metadata
and still refuses, because its released version predates CIMD. Without a
registration_endpoint those clients cannot connect at all.

So this branch now also implements RFC 7591, off by default, behind
--enable-dynamic-client-registration. This does not demote CIMD: the MCP client
priority order is pre-registered → CIMD → DCR, so a CIMD-capable client never
reaches the DCR path.

Verified against Claude Code 2.1.226 on a real server:

GET  /.well-known/oauth-protected-resource/mcp   200
GET  /.well-known/oauth-authorization-server     200
POST /oauth/register                             201

It registered as a public client — token_endpoint_auth_method: none,
redirect_uris: http://localhost:3118/callback,
grant_types: authorization_code,refresh_token. The step that previously failed
now succeeds. The interactive browser leg of OAuth still has to be driven by a
human; it is not claimed here.

Self-registered clients are held to the rules the specs make mandatory for public
clients, and the same consent screen CIMD clients get:

Guard Source
S256 PKCE required at /authorize RFC 9700 §2.1.1, OAuth 2.1 §4.1.1
Implicit response types refused OAuth 2.1 removes the implicit grant
Consent screen, name shown as unverified RFC 7591 §5
redirect_uris https or loopback http only MCP authorization spec
Registry ceiling + per-IP rate limit RFC 7591 §5
Off by default matches Keycloak, Ory Hydra, Auth0

Two latent issues were fixed on the way: /authorize treated a client-lookup
storage error as "no such client" and fell through to the laxer AllowedOrigins
check, and the redirectURIClientStore test stub disagreed with all six real
providers on the one method documented to return (nil, nil).

CIMD, not DCR (original reasoning, retained)

The MCP spec (2025-11-25) demoted dynamic client registration: authorization servers SHOULD support Client ID Metadata Documents and MAY support DCR, kept only "for backwards compatibility with earlier versions". Auth0 ships DCR Enterprise-only behind ACLs and recommends CIMD instead; Anthropic steers directory traffic away from it because DCR registers a fresh client per connection — unbounded row growth in every self-hosted deployment.

CIMD needs no endpoint, no rows and no schema change: the client_id is an HTTPS URL the server fetches and validates.

The consent screen is mandatory here

A CIMD client asserts its own identity — it picks its client_name, and anyone who can host a JSON file can claim any name. So /authorize shows a consent page before issuing a code, leading with the redirect host, the only fact the server has actually verified. Loopback-only clients get an explicit warning, because any local process can bind the same port and present the same document.

Pre-registered clients are unaffected and keep silent approval — an operator already vouched for them.

Two properties make the split flow safe:

  • Only the consent_id crosses the page. Scope, redirect URI and the original query come back from the store, so a tampered form cannot widen scope or swap the client.
  • Approval replays the original query verbatim, so the request that executes is byte-identical to the one shown.

Plus single-use, session-pinned, and refusal redirects with access_denied per RFC 6749 §4.1.2.1.

Security notes for review

  • The URL is attacker-supplied and the server fetches it, so the whole idea rests on not being an SSRF primitive. validators.SafeHTTPClient (one-shot DNS, dial pinned to the validated IP) is what makes this small rather than dangerous.
  • CIMD clients are refused client_credentials outright and a presented secret is rejected rather than ignored — a grant with no user and no secret must not be reachable by a client that registered itself by hosting a JSON file.
  • Resolution failure at /authorize is fatal, not a fall-through to the AllowedOrigins fallback; falling through would give an unresolvable client a laxer check than a resolved one.
  • Everything else bounds attacker-controlled input: response size cap, timeout, cache TTL clamped both ways (Cache-Control is written by the party being validated), bounded cache.

Off by default — it changes the authorization endpoint's trust model for every client, not just MCP.

On testing — one thing I did not do

There is no e2e-playground test for the browser leg, and the reason is recorded in the package doc rather than left as an apparent oversight. CIMD requires an https client_id and the server fetches it, so the document needs TLS from a host the server trusts; the compose network is http-only with private addresses.

I built the mock host and the spec, hit this, and reverted both rather than either relax the https check under --env=e2e (which would put an environment-dependent branch inside a security check) or bolt a private CA into the compose stack (defensible, but its own change). The e2e-gated allowPrivate fetch that attempt needed is reverted too — it added a third caller to SafeHTTPClientAllowPrivate, whose doc comment asks for review before that, and now buys nothing.

Covered instead: resolver, validation, SSRF guard and caching (unit, 40 subtests); consent rules and the /authorize gate (integration); and the real-client check above. The gap is specifically "a human clicks Allow in a browser".

One finding worth repeating: the integration harness initially did not construct the provider, so /authorize silently skipped the CIMD branch and issued a code via the AllowedOrigins fallback — exactly the downgrade the test was written to catch. It only surfaced because the assertion was specific rather than "expect an error".

Verification

go build · go vet · make test (43 packages) · make lint (0 issues) · make smoke · real Claude Code client

Docs: authorizerdev/docs (companion branch feat/cimd-and-consent).

First half of making the OAuth path to /mcp reachable. Verified against Claude
Code 2.1.226, that path does not work today: the client refuses with
"Incompatible auth server: does not support dynamic client registration" and
offers no manual-client-id fallback, so everything #757 built — PKCE, discovery,
audience binding, loopback redirects — is unreachable by it.

CIMD rather than RFC 7591 DCR, because the MCP spec (2025-11-25) demoted DCR to
"backwards compatibility with earlier versions" and made CIMD the SHOULD; Auth0
ships DCR Enterprise-only behind ACLs and recommends CIMD instead; and DCR mints
a client row per connection, which on a self-hosted product is unbounded growth
in every operator's deployment. CIMD adds no endpoint, no rows and no schema
change: the client_id IS an HTTPS URL this package fetches and validates.

The security of the whole idea rests on not being an SSRF primitive — the URL is
attacker-supplied and this server fetches it. validators.SafeHTTPClient already
solves exactly that (one-shot DNS, dial pinned to the validated IP, so rebinding
cannot occur between check and connect), which is why this lands as a small
package rather than a risky one.

Everything else is a bound on attacker-controlled input: response size cap,
request timeout, cache TTL clamped in both directions (the Cache-Control header
is written by the party being validated, so max-age=0 is a DoS amplifier and
max-age=1yr pins a document that may later be revoked), and a bounded cache.

Note on the SSRF test: its first version passed even with the guard swapped for
SafeHTTPClientAllowPrivate, because the request then failed TLS verification
against httptest's self-signed certificate instead — a different failure that
would NOT occur against a real attacker-controlled host with a valid cert. It
now asserts the specific rejection, and was verified to fail when the guard is
weakened.

Not yet wired into /authorize — that needs the consent screen, which the spec
makes mandatory once client identity is self-asserted. Next commit.

Refs authorizer-docs specs/2026-08-11-cimd-and-consent.md
Second half of CIMD. The spec makes consent mandatory here, and the reason is
specific rather than ceremonial: a Client ID Metadata Document client asserts
its OWN identity. It picks its client_name, and anyone who can host a JSON file
can claim any name. The only fact this server has verified about it is the
redirect host — which is what the page leads with, and what a person can
actually judge.

Pre-registered clients keep today's silent approval. An operator entered their
redirect URIs by hand, so the trust decision was already made; changing that
would be an unrelated behaviour break.

Two things carry the security of the split flow:

- Only the consent_id travels through the page. Scope, redirect_uri, client and
  the original query are read back from the store, so a tampered form cannot
  widen scope, redirect elsewhere or swap the client between render and submit.
- Approval REPLAYS the original /authorize query rather than rebuilding one from
  form fields, so the request that executes is byte-identical to the one the
  user was shown.

Plus: single-use (removed before the decision is acted on, so a replayed
approval cannot mint a second code), pinned to the session that was shown the
page (otherwise a page rendered for one user could be submitted in another's
browser), and a denial redirects to the client with access_denied per RFC 6749
§4.1.2.1 rather than rendering here, because the client is waiting on that
callback.

Loopback-only clients get an explicit warning. Any local process can bind the
same port and present the legitimate client's document; the spec says so, and
says it cannot be solved server-side — so the page says it too.

Not yet wired into /authorize — that is the next commit, along with advertising
client_id_metadata_document_supported and the two config flags.
Completes CIMD. `--enable-client-id-metadata-document` (off by default) makes an
HTTPS-URL client_id resolvable, which is what lets a client with no prior
relationship to this server authenticate at all.

Three hook points, each with a reason it is where it is:

- /authorize redirect validation resolves the document and checks redirect_uri
  against ITS list, using the same matcher registered clients use so RFC 8252
  loopback rules apply identically. A resolution failure is fatal rather than a
  fall-through to the AllowedOrigins fallback — falling through would give an
  unresolvable client a LAXER check than a resolved one, which is backwards.
- The consent gate sits after the session is validated and the user resolved:
  the page names the account being granted, and an unauthenticated visitor must
  reach the login UI first rather than approve something on behalf of nobody.
- The client-auth resolver returns a synthetic public client for CIMD ids, and
  refuses two things explicitly rather than leaving them to downstream
  assumptions: client_credentials (a grant with no user and no secret must not
  be reachable by a client that registered itself by hosting a JSON file), and a
  presented secret (accepting it would tell a caller their credential worked
  when nothing verified it). PKCE still gates the code exchange.

client_id_metadata_document_supported is advertised only when the flag is on;
advertising it while off would make a client select CIMD and then fail.

Verified against Claude Code 2.1.226, which is what this feature exists for.
Before: "Failed to connect — Incompatible auth server: does not support dynamic
client registration". After: "Needs authentication", and the server log shows
the client walking the full chain — POST /mcp, the protected-resource document,
then the authorization-server metadata twice.

That is the blocker removed, NOT the full flow proven: completing OAuth needs a
browser for login and consent, which a headless check cannot drive. The browser
leg still needs a Playwright case for CIMD; I am not claiming more than was
observed.

A wiring gap surfaced while testing and is worth recording: the integration
harness did not construct the provider, so /authorize skipped the CIMD branch
and issued a code via the AllowedOrigins fallback — exactly the silent downgrade
the test was written to catch. The harness now builds it as cmd/root.go does.
Changelog entries for --enable-client-id-metadata-document and the consent
screen.

Also records, in the package doc, that the CIMD browser flow has no
e2e-playground test and why — because the reason is structural and a future
reader should not assume it was forgotten.

The spec requires a CIMD client_id to be an https URL, and the SERVER fetches
it, so the document must be served over TLS from a host the server can reach
with a certificate it trusts. The compose network is http-only with private
addresses, so a mock client host is rejected before CIMD engages.

Two ways to make a test pass anyway were rejected:

- Relaxing the https requirement under --env=e2e. That puts an
  environment-dependent branch inside a security check, so the thing under test
  stops being the thing that runs in production.
- Adding a private CA to the compose stack and serving the mock over TLS.
  Defensible, but it is cert generation plus trust injection into the server
  image — its own change, not something to smuggle into this one.

I built both the mock host and the spec before concluding this, and reverted
them rather than ship a weakened check or a skipped test. The e2e-gated
allowPrivate fetch that attempt required is reverted too: it added a third
caller to SafeHTTPClientAllowPrivate, whose doc comment asks for careful review
before that, and it now buys nothing.

What remains covered: the resolver, validation, SSRF guard and caching (unit);
the consent rules and the /authorize gate (integration); and a real Claude Code
client, which now engages with the server instead of refusing it. The gap is
specifically "a human clicks Allow in a browser".

Verified: go build, go vet, make test (43 packages), make lint, make smoke.
Two real bugs, both found only by driving the flow in a browser. Every Go test
called the handler directly and so saw neither.

1. CSRF blocked the consent form POST with 403. The middleware requires
   `Content-Type: application/json` or `X-Requested-With`, and a plain HTML form
   can send neither.

   /authorize/consent is now exempt, and this is not a weakening: the form's
   consent_id is a random UUID that exists only in a page this server rendered
   for one session, is single-use, and the handler separately verifies the
   submitting session is the one it was issued to. That is a stronger
   synchronizer token than the generic check. The alternative — driving the form
   with JavaScript — would make consent, the one screen a user must be able to
   read and trust, fail silently with JS disabled.

2. Approval resumed the authorization in-process by rewriting
   Request.URL.RawQuery and calling AuthorizeHandler. That does not work: gin
   caches parsed query parameters on the Context at first access, so the handler
   read the POST's empty query and failed with "response_type is required".

   Approval now records a single-use grant keyed to (user, client) and redirects
   to /authorize with the original query. That is also the more honest shape —
   it is what any other authorization server does, and it re-enters /authorize
   through the front door with every middleware applied.

Also adds the e2e TLS fixture this required: a one-shot CA generator, an HTTPS
CIMD mock host, and SSL_CERT_FILE trust injection into the server container. The
bundle APPENDS our CA to the public roots rather than replacing them, since Go's
SSL_CERT_FILE overrides the system pool wholesale.

The server-side flow was observed completing end to end in the container logs:
login redirect, consent page rendered, grant recorded, then a redirect to the
callback carrying ?code=. The three browser assertions are marked fixme with
that stated plainly — the fixture is correct and done, but the final leg is not
reproducibly observable from the browser side yet, and I would rather leave an
accurate marker than a test that appears to prove something it does not.

Verified: go build, go vet, make test (43 packages), make lint.
The fixture CIMD needs in order to be testable at all: the spec requires an
https client_id and the SERVER fetches it, so the document host must present a
certificate the server trusts.

- mocks/tls-certs: one-shot CA + leaf generator into a shared volume. The
  bundle APPENDS our CA to the public roots rather than replacing them, because
  Go's SSL_CERT_FILE overrides the system pool wholesale and shipping only our
  CA would silently break every other TLS dial the server makes. SAN not CN,
  since Go ignores CN entirely.
- mocks/cimd-client: serves a metadata document over HTTPS, plus a mismatched
  variant for the impersonation case and a callback that echoes its query.
- compose: SSL_CERT_FILE trust injection, --enable-client-id-metadata-document,
  and the mock's origin in --allowed-origins so the login UI accepts it.

Instrumenting this found two production bugs, already fixed in the preceding
commit: CSRF rejected the consent form POST, and resuming the authorization
in-process failed because gin caches parsed query parameters.

Three browser assertions are marked fixme with a precise account of what is
established and what is not — see the note at the top of the spec. Nothing here
is claimed to pass that does not.

Note: e2e-playground/ is listed in this clone's .git/info/exclude, so these
needed `git add -f`. Anyone adding e2e files on a similarly-configured clone
will hit the same silent drop.
Closes the gap the previous commit left open, by using the right tool rather
than fighting the wrong one.

TestCIMDConsentEndToEnd drives the whole flow over real HTTP with a cookie jar
— /authorize, consent page, POST the decision, resumed /authorize — and asserts
the redirect the client actually receives:

- approving issues a code to the redirect_uri from the client's OWN document
- declining returns access_denied with no code, to the client rather than to a
  page only the user sees (RFC 6749 §4.1.2.1)
- a document whose client_id does not match its URL never reaches consent

Verified by reverting the decision check: the declining case fails.

Why not a browser: the consent page is plain HTML with no JavaScript, so a
browser adds nothing to an assertion about status codes and Location headers.
And the browser attempt failed for a reason unrelated to the feature — the flow
ends in a cross-origin redirect into an https host whose certificate is signed
by a throwaway CA, and Chromium cancels that navigation (net::ERR_ABORTED,
canceled=true, no response event) even with ignoreHTTPSErrors AND
--ignore-certificate-errors.

That was measured rather than assumed, via CDP Network events: a DIRECT
navigation to the same host returns 200, so it is the redirect INTO it that
Chromium refuses. Reading the Location in Go sidesteps the question entirely.

Playwright keeps what only a browser can show — that the page renders, names
the client, presents the redirect host as its own field, and offers both
decisions. Two tests, both passing, none skipped.

CIMD needs an https document host and the SSRF guard refuses loopback, so the
resolver gains SetHTTPClientForTest: unreachable from New, so no production
path can call it, and the same seam fetchViaClient already established.

Verified: go build, go vet, make test (43 packages), make lint, and the
e2e-playground spec.
…as CIMD

Two findings from my own pre-merge pass, both in code this branch added.

1. prompt=none was violated. OIDC Core §3.1.2.1 says the authorization server
   "MUST NOT display any authentication or consent user interface pages", and
   the two existing prompt=none guards only cover the UNAUTHENTICATED case. A
   caller with a perfectly valid session and a CIMD client_id was therefore
   shown a consent page in response to a request that forbids one.

   A self-asserted client genuinely requires consent, so the two demands cannot
   both be met: the request now fails to the client with consent_required, and
   the client decides whether to retry interactively. Verified by reverting the
   guard — the new test fails.

2. The deployment's own reserved client could be shadowed. --client-id is
   free-form, so an operator who set it to something parsing as a document URL
   would have their primary client stop resolving from the registry and start
   resolving by FETCHING that URL — a silent change of identity source, with
   whoever controls the URL then describing the deployment's own client.

   No attacker path exists (the admin API has no client_id field; it is
   server-generated), which is why this is a cheap exclusion rather than an
   argument about likelihood. IsMetadataClientIDFor makes the precedence
   explicit instead of accidental.

Verified: go build, go vet, make test (43 packages), make lint.
…cally

Pre-merge review findings, all in code this branch added. The first three are
real defects in the consent flow.

1. A grant authorized the wrong request. The marker was keyed on (user, client)
   only, so one that was never redeemed — tab closed, browser back, network drop
   — sat in the store for its whole TTL and then satisfied ANY later /authorize
   for that pair: a wider scope, a different redirect_uri from the document's
   list, a different PKCE challenge. The user approves one request and a
   materially different one executes.

   That defeats the design this branch states elsewhere: pendingConsent stores
   the full parameter set precisely so the request cannot change between render
   and submit, and the grant key threw it away. The key now includes a SHA-256
   of the encoded parameters (hashed, not raw — the key lands in a shared store
   and a redirect_uri or login_hint in a key is needless exposure).

2. POST /authorize could never complete. /authorize is registered for POST as
   well as GET, and the handler reads parameters via FormValue, but the pending
   record stored only URL.RawQuery — empty for a POST. Approval then replayed a
   parameterless request and dropped the user on "response_type is required"
   AFTER they had approved, while the client waited on a callback that never
   came. Captured from the parsed form instead.

3. Single-use was not single-use under concurrency. Both the pending record and
   the grant were read then deleted, so two concurrent submissions could both
   pass. This repo already ships GetAndRemoveState for exactly this, and its own
   doc comment says returning state "on the strength of the read alone would
   hand the same code to every racer, which is an authorization-code replay".
   Both sites now use it.

Also fixed:

- The deny path ignored response_mode, delivering a query-string 302 to clients
  that asked for fragment/form_post/web_message. It now uses redirectErrorToRP,
  the helper every other /authorize error path uses.
- Document redirect_uris accepted fragments and userinfo, so a document could
  register one and the consent page would display a host that is not where the
  code lands. Rejected at resolution, matching redirectURIMatches.
- The consent page is served no-store: it carries a single-use id and names the
  signed-in user, so a cached copy shows one user's email to the next person.
- cimd_consent_test.go's comments claimed end-to-end coverage it did not have.
  Corrected to describe the one property it actually asserts.

Each fix has a test verified to fail without it — including the grant binding,
where reverting the key to its unbound form fails the new case.

Verified: go build, go vet, make test (43 packages), make lint, make smoke.
Shipping MCP clients still look for `registration_endpoint` and give up
when it is absent, even where `client_id_metadata_document_supported` is
advertised — Claude Code refuses with "Incompatible auth server: does
not support dynamic client registration". CIMD stays the preferred path;
DCR is the fallback for clients that cannot use it yet.

Off by default: it is an unauthenticated write endpoint, matching how
Keycloak, Ory Hydra and Auth0 all ship it. Advertising and mounting are
driven by the same flag, so discovery cannot promise what is not routed.
The MCP client priority order (pre-registered -> CIMD -> DCR) means
enabling this cannot downgrade a CIMD-capable client.

Self-registered clients are public-only, get the consent screen that
RFC 7591 section 5 asks for, and are held to the PKCE rules the specs
make mandatory for public clients:

- S256 required at /authorize (RFC 9700 2.1.1, OAuth 2.1 4.1.1),
  rejected before the user is asked to log in and approve rather than at
  the token endpoint with an error naming client_secret
- implicit response types refused; both registration paths declare
  response_types ["code"]
- redirect_uris limited to https or loopback http, no fragments
- registry ceiling on top of the existing per-IP rate limiter

Also fixes two latent issues found on the way:

- /authorize treated a client-lookup storage error as "no such client"
  and fell through to the laxer AllowedOrigins check, which would also
  have let a self-asserted client skip consent during a database blip
- the redirectURIClientStore test stub returned an error for an absent
  row, disagreeing with all six real providers on the one method
  documented to return (nil, nil)

Refs #758
Fold the comma check into validateRegistrationRedirectURI, drop a
single-use error type in favour of errors.New, and stop parsing
client_uri/logo_uri/scope — unrendered self-asserted metadata is a
phishing surface, not a feature.
Claude Code registers grant_types [authorization_code, refresh_token],
so this path runs on every long-lived connection. Asserts the refresh
works with client_id and no secret (RFC 6749 section 6) and that the
rotated access token keeps its RFC 8707 audience.
@lakhansamani lakhansamani changed the title feat(cimd): Client ID Metadata Documents and the consent screen feat(oauth): Client ID Metadata Documents, consent screen, and opt-in DCR Aug 12, 2026
/authorize checks a presented redirect_uri against the client's
registered URIs, then hands that same URI to /app to render the login
page. /app checked it against AllowedOrigins alone, so the two halves of
one flow applied different rules: any client whose registered redirect
was not also a globally allowed origin passed the first check and got
"invalid redirect url" from the second, before the user could type
anything.

Every test missed it because every fixture allow-listed its own callback
origin - cimd.spec.ts via --allowed-origins, the Go tests via a wildcard.
Production cannot do that: an MCP client binds an EPHEMERAL loopback
port, so there is no origin to allow-list in advance. Claude Code used
port 3118 on one run and a different one on the next.

Both handlers now share checkClientRedirectURI, so the rule cannot drift
again. Precedence is unchanged from /authorize: no client_id means the
allow-list, a CIMD client is matched against its document, a registry row
with registered URIs is held to an exact match with no fallback, a row
without them falls back, and a storage error fails closed rather than
silently downgrading to the laxer check.

Found by e2e-playground/tests/dcr.spec.ts, added here: six cases over the
real middleware chain, including the registration POST with no Origin
that no Go test can exercise. TestAppHandlerHonoursRegisteredRedirectURIs
is the equivalent tripwire for `make test`, so a regression is caught
without Docker.

Refs #758
make smoke booted the real binary but authenticated everything with a
token minted directly by signup, so /authorize, the code store, the PKCE
comparison and /oauth/token could all have broken without a single
failure. SCIM was untested end to end entirely, and it is the one surface
authenticated by a per-org bearer token rather than a session or the
admin secret.

Adds two subtests against the booted binary:

- authorization code + PKCE: /authorize -> /oauth/token -> /userinfo,
  including single-use enforcement on the code (RFC 6749 4.1.2)
- SCIM: unauthenticated call refused, then provision and read back a user

The OAuth case runs on its OWN user and cookie jar. /authorize rolls the
session over on success, which invalidates the token minted at signup -
sharing the user silently broke the MCP stdio subtest that still held it.

Also corrects docs that said DCR was unimplemented: the CHANGELOG entry
for the removed registration_endpoint, the roadmap items, and the
compliance test's assertion message, which now pins the real property
(absent while the feature is disabled) rather than "until RFC 7591 is
implemented".

Refs #758
Clicking "Allow access" appeared to do nothing, and clicking again gave
"this consent request has expired or was already used".

The consent page is served with the default `form-action 'self'`.
Browsers enforce form-action across the ENTIRE redirect chain of a form
submission, not just its immediate target. Approving POSTs to
/authorize/consent -> 302 /authorize -> 302 the client's redirect_uri,
so the last hop is blocked: the navigation aborts silently, the user is
left on the consent page assuming the click missed, and the second click
fails because the first already consumed the single-use consent.

The server was doing everything right - the log shows consent 302 and
the authorize 302 carrying the code - so nothing server-side looked
broken. Reproduced in a real browser, confirmed by the Location header
being correct while the browser refused to follow it, and confirmed
fixed by the same browser landing on the callback after one click.

This is the third instance of this rule in this codebase:
setFormPostCSP already relaxes form-action for OIDC form_post, and
samlIDPSSOCSP omits it because "form-action 'self' would silently break
every SAML IdP login". The consent page was missed.

Scoped to the single origin being approved rather than form_post's
`form-action *`: the redirect_uri is already validated against the
client's registered list, so the exact destination is known here.

No test caught this because every automated check either drove the
redirects itself or asserted the Location header instead of letting a
browser follow it. The regression guard asserts the CSP names the
redirect origin.

Refs #758
The consent screen looked nothing like the hosted login UI a user
reaches it from - unstyled system fonts, no logo, no organization name.
On the one screen whose entire job is helping someone decide whether to
trust a client, looking like a different site is the wrong signal.

It now uses the app's own design tokens (web/app/src/index.css): the
same brand header with --organization-logo and --organization-name, card,
type scale, and primary/secondary buttons. Tokens are copied rather than
imported because that file is bundled by Vite and is not servable to a
standalone template; au_shell.tmpl holds the shared chrome so the consent
page and its error page cannot drift apart.

Consent failures were raw JSON. A user who clicked "Allow access" twice
got {"error":"invalid_request","error_description":"this consent request
has expired or was already used"} with no idea what to do - reported by a
real user. They are now pages in the same shell, saying what happened,
that nothing was shared, and to reconnect from the application. The OAuth
error code stays in logs and metrics, where it is actionable.

Also widens img-src to `https:` on the consent CSP, matching the default
policy. --organization-logo is normally hosted elsewhere, so the tighter
`'self' data:` rendered the operator's own branding as a broken image -
caught by looking at the page rather than by any assertion.

Refs #758
None of these are exploitable as shipped; all three are values a client
chooses reaching somewhere they should be checked first.

- consent CSP: url.Parse rejects spaces and control characters in a host
  but ACCEPTS ";", which separates CSP directives. A client registering
  https://evil.com;x/cb would have terminated form-action and opened a
  new one. Not escalatable - a directive needs a space before its values,
  and form-action is the last directive here - but the host is now
  checked against an allow-list of authority characters before it is
  embedded, and dropped if it fails.

- client_name was truncated by BYTES. Slicing 200 bytes can split a
  multi-byte rune and leave invalid UTF-8, which SQLite stores happily
  and Postgres rejects outright: a registration that works on one backend
  and 500s on another, from a name an anonymous caller chose. Truncated
  on rune boundaries now.

- grant_types were validated trimmed but stored untrimmed, so
  " authorization_code" passed and persisted with the space. Nothing
  reads the stored value today, which is exactly why it would have been
  found the hard way later.

Refs #758
@lakhansamani
lakhansamani merged commit 9fe382b into main Aug 13, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant