diff --git a/src/auth.rs b/src/auth.rs index 817798e..4e553f0 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -515,6 +515,12 @@ const DEFAULT_ALLOWED_REDIRECTS: &[(&str, &str, PathPin)] = &[ ("perplexity.com", "/rest/connections/oauth_callback", PathPin::Prefix), ]; +/// The compiled-in allow-list's vendor domains, for the branding parity test. +#[cfg(test)] +pub(crate) fn default_redirect_domains() -> std::collections::BTreeSet<&'static str> { + DEFAULT_ALLOWED_REDIRECTS.iter().map(|(domain, _, _)| *domain).collect() +} + /// The effective hosted-redirect allow-list: the compiled-in defaults plus any /// entries in `OAUTH_ALLOWED_REDIRECT_PREFIXES`. Each env entry is a bare /// `https://host/path` value pinning a host + path prefix; an entry that is not @@ -568,7 +574,7 @@ fn parse_redirect_prefix(raw: &str) -> Option<(String, String)> { { return None; } - let host = u.host_str()?.trim_end_matches('.').to_ascii_lowercase(); + let host = host_key(u.host_str()?); let path = u.path().to_string(); (!host.is_empty() && path != "/" && !path.is_empty()).then_some((host, path)) } @@ -601,6 +607,14 @@ fn path_has_percent_encoding(path: &str) -> bool { path.contains('%') } +/// Whether `host` (already [`host_key`]-normalized) is `domain` or a +/// dot-boundary subdomain of it, so a look-alike apex (`evilchatgpt.com`) never +/// matches `chatgpt.com`. The one host rule for redirect validation, the CIMD +/// trust policy, and [`crate::branding`], so none can disagree about a vendor. +pub(crate) fn host_is_or_under(host: &str, domain: &str) -> bool { + host == domain || host.strip_suffix(domain).is_some_and(|p| p.ends_with('.')) +} + /// Whether a `redirect_uri` may be registered or receive an authorization code. /// No redirect (loopback or hosted) may carry a query or fragment component: the /// authorization endpoint appends `?code=…&state=…`, so a pre-existing query would @@ -619,7 +633,7 @@ fn path_has_percent_encoding(path: &str) -> bool { /// `https://claude.ai.evil.com` resolve to their real host and are refused; a bare /// `https://user@claude.ai` is refused too (userinfo serves no purpose in a /// redirect target, only muddies which host is addressed, and loopback rejects it). -fn redirect_uri_permitted(redirect_uri: &str) -> bool { +pub(crate) fn redirect_uri_permitted(redirect_uri: &str) -> bool { let Ok(url) = url::Url::parse(redirect_uri) else { return false; }; @@ -638,6 +652,40 @@ fn redirect_uri_permitted(redirect_uri: &str) -> bool { if is_loopback_url(&url) { return true; } + hosted_redirect_admitted( + &url, + allowed_redirects() + .iter() + .map(|(domain, path, pin)| (domain.as_str(), path.as_str(), *pin)), + ) +} + +/// Whether `redirect_uri` is a hosted redirect admitted by a **compiled-in** +/// [`DEFAULT_ALLOWED_REDIRECTS`] entry, under the same rules as +/// [`redirect_uri_permitted`] (no query or fragment, canonical `https` shape, no +/// percent-encoding, pinned path). Operator entries from +/// `OAUTH_ALLOWED_REDIRECT_PREFIXES` do not count, and neither does loopback. +/// Connector branding ([`crate::branding`]) rests on this, so a vendor's name is +/// only ever shown for the reviewed callback paths in this repo, never for a path +/// a deployment added on the vendor's domain. +pub(crate) fn redirect_uri_on_default_allow_list(redirect_uri: &str) -> bool { + let Ok(url) = url::Url::parse(redirect_uri) else { + return false; + }; + if url.query().is_some() || url.fragment().is_some() { + return false; + } + hosted_redirect_admitted(&url, DEFAULT_ALLOWED_REDIRECTS.iter().copied()) +} + +/// The hosted half of [`redirect_uri_permitted`], over an explicit set of +/// `(domain, path, pin)` entries: `url` (already free of query and fragment) must +/// have the canonical `https://` shape, a path with no +/// percent-encoding, and a host and path that some entry admits. +fn hosted_redirect_admitted<'a>( + url: &url::Url, + entries: impl IntoIterator, +) -> bool { let Some(host) = url.host_str() else { return false; }; @@ -651,10 +699,10 @@ fn redirect_uri_permitted(redirect_uri: &str) -> bool { let Ok(canonical) = url::Url::parse(&format!("https://{host}{}", url.path())) else { return false; }; - if url != canonical { + if url != &canonical { return false; } - let host = host.trim_end_matches('.').to_ascii_lowercase(); + let host = host_key(host); let path = url.path(); // url::Url collapses only LITERAL `.`/`..` segments (raw or `%2e`-encoded whole // segments), so a real-slash traversal `/connector/oauth/../../g/evil` arrives @@ -673,8 +721,8 @@ fn redirect_uri_permitted(redirect_uri: &str) -> bool { // or descendants too). The path pin is what keeps a registration off // third-party/user-content paths (e.g. `/page/…`, `/g/…`) on the same origin; // without it, domain-only matching would let those capture the code. - allowed_redirects().iter().any(|(domain, prefix, pin)| { - (host == *domain || host.strip_suffix(domain.as_str()).is_some_and(|p| p.ends_with('.'))) + entries.into_iter().any(|(domain, prefix, pin)| { + host_is_or_under(&host, domain) && match pin { PathPin::Exact => path == prefix, PathPin::Prefix => path_within_prefix(path, prefix), @@ -990,15 +1038,17 @@ fn allow_listed_domain(host: &str) -> bool { /// subdomain of, if any: the trust policy's match. fn vetted_domain(host: &str) -> Option<&'static str> { let host = host_key(host); - allowed_redirects().iter().map(|(domain, _, _)| domain.as_str()).find(|domain| { - host == *domain || host.strip_suffix(*domain).is_some_and(|p| p.ends_with('.')) - }) + allowed_redirects() + .iter() + .map(|(domain, _, _)| domain.as_str()) + .find(|domain| host_is_or_under(&host, domain)) } /// One spelling per host — lower-case, no trailing dot — so that whatever is -/// keyed by host (the trust policy's match, the per-host in-flight slots) treats -/// `claude.ai`, `Claude.AI` and `claude.ai.` as the one host they resolve to. -fn host_key(host: &str) -> String { +/// keyed or matched by host (redirect validation, the trust policy's match, the +/// per-host in-flight slots, [`crate::branding`]) treats `claude.ai`, +/// `Claude.AI` and `claude.ai.` as the one host they resolve to. +pub(crate) fn host_key(host: &str) -> String { host.trim_end_matches('.').to_ascii_lowercase() } @@ -1624,8 +1674,9 @@ impl AuthStore { /// This instance's AS issuer: `{public_url}{mcp_path}` (an RFC 8414 *path /// issuer* whenever the router is nested below the root). Every OAuth - /// endpoint lives under it, at `{issuer}/oauth/*`. - fn issuer(&self) -> String { + /// endpoint lives under it, at `{issuer}/oauth/*`; the branding endpoints at + /// `{issuer}/branding*` ([`crate::branding`]) root here too. + pub(crate) fn issuer(&self) -> String { format!("{}{}", self.public_url, self.mcp_path) } @@ -1854,6 +1905,20 @@ impl AuthStore { authz.insert(session_id, pending); } + /// The validated `redirect_uri` of the pending, unexpired connect + /// `session_id`, if there is one. Read-only — it neither consumes nor touches + /// the entry — so a lookup cannot interfere with the handshake. Branding + /// ([`crate::branding`]) derives the connector from THIS, the server's record + /// of what `authorize` validated, and never from anything the (craftable) + /// connect-link fragment asserts. + pub(crate) async fn pending_redirect_uri(&self, session_id: &str) -> Option { + let authz = self.authz.read().await; + authz + .get(session_id) + .filter(|pending| !pending.remaining().is_zero()) + .map(|pending| pending.redirect_uri.clone()) + } + /// Drop every expired entry from the short-lived OAuth maps, returning how /// many went from each. The admission bounds ([`make_room`]) cap these maps at /// all times; this is what returns the memory once a burst has passed, so an @@ -3212,6 +3277,33 @@ mod tests { /// pinned callback path; loopback always passes; everything else (a /// user-content path on an allow-listed origin, a wrong path, an unlisted /// domain, or an authority-trick look-alike) is refused. + // Branding's vetting ignores operator entries: a path a deployment adds on a + // vendor's domain is a valid redirect but never earns that vendor's name. + #[test] + fn default_allow_list_vetting_ignores_operator_entries() { + use super::{ + hosted_redirect_admitted, redirect_uri_on_default_allow_list, PathPin, + DEFAULT_ALLOWED_REDIRECTS, + }; + let ops_added = "https://claude.ai/ops/added/cb"; + let with_operator_entry = DEFAULT_ALLOWED_REDIRECTS.iter().copied().chain([( + "claude.ai", + "/ops/added", + PathPin::Prefix, + )]); + let url = url::Url::parse(ops_added).unwrap(); + assert!( + hosted_redirect_admitted(&url, with_operator_entry), + "the operator entry admits it" + ); + assert!(!redirect_uri_on_default_allow_list(ops_added), "but it is not a vetted callback"); + // The compiled-in callbacks are vetted, with validation's own rules. + assert!(redirect_uri_on_default_allow_list("https://claude.ai/api/mcp/auth_callback")); + assert!(redirect_uri_on_default_allow_list("https://claude.ai./api/mcp/auth_callback")); + assert!(!redirect_uri_on_default_allow_list("https://claude.ai/api/mcp/auth_callback?x=1")); + assert!(!redirect_uri_on_default_allow_list("http://127.0.0.1:5173/cb")); + } + #[test] fn hosted_redirect_allow_list() { // Allow-listed vendor domains/subdomains UNDER their pinned callback path. @@ -4603,12 +4695,123 @@ mod tests { assert!(url.starts_with("https://ii.test/mcp#"), "everything rides the fragment: {url}"); assert!(url.contains("state=sess-1")); assert!(url.contains("registration_key=PUBX")); + // The fragment is attacker-craftable, so it must never ASSERT branding: + // II asks the server (`/branding?state=…`), which answers from the + // session's validated redirect (see `branding_is_bound_to_the_session`). + assert!(!url.contains("connector"), "no branding hint in the fragment: {url}"); // The callback lives under the instance's mount ({public_url}{mcp_path}). let encoded = urlencoding::encode("https://mcp.test/mcp/oauth/connect/callback").into_owned(); assert!(url.contains(&format!("callback={encoded}")), "callback under the mount: {url}"); } + /// Record a pending connect for `redirect` the way `authorize` does (through + /// the bounded insert), created at `created`. + async fn seed_pending_redirect( + store: &super::AuthStore, + id: &str, + redirect: &str, + created: std::time::Instant, + ) { + store + .insert_pending( + id.to_string(), + super::AuthzPending { + client_id: "c".into(), + redirect_uri: redirect.into(), + client_state: String::new(), + code_challenge: Some("cc".into()), + cookie: "k".into(), + created, + code: None, + redeeming: false, + }, + ) + .await; + } + + /// GET `/branding?state=` for `state`: the status and (on 200) the JSON body. + async fn branding_for( + store: &super::AuthStore, + state: Option<&str>, + ) -> (axum::http::StatusCode, serde_json::Value) { + use axum::extract::{Query, State}; + let resp = crate::branding::branding_metadata( + State(store.clone()), + Ok(Query(crate::branding::BrandingQuery { state: state.map(str::to_string) })), + ) + .await; + let status = resp.status(); + let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX).await.expect("body"); + (status, serde_json::from_slice(&bytes).unwrap_or(serde_json::Value::Null)) + } + + // Branding is BOUND to the authorization session: II asks by `state`, and the + // server answers from that pending connect's VALIDATED redirect — never from + // anything the craftable fragment asserts. So a loopback/native client that + // drives navigation cannot make its own (valid) session read as a vetted + // connector: there is no slug to add or swap, and its session's redirect is + // loopback. Unknown, missing, and expired states all get the same 404. + #[tokio::test] + async fn branding_is_bound_to_the_session() { + use axum::http::StatusCode; + let store = test_store(); + let now = std::time::Instant::now(); + seed_pending_redirect( + &store, + "sess-claude", + "https://claude.ai/api/mcp/auth_callback", + now, + ) + .await; + seed_pending_redirect(&store, "sess-native", "http://127.0.0.1:5173/cb", now).await; + // Validation trims a trailing root dot, so branding must too. + seed_pending_redirect(&store, "sess-dot", "https://claude.ai./api/mcp/auth_callback", now) + .await; + + // A vetted session reads as its connector, with the logo under the issuer. + let (status, doc) = branding_for(&store, Some("sess-claude")).await; + assert_eq!(status, StatusCode::OK); + assert_eq!(doc["name"], "Claude"); + assert_eq!(doc["verified"], true); + assert_eq!(doc["logo"], "https://mcp.test/mcp/branding/claude/logo"); + + // The core of the finding: a valid NATIVE session gets no branding. + assert_eq!(branding_for(&store, Some("sess-native")).await.0, StatusCode::NOT_FOUND); + + // The trailing-dot form of a vetted redirect keeps its branding. + let (status, doc) = branding_for(&store, Some("sess-dot")).await; + assert_eq!(status, StatusCode::OK); + assert_eq!(doc["name"], "Claude"); + + // Unknown and missing states are indistinguishable from "no branding". + assert_eq!(branding_for(&store, Some("sess-unknown")).await.0, StatusCode::NOT_FOUND); + assert_eq!(branding_for(&store, None).await.0, StatusCode::NOT_FOUND); + + // An EXPIRED pending connect is gone for branding too (same definition of + // expired as the handshake: `AuthzPending::remaining`). Skipped only on a + // host whose monotonic clock started less than CONNECT_TTL ago. + if let Some(past) = now.checked_sub(super::CONNECT_TTL + std::time::Duration::from_secs(1)) + { + seed_pending_redirect( + &store, + "sess-old", + "https://claude.ai/api/mcp/auth_callback", + past, + ) + .await; + assert_eq!(branding_for(&store, Some("sess-old")).await.0, StatusCode::NOT_FOUND); + } + + // Read-only: the lookups neither consumed nor advanced the pending + // connect, so the handshake proceeds exactly as if II never asked. + let authz = store.authz.read().await; + let pending = authz.get("sess-claude").expect("still pending"); + assert_eq!(pending.redirect_uri, "https://claude.ai/api/mcp/auth_callback"); + assert_eq!(pending.created, now); + assert!(pending.code.is_none() && !pending.redeeming); + } + // The allow-list invariant (II #4091 matches by EXACT string equality): the // /.well-known/ii-auth-callbacks document must declare, verbatim, the same // callback URLs the II links embed — for every instance. Built from one diff --git a/src/branding.rs b/src/branding.rs new file mode 100644 index 0000000..5779d82 --- /dev/null +++ b/src/branding.rs @@ -0,0 +1,378 @@ +//! Verified-connector branding: surface a vetted MCP client's product name and +//! logo to Internet Identity's consent screen, so the user sees WHICH vetted +//! product will receive the access a connect grants, rather than granting their +//! II accounts on behalf of an unnamed client. It names where the authorization +//! code is delivered — not who started the connect, nor which account at that +//! product ends up holding the grant. +//! +//! **The server is the only source of branding, and it answers per session.** II +//! asks `GET {issuer}/branding?state={state}` — the `state` it already holds from +//! the connect link — and the server derives the connector from that pending, +//! unexpired connect's **validated** `redirect_uri` (see +//! [`AuthStore::pending_redirect_uri`]). Nothing in the connect-link fragment +//! asserts branding: the fragment is attacker-craftable (a native client drives +//! the navigation and sees the 302), so a client-supplied slug there would let +//! any valid session claim to be "verified Claude". Binding to the session's +//! validated redirect closes that at the root: a loopback (native-app) session, +//! or one on an unlisted host, reads as anonymous no matter what the fragment +//! says. +//! +//! Keyed on the **vetted redirect vendor**, never on the client-supplied +//! `client_name`/`logo_uri`: open DCR takes all callers, so client-supplied +//! identity is attacker-controlled. Anyone may *register* a vetted +//! `redirect_uri`, but the path-pinned allow-list means the authorization code +//! for it is delivered only to that vendor's own callback — so whoever completes +//! the connect is that vendor, and the redirect vendor is a sound, +//! server-curated proxy for product identity. (That holds for a conforming +//! browser; a user agent the attacker controls, such as an embedded webview, is +//! out of scope for branding as it is for the rest of the flow.) Only a redirect +//! admitted by a **compiled-in** allow-list entry resolves +//! ([`crate::auth::redirect_uri_on_default_allow_list`]), so a vendor's name is +//! never shown for a path an operator added on the vendor's domain. Host matching +//! reuses validation's own rule ([`crate::auth::host_key`], +//! [`crate::auth::host_is_or_under`]), so branding and validation cannot disagree +//! about a vendor. +//! +//! Endpoints, both issuer-rooted (same origin as the #4091-validated callback): +//! `GET /branding?state=…` (session-bound metadata: name, logo URL, `verified`) +//! and `GET /branding/{slug}/logo` (the bundled image — a static per-connector +//! asset, no trust decision). II-side coordination is required to render any of +//! this — including showing the `verified` badge only for imcp2 issuer origins II +//! itself trusts, and using the one `state` it parsed from the link for both the +//! lookup and the callback. Until that ships, the endpoints are inert. + +use axum::{ + extract::{rejection::QueryRejection, Path, Query, State}, + http::{header, HeaderValue, StatusCode}, + response::{IntoResponse, Response}, + Json, +}; +use serde::Deserialize; +use serde_json::json; + +use crate::auth::{host_is_or_under, host_key, redirect_uri_on_default_allow_list, AuthStore}; + +/// A vetted connector's server-curated branding. +pub(crate) struct Connector { + /// Stable slug: the `/branding/{slug}/logo` path segment. Server-determined, + /// never client-supplied. + pub slug: &'static str, + /// Human display name shown on the consent screen. + pub name: &'static str, + /// Apex domains identifying this vendor, matched against the validated + /// `redirect_uri` host — the host itself or a dot-boundary subdomain of it. + domains: &'static [&'static str], + /// The bundled logo (SVG markup). A NEUTRAL PLACEHOLDER today + /// ([`PLACEHOLDER_LOGO_SVG`]); swap this per-connector for the vendor's own + /// licensed mark. + logo: &'static str, +} + +/// A neutral, generic "verified connector" mark — ORIGINAL artwork (a check in a +/// ring on a parchment tile), deliberately NOT a reproduction of any vendor's +/// logo, which would be a trademark/licensing matter this repo should not decide. +/// It is the placeholder for every connector until each vendor's own licensed +/// mark is dropped into its [`Connector::logo`]. II renders it via a fixed-size +/// `` (an ``-loaded SVG cannot execute script), but the logo URL can +/// also be opened top-level on the issuer origin, so every bundled logo must be +/// static and script-free (a test checks) and is served under a sandboxing CSP. +pub(crate) const PLACEHOLDER_LOGO_SVG: &str = r##""##; + +/// The vetted connectors and their curated branding. The domains are exactly the +/// vendor domains of the compiled-in `DEFAULT_ALLOWED_REDIRECTS` (a test holds +/// the two sets equal). An `OAUTH_ALLOWED_REDIRECT_PREFIXES` entry is accepted for +/// redirects but never branded, even on a vendor's domain, because resolution +/// requires a compiled-in entry ([`connector_for_redirect`]). v1 +/// covers self-authenticating WEB connectors only; native/loopback apps stay +/// anonymous (their only identity signal is a spoofable `client_name`). Logos +/// are placeholders — see [`PLACEHOLDER_LOGO_SVG`]. +pub(crate) const CONNECTORS: &[Connector] = &[ + Connector { + slug: "chatgpt", + name: "ChatGPT", + domains: &["chatgpt.com"], + logo: PLACEHOLDER_LOGO_SVG, + }, + Connector { + slug: "claude", + name: "Claude", + domains: &["claude.ai"], + logo: PLACEHOLDER_LOGO_SVG, + }, + Connector { + slug: "cursor", + name: "Cursor", + domains: &["cursor.com"], + logo: PLACEHOLDER_LOGO_SVG, + }, + Connector { slug: "grok", name: "Grok", domains: &["grok.com"], logo: PLACEHOLDER_LOGO_SVG }, + Connector { + slug: "perplexity", + name: "Perplexity", + domains: &["perplexity.ai", "perplexity.com"], + logo: PLACEHOLDER_LOGO_SVG, + }, + Connector { + slug: "antigravity", + name: "Google Antigravity", + domains: &["antigravity.google"], + logo: PLACEHOLDER_LOGO_SVG, + }, +]; + +/// The vetted connector a `redirect_uri` belongs to, if any. The redirect must be +/// admitted by a compiled-in allow-list entry under validation's own rules +/// ([`redirect_uri_on_default_allow_list`]: canonical `https`, pinned path, no +/// port, userinfo, query, fragment, or percent-encoding), so this never depends +/// on its caller having validated, and an operator-added entry never brands. The +/// host then matches exactly as validation does — trailing root dots trimmed, +/// lowercased, dot-boundary subdomains. `None` for loopback, an unlisted host, or +/// an operator entry, which read as anonymous. +pub(crate) fn connector_for_redirect(redirect_uri: &str) -> Option<&'static Connector> { + if !redirect_uri_on_default_allow_list(redirect_uri) { + return None; + } + let url = url::Url::parse(redirect_uri).ok()?; + let host = host_key(url.host_str()?); + CONNECTORS.iter().find(|c| c.domains.iter().any(|domain| host_is_or_under(&host, domain))) +} + +/// The connector for a curated slug, or `None` for any slug outside the set (so a +/// `/branding/{slug}/logo` path segment can never probe arbitrary keys). +pub(crate) fn connector_by_slug(slug: &str) -> Option<&'static Connector> { + CONNECTORS.iter().find(|c| c.slug == slug) +} + +/// `?state=` of the session-bound metadata request: the connect `state` (= the +/// pending session id) II already holds from the connect link. +#[derive(Deserialize)] +pub(crate) struct BrandingQuery { + pub(crate) state: Option, +} + +/// `GET {issuer}/branding?state={state}` — the connector branding for ONE pending +/// connect: the curated name, the absolute logo URL, and `verified`, derived from +/// that connect's validated redirect. `404` — identically — for a missing, +/// unknown, or expired `state` and for a session whose redirect is not a vetted +/// connector (loopback / unlisted), so the response is no oracle for which +/// sessions exist; a malformed query (e.g. a repeated `state`) gets that same +/// 404 rather than axum's 400. Every response is `no-store`: the answer is +/// per-session. +pub(crate) async fn branding_metadata( + State(store): State, + query: Result, QueryRejection>, +) -> Response { + let Some(state) = query.ok().and_then(|Query(query)| query.state) else { + return branding_not_found(); + }; + let Some(redirect) = store.pending_redirect_uri(&state).await else { + return branding_not_found(); + }; + let Some(connector) = connector_for_redirect(&redirect) else { + return branding_not_found(); + }; + let logo = format!("{}/branding/{}/logo", store.issuer(), connector.slug); + let mut resp = + Json(json!({ "name": connector.name, "logo": logo, "verified": true })).into_response(); + resp.headers_mut().insert(header::CACHE_CONTROL, HeaderValue::from_static("no-store")); + resp +} + +/// `GET {issuer}/branding/{slug}/logo` — the bundled connector logo (SVG bytes), +/// with `nosniff` and a cache header. `404` outside the curated set. A static +/// per-connector asset: it makes no claim about any session (that is +/// [`branding_metadata`]'s job, whose `logo` URL points here). II MUST render it +/// via a fixed-size ``, never inline the SVG into the consent DOM: an +/// ``-loaded SVG cannot execute script, an inlined one can. Opened top-level +/// it would render as a document on the issuer origin, so it also carries a +/// sandboxing CSP ([`LOGO_CSP`]); an `` ignores that header. +pub(crate) async fn branding_logo(Path(slug): Path) -> Response { + let Some(connector) = connector_by_slug(&slug) else { + return branding_not_found(); + }; + let mut resp = (StatusCode::OK, connector.logo).into_response(); + let headers = resp.headers_mut(); + headers.insert(header::CONTENT_TYPE, HeaderValue::from_static("image/svg+xml")); + headers.insert(header::X_CONTENT_TYPE_OPTIONS, HeaderValue::from_static("nosniff")); + headers.insert(header::CACHE_CONTROL, HeaderValue::from_static("public, max-age=3600")); + headers.insert(header::CONTENT_SECURITY_POLICY, HeaderValue::from_static(LOGO_CSP)); + resp +} + +/// The logo's `Content-Security-Policy`: no script, no subresources, a sandboxed +/// document — inert even when the SVG is opened top-level on the issuer origin. +/// Inline styles stay allowed, since SVG presentation may use them. +const LOGO_CSP: &str = "default-src 'none'; style-src 'unsafe-inline'; sandbox"; + +/// No branding for this request: 404, the same for every reason, and `no-store` +/// like the answer it stands in for. +fn branding_not_found() -> Response { + let mut resp = (StatusCode::NOT_FOUND, "no connector branding").into_response(); + resp.headers_mut().insert(header::CACHE_CONTROL, HeaderValue::from_static("no-store")); + resp +} + +#[cfg(test)] +mod tests { + use std::collections::BTreeSet; + + use super::{connector_by_slug, connector_for_redirect, CONNECTORS}; + + #[test] + fn resolves_vetted_redirect_vendors_to_their_slug() { + let cases = [ + ("https://chatgpt.com/connector/oauth/abc", "chatgpt"), + ("https://claude.ai/api/mcp/auth_callback", "claude"), + ("https://cursor.com/agents/mcp/oauth/callback", "cursor"), + ("https://grok.com/mcp/callback", "grok"), + ("https://perplexity.ai/rest/connections/oauth_callback", "perplexity"), + ("https://antigravity.google/oauth-callback", "antigravity"), + // Subdomains (Perplexity uses www/staging/…; Cursor registers www). + ("https://www.perplexity.ai/rest/connections/oauth_callback", "perplexity"), + ("https://www.cursor.com/agents/mcp/oauth/callback", "cursor"), + // A trailing root dot names the same host; validation accepts it, so + // branding must resolve it too. + ("https://claude.ai./api/mcp/auth_callback", "claude"), + // Host case never matters. + ("https://CLAUDE.AI/api/mcp/auth_callback", "claude"), + ]; + for (redirect, slug) in cases { + assert_eq!( + connector_for_redirect(redirect).map(|c| c.slug), + Some(slug), + "redirect {redirect}" + ); + } + } + + #[test] + fn unvetted_and_lookalike_and_loopback_get_no_branding() { + // Loopback (native app) → no branding, by design. + assert!(connector_for_redirect("http://127.0.0.1:5173/cb").is_none()); + assert!(connector_for_redirect("http://[::1]:8080/cb").is_none()); + assert!(connector_for_redirect("http://localhost/cb").is_none()); + // An unlisted vendor → none. + assert!(connector_for_redirect("https://attacker.example/cb").is_none()); + // Look-alikes must NOT match at a non-segment boundary. + assert!(connector_for_redirect("https://evilchatgpt.com/cb").is_none()); + assert!(connector_for_redirect("https://claude.ai.attacker.example/cb").is_none()); + // A non-https vetted host never resolves (validation would refuse it). + assert!(connector_for_redirect("http://claude.ai/api/mcp/auth_callback").is_none()); + // Not a URL at all → none, never a panic. + assert!(connector_for_redirect("not a url").is_none()); + } + + #[test] + fn a_redirect_that_validation_refuses_never_resolves() { + // A vetted host, but a path off its pin, a port, userinfo, a query, or + // percent-encoding: validation refuses each, so branding must too. + for redirect in [ + "https://claude.ai/not/the/callback", + "https://claude.ai:8443/api/mcp/auth_callback", + "https://user@claude.ai/api/mcp/auth_callback", + "https://claude.ai/api/mcp/auth_callback?x=1", + "https://claude.ai/api/mcp/auth_callback/%2e%2e", + ] { + assert!(connector_for_redirect(redirect).is_none(), "redirect {redirect}"); + } + } + + #[test] + fn connector_domains_are_the_compiled_in_allow_list_vendors() { + // Every vetted vendor is curated, and nothing is curated that cannot + // obtain a vetted redirect — so a new allow-list entry fails this test + // until someone decides its branding. + let curated: BTreeSet<&str> = + CONNECTORS.iter().flat_map(|c| c.domains.iter().copied()).collect(); + assert_eq!(curated, crate::auth::default_redirect_domains()); + } + + /// What makes `svg` unfit to bundle as a logo, if anything. A logo URL can be + /// opened top-level on the issuer origin, where an SVG is a document, so every + /// bundled mark must be inert on its own: no script, event handlers, embedded + /// HTML, animation, or DTD, and no reference that leaves the document — every + /// `href`, `src`, and `url()` must be an internal `#…` reference. A guard for + /// hand-reviewed assets, not a general SVG sanitizer. + fn logo_problems(svg: &str) -> Vec { + let lower = svg.to_ascii_lowercase(); + let mut problems = Vec::new(); + if !lower.trim_start().starts_with(" 0 && rest[name..].trim_start().starts_with('=') + }); + if handler { + problems.push("event-handler attribute".to_string()); + } + problems + } + + #[test] + fn bundled_logos_are_static_svg() { + for c in CONNECTORS { + assert_eq!(logo_problems(c.logo), Vec::::new(), "{}: bundled logo", c.slug); + } + // Internal references are fine. + let internal = r##""##; + assert!(logo_problems(internal).is_empty()); + // The guard catches each unsafe form, including spaced and quoted spellings. + for bad in [ + r#""#, + r#""#, + r#""#, + r#""#, + r#""#, + r#""#, + r#""#, + r#""#, + r#"
"#, + r#""#, + r#""#, + r#""#, + r#"]>&e;"#, + ] { + assert!(!logo_problems(bad).is_empty(), "the guard missed: {bad}"); + } + } + + #[test] + fn slug_lookup_is_closed_to_the_curated_set() { + assert!(connector_by_slug("claude").is_some()); + assert!(connector_by_slug("unknown").is_none()); + assert!(connector_by_slug("../secrets").is_none()); + assert!(connector_by_slug("").is_none()); + for c in CONNECTORS { + assert!(!c.slug.is_empty() && !c.name.is_empty()); + } + } +} diff --git a/src/lib.rs b/src/lib.rs index 901a600..fd10831 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -73,6 +73,7 @@ //! and `II_URL_PROD` / `II_CANISTER_ID_PROD` (the Internet Identity instances). mod auth; +mod branding; // Docs live in the module itself (`//!` in src/metrics.rs): an outer doc // comment here would resolve its intra-doc links in *this* scope rather than // the module's, silently breaking the links to `Registry` and `MatchedPath`. @@ -236,9 +237,14 @@ impl McpServer { /// * `/.well-known/oauth-authorization-server` — the OIDC-style /// alternate location of the AS metadata (some clients derive /// `/.well-known/…` instead of RFC 8414 path insertion); - /// * everything else — the MCP streamable-HTTP endpoint (the router + /// * `/branding?state=…` and `/branding/{slug}/logo` — verified-connector + /// branding for Internet Identity's consent screen, CORS-open: the + /// first answers for one pending connect (from its validated + /// `redirect_uri`, `no-store`, `404` otherwise), the second serves a + /// curated connector's static logo; + /// * every other path — the MCP streamable-HTTP endpoint (the router /// fallback, so the bare mount path, its trailing-slash form, and - /// sub-paths all reach it), bearer-token gated, with the CORS + /// unmatched sub-paths all reach it), bearer-token gated, with the CORS /// preflight answered before authentication and `WWW-Authenticate` /// exposed cross-origin. /// @@ -330,9 +336,21 @@ impl McpServer { .with_state(self.store.clone()) .layer(permissive_cors()); + // Verified-connector branding (see [`branding`]), rooted at the issuer so + // II reaches it same-origin with the #4091-validated callback: the + // session-bound metadata (`/branding?state=…`, answered from that pending + // connect's validated redirect) and the static per-connector logo. + // CORS-open, like the other endpoints II fetches cross-origin. + let branding = Router::new() + .route("/branding", get(branding::branding_metadata)) + .route("/branding/{slug}/logo", get(branding::branding_logo)) + .with_state(self.store.clone()) + .layer(permissive_cors()); + Router::new() .nest("/oauth", oauth) .merge(oidc_alternate) + .merge(branding) // Everything else is the MCP endpoint: the bare mount path, its // trailing-slash form, and any sub-path (the streamable service // dispatches on method, not path) — same breadth `nest_service` diff --git a/tests/routers.rs b/tests/routers.rs index a5af63c..b82b562 100644 --- a/tests/routers.rs +++ b/tests/routers.rs @@ -215,6 +215,11 @@ async fn unauthenticated_mcp_requests_get_the_path_aware_challenge() { /// registration lands in the state directory's `oauth-clients.json` so it /// survives a restart, and a redirect off the hosted allow-list is refused /// before anything is stored. +/// +/// It also drives verified-connector branding end to end over HTTP (authorize → +/// the `state` in II's link → `GET /branding?state=`), because this is the one +/// test that registers a client: a second accepting registration elsewhere would +/// race the persistence poll below on the shared store file. #[tokio::test] async fn dynamic_client_registration_round_trips_and_persists() { // One app, cloned per request, so both calls share the same client store. @@ -237,28 +242,86 @@ async fn dynamic_client_registration_round_trips_and_persists() { } }; - let (status, doc) = register(r#"{"redirect_uris":["http://127.0.0.1:4321/cb"]}"#).await; + // Loopback first (asserted below); the two claude.ai spellings — plain and + // with a trailing root dot, which validation accepts — are for branding. + let (status, doc) = register( + r#"{"redirect_uris":["http://127.0.0.1:4321/cb", + "https://claude.ai/api/mcp/auth_callback", + "https://claude.ai./api/mcp/auth_callback"]}"#, + ) + .await; assert_eq!(status, StatusCode::CREATED); let client_id = doc["client_id"].as_str().expect("a client_id").to_string(); assert_eq!(doc["redirect_uris"][0], "http://127.0.0.1:4321/cb"); // The fresh registration is usable: authorize accepts it and hands the - // browser to Internet Identity (302 + the binding cookie). - let resp = app - .clone() - .oneshot( - Request::get(format!( - "/mcp/oauth/authorize?response_type=code&client_id={client_id}\ - &redirect_uri=http://127.0.0.1:4321/cb&code_challenge=abc\ - &code_challenge_method=S256" - )) - .body(Body::empty()) - .unwrap(), - ) - .await - .unwrap(); - assert_eq!(resp.status(), StatusCode::FOUND, "a registered client can start a sign-in"); - assert!(resp.headers().contains_key("set-cookie"), "the binding cookie must be set"); + // browser to Internet Identity (302 + the binding cookie). Returns the + // connect `state` (the pending session id) from the II link's fragment — + // what II passes back to `/branding`. + let authorize = |redirect: &'static str| { + let app = app.clone(); + let client_id = client_id.clone(); + async move { + let resp = app + .oneshot( + Request::get(format!( + "/mcp/oauth/authorize?response_type=code&client_id={client_id}\ + &redirect_uri={redirect}&code_challenge=abc\ + &code_challenge_method=S256" + )) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::FOUND, "a registered client can start a sign-in"); + assert!(resp.headers().contains_key("set-cookie"), "the binding cookie must be set"); + let location = resp.headers()["location"].to_str().unwrap().to_string(); + let (_, fragment) = location.split_once('#').expect("the II link carries a fragment"); + url::form_urlencoded::parse(fragment.as_bytes()) + .find(|(key, _)| key == "state") + .map(|(_, state)| state.into_owned()) + .expect("the II link carries the connect state") + } + }; + let branding = |state: String| { + let app = app.clone(); + async move { + let resp = app + .oneshot( + Request::get(format!("/mcp/branding?state={state}")) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + let status = resp.status(); + let cache = resp + .headers() + .get("cache-control") + .and_then(|v| v.to_str().ok()) + .map(str::to_owned); + let bytes = resp.into_body().collect().await.unwrap().to_bytes(); + let doc = serde_json::from_slice(&bytes).unwrap_or(serde_json::Value::Null); + (status, cache, doc) + } + }; + + // Branding is answered per session, from that session's validated redirect: + // a loopback (native-app) session stays anonymous ... + let (status, _, _) = branding(authorize("http://127.0.0.1:4321/cb").await).await; + assert_eq!(status, StatusCode::NOT_FOUND, "a loopback session gets no branding"); + // ... and both claude.ai spellings brand as Claude, uncached. + for redirect in + ["https://claude.ai/api/mcp/auth_callback", "https://claude.ai./api/mcp/auth_callback"] + { + let (status, cache, doc) = branding(authorize(redirect).await).await; + assert_eq!(status, StatusCode::OK, "branding for {redirect}"); + assert_eq!(doc["name"], "Claude", "branding for {redirect}"); + assert_eq!(doc["verified"], true, "branding for {redirect}"); + assert_eq!(doc["logo"], format!("{PUBLIC_URL}/mcp/branding/claude/logo")); + assert_eq!(cache.as_deref(), Some("no-store"), "a per-session answer must not be cached"); + } // A hosted redirect that isn't allow-listed is refused (nothing stored). let (status, doc) = register(r#"{"redirect_uris":["https://attacker.example/cb"]}"#).await; @@ -451,3 +514,66 @@ async fn oauth_endpoints_live_under_each_mount() { ); } } + +/// Verified-connector branding routing (II fetches these from the issuer origin to +/// brand the consent screen). The metadata is SESSION-BOUND — answered only for a +/// pending connect's validated redirect (the 200 path over HTTP is driven by +/// `dynamic_client_registration_round_trips_and_persists`, and expiry by +/// `branding_is_bound_to_the_session` in `src/auth.rs`); here: no session → 404, +/// CORS-open for II's cross-origin fetch, the unbound per-slug metadata path is +/// gone, and the static logo serves. +#[tokio::test] +async fn branding_endpoints_are_session_bound_and_serve_logos() { + // No such session, and no `state` at all: the same 404. + for path in ["/mcp/branding?state=sess-does-not-exist", "/mcp/branding"] { + let resp = app() + .oneshot( + Request::get(path).header("origin", "https://id.ai").body(Body::empty()).unwrap(), + ) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::NOT_FOUND, "GET {path}"); + assert!( + resp.headers().contains_key("access-control-allow-origin"), + "GET {path}: II fetches this cross-origin, so it must be CORS-open" + ); + assert_eq!( + resp.headers().get("cache-control").and_then(|v| v.to_str().ok()), + Some("no-store"), + "GET {path}: the 404 is per-session too" + ); + } + + // The old UNBOUND per-slug metadata endpoint (it returned `verified: true` for + // any catalog slug) no longer exists: it must not answer with branding. + let resp = app() + .oneshot(Request::get("/mcp/branding/claude").body(Body::empty()).unwrap()) + .await + .unwrap(); + assert_ne!(resp.status(), StatusCode::OK, "no slug-keyed branding metadata"); + + // The logo is an SVG with nosniff (II renders it via , never inlined). + let resp = app() + .oneshot(Request::get("/mcp/branding/claude/logo").body(Body::empty()).unwrap()) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + assert_eq!( + resp.headers().get("content-type").and_then(|v| v.to_str().ok()), + Some("image/svg+xml") + ); + assert_eq!( + resp.headers().get("x-content-type-options").and_then(|v| v.to_str().ok()), + Some("nosniff") + ); + // Opened top-level, the SVG is a document on the issuer origin: keep it inert. + let csp = resp.headers().get("content-security-policy").and_then(|v| v.to_str().ok()); + assert!(csp.is_some_and(|v| v.contains("sandbox") && v.contains("default-src 'none'"))); + + // A slug outside the curated set 404s, so the path can't probe. + let resp = app() + .oneshot(Request::get("/mcp/branding/not-a-connector/logo").body(Body::empty()).unwrap()) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::NOT_FOUND); +}