MikolajBalcerek opened a new pull request, #45391:
URL: https://github.com/apache/superset/pull/45391
### SUMMARY
Fixes #45390.
`BrowserHelloMiddleware` (added in #40471) shows the MCP hello page for
every browser `GET` request, whatever the path. This breaks:
- the browser-facing routes of an `MCP_AUTH_FACTORY` OAuth-proxy provider
(e.g. fastmcp's `GoogleProvider` / `OAuthProxy`): `/authorize`, `/consent`,
`/auth/callback`;
- the `/.well-known/*` OAuth discovery documents.
As a result, OAuth login from an MCP client (Claude Code, Cursor, ...) never
completes: the browser opened on `/authorize` gets the hello page instead of
the login redirect. Nothing is logged as an error, as the response is a `200`.
This change makes the middleware intercept only the MCP endpoint itself
(`fastmcp.settings.streamable_http_path`, `/mcp` by default; a trailing slash
is tolerated). An optional `mcp_path` argument lets callers pin the path
explicitly. Other routes are passed through.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Browser requests (`Accept: text/html`) to a FastMCP streamable-HTTP app with
the middleware and an OAuth-style `/authorize` route:
| Request | Before | After |
|---|---|---|
| `GET /mcp` | 200 hello page | 200 hello page |
| `GET /authorize` | **200 hello page** | **302 → `/consent`** |
| `GET /.well-known/oauth-authorization-server` | **200 hello page** | 200
discovery document |
| MCP client `POST /mcp` (initialize) | 200 | 200 |
### TESTING INSTRUCTIONS
1. `pytest tests/unit_tests/mcp_service/test_browser_hello_middleware.py
tests/unit_tests/mcp_service/test_jwt_verifier_browser_hello.py`
- New tests: hello page on `/mcp`, `/mcp/` and `HEAD`; pass-through for
`/authorize`, `/consent`, `/auth/callback`, `/.well-known/*`, `/health` and
`/`; non-browser requests to `/mcp` untouched; custom `mcp_path`.
- 8 of the new tests fail on `master` without the fix and pass with it;
the existing hello-page tests are unchanged.
2. Manual: configure `MCP_AUTH_FACTORY` to return
`fastmcp.server.auth.providers.google.GoogleProvider(...)`, then:
```bash
curl -s -o /dev/null -w "%{http_code} %{redirect_url}\n" -H 'Accept:
text/html' \
'http://localhost:5008/authorize?response_type=code&client_id=...&redirect_uri=...&code_challenge=...&code_challenge_method=S256'
```
Before: `200` (hello page). After: `302` to `/consent?...`. Opening
`http://localhost:5008/mcp` in a browser still shows the hello page.
### ADDITIONAL INFORMATION
- [x] Has associated issue: Fixes #45390
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
AI assistance: drafted with Claude (`Co-authored-by` trailer in the
commits); reviewed, tested and submitted by the author.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
