bito-code-review[bot] commented on code in PR #40676:
URL: https://github.com/apache/superset/pull/40676#discussion_r3385390272
##########
superset/security/manager.py:
##########
@@ -3681,6 +3685,67 @@ 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``).
+
+ A token missing ``iat`` cannot prove it was issued after a revocation
+ cutoff, so it is treated as revoked whenever any of its dashboard
+ resources has an active cutoff; otherwise it is not revoked.
+ """
+ issued_at = token.get("iat")
+
+ # pylint: disable=import-outside-toplevel
+ from superset.daos.dashboard import EmbeddedDashboardDAO
+ from superset.models.dashboard import Dashboard
+
+ for resource in token.get("resources") or []:
+ if resource.get("type") != GuestTokenResourceType.DASHBOARD.value:
+ continue
+ resource_id = str(resource.get("id"))
+ # A dashboard resource id may be an embedded UUID or, during the
+ # UUID migration, a legacy dashboard id. Resolve the embedded
+ # config(s) for either form (mirrors
validate_guest_token_resources).
+ embedded = EmbeddedDashboardDAO.find_by_id(resource_id)
+ if embedded:
+ embedded_configs = [embedded]
+ else:
+ dashboard = Dashboard.get(resource_id)
+ embedded_configs = dashboard.embedded if dashboard else []
+ for embedded_config in embedded_configs:
+ revoked_before = getattr(
+ embedded_config, "guest_token_revoked_before", None
+ )
+ if revoked_before is None:
+ continue
+ # Without an issued-at claim the token cannot be shown to
+ # postdate the cutoff, so fail closed and treat it as revoked.
+ if not issued_at or 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
+ # Round the cutoff up to the next whole second so that tokens whose
+ # fractional ``iat`` falls within the current second are reliably
+ # revoked (the column stores integer seconds). Rounding up fails
+ # closed: at worst it revokes a token issued slightly after the call.
+ embedded.guest_token_revoked_before = (
+ before if before is not None else
ceil(self._get_current_epoch_time())
+ )
Review Comment:
<!-- Bito Reply -->
The addition of unit tests for `revoke_guest_token_access` addresses the
previous review suggestion regarding missing test coverage. The implementation
covers the specified edge cases, including the `before` argument, default
behavior for fractional timestamps, and the no-op path for unresolved UUIDs,
which aligns with the requirement for comprehensive testing of new features.
--
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]
