Harden OAuth Proxy silent consent against AS-in-the-middle (#3960)

This commit is contained in:
Jeremiah Lowin 2026-04-17 16:39:50 -04:00 committed by GitHub
commit 7fb037f20c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
18 changed files with 413 additions and 107 deletions

View file

@ -297,14 +297,20 @@ auth = OAuthProxy(..., client_storage=MemoryStore())
</ParamField>
<ParamField body="require_authorization_consent" type="bool" default="True">
Whether to require user consent before authorizing MCP clients. When enabled (default), users see a consent screen that displays which client is requesting access, preventing [confused deputy attacks](https://modelcontextprotocol.io/specification/2025-06-18/basic/security_best_practices#confused-deputy-problem) by ensuring users explicitly approve new clients.
<ParamField body="require_authorization_consent" type='bool | Literal["remember", "external"]' default="True">
Consent screen behavior for authorization requests. The consent page displays which client is requesting access, defending against [confused deputy attacks](https://modelcontextprotocol.io/specification/2025-06-18/basic/security_best_practices#confused-deputy-problem) and AS-in-the-middle attacks (GHSA-6x8h-498w-gv8c) by requiring explicit user approval.
**Default behavior (True):**
Users see a consent screen on first authorization. Consent choices are remembered via signed cookies, so users only need to approve each client once. This protects against malicious clients impersonating the user.
**`True` (default) — always prompt:**
Users see the consent screen on every authorization. Strongest protection against AS-in-the-middle attacks where a malicious MCP server redirects the victim's browser into a legitimate proxy and relies on a previously-remembered approval to silently complete the flow.
**Disabling consent (False):**
Authorization proceeds directly to the upstream provider without user confirmation. Only use this for local development or testing environments where the security trade-off is acceptable.
**`"remember"` — silent consent on return:**
Users see the consent screen on first authorization; subsequent flows from the same browser for the same `(client_id, redirect_uri)` are silently approved via a signed cookie. Cross-site navigations (detected via `Sec-Fetch-Site`) still prompt, which preserves the mitigation against AS-in-the-middle while keeping the prompt-free UX for direct client-initiated flows. Weaker than `True` — use only if the prompt friction is a problem.
**`"external"` — delegate to upstream:**
Skip the built-in consent page; consent is collected by the upstream IdP or a custom login page referenced via `upstream_authorization_endpoint`. No security warning is logged.
**`False` — disable entirely:**
Authorization proceeds directly to the upstream provider without any consent UI. Logs a security warning. Only for local development or testing.
```python
# Development/testing only - skip consent screen
@ -312,6 +318,12 @@ auth = OAuthProxy(..., client_storage=MemoryStore())
...,
require_authorization_consent=False # ⚠️ Security warning: only for local/testing
)
# Convenience mode - silent consent on return visits (less safe than True)
auth = OAuthProxy(
...,
require_authorization_consent="remember",
)
```
<Warning>

View file

@ -198,8 +198,8 @@ auth = OIDCProxy(
</ParamField>
<ParamField body="require_authorization_consent" type="bool" default="True">
Whether to require user consent before authorizing MCP clients. When enabled (default), users see a consent screen that displays which client is requesting access. See [OAuthProxy documentation](/servers/auth/oauth-proxy#confused-deputy-attacks) for details on confused deputy attack protection.
<ParamField body="require_authorization_consent" type='bool | Literal["remember", "external"]' default="True">
Consent screen behavior for authorization requests. Accepts `True` (default; always prompt — strongest protection), `"remember"` (silent consent on return visits via signed cookie, gated by `Sec-Fetch-Site` to block AS-in-the-middle attacks), `"external"` (consent handled by upstream IdP or custom page), or `False` (disable entirely; local/testing only). See the [OAuthProxy documentation](/servers/auth/oauth-proxy) for full details on each mode and the security trade-offs.
</ParamField>
<ParamField body="consent_csp_policy" type="str | None" default="None">

View file

@ -287,14 +287,20 @@ auth = OAuthProxy(..., client_storage=MemoryStore())
</ParamField>
<ParamField body="require_authorization_consent" type="bool" default="True">
Whether to require user consent before authorizing MCP clients. When enabled (default), users see a consent screen that displays which client is requesting access, preventing [confused deputy attacks](https://modelcontextprotocol.io/specification/2025-06-18/basic/security_best_practices#confused-deputy-problem) by ensuring users explicitly approve new clients.
<ParamField body="require_authorization_consent" type='bool | Literal["remember", "external"]' default="True">
Consent screen behavior for authorization requests. The consent page displays which client is requesting access, defending against [confused deputy attacks](https://modelcontextprotocol.io/specification/2025-06-18/basic/security_best_practices#confused-deputy-problem) and AS-in-the-middle attacks (GHSA-6x8h-498w-gv8c) by requiring explicit user approval.
**Default behavior (True):**
Users see a consent screen on first authorization. Consent choices are remembered via signed cookies, so users only need to approve each client once. This protects against malicious clients impersonating the user.
**`True` (default) — always prompt:**
Users see the consent screen on every authorization. Strongest protection against AS-in-the-middle attacks where a malicious MCP server redirects the victim's browser into a legitimate proxy and relies on a previously-remembered approval to silently complete the flow.
**Disabling consent (False):**
Authorization proceeds directly to the upstream provider without user confirmation. Only use this for local development or testing environments where the security trade-off is acceptable.
**`"remember"` — silent consent on return:**
Users see the consent screen on first authorization; subsequent flows from the same browser for the same `(client_id, redirect_uri)` are silently approved via a signed cookie. Cross-site navigations (detected via `Sec-Fetch-Site`) still prompt, which preserves the mitigation against AS-in-the-middle while keeping the prompt-free UX for direct client-initiated flows. Weaker than `True` — use only if the prompt friction is a problem.
**`"external"` — delegate to upstream:**
Skip the built-in consent page; consent is collected by the upstream IdP or a custom login page referenced via `upstream_authorization_endpoint`. No security warning is logged.
**`False` — disable entirely:**
Authorization proceeds directly to the upstream provider without any consent UI. Logs a security warning. Only for local development or testing.
```python
# Development/testing only - skip consent screen
@ -302,6 +308,12 @@ auth = OAuthProxy(..., client_storage=MemoryStore())
...,
require_authorization_consent=False # ⚠️ Security warning: only for local/testing
)
# Convenience mode - silent consent on return visits (less safe than True)
auth = OAuthProxy(
...,
require_authorization_consent="remember",
)
```
<Warning>

View file

@ -198,8 +198,8 @@ auth = OIDCProxy(
</ParamField>
<ParamField body="require_authorization_consent" type="bool" default="True">
Whether to require user consent before authorizing MCP clients. When enabled (default), users see a consent screen that displays which client is requesting access. See [OAuthProxy documentation](/v2/servers/auth/oauth-proxy#confused-deputy-attacks) for details on confused deputy attack protection.
<ParamField body="require_authorization_consent" type='bool | Literal["remember", "external"]' default="True">
Consent screen behavior for authorization requests. Accepts `True` (default; always prompt — strongest protection), `"remember"` (silent consent on return visits via signed cookie, gated by `Sec-Fetch-Site` to block AS-in-the-middle attacks), `"external"` (consent handled by upstream IdP or custom page), or `False` (disable entirely; local/testing only). See the [OAuthProxy documentation](/v2/servers/auth/oauth-proxy) for full details on each mode and the security trade-offs.
</ParamField>
<ParamField body="consent_csp_policy" type="str | None" default="None">

View file

@ -329,28 +329,54 @@ class ConsentMixin:
txn = txn_model.model_dump()
client_key = self._make_client_key(txn["client_id"], txn["client_redirect_uri"])
approved = set(self._decode_list_cookie(request, "MCP_APPROVED_CLIENTS"))
denied = set(self._decode_list_cookie(request, "MCP_DENIED_CLIENTS"))
# Silent consent only fires in "remember" mode, and only when the
# request arrived via a safe navigation context. AS-in-the-middle
# attacks (GHSA-6x8h-498w-gv8c) surface as cross-site redirects from
# a third-party origin into /authorize; forcing the HTML prompt in
# that case preserves the consent-screen mitigation without blocking
# legitimate client-initiated flows (Sec-Fetch-Site: none).
if self._require_authorization_consent == "remember":
sec_fetch_site = request.headers.get("Sec-Fetch-Site")
# Fail closed on missing header: legacy clients degrade to the
# explicit prompt rather than silent approval.
silent_eligible = sec_fetch_site in ("same-origin", "same-site", "none")
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)
response = RedirectResponse(url=upstream_url, status_code=302)
self._set_consent_binding_cookie(request, response, txn_id, consent_token)
return response
if silent_eligible:
approved = set(
self._decode_list_cookie(request, "MCP_APPROVED_CLIENTS")
)
denied = set(self._decode_list_cookie(request, "MCP_DENIED_CLIENTS"))
if client_key in denied:
callback_params = {
"error": "access_denied",
"state": txn.get("client_state") or "",
}
sep = "&" if "?" in txn["client_redirect_uri"] else "?"
return RedirectResponse(
url=f"{txn['client_redirect_uri']}{sep}{urlencode(callback_params)}",
status_code=302,
)
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)
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 = {
"error": "access_denied",
"state": txn.get("client_state") or "",
}
sep = "&" if "?" in txn["client_redirect_uri"] else "?"
return RedirectResponse(
url=f"{txn['client_redirect_uri']}{sep}{urlencode(callback_params)}",
status_code=302,
)
else:
logger.info(
"Silent consent skipped for transaction %s: Sec-Fetch-Site=%r "
"(cross-site navigation; forcing explicit consent prompt)",
txn_id,
sec_fetch_site,
)
# Need consent: issue CSRF token and show HTML
csrf_token = secrets.token_urlsafe(32)
@ -465,23 +491,33 @@ class ConsentMixin:
client_key = self._make_client_key(txn["client_id"], txn["client_redirect_uri"])
if action == "approve":
approved = list(self._decode_list_cookie(request, "MCP_APPROVED_CLIENTS"))
if client_key in approved:
approved.remove(client_key)
approved.append(client_key)
approved = approved[-_MAX_REMEMBERED_CLIENTS:]
approved_b64 = self._encode_list_cookie(approved)
remember_mode = self._require_authorization_consent == "remember"
if action == "approve":
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(
response, "MCP_APPROVED_CLIENTS", approved_b64, max_age=365 * 24 * 3600
)
# Only persist the approval for future silent consent in "remember"
# mode; in the default "always" mode the cookie would never be read.
if remember_mode:
approved = list(
self._decode_list_cookie(request, "MCP_APPROVED_CLIENTS")
)
if client_key in approved:
approved.remove(client_key)
approved.append(client_key)
approved = approved[-_MAX_REMEMBERED_CLIENTS:]
self._set_list_cookie(
response,
"MCP_APPROVED_CLIENTS",
self._encode_list_cookie(approved),
max_age=365 * 24 * 3600,
)
# Clear CSRF cookie by setting empty short-lived value
self._set_list_cookie(
response, "MCP_CONSENT_STATE", self._encode_list_cookie([]), max_age=60
@ -490,13 +526,6 @@ class ConsentMixin:
return response
elif action == "deny":
denied = list(self._decode_list_cookie(request, "MCP_DENIED_CLIENTS"))
if client_key in denied:
denied.remove(client_key)
denied.append(client_key)
denied = denied[-_MAX_REMEMBERED_CLIENTS:]
denied_b64 = self._encode_list_cookie(denied)
callback_params = {
"error": "access_denied",
"state": txn.get("client_state") or "",
@ -506,9 +535,20 @@ class ConsentMixin:
f"{txn['client_redirect_uri']}{sep}{urlencode(callback_params)}"
)
response = RedirectResponse(url=client_callback_url, status_code=302)
self._set_list_cookie(
response, "MCP_DENIED_CLIENTS", denied_b64, max_age=365 * 24 * 3600
)
if remember_mode:
denied = list(self._decode_list_cookie(request, "MCP_DENIED_CLIENTS"))
if client_key in denied:
denied.remove(client_key)
denied.append(client_key)
denied = denied[-_MAX_REMEMBERED_CLIENTS:]
self._set_list_cookie(
response,
"MCP_DENIED_CLIENTS",
self._encode_list_cookie(denied),
max_age=365 * 24 * 3600,
)
self._set_list_cookie(
response, "MCP_CONSENT_STATE", self._encode_list_cookie([]), max_age=60
)

View file

@ -262,7 +262,7 @@ class OAuthProxy(OAuthProvider, ConsentMixin):
# JWT signing key
jwt_signing_key: str | bytes | None = None,
# Consent screen configuration
require_authorization_consent: bool | Literal["external"] = True,
require_authorization_consent: bool | Literal["remember", "external"] = True,
consent_csp_policy: str | None = None,
# Token expiry fallback
fallback_access_token_expiry_seconds: int | None = None,
@ -311,12 +311,19 @@ class OAuthProxy(OAuthProvider, ConsentMixin):
If bytes are provided, they will be used as-is.
If a string is provided, it will be derived into a 32-byte key using PBKDF2 (1.2M iterations).
If not provided, it will be derived from the upstream client secret using HKDF.
require_authorization_consent: Whether to require user consent before authorizing clients (default True).
When True, users see a consent screen before being redirected to the upstream IdP.
When False, authorization proceeds directly without user confirmation.
When "external", the built-in consent screen is skipped but no warning is
logged, indicating that consent is handled externally (e.g. by the upstream IdP).
SECURITY WARNING: Only set to False for local development or testing environments.
require_authorization_consent: Consent screen behavior (default True).
- True: always show the consent screen before redirecting to the
upstream IdP. Strongest protection against AS-in-the-middle attacks
(GHSA-6x8h-498w-gv8c).
- "remember": show the consent screen the first time, then silently
approve subsequent authorizations for the same (client_id,
redirect_uri) in the same browser. Cross-site navigations are
still prompted to block AS-in-the-middle attacks. Lower UX
friction, but weaker protection than True.
- "external": skip the built-in consent screen; consent is handled
externally (e.g. by the upstream IdP or a custom login page).
- False: skip consent entirely. SECURITY WARNING: only set to
False for local development or testing environments.
consent_csp_policy: Content Security Policy for the consent page.
If None (default), uses the built-in CSP policy with appropriate directives.
If empty string "", disables CSP entirely (no meta tag is rendered).
@ -397,7 +404,7 @@ class OAuthProxy(OAuthProvider, ConsentMixin):
self._token_endpoint_auth_method: str | None = token_endpoint_auth_method
# Consent screen configuration
self._require_authorization_consent: bool | Literal["external"] = (
self._require_authorization_consent: bool | Literal["remember", "external"] = (
require_authorization_consent
)
self._consent_csp_policy: str | None = consent_csp_policy
@ -405,6 +412,13 @@ class OAuthProxy(OAuthProvider, ConsentMixin):
logger.info(
"Built-in consent screen disabled; consent is handled externally."
)
elif require_authorization_consent == "remember":
logger.info(
"Consent screen in 'remember' mode: silent consent on return visits "
"for previously-approved clients (with Sec-Fetch-Site gating). "
"Set require_authorization_consent=True for strongest protection "
"against AS-in-the-middle attacks (GHSA-6x8h-498w-gv8c)."
)
elif not require_authorization_consent:
logger.warning(
"Authorization consent screen disabled - only use for local development or testing. "
@ -785,8 +799,10 @@ class OAuthProxy(OAuthProvider, ConsentMixin):
3. Return local /consent URL; browser visits consent first
4. Consent handler redirects to upstream IdP if approved/already approved
If consent is disabled (require_authorization_consent=False), skip the consent screen
and redirect directly to the upstream IdP.
If consent is disabled (require_authorization_consent=False or "external"),
skip the consent screen and redirect directly to the upstream IdP. In
"remember" mode, still route through /consent so the cookie lookup and
Sec-Fetch-Site gating can run.
"""
# Security check: validate client's requested resource matches this server
# This prevents tokens intended for one server from being used on another
@ -863,8 +879,10 @@ class OAuthProxy(OAuthProvider, ConsentMixin):
ttl=15 * 60, # Auto-expire after 15 minutes
)
# If consent is disabled or handled externally, skip consent screen
if self._require_authorization_consent is not True:
# If consent is disabled or handled externally, skip consent screen.
# "remember" mode still routes through /consent so cookie lookup and
# Sec-Fetch-Site gating can run.
if self._require_authorization_consent in (False, "external"):
upstream_url = self._build_upstream_authorize_url(
txn_id, transaction.model_dump()
)
@ -1971,8 +1989,10 @@ class OAuthProxy(OAuthProvider, ConsentMixin):
# 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 is True:
# IdP URL) won't have this cookie and will be rejected. "remember"
# mode still issues consent tokens (on silent approval or HTML
# approval), so both code paths need binding verification.
if self._require_authorization_consent in (True, "remember"):
consent_token = transaction_model.consent_token
if not consent_token:
logger.error("Transaction %s missing consent_token", txn_id)

View file

@ -225,7 +225,7 @@ class OIDCProxy(OAuthProxy):
# Token validation configuration
token_endpoint_auth_method: str | None = None,
# Consent screen configuration
require_authorization_consent: bool | Literal["external"] = True,
require_authorization_consent: bool | Literal["remember", "external"] = True,
consent_csp_policy: str | None = None,
forward_resource: bool = True,
# Extra parameters

View file

@ -72,7 +72,7 @@ class Auth0Provider(OIDCProxy):
allowed_client_redirect_uris: list[str] | None = None,
client_storage: AsyncKeyValue | None = None,
jwt_signing_key: str | bytes | None = None,
require_authorization_consent: bool | Literal["external"] = True,
require_authorization_consent: bool | Literal["remember", "external"] = True,
consent_csp_policy: str | None = None,
forward_resource: bool = True,
) -> None:

View file

@ -133,7 +133,7 @@ class AWSCognitoProvider(OIDCProxy):
allowed_client_redirect_uris: list[str] | None = None,
client_storage: AsyncKeyValue | None = None,
jwt_signing_key: str | bytes | None = None,
require_authorization_consent: bool | Literal["external"] = True,
require_authorization_consent: bool | Literal["remember", "external"] = True,
consent_csp_policy: str | None = None,
forward_resource: bool = True,
):

View file

@ -112,7 +112,7 @@ class AzureProvider(OAuthProxy):
allowed_client_redirect_uris: list[str] | None = None,
client_storage: AsyncKeyValue | None = None,
jwt_signing_key: str | bytes | None = None,
require_authorization_consent: bool | Literal["external"] = True,
require_authorization_consent: bool | Literal["remember", "external"] = True,
consent_csp_policy: str | None = None,
forward_resource: bool = True,
base_authority: str = "login.microsoftonline.com",

View file

@ -286,7 +286,7 @@ class ClerkProvider(OAuthProxy):
allowed_client_redirect_uris: list[str] | None = None,
client_storage: AsyncKeyValue | None = None,
jwt_signing_key: str | bytes | None = None,
require_authorization_consent: bool | Literal["external"] = True,
require_authorization_consent: bool | Literal["remember", "external"] = True,
consent_csp_policy: str | None = None,
forward_resource: bool = True,
extra_authorize_params: dict[str, str] | None = None,

View file

@ -204,7 +204,7 @@ class DiscordProvider(OAuthProxy):
allowed_client_redirect_uris: list[str] | None = None,
client_storage: AsyncKeyValue | None = None,
jwt_signing_key: str | bytes | None = None,
require_authorization_consent: bool | Literal["external"] = True,
require_authorization_consent: bool | Literal["remember", "external"] = True,
consent_csp_policy: str | None = None,
forward_resource: bool = True,
http_client: httpx.AsyncClient | None = None,

View file

@ -219,7 +219,7 @@ class GitHubProvider(OAuthProxy):
allowed_client_redirect_uris: list[str] | None = None,
client_storage: AsyncKeyValue | None = None,
jwt_signing_key: str | bytes | None = None,
require_authorization_consent: bool | Literal["external"] = True,
require_authorization_consent: bool | Literal["remember", "external"] = True,
consent_csp_policy: str | None = None,
forward_resource: bool = True,
http_client: httpx.AsyncClient | None = None,

View file

@ -244,7 +244,7 @@ class GoogleProvider(OAuthProxy):
allowed_client_redirect_uris: list[str] | None = None,
client_storage: AsyncKeyValue | None = None,
jwt_signing_key: str | bytes | None = None,
require_authorization_consent: bool | Literal["external"] = True,
require_authorization_consent: bool | Literal["remember", "external"] = True,
consent_csp_policy: str | None = None,
forward_resource: bool = True,
extra_authorize_params: dict[str, str] | None = None,

View file

@ -131,7 +131,7 @@ class OCIProvider(OIDCProxy):
allowed_client_redirect_uris: list[str] | None = None,
client_storage: AsyncKeyValue | None = None,
jwt_signing_key: str | bytes | None = None,
require_authorization_consent: bool | Literal["external"] = True,
require_authorization_consent: bool | Literal["remember", "external"] = True,
consent_csp_policy: str | None = None,
forward_resource: bool = True,
) -> None:

View file

@ -172,7 +172,7 @@ class WorkOSProvider(OAuthProxy):
allowed_client_redirect_uris: list[str] | None = None,
client_storage: AsyncKeyValue | None = None,
jwt_signing_key: str | bytes | None = None,
require_authorization_consent: bool | Literal["external"] = True,
require_authorization_consent: bool | Literal["remember", "external"] = True,
consent_csp_policy: str | None = None,
forward_resource: bool = True,
http_client: httpx.AsyncClient | None = None,

View file

@ -95,6 +95,22 @@ def oauth_proxy_https():
)
@pytest.fixture
def oauth_proxy_https_remember():
"""OAuthProxy in 'remember' mode for silent-consent cookie tests."""
return 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="remember",
)
async def _start_flow(
proxy: OAuthProxy, client_id: str, redirect: str
) -> tuple[str, str]:
@ -587,11 +603,15 @@ class TestConsentSecurity:
assert r.status_code == 200
assert r.headers.get("X-Frame-Options") == "DENY"
async def test_deny_sets_cookie_and_redirects_with_error(self, oauth_proxy_https):
"""Verify denying consent sets signed cookie and redirects with error."""
async def test_deny_sets_cookie_and_redirects_with_error(
self, oauth_proxy_https_remember
):
"""Verify denying consent in 'remember' mode sets signed cookie and redirects with error."""
client_redirect = "http://localhost:5002/callback"
txn_id, _ = await _start_flow(oauth_proxy_https, "client-b", client_redirect)
app = Starlette(routes=oauth_proxy_https.get_routes())
txn_id, _ = await _start_flow(
oauth_proxy_https_remember, "client-b", client_redirect
)
app = Starlette(routes=oauth_proxy_https_remember.get_routes())
with TestClient(app) as c:
consent = c.get(f"/consent?txn_id={txn_id}")
csrf = _extract_csrf(consent.text)
@ -617,13 +637,13 @@ class TestConsentSecurity:
)
async def test_approve_sets_cookie_and_redirects_to_upstream(
self, oauth_proxy_https
self, oauth_proxy_https_remember
):
"""Verify approving consent sets signed cookie and redirects to upstream."""
"""Verify approving consent in 'remember' mode sets signed cookie and redirects to upstream."""
txn_id, _ = await _start_flow(
oauth_proxy_https, "client-c", "http://localhost:5003/callback"
oauth_proxy_https_remember, "client-c", "http://localhost:5003/callback"
)
app = Starlette(routes=oauth_proxy_https.get_routes())
app = Starlette(routes=oauth_proxy_https_remember.get_routes())
with TestClient(app) as c:
consent = c.get(f"/consent?txn_id={txn_id}")
csrf = _extract_csrf(consent.text)
@ -643,29 +663,33 @@ class TestConsentSecurity:
set_cookie = ";\n".join(r.headers.get("set-cookie", "").splitlines())
assert "__Host-MCP_APPROVED_CLIENTS" in set_cookie
async def test_tampered_cookie_is_ignored(self, oauth_proxy_https):
async def test_tampered_cookie_is_ignored(self, oauth_proxy_https_remember):
"""Verify tampered approval cookie is ignored and consent page shown."""
txn_id, _ = await _start_flow(
oauth_proxy_https, "client-d", "http://localhost:5004/callback"
oauth_proxy_https_remember, "client-d", "http://localhost:5004/callback"
)
app = Starlette(routes=oauth_proxy_https.get_routes())
app = Starlette(routes=oauth_proxy_https_remember.get_routes())
with TestClient(app) as c:
# Create a tampered cookie (invalid signature)
# Value format: payload.signature; using wrong signature to force failure
tampered_value = "W10=.invalidsig"
c.cookies.set("__Host-MCP_APPROVED_CLIENTS", tampered_value)
r = c.get(f"/consent?txn_id={txn_id}", follow_redirects=False)
r = c.get(
f"/consent?txn_id={txn_id}",
headers={"Sec-Fetch-Site": "none"},
follow_redirects=False,
)
# Should not auto-redirect to upstream; should show consent page
assert r.status_code == 200
# httpx returns a URL object; compare path or stringify
assert urlparse(str(r.request.url)).path == "/consent"
async def test_autoapprove_cookie_skips_consent(self, oauth_proxy_https):
"""Verify valid approval cookie auto-approves and redirects to upstream."""
async def test_autoapprove_cookie_skips_consent(self, oauth_proxy_https_remember):
"""Verify valid approval cookie auto-approves and redirects to upstream in 'remember' mode."""
client_id = "client-e"
redirect = "http://localhost:5005/callback"
txn_id, _ = await _start_flow(oauth_proxy_https, client_id, redirect)
app = Starlette(routes=oauth_proxy_https.get_routes())
txn_id, _ = await _start_flow(oauth_proxy_https_remember, client_id, redirect)
app = Starlette(routes=oauth_proxy_https_remember.get_routes())
with TestClient(app) as c:
# Approve once to set approved cookie
consent = c.get(f"/consent?txn_id={txn_id}")
@ -684,15 +708,188 @@ class TestConsentSecurity:
# Extract approved cookie value
set_cookie = ";\n".join(r.headers.get("set-cookie", "").splitlines())
m = re.search(r"__Host-MCP_APPROVED_CLIENTS=([^;]+)", set_cookie)
assert m, "approved cookie should be set"
assert m, "approved cookie should be set in 'remember' mode"
approved_cookie = m.group(1)
# Start a new flow for the same client and redirect
new_txn, _ = await _start_flow(oauth_proxy_https, client_id, redirect)
new_txn, _ = await _start_flow(
oauth_proxy_https_remember, client_id, redirect
)
# Should auto-redirect to upstream when visiting consent due to cookie
# (requires a safe Sec-Fetch-Site value — "none" simulates a direct
# client-initiated navigation like an MCP host opening the browser).
c.cookies.set("__Host-MCP_APPROVED_CLIENTS", approved_cookie)
r2 = c.get(f"/consent?txn_id={new_txn}", follow_redirects=False)
r2 = c.get(
f"/consent?txn_id={new_txn}",
headers={"Sec-Fetch-Site": "none"},
follow_redirects=False,
)
assert r2.status_code in (302, 303)
assert r2.headers.get("location", "").startswith(
"https://github.com/login/oauth/authorize"
)
class TestConsentModes:
"""Tests for the three consent modes: True (always), 'remember' (silent), 'external'.
Covers the default-behavior change in GHSA-6x8h-498w-gv8c: True no longer
silently approves return visits, and 'remember' mode gates silent consent
on Sec-Fetch-Site to block AS-in-the-middle attacks.
"""
async def test_default_mode_does_not_set_approval_cookie(self, oauth_proxy_https):
"""Approving in default (True) mode must NOT set a long-lived approval cookie.
In 'always prompt' mode there's no reason to remember approvals, and
writing the cookie would only create a no-op attack surface.
"""
txn_id, _ = await _start_flow(
oauth_proxy_https, "client-default", "http://localhost:7001/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 = ";\n".join(r.headers.get("set-cookie", "").splitlines())
# Consent binding cookie still set for the in-flight transaction
assert "__Host-MCP_CONSENT_BINDING" in set_cookie
# But no remembered-approval cookie
assert "MCP_APPROVED_CLIENTS" not in set_cookie
async def test_default_mode_ignores_existing_approval_cookie(
self, oauth_proxy_https_remember, oauth_proxy_https
):
"""A valid approval cookie is ignored in default (True) mode — always prompts.
Uses the 'remember' proxy to generate a legitimately-signed cookie, then
replays it against a default-mode proxy (same jwt_signing_key) to confirm
the consent page is still shown.
"""
# Both fixtures use the same signing key, so cookies generated by one
# verify against the other.
client_id = "client-xmode"
redirect = "http://localhost:7002/callback"
txn_id, _ = await _start_flow(oauth_proxy_https_remember, client_id, redirect)
app_remember = Starlette(routes=oauth_proxy_https_remember.get_routes())
with TestClient(app_remember) 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,
)
set_cookie_remember = ";\n".join(
r.headers.get("set-cookie", "").splitlines()
)
m = re.search(r"__Host-MCP_APPROVED_CLIENTS=([^;]+)", set_cookie_remember)
assert m, "remember-mode proxy should have set approval cookie"
approved_cookie = m.group(1)
# Now replay against the default-mode proxy for the same client/redirect.
new_txn, _ = await _start_flow(oauth_proxy_https, client_id, redirect)
app_default = Starlette(routes=oauth_proxy_https.get_routes())
with TestClient(app_default) as c:
c.cookies.set("__Host-MCP_APPROVED_CLIENTS", approved_cookie)
r = c.get(
f"/consent?txn_id={new_txn}",
headers={"Sec-Fetch-Site": "none"},
follow_redirects=False,
)
# Default mode: HTML consent page, no silent redirect to upstream.
assert r.status_code == 200
assert "text/html" in r.headers.get("content-type", "")
async def test_remember_mode_cross_site_forces_prompt(
self, oauth_proxy_https_remember
):
"""Sec-Fetch-Site=cross-site must skip silent consent even with valid cookie.
Blocks the GHSA-6x8h-498w-gv8c AS-in-the-middle scenario: attacker
redirects the victim's browser to /authorize from a third-party origin,
which browsers mark cross-site. Legit local-host flows are Sec-Fetch-Site=none.
"""
client_id = "client-cross"
redirect = "http://localhost:7003/callback"
txn_id, _ = await _start_flow(oauth_proxy_https_remember, client_id, redirect)
app = Starlette(routes=oauth_proxy_https_remember.get_routes())
with TestClient(app) as c:
# Seed a valid approval cookie via the normal consent flow.
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,
)
set_cookie = ";\n".join(r.headers.get("set-cookie", "").splitlines())
m = re.search(r"__Host-MCP_APPROVED_CLIENTS=([^;]+)", set_cookie)
assert m
approved_cookie = m.group(1)
# Start a new flow and simulate a cross-site navigation.
new_txn, _ = await _start_flow(
oauth_proxy_https_remember, client_id, redirect
)
c.cookies.set("__Host-MCP_APPROVED_CLIENTS", approved_cookie)
r2 = c.get(
f"/consent?txn_id={new_txn}",
headers={"Sec-Fetch-Site": "cross-site"},
follow_redirects=False,
)
# Silent consent skipped; HTML prompt shown.
assert r2.status_code == 200
assert "text/html" in r2.headers.get("content-type", "")
async def test_remember_mode_missing_sec_fetch_site_fails_closed(
self, oauth_proxy_https_remember
):
"""Missing Sec-Fetch-Site header must fall back to the explicit prompt.
Legacy clients that don't send the header degrade to the safer behavior
rather than granting silent consent.
"""
client_id = "client-nofetch"
redirect = "http://localhost:7004/callback"
txn_id, _ = await _start_flow(oauth_proxy_https_remember, client_id, redirect)
app = Starlette(routes=oauth_proxy_https_remember.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,
)
set_cookie = ";\n".join(r.headers.get("set-cookie", "").splitlines())
m = re.search(r"__Host-MCP_APPROVED_CLIENTS=([^;]+)", set_cookie)
assert m
approved_cookie = m.group(1)
new_txn, _ = await _start_flow(
oauth_proxy_https_remember, client_id, redirect
)
c.cookies.set("__Host-MCP_APPROVED_CLIENTS", approved_cookie)
# No Sec-Fetch-Site header at all.
r2 = c.get(f"/consent?txn_id={new_txn}", follow_redirects=False)
assert r2.status_code == 200
assert "text/html" in r2.headers.get("content-type", "")

View file

@ -48,6 +48,22 @@ def oauth_proxy_https():
)
@pytest.fixture
def oauth_proxy_https_remember():
"""OAuthProxy in 'remember' mode for silent-consent cookie tests."""
return 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="remember",
)
async def _start_flow(
proxy: OAuthProxy, client_id: str, redirect: str
) -> tuple[str, str]:
@ -476,12 +492,14 @@ class TestConsentBindingCookie:
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."""
async def test_auto_approve_sets_consent_binding_cookie(
self, oauth_proxy_https_remember
):
"""Auto-approve path (previously approved client in 'remember' mode) 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())
txn_id, _ = await _start_flow(oauth_proxy_https_remember, client_id, redirect)
app = Starlette(routes=oauth_proxy_https_remember.get_routes())
with TestClient(app) as c:
# First: approve manually to get the approved cookie
consent = c.get(f"/consent?txn_id={txn_id}")
@ -503,9 +521,16 @@ class TestConsentBindingCookie:
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)
# (Sec-Fetch-Site=none simulates a direct client-initiated navigation.)
new_txn, _ = await _start_flow(
oauth_proxy_https_remember, client_id, redirect
)
c.cookies.set("__Host-MCP_APPROVED_CLIENTS", approved_cookie)
r2 = c.get(f"/consent?txn_id={new_txn}", follow_redirects=False)
r2 = c.get(
f"/consent?txn_id={new_txn}",
headers={"Sec-Fetch-Site": "none"},
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