diff --git a/docs/python-sdk/fastmcp-server-auth-oauth_proxy-models.mdx b/docs/python-sdk/fastmcp-server-auth-oauth_proxy-models.mdx index bca8b088e..c3d766a30 100644 --- a/docs/python-sdk/fastmcp-server-auth-oauth_proxy-models.mdx +++ b/docs/python-sdk/fastmcp-server-auth-oauth_proxy-models.mdx @@ -22,7 +22,7 @@ Stored server-side to track active authorization flows with client context. Includes CSRF tokens for consent protection per MCP security best practices. -### `ClientCode` +### `ClientCode` Client authorization code with PKCE and upstream tokens. @@ -31,7 +31,7 @@ Stored server-side after upstream IdP callback. Contains the upstream tokens bound to the client's PKCE challenge for secure token exchange. -### `UpstreamTokenSet` +### `UpstreamTokenSet` Stored upstream OAuth tokens from identity provider. @@ -41,7 +41,7 @@ and stored in plaintext within this model. Encryption is handled transparently at the storage layer via FernetEncryptionWrapper. Tokens are never exposed to MCP clients. -### `JTIMapping` +### `JTIMapping` Maps FastMCP token JTI to upstream token ID. @@ -50,7 +50,7 @@ This allows stateless JWT validation while still being able to look up the corresponding upstream token when tools need to access upstream APIs. -### `RefreshTokenMetadata` +### `RefreshTokenMetadata` Metadata for a refresh token, stored keyed by token hash. @@ -59,7 +59,7 @@ We store only metadata (not the token itself) for security - if storage is compromised, attackers get hashes they can't reverse into usable tokens. -### `ProxyDCRClient` +### `ProxyDCRClient` Client for DCR proxy with configurable redirect URI validation. @@ -89,7 +89,7 @@ arise from accepting arbitrary redirect URIs. **Methods:** -#### `validate_redirect_uri` +#### `validate_redirect_uri` ```python validate_redirect_uri(self, redirect_uri: AnyUrl | None) -> AnyUrl diff --git a/docs/servers/auth/oauth-proxy.mdx b/docs/servers/auth/oauth-proxy.mdx index b07092a1d..e8e79507c 100644 --- a/docs/servers/auth/oauth-proxy.mdx +++ b/docs/servers/auth/oauth-proxy.mdx @@ -404,10 +404,12 @@ sequenceDiagram Note over Proxy: Store transaction with client PKCE
Generate proxy PKCE pair Proxy->>User: 4. Show consent page
(client details, redirect URI, scopes) User->>Proxy: 5. Approve/deny consent + Note over Proxy: Set consent binding cookie
(binds browser to this flow) Proxy->>Provider: 6. Redirect to provider
redirect_uri=server:8000/auth/callback
code_challenge=PROXY_CHALLENGE Note over Provider, Proxy: Provider Callback Provider->>Proxy: 7. GET /auth/callback
with authorization code + Note over Proxy: Verify consent binding cookie
(reject if missing or mismatched) Proxy->>Provider: 8. Exchange code for tokens
code_verifier=PROXY_VERIFIER Provider-->>Proxy: 9. Access & refresh tokens @@ -432,18 +434,19 @@ The client initiates OAuth by redirecting to the proxy's `/authorize` endpoint. 1. Stores the client's transaction with its PKCE challenge 2. Generates its own PKCE parameters for upstream security 3. Shows the user a consent page with the client's details, redirect URI, and requested scopes -4. If the user approves (or the client was previously approved), redirects to the upstream provider using the fixed callback URL +4. If the user approves (or the client was previously approved), sets a consent binding cookie and redirects to the upstream provider using the fixed callback URL -This dual-PKCE approach maintains end-to-end security at both the client-to-proxy and proxy-to-provider layers. The consent step protects against confused deputy attacks by ensuring you explicitly approve each client before it can complete authorization. +This dual-PKCE approach maintains end-to-end security at both the client-to-proxy and proxy-to-provider layers. The consent step protects against confused deputy attacks by ensuring you explicitly approve each client before it can complete authorization, and the consent binding cookie ensures that only the browser that approved consent can complete the callback. ### Callback Phase After user authorization, the provider redirects back to the proxy's fixed callback URL. The proxy: -1. Exchanges the authorization code for tokens with the provider -2. Stores these tokens temporarily -3. Generates a new authorization code for the client -4. Redirects to the client's original dynamic callback URL +1. Verifies the consent binding cookie matches the transaction (rejecting requests from a different browser) +2. Exchanges the authorization code for tokens with the provider +3. Stores these tokens temporarily +4. Generates a new authorization code for the client +5. Redirects to the client's original dynamic callback URL ### Token Exchange Phase @@ -618,13 +621,15 @@ The OAuth proxy works by bridging DCR clients to traditional auth providers, whi #### Mitigation -FastMCP's OAuth proxy requires you to explicitly consent whenever any new or unrecognized client attempts to connect to your server. Before any authorization happens, you see a consent page showing the client's details, redirect URI, and requested scopes. This gives you the opportunity to review and deny suspicious requests. Once you approve a client, it's remembered so you don't see the consent page again for that client. The consent mechanism is implemented with CSRF tokens and cryptographically signed cookies to prevent tampering. +FastMCP's OAuth proxy defends against confused deputy attacks with two layers of protection: + +**Consent screen.** Before any authorization happens, you see a consent page showing the client's details, redirect URI, and requested scopes. This gives you the opportunity to review and deny suspicious requests. Once you approve a client, it's remembered so you don't see the consent page again for that client. The consent mechanism is implemented with CSRF tokens and cryptographically signed cookies to prevent tampering. ![](/assets/images/oauth-proxy-consent-screen.png) The consent page automatically displays your server's name, icon, and website URL, if available. These visual identifiers help users confirm they're authorizing the correct server. - +**Browser-session binding.** When you approve consent (or when a previously-approved client auto-approves), the proxy sets a cryptographically signed cookie that binds your browser session to the authorization flow. When the identity provider redirects back to the proxy's callback, the proxy verifies that this cookie is present and matches the expected transaction. A different browser — such as a victim who was sent the authorization URL by an attacker — won't have this cookie, and the callback will be rejected with a 403 error. This prevents the attack even when the identity provider skips the consent page for previously-authorized applications. **Learn more:** - [MCP Security Best Practices](https://modelcontextprotocol.io/specification/2025-06-18/basic/security_best_practices#confused-deputy-problem) - Official specification guidance diff --git a/loq.toml b/loq.toml index bbfe81827..f11d21871 100644 --- a/loq.toml +++ b/loq.toml @@ -52,7 +52,7 @@ max_lines = 1009 [[rules]] path = "tests/server/auth/test_oauth_consent_flow.py" -max_lines = 1056 +max_lines = 1274 [[rules]] path = "src/fastmcp/server/server.py" @@ -76,7 +76,7 @@ max_lines = 1584 [[rules]] path = "src/fastmcp/server/auth/oauth_proxy/proxy.py" -max_lines = 1740 +max_lines = 1796 [[rules]] path = "tests/server/test_dependencies.py" diff --git a/src/fastmcp/server/auth/oauth_proxy/consent.py b/src/fastmcp/server/auth/oauth_proxy/consent.py index 87b63d88f..0d7ccc32f 100644 --- a/src/fastmcp/server/auth/oauth_proxy/consent.py +++ b/src/fastmcp/server/auth/oauth_proxy/consent.py @@ -148,6 +148,103 @@ class ConsentMixin: path="/", ) + def _read_consent_bindings(self: OAuthProxy, request: Request) -> dict[str, str]: + """Read the consent binding map from the signed cookie. + + Returns a dict of {txn_id: consent_token} for all pending flows. + """ + cookie_name = self._cookie_name("MCP_CONSENT_BINDING") + raw = request.cookies.get(cookie_name) + # Only fall back to the non-__Host- name over plain HTTP. On HTTPS, + # __Host- enforces host-only scope; accepting the weaker name would + # bypass that guarantee. + if not raw and not self._is_https: + raw = request.cookies.get("__MCP_CONSENT_BINDING") + if not raw: + return {} + payload = self._verify_cookie(raw) + if not payload: + return {} + try: + data = json.loads(base64.b64decode(payload.encode()).decode()) + if isinstance(data, dict): + return {str(k): str(v) for k, v in data.items()} + except Exception: + logger.debug("Failed to decode consent binding cookie") + return {} + + def _write_consent_bindings( + self: OAuthProxy, + response: HTMLResponse | RedirectResponse, + bindings: dict[str, str], + ) -> None: + """Write the consent binding map to a signed cookie.""" + name = self._cookie_name("MCP_CONSENT_BINDING") + if not bindings: + response.set_cookie( + name, + "", + max_age=0, + secure=self._is_https, + httponly=True, + samesite="lax", + path="/", + ) + return + payload_bytes = json.dumps(bindings, separators=(",", ":")).encode() + payload_b64 = base64.b64encode(payload_bytes).decode() + signed_value = self._sign_cookie(payload_b64) + response.set_cookie( + name, + signed_value, + max_age=15 * 60, + secure=self._is_https, + httponly=True, + samesite="lax", + path="/", + ) + + def _set_consent_binding_cookie( + self: OAuthProxy, + request: Request, + response: HTMLResponse | RedirectResponse, + txn_id: str, + consent_token: str, + ) -> None: + """Add a consent binding entry for a transaction. + + This cookie binds the browser that approved consent to the IdP callback, + ensuring a different browser cannot complete the OAuth flow. Multiple + concurrent flows are supported by storing a map of txn_id → consent_token. + """ + bindings = self._read_consent_bindings(request) + bindings[txn_id] = consent_token + self._write_consent_bindings(response, bindings) + + def _clear_consent_binding_cookie( + self: OAuthProxy, + request: Request, + response: HTMLResponse | RedirectResponse, + txn_id: str, + ) -> None: + """Remove a specific consent binding entry after successful callback.""" + bindings = self._read_consent_bindings(request) + bindings.pop(txn_id, None) + self._write_consent_bindings(response, bindings) + + def _verify_consent_binding_cookie( + self: OAuthProxy, + request: Request, + txn_id: str, + expected_token: str, + ) -> bool: + """Verify the consent binding for a specific transaction.""" + bindings = self._read_consent_bindings(request) + actual = bindings.get(txn_id) + if not actual: + return False + return hmac.compare_digest(actual, expected_token) + def _build_upstream_authorize_url( self: OAuthProxy, txn_id: str, transaction: dict[str, Any] ) -> str: @@ -217,8 +314,13 @@ class ConsentMixin: denied = set(self._decode_list_cookie(request, "MCP_DENIED_CLIENTS")) if client_key in approved: + consent_token = secrets.token_urlsafe(32) + txn_model.consent_token = consent_token + await self._transaction_store.put(key=txn_id, value=txn_model, ttl=15 * 60) upstream_url = self._build_upstream_authorize_url(txn_id, txn) - return RedirectResponse(url=upstream_url, status_code=302) + response = RedirectResponse(url=upstream_url, status_code=302) + self._set_consent_binding_cookie(request, response, txn_id, consent_token) + return response if client_key in denied: callback_params = { @@ -331,6 +433,10 @@ class ConsentMixin: approved.add(client_key) approved_b64 = self._encode_list_cookie(sorted(approved)) + consent_token = secrets.token_urlsafe(32) + txn_model.consent_token = consent_token + await self._transaction_store.put(key=txn_id, value=txn_model, ttl=15 * 60) + upstream_url = self._build_upstream_authorize_url(txn_id, txn) response = RedirectResponse(url=upstream_url, status_code=302) self._set_list_cookie( @@ -340,6 +446,7 @@ class ConsentMixin: self._set_list_cookie( response, "MCP_CONSENT_STATE", self._encode_list_cookie([]), max_age=60 ) + self._set_consent_binding_cookie(request, response, txn_id, consent_token) return response elif action == "deny": diff --git a/src/fastmcp/server/auth/oauth_proxy/models.py b/src/fastmcp/server/auth/oauth_proxy/models.py index 7525b6a0b..b78decf17 100644 --- a/src/fastmcp/server/auth/oauth_proxy/models.py +++ b/src/fastmcp/server/auth/oauth_proxy/models.py @@ -56,6 +56,7 @@ class OAuthTransaction(BaseModel): proxy_code_verifier: str | None = None csrf_token: str | None = None csrf_expires_at: float | None = None + consent_token: str | None = None class ClientCode(BaseModel): diff --git a/src/fastmcp/server/auth/oauth_proxy/proxy.py b/src/fastmcp/server/auth/oauth_proxy/proxy.py index 927e4f1db..cca772db3 100644 --- a/src/fastmcp/server/auth/oauth_proxy/proxy.py +++ b/src/fastmcp/server/auth/oauth_proxy/proxy.py @@ -1639,6 +1639,38 @@ class OAuthProxy(OAuthProvider, ConsentMixin): error_message="Invalid or expired authorization transaction. Please try authenticating again.", ) return HTMLResponse(content=html_content, status_code=400) + # Verify consent binding cookie to prevent confused deputy attacks. + # When consent is enabled, the browser that approved consent receives + # a signed cookie. A different browser (e.g., a victim lured to the + # IdP URL) won't have this cookie and will be rejected. + if self._require_authorization_consent: + consent_token = transaction_model.consent_token + if not consent_token: + logger.error("Transaction %s missing consent_token", txn_id) + html_content = create_error_html( + error_title="Authorization Error", + error_message="Invalid authorization flow. Please try authenticating again.", + ) + return HTMLResponse(content=html_content, status_code=403) + + if not self._verify_consent_binding_cookie( + request, txn_id, consent_token + ): + logger.warning( + "Consent binding cookie missing or invalid for transaction %s " + "(possible confused deputy attack)", + txn_id, + ) + html_content = create_error_html( + error_title="Authorization Error", + error_message=( + "Authorization session mismatch. This can happen if you " + "followed a link from another person or your session expired. " + "Please try authenticating again." + ), + ) + return HTMLResponse(content=html_content, status_code=403) + transaction = transaction_model.model_dump() # Exchange IdP code for tokens (server-side) @@ -1751,7 +1783,9 @@ class OAuthProxy(OAuthProvider, ConsentMixin): logger.debug(f"Forwarding to client callback for transaction {txn_id}") - return RedirectResponse(url=client_callback_url, status_code=302) + response = RedirectResponse(url=client_callback_url, status_code=302) + self._clear_consent_binding_cookie(request, response, txn_id) + return response except Exception as e: logger.error("Error in IdP callback handler: %s", e, exc_info=True) diff --git a/tests/server/auth/test_oauth_consent_flow.py b/tests/server/auth/test_oauth_consent_flow.py index 8ff6fc404..25f53fe21 100644 --- a/tests/server/auth/test_oauth_consent_flow.py +++ b/tests/server/auth/test_oauth_consent_flow.py @@ -9,6 +9,7 @@ This test suite verifies: 6. Security headers (X-Frame-Options) are set correctly 7. Cookie signing and tampering detection 8. Auto-approve behavior with valid cookies +9. Consent binding cookie prevents confused deputy attacks (GHSA-rww4-4w9c-7733) """ import re @@ -1022,3 +1023,252 @@ class TestConsentCSPPolicy: assert html.escape(custom_csp, quote=True) in response.text # Default form-action should NOT be present (we're using custom) assert "form-action" not in response.text + + +class TestConsentBindingCookie: + """Tests for consent binding cookie that prevents confused deputy attacks. + + GHSA-rww4-4w9c-7733: Without browser-binding between consent approval and + the IdP callback, an attacker can intercept the upstream authorization URL + and send it to a victim whose browser completes the flow. + """ + + async def test_approve_sets_consent_binding_cookie(self, oauth_proxy_https): + """Approving consent must set a signed consent binding cookie.""" + txn_id, _ = await _start_flow( + oauth_proxy_https, "client-binding", "http://localhost:6001/callback" + ) + app = Starlette(routes=oauth_proxy_https.get_routes()) + with TestClient(app) as c: + consent = c.get(f"/consent?txn_id={txn_id}") + csrf = _extract_csrf(consent.text) + assert csrf + for k, v in consent.cookies.items(): + c.cookies.set(k, v) + r = c.post( + "/consent", + data={"action": "approve", "txn_id": txn_id, "csrf_token": csrf}, + follow_redirects=False, + ) + assert r.status_code in (302, 303) + set_cookie_header = r.headers.get("set-cookie", "") + assert "__Host-MCP_CONSENT_BINDING" in set_cookie_header + + async def test_auto_approve_sets_consent_binding_cookie(self, oauth_proxy_https): + """Auto-approve path (previously approved client) must also set the binding cookie.""" + client_id = "client-autobinding" + redirect = "http://localhost:6002/callback" + txn_id, _ = await _start_flow(oauth_proxy_https, client_id, redirect) + app = Starlette(routes=oauth_proxy_https.get_routes()) + with TestClient(app) as c: + # First: approve manually to get the approved cookie + consent = c.get(f"/consent?txn_id={txn_id}") + csrf = _extract_csrf(consent.text) + assert csrf + for k, v in consent.cookies.items(): + c.cookies.set(k, v) + r = c.post( + "/consent", + data={"action": "approve", "txn_id": txn_id, "csrf_token": csrf}, + follow_redirects=False, + ) + # Extract approved cookie + m = re.search( + r"__Host-MCP_APPROVED_CLIENTS=([^;]+)", + r.headers.get("set-cookie", ""), + ) + assert m + approved_cookie = m.group(1) + + # Second: start new flow, auto-approve should set binding cookie + new_txn, _ = await _start_flow(oauth_proxy_https, client_id, redirect) + c.cookies.set("__Host-MCP_APPROVED_CLIENTS", approved_cookie) + r2 = c.get(f"/consent?txn_id={new_txn}", follow_redirects=False) + assert r2.status_code in (302, 303) + set_cookie_header = r2.headers.get("set-cookie", "") + assert "__Host-MCP_CONSENT_BINDING" in set_cookie_header + + async def test_parallel_flows_do_not_interfere(self, oauth_proxy_https): + """Multiple concurrent consent flows in the same browser must not clobber each other. + + Uses two different clients so the second flow also shows a consent form + (auto-approve only kicks in for the same client+redirect pair). + """ + txn1, _ = await _start_flow( + oauth_proxy_https, "client-par-a", "http://localhost:6010/callback" + ) + txn2, _ = await _start_flow( + oauth_proxy_https, "client-par-b", "http://localhost:6011/callback" + ) + app = Starlette(routes=oauth_proxy_https.get_routes()) + with TestClient(app) as c: + # Approve first flow + consent1 = c.get(f"/consent?txn_id={txn1}") + csrf1 = _extract_csrf(consent1.text) + assert csrf1 + for k, v in consent1.cookies.items(): + c.cookies.set(k, v) + r1 = c.post( + "/consent", + data={"action": "approve", "txn_id": txn1, "csrf_token": csrf1}, + follow_redirects=False, + ) + assert r1.status_code in (302, 303) + for k, v in r1.cookies.items(): + c.cookies.set(k, v) + + # Approve second flow (different client, so consent form is shown) + consent2 = c.get(f"/consent?txn_id={txn2}") + csrf2 = _extract_csrf(consent2.text) + assert csrf2 + for k, v in consent2.cookies.items(): + c.cookies.set(k, v) + r2 = c.post( + "/consent", + data={"action": "approve", "txn_id": txn2, "csrf_token": csrf2}, + follow_redirects=False, + ) + assert r2.status_code in (302, 303) + for k, v in r2.cookies.items(): + c.cookies.set(k, v) + + # Both transactions should have consent tokens + txn1_model = await oauth_proxy_https._transaction_store.get(key=txn1) + txn2_model = await oauth_proxy_https._transaction_store.get(key=txn2) + assert txn1_model is not None and txn1_model.consent_token + assert txn2_model is not None and txn2_model.consent_token + + # First flow's callback should still work (cookie has both bindings) + r_cb1 = c.get( + f"/auth/callback?code=fake&state={txn1}", follow_redirects=False + ) + # Should NOT be 403 — the binding for txn1 should still be in the cookie. + # It will fail at token exchange (500) but not at consent verification. + assert r_cb1.status_code != 403 + + async def test_idp_callback_rejects_missing_consent_cookie(self, oauth_proxy_https): + """IdP callback must reject requests without the consent binding cookie. + + This is the core confused deputy scenario: a different browser (the victim) + hits the callback without the cookie that was set on the attacker's browser. + """ + txn_id, _ = await _start_flow( + oauth_proxy_https, "client-nocd", "http://localhost:6003/callback" + ) + # Manually set consent_token on transaction (simulating consent approval) + txn_model = await oauth_proxy_https._transaction_store.get(key=txn_id) + assert txn_model is not None + txn_model.consent_token = secrets.token_urlsafe(32) + await oauth_proxy_https._transaction_store.put( + key=txn_id, value=txn_model, ttl=15 * 60 + ) + + app = Starlette(routes=oauth_proxy_https.get_routes()) + with TestClient(app) as c: + # Hit callback WITHOUT the consent binding cookie + r = c.get( + f"/auth/callback?code=fake-code&state={txn_id}", + follow_redirects=False, + ) + assert r.status_code == 403 + assert ( + "session mismatch" in r.text.lower() or "Authorization Error" in r.text + ) + + async def test_idp_callback_rejects_wrong_consent_cookie(self, oauth_proxy_https): + """IdP callback must reject requests with a tampered consent binding cookie.""" + txn_id, _ = await _start_flow( + oauth_proxy_https, "client-wrongcd", "http://localhost:6004/callback" + ) + txn_model = await oauth_proxy_https._transaction_store.get(key=txn_id) + assert txn_model is not None + txn_model.consent_token = secrets.token_urlsafe(32) + await oauth_proxy_https._transaction_store.put( + key=txn_id, value=txn_model, ttl=15 * 60 + ) + + app = Starlette(routes=oauth_proxy_https.get_routes()) + with TestClient(app) as c: + # Set a wrong/tampered consent binding cookie + c.cookies.set("__Host-MCP_CONSENT_BINDING", "wrong-token.invalidsig") + r = c.get( + f"/auth/callback?code=fake-code&state={txn_id}", + follow_redirects=False, + ) + assert r.status_code == 403 + + async def test_idp_callback_rejects_missing_consent_token_on_transaction( + self, oauth_proxy_https + ): + """IdP callback must reject when transaction has no consent_token set.""" + txn_id, _ = await _start_flow( + oauth_proxy_https, "client-notxntoken", "http://localhost:6005/callback" + ) + # Transaction exists but consent_token is None (consent was never completed) + app = Starlette(routes=oauth_proxy_https.get_routes()) + with TestClient(app) as c: + r = c.get( + f"/auth/callback?code=fake-code&state={txn_id}", + follow_redirects=False, + ) + assert r.status_code == 403 + + async def test_consent_disabled_skips_binding_check(self): + """When require_authorization_consent=False, the binding check is skipped.""" + proxy = OAuthProxy( + upstream_authorization_endpoint="https://github.com/login/oauth/authorize", + upstream_token_endpoint="https://github.com/login/oauth/access_token", + upstream_client_id="client-id", + upstream_client_secret="client-secret", + token_verifier=_Verifier(), + base_url="https://myserver.example", + client_storage=MemoryStore(), + jwt_signing_key="test-secret", + require_authorization_consent=False, + ) + client_id = "client-noconsent" + redirect = "http://localhost:6006/callback" + await proxy.register_client( + OAuthClientInformationFull( + client_id=client_id, + client_secret="s", + redirect_uris=[AnyUrl(redirect)], + ) + ) + params = AuthorizationParams( + redirect_uri=AnyUrl(redirect), + redirect_uri_provided_explicitly=True, + state="st", + code_challenge="ch", + scopes=["read"], + ) + upstream_url = await proxy.authorize( + OAuthClientInformationFull( + client_id=client_id, + client_secret="s", + redirect_uris=[AnyUrl(redirect)], + ), + params, + ) + # With consent disabled, authorize returns upstream URL directly + assert upstream_url.startswith("https://github.com/login/oauth/authorize") + qs = parse_qs(urlparse(upstream_url).query) + txn_id = qs["state"][0] + + # The transaction should have no consent_token + txn_model = await proxy._transaction_store.get(key=txn_id) + assert txn_model is not None + assert txn_model.consent_token is None + + # IdP callback should NOT reject due to missing consent cookie + # (it will fail at token exchange, but not at the consent check) + app = Starlette(routes=proxy.get_routes()) + with TestClient(app) as c: + r = c.get( + f"/auth/callback?code=fake-code&state={txn_id}", + follow_redirects=False, + ) + # Should NOT be 403 (consent binding rejection) + # It will be 500 because the fake code can't be exchanged with GitHub, + # but that's fine — we're verifying the consent check was skipped. + assert r.status_code != 403