rusackas commented on code in PR #40676:
URL: https://github.com/apache/superset/pull/40676#discussion_r3384913376
##########
superset/security/manager.py:
##########
@@ -3681,6 +3684,43 @@ def get_guest_user_from_request(self, req: Request) ->
Optional[GuestUser]:
return self.get_guest_user_from_token(cast(GuestToken, token))
+ @staticmethod
+ def _is_guest_token_revoked(token: dict[str, Any]) -> bool:
+ """Return True if the token predates a revocation on any of its
+ embedded-dashboard resources (``guest_token_revoked_before``)."""
+ issued_at = token.get("iat")
+ if not issued_at:
+ return False
Review Comment:
Good point — fixed in 6f07238acf. `_is_guest_token_revoked` now fails
closed: when a dashboard resource has an active `guest_token_revoked_before`
cutoff and the token lacks an `iat` claim, the token is treated as revoked (it
cannot prove it was issued after the cutoff). Tokens without `iat` are only
allowed through when no cutoff is set. Added unit tests for both the cutoff and
no-cutoff cases.
##########
superset/security/manager.py:
##########
@@ -3681,6 +3684,43 @@ def get_guest_user_from_request(self, req: Request) ->
Optional[GuestUser]:
return self.get_guest_user_from_token(cast(GuestToken, token))
+ @staticmethod
+ def _is_guest_token_revoked(token: dict[str, Any]) -> bool:
+ """Return True if the token predates a revocation on any of its
+ embedded-dashboard resources (``guest_token_revoked_before``)."""
+ issued_at = token.get("iat")
+ if not issued_at:
+ return False
+
+ # pylint: disable=import-outside-toplevel
+ from superset.daos.dashboard import EmbeddedDashboardDAO
+
+ for resource in token.get("resources") or []:
+ if resource.get("type") != GuestTokenResourceType.DASHBOARD.value:
+ continue
+ embedded = EmbeddedDashboardDAO.find_by_id(str(resource.get("id")))
+ revoked_before = getattr(embedded, "guest_token_revoked_before",
None)
+ if revoked_before is not None and issued_at < revoked_before:
+ return True
+ return False
+
+ @transaction()
+ def revoke_guest_token_access(
+ self, embedded_uuid: str, before: Optional[int] = None
+ ) -> None:
+ """Revoke all guest tokens issued for an embedded dashboard before
+ ``before`` (epoch seconds, default: now). Subsequent tokens are
+ unaffected."""
+ # pylint: disable=import-outside-toplevel
+ from superset.daos.dashboard import EmbeddedDashboardDAO
+
+ embedded = EmbeddedDashboardDAO.find_by_id(str(embedded_uuid))
+ if embedded is None:
+ return
+ embedded.guest_token_revoked_before = (
+ before if before is not None else
int(self._get_current_epoch_time())
Review Comment:
Agreed — fixed in 6f07238acf. `revoke_guest_token_access` now rounds the
cutoff up to the next whole second with `ceil(...)` (the column stores integer
seconds), so any token whose fractional `iat` falls within the revocation
second is reliably `< revoked_before` and gets rejected. Rounding up fails
closed.
##########
superset/security/manager.py:
##########
@@ -3681,6 +3684,43 @@ def get_guest_user_from_request(self, req: Request) ->
Optional[GuestUser]:
return self.get_guest_user_from_token(cast(GuestToken, token))
+ @staticmethod
+ def _is_guest_token_revoked(token: dict[str, Any]) -> bool:
+ """Return True if the token predates a revocation on any of its
+ embedded-dashboard resources (``guest_token_revoked_before``)."""
+ issued_at = token.get("iat")
+ if not issued_at:
+ return False
+
+ # pylint: disable=import-outside-toplevel
+ from superset.daos.dashboard import EmbeddedDashboardDAO
+
+ for resource in token.get("resources") or []:
+ if resource.get("type") != GuestTokenResourceType.DASHBOARD.value:
+ continue
+ embedded = EmbeddedDashboardDAO.find_by_id(str(resource.get("id")))
+ revoked_before = getattr(embedded, "guest_token_revoked_before",
None)
+ if revoked_before is not None and issued_at < revoked_before:
+ return True
Review Comment:
This is already handled. `_is_guest_token_revoked` resolves both resource id
forms: it tries `EmbeddedDashboardDAO.find_by_id(resource_id)` (embedded UUID)
and falls back to `Dashboard.get(resource_id).embedded` for legacy dashboard-id
resources, then honors `guest_token_revoked_before` on whichever config(s)
resolve. So a revoked dashboard cannot be accessed via a legacy dashboard-id
token. Covered by `test_guest_token_revoked_via_legacy_dashboard_id_resource`.
--
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]
