bito-code-review[bot] commented on code in PR #40671:
URL: https://github.com/apache/superset/pull/40671#discussion_r3385608261
##########
superset/security/guest_token.py:
##########
@@ -14,13 +14,70 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
+import logging
from typing import Optional, TypedDict, Union
from flask_appbuilder.security.sqla.models import Group, Role
from flask_login import AnonymousUserMixin
from superset.utils.backports import StrEnum
+logger = logging.getLogger(__name__)
+
+# JWT claim that carries the revocation version a token was minted with.
+GUEST_TOKEN_REVOCATION_CLAIM = "rev" # noqa: S105
+
+# Tokens minted before the revocation feature existed carry no version claim.
+# They are treated as version 0, which is only rejected once an admin has
+# explicitly bumped the expected version above 0.
+DEFAULT_GUEST_TOKEN_REVOCATION_VERSION = 0
+
+
+def get_current_guest_token_revocation_version() -> int:
+ """
+ Return the minimum guest-token revocation version accepted at validation
time.
+
+ The value is stored in the metadata database via the ``key_value`` store
so it
+ can be bumped at runtime (e.g. via the ``revoke-guest-tokens`` CLI command)
+ without a code deploy. A missing entry means revocation has never been
+ triggered, so the default version is returned.
+ """
+ # pylint: disable=import-outside-toplevel
+ from superset.key_value.shared_entries import get_shared_value
+ from superset.key_value.types import SharedKey
+
+ try:
+ value = get_shared_value(SharedKey.GUEST_TOKEN_REVOCATION_VERSION)
+ except Exception: # pylint: disable=broad-except
+ # Never let a metadata-store hiccup hard-fail token validation; fall
back
+ # to the default version (fail-open to the pre-feature behaviour).
+ logger.warning("Could not read guest token revocation version",
exc_info=True)
Review Comment:
<!-- Bito Reply -->
The explanation provided for maintaining the broad exception catch is clear
and aligns with the design goal of ensuring guest-token validation fails open
during metadata-store outages. Given that specific exceptions like `KeyError`,
`ValueError`, and `TypeError` are already handled by the subsequent logic, the
broad catch serves as a necessary safeguard against database-level connection
errors. No further changes are required to address this specific review comment.
--
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]
