codeant-ai-for-open-source[bot] commented on code in PR #33924:
URL: https://github.com/apache/superset/pull/33924#discussion_r2585049409
##########
superset/jinja_context.py:
##########
@@ -121,7 +126,8 @@ class ExtraCache:
r"current_user_rls_rules\([^()]*\)|"
r"current_user_roles\([^()]*\)|"
r"cache_key_wrapper\([^()]*\)|"
- r"url_param\([^()]*\)"
+ r"url_param\([^()]*\)|"
+ r"get_guest_user_attribute\([^()]*\)|"
Review Comment:
**Suggestion:** The regular expression alternation in `ExtraCache.regex`
ends with a trailing '|' which creates an empty alternative, causing the inner
group to match an empty string and therefore match templates even when none of
the listed macros are present — this breaks the intended macro-detection logic
and can cause incorrect cache-key behavior and false positives. Remove the
trailing '|' from the last alternation so the group doesn't include an empty
alternative. [logic error]
**Severity Level:** Minor ⚠️
```suggestion
r"get_guest_user_attribute\([^()]*\)"
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
The suggestion correctly identifies a logic bug: a trailing '|' before the
closing alternation creates an empty alternative which makes the regex match
when none of the intended macros are present. Removing the trailing '|' from
the final alternative fixes the false-positive matching behaviour with a
minimal, local change.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/jinja_context.py
**Line:** 130:130
**Comment:**
*Logic Error: The regular expression alternation in `ExtraCache.regex`
ends with a trailing '|' which creates an empty alternative, causing the inner
group to match an empty string and therefore match templates even when none of
the listed macros are present — this breaks the intended macro-detection logic
and can cause incorrect cache-key behavior and false positives. Remove the
trailing '|' from the last alternation so the group doesn't include an empty
alternative.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
##########
superset/security/api.py:
##########
@@ -57,6 +57,7 @@ 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)
Review Comment:
**Suggestion:** Marshmallow will reject None values inside the dict because
`values=fields.Raw()` by default does not allow None; if a client sends an
attribute with a None value or the downstream code expects None to be
preserved, validation will fail. Make the inner value field allow None. [type
error]
**Severity Level:** Minor ⚠️
```suggestion
attributes = fields.Dict(
keys=fields.String(), values=fields.Raw(allow_none=True),
allow_none=True
)
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
Correct: fields.Dict(allow_none=True) only allows the entire dict to be
None, it does not make the nested value_field accept None. If callers may send
attribute values that are explicit nulls, changing
values=fields.Raw(allow_none=True) is a minimal, correct fix to avoid
ValidationError. This is a targeted schema change and aligns with marshmallow
semantics.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/security/api.py
**Line:** 60:60
**Comment:**
*Type Error: Marshmallow will reject None values inside the dict
because `values=fields.Raw()` by default does not allow None; if a client sends
an attribute with a None value or the downstream code expects None to be
preserved, validation will fail. Make the inner value field allow None.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
##########
tests/integration_tests/security/api_tests.py:
##########
@@ -123,6 +123,174 @@ def test_post_guest_token_authorized(self):
assert user == decoded_token["user"]
assert resource == decoded_token["resources"][0]
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ def test_post_guest_token_with_attributes(self):
+ """
+ Security API: Create a guest token with user attributes
+ """
+ self.dash =
db.session.query(Dashboard).filter_by(slug="births").first()
+ self.embedded = EmbeddedDashboardDAO.upsert(self.dash, [])
+ self.login(ADMIN_USERNAME)
+
+ user = {
+ "username": "bob_with_attrs",
+ "first_name": "Bob",
+ "last_name": "Also Bob",
+ "attributes": {
+ "department": "Engineering",
+ "region": "US",
+ "role": "developer",
+ "team": "data-platform",
+ "clearance_level": "standard",
+ "projects": ["analytics", "ml-platform"],
+ "team_lead": True,
+ },
+ }
+ resource = {"type": "dashboard", "id": str(self.embedded.uuid)}
+ rls_rule = {"dataset": 1, "clause": "1=1"}
+ params = {"user": user, "resources": [resource], "rls": [rls_rule]}
+
+ response = self.client.post(
+ self.uri, data=json.dumps(params), content_type="application/json"
+ )
+
+ self.assert200(response)
+ token = json.loads(response.data)["token"]
+ decoded_token = jwt.decode(
+ token,
+ self.app.config["GUEST_TOKEN_JWT_SECRET"],
+ audience=get_url_host(),
+ algorithms=["HS256"],
+ )
+
+ # Verify user attributes are preserved in the token
+ assert user == decoded_token["user"]
+ assert "attributes" in decoded_token["user"]
+ assert decoded_token["user"]["attributes"]["department"] ==
"Engineering"
+ assert decoded_token["user"]["attributes"]["region"] == "US"
+ assert decoded_token["user"]["attributes"]["role"] == "developer"
+ assert decoded_token["user"]["attributes"]["team"] == "data-platform"
+ assert decoded_token["user"]["attributes"]["clearance_level"] ==
"standard"
+ assert decoded_token["user"]["attributes"]["projects"] == [
+ "analytics",
+ "ml-platform",
+ ]
+ assert decoded_token["user"]["attributes"]["team_lead"] is True
+ assert resource == decoded_token["resources"][0]
+
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ def test_post_guest_token_with_empty_attributes(self):
+ """
+ Security API: Create a guest token with empty user attributes
+ """
+ self.dash =
db.session.query(Dashboard).filter_by(slug="births").first()
+ self.embedded = EmbeddedDashboardDAO.upsert(self.dash, [])
+ self.login(ADMIN_USERNAME)
+
+ user = {
+ "username": "bob_empty_attrs",
+ "first_name": "Bob",
+ "last_name": "Also Bob",
+ "attributes": {},
+ }
+ resource = {"type": "dashboard", "id": str(self.embedded.uuid)}
+ rls_rule = {"dataset": 1, "clause": "1=1"}
+ params = {"user": user, "resources": [resource], "rls": [rls_rule]}
+
+ response = self.client.post(
+ self.uri, data=json.dumps(params), content_type="application/json"
+ )
+
+ self.assert200(response)
+ token = json.loads(response.data)["token"]
+ decoded_token = jwt.decode(
+ token,
+ self.app.config["GUEST_TOKEN_JWT_SECRET"],
+ audience=get_url_host(),
+ algorithms=["HS256"],
+ )
+
+ # Verify empty attributes are preserved in the token
+ assert user == decoded_token["user"]
Review Comment:
**Suggestion:** The test compares the entire `user` dict to the decoded
token's `user` claim, which will fail if the backend adds/omits non-essential
fields; check the core user fields instead (username/first_name/last_name) to
avoid brittle tests. [logic error]
**Severity Level:** Minor ⚠️
```suggestion
# Verify empty attributes are preserved in the token (check core
fields explicitly)
assert decoded_token["user"]["username"] == user["username"]
assert decoded_token["user"]["first_name"] == user["first_name"]
assert decoded_token["user"]["last_name"] == user["last_name"]
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
As above, full equality will break if the implementation adds harmless
fields. Checking core fields keeps the assertion resilient while still
validating the important data.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/integration_tests/security/api_tests.py
**Line:** 213:214
**Comment:**
*Logic Error: The test compares the entire `user` dict to the decoded
token's `user` claim, which will fail if the backend adds/omits non-essential
fields; check the core user fields instead (username/first_name/last_name) to
avoid brittle tests.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
##########
tests/integration_tests/security_tests.py:
##########
@@ -2078,6 +2078,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"]
Review Comment:
**Suggestion:** The test mutates the class-level `FakeRequest.headers` dict
by setting a header key on the instance (via item assignment).
`FakeRequest.headers` is defined as a class attribute (a shared dict), so
modifying it in one test will leak state across tests and cause flakiness. Also
the test relies on `guest_user` having a `guest_token` side-effect which may
not be guaranteed; decode the token directly to verify attributes instead of
relying on `guest_user.guest_token`. [possible bug]
**Severity Level:** Critical 🚨
```suggestion
# Ensure headers is an instance dict to avoid mutating the
class-level dict shared across tests.
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
# Decode the token directly inside an application context to verify
attributes,
# instead of relying on a possible side-effect attaching
`guest_token` to the user.
with self.client.application.test_request_context():
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
token_user = decoded_token["user"]
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
This is a correct and important catch: FakeRequest.headers is defined as a
class-level mutable dict, so setting an item on an instance without first
creating an instance-level dict will mutate shared state across tests and cause
flakiness. The improved code assigns an instance dict (avoiding the class-level
mutation) and verifying the token directly is a reasonable, more robust
assertion. The suggested extra jwt.decode inside a request context is harmless
and defensive.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/integration_tests/security_tests.py
**Line:** 2140:2149
**Comment:**
*Possible Bug: The test mutates the class-level `FakeRequest.headers`
dict by setting a header key on the instance (via item assignment).
`FakeRequest.headers` is defined as a class attribute (a shared dict), so
modifying it in one test will leak state across tests and cause flakiness. Also
the test relies on `guest_user` having a `guest_token` side-effect which may
not be guaranteed; decode the token directly to verify attributes instead of
relying on `guest_user.guest_token`.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
##########
tests/integration_tests/security/api_tests.py:
##########
@@ -123,6 +123,174 @@ def test_post_guest_token_authorized(self):
assert user == decoded_token["user"]
assert resource == decoded_token["resources"][0]
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ def test_post_guest_token_with_attributes(self):
+ """
+ Security API: Create a guest token with user attributes
+ """
+ self.dash =
db.session.query(Dashboard).filter_by(slug="births").first()
+ self.embedded = EmbeddedDashboardDAO.upsert(self.dash, [])
+ self.login(ADMIN_USERNAME)
+
+ user = {
+ "username": "bob_with_attrs",
+ "first_name": "Bob",
+ "last_name": "Also Bob",
+ "attributes": {
+ "department": "Engineering",
+ "region": "US",
+ "role": "developer",
+ "team": "data-platform",
+ "clearance_level": "standard",
+ "projects": ["analytics", "ml-platform"],
+ "team_lead": True,
+ },
+ }
+ resource = {"type": "dashboard", "id": str(self.embedded.uuid)}
+ rls_rule = {"dataset": 1, "clause": "1=1"}
+ params = {"user": user, "resources": [resource], "rls": [rls_rule]}
+
+ response = self.client.post(
+ self.uri, data=json.dumps(params), content_type="application/json"
+ )
+
+ self.assert200(response)
+ token = json.loads(response.data)["token"]
+ decoded_token = jwt.decode(
+ token,
+ self.app.config["GUEST_TOKEN_JWT_SECRET"],
+ audience=get_url_host(),
+ algorithms=["HS256"],
+ )
+
+ # Verify user attributes are preserved in the token
+ assert user == decoded_token["user"]
+ assert "attributes" in decoded_token["user"]
+ assert decoded_token["user"]["attributes"]["department"] ==
"Engineering"
+ assert decoded_token["user"]["attributes"]["region"] == "US"
+ assert decoded_token["user"]["attributes"]["role"] == "developer"
+ assert decoded_token["user"]["attributes"]["team"] == "data-platform"
+ assert decoded_token["user"]["attributes"]["clearance_level"] ==
"standard"
+ assert decoded_token["user"]["attributes"]["projects"] == [
+ "analytics",
+ "ml-platform",
+ ]
+ assert decoded_token["user"]["attributes"]["team_lead"] is True
+ assert resource == decoded_token["resources"][0]
+
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ def test_post_guest_token_with_empty_attributes(self):
+ """
+ Security API: Create a guest token with empty user attributes
+ """
+ self.dash =
db.session.query(Dashboard).filter_by(slug="births").first()
+ self.embedded = EmbeddedDashboardDAO.upsert(self.dash, [])
+ self.login(ADMIN_USERNAME)
+
+ user = {
+ "username": "bob_empty_attrs",
+ "first_name": "Bob",
+ "last_name": "Also Bob",
+ "attributes": {},
+ }
+ resource = {"type": "dashboard", "id": str(self.embedded.uuid)}
+ rls_rule = {"dataset": 1, "clause": "1=1"}
+ params = {"user": user, "resources": [resource], "rls": [rls_rule]}
+
+ response = self.client.post(
+ self.uri, data=json.dumps(params), content_type="application/json"
+ )
+
+ self.assert200(response)
+ token = json.loads(response.data)["token"]
+ decoded_token = jwt.decode(
+ token,
+ self.app.config["GUEST_TOKEN_JWT_SECRET"],
+ audience=get_url_host(),
+ algorithms=["HS256"],
+ )
+
+ # Verify empty attributes are preserved in the token
+ assert user == decoded_token["user"]
+ assert "attributes" in decoded_token["user"]
+ assert decoded_token["user"]["attributes"] == {}
+ assert resource == decoded_token["resources"][0]
+
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ def test_post_guest_token_with_null_attributes(self):
+ """
+ Security API: Create a guest token with null user attributes
+ """
+ self.dash =
db.session.query(Dashboard).filter_by(slug="births").first()
+ self.embedded = EmbeddedDashboardDAO.upsert(self.dash, [])
+ self.login(ADMIN_USERNAME)
+
+ user = {
+ "username": "bob_null_attrs",
+ "first_name": "Bob",
+ "last_name": "Also Bob",
+ "attributes": None,
+ }
+ resource = {"type": "dashboard", "id": str(self.embedded.uuid)}
+ rls_rule = {"dataset": 1, "clause": "1=1"}
+ params = {"user": user, "resources": [resource], "rls": [rls_rule]}
+
+ response = self.client.post(
+ self.uri, data=json.dumps(params), content_type="application/json"
+ )
+
+ self.assert200(response)
+ token = json.loads(response.data)["token"]
+ decoded_token = jwt.decode(
+ token,
+ self.app.config["GUEST_TOKEN_JWT_SECRET"],
+ audience=get_url_host(),
+ algorithms=["HS256"],
+ )
+
+ # Verify null attributes are preserved in the token
+ assert user == decoded_token["user"]
Review Comment:
**Suggestion:** The test asserts full equality of `user` vs the decoded
token `user` while also checking `attributes` is present and None; strict dict
equality is fragile if implementation injects other claims — verify core fields
explicitly and keep the null attribute assertion separate. [logic error]
**Severity Level:** Minor ⚠️
```suggestion
# Verify null attributes are preserved in the token (check core
fields explicitly)
assert decoded_token["user"]["username"] == user["username"]
assert decoded_token["user"]["first_name"] == user["first_name"]
assert decoded_token["user"]["last_name"] == user["last_name"]
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
The suggested change prevents false negatives caused by extra fields
injected by the token builder while still asserting that attributes are null.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/integration_tests/security/api_tests.py
**Line:** 251:252
**Comment:**
*Logic Error: The test asserts full equality of `user` vs the decoded
token `user` while also checking `attributes` is present and None; strict dict
equality is fragile if implementation injects other claims — verify core fields
explicitly and keep the null attribute assertion separate.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
##########
tests/integration_tests/security/api_tests.py:
##########
@@ -123,6 +123,174 @@ def test_post_guest_token_authorized(self):
assert user == decoded_token["user"]
assert resource == decoded_token["resources"][0]
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ def test_post_guest_token_with_attributes(self):
+ """
+ Security API: Create a guest token with user attributes
+ """
+ self.dash =
db.session.query(Dashboard).filter_by(slug="births").first()
+ self.embedded = EmbeddedDashboardDAO.upsert(self.dash, [])
+ self.login(ADMIN_USERNAME)
+
+ user = {
+ "username": "bob_with_attrs",
+ "first_name": "Bob",
+ "last_name": "Also Bob",
+ "attributes": {
+ "department": "Engineering",
+ "region": "US",
+ "role": "developer",
+ "team": "data-platform",
+ "clearance_level": "standard",
+ "projects": ["analytics", "ml-platform"],
+ "team_lead": True,
+ },
+ }
+ resource = {"type": "dashboard", "id": str(self.embedded.uuid)}
+ rls_rule = {"dataset": 1, "clause": "1=1"}
+ params = {"user": user, "resources": [resource], "rls": [rls_rule]}
+
+ response = self.client.post(
+ self.uri, data=json.dumps(params), content_type="application/json"
+ )
+
+ self.assert200(response)
+ token = json.loads(response.data)["token"]
+ decoded_token = jwt.decode(
+ token,
+ self.app.config["GUEST_TOKEN_JWT_SECRET"],
+ audience=get_url_host(),
+ algorithms=["HS256"],
+ )
+
+ # Verify user attributes are preserved in the token
+ assert user == decoded_token["user"]
Review Comment:
**Suggestion:** The test asserts full dictionary equality between the
original `user` input and the decoded token `user` claim; this is brittle
because the service may add extra fields (timestamps, ids) to the token or
normalize values, making the equality fail even when the important fields are
preserved. Replace the strict equality with explicit checks for the required
fields so the test validates intent without being fragile. [logic error]
**Severity Level:** Minor ⚠️
```suggestion
# Verify user attributes are preserved in the token (check core
fields explicitly)
assert decoded_token["user"]["username"] == user["username"]
assert decoded_token["user"]["first_name"] == user["first_name"]
assert decoded_token["user"]["last_name"] == user["last_name"]
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
The strict dict equality is brittle — if the token includes additional
metadata (timestamps/ids) tests will fail even though core behavior is correct.
Replacing the equality with explicit checks for
username/first_name/last_name reduces flakiness and preserves intent.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/integration_tests/security/api_tests.py
**Line:** 166:167
**Comment:**
*Logic Error: The test asserts full dictionary equality between the
original `user` input and the decoded token `user` claim; this is brittle
because the service may add extra fields (timestamps, ids) to the token or
normalize values, making the equality fail even when the important fields are
preserved. Replace the strict equality with explicit checks for the required
fields so the test validates intent without being fragile.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
##########
superset/jinja_context.py:
##########
@@ -293,6 +299,70 @@ 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,
+ ) -> 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
+
+ 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"
+ """
+
+ # Check if we have a request context and user
+ if not has_request_context():
+ return default
+
+ if not hasattr(g, "user") or g.user is None:
+ return default
+
+ user = g.user
+
+ # Check if current user is a guest user
+ if not (hasattr(user, "is_guest_user") and user.is_guest_user):
+ return default
+
+ # Get attributes from guest token
+ if hasattr(user, "guest_token") and user.guest_token:
+ token = user.guest_token
+ token_user = token.get("user", {})
Review Comment:
**Suggestion:** `get_guest_user_attribute` assumes `user.guest_token` is a
mapping and calls `.get(...)` without validating its type; if `guest_token` is
a string or another non-dict truthy object this will raise an AttributeError
during template rendering — add explicit checks that `guest_token` and the
nested `token_user` are dictionaries before using `.get`, and return `default`
if they are not. [possible bug]
**Severity Level:** Critical 🚨
```suggestion
"""
# Check if we have a request context and user
if not has_request_context():
return default
if not hasattr(g, "user") or g.user is None:
return default
user = g.user
# Check if current user is a guest user
if not (hasattr(user, "is_guest_user") and user.is_guest_user):
return default
# Get attributes from guest token
if hasattr(user, "guest_token") and user.guest_token:
token = user.guest_token
# ensure token is a mapping before calling .get
if not isinstance(token, dict):
return default
token_user = token.get("user", {})
if not isinstance(token_user, dict):
return default
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
The suggestion is a correct defensive improvement: if guest_token is ever a
non-mapping truthy value (string, object, etc.), calling .get will raise an
AttributeError during template rendering. Adding explicit isinstance checks for
dict (and for token_user) prevents that and returns the safe default path. It's
a small, safe hardening that avoids runtime crashes.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/jinja_context.py
**Line:** 313:348
**Comment:**
*Possible Bug: `get_guest_user_attribute` assumes `user.guest_token` is
a mapping and calls `.get(...)` without validating its type; if `guest_token`
is a string or another non-dict truthy object this will raise an AttributeError
during template rendering — add explicit checks that `guest_token` and the
nested `token_user` are dictionaries before using `.get`, and return `default`
if they are not.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
##########
docs/docs/configuration/sql-templating.mdx:
##########
@@ -301,6 +301,111 @@ Here's a concrete example:
WHERE country_code = 'US'
```
+**Guest User Attributes**
+
+The `{{ get_guest_user_attribute('attribute_name') }}` macro returns a
specific attribute value from the
+guest user context. This is useful when working with embedded Superset where
guest tokens can contain
+custom attributes that need to be accessed in SQL queries.
+
+This macro supports all JSON-native types (string, number, boolean, array,
object, null) for both attribute values
+and default values. The return type depends on the type of the attribute
stored in the guest token.
+
+This macro only works when the current user is a guest user (authenticated via
guest token). If the current user is
+not a guest user, or if the specified attribute doesn't exist, the macro will
return `None` or the provided default value.
+
+Guest user attributes are set when creating guest tokens via the
`/security/guest_token` API endpoint.
+The attributes are passed in the `user` object within the request payload.
+
+Here's an example of how to create a guest token with custom attributes of
various types:
+
+```json
+{
+ "user": {
+ "username": "bob_with_attrs",
+ "first_name": "Bob",
+ "last_name": "Smith",
+ "attributes": {
+ "department": "Engineering",
+ "region": "US",
+ "role": "developer",
+ "team": "data-platform",
+ "clearance_level": "standard",
+ "projects": ["analytics", "ml-platform"],
+ "team_lead": true,
+ "employee_id": 12345,
+ "budget_limit": 50000.75,
+ "config": {
+ "theme": "dark",
+ "notifications": true
+ }
+ }
+ },
+ "resources": [{
+ "type": "dashboard",
+ "id": "dashboard-uuid"
+ }],
+ "rls": []
+}
+```
+
+The `attributes` field in the `user` object can contain any custom key-value
pairs that your application needs.
+These attributes will be available in SQL queries through the
`get_guest_user_attribute()` macro and can be
+any JSON-native type.
+
+For more information about setting up embedded Superset and creating guest
tokens, see the
+[embedded SDK
documentation](https://www.npmjs.com/package/@superset-ui/embedded-sdk) and
[networking
settings](/docs/configuration/networking-settings#html-embedding-of-dashboards-and-charts).
+
+If you have caching enabled in your Superset configuration, then by default
the attribute value will be used
+by Superset when calculating the cache key. A cache key is a unique identifier
that determines if there's a
+cache hit in the future and Superset can retrieve cached data.
+
+You can disable the inclusion of the attribute value in the calculation of the
+cache key by adding the following parameter to your Jinja code:
+
+```
+{{ get_guest_user_attribute('department', add_to_cache_keys=False) }}
+```
+
+You can also provide a default value if the attribute is not found. The
default value can be any JSON-native type:
+
+```
+{{ get_guest_user_attribute('region', default='US') }}
+{{ get_guest_user_attribute('team_lead', default=False) }}
+{{ get_guest_user_attribute('budget_limit', default=0) }}
+{{ get_guest_user_attribute('projects', default=[]) }}
+```
+
+Here are concrete examples of using guest user attributes of different types
in queries:
+
+```sql
+-- String attributes
+SELECT *
+FROM sales_data
+WHERE region = '{{ get_guest_user_attribute("region", default="global") }}'
+ AND department = '{{ get_guest_user_attribute("department") }}'
+
+-- Boolean attributes
+SELECT *
+FROM sensitive_data
+WHERE {{ get_guest_user_attribute("has_access", default=False) }} = true
Review Comment:
**Suggestion:** Boolean rendering ambiguity: the boolean example
interpolates the attribute directly and compares to the SQL literal `true`;
depending on how Jinja serializes booleans this may render capitalized Python
booleans or strings, causing dialect-dependent failures—serialize booleans to
JSON (lowercase true/false) or otherwise normalize them before comparison.
[possible bug]
**Severity Level:** Critical 🚨
```suggestion
WHERE {{ get_guest_user_attribute("has_access", default=False) | tojson }} =
true
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
Jinja may render Python booleans as "True"/"False" (capitalized) which some
SQL dialects don't accept. Using |tojson
yields lowercase true/false and is a practical way to make the example
dialect-robust.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** docs/docs/configuration/sql-templating.mdx
**Line:** 390:390
**Comment:**
*Possible Bug: Boolean rendering ambiguity: the boolean example
interpolates the attribute directly and compares to the SQL literal `true`;
depending on how Jinja serializes booleans this may render capitalized Python
booleans or strings, causing dialect-dependent failures—serialize booleans to
JSON (lowercase true/false) or otherwise normalize them before comparison.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
##########
docs/docs/configuration/sql-templating.mdx:
##########
@@ -301,6 +301,111 @@ Here's a concrete example:
WHERE country_code = 'US'
```
+**Guest User Attributes**
+
+The `{{ get_guest_user_attribute('attribute_name') }}` macro returns a
specific attribute value from the
+guest user context. This is useful when working with embedded Superset where
guest tokens can contain
+custom attributes that need to be accessed in SQL queries.
+
+This macro supports all JSON-native types (string, number, boolean, array,
object, null) for both attribute values
+and default values. The return type depends on the type of the attribute
stored in the guest token.
+
+This macro only works when the current user is a guest user (authenticated via
guest token). If the current user is
+not a guest user, or if the specified attribute doesn't exist, the macro will
return `None` or the provided default value.
+
+Guest user attributes are set when creating guest tokens via the
`/security/guest_token` API endpoint.
+The attributes are passed in the `user` object within the request payload.
+
+Here's an example of how to create a guest token with custom attributes of
various types:
+
+```json
+{
+ "user": {
+ "username": "bob_with_attrs",
+ "first_name": "Bob",
+ "last_name": "Smith",
+ "attributes": {
+ "department": "Engineering",
+ "region": "US",
+ "role": "developer",
+ "team": "data-platform",
+ "clearance_level": "standard",
+ "projects": ["analytics", "ml-platform"],
+ "team_lead": true,
+ "employee_id": 12345,
+ "budget_limit": 50000.75,
+ "config": {
+ "theme": "dark",
+ "notifications": true
+ }
+ }
+ },
+ "resources": [{
+ "type": "dashboard",
+ "id": "dashboard-uuid"
+ }],
+ "rls": []
+}
+```
+
+The `attributes` field in the `user` object can contain any custom key-value
pairs that your application needs.
+These attributes will be available in SQL queries through the
`get_guest_user_attribute()` macro and can be
+any JSON-native type.
+
+For more information about setting up embedded Superset and creating guest
tokens, see the
+[embedded SDK
documentation](https://www.npmjs.com/package/@superset-ui/embedded-sdk) and
[networking
settings](/docs/configuration/networking-settings#html-embedding-of-dashboards-and-charts).
+
+If you have caching enabled in your Superset configuration, then by default
the attribute value will be used
+by Superset when calculating the cache key. A cache key is a unique identifier
that determines if there's a
+cache hit in the future and Superset can retrieve cached data.
+
+You can disable the inclusion of the attribute value in the calculation of the
+cache key by adding the following parameter to your Jinja code:
+
+```
+{{ get_guest_user_attribute('department', add_to_cache_keys=False) }}
+```
+
+You can also provide a default value if the attribute is not found. The
default value can be any JSON-native type:
+
+```
+{{ get_guest_user_attribute('region', default='US') }}
+{{ get_guest_user_attribute('team_lead', default=False) }}
+{{ get_guest_user_attribute('budget_limit', default=0) }}
+{{ get_guest_user_attribute('projects', default=[]) }}
+```
+
+Here are concrete examples of using guest user attributes of different types
in queries:
+
+```sql
+-- String attributes
+SELECT *
+FROM sales_data
+WHERE region = '{{ get_guest_user_attribute("region", default="global") }}'
+ AND department = '{{ get_guest_user_attribute("department") }}'
Review Comment:
**Suggestion:** Security risk: the examples interpolate string guest
attributes directly into SQL without escaping, enabling SQL injection if an
attribute contains malicious characters; escape single quotes before inserting
into quoted SQL literals (or use a safe-quoting filter) to prevent injection.
[security]
**Severity Level:** Critical 🚨
```suggestion
WHERE region = '{{ get_guest_user_attribute("region", default="global") |
replace("'", "''") }}'
AND department = '{{ get_guest_user_attribute("department") | replace("'",
"''") }}'
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
The examples interpolate user-provided attributes directly into
single-quoted SQL literals. If an attribute
contains a single quote (or other malicious content) it will break the SQL
and could enable injection in real usage.
Escaping single quotes (or otherwise serializing safely) in the example is a
practical mitigation and improves the docs.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** docs/docs/configuration/sql-templating.mdx
**Line:** 384:385
**Comment:**
*Security: Security risk: the examples interpolate string guest
attributes directly into SQL without escaping, enabling SQL injection if an
attribute contains malicious characters; escape single quotes before inserting
into quoted SQL literals (or use a safe-quoting filter) to prevent injection.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
##########
docs/docs/configuration/sql-templating.mdx:
##########
@@ -301,6 +301,111 @@ Here's a concrete example:
WHERE country_code = 'US'
```
+**Guest User Attributes**
Review Comment:
**Suggestion:** The new section header is formatted as bold text instead of
using a heading level consistent with the rest of the document; this breaks the
document structure and will not appear in the generated table of contents or
match other macro sections. Replace the bold formatting with a proper heading
(e.g., "### Guest User Attributes") to be consistent. [possible bug]
**Severity Level:** Critical 🚨
```suggestion
### Guest User Attributes
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
The file shows other macro sections using "###" headings. The added line is
bold text, not a heading,
so it won't be treated the same by the renderer/TOC generator. Changing this
single line to "### Guest User Attributes"
fixes structure and is a simple, correct doc change.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** docs/docs/configuration/sql-templating.mdx
**Line:** 304:304
**Comment:**
*Possible Bug: The new section header is formatted as bold text instead
of using a heading level consistent with the rest of the document; this breaks
the document structure and will not appear in the generated table of contents
or match other macro sections. Replace the bold formatting with a proper
heading (e.g., "### Guest User Attributes") to be consistent.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
##########
tests/integration_tests/security/api_tests.py:
##########
@@ -123,6 +123,174 @@ def test_post_guest_token_authorized(self):
assert user == decoded_token["user"]
assert resource == decoded_token["resources"][0]
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ def test_post_guest_token_with_attributes(self):
+ """
+ Security API: Create a guest token with user attributes
+ """
+ self.dash =
db.session.query(Dashboard).filter_by(slug="births").first()
+ self.embedded = EmbeddedDashboardDAO.upsert(self.dash, [])
+ self.login(ADMIN_USERNAME)
+
+ user = {
+ "username": "bob_with_attrs",
+ "first_name": "Bob",
+ "last_name": "Also Bob",
+ "attributes": {
+ "department": "Engineering",
+ "region": "US",
+ "role": "developer",
+ "team": "data-platform",
+ "clearance_level": "standard",
+ "projects": ["analytics", "ml-platform"],
+ "team_lead": True,
+ },
+ }
+ resource = {"type": "dashboard", "id": str(self.embedded.uuid)}
+ rls_rule = {"dataset": 1, "clause": "1=1"}
+ params = {"user": user, "resources": [resource], "rls": [rls_rule]}
+
+ response = self.client.post(
+ self.uri, data=json.dumps(params), content_type="application/json"
+ )
+
+ self.assert200(response)
+ token = json.loads(response.data)["token"]
+ decoded_token = jwt.decode(
+ token,
+ self.app.config["GUEST_TOKEN_JWT_SECRET"],
+ audience=get_url_host(),
+ algorithms=["HS256"],
+ )
+
+ # Verify user attributes are preserved in the token
+ assert user == decoded_token["user"]
+ assert "attributes" in decoded_token["user"]
+ assert decoded_token["user"]["attributes"]["department"] ==
"Engineering"
+ assert decoded_token["user"]["attributes"]["region"] == "US"
+ assert decoded_token["user"]["attributes"]["role"] == "developer"
+ assert decoded_token["user"]["attributes"]["team"] == "data-platform"
+ assert decoded_token["user"]["attributes"]["clearance_level"] ==
"standard"
+ assert decoded_token["user"]["attributes"]["projects"] == [
+ "analytics",
+ "ml-platform",
+ ]
+ assert decoded_token["user"]["attributes"]["team_lead"] is True
+ assert resource == decoded_token["resources"][0]
+
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ def test_post_guest_token_with_empty_attributes(self):
+ """
+ Security API: Create a guest token with empty user attributes
+ """
+ self.dash =
db.session.query(Dashboard).filter_by(slug="births").first()
+ self.embedded = EmbeddedDashboardDAO.upsert(self.dash, [])
+ self.login(ADMIN_USERNAME)
+
+ user = {
+ "username": "bob_empty_attrs",
+ "first_name": "Bob",
+ "last_name": "Also Bob",
+ "attributes": {},
+ }
+ resource = {"type": "dashboard", "id": str(self.embedded.uuid)}
+ rls_rule = {"dataset": 1, "clause": "1=1"}
+ params = {"user": user, "resources": [resource], "rls": [rls_rule]}
+
+ response = self.client.post(
+ self.uri, data=json.dumps(params), content_type="application/json"
+ )
+
+ self.assert200(response)
+ token = json.loads(response.data)["token"]
+ decoded_token = jwt.decode(
+ token,
+ self.app.config["GUEST_TOKEN_JWT_SECRET"],
+ audience=get_url_host(),
+ algorithms=["HS256"],
+ )
+
+ # Verify empty attributes are preserved in the token
+ assert user == decoded_token["user"]
+ assert "attributes" in decoded_token["user"]
+ assert decoded_token["user"]["attributes"] == {}
+ assert resource == decoded_token["resources"][0]
+
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ def test_post_guest_token_with_null_attributes(self):
+ """
+ Security API: Create a guest token with null user attributes
+ """
+ self.dash =
db.session.query(Dashboard).filter_by(slug="births").first()
+ self.embedded = EmbeddedDashboardDAO.upsert(self.dash, [])
+ self.login(ADMIN_USERNAME)
+
+ user = {
+ "username": "bob_null_attrs",
+ "first_name": "Bob",
+ "last_name": "Also Bob",
+ "attributes": None,
+ }
+ resource = {"type": "dashboard", "id": str(self.embedded.uuid)}
+ rls_rule = {"dataset": 1, "clause": "1=1"}
+ params = {"user": user, "resources": [resource], "rls": [rls_rule]}
+
+ response = self.client.post(
+ self.uri, data=json.dumps(params), content_type="application/json"
+ )
+
+ self.assert200(response)
+ token = json.loads(response.data)["token"]
+ decoded_token = jwt.decode(
+ token,
+ self.app.config["GUEST_TOKEN_JWT_SECRET"],
+ audience=get_url_host(),
+ algorithms=["HS256"],
+ )
+
+ # Verify null attributes are preserved in the token
+ assert user == decoded_token["user"]
+ assert "attributes" in decoded_token["user"]
+ assert decoded_token["user"]["attributes"] is None
+ assert resource == decoded_token["resources"][0]
+
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ def test_post_guest_token_without_attributes_backward_compatibility(self):
+ """
+ Security API: Create a guest token without attributes (backward
compatibility)
+ """
+ self.dash =
db.session.query(Dashboard).filter_by(slug="births").first()
+ self.embedded = EmbeddedDashboardDAO.upsert(self.dash, [])
+ self.login(ADMIN_USERNAME)
+
+ user = {
+ "username": "bob_no_attrs",
+ "first_name": "Bob",
+ "last_name": "Also Bob",
+ # Note: no attributes field
+ }
+ resource = {"type": "dashboard", "id": str(self.embedded.uuid)}
+ rls_rule = {"dataset": 1, "clause": "1=1"}
+ params = {"user": user, "resources": [resource], "rls": [rls_rule]}
+
+ response = self.client.post(
+ self.uri, data=json.dumps(params), content_type="application/json"
+ )
+
+ self.assert200(response)
+ token = json.loads(response.data)["token"]
+ decoded_token = jwt.decode(
+ token,
+ self.app.config["GUEST_TOKEN_JWT_SECRET"],
+ audience=get_url_host(),
+ algorithms=["HS256"],
+ )
+
+ # Verify user without attributes works and no attributes field is
present
Review Comment:
**Suggestion:** The test asserts the entire `user` dict equals the decoded
`user` and separately asserts `attributes` is absent; equality will fail if the
backend adds metadata fields even when `attributes` is absent. Replace the
full-dict equality with explicit checks for required fields and the absence of
`attributes`. [logic error]
**Severity Level:** Minor ⚠️
```suggestion
# Verify user without attributes works and no attributes field is
present (check core fields explicitly)
assert decoded_token["user"]["username"] == user["username"]
assert decoded_token["user"]["first_name"] == user["first_name"]
assert decoded_token["user"]["last_name"] == user["last_name"]
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
The equality assert will fail if harmless extra fields exist; checking core
user fields plus asserting absence of the attributes key is clearer and less
brittle.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/integration_tests/security/api_tests.py
**Line:** 289:289
**Comment:**
*Logic Error: The test asserts the entire `user` dict equals the
decoded `user` and separately asserts `attributes` is absent; equality will
fail if the backend adds metadata fields even when `attributes` is absent.
Replace the full-dict equality with explicit checks for required fields and the
absence of `attributes`.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
##########
tests/unit_tests/jinja_context_test.py:
##########
@@ -1639,3 +1640,963 @@ def test_jinja2_server_error_handling(mocker:
MockerFixture) -> None:
assert "Internal Jinja2 template error" in str(exception)
assert "MemoryError" in str(exception)
assert "Out of memory" in str(exception)
+
+def test_get_guest_user_attribute_no_request_context(mocker: MockerFixture) ->
None:
+ """
+ Test that get_guest_user_attribute returns default when no request context.
+ """
+ mocker.patch("superset.jinja_context.has_request_context",
return_value=False)
+
+ cache = ExtraCache()
+ result = cache.get_guest_user_attribute("department", "default_dept")
+ assert result == "default_dept"
+
+
+def test_get_guest_user_attribute_no_user(mocker: MockerFixture) -> None:
+ """
+ Test that get_guest_user_attribute returns default when no user in context.
+ """
+ mocker.patch("superset.jinja_context.has_request_context",
return_value=True)
+ mock_g = mocker.patch("superset.jinja_context.g")
+ mock_g.user = None
+
+ cache = ExtraCache()
+ result = cache.get_guest_user_attribute("department", "default_dept")
+ assert result == "default_dept"
+
+
+def test_get_guest_user_attribute_not_guest_user(mocker: MockerFixture) ->
None:
+ """
+ Test that get_guest_user_attribute returns default when user is not a
guest user.
+ """
+ mocker.patch("superset.jinja_context.has_request_context",
return_value=True)
+ mock_g = mocker.patch("superset.jinja_context.g")
+
+ # Create a regular user (not guest)
+ regular_user = mocker.Mock()
+ regular_user.is_guest_user = False
+ mock_g.user = regular_user
+
+ cache = ExtraCache()
+ result = cache.get_guest_user_attribute("department", "default_dept")
+ assert result == "default_dept"
+
+
+def test_get_guest_user_attribute_with_attributes(mocker: MockerFixture) ->
None:
+ """
+ Test that get_guest_user_attribute returns correct attribute value for
guest user.
+ """
+ mocker.patch("superset.jinja_context.has_request_context",
return_value=True)
+ mock_g = mocker.patch("superset.jinja_context.g")
+
+ # Create guest user with attributes
+ guest_user = mocker.Mock()
+ guest_user.is_guest_user = True
+ guest_user.guest_token = {
+ "user": {
+ "username": "test_guest",
+ "attributes": {
+ "department": "Engineering",
+ "region": "US",
+ "role": "developer",
+ },
+ },
+ "resources": [{"type": "dashboard", "id": "test-id"}],
+ "rls_rules": [],
+ }
+ mock_g.user = guest_user
+
+ cache = ExtraCache()
+
+ # Test existing attribute
+ result = cache.get_guest_user_attribute("department")
+ assert result == "Engineering"
+
+ # Test another existing attribute
+ result = cache.get_guest_user_attribute("region")
+ assert result == "US"
+
+ # Test non-existing attribute returns default
+ result = cache.get_guest_user_attribute("non_existing", "default_value")
+ assert result == "default_value"
+
+
+def test_get_guest_user_attribute_without_attributes(mocker: MockerFixture) ->
None:
+ """
+ Test that get_guest_user_attribute returns default when guest user has no
+ attributes.
+ """
+ mocker.patch("superset.jinja_context.has_request_context",
return_value=True)
+ mock_g = mocker.patch("superset.jinja_context.g")
+
+ # Create guest user without attributes
+ guest_user = mocker.Mock()
+ guest_user.is_guest_user = True
+ guest_user.guest_token = {
+ "user": {"username": "test_guest"},
+ "resources": [{"type": "dashboard", "id": "test-id"}],
+ "rls_rules": [],
+ }
+ mock_g.user = guest_user
+
+ cache = ExtraCache()
+ result = cache.get_guest_user_attribute("department", "default_dept")
+ assert result == "default_dept"
+
+
+def test_get_guest_user_attribute_empty_attributes(mocker: MockerFixture) ->
None:
+ """
+ Test that get_guest_user_attribute returns default when guest user has
empty
+ attributes.
+ """
+ mocker.patch("superset.jinja_context.has_request_context",
return_value=True)
+ mock_g = mocker.patch("superset.jinja_context.g")
+
+ # Create guest user with empty attributes
+ guest_user = mocker.Mock()
+ guest_user.is_guest_user = True
+ guest_user.guest_token = {
+ "user": {"username": "test_guest", "attributes": {}},
+ "resources": [{"type": "dashboard", "id": "test-id"}],
+ "rls_rules": [],
+ }
+ mock_g.user = guest_user
+
+ cache = ExtraCache()
+ result = cache.get_guest_user_attribute("department", "default_dept")
+ assert result == "default_dept"
+
+
+def test_get_guest_user_attribute_null_attributes(mocker: MockerFixture) ->
None:
+ """
+ Test that get_guest_user_attribute returns default when guest user has null
+ attributes.
+ """
+ mocker.patch("superset.jinja_context.has_request_context",
return_value=True)
+ mock_g = mocker.patch("superset.jinja_context.g")
+
+ # Create guest user with null attributes
+ guest_user = mocker.Mock()
+ guest_user.is_guest_user = True
+ guest_user.guest_token = {
+ "user": {"username": "test_guest", "attributes": None},
+ "resources": [{"type": "dashboard", "id": "test-id"}],
+ "rls_rules": [],
+ }
+ mock_g.user = guest_user
+
+ cache = ExtraCache()
+ result = cache.get_guest_user_attribute("department", "default_dept")
+ assert result == "default_dept"
+
+
+def test_get_guest_user_attribute_cache_key_behavior(mocker: MockerFixture) ->
None:
+ """
+ Test that get_guest_user_attribute correctly handles cache key behavior.
+ """
+ mocker.patch("superset.jinja_context.has_request_context",
return_value=True)
+ mock_g = mocker.patch("superset.jinja_context.g")
+
+ # Create guest user with attributes
+ guest_user = mocker.Mock()
+ guest_user.is_guest_user = True
+ guest_user.guest_token = {
+ "user": {
+ "username": "test_guest",
+ "attributes": {"department": "Engineering", "region": "US"},
+ },
+ "resources": [{"type": "dashboard", "id": "test-id"}],
+ "rls_rules": [],
+ }
+ mock_g.user = guest_user
+
+ cache = ExtraCache()
+ # Mock the cache_key_wrapper method
+ mock_cache_wrapper = mocker.Mock()
Review Comment:
**Suggestion:** Replacing `cache.cache_key_wrapper` with a plain Mock
without a return value can change observable behavior (the real method returns
the key). Use a Mock that preserves the input by returning it
(side_effect=lambda x: x) so tests that rely on behavior or callers expecting
the original return value won't break. [logic error]
**Severity Level:** Minor ⚠️
```suggestion
# Use a mock that preserves the original method contract by returning
the passed value.
mock_cache_wrapper = mocker.Mock(side_effect=lambda value: value)
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
Replacing the real method with a Mock that preserves the contract (returns
the passed key)
is a harmless improvement and prevents subtle breakage if callers start
using the return
value. The suggested change (side_effect=lambda v: v) better mirrors the
original behavior
and is low cost; it's a sensible test hardening.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/jinja_context_test.py
**Line:** 1815:1815
**Comment:**
*Logic Error: Replacing `cache.cache_key_wrapper` with a plain Mock
without a return value can change observable behavior (the real method returns
the key). Use a Mock that preserves the input by returning it
(side_effect=lambda x: x) so tests that rely on behavior or callers expecting
the original return value won't break.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
##########
docs/docs/configuration/sql-templating.mdx:
##########
@@ -301,6 +301,111 @@ Here's a concrete example:
WHERE country_code = 'US'
```
+**Guest User Attributes**
+
+The `{{ get_guest_user_attribute('attribute_name') }}` macro returns a
specific attribute value from the
+guest user context. This is useful when working with embedded Superset where
guest tokens can contain
+custom attributes that need to be accessed in SQL queries.
+
+This macro supports all JSON-native types (string, number, boolean, array,
object, null) for both attribute values
+and default values. The return type depends on the type of the attribute
stored in the guest token.
+
+This macro only works when the current user is a guest user (authenticated via
guest token). If the current user is
+not a guest user, or if the specified attribute doesn't exist, the macro will
return `None` or the provided default value.
+
+Guest user attributes are set when creating guest tokens via the
`/security/guest_token` API endpoint.
+The attributes are passed in the `user` object within the request payload.
+
+Here's an example of how to create a guest token with custom attributes of
various types:
+
+```json
+{
+ "user": {
+ "username": "bob_with_attrs",
+ "first_name": "Bob",
+ "last_name": "Smith",
+ "attributes": {
+ "department": "Engineering",
+ "region": "US",
+ "role": "developer",
+ "team": "data-platform",
+ "clearance_level": "standard",
+ "projects": ["analytics", "ml-platform"],
+ "team_lead": true,
+ "employee_id": 12345,
+ "budget_limit": 50000.75,
+ "config": {
+ "theme": "dark",
+ "notifications": true
+ }
+ }
+ },
+ "resources": [{
+ "type": "dashboard",
+ "id": "dashboard-uuid"
+ }],
+ "rls": []
+}
+```
+
+The `attributes` field in the `user` object can contain any custom key-value
pairs that your application needs.
+These attributes will be available in SQL queries through the
`get_guest_user_attribute()` macro and can be
+any JSON-native type.
+
+For more information about setting up embedded Superset and creating guest
tokens, see the
+[embedded SDK
documentation](https://www.npmjs.com/package/@superset-ui/embedded-sdk) and
[networking
settings](/docs/configuration/networking-settings#html-embedding-of-dashboards-and-charts).
+
+If you have caching enabled in your Superset configuration, then by default
the attribute value will be used
+by Superset when calculating the cache key. A cache key is a unique identifier
that determines if there's a
+cache hit in the future and Superset can retrieve cached data.
+
+You can disable the inclusion of the attribute value in the calculation of the
+cache key by adding the following parameter to your Jinja code:
+
+```
+{{ get_guest_user_attribute('department', add_to_cache_keys=False) }}
+```
+
+You can also provide a default value if the attribute is not found. The
default value can be any JSON-native type:
+
+```
+{{ get_guest_user_attribute('region', default='US') }}
+{{ get_guest_user_attribute('team_lead', default=False) }}
+{{ get_guest_user_attribute('budget_limit', default=0) }}
+{{ get_guest_user_attribute('projects', default=[]) }}
+```
+
+Here are concrete examples of using guest user attributes of different types
in queries:
+
+```sql
+-- String attributes
+SELECT *
+FROM sales_data
+WHERE region = '{{ get_guest_user_attribute("region", default="global") }}'
+ AND department = '{{ get_guest_user_attribute("department") }}'
+
+-- Boolean attributes
+SELECT *
+FROM sensitive_data
+WHERE {{ get_guest_user_attribute("has_access", default=False) }} = true
+
+-- Numeric attributes
+SELECT *
+FROM transactions
+WHERE amount <= {{ get_guest_user_attribute("budget_limit", default=1000) }}
Review Comment:
**Suggestion:** Type/formatting issue: numeric example inserts the attribute
raw into SQL; if the attribute is non-numeric (string) the resulting SQL will
be invalid or behave unexpectedly. Coerce the value to a numeric type in the
template (e.g., apply a float/int filter) to ensure valid SQL numeric
comparison. [type error]
**Severity Level:** Minor ⚠️
```suggestion
WHERE amount <= {{ get_guest_user_attribute("budget_limit", default=1000) |
float }}
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
If the attribute comes back as a string (or null) the rendered SQL may be
invalid or behave unexpectedly.
Applying a numeric coercion like |float in the example makes the intent
explicit and yields safer, more predictable SQL.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** docs/docs/configuration/sql-templating.mdx
**Line:** 395:395
**Comment:**
*Type Error: Type/formatting issue: numeric example inserts the
attribute raw into SQL; if the attribute is non-numeric (string) the resulting
SQL will be invalid or behave unexpectedly. Coerce the value to a numeric type
in the template (e.g., apply a float/int filter) to ensure valid SQL numeric
comparison.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
--
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]
