fix(consent): allow IdP origin in CSP form-action for Chromium

config/config.go

@@ -181,6 +181,20 @@ type Config struct {
 	// env: UPSTREAM_AUTHORIZATION_HEADER. Treat as a secret in
 	// deployment (mount from a Secret, not a ConfigMap).
 	UpstreamAuthorization string
+	// CSPFormActionExtra lists additional scheme://host[:port]
+	// origins appended to the consent page's CSP form-action source
+	// list, alongside 'self' and the discovered upstream authorize
+	// endpoint origin. Needed for IdP redirect chains that cross the
+	// authorize host: Entra B2C (*.b2clogin.com), personal Microsoft
+	// accounts (login.live.com), federated AD FS (customer-owned
+	// host), sovereign clouds (login.microsoftonline.us /
+	// .partner.microsoftonline.cn). Each entry is a single fixed
+	// origin — wildcards are intentionally not supported because the
+	// directive's role is an explicit allowlist defense-in-depth
+	// against HTML injection on the consent page. Empty for the
+	// common one-host-IdP case. env: CSP_FORM_ACTION_EXTRA,
+	// comma-separated.
+	CSPFormActionExtra []string
 	// secretWeakWarning is non-empty when TOKEN_SIGNING_SECRET matches
 	// an obvious-weakness pattern (all-same byte, or short repeating
 	// period). Exposed via
@@ -493,6 +507,20 @@ func Load() (*Config, error) {
 
 	c.UpstreamAuthorization = os.Getenv("UPSTREAM_AUTHORIZATION_HEADER")
 
+	if raw := os.Getenv("CSP_FORM_ACTION_EXTRA"); raw != "" {
+		for _, o := range strings.Split(raw, ",") {
+			o = strings.TrimSpace(o)
+			if o == "" {
+				continue
+			}
+			canon, err := canonicalCSPHostSource(o)
+			if err != nil {
+				return nil, fmt.Errorf("CSP_FORM_ACTION_EXTRA: %w", err)
+			}
+			c.CSPFormActionExtra = append(c.CSPFormActionExtra, canon)
+		}
+	}
+
 	c.PerSubjectConcurrency = 16
 	if v := os.Getenv("MCP_PER_SUBJECT_CONCURRENCY"); v != "" {
 		n, err := strconv.ParseInt(v, 10, 64)
@@ -666,6 +694,65 @@ func validateOIDCIssuerURL(raw string, allowInsecureHTTP bool) error {
 	}
 }
 
+// cspHostSourceHostPort matches the CSP3 host-source grammar's
+// host-part + optional port-part, minus wildcards (we reject those
+// upstream). See https://w3c.github.io/webappsec-csp/#grammardef-host-source
+//
+//	host-part = 1*host-char *( "." 1*host-char ) / IP-literal
+//	host-char = ALPHA / DIGIT / "-"
+//	IP-literal = "[" ( IPv6 / IPvFuture ) "]"   (from RFC 3986)
+//	port-part = ":" 1*DIGIT
+//
+// Intentionally stricter than RFC 3986 reg-name (which accepts
+// sub-delims like ";", ",", "&"): an operator-supplied origin that
+// round-trips through url.Parse may still contain CSP-meaningful
+// characters that break out of the directive when emitted into the
+// header ("https://host.com;foo" terminates form-action and starts
+// a bogus directive). Mirroring the CSP grammar verbatim closes
+// that header-injection foot-gun, which is operator-foot-gun rather
+// than RCE but still warrants fail-loud over fail-silent.
+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
+// (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
+// validation failure.
+//
+// Canonicalisation: scheme and host are ASCII-lower-cased per CSP3
+// §6.7.2.5 (host-source matching is case-insensitive); operators
+// who paste `HTTPS://Foo.example.com` get `https://foo.example.com`
+// in the emitted header so a grep returns the form they expect.
+//
+// Rejection classes (paste-error shapes that should fail startup
+// rather than silently weaken the deployment's CSP):
+//   - wildcards (`*` anywhere — the directive's role is an explicit
+//     allowlist; smuggling wildcards in defeats it under a
+//     misleading name)
+//   - missing scheme (`host.com` with a `/` in it but no `://`)
+//   - any path / query / fragment / userinfo component
+//   - 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) {
+	if in == "*" || strings.Contains(in, "*") {
+		return "", fmt.Errorf("%q wildcard sources are not supported; supply explicit origins", in)
+	}
+	if strings.Contains(in, "/") && !strings.Contains(in, "://") {
+		return "", fmt.Errorf("%q is not a valid origin (want scheme://host[:port])", in)
+	}
+	u, err := url.Parse(in)
+	if err != nil || u.Scheme == "" || u.Host == "" || u.Path != "" || u.RawQuery != "" || u.Fragment != "" || u.User != nil {
+		return "", fmt.Errorf("%q is not a valid origin (want scheme://host[:port], no path/query/fragment/userinfo)", in)
+	}
+	if !cspHostSourceHostPort.MatchString(u.Host) {
+		return "", fmt.Errorf("%q host component contains characters not allowed in a CSP host-source (must be ALPHA/DIGIT/'-'/'.' or [IPv6])", in)
+	}
+	return strings.ToLower(u.Scheme) + "://" + strings.ToLower(u.Host), nil
+}
+
 // validateUpstreamMCPURL enforces the shape the reverse-proxy relies
 // on: absolute URL with a real authority, http(s) scheme, no
 // userinfo/fragment/query, no opaque form, and a non-empty path

config/config_test.go

@@ -206,6 +206,182 @@ func TestLoad_AllowedGroups_Empty(t *testing.T) {
 	}
 }
 
+// TestLoad_CSPFormActionExtra pins the validation contract for the
+// operator-supplied form-action allowlist: accepts scheme://host[:port]
+// entries (with whitespace tolerance and empty-entry skip), rejects
+// every common paste-error shape (path/query/fragment, userinfo,
+// missing scheme, bare wildcard, embedded wildcard). A misconfigured
+// allowlist MUST fail startup rather than silently regress Chromium
+// consent at first user click.
+func TestLoad_CSPFormActionExtra(t *testing.T) {
+	t.Run("unset is nil", func(t *testing.T) {
+		setAllRequired(t)
+		cfg, err := Load()
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+		if cfg.CSPFormActionExtra != nil {
+			t.Errorf("CSPFormActionExtra = %v, want nil", cfg.CSPFormActionExtra)
+		}
+	})
+
+	t.Run("parses multiple with whitespace and empty skip", func(t *testing.T) {
+		setAllRequired(t)
+		t.Setenv("CSP_FORM_ACTION_EXTRA", " https://a.example , https://b.example:8443 , , https://c.example ")
+		cfg, err := Load()
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+		want := []string{"https://a.example", "https://b.example:8443", "https://c.example"}
+		if len(cfg.CSPFormActionExtra) != len(want) {
+			t.Fatalf("CSPFormActionExtra = %v, want %v", cfg.CSPFormActionExtra, want)
+		}
+		for i, w := range want {
+			if cfg.CSPFormActionExtra[i] != w {
+				t.Errorf("CSPFormActionExtra[%d] = %q, want %q", i, cfg.CSPFormActionExtra[i], w)
+			}
+		}
+	})
+
+	t.Run("commas-only collapses to nil", func(t *testing.T) {
+		// Operators paste-stripping the env value down to just
+		// commas (and incidentally whitespace) should land in the
+		// same place as unset — not a startup error, just no extras.
+		setAllRequired(t)
+		t.Setenv("CSP_FORM_ACTION_EXTRA", " , , , ")
+		cfg, err := Load()
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+		if cfg.CSPFormActionExtra != nil {
+			t.Errorf("CSPFormActionExtra = %v, want nil", cfg.CSPFormActionExtra)
+		}
+	})
+
+	t.Run("accepts explicit default port", func(t *testing.T) {
+		// CSP source matching is default-port-aware per CSP3 §6.7.2,
+		// but operators sometimes paste the explicit port. The
+		// parser must accept it round-trip without trying to be
+		// clever (stripping :443 would be normalisation we don't owe).
+		setAllRequired(t)
+		t.Setenv("CSP_FORM_ACTION_EXTRA", "https://idp.example.com:443")
+		cfg, err := Load()
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+		if len(cfg.CSPFormActionExtra) != 1 || cfg.CSPFormActionExtra[0] != "https://idp.example.com:443" {
+			t.Errorf("CSPFormActionExtra = %v, want [https://idp.example.com:443]", cfg.CSPFormActionExtra)
+		}
+	})
+
+	t.Run("accepts IPv6 bracketed host", func(t *testing.T) {
+		setAllRequired(t)
+		t.Setenv("CSP_FORM_ACTION_EXTRA", "https://[2001:db8::1]:8443")
+		cfg, err := Load()
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+		if len(cfg.CSPFormActionExtra) != 1 || cfg.CSPFormActionExtra[0] != "https://[2001:db8::1]:8443" {
+			t.Errorf("CSPFormActionExtra = %v, want [https://[2001:db8::1]:8443]", cfg.CSPFormActionExtra)
+		}
+	})
+
+	t.Run("accepts http scheme", func(t *testing.T) {
+		// http:// is accepted at parse time — the gating for
+		// insecure schemes is OIDC_ALLOW_INSECURE_HTTP on the issuer
+		// URL, not a second gate here. An operator allowlisting an
+		// http origin in form-action is making an intentional dev
+		// choice.
+		setAllRequired(t)
+		t.Setenv("CSP_FORM_ACTION_EXTRA", "http://keycloak.dev.local:8080")
+		cfg, err := Load()
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+		if len(cfg.CSPFormActionExtra) != 1 || cfg.CSPFormActionExtra[0] != "http://keycloak.dev.local:8080" {
+			t.Errorf("CSPFormActionExtra = %v, want [http://keycloak.dev.local:8080]", cfg.CSPFormActionExtra)
+		}
+	})
+
+	t.Run("preserves order and accepts many entries", func(t *testing.T) {
+		// Order matters for the CSP header readability (and grep
+		// when debugging) — a future micro-optimisation that
+		// sorts or dedups entries would break the operator's
+		// expectation that "what I configured is what I see".
+		setAllRequired(t)
+		t.Setenv("CSP_FORM_ACTION_EXTRA",
+			"https://e.example,https://d.example,https://c.example,https://b.example,https://a.example")
+		cfg, err := Load()
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+		want := []string{
+			"https://e.example",
+			"https://d.example",
+			"https://c.example",
+			"https://b.example",
+			"https://a.example",
+		}
+		if len(cfg.CSPFormActionExtra) != len(want) {
+			t.Fatalf("CSPFormActionExtra len = %d, want %d (got %v)", len(cfg.CSPFormActionExtra), len(want), cfg.CSPFormActionExtra)
+		}
+		for i, w := range want {
+			if cfg.CSPFormActionExtra[i] != w {
+				t.Errorf("CSPFormActionExtra[%d] = %q, want %q", i, cfg.CSPFormActionExtra[i], w)
+			}
+		}
+	})
+
+	t.Run("canonicalises case to lowercase", func(t *testing.T) {
+		// CSP3 §6.7.2.5 makes host-source matching case-insensitive;
+		// we lower-case on the way in so the emitted header is the
+		// form an operator would grep for.
+		setAllRequired(t)
+		t.Setenv("CSP_FORM_ACTION_EXTRA", "HTTPS://Foo.Example.COM:8443")
+		cfg, err := Load()
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+		const want = "https://foo.example.com:8443"
+		if len(cfg.CSPFormActionExtra) != 1 || cfg.CSPFormActionExtra[0] != want {
+			t.Errorf("CSPFormActionExtra = %v, want [%s]", cfg.CSPFormActionExtra, want)
+		}
+	})
+
+	for _, bad := range []struct {
+		name string
+		val  string
+	}{
+		{"path component", "https://idp.example.com/path"},
+		{"trailing slash", "https://idp.example.com/"},
+		{"query string", "https://idp.example.com?x=1"},
+		{"fragment", "https://idp.example.com#frag"},
+		{"userinfo", "https://user@idp.example.com"},
+		{"missing scheme", "idp.example.com"},
+		{"bare wildcard", "*"},
+		{"embedded wildcard", "https://*.example.com"},
+		{"empty scheme only", "https://"},
+		// CSP3 §2.4 host-char is ALPHA/DIGIT/'-' only — stricter
+		// than RFC 3986 reg-name. The remaining sub-delims would
+		// terminate the form-action directive when emitted, so the
+		// parser rejects them rather than letting a misconfigured
+		// allowlist silently weaken the consent page's CSP.
+		{"semicolon in host", "https://idp.example.com;injected"},
+		{"comma in host", "https://idp.example.com,injected"},
+		{"ampersand in host", "https://idp.example.com&injected"},
+		{"underscore in host", "https://host_name.example.com"},
+		{"space in value", "https://idp.example.com /path"},
+	} {
+		t.Run("rejects "+bad.name, func(t *testing.T) {
+			setAllRequired(t)
+			t.Setenv("CSP_FORM_ACTION_EXTRA", bad.val)
+			if _, err := Load(); err == nil {
+				t.Errorf("Load() unexpectedly succeeded for %q", bad.val)
+			}
+		})
+	}
+}
+
 func TestLoad_RevokeBefore(t *testing.T) {
 	setAllRequired(t)
 	t.Setenv("REVOKE_BEFORE", "2026-03-28T12:00:00Z")

docs/configuration.md

@@ -92,6 +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. |
 | `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

handlers/authorize.go

@@ -55,6 +55,14 @@ type AuthorizeConfig struct {
 	// name via MCP_RESOURCE_NAME. Falls back to CanonicalResource
 	// when empty.
 	ResourceName string
+	// CSPFormActionExtra is an operator-supplied list of additional
+	// scheme://host[:port] origins appended to the consent page's
+	// form-action source list (alongside 'self' and the discovered
+	// upstream authorize endpoint origin). Needed for IdP redirect
+	// chains that cross the authorize host (Entra B2C, federated
+	// AD FS, personal MS accounts, sovereign clouds). Validated at
+	// config.Load time. Empty for the common case.
+	CSPFormActionExtra []string
 }
 
 // Authorize handles GET /authorize (OAuth 2.1 PKCE authorization request).
@@ -70,6 +78,29 @@ 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).
+	//
+	// 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
+	if authzCfg.RenderConsentPage {
+		var idpOriginOK bool
+		consentCSP, idpOriginOK = buildConsentCSP(oauth2Cfg.Endpoint.AuthURL, authzCfg.CSPFormActionExtra)
+		if !idpOriginOK {
+			logger.Warn("consent_csp_idp_origin_missing",
+				zap.String("auth_url", oauth2Cfg.Endpoint.AuthURL),
+				zap.String("hint", "consent submit will be blocked in Chromium browsers; verify OIDC discovery"),
+			)
+		}
+	}
 	return func(w http.ResponseWriter, r *http.Request) {
 		q := r.URL.Query()
 		if rejectRepeatedParams(w, q,
@@ -233,7 +264,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, sealedConsent{
+			renderConsent(w, r, tm, logger, baseURL, authzCfg.ResourceName, consentCSP, 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.