Skip to content

fix: use intent-based flag for OIDC scope patch in load_access_token#3465

Merged
jlowin merged 1 commit intoPrefectHQ:mainfrom
voidborne-d:fix/oidc-proxy-scope-patch-3461
Mar 13, 2026
Merged

fix: use intent-based flag for OIDC scope patch in load_access_token#3465
jlowin merged 1 commit intoPrefectHQ:mainfrom
voidborne-d:fix/oidc-proxy-scope-patch-3461

Conversation

@voidborne-d
Copy link
Copy Markdown
Contributor

@voidborne-d voidborne-d commented Mar 12, 2026

Summary

Fixes #3461 — OIDCProxy with verify_id_token=True returns empty scopes when the IdP issues the same JWT for both access_token and id_token.

Root Cause

In OAuthProxy.load_access_token(), the condition:

if verification_token != upstream_token_set.access_token:

uses value equality to detect id_token verification. When an IdP (e.g., SAP Cloud Identity Services) returns the same JWT for both tokens, this evaluates to False, so the scope patch is never applied. AccessToken.scopes ends up [], causing 403 insufficient_scope.

Fix

Replace the value-equality check with an intent-based virtual method _uses_alternate_verification():

  • OAuthProxy (base): returns False — preserves existing behavior
  • OIDCProxy (override): returns self._verify_id_token — always patches scopes when id_token verification was explicitly requested, regardless of token values

Changes

File Change
proxy.py Add _uses_alternate_verification() base method; replace value-equality check
oidc_proxy.py Override _uses_alternate_verification() → self._verify_id_token
test_oidc_proxy_token.py 3 new tests: disabled default, enabled flag, identical-token regression

Design Notes

  • Non-breaking: base class returns False, so all existing OAuthProxy subclasses behave identically
  • Extensible: future subclasses that verify alternate tokens can override _uses_alternate_verification() without repeating the scope-patch logic
  • Minimal: 3-file change, no new dependencies

@voidborne-d
fix: use intent-based flag for OIDC scope patch in load_access_token
1a99daf 
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 PrefectHQ#3461
@marvin-context-protocol marvin-context-protocol Bot added bug Something isn't working. Reports of errors, unexpected behavior, or broken functionality. auth Related to authentication (Bearer, JWT, OAuth, WorkOS) for client or server. labels Mar 12, 2026
jlowin
jlowin approved these changes Mar 13, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@jlowin jlowin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@jlowin jlowin merged commit 33c3acf into PrefectHQ:main Mar 13, 2026
8 checks passed
@marvin-context-protocol
Copy link
Copy Markdown
Contributor

marvin-context-protocol Bot commented Mar 13, 2026

Test Failure Analysis

Summary: One newly added test in test_azure.py times out (>5s) on Windows Python 3.10, specifically test_prepare_scopes_for_upstream_refresh_filters_duplicate_additional_scopes.

Root Cause: The test creates a fresh AzureProvider with a string jwt_signing_key="test-secret", which triggers derive_jwt_key(low_entropy_material=..., iterations=1_000_000) — 1 million PBKDF2-SHA256 iterations — in OAuthProxy.__init__. By the time test 19 runs (after 18 identical AzureProvider constructions), the Windows CI runner appears to be under enough CPU load to push this one over the 5-second per-test timeout.

Additionally, the stack trace shows the timeout was fired exactly while Rich was calling isatty() on pytest's SpooledTemporaryFile capture buffer — a known slow path on Windows after the buffer accumulates many log messages from prior tests.

Suggested Solution: Replace the string jwt_signing_key with a pre-derived bytes value in the test_prepare_scopes_for_upstream_refresh_* tests to skip the PBKDF2 derivation entirely:

import base64

# At the top of the class or as a module-level constant
_TEST_SIGNING_KEY: bytes = base64.urlsafe_b64encode(b"x" * 32)

# In each test_prepare_scopes_for_upstream_refresh_* test, change:
#   jwt_signing_key="test-secret"
# to:
#   jwt_signing_key=_TEST_SIGNING_KEY

When jwt_signing_key is already bytes, OAuthProxy.__init__ skips PBKDF2 entirely (see proxy.py lines 399–417: the slow path only runs when the key is a str). This makes each test ~0.5s faster and eliminates the cumulative load that tips test 19 over the timeout threshold on Windows.

Alternatively, a shared @pytest.fixture(scope="class") that creates one AzureProvider and reuses it across all the scope-testing methods would reduce PBKDF2 calls from 10 to 1 for that test group.

Detailed Analysis

Workflow run: 23072166600
Job: Tests: Python 3.10 on windows-latest (only failing job)

Key log excerpt (timeout + stack trace):

tests\server\auth\providers\test_azure.py [18 dots]+++++ Timeout ++++++++++++++++++++++++++

Stack of MainThread:
  File "...test_azure.py", line 553, in test_prepare_scopes_for_upstream_refresh_filters_duplicate_additional_scopes
    provider = AzureProvider(
  File "...azure.py", line 224, in __init__
    super().__init__(
  File "...oauth_proxy/proxy.py", line 531, in __init__
    logger.debug(
  ...
  File "...rich/console.py", line 977, in is_terminal
    return False if isatty is None else isatty()
  File "C:\...\tempfile.py", line 483, in func_wrapper
    return func(*args, **kwargs)
+++++++++++++++++++++++++++++++++++ Timeout +++++++++++++++++++++++++++++++++++

Why test 19 and not 1–18?
Tests 1–18 all also create AzureProvider(jwt_signing_key="test-secret", ...) and each run PBKDF2 (~0.3–0.5s on a fast runner). At ~0.5s per test × 18 tests ≈ 9s of accumulated CPU work, the Windows runner is under thermal/load pressure when test 19 starts. Test 19's PBKDF2 then takes slightly longer than 5s, or the isatty() call (which is also hit in every test but cached differently by pytest's capture buffer state) finally becomes slow after the buffer has been written to many times.

Code path for PBKDF2 (src/fastmcp/server/auth/oauth_proxy/proxy.py, lines 399–414):

if isinstance(jwt_signing_key, str):
    jwt_signing_key = derive_jwt_key(
        low_entropy_material=jwt_signing_key,  # ← triggers PBKDF2 (1M iterations)
        salt="fastmcp-jwt-signing-key",
    )

KDF_ITERATIONS = 1_000_000 in src/fastmcp/server/auth/jwt_issuer.py:24.

Why Ubuntu doesn't fail: Linux isatty() is consistently fast, and Ubuntu runners tend to have better thermal headroom for repeated CPU-intensive operations.

Related Files
  • tests/server/auth/providers/test_azure.py:548 — failing test, newly added in this PR
  • src/fastmcp/server/auth/oauth_proxy/proxy.py:399–417 — string jwt_signing_key triggers PBKDF2
  • src/fastmcp/server/auth/jwt_issuer.py:24 — KDF_ITERATIONS = 1_000_000

🤖 Triage bot analysis — workflow run 23072166600

jlowin added a commit that referenced this pull request Mar 30, 2026
@jlowin @claude
@voidborne-d @marvin-context-protocol @dependabot @strawgate @Sumanshu-Nankana @ericrobinson-indeed @martimfasantos @mtthidoteu @saschabuehrle @hkc5 @nightcityblade @mhallo @zzstoatzz @shulkx @radoshi @shigechika @jer805 @pandego @4444J99 @mostafa6765
Publish docs for v3.2.0 (#3713)
3a9717e 
Co-authored-by: Claude Opus 4.6 <[email protected]>
Co-authored-by: Jeremiah Lowin <[email protected]>
Co-authored-by: Marvin Context Protocol <41898282+Marvin Context [email protected]>
Co-authored-by: voidborne-d <[email protected]>
Co-authored-by: marvin-context-protocol[bot] <225465937+marvin-context-protocol[bot]@users.noreply.github.com>
Co-authored-by: Claude <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: d 🔹 <[email protected]>
Co-authored-by: Jeremiah Lowin <[email protected]>
Co-authored-by: nightcityblade <[email protected]>
Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
Co-authored-by: Claude Sonnet 4.6 <[email protected]>
Co-authored-by: Bill Easton <[email protected]>
Co-authored-by: Sumanshu Nankana <[email protected]>
Co-authored-by: Eric Robinson <[email protected]>
Co-authored-by: Martim Santos <[email protected]>
Co-authored-by: d 🔹 <[email protected]>
Co-authored-by: Matthieu B <[email protected]>
Co-authored-by: Sascha Buehrle <[email protected]>
Co-authored-by: Hakancan <[email protected]>
Co-authored-by: nightcityblade <[email protected]>
Co-authored-by: Matt Hallowell <[email protected]>
Co-authored-by: nate nowack <[email protected]>
Co-authored-by: Bill Easton <[email protected]>
Co-authored-by: Marcus Shu <[email protected]>
Co-authored-by: Rushabh Doshi <[email protected]>
Co-authored-by: AIKAWA Shigechika <[email protected]>
Co-authored-by: Jeremy Simon <[email protected]>
Co-authored-by: Miguel Miranda Dias <[email protected]>
Co-authored-by: Anthony James Padavano <[email protected]>
Co-authored-by: Mostafa Kamal <[email protected]>
Fix auto-close MRE script posting comment without closing (#3386)
Fix WorkOS token scope verification bypass 🤖 Generated with Codex (#3407)
Fix initialize McpError fallthrough 🤖 Generated with Codex (#3413)
Fix transform arg collisions with passthrough params (#3431)
Fix get_* returning None when latest version is disabled (#3439)
Fix get_* returning None when latest version is disabled (#3421)
Fix server lifespan overlap teardown (#3415)
Fix $ref output schema object detection regression (#3420)
resolved annotations (#3429)
Fix async partial callables rejected by iscoroutinefunction (#3438)
Fix async partial callables rejected by iscoroutinefunction (#3423)
fix: add version to components (#3458)
fix: use intent-based flag for OIDC scope patch in load_access_token (#3465)
Fixes #3461
fix: normalize Google scope shorthands and surface valid_scopes (#3477)
fix: resolve ty 0.0.23 type-checking errors and bump pin (#3481)
fix: shield lifespan teardown from cancellation (#3480)
fix: forward custom_route endpoints from mounted servers (#3462)
fix updates _get_additional_http_routes() to traverse providers,
Fixes #3457
fix: remove hardcoded version from CLI help text (#3456)
fix: monty 0.0.8 compatibility, drop external_functions from constructor (#3468)
fix: task test teardown hanging 5s per test (#3499)
Closes #3498
fix: validate workspace path is a directory before cursor install (#3440)
Fixes #3426
fix: handle re.error from malformed URI templates in build_regex (#3501)
fix: reject empty/OIDC-only required_scopes in AzureProvider (#3503)
fix: restrict $ref resolution to local refs only (SSRF/LFI) (#3502)
fix warnings and timeouts (#3504)
close upgrade check issue when build passes (#3505)
Closes #3484
fix: URL-encode path params to prevent SSRF/path traversal (GHSA-vv7q-7jx5-f767) (#3507)
fix: prevent path traversal in skill download (#3493)
fix: prefer IdP-granted scopes over client-requested scopes in OAuthProxy (#3492)
fix: remove unrelated transform and http.py changes from PR scope
fix: remove forced follow_redirects from httpx_client_factory calls (#3496)
fix: stop passing follow_redirects to httpx_client_factory
fix: restore follow_redirects=True for custom httpx client factories
Closes #3509
fix: CSRF double-submit cookie check in consent flow (#3519)
fix: validate server names in install commands (#3522)
fix: use raw strings for regex in pytest.raises match (#3523)
fix: reject refresh tokens used as Bearer access tokens (#3524)
fix: route ResourcesAsTools/PromptsAsTools through server middleware (#3495)
fix: resolve Pyright "Module is not callable" on @tool, @resource, @prompt decorators (#3540)
fix: filter warnings by message in KEY_PREFIX test (#3549)
fix: suppress output schema for ToolResult subclass annotations (#3548)
fix: increase sleep duration in proxy cache tests (#3567)
fix: store absolute token expiry to prevent stale expires_in on reload (#3572)
fix: preserve tool properties named 'title' during schema compression (#3582)
Fix loopback redirect URI port matching per RFC 8252 §7.3 (#3589)
Fix app tool routing: visibility check and middleware propagation (#3591)
Fix query parameter serialization to respect OpenAPI explode/style settings (#3595)
Fix dev apps form: union types, textarea support, JSON parsing (#3597)
fix(google): replace deprecated /oauth2/v1/tokeninfo with /oauth2/v3/userinfo (#3603)
fix: resolve EntraOBOToken dependency injection through MultiAuth (#3609)
fix(docs): correct misleading stateless_http header (#3622)
fix: filesystem provider import machinery (#3626)
Closes #3625 (issues 2, 3, 6)
fix: recover StdioTransport after subprocess exits (#3630)
fix(server): preserve mounted tool task metadata (#3632)
fix: scope deprecation warning filter to FastMCPDeprecationWarning (#3649)
fix imports, add PrefabAppConfig (#3650)
fix: resolve CurrentFastMCP/ctx.fastmcp to child server in mounted background tasks (#3651)
Fix blocking docs issues: chart imports, Select API, Rx consistency (#3652)
closed by default (#3657)
Fix prompt caching middleware missing wrap/unwrap round-trip (#3666)
fix: serialize object query params per OpenAPI style/explode rules (#3662)
Fixes #2857
fix: HTTP request headers not accessible in background task workers (#3631)
fix: restore HTTP headers in worker execution path for background tasks (#3681)
fix: strip discriminator after dereferencing schemas (#3682)
fix: remove stale ty:ignore directives for ty 0.0.26 (#3684)
Fix docs gaps in app provider pages (#3690)
fix: dev apps log panel UX improvements (#3698)
fix dev server empty string args (#3700)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jlowin jlowin jlowin approved these changes

Assignees

No one assigned

Labels

auth Related to authentication (Bearer, JWT, OAuth, WorkOS) for client or server. bug Something isn't working. Reports of errors, unexpected behavior, or broken functionality.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

OIDCProxy with verify_id_token=True returns empty scopes when IdP issues identical access_token and id_token

2 participants

@voidborne-d @jlowin