aminghadersohi commented on code in PR #42297:
URL: https://github.com/apache/superset/pull/42297#discussion_r3717204811
##########
superset/mcp_service/auth.py:
##########
@@ -398,7 +467,9 @@ def check_tool_permission( # noqa: C901
# advertises scopes. Tokens/deployments that don't use scopes (API
keys,
# scope-less JWTs, dev-mode) fall through to RBAC-only behavior — see
# ``_token_scope_allows``.
- if has_permission and not _token_scope_allows(method_permission_name):
+ if has_permission and not _token_scope_allows(
+ method_permission_name, class_permission_name
+ ):
_log_scope_denial(
Review Comment:
Fixed in 6af6c99791a2504ac47e3d40cc058b9dbc309fa5. `get_schema` now
intersects its dynamic per-model RBAC check with `_token_scope_allows("read",
class_permission)`, and a regression test proves RBAC access alone cannot
bypass a mismatched token resource scope.
##########
superset/mcp_service/composite_token_verifier.py:
##########
@@ -168,32 +181,38 @@ async def verify_token(self, token: str) -> AccessToken |
None:
if any(token.startswith(prefix) for prefix in self._api_key_prefixes):
if self._app is not None:
loop = asyncio.get_running_loop()
- username = await loop.run_in_executor(
+ result = await loop.run_in_executor(
None, self._validate_api_key_sync, token
)
- if username is None:
+ if result is None:
logger.debug(
"API key rejected at transport layer (invalid or
expired)"
)
return None
+ username, key_scopes = result
logger.debug(
"API key validated at transport layer for user=%s",
username
)
return AccessToken(
token=token,
client_id="api_key",
- scopes=list(self.required_scopes or []),
+ # Prefer the key's own scopes over verifier-global
+ # required_scopes. Empty list = no scopes set = "no scopes
+ # advertised" back-compat per auth.py's
+ # _get_token_scopes()/_token_scope_allows().
+ scopes=key_scopes or list(self.required_scopes or []),
Review Comment:
Fixed in 6af6c99791a2504ac47e3d40cc058b9dbc309fa5. Transport-validated API
keys now preserve `key_scopes` exactly, including an empty list for legacy
unscoped/RBAC-only keys, rather than substituting verifier-global JWT
requirements. The test now covers a global required scope being configured
while the API key remains unscoped.
--
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]
