Skip to content

Fix/simple auth example#2846

Open
nikodemas wants to merge 2 commits into
modelcontextprotocol:mainfrom
nikodemas:fix/simple-auth-example
Open

Fix/simple auth example#2846
nikodemas wants to merge 2 commits into
modelcontextprotocol:mainfrom
nikodemas:fix/simple-auth-example

Conversation

@nikodemas

Copy link
Copy Markdown

Fixes #2826 .

Motivation and Context

Fixes an error in the simple-auth example:

  • Fix server_url link
  • Fix the TODO related to the type hint with revoke_token method

How Has This Been Tested?

In addition to the tests, type checking and linting provided in the repo, I have tried to recreate the issue described in #2826 and it is now working fine.

Breaking Changes

No.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

@STiFLeR7

Copy link
Copy Markdown

Reproduced #2826 end-to-end against current `upstream/main` and verified this fix resolves it.

Before the fix, running the exact repro from #2826 (auth server on :9000, resource server on :8001, then the auth client with `MCP_TRANSPORT_TYPE=streamable-http`) fails during the OAuth flow with:
```
mcp.client.auth.exceptions.OAuthFlowError: Protected resource http://localhost:8001/mcp does not match expected http://localhost:8001
```
This confirms the root cause the reporter identified: `ResourceServerSettings.server_url` defaulted to `http://localhost:8001/mcp\`, which becomes the RFC 9728 protected-resource identifier advertised in the `WWW-Authenticate`/PRM response, but the client's `_validate_resource_match` computes its expected resource from the request's origin (no path), so the two never match and every auth attempt fails — not a raw connection failure as the original traceback suggested, but a resource-mismatch rejection one layer up in the auth flow.

After applying this PR's diff to the same checkout: the resource server now logs `MCP Resource Server running on http://localhost:8001/\` (no `/mcp`), and the client completes the full OAuth flow (`resource=http%3A%2F%2Flocalhost%3A8001%2F` in the `/authorize` request now matches) and reaches `✨ Session initialization complete!` / `✅ Connected to MCP server`.

Also checked the second half of the diff (`revoke_token` signature fix in `simple_auth_provider.py`): the base `OAuthAuthorizationServerProvider.revoke_token` protocol in `src/mcp/server/auth/provider.py` is `async def revoke_token(self, token: AccessTokenT | RefreshTokenT) -> None`, so the old override (`token: str, token_type_hint: str | None = None`) was indeed a real signature mismatch, not just a stylistic TODO — this PR's `token: AccessToken | RefreshToken` + `token.token` lookup now matches the protocol it implements.

Both fixes are small, correct, and directly resolve #2826 — no regressions spotted in the surrounding code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

simple-auth is not working as expected

2 participants