feat: make upstream_client_secret optional in OAuthProxy (#3486)

* feat: make upstream_client_secret optional in OAuthProxy

Extract _create_upstream_oauth_client() factory method for subclass
override. Cookie signing falls back to JWT key material when no secret.

* fix: include client_id in revocation requests for public clients

* fix: use factory method for revocation auth
This commit is contained in:
Jeremiah Lowin 2026-03-15 11:14:56 -04:00 committed by GitHub
commit ea529f6a49
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
16 changed files with 205 additions and 45 deletions

View file

@ -100,8 +100,11 @@ mcp = FastMCP(name="My Server", auth=auth)
Client ID from your registered OAuth application
</ParamField>
<ParamField body="upstream_client_secret" type="str" required>
Client secret from your registered OAuth application
<ParamField body="upstream_client_secret" type="str | None">
Client secret from your registered OAuth application. Optional for PKCE public
clients or when using alternative credentials (e.g., managed identity client
assertions via a subclass). When omitted, `jwt_signing_key` must be provided
explicitly since it cannot be derived from the secret.
</ParamField>
<ParamField body="token_verifier" type="TokenVerifier" required>

View file

@ -70,8 +70,9 @@ mcp = FastMCP(name="My Server", auth=auth)
Client ID from your registered OAuth application
</ParamField>
<ParamField body="client_secret" type="str" required>
Client secret from your registered OAuth application
<ParamField body="client_secret" type="str | None">
Client secret from your registered OAuth application. Optional for PKCE public
clients. When omitted, `jwt_signing_key` must be provided.
</ParamField>
<ParamField body="base_url" type="AnyHttpUrl | str" required>

View file

@ -62,13 +62,23 @@ class ConsentMixin:
return f"__Host-{base_name}"
return f"__{base_name}"
def _cookie_signing_key(self: OAuthProxy) -> bytes:
"""Return the key used for HMAC-signing consent cookies.
Uses the upstream client secret when available, falling back to the
JWT signing key (which is always present OAuthProxy requires it
when no client secret is provided).
"""
if self._upstream_client_secret is not None:
return self._upstream_client_secret.get_secret_value().encode()
return self._jwt_signing_key
def _sign_cookie(self: OAuthProxy, payload: str) -> str:
"""Sign a cookie payload with HMAC-SHA256.
Returns: base64(payload).base64(signature)
"""
# Use upstream client secret as signing key
key = self._upstream_client_secret.get_secret_value().encode()
key = self._cookie_signing_key()
signature = hmac.new(key, payload.encode(), hashlib.sha256).digest()
signature_b64 = base64.b64encode(signature).decode()
return f"{payload}.{signature_b64}"
@ -84,7 +94,7 @@ class ConsentMixin:
payload, signature_b64 = signed_value.rsplit(".", 1)
# Verify signature
key = self._upstream_client_secret.get_secret_value().encode()
key = self._cookie_signing_key()
expected_sig = hmac.new(key, payload.encode(), hashlib.sha256).digest()
provided_sig = base64.b64decode(signature_b64.encode())

View file

@ -232,7 +232,7 @@ class OAuthProxy(OAuthProvider, ConsentMixin):
upstream_authorization_endpoint: str,
upstream_token_endpoint: str,
upstream_client_id: str,
upstream_client_secret: str,
upstream_client_secret: str | None = None,
upstream_revocation_endpoint: str | None = None,
# Token validation
token_verifier: TokenVerifier,
@ -270,7 +270,9 @@ class OAuthProxy(OAuthProvider, ConsentMixin):
upstream_authorization_endpoint: URL of upstream authorization endpoint
upstream_token_endpoint: URL of upstream token endpoint
upstream_client_id: Client ID registered with upstream server
upstream_client_secret: Client secret for upstream server
upstream_client_secret: Client secret for upstream server. Optional for
PKCE public clients or when using alternative credentials (e.g.,
managed identity). When omitted, jwt_signing_key must be provided.
upstream_revocation_endpoint: Optional upstream revocation endpoint
token_verifier: Token verifier for validating access tokens
base_url: Public URL of the server that exposes this FastMCP server; redirect path is
@ -348,8 +350,10 @@ class OAuthProxy(OAuthProvider, ConsentMixin):
self._upstream_authorization_endpoint: str = upstream_authorization_endpoint
self._upstream_token_endpoint: str = upstream_token_endpoint
self._upstream_client_id: str = upstream_client_id
self._upstream_client_secret: SecretStr = SecretStr(
secret_value=upstream_client_secret
self._upstream_client_secret: SecretStr | None = (
SecretStr(secret_value=upstream_client_secret)
if upstream_client_secret is not None
else None
)
self._upstream_revocation_endpoint: str | None = upstream_revocation_endpoint
self._default_scope_str: str = " ".join(self.required_scopes or [])
@ -405,6 +409,11 @@ class OAuthProxy(OAuthProvider, ConsentMixin):
)
if jwt_signing_key is None:
if upstream_client_secret is None:
raise ValueError(
"jwt_signing_key is required when upstream_client_secret is not provided. "
"The JWT signing key cannot be derived without a client secret."
)
jwt_signing_key = derive_jwt_key(
high_entropy_material=upstream_client_secret,
salt="fastmcp-jwt-signing-key",
@ -582,6 +591,29 @@ class OAuthProxy(OAuthProvider, ConsentMixin):
)
return self._jwt_issuer
# -------------------------------------------------------------------------
# Upstream OAuth Client
# -------------------------------------------------------------------------
def _create_upstream_oauth_client(self) -> AsyncOAuth2Client:
"""Create an OAuth2 client for communicating with the upstream IdP.
This is the single point for constructing the client used in token
exchange, refresh, and other upstream interactions. Subclasses can
override this to provide alternative authentication methods (e.g.,
managed-identity client assertions instead of a static client secret).
"""
return AsyncOAuth2Client(
client_id=self._upstream_client_id,
client_secret=(
self._upstream_client_secret.get_secret_value()
if self._upstream_client_secret is not None
else None
),
token_endpoint_auth_method=self._token_endpoint_auth_method,
timeout=HTTP_TIMEOUT_SECONDS,
)
# -------------------------------------------------------------------------
# PKCE Helper Methods
# -------------------------------------------------------------------------
@ -1190,12 +1222,7 @@ class OAuthProxy(OAuthProvider, ConsentMixin):
raise TokenError("invalid_grant", "Refresh not supported for this token")
# Refresh upstream token using authlib
oauth_client = AsyncOAuth2Client(
client_id=self._upstream_client_id,
client_secret=self._upstream_client_secret.get_secret_value(),
token_endpoint_auth_method=self._token_endpoint_auth_method,
timeout=HTTP_TIMEOUT_SECONDS,
)
oauth_client = self._create_upstream_oauth_client()
# Allow child classes to transform scopes before sending to upstream
# This enables provider-specific scope formatting (e.g., Azure prefixing)
@ -1501,13 +1528,26 @@ class OAuthProxy(OAuthProvider, ConsentMixin):
async with httpx.AsyncClient(
timeout=HTTP_TIMEOUT_SECONDS
) as http_client:
revocation_data: dict[str, str] = {"token": token.token}
request_kwargs: dict[str, Any] = {"data": revocation_data}
# Use the factory method when available (supports alternative auth like
# client assertions for managed identity), falling back to basic auth
# or client_id-only for public clients per RFC 7009
oauth_client = self._create_upstream_oauth_client()
if oauth_client.client_secret is not None:
# Client secret is available, use HTTP Basic auth
request_kwargs["auth"] = (
self._upstream_client_id,
oauth_client.client_secret,
)
else:
# No secret; public client must still identify itself per RFC 7009
revocation_data["client_id"] = self._upstream_client_id
await http_client.post(
self._upstream_revocation_endpoint,
data={"token": token.token},
auth=(
self._upstream_client_id,
self._upstream_client_secret.get_secret_value(),
),
**request_kwargs,
)
logger.debug("Successfully revoked token with upstream server")
except Exception as e:
@ -1732,12 +1772,7 @@ class OAuthProxy(OAuthProvider, ConsentMixin):
transaction = transaction_model.model_dump()
# Exchange IdP code for tokens (server-side)
oauth_client = AsyncOAuth2Client(
client_id=self._upstream_client_id,
client_secret=self._upstream_client_secret.get_secret_value(),
token_endpoint_auth_method=self._token_endpoint_auth_method,
timeout=HTTP_TIMEOUT_SECONDS,
)
oauth_client = self._create_upstream_oauth_client()
try:
idp_redirect_uri = (

View file

@ -204,7 +204,7 @@ class OIDCProxy(OAuthProxy):
strict: bool | None = None,
# Upstream server configuration
client_id: str,
client_secret: str,
client_secret: str | None = None,
audience: str | None = None,
timeout_seconds: int | None = None,
# Token verifier
@ -240,7 +240,9 @@ class OIDCProxy(OAuthProxy):
config_url: URL of upstream configuration
strict: Optional strict flag for the configuration
client_id: Client ID registered with upstream server
client_secret: Client secret for upstream server
client_secret: Client secret for upstream server. Optional for PKCE public
clients or when using alternative credentials. When omitted,
jwt_signing_key must be provided.
audience: Audience for upstream server
timeout_seconds: HTTP request timeout in seconds
token_verifier: Optional custom token verifier (e.g., IntrospectionTokenVerifier for opaque tokens).
@ -298,8 +300,12 @@ class OIDCProxy(OAuthProxy):
if not client_id:
raise ValueError("Missing required client id")
if not client_secret:
raise ValueError("Missing required client secret")
if not client_secret and not jwt_signing_key:
raise ValueError(
"Either client_secret or jwt_signing_key must be provided. "
"jwt_signing_key is required when client_secret is omitted "
"(e.g., for PKCE public clients)."
)
if not base_url:
raise ValueError("Missing required base URL")

View file

@ -96,7 +96,7 @@ class AzureProvider(OAuthProxy):
self,
*,
client_id: str,
client_secret: str,
client_secret: str | None = None,
tenant_id: str,
required_scopes: list[str],
base_url: str,
@ -116,7 +116,10 @@ class AzureProvider(OAuthProxy):
Args:
client_id: Azure application (client) ID from your App registration
client_secret: Azure client secret from your App registration
client_secret: Azure client secret from your App registration. Optional when
using alternative credentials (e.g., managed identity with a custom
_create_upstream_oauth_client override). When omitted, jwt_signing_key
must be provided.
tenant_id: Azure tenant ID (specific tenant GUID, "organizations", or "consumers")
identifier_uri: Optional Application ID URI for your custom API (defaults to api://{client_id}).
This URI is automatically prefixed to all required_scopes during initialization.
@ -504,13 +507,23 @@ class AzureProvider(OAuthProxy):
self._obo_credentials.move_to_end(key)
return self._obo_credentials[key]
credential = OnBehalfOfCredential(
tenant_id=self._tenant_id,
client_id=self._upstream_client_id,
client_secret=self._upstream_client_secret.get_secret_value(),
user_assertion=user_assertion,
authority=f"https://{self._base_authority}",
)
obo_kwargs: dict[str, Any] = {
"tenant_id": self._tenant_id,
"client_id": self._upstream_client_id,
"user_assertion": user_assertion,
"authority": f"https://{self._base_authority}",
}
if self._upstream_client_secret is not None:
obo_kwargs["client_secret"] = (
self._upstream_client_secret.get_secret_value()
)
else:
raise ValueError(
"OBO token exchange requires either a client_secret or a subclass "
"that overrides get_obo_credential() to provide alternative credentials "
"(e.g., client_assertion_func for managed identity)."
)
credential = OnBehalfOfCredential(**obo_kwargs)
self._obo_credentials[key] = credential
# Evict oldest if over capacity

View file

@ -219,7 +219,7 @@ class GoogleProvider(OAuthProxy):
self,
*,
client_id: str,
client_secret: str,
client_secret: str | None = None,
base_url: AnyHttpUrl | str,
issuer_url: AnyHttpUrl | str | None = None,
redirect_path: str | None = None,
@ -238,7 +238,9 @@ class GoogleProvider(OAuthProxy):
Args:
client_id: Google OAuth client ID (e.g., "123456789.apps.googleusercontent.com")
client_secret: Google OAuth client secret (e.g., "GOCSPX-abc123...")
client_secret: Google OAuth client secret (e.g., "GOCSPX-abc123...").
Optional for PKCE public clients (e.g., native apps). When omitted,
jwt_signing_key must be provided.
base_url: Public URL where OAuth endpoints will be accessible (includes any mount path)
issuer_url: Issuer URL for OAuth metadata (defaults to base_url). Use root-level URL
to avoid 404s during discovery when mounting under a path.

View file

@ -1,6 +1,8 @@
"""Tests for OAuth proxy initialization and configuration."""
import httpx
import pytest
from authlib.integrations.httpx_client import AsyncOAuth2Client
from key_value.aio.stores.memory import MemoryStore
from starlette.applications import Starlette
@ -29,6 +31,7 @@ class TestOAuthProxyInitialization:
)
assert proxy._upstream_token_endpoint == "https://auth.example.com/token"
assert proxy._upstream_client_id == "client-123"
assert proxy._upstream_client_secret is not None
assert proxy._upstream_client_secret.get_secret_value() == "secret-456"
assert str(proxy.base_url) == "https://api.example.com/"
@ -100,3 +103,79 @@ class TestOAuthProxyInitialization:
assert response.status_code == 200
metadata = response.json()
assert metadata.get("client_id_metadata_document_supported") is True
class TestOptionalClientSecret:
"""Tests for OAuthProxy without upstream_client_secret."""
def test_no_secret_requires_jwt_signing_key(self, jwt_verifier):
"""OAuthProxy requires jwt_signing_key when client_secret is omitted."""
with pytest.raises(ValueError, match="jwt_signing_key is required"):
OAuthProxy(
upstream_authorization_endpoint="https://auth.example.com/authorize",
upstream_token_endpoint="https://auth.example.com/token",
upstream_client_id="client-123",
token_verifier=jwt_verifier,
base_url="https://api.example.com",
client_storage=MemoryStore(),
)
def test_no_secret_with_jwt_key_succeeds(self, jwt_verifier):
"""OAuthProxy initializes successfully without client_secret when jwt_signing_key is given."""
proxy = OAuthProxy(
upstream_authorization_endpoint="https://auth.example.com/authorize",
upstream_token_endpoint="https://auth.example.com/token",
upstream_client_id="client-123",
token_verifier=jwt_verifier,
base_url="https://api.example.com",
jwt_signing_key=b"a" * 32,
client_storage=MemoryStore(),
)
assert proxy._upstream_client_secret is None
assert proxy._upstream_client_id == "client-123"
def test_factory_method_without_secret(self, jwt_verifier):
"""_create_upstream_oauth_client works when no secret is configured."""
proxy = OAuthProxy(
upstream_authorization_endpoint="https://auth.example.com/authorize",
upstream_token_endpoint="https://auth.example.com/token",
upstream_client_id="client-123",
token_verifier=jwt_verifier,
base_url="https://api.example.com",
jwt_signing_key=b"a" * 32,
client_storage=MemoryStore(),
)
client = proxy._create_upstream_oauth_client()
assert isinstance(client, AsyncOAuth2Client)
assert client.client_id == "client-123"
def test_factory_method_with_secret(self, jwt_verifier):
"""_create_upstream_oauth_client includes the secret when configured."""
proxy = OAuthProxy(
upstream_authorization_endpoint="https://auth.example.com/authorize",
upstream_token_endpoint="https://auth.example.com/token",
upstream_client_id="client-123",
upstream_client_secret="secret-456",
token_verifier=jwt_verifier,
base_url="https://api.example.com",
jwt_signing_key="test-secret",
client_storage=MemoryStore(),
)
client = proxy._create_upstream_oauth_client()
assert isinstance(client, AsyncOAuth2Client)
assert client.client_secret == "secret-456"
def test_consent_cookies_work_without_secret(self, jwt_verifier):
"""Cookie signing/verification works using JWT key when no secret is configured."""
proxy = OAuthProxy(
upstream_authorization_endpoint="https://auth.example.com/authorize",
upstream_token_endpoint="https://auth.example.com/token",
upstream_client_id="client-123",
token_verifier=jwt_verifier,
base_url="https://api.example.com",
jwt_signing_key=b"a" * 32,
client_storage=MemoryStore(),
)
signed = proxy._sign_cookie("test-payload")
assert proxy._verify_cookie(signed) == "test-payload"
assert proxy._verify_cookie("tampered.payload") is None

View file

@ -61,6 +61,7 @@ class TestAuth0Provider:
assert str(call_args[0][0]) == TEST_CONFIG_URL
assert provider._upstream_client_id == TEST_CLIENT_ID
assert provider._upstream_client_secret is not None
assert (
provider._upstream_client_secret.get_secret_value()
== TEST_CLIENT_SECRET

View file

@ -53,6 +53,7 @@ class TestAWSCognitoProvider:
# Check that the provider was initialized correctly
assert provider._upstream_client_id == "test_client"
assert provider._upstream_client_secret is not None
assert provider._upstream_client_secret.get_secret_value() == "test_secret"
assert (
str(provider.base_url) == "https://example.com/"

View file

@ -34,6 +34,7 @@ class TestAzureProvider:
)
assert provider._upstream_client_id == "12345678-1234-1234-1234-123456789012"
assert provider._upstream_client_secret is not None
assert provider._upstream_client_secret.get_secret_value() == "azure_secret_123"
assert str(provider.base_url) == "https://myserver.com/"
# Check tenant is in the endpoints

View file

@ -29,6 +29,7 @@ class TestDiscordProvider:
)
assert provider._upstream_client_id == "env_client_id"
assert provider._upstream_client_secret is not None
assert provider._upstream_client_secret.get_secret_value() == "GOCSPX-test123"
assert str(provider.base_url) == "https://myserver.com/"

View file

@ -35,6 +35,7 @@ class TestGitHubProvider:
# Check that the provider was initialized correctly
assert provider._upstream_client_id == "test_client"
assert provider._upstream_client_secret is not None
assert provider._upstream_client_secret.get_secret_value() == "test_secret"
assert (
str(provider.base_url) == "https://example.com/"

View file

@ -32,6 +32,7 @@ class TestGoogleProvider:
)
assert provider._upstream_client_id == "123456789.apps.googleusercontent.com"
assert provider._upstream_client_secret is not None
assert provider._upstream_client_secret.get_secret_value() == "GOCSPX-test123"
assert str(provider.base_url) == "https://myserver.com/"

View file

@ -39,6 +39,7 @@ class TestWorkOSProvider:
)
assert provider._upstream_client_id == "client_test123"
assert provider._upstream_client_secret is not None
assert provider._upstream_client_secret.get_secret_value() == "secret_test456"
assert str(provider.base_url) == "https://myserver.com/"

View file

@ -436,6 +436,7 @@ def validate_proxy(mock_get, proxy, oidc_config):
assert proxy._upstream_authorization_endpoint == TEST_AUTHORIZATION_ENDPOINT
assert proxy._upstream_token_endpoint == TEST_TOKEN_ENDPOINT
assert proxy._upstream_client_id == TEST_CLIENT_ID
assert proxy._upstream_client_secret is not None
assert proxy._upstream_client_secret.get_secret_value() == TEST_CLIENT_SECRET
assert str(proxy.base_url) == str(TEST_BASE_URL)
assert proxy.oidc_config == oidc_config
@ -623,11 +624,14 @@ class TestOIDCProxyInitialization:
)
mock_get.return_value = oidc_config
with pytest.raises(ValueError, match="Missing required client secret"):
with pytest.raises(
ValueError,
match="Either client_secret or jwt_signing_key must be provided",
):
OIDCProxy(
config_url=TEST_CONFIG_URL,
client_id=TEST_CLIENT_ID,
client_secret=None, # type: ignore
client_secret=None,
base_url=TEST_BASE_URL,
)