aminghadersohi commented on code in PR #42297:
URL: https://github.com/apache/superset/pull/42297#discussion_r3781790433
##########
superset/security/manager.py:
##########
@@ -4926,6 +4927,111 @@ def parse_jwt_guest_token(self, raw_token: str) ->
dict[str, Any]:
raw_token, secret, algorithms=[algo], audience=audience
)
+ def get_api_key_scopes(self, api_key_string: str) -> Optional[str]:
+ """Return the ``scopes`` value for a validated API key.
+
+ FAB's ``validate_api_key`` resolves the matching ``ApiKey`` row
+ internally (by lookup hash) but only returns the associated
+ ``User`` — the row's ``scopes`` column is otherwise unreachable by
+ callers. This repeats the same cheap, indexed lookup so MCP's
+ ``CompositeTokenVerifier`` can propagate per-key scopes instead of
+ silently falling back to verifier-global scopes. Call only after
+ ``validate_api_key`` has already succeeded for this token — this
+ method does not itself verify the key hash or active status.
+ """
+ lookup = self._compute_lookup_hash(api_key_string) # type:
ignore[attr-defined]
+ api_key = (
+ self.session.query(self.api_key_model) # type:
ignore[attr-defined]
+ .filter(self.api_key_model.lookup_hash == lookup)
+ .one_or_none()
+ )
+ return api_key.scopes if api_key else None
+
+ def _validate_requested_api_key_scopes(
+ self, user: Any, scopes: Optional[str]
+ ) -> None:
+ """Raise if ``scopes`` would grant a user more than their own RBAC.
+
+ Enforces the "intersection, never broader" rule confirmed for this
+ feature: a user must never be able to mint a token scoped beyond
+ what their own role already permits, even if they hand-author the
+ scopes string themselves at issuance time.
+
+ Per-resource scopes (``superset:<resource>:<action>``) are checked
+ against the user's actual ``can_<method>`` RBAC grant for that
+ resource. Flat scopes (``superset:read``/``superset:write``, the
+ pre-per-resource form) can only be self-issued by Admins — a flat
+ scope grants a method across every resource, and there's no single
+ RBAC check that soundly proves a non-Admin has that for "every
+ resource," so it's rejected for anyone else rather than guessed at.
+ Unrecognized scope strings are rejected outright (fail closed).
+
+ NOTE: this only prevents the request from being honored; it does
+ not (yet) produce a clean 400 response, since FAB's ``ApiKeyApi``
+ has no validation hook this can plug into without replacing the API
+ registration entirely. Raising here surfaces as a 500 via FAB's
+ ``@safe`` decorator until that's addressed — tracked as a known
+ follow-up, not silently accepted.
+ """
+ if not scopes:
+ return
+ # pylint: disable-next=import-outside-toplevel
+ from superset.security.api_key_scopes import (
+ RESOURCE_SCOPE_ACTIONS,
+ RESOURCE_SCOPE_CLASS,
+ )
+
+ is_admin = any(role.name == "Admin" for role in getattr(user, "roles",
[]))
+ for raw_scope in scopes.split(","):
+ scope = raw_scope.strip()
+ if not scope:
+ continue
+ parts = scope.split(":")
+ if len(parts) == 3 and parts[0] == "superset":
+ _, resource_slug, action = parts
+ class_permission_name = RESOURCE_SCOPE_CLASS.get(resource_slug)
+ if class_permission_name is None:
+ raise ValueError(
+ f"Requested scope '{scope}' names an unrecognized "
+ f"resource '{resource_slug}'"
+ )
+ if action not in RESOURCE_SCOPE_ACTIONS:
+ raise ValueError(
+ f"Requested scope '{scope}' names an unrecognized "
+ f"action '{action}'"
+ )
+ if self._has_view_access(user, f"can_{action}",
class_permission_name):
+ continue
+ raise ValueError(
+ f"Requested scope '{scope}' exceeds the issuing user's "
+ "own permissions"
+ )
+ if len(parts) == 2 and parts[0] == "superset" and is_admin:
+ continue
+ raise ValueError(
+ f"Requested scope '{scope}' is not a recognized "
+ "superset:<resource>:<action> scope, or requires Admin to "
Review Comment:
Fixed in 0efacae626a80f9b1e7c4362a5c7c709d2b92037. Admin flat scopes are now
accepted only when the action is in the supported read/write vocabulary;
undefined values such as `superset:garbage` fail closed. Added regression
coverage.
##########
superset/mcp_service/auth.py:
##########
@@ -223,12 +250,16 @@ def _log_scope_denial(
cyclomatic complexity in check.
"""
required_scope = _METHOD_TO_REQUIRED_SCOPE.get(method_permission_name)
+ resource_scope = _required_resource_scope(
+ class_permission_name, method_permission_name
+ )
+ scope_desc = resource_scope or required_scope
Review Comment:
Fixed in 0efacae626a80f9b1e7c4362a5c7c709d2b92037. Scope-denial logs now use
an explicit `unmapped method permission <name>` fallback instead of logging a
required scope of `None`, with regression coverage.
--
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]
