gabotorresruiz commented on code in PR #45452:
URL: https://github.com/apache/superset/pull/45452#discussion_r4330500410
##########
superset/security/guest_token.py:
##########
@@ -138,6 +139,44 @@ class GuestTokenUser(TypedDict, total=False):
attributes: Optional[dict[str, Any]]
+# Upper bound for the identity strings carried in a guest token's ``user``
+# claim, a generous bound for a display identity.
+GUEST_TOKEN_IDENTITY_MAX_LENGTH = 255
+GUEST_TOKEN_IDENTITY_FIELDS = ("username", "first_name", "last_name")
Review Comment:
Makes sense, thanks. The cache key half of the docstring checks out too:
`superset/semantic_layers/cache_host.py:89` folds the whole `guest_token` claim
into the hashed identity payload.
##########
superset/security/guest_token.py:
##########
@@ -138,6 +139,44 @@ class GuestTokenUser(TypedDict, total=False):
attributes: Optional[dict[str, Any]]
+# Upper bound for the identity strings carried in a guest token's ``user``
+# claim, a generous bound for a display identity.
+GUEST_TOKEN_IDENTITY_MAX_LENGTH = 255
+GUEST_TOKEN_IDENTITY_FIELDS = ("username", "first_name", "last_name")
+
+
+def validate_guest_token_identity(user: Optional[GuestTokenUser]) -> None:
+ """Validate the identity strings carried in a guest token's ``user`` claim.
+
+ ``username``/``first_name``/``last_name`` are supplied by the host
+ application that mints the token and later surface in logging, request
+ context, and templated query contexts. Constrain them to bounded,
+ single-line strings where they enter the system, consistent with how other
+ request-supplied strings are validated: a display identity has no reason to
+ carry control characters (e.g. an embedded newline) or an unbounded value.
+
+ A missing ``user`` claim is not rejected here; that check belongs to the
+ point where the token is consumed.
+
+ :param user: the ``user`` claim from a guest token
+ :raises ValueError: if a provided identity field is not a valid string
+ """
+ if user is None:
+ return
+ if not isinstance(user, dict):
+ raise ValueError("Guest token user must be an object")
+ for field in GUEST_TOKEN_IDENTITY_FIELDS:
+ value = user.get(field)
+ if value is None:
+ continue
+ if not isinstance(value, str):
+ raise ValueError(f"Guest token {field} must be a string")
+ if len(value) > GUEST_TOKEN_IDENTITY_MAX_LENGTH:
+ raise ValueError(f"Guest token {field} exceeds the maximum length")
+ if CONTROL_CHARS_RE.search(value):
Review Comment:
Checked it: the new class catches all ten codepoints `str.splitlines()`
splits on with none left over, and it does not over-refuse. Emoji, combining
accents, bidi marks and `U+00A0` all still pass. Closed.
##########
superset/utils/file.py:
##########
@@ -22,7 +22,7 @@
# All C0 (U+0000–U+001F) and C1 (U+007F–U+009F) control characters.
# Stripping every control char (including tab, LF, CR) keeps titles safe for
# SMTP headers, Content-Disposition filenames, and headless-browser
document.title.
-_CONTROL_CHARS_RE = re.compile(r"[\x00-\x1f\x7f-\x9f]")
+CONTROL_CHARS_RE = re.compile(r"[\x00-\x1f\x7f-\x9f]")
Review Comment:
Even better, thanks. `_CONTROL_CHARS_RE` has no callers outside
`superset/utils/file.py` now, so there is nothing left to mislead.
--
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]
