fix(consent): widen CSP form-action to the client redirect_uri

config/config.go

@@ -513,7 +513,7 @@ func Load() (*Config, error) {
 			if o == "" {
 				continue
 			}
-			canon, err := canonicalCSPHostSource(o)
+			canon, err := CanonicalCSPHostSource(o)
 			if err != nil {
 				return nil, fmt.Errorf("CSP_FORM_ACTION_EXTRA: %w", err)
 			}
@@ -715,7 +715,7 @@ var cspHostSourceHostPort = regexp.MustCompile(
 	`^(?:\[[0-9A-Fa-f:.]+\]|[A-Za-z0-9-]+(?:\.[A-Za-z0-9-]+)*)(?::[0-9]+)?$`,
 )
 
-// canonicalCSPHostSource validates an operator-supplied CSP host-source
+// CanonicalCSPHostSource validates an operator-supplied CSP host-source
 // (scheme://host[:port]) and returns the lower-cased canonical form
 // suitable for emission into the consent page's CSP form-action
 // directive. Returns an error with the offending input quoted on any
@@ -736,7 +736,7 @@ var cspHostSourceHostPort = regexp.MustCompile(
 //   - host containing any character outside the CSP host-char set
 //     (catches "https://host.com;foo", "https://host_name.com", and
 //     similar header-injection-shaped paste errors)
-func canonicalCSPHostSource(in string) (string, error) {
+func CanonicalCSPHostSource(in string) (string, error) {
 	if in == "*" || strings.Contains(in, "*") {
 		return "", fmt.Errorf("%q wildcard sources are not supported; supply explicit origins", in)
 	}

docs/configuration.md

@@ -92,7 +92,7 @@ control.
 | `PKCE_REQUIRED` | `true` | Set `false` for legacy clients that omit PKCE (Cursor, MCP Inspector, ChatGPT). Rejected by `PROD_MODE`. |
 | `COMPAT_ALLOW_STATELESS` | `false` | Synthesize a server-side `state` on `/authorize` when the client omits it. Strict mode refuses the request; counter `mcp_auth_access_denied_total{reason="state_missing"}` fires either way. Rejected by `PROD_MODE`. |
 | `RENDER_CONSENT_PAGE` | `true` | Render an explicit proxy-side consent page on `/authorize` so the user sees who's asking and where they'll be redirected before the IdP login. Closes the silent-token-issuance path where a malicious DCR client + an active IdP session = tokens issued without any user interaction. Plain HTML, no JavaScript. Set `false` to fall back to the legacy silent-redirect — only when every caller is non-interactive and known-trusted. |
-| `CSP_FORM_ACTION_EXTRA` | (empty) | Comma-separated additional `scheme://host[:port]` origins appended to the consent page's CSP `form-action` source list, alongside `'self'` and the discovered OIDC authorize endpoint. Needed when the IdP redirect chain crosses the authorize host: Entra B2C (`tenant.b2clogin.com`), personal Microsoft accounts (`login.live.com`), federated AD FS (customer host), sovereign clouds (`login.microsoftonline.us` / `login.partner.microsoftonline.cn`). Each entry is validated at startup against the CSP3 §2.4 host-source ABNF (stricter than RFC 3986 reg-name — only `ALPHA`/`DIGIT`/`-`/`.` in hostname labels, or `[IPv6]`); paths, queries, fragments, userinfo, wildcards, and sub-delim host characters (`;`, `,`, `&`, `_`, …) are rejected loud so a misconfigured allowlist cannot silently weaken the emitted header. Scheme and host are ASCII-lower-cased on the way in (per CSP3 §6.7.2.5) so the emitted header is greppable in the form an operator typed. Empty for the common one-host-IdP case. |
+| `CSP_FORM_ACTION_EXTRA` | (empty) | Comma-separated additional `scheme://host[:port]` origins appended to the consent page's CSP `form-action` source list, alongside `'self'`, the discovered OIDC authorize endpoint, and the current request's validated `redirect_uri` origin (added per render so already-authenticated upstream sessions whose redirect chain stays in one navigation — `POST /consent` → IdP authorize → `/callback` → client `redirect_uri` — are not blocked by Chromium's form-action enforcement at the final hop; the redirect_uri origin is itself filtered through the same CSP3 host-source check below, with a `consent_csp_redirect_uri_skipped` warn when a registered client's host fails the check). Needed when the IdP redirect chain crosses the authorize host: Entra B2C (`tenant.b2clogin.com`), personal Microsoft accounts (`login.live.com`), federated AD FS (customer host), sovereign clouds (`login.microsoftonline.us` / `login.partner.microsoftonline.cn`). Each entry is validated at startup against the CSP3 §2.4 host-source ABNF (stricter than RFC 3986 reg-name — only `ALPHA`/`DIGIT`/`-`/`.` in hostname labels, or `[IPv6]`); paths, queries, fragments, userinfo, wildcards, and sub-delim host characters (`;`, `,`, `&`, `_`, …) are rejected loud so a misconfigured allowlist cannot silently weaken the emitted header. Scheme and host are ASCII-lower-cased on the way in (per CSP3 §6.7.2.5) so the emitted header is greppable in the form an operator typed. Empty for the common one-host-IdP case. |
 | `OIDC_ALLOW_INSECURE_HTTP` | `false` | Dev-only escape hatch for cleartext `http://` OIDC issuers (Docker Compose Keycloak demo). Rejected when `PROD_MODE=true`. |
 
 ## Logging and observability

docs/threat-model.md

@@ -45,9 +45,14 @@ rather than assuming they're already covered.
   the IdP operator's own threat model.
 - **Browser-side XSS in the consent page.** The page is JS-free and
   CSP-locked (`default-src 'none'`, `style-src 'unsafe-inline'`,
-  `script-src` defaults to none, `frame-ancestors 'none'`). A
-  browser-engine bug that escapes contextual HTML escaping is not
-  separately mitigated.
+  `script-src` defaults to none, `frame-ancestors 'none'`,
+  `base-uri 'none'`). `form-action` is widened only to the upstream
+  IdP origin (from OIDC discovery), any `CSP_FORM_ACTION_EXTRA`
+  entries, and the current request's validated `redirect_uri` origin
+  — each of the three filtered through the CSP3 §2.4 host-source
+  ABNF check so a DCR-registered redirect_uri whose host smuggles a
+  sub-delim cannot break out of the directive. A browser-engine bug
+  that escapes contextual HTML escaping is not separately mitigated.
 - **Network-level MITM between proxy and IdP.** TLS verification is
   on by default in the `oauth2` library; an operator who disables
   it (or a CA compromise) lets a MITM observe the upstream code

handlers/authorize.go

@@ -78,22 +78,24 @@ type AuthorizeConfig struct {
 // front-loaded above the response_type / resource / PKCE / state
 // checks.
 func Authorize(tm *token.Manager, logger *zap.Logger, baseURL string, oauth2Cfg *oauth2.Config, authzCfg AuthorizeConfig) http.HandlerFunc {
-	// Precompute the consent page's CSP once at startup. The IdP
-	// origin in form-action must match the upstream AuthURL so the
-	// consent POST's 302 to the IdP is not blocked by Chromium's
-	// form-action redirect-chain enforcement. Extra operator-supplied
-	// origins (CSP_FORM_ACTION_EXTRA) cover IdP topologies whose
-	// redirect chain leaves the authorize host (Entra B2C, federated
-	// AD FS, personal MS accounts, sovereign clouds).
+	// Precompute the static form-action source list once at startup.
+	// The IdP origin in form-action must match the upstream AuthURL
+	// so the consent POST's 302 to the IdP is not blocked by
+	// Chromium's form-action redirect-chain enforcement. Extra
+	// operator-supplied origins (CSP_FORM_ACTION_EXTRA) cover IdP
+	// topologies whose redirect chain leaves the authorize host
+	// (Entra B2C, federated AD FS, personal MS accounts, sovereign
+	// clouds). The client's redirect_uri origin is appended per
+	// render — see formatConsentCSP.
 	//
 	// Skipped entirely when RenderConsentPage is false: the silent
-	// fork bypasses renderConsent, the precomputed CSP is unused, and
-	// a "consent CSP misconfigured" warn on a deployment that doesn't
-	// render the consent page would be misleading noise.
-	var consentCSP string
+	// fork bypasses renderConsent, the precomputed sources are
+	// unused, and a "consent CSP misconfigured" warn on a deployment
+	// that doesn't render the consent page would be misleading noise.
+	var consentCSPSources []string
 	if authzCfg.RenderConsentPage {
 		var idpOriginOK bool
-		consentCSP, idpOriginOK = buildConsentCSP(oauth2Cfg.Endpoint.AuthURL, authzCfg.CSPFormActionExtra)
+		consentCSPSources, idpOriginOK = buildConsentCSPSources(oauth2Cfg.Endpoint.AuthURL, authzCfg.CSPFormActionExtra)
 		if !idpOriginOK {
 			logger.Warn("consent_csp_idp_origin_missing",
 				zap.String("auth_url", oauth2Cfg.Endpoint.AuthURL),
@@ -264,7 +266,7 @@ func Authorize(tm *token.Manager, logger *zap.Logger, baseURL string, oauth2Cfg
 		// redirect) replays from POST /consent on approval.
 		if authzCfg.RenderConsentPage {
 			metrics.AuthorizeInitiated.WithLabelValues("consent").Inc()
-			renderConsent(w, r, tm, logger, baseURL, authzCfg.ResourceName, consentCSP, sealedConsent{
+			renderConsent(w, r, tm, logger, baseURL, authzCfg.ResourceName, consentCSPSources, sealedConsent{
 				// Per-render JTI: a fresh id every GET /authorize so
 				// back-button = re-consent (each render gets its own
 				// single-use claim slot) rather than dead-state errors.

handlers/consent.go

@@ -7,9 +7,11 @@ import (
 	"html/template"
 	"net/http"
 	"net/url"
+	"slices"
 	"strings"
 	"time"
 
+	"github.com/babs/mcp-auth-proxy/config"
 	"github.com/babs/mcp-auth-proxy/metrics"
 	"github.com/babs/mcp-auth-proxy/replay"
 	"github.com/babs/mcp-auth-proxy/token"
@@ -119,10 +121,12 @@ var consentTmpl = template.Must(template.New("consent").Parse(`<!doctype html>
 </html>
 `))
 
-// buildConsentCSP returns the Content-Security-Policy header value
-// for the consent page, with form-action widened to include the
-// upstream IdP origin derived from authURL and any operator-supplied
-// extra origins.
+// buildConsentCSPSources returns the static form-action source list
+// for the consent page CSP — 'self', the upstream IdP origin (when
+// derivable from authURL) and any operator-supplied extras. The
+// client's redirect_uri origin is NOT included here; it is appended
+// per-render in formatConsentCSP because each consent page is bound
+// to one validated redirect_uri.
 //
 // form-action is enforced against every URL in the redirect chain
 // initiated by a form submit on Blink-based browsers (Chrome, Edge,
@@ -134,9 +138,9 @@ var consentTmpl = template.Must(template.New("consent").Parse(`<!doctype html>
 //
 // authURL is the upstream OIDC authorization endpoint (set from
 // discovery at startup). On parse failure / empty value the
-// returned CSP falls back to 'self' only — the consent page will
-// be Chrome-broken but the proxy still serves; a startup log emits
-// the warning so deployments do not silently regress.
+// returned source list falls back to 'self' only — the consent page
+// will be Chrome-broken but the proxy still serves; a startup log
+// emits the warning so deployments do not silently regress.
 //
 // extraOrigins are additional scheme://host[:port] entries appended
 // verbatim. Validated at config-load time (config.Load) so they are
@@ -145,8 +149,8 @@ var consentTmpl = template.Must(template.New("consent").Parse(`<!doctype html>
 // personal MS accounts → login.live.com, federated AD FS → customer
 // host, sovereign clouds → cloud-specific login domains. Each extra
 // is a single fixed origin, not a wildcard.
-func buildConsentCSP(authURL string, extraOrigins []string) (csp string, idpOriginOK bool) {
-	sources := make([]string, 0, 2+len(extraOrigins))
+func buildConsentCSPSources(authURL string, extraOrigins []string) (sources []string, idpOriginOK bool) {
+	sources = make([]string, 0, 3+len(extraOrigins))
 	sources = append(sources, "'self'")
 	if authURL != "" {
 		if u, err := url.Parse(authURL); err == nil && u.Scheme != "" && u.Host != "" {
@@ -160,7 +164,70 @@ func buildConsentCSP(authURL string, extraOrigins []string) (csp string, idpOrig
 		}
 	}
 	sources = append(sources, extraOrigins...)
-	return "default-src 'none'; style-src 'unsafe-inline'; form-action " + strings.Join(sources, " ") + "; frame-ancestors 'none'; base-uri 'none'", idpOriginOK
+	return sources, idpOriginOK
+}
+
+// formatConsentCSP joins the precomputed source list with the
+// per-render redirect_uri origin and returns the full
+// Content-Security-Policy header value.
+//
+// The redirect_uri origin is added because Chromium's form-action
+// check covers the *entire* redirect chain a form submit triggers.
+// When the user already has a live session with the upstream IdP,
+// the chain doesn't terminate at the IdP login page (which would
+// end form-action enforcement at a 200 HTML response) — it stays in
+// one navigation: POST /consent → IdP authorize 302 → proxy
+// /callback 302 → client redirect_uri 302. The final hop crosses
+// to the client's origin (e.g. https://claude.ai), and without it
+// in form-action Chrome blocks the navigation at that step. The
+// error surfaces in DevTools as "Sending form data to /consent
+// violates form-action" — misleading; the form's action URL is
+// just what Chrome names in the message, the actual block is the
+// downstream hop.
+//
+// redirectURI has already been validated against the registered
+// sealedClient.RedirectURIs at /authorize. Skipped when empty,
+// when the parsed origin matches an entry already in baseSources
+// (same-host loopback / proxy-hosted clients), or when the origin
+// would not pass the CSP3 host-source ABNF check.
+//
+// The host-source check matters because RFC 3986 reg-name allows
+// sub-delim characters (`;`, `,`, `&`, `=`) that, while accepted by
+// Go's url.Parse, would terminate the form-action directive early
+// when emitted into the header — same header-injection foot-gun the
+// startup validator on CSP_FORM_ACTION_EXTRA closes for operator
+// input. A malicious DCR client cannot weaken its own consent
+// page's CSP by registering `https://evil.example;injected/cb`.
+// When the redirect_uri host fails the check, the consent page
+// renders with the unwidened CSP and a warn fires carrying the
+// offending redirect_uri AND the client_id so operators can
+// alert/group by client without a cross-grep against
+// client_registered. In Chromium this surfaces as the original
+// form-action block on the final redirect hop, but no header
+// smuggling occurs.
+func formatConsentCSP(logger *zap.Logger, baseSources []string, clientID, redirectURI string) string {
+	sources := baseSources
+	if redirectURI != "" {
+		if u, err := url.Parse(redirectURI); err == nil && u.Scheme != "" && u.Host != "" {
+			origin := strings.ToLower(u.Scheme) + "://" + strings.ToLower(u.Host)
+			// Skip when already covered by 'self' / IdP origin /
+			// operator extras (proxy-hosted clients, same-IdP-tenant
+			// redirects). The dedup keeps the emitted header tight
+			// and matches the case-folded canonical form above.
+			if !slices.Contains(baseSources, origin) {
+				if _, err := config.CanonicalCSPHostSource(origin); err != nil {
+					logger.Warn("consent_csp_redirect_uri_skipped",
+						zap.String("client_id", clientID),
+						zap.String("redirect_uri", redirectURI),
+						zap.Error(err),
+					)
+				} else {
+					sources = slices.Concat(baseSources, []string{origin})
+				}
+			}
+		}
+	}
+	return "default-src 'none'; style-src 'unsafe-inline'; form-action " + strings.Join(sources, " ") + "; frame-ancestors 'none'; base-uri 'none'"
 }
 
 // renderConsent seals the validated /authorize parameters into a
@@ -173,7 +240,7 @@ func buildConsentCSP(authURL string, extraOrigins []string) (csp string, idpOrig
 // the registered redirect_uri (server_error, RFC 6749 §4.1.2.1)
 // rather than rendering a partial page — the client is already
 // trusted at this point in the flow.
-func renderConsent(w http.ResponseWriter, r *http.Request, tm *token.Manager, logger *zap.Logger, baseURL, resourceName, csp string, consent sealedConsent) {
+func renderConsent(w http.ResponseWriter, r *http.Request, tm *token.Manager, logger *zap.Logger, baseURL, resourceName string, cspSources []string, consent sealedConsent) {
 	consentToken, err := tm.SealJSON(consent, token.PurposeConsent)
 	if err != nil {
 		logger.Error("consent_seal_failed", zap.Error(err))
@@ -205,10 +272,13 @@ func renderConsent(w http.ResponseWriter, r *http.Request, tm *token.Manager, lo
 	// style-src for this response only; script-src stays default
 	// (none) so the page remains JavaScript-free, and frame-ancestors
 	// stays none so the consent UI cannot be framed by an attacker
-	// origin. form-action also names the upstream IdP origin so the
-	// approve POST's 302 to the IdP is not blocked by Chromium's
-	// redirect-chain enforcement (see buildConsentCSP).
-	w.Header().Set("Content-Security-Policy", csp)
+	// origin. form-action names the upstream IdP origin AND the
+	// client's redirect_uri origin so the approve POST's redirect
+	// chain (POST /consent → IdP authorize → /callback → client
+	// redirect_uri, when the upstream session is already live) is
+	// not blocked by Chromium's form-action enforcement at the final
+	// hop. See formatConsentCSP.
+	w.Header().Set("Content-Security-Policy", formatConsentCSP(logger, cspSources, consent.ClientID, consent.RedirectURI))
 	w.WriteHeader(http.StatusOK)
 	if err := consentTmpl.Execute(w, data); err != nil {
 		// Body already started — log only.