rusackas commented on code in PR #33924:
URL: https://github.com/apache/superset/pull/33924#discussion_r3607114515
##########
superset/jinja_context.py:
##########
@@ -305,6 +311,67 @@ def url_param(
self.cache_key_wrapper(result)
return result
+ def get_guest_user_attribute(
+ self,
+ attribute_name: str,
+ default: JsonValue = None,
+ add_to_cache_keys: bool = True,
+ escape_result: bool = True,
+ ) -> JsonValue:
+ """
+ Get a specific user attribute from guest user.
+
+ This function retrieves attributes from the guest user token and
supports
+ all JSON-native types (string, number, boolean, array, object, null).
+
+ Args:
+ attribute_name: Name of the attribute to retrieve
+ default: Default value if attribute not found (can be any
JSON-native type)
+ add_to_cache_keys: Whether the value should be included in the
cache key
+ escape_result: Escape string values (and strings inside lists)
using
+ the database dialect's quoting so they are safe to interpolate
+ into SQL, mirroring ``url_param``. Enabled by default;
non-string
+ JSON types are returned unchanged. Set to False for the raw
value.
+
+ Returns:
+ The attribute value from the guest user token, or the default
value.
+ Can be any JSON-native type: string, number, boolean, array,
object, or
+ null.
+
+ Examples:
+ {{ get_guest_user_attribute('department') }} # Returns:
"Engineering"
+ {{ get_guest_user_attribute('is_admin') }} # Returns: True
+ {{ get_guest_user_attribute('permissions') }} # Returns: ["read",
"write"]
+ {{ get_guest_user_attribute('config') }} # Returns: {"theme":
"dark"}
+ {{ get_guest_user_attribute('missing', 'default') }} # Returns:
"default"
+ """
+
+ # The macro only applies to guest users (embedded). is_guest_user()
+ # handles the feature-flag and request-context checks internally.
+ if not security_manager.is_guest_user():
+ return default
+
+ token = g.user.guest_token
+ user_attributes = token.get("user", {}).get("attributes") or {}
+
+ # Only add to the cache key if the attribute exists in the guest token
+ if attribute_name not in user_attributes:
+ return default
+
+ result = user_attributes[attribute_name]
+ if add_to_cache_keys and result is not None:
+ # Use json.dumps for consistent serialization of all JSON-native
types
+ cache_value = json.dumps(result, sort_keys=True)
+ self.cache_key_wrapper(
+ f"guest_user_attribute:{attribute_name}:{cache_value}"
+ )
+ # Guest attributes are interpolated into the rendered SQL, so escape
+ # string values (and strings within lists) with the dialect's quoting
+ # by default, mirroring url_param. Non-string JSON types pass through.
+ if escape_result:
+ result = self._escape_value(result)
Review Comment:
Good catch. Escaping now goes through the dialect compiler's
`render_literal_value`, which doubles backslashes on MySQL/MariaDB (there's a
regression test using this exact payload). `url_param` and `get_filters` share
the same helper now, so they get the same treatment. True bind params aren't
really an option here since Jinja renders text before the SQL is ever parsed...
docstring and the docs warning are updated to match the actual behavior.
##########
superset/jinja_context.py:
##########
@@ -305,6 +311,67 @@ def url_param(
self.cache_key_wrapper(result)
return result
+ def get_guest_user_attribute(
+ self,
+ attribute_name: str,
+ default: JsonValue = None,
+ add_to_cache_keys: bool = True,
+ escape_result: bool = True,
+ ) -> JsonValue:
+ """
+ Get a specific user attribute from guest user.
+
+ This function retrieves attributes from the guest user token and
supports
+ all JSON-native types (string, number, boolean, array, object, null).
+
+ Args:
+ attribute_name: Name of the attribute to retrieve
+ default: Default value if attribute not found (can be any
JSON-native type)
+ add_to_cache_keys: Whether the value should be included in the
cache key
+ escape_result: Escape string values (and strings inside lists)
using
+ the database dialect's quoting so they are safe to interpolate
+ into SQL, mirroring ``url_param``. Enabled by default;
non-string
+ JSON types are returned unchanged. Set to False for the raw
value.
+
+ Returns:
+ The attribute value from the guest user token, or the default
value.
+ Can be any JSON-native type: string, number, boolean, array,
object, or
+ null.
+
+ Examples:
+ {{ get_guest_user_attribute('department') }} # Returns:
"Engineering"
+ {{ get_guest_user_attribute('is_admin') }} # Returns: True
+ {{ get_guest_user_attribute('permissions') }} # Returns: ["read",
"write"]
+ {{ get_guest_user_attribute('config') }} # Returns: {"theme":
"dark"}
+ {{ get_guest_user_attribute('missing', 'default') }} # Returns:
"default"
+ """
+
+ # The macro only applies to guest users (embedded). is_guest_user()
+ # handles the feature-flag and request-context checks internally.
+ if not security_manager.is_guest_user():
+ return default
+
+ token = g.user.guest_token
+ user_attributes = token.get("user", {}).get("attributes") or {}
+
+ # Only add to the cache key if the attribute exists in the guest token
+ if attribute_name not in user_attributes:
+ return default
+
+ result = user_attributes[attribute_name]
+ if add_to_cache_keys and result is not None:
+ # Use json.dumps for consistent serialization of all JSON-native
types
+ cache_value = json.dumps(result, sort_keys=True)
+ self.cache_key_wrapper(
+ f"guest_user_attribute:{attribute_name}:{cache_value}"
+ )
Review Comment:
Good catch on both halves of this. The resolved value (attribute, default,
or null) is keyed on every branch now, and `has_extra_cache_key_calls` scans
guest-token RLS clauses as well... it really was only looking at persisted RLS
before. Added a collision test along the lines of what #42122 did for the
`current_user_*` macros.
##########
superset/security/api.py:
##########
@@ -69,6 +69,9 @@ class UserSchema(PermissiveSchema):
username = fields.String()
first_name = fields.String()
last_name = fields.String()
+ attributes = fields.Dict(
+ keys=fields.String(), values=fields.Raw(allow_none=True),
allow_none=True
+ )
Review Comment:
Added `attributes` to the `GuestTokenCreate` user schema in the spec. A full
`update-api-docs` regen wanted to rewrite a few thousand unrelated lines (the
committed spec has drifted quite a bit), so I scoped this to just the schema
change rather than folding all that churn into this PR.
##########
superset/jinja_context.py:
##########
@@ -305,6 +311,67 @@ def url_param(
self.cache_key_wrapper(result)
return result
+ def get_guest_user_attribute(
+ self,
+ attribute_name: str,
+ default: JsonValue = None,
+ add_to_cache_keys: bool = True,
+ escape_result: bool = True,
+ ) -> JsonValue:
+ """
+ Get a specific user attribute from guest user.
+
+ This function retrieves attributes from the guest user token and
supports
+ all JSON-native types (string, number, boolean, array, object, null).
+
+ Args:
+ attribute_name: Name of the attribute to retrieve
+ default: Default value if attribute not found (can be any
JSON-native type)
+ add_to_cache_keys: Whether the value should be included in the
cache key
+ escape_result: Escape string values (and strings inside lists)
using
+ the database dialect's quoting so they are safe to interpolate
+ into SQL, mirroring ``url_param``. Enabled by default;
non-string
+ JSON types are returned unchanged. Set to False for the raw
value.
+
+ Returns:
+ The attribute value from the guest user token, or the default
value.
+ Can be any JSON-native type: string, number, boolean, array,
object, or
+ null.
+
+ Examples:
+ {{ get_guest_user_attribute('department') }} # Returns:
"Engineering"
+ {{ get_guest_user_attribute('is_admin') }} # Returns: True
+ {{ get_guest_user_attribute('permissions') }} # Returns: ["read",
"write"]
+ {{ get_guest_user_attribute('config') }} # Returns: {"theme":
"dark"}
+ {{ get_guest_user_attribute('missing', 'default') }} # Returns:
"default"
+ """
+
+ # The macro only applies to guest users (embedded). is_guest_user()
+ # handles the feature-flag and request-context checks internally.
+ if not security_manager.is_guest_user():
+ return default
+
+ token = g.user.guest_token
+ user_attributes = token.get("user", {}).get("attributes") or {}
+
+ # Only add to the cache key if the attribute exists in the guest token
+ if attribute_name not in user_attributes:
+ return default
+
+ result = user_attributes[attribute_name]
Review Comment:
Annotations added.
##########
tests/integration_tests/security_tests.py:
##########
@@ -2345,6 +2345,170 @@ def test_get_guest_user(self):
assert guest_user is not None
assert "test_guest" == guest_user.username
+ def create_guest_token_with_attributes(self):
+ user = {
+ "username": "test_guest_with_attrs",
+ "first_name": "Test",
+ "last_name": "Guest",
+ "attributes": {
+ "department": "Engineering",
+ "region": "US",
+ "role": "developer",
+ "team": "data-platform",
+ },
+ }
+ resources = [{"some": "resource"}]
+ rls = [{"dataset": 1, "clause": "access = 1"}]
+ return security_manager.create_guest_access_token(user, resources, rls)
+
+ def test_create_guest_access_token_with_attributes(self):
+ """Test creating guest access token with user attributes."""
+ user_with_attributes = {
+ "username": "test_guest_attrs",
+ "first_name": "Test",
+ "last_name": "Guest",
+ "attributes": {
+ "department": "Engineering",
+ "region": "US",
+ "clearance_level": "standard",
+ "projects": ["analytics", "ml-platform"],
+ "team_lead": True,
+ },
+ }
+ resources = [{"type": "dashboard", "id": "test-dashboard"}]
+ rls = [{"dataset": 1, "clause": "id = 1"}]
+
+ token = security_manager.create_guest_access_token(
+ user_with_attributes, resources, rls
+ )
+
+ # Decode and verify the token contains attributes
+ aud = get_url_host()
+ decoded_token = jwt.decode(
+ token,
+ self.app.config["GUEST_TOKEN_JWT_SECRET"],
+ algorithms=[self.app.config["GUEST_TOKEN_JWT_ALGO"]],
+ audience=aud,
+ )
+
+ assert "user" in decoded_token
+ user = decoded_token["user"]
+ assert "attributes" in user
+ assert user["attributes"]["department"] == "Engineering"
+ assert user["attributes"]["region"] == "US"
+ assert user["attributes"]["clearance_level"] == "standard"
+ assert user["attributes"]["projects"] == ["analytics", "ml-platform"]
+ assert user["attributes"]["team_lead"] is True
+
+ def test_get_guest_user_with_attributes(self):
Review Comment:
Added `-> None` to the new test methods.
##########
tests/integration_tests/security_tests.py:
##########
@@ -2345,6 +2345,170 @@ def test_get_guest_user(self):
assert guest_user is not None
assert "test_guest" == guest_user.username
+ def create_guest_token_with_attributes(self):
+ user = {
+ "username": "test_guest_with_attrs",
+ "first_name": "Test",
+ "last_name": "Guest",
+ "attributes": {
+ "department": "Engineering",
+ "region": "US",
+ "role": "developer",
+ "team": "data-platform",
+ },
+ }
+ resources = [{"some": "resource"}]
+ rls = [{"dataset": 1, "clause": "access = 1"}]
+ return security_manager.create_guest_access_token(user, resources, rls)
+
+ def test_create_guest_access_token_with_attributes(self):
+ """Test creating guest access token with user attributes."""
+ user_with_attributes = {
+ "username": "test_guest_attrs",
+ "first_name": "Test",
+ "last_name": "Guest",
+ "attributes": {
+ "department": "Engineering",
+ "region": "US",
+ "clearance_level": "standard",
+ "projects": ["analytics", "ml-platform"],
+ "team_lead": True,
+ },
+ }
+ resources = [{"type": "dashboard", "id": "test-dashboard"}]
+ rls = [{"dataset": 1, "clause": "id = 1"}]
+
+ token = security_manager.create_guest_access_token(
+ user_with_attributes, resources, rls
+ )
+
+ # Decode and verify the token contains attributes
+ aud = get_url_host()
+ decoded_token = jwt.decode(
+ token,
+ self.app.config["GUEST_TOKEN_JWT_SECRET"],
+ algorithms=[self.app.config["GUEST_TOKEN_JWT_ALGO"]],
+ audience=aud,
+ )
+
+ assert "user" in decoded_token
+ user = decoded_token["user"]
+ assert "attributes" in user
+ assert user["attributes"]["department"] == "Engineering"
+ assert user["attributes"]["region"] == "US"
+ assert user["attributes"]["clearance_level"] == "standard"
+ assert user["attributes"]["projects"] == ["analytics", "ml-platform"]
+ assert user["attributes"]["team_lead"] is True
+
+ def test_get_guest_user_with_attributes(self):
+ """Test that guest user properly retains attributes from token."""
+ token = self.create_guest_token_with_attributes()
+ fake_request = FakeRequest()
+ fake_request.headers[current_app.config["GUEST_TOKEN_HEADER_NAME"]] =
token
+
+ guest_user = security_manager.get_guest_user_from_request(fake_request)
+
+ assert guest_user is not None
+ assert "test_guest_with_attrs" == guest_user.username
+
+ # Verify attributes are accessible through guest_token
+ assert hasattr(guest_user, "guest_token")
+ token_user = guest_user.guest_token["user"]
+ assert "attributes" in token_user
+ assert token_user["attributes"]["department"] == "Engineering"
+ assert token_user["attributes"]["region"] == "US"
+ assert token_user["attributes"]["role"] == "developer"
+ assert token_user["attributes"]["team"] == "data-platform"
+
+ def test_create_guest_access_token_without_attributes(self):
Review Comment:
Done, same as above.
--
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]
