Add Azure OBO dependencies, auth token injection, and documentation#2918
Add Azure OBO dependencies, auth token injection, and documentation#2918
Conversation
marvin-context-protocol
Bot
added
enhancement
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds Azure On-Behalf-Of (OBO) token-exchange support and a token-claim extraction dependency. Implements AzureProvider.create_obo_credential using runtime-guarded Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Test Failure AnalysisSummary: Two tests in Root Cause: The tests Suggested Solution: Convert these two test functions to async and use File:
This pattern matches the existing Detailed AnalysisError from logs:Stack trace shows:File "C:\hostedtoolcache\windows\Python\3.10.11\x64\lib\asyncio\events.py", line 656
raise RuntimeError('There is no current event loop in thread %r.' % threading.current_thread().name)
RuntimeError: There is no current event loop in thread 'MainThread'.The issue is that in Python 3.10+ on Windows, Related Files
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d6f21fcddd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # Perform the OBO exchange | ||
| result = msal_app.acquire_token_on_behalf_of( | ||
| user_assertion=access_token.token, | ||
| scopes=self.scopes, |
There was a problem hiding this comment.
msal_app.acquire_token_on_behalf_of(...) is a synchronous network call (MSAL uses requests under the hood), but it is invoked directly inside an async def __aenter__. In an async server, this blocks the event loop for the duration of the token exchange. Under slow Azure responses or concurrent calls, this can stall unrelated requests and hurt throughput. Consider offloading the MSAL call with call_sync_fn_in_threadpool or otherwise ensuring the OBO exchange runs outside the event loop.
Useful? React with 👍 / 👎.
jlowin
changed the title
jlowin
force-pushed
the
azure-obo-dependencies
branch
from
1b7fce0 to
3cdbc8b
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
docs/integrations/azure.mdx (1)
281-456: Add a short verification step + expected outcome for OBO setup.
The new OBO section is solid, but the guide doesn’t include an explicit “verify” step or expected output, and several examples use placeholder IDs. The MDX guidelines call for verification steps and realistic data.As per coding guidelines, include a small verification section (e.g., call the tool and show a successful response) and replace placeholders with realistic sample UUIDs already used earlier in the doc.
src/fastmcp/server/auth/providers/azure.py (1)
369-424: Cache the MSAL app to enable token reuse across requests.
get_msal_app()creates a newTokenCacheandConfidentialClientApplicationon each call. MSAL best practices recommend reusing a singleConfidentialClientApplicationinstance to allow token caching and minimize instance discovery overhead. A simple lazy initialization cache on the provider will enable token reuse and improve OBO throughput.Suggested implementation
class AzureProvider(OAuthProxy): ... + _msal_app: ConfidentialClientApplication | None = None ... def get_msal_app(self) -> ConfidentialClientApplication: ... - return ConfidentialClientApplication( - client_id=self._upstream_client_id, - client_credential=self._upstream_client_secret.get_secret_value(), - authority=authority, - token_cache=TokenCache(), - ) + if self._msal_app is None: + self._msal_app = ConfidentialClientApplication( + client_id=self._upstream_client_id, + client_credential=self._upstream_client_secret.get_secret_value(), + authority=authority, + token_cache=TokenCache(), + ) + return self._msal_app
| ### Token Claims | ||
|
|
||
| When you need just one specific value from the token—like a user ID or tenant identifier—`TokenClaim()` extracts it directly without needing the full token object. | ||
|
|
||
| ```python | ||
| from fastmcp import FastMCP | ||
| from fastmcp.server.dependencies import TokenClaim | ||
|
|
||
| mcp = FastMCP("Demo") | ||
|
|
||
|
|
||
| @mcp.tool | ||
| async def add_expense( | ||
| amount: float, | ||
| user_id: str = TokenClaim("oid"), # Azure object ID | ||
| ) -> dict: | ||
| await db.insert({"user_id": user_id, "amount": amount}) | ||
| return {"status": "created", "user_id": user_id} | ||
| ``` | ||
|
|
||
| `TokenClaim()` raises a `RuntimeError` if the claim doesn't exist, listing available claims to help with debugging. |
There was a problem hiding this comment.
Make the TokenClaim example runnable and show failure behavior.
The snippet uses an undefined db and doesn’t show how missing-claim errors surface. The MDX guidelines require runnable examples with error handling and expected outcomes.
✅ Possible update
from fastmcp import FastMCP
from fastmcp.server.dependencies import TokenClaim
mcp = FastMCP("Demo")
+_expenses: list[dict[str, object]] = []
`@mcp.tool`
async def add_expense(
amount: float,
user_id: str = TokenClaim("oid"), # Azure object ID
) -> dict:
- await db.insert({"user_id": user_id, "amount": amount})
- return {"status": "created", "user_id": user_id}
+ # TokenClaim errors are raised before this function runs.
+ _expenses.append({"user_id": user_id, "amount": amount})
+ return {"status": "created", "user_id": user_id}
+
+# Expected result: {"status": "created", "user_id": "<oid>"}Add a brief note explaining that TokenClaim failures surface as runtime errors and how clients should handle them.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ### Token Claims | |
| When you need just one specific value from the token—like a user ID or tenant identifier—`TokenClaim()` extracts it directly without needing the full token object. | |
| ```python | |
| from fastmcp import FastMCP | |
| from fastmcp.server.dependencies import TokenClaim | |
| mcp = FastMCP("Demo") | |
| @mcp.tool | |
| async def add_expense( | |
| amount: float, | |
| user_id: str = TokenClaim("oid"), # Azure object ID | |
| ) -> dict: | |
| await db.insert({"user_id": user_id, "amount": amount}) | |
| return {"status": "created", "user_id": user_id} | |
| ``` | |
| `TokenClaim()` raises a `RuntimeError` if the claim doesn't exist, listing available claims to help with debugging. | |
| from fastmcp import FastMCP | |
| from fastmcp.server.dependencies import TokenClaim | |
| mcp = FastMCP("Demo") | |
| _expenses: list[dict[str, object]] = [] | |
| `@mcp.tool` | |
| async def add_expense( | |
| amount: float, | |
| user_id: str = TokenClaim("oid"), # Azure object ID | |
| ) -> dict: | |
| # TokenClaim errors are raised before this function runs. | |
| _expenses.append({"user_id": user_id, "amount": amount}) | |
| return {"status": "created", "user_id": user_id} | |
| # Expected result: {"status": "created", "user_id": "<oid>"} |
| def __init__(self, scopes: list[str]): | ||
| self.scopes = scopes | ||
|
|
There was a problem hiding this comment.
Add missing return type annotation.
_EntraOBOToken.__init__ needs -> None to satisfy the full-annotation rule for src/**/*.py.
✅ Suggested fix
- def __init__(self, scopes: list[str]):
+ def __init__(self, scopes: list[str]) -> None:
self.scopes = scopes📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def __init__(self, scopes: list[str]): | |
| self.scopes = scopes | |
| def __init__(self, scopes: list[str]) -> None: | |
| self.scopes = scopes | |
| class _TokenClaim(Dependency): # type: ignore[misc] | ||
| """Dependency that extracts a specific claim from the access token.""" | ||
|
|
||
| def __init__(self, claim_name: str): | ||
| self.claim_name = claim_name | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and read the specific lines from the file
head -n 1050 src/fastmcp/server/dependencies.py | tail -n 50Repository: jlowin/fastmcp
Length of output: 172
🏁 Script executed:
# Get the full context of the _TokenClaim class
sed -n '1030,1060p' src/fastmcp/server/dependencies.pyRepository: jlowin/fastmcp
Length of output: 939
🏁 Script executed:
# Check for other __init__ methods in this file that might be missing -> None
rg -n "def __init__\(" src/fastmcp/server/dependencies.py -A 1 | head -100Repository: jlowin/fastmcp
Length of output: 229
Add missing return type annotation.
The __init__ method lacks an explicit -> None return type annotation, which violates the full-annotation requirement for src/**/*.py.
✅ Suggested fix
- def __init__(self, claim_name: str):
+ def __init__(self, claim_name: str) -> None:
self.claim_name = claim_name
| base_url="http://localhost:8000", | ||
| required_scopes=["mcp-access"], # Your API scope | ||
| # Include Graph scopes for OBO | ||
| additional_authorize_scopes=[ |
There was a problem hiding this comment.
Is this still necessary, given admin consent? I don't believe I needed to do this in my implementation, since admin consent is given on the Entra app registration that has those scopes.
There was a problem hiding this comment.
I answered below, but admin consent isn't a requirement, nor should we make it one.
There was a problem hiding this comment.
But — additional_authorize_scopes isn't just for graph scopes, so we should remove that comment! 😊
There was a problem hiding this comment.
True, yes. I think it's a bit confusing since the section just before here describes how to grant admin consent. Perhaps a note here that if you have granted admin consent to the Entra app registration, additional_authorize_scopes may not be necessary?
|
Hello @jlowin Thank you for the PR and the nice FastMCP package. I wanted to get your inputs on a similar use case we’re working on. We had setup FastMCP in proxy mode and connecting it to 2–3 MCP servers that rely on Azure AD authentication and it worked as POC where it managed for one client scenario, but we are yet to make changes for multiple clients using the FastMCP proxy With this PR, do you think it would be possible to use FastMCP in proxy mode with authentication and then leverage the OBO flow for downstream MCP servers? Also right now FastMCP support storing the auth session the redis store, do we think we could extend this to even for OBO token One additional note: we ran into a small issue with refresh tokens when using OBO. Our understanding is that the MSAL library maintains an internal cache and automatically refreshes tokens, but in our case the token was not refreshed as expected. Thanks a lot! |
|
Great PR! The dependency injection pattern for OBO is really clean. For anyone looking for a Managed Identity-only approach (no client_secret at all), I wrote a guide that uses:
This works well for Azure Container Apps deployments where you want zero secrets. Guide: https://jaydoubleu.dev/guides/mcp-azure-oauth2-obo The approaches are complementary - this PR is great for scenarios where you have a client_secret, while the MI approach avoids secrets entirely. |
|
I think it'd be fantastic for MCP AzureProvider to support the Federated Identity Credentials, since then developers could have the best of both worlds. Last I looked, the underlying OAuthProvider class would need to be more flexible to allow for use of managed identity, it currently assumes everyone is using a secret-based approach. I think that could be done in a separate PR, however, as that aspect is orthogonal to the OBO functionality, from my perspective? Or @JayDoubleu do you think it'd require changes to this? (Disclosure: I'm from Microsoft) |
|
@mvgadagi Were you seeing a token expiry error? Are you using TokenCache() for the token_cache setting? After how much time did the token refresh issue happen? Was that with a deployed server in production? It'd be good to test this setup doesnt run into that issue. I haven't seen it yet with my similar OBO demo (https://github.com/Azure-Samples/python-mcp-demos/blob/main/servers/auth_entra_mcp.py#L110), but haven't battle tested in production. |
You're right. AzureProvider can't be used for MI because it requires client_secret for OAuth proxy functionality. Perhaps another PR addressing it would be fine, although there might be extra rework required to make Azure happy. I'm currently rewriting my accelerator against |
Thank you @pamelafox for the response And I will go through the reference you given and try to related to our implementation. Thanks a lot !! |
|
Hello everyone! I've taken some time to assess the AzureProvider again. First of all, great writeup @pamelafox! There are some details I want to address, and I hope it's okay to do it here, as I think I've uncovered a bug or two. From your blog:
This is not true. This is auth code flow under the hood, and the FastMCP server is responsible for constructing the redirect URI. In other words, we can send something like this: azure = AzureProvider(
client_id=settings.MCP_SERVER_CLIENT_ID,
client_secret=settings.MCP_SERVER_CLIENT_SECRET.get_secret_value(),
tenant_id=settings.TENANT_ID,
required_scopes=[f"user_impersonation"],
additional_authorize_scopes=[
"openid",
"profile",
"offline_access",
"api://<appID1>/user_impersonation", # <-- request scope for multiple clients
"api://<appID2>/user_impersonation" # <-- these are NOT admin consented
],
base_url="http://localhost:8000",
)and it would correctly prompt us for consent for 
I haven't really looked into a beautiful fix here, but something like this should work: ... code
# Advertise full scopes including OIDC (even though we only validate non-OIDC)
valid_scopes=parsed_required_scopes,
)
token_exchange_scopes = self._prefix_scopes_for_azure(
parsed_required_scopes or []
)
# Only add OIDC scopes from additional_authorize_scopes, not other API scopes
if parsed_additional_scopes:
oidc_scopes = [s for s in parsed_additional_scopes if s in OIDC_SCOPES]
token_exchange_scopes.extend(oidc_scopes)
# Deduplicate while preserving order
token_exchange_scopes = list(dict.fromkeys(token_exchange_scopes))
self._extra_token_params = {"scope": " ".join(token_exchange_scopes)}
logger.debug("Token exchange scopes: %s", token_exchange_scopes)This basically gets the prefixed @mcp.tool(name="change_state", description="Use this tool to change the state")
async def change_state(ctx: Context):
# token = get_access_token()
token1 = OnBehalfOfCredential(...).get_token(scope=api://<appID1>/user_impersonation)
token2 = OnBehalfOfCredential(...).get_token(scope=api://<appID1>/user_impersonation)
print(f"{token1=}")
print(f"{token2=}")
...As for this PR — I really like this! OBO is a very common use-case for Azure, but I find it really nice for MCPs in general — no reason to double-implement authorization — just use the upstream token. |
|
@JonasKs In that code, you have appID1, so wouldn't that be the case of a pre-known client ID? Or is app ID referring to something else? I think I'm a bit confused. |
|
@pamelafox AppId1 and 2 are other appregs, yes. But they are not admin consented! |
|
@JonasKs Ah, yes, for known client apps, the admin consent is not needed. I do not know how often that will be the case, but it's a good idea to make that clear in documentation. |
|
What do you mean with "known"? Example of an unknown app? |
|
I've raised a PR for the bug discovered above: #3013 |
@JonasKs @jlowin I too confirm the issue and tested with fastmcp==3.0.0b1, adding extra scopes fails with: Token exchange with identity provider failed: invalid_request: AADSTS28003: Provided value for the input parameter scope cannot be empty when requesting an access token using the provided authorization code. Please specify a valid scope. Probably the #3013 the fix is not released yet |
|
Correct. Install from the main branch and try again. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/fastmcp/server/auth/providers/azure.py (1)
408-416: Remove unusednoqadirective.The
# noqa: F401comment is unnecessary here per Ruff. The import is intentionally for availability checking, and the linter doesn't flag it.🧹 Suggested fix
def _require_azure_identity(feature: str) -> None: """Raise ImportError with install instructions if azure-identity is not available.""" try: - import azure.identity # noqa: F401 + import azure.identity except ImportError as e: raise ImportError( f"{feature} requires the `azure` extra. " "Install with: pip install 'fastmcp[azure]'" ) from e
|
Updated this PR to use |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/fastmcp/server/auth/providers/azure.py (1)
461-483: Reuse the shared azure-identity guard to keep error messaging consistent.The
create_obo_credentialmethod duplicates the import-guard logic from_require_azure_identity. Call_require_azure_identity("OBO token exchange")before importingOnBehalfOfCredentialto align with existing patterns in the codebase and maintain consistent error messaging across all OBO helpers.♻️ Suggested refactor
- try: - from azure.identity.aio import OnBehalfOfCredential - except ImportError as e: - raise ImportError( - "azure-identity is required for OBO token exchange. " - "Install with: pip install 'fastmcp[azure]'" - ) from e + _require_azure_identity("OBO token exchange") + from azure.identity.aio import OnBehalfOfCredential
- Add CurrentAccessToken and TokenClaim dependencies for extracting access tokens and claims in tool/resource/prompt handlers - Add EntraOBOToken and MSALApp dependencies for Azure On-Behalf-Of token exchange flows - Add get_msal_app() method to AzureProvider for MSAL client access - Add fastmcp[azure] extra with msal dependency - Split context.mdx into context.mdx and dependencies.mdx for clarity
- Use function pattern for MSALApp() consistent with other dependencies - Add TokenClaim to dependency-injection docs and exports - Remove duplicate CurrentAccessToken singleton (use function from main) - Fix test to use CurrentAccessToken() with parens
jlowin
force-pushed
the
azure-obo-dependencies
branch
from
34785ae to
ee8b1f5
Compare
|
Does OBO flow work on tools created using FastMCP.from_openapi( directly? i am getting invalid_token issue when used with openapi spec tools |
6 participants
Inspired by @pamelafox's blog post on OBO flow for Entra-based MCP servers.
FastMCP's OAuth Proxy pattern already enables On-Behalf-Of (OBO) flows with Azure/Entra - when a user authenticates, the upstream Azure token is stored and returned via
get_access_token().token. But actually using that token for OBO required boilerplate: configuring MSAL, callingacquire_token_on_behalf_of, handling errors.This PR adds dependency injection support and documentation that makes token access and OBO exchanges declarative.
Token Access (Any Auth Provider)
Extract the authenticated user's token or specific claims directly into your function parameters:
Azure OBO (requires
fastmcp[azure])OBO typically requires setting up MSAL's
ConfidentialClientApplicationwith your credentials, callingacquire_token_on_behalf_ofwith the user's assertion token, and handling the response. We provide dependencies that wrap all of this.EntraOBOTokenhandles the complete exchange - just declare what scopes you need:MSALAppprovides direct access to a pre-configured MSAL client for custom scenarios:Other Changes
fastmcp[azure]extra withmsaldependencycontext.mdxinto separate Context and Dependencies doc pagesRelated: #2447, #2495