Re: [PR] fix(mcp): create ApiKey permissions on init and support API keys with JWT auth [superset]

["aminghadersohi (via GitHub)" <gi...@apache.org>]

aminghadersohi commented on PR #39604:
URL: https://github.com/apache/superset/pull/39604#issuecomment-4381227923

   ## Review: `CompositeTokenVerifier` scope handling
   
   Two findings from digging into the FastMCP internals:
   
   ### 🔴 Bug: pass-through tokens will 403 when `MCP_JWT_REQUIRED_SCOPES` is set
   
   There are two independent scope enforcement layers in FastMCP:
   
   1. **Inside `verify_token()`** — `JWTVerifier.load_access_token()` (`providers/jwt.py:463-473`) checks `required_scopes` before returning. The composite bypasses this for API key tokens — fine.
   2. **Transport middleware** — `RequireAuthMiddleware.__call__()` (`bearer_auth.py:78-96`) independently checks each required scope against `AuthCredentials(auth_info.scopes)`. The pass-through `AccessToken` has `scopes=[]`, so this check **will 403 every API key request** when `MCP_JWT_REQUIRED_SCOPES` is non-empty.
   
   Fix: populate `scopes` from `self.required_scopes` on the pass-through token so the middleware is satisfied while `_api_key_passthrough` still tells `_resolve_user_from_jwt_context` to defer:
   
   ```python
   # composite_token_verifier.py
   return AccessToken(
       token=token,
       client_id="api_key",
       scopes=list(self.required_scopes),  # satisfy RequireAuthMiddleware
       claims={"_api_key_passthrough": True},
   )
   ```
   
   ### 🟡 Minor: `_api_key_passthrough` claim name collision
   
   A JWT issued by an external IdP that happens to include `{"_api_key_passthrough": true}` as a custom claim would be silently misidentified in `_resolve_user_from_jwt_context` and cause auth failure. Not an auth bypass, but a subtle footgun. Consider a namespaced sentinel like `_superset_mcp_api_key_passthrough` or keying off `client_id == "api_key"` instead.
   
   ---
   
   (`base_url=None` on `super().__init__()` for HS256 verifiers is fine — silently accepted, only suppresses RFC 9728 metadata routes which aren't needed here.)


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org