From 33c3acfc8741091b10a0d2cd1f08539f2ed7c08b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?d=20=F0=9F=94=B9?= Date: Sat, 14 Mar 2026 05:59:03 +0800 Subject: [PATCH] fix: use intent-based flag for OIDC scope patch in load_access_token (#3465) When OIDCProxy has verify_id_token=True and the IdP issues the same JWT for both access_token and id_token, the value-equality check `verification_token != upstream_token_set.access_token` evaluated to False, skipping the scope patch entirely. This left AccessToken.scopes empty, causing RequireAuthMiddleware to return 403 insufficient_scope. Replace the value-equality check with an intent-based virtual method `_uses_alternate_verification()` that OIDCProxy overrides to return `self._verify_id_token`. The base OAuthProxy returns False (preserving existing behavior for non-OIDC providers). Fixes #3461 Co-authored-by: voidborne-d --- src/fastmcp/server/auth/oauth_proxy/proxy.py | 27 +++++- src/fastmcp/server/auth/oidc_proxy.py | 9 ++ tests/server/auth/test_oidc_proxy_token.py | 94 ++++++++++++++++++++ 3 files changed, 126 insertions(+), 4 deletions(-) diff --git a/src/fastmcp/server/auth/oauth_proxy/proxy.py b/src/fastmcp/server/auth/oauth_proxy/proxy.py index 0a72ae9f9..281d194ce 100644 --- a/src/fastmcp/server/auth/oauth_proxy/proxy.py +++ b/src/fastmcp/server/auth/oauth_proxy/proxy.py @@ -1381,6 +1381,22 @@ class OAuthProxy(OAuthProvider, ConsentMixin): """ return upstream_token_set.access_token + def _uses_alternate_verification(self) -> bool: + """Whether this provider verifies a different token than the access token. + + When True, ``load_access_token`` patches the validated result with + the upstream access token, scopes, and expiry so that the returned + ``AccessToken`` reflects the access token rather than the + verification token. + + The default implementation compares token values, but subclasses + should override this to use an intent-based flag so the patch is + applied even when the verification token and access token happen to + carry the same value (e.g., some OIDC providers issue identical + JWTs for both). + """ + return False + async def load_access_token(self, token: str) -> AccessToken | None: # type: ignore[override] """Validate FastMCP JWT by swapping for upstream token. @@ -1429,11 +1445,14 @@ class OAuthProxy(OAuthProvider, ConsentMixin): logger.debug("Upstream token validation failed") return None - # When the verification token differs from the access token - # (e.g., id_token verification), ensure the returned AccessToken + # When alternate verification is in use (e.g., id_token + # verification in OIDCProxy), ensure the returned AccessToken # carries the upstream access token and its scopes, not the - # verification token's values. - if verification_token != upstream_token_set.access_token: + # verification token's values. We use an intent-based check + # rather than value equality because some IdPs issue identical + # JWTs for both access_token and id_token, which would cause + # the scope patch to be skipped even though it's needed. + if self._uses_alternate_verification(): validated = validated.model_copy( update={ "token": upstream_token_set.access_token, diff --git a/src/fastmcp/server/auth/oidc_proxy.py b/src/fastmcp/server/auth/oidc_proxy.py index 418bea34c..7081e05be 100644 --- a/src/fastmcp/server/auth/oidc_proxy.py +++ b/src/fastmcp/server/auth/oidc_proxy.py @@ -429,6 +429,15 @@ class OIDCProxy(OAuthProxy): return id_token return upstream_token_set.access_token + def _uses_alternate_verification(self) -> bool: + """Return True when id_token verification is enabled. + + This ensures ``load_access_token`` always patches the validated + result with upstream scopes, even when the IdP issues the same + JWT for both ``access_token`` and ``id_token``. + """ + return self._verify_id_token + def get_oidc_configuration( self, config_url: AnyHttpUrl, diff --git a/tests/server/auth/test_oidc_proxy_token.py b/tests/server/auth/test_oidc_proxy_token.py index 57083d50d..c261fb02e 100644 --- a/tests/server/auth/test_oidc_proxy_token.py +++ b/tests/server/auth/test_oidc_proxy_token.py @@ -329,3 +329,97 @@ class TestVerifyIdToken: "read", "write", ] + + +class TestUsesAlternateVerification: + """Tests for _uses_alternate_verification intent-based flag.""" + + def test_disabled_by_default(self, valid_oidc_configuration_dict): + """OIDCProxy without verify_id_token returns False.""" + with patch( + "fastmcp.server.auth.oidc_proxy.OIDCConfiguration.get_oidc_configuration" + ) as mock_get: + oidc_config = OIDCConfiguration.model_validate( + valid_oidc_configuration_dict + ) + mock_get.return_value = oidc_config + + proxy = OIDCProxy( + config_url=TEST_CONFIG_URL, + client_id=TEST_CLIENT_ID, + client_secret=TEST_CLIENT_SECRET, + base_url=TEST_BASE_URL, + jwt_signing_key="test-secret", + ) + + assert proxy._uses_alternate_verification() is False + + def test_enabled_with_verify_id_token(self, valid_oidc_configuration_dict): + """OIDCProxy with verify_id_token=True returns True.""" + with patch( + "fastmcp.server.auth.oidc_proxy.OIDCConfiguration.get_oidc_configuration" + ) as mock_get: + oidc_config = OIDCConfiguration.model_validate( + valid_oidc_configuration_dict + ) + mock_get.return_value = oidc_config + + proxy = OIDCProxy( + config_url=TEST_CONFIG_URL, + client_id=TEST_CLIENT_ID, + client_secret=TEST_CLIENT_SECRET, + base_url=TEST_BASE_URL, + jwt_signing_key="test-secret", + verify_id_token=True, + ) + + assert proxy._uses_alternate_verification() is True + + def test_scope_patch_applied_when_tokens_identical( + self, valid_oidc_configuration_dict + ): + """Regression test: scopes must be patched even when id_token and + access_token carry the same JWT value (fixes #3461).""" + with patch( + "fastmcp.server.auth.oidc_proxy.OIDCConfiguration.get_oidc_configuration" + ) as mock_get: + oidc_config = OIDCConfiguration.model_validate( + valid_oidc_configuration_dict + ) + mock_get.return_value = oidc_config + + proxy = OIDCProxy( + config_url=TEST_CONFIG_URL, + client_id=TEST_CLIENT_ID, + client_secret=TEST_CLIENT_SECRET, + base_url=TEST_BASE_URL, + jwt_signing_key="test-secret", + verify_id_token=True, + ) + + # Same JWT for both access_token and id_token — the scenario + # that triggered the bug. + same_jwt = "eyJhbGciOiJSUzI1NiJ9.identical-token" + token_set = UpstreamTokenSet( + upstream_token_id="test-id", + access_token=same_jwt, + refresh_token=None, + refresh_token_expires_at=None, + expires_at=9999999999.0, + token_type="Bearer", + scope="openid offline_access", + client_id="test-client", + created_at=1000000000.0, + raw_token_data={ + "access_token": same_jwt, + "id_token": same_jwt, + }, + ) + + # _uses_alternate_verification should be True regardless of + # token value equality + assert proxy._uses_alternate_verification() is True + # _get_verification_token returns the id_token (same value) + assert proxy._get_verification_token(token_set) == same_jwt + # The key point: even though the tokens are equal, the intent + # flag ensures load_access_token will patch scopes