From 42e4030104c3cea5e1567cfbcb5adb2a7b993b4e Mon Sep 17 00:00:00 2001 From: Yash Janoria <44284086+Yash2412@users.noreply.github.com> Date: Sun, 9 Aug 2026 05:26:07 +0530 Subject: [PATCH] feat(security): add guest user attributes and get_guest_user_attribute() macro (#33924) Co-authored-by: Yash Janoria Co-authored-by: Evan Co-authored-by: Claude Opus 4.8 (1M context) Co-authored-by: Evan Rusackas --- .../configuration/sql-templating.mdx | 70 + docs/static/resources/openapi.json | 7 + superset/connectors/sqla/models.py | 5 + superset/jinja_context.py | 155 ++- superset/security/api.py | 3 + superset/security/guest_token.py | 1 + tests/integration_tests/security/api_tests.py | 168 +++ tests/integration_tests/security_tests.py | 182 +++ .../unit_tests/connectors/sqla/models_test.py | 37 + tests/unit_tests/jinja_context_test.py | 1132 +++++++++++++++++ 10 files changed, 1743 insertions(+), 17 deletions(-) diff --git a/docs/admin_docs/configuration/sql-templating.mdx b/docs/admin_docs/configuration/sql-templating.mdx index f55bd46846d..6f8b2c31f01 100644 --- a/docs/admin_docs/configuration/sql-templating.mdx +++ b/docs/admin_docs/configuration/sql-templating.mdx @@ -315,6 +315,76 @@ 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 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. + +If you have caching enabled in your Superset configuration, then by default the resolved value (whether it +came from the guest token, a null attribute, or the provided default) 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. Including the resolved value on every branch ensures two guests +whose tokens render different SQL never share a cache entry. + +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, but only do so +when the value cannot affect the query results: + +``` +{{ get_guest_user_attribute('department', add_to_cache_keys=False) }} +``` + +You can also provide a default value if the attribute is not found: + +``` +{{ get_guest_user_attribute('region', default='US') }} +``` + +Here's a concrete example of using guest user attributes in a query: + +```sql +SELECT * +FROM sales_data +WHERE region = '{{ get_guest_user_attribute("user_region", default="global") }}' + AND department = '{{ get_guest_user_attribute("department") }}' +``` + +:::warning[Security Warning] + +Guest token attributes come from the embedding application. By default, +`get_guest_user_attribute()` escapes string values — including strings nested inside +arrays and object values, and caller-supplied defaults — through the database dialect's +literal rendering (the same mechanism as `url_param()`). This covers dialect-specific +escape characters such as the backslash on MySQL/MariaDB, so the example above is safe +to interpolate directly. If you pass `escape_result=False`, or interpolate non-string +values (numbers, booleans), you are responsible for validating or allowlisting the +values, since they originate outside Superset. + +If a guest attribute is an array and you plan to pipe it through the `|where_in` filter +(for example `full_name IN {{ get_guest_user_attribute('names')|where_in }}`), call +`get_guest_user_attribute('names', escape_result=False)`. `where_in` already applies its +own dialect-safe quoting, so escaping the values twice can corrupt them (a value such as +`O'Brien` would come back doubly escaped and match nothing). + +Only individual string values are escaped as SQL literals. Interpolating an entire array +or object directly (rather than through `|where_in`, or by accessing a specific element) +renders Python's string form of that structure, which is not valid SQL, and object keys +are not escaped at all. Use `|where_in` for arrays, `|tojson` where you need a +JSON-stringified value, or read individual keys/elements out of the structure yourself. + +The same double-escaping problem described above for `|where_in` applies to `|tojson`: +pass `escape_result=False` before piping to `|tojson` (for example +`{{ get_guest_user_attribute('profile', escape_result=False)|tojson }}`), since JSON +already handles its own quoting and re-escaping a value first would corrupt it (a nested +string such as `O'Brien` would come back as the altered `O''Brien` in the serialized +JSON). + +::: + ### Explicitly Including Values in Cache Key The `{{ cache_key_wrapper() }}` function explicitly instructs Superset to add a value to the diff --git a/docs/static/resources/openapi.json b/docs/static/resources/openapi.json index 357280686a8..ac0264764ce 100644 --- a/docs/static/resources/openapi.json +++ b/docs/static/resources/openapi.json @@ -11383,6 +11383,13 @@ }, "User3": { "properties": { + "attributes": { + "additionalProperties": { + "nullable": true + }, + "nullable": true, + "type": "object" + }, "first_name": { "type": "string" }, diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py index f750d8802c7..a221ef3faff 100644 --- a/superset/connectors/sqla/models.py +++ b/superset/connectors/sqla/models.py @@ -2199,6 +2199,11 @@ class SqlaTable( templatable_statements += [ f.clause for f in security_manager.get_rls_filters(self) ] + if is_feature_enabled("EMBEDDED_SUPERSET"): + # Guest-token RLS clauses are templated when the query is built, so + # macros they contain (e.g. get_guest_user_attribute) must also + # trigger extra cache key extraction. + templatable_statements += security_manager.get_guest_rls_filters_str(self) for statement in templatable_statements: if ExtraCache.regex.search(statement): return True diff --git a/superset/jinja_context.py b/superset/jinja_context.py index 806a82280c7..04fc082f540 100644 --- a/superset/jinja_context.py +++ b/superset/jinja_context.py @@ -61,6 +61,7 @@ if TYPE_CHECKING: from superset.connectors.sqla.models import SqlaTable from superset.models.core import Database from superset.models.sql_lab import Query + from superset.security.guest_token import GuestToken logger = logging.getLogger(__name__) @@ -88,6 +89,11 @@ ALLOWED_TYPES = ( ) COLLECTION_TYPES = ("list", "dict", "tuple", "set") +# Type alias for JSON-native types +JsonValue = Union[ + str, int, float, bool, list["JsonValue"], dict[str, "JsonValue"], None +] + @lru_cache(maxsize=LRU_CACHE_MAX_SIZE) def context_addons() -> dict[str, Any]: @@ -112,6 +118,25 @@ class TimeFilter: time_range: str | None +def _normalize_postgresql_backslash_escapes(dialect: Dialect) -> None: + """Correct a PostgreSQL dialect instance's ``_backslash_escapes`` default + in place so backslashes round-trip unchanged when the dialect is used to + render literals without a live connection. + + A dialect built without a live connection (as ``Database.get_dialect()`` + does) defaults ``_backslash_escapes`` to ``True``, which would double + every backslash even though every supported PostgreSQL version treats + the backslash as a plain character by default + (``standard_conforming_strings`` has been on since PostgreSQL 9.1). Left + uncorrected, a value like ``C:\\Users`` would be rewritten to + ``C:\\\\Users`` and silently fail to match the original value. Other + dialects (for example MySQL/MariaDB, which do treat the backslash as an + escape character) are left untouched. + """ + if dialect.name == "postgresql": + dialect._backslash_escapes = False + + class ExtraCache: """ Dummy class that exposes a method used to store additional values used in @@ -128,7 +153,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\([^)]*\)" r")" r"[^{}]*?(\}\}|\%\})" ) @@ -295,15 +321,88 @@ class ExtraCache: result = url_params.get(param, default) # Escape the value regardless of its source (request args or form # data); both are interpolated into the rendered SQL. - if result and escape_result and self.dialect: - # use the dialect specific quoting logic to escape string - result = String().literal_processor(dialect=self.dialect)(value=result)[ - 1:-1 - ] + if result and escape_result: + # use the dialect-specific literal rendering to escape the string + result = self._escape_value(result) if add_to_cache_keys: 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 resolved value should be included in the + cache key. The resolved value is keyed on every branch (including + the default and null) so two principals whose tokens render + different SQL never share a cache entry. Opting out is only safe + when the value cannot affect the query results. + escape_result: Escape string values (including strings nested inside + lists and object values) through the database dialect's literal + rendering 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, in which case + the template author is responsible for validating the value. Pass + ``escape_result=False`` when piping a list-valued attribute + through the ``where_in`` filter: ``where_in`` applies its own + dialect-safe quoting, so leaving the default escaping on would + escape each value twice. + + 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" + full_name IN {{ get_guest_user_attribute('names', escape_result=False) + |where_in }} + """ + + result: JsonValue = default + # The macro only applies to guest users (embedded). is_guest_user() + # handles the feature-flag and request-context checks internally. + if security_manager.is_guest_user(): + token: GuestToken = g.user.guest_token + user_attributes: dict[str, JsonValue] = ( + token.get("user", {}).get("attributes") or {} + ) + result = user_attributes.get(attribute_name, default) + + if add_to_cache_keys: + # Key the resolved value on every branch (attribute, default, or + # null); a guest whose attribute is absent renders different SQL + # than one whose attribute is set, so both must contribute to the + # cache key. json.dumps gives a stable serialization for 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 (and caller-supplied defaults) are interpolated into + # the rendered SQL, so escape strings with the dialect's literal + # rendering by default, mirroring url_param. Non-string JSON types pass + # through. + if escape_result: + result = self._escape_value(result) + return result + def filter_values( self, column: str, default: str | None = None, remove_filter: bool = False ) -> list[Any]: @@ -348,22 +447,37 @@ class ExtraCache: def _escape_value(self, val: Any) -> Any: """Return a dialect-quoted form of ``val`` suitable for direct SQL interpolation. When no dialect is configured the value is returned - unchanged so callers see the raw value as before. Strings are - passed through SQLAlchemy's ``String`` literal processor (with the - surrounding quotes stripped, mirroring ``url_param``). Lists are - processed element-wise; non-string members are left as-is. + unchanged so callers see the raw value as before. + + Strings are rendered through the dialect compiler's + ``render_literal_value`` (with the surrounding quotes stripped), + which applies dialect-specific escaping beyond quote doubling; in + particular, MySQL/MariaDB treat the backslash as an escape + character, so backslashes are doubled there to prevent a trailing + ``\\'`` from re-opening the string literal. Dialects whose escaping + mode cannot be introspected without a live connection err on the + side of over-escaping, which can distort a backslash-containing + value but can never widen the query. + + PostgreSQL is special-cased via ``_normalize_postgresql_backslash_escapes`` + to restore parity with PostgreSQL's default configuration, while + MySQL/MariaDB keep the stricter, backslash-doubling behavior above. + + Lists are processed element-wise and dict values recursively, so + strings nested inside JSON structures are also escaped; dict keys + are left untouched since they are used for member lookups, not + interpolation. Non-string leaf values are left as-is. """ if not self.dialect: return val if isinstance(val, str): - return String().literal_processor(dialect=self.dialect)(value=val)[1:-1] + compiler = self.dialect.statement_compiler(self.dialect, None) + _normalize_postgresql_backslash_escapes(compiler.dialect) + return compiler.render_literal_value(val, String())[1:-1] if isinstance(val, list): - return [ - String().literal_processor(dialect=self.dialect)(value=v)[1:-1] - if isinstance(v, str) - else v - for v in val - ] + return [self._escape_value(v) for v in val] + if isinstance(val, dict): + return {k: self._escape_value(v) for k, v in val.items()} return val def get_filters(self, column: str, remove_filter: bool = False) -> list[Filter]: @@ -672,6 +786,10 @@ def validate_template_context( class WhereInMacro: # pylint: disable=too-few-public-methods def __init__(self, dialect: Dialect): + # Without this, a PostgreSQL value like ``C:\Users`` would render as + # ``C:\\Users`` and silently fail to match the original value; see + # ``_normalize_postgresql_backslash_escapes`` for the full rationale. + _normalize_postgresql_backslash_escapes(dialect) self.dialect = dialect def __call__( @@ -917,6 +1035,9 @@ class JinjaTemplateProcessor(BaseTemplateProcessor): "get_filters": partial(safe_proxy, extra_cache.get_filters), "dataset": partial(safe_proxy, dataset_macro), "get_time_filter": partial(safe_proxy, extra_cache.get_time_filter), + "get_guest_user_attribute": partial( + safe_proxy, extra_cache.get_guest_user_attribute + ), } ) diff --git a/superset/security/api.py b/superset/security/api.py index c85408c76f6..bb8995d4665 100644 --- a/superset/security/api.py +++ b/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 + ) class ResourceSchema(PermissiveSchema): diff --git a/superset/security/guest_token.py b/superset/security/guest_token.py index 884d41d827c..82111e4be72 100644 --- a/superset/security/guest_token.py +++ b/superset/security/guest_token.py @@ -116,6 +116,7 @@ class GuestTokenUser(TypedDict, total=False): username: str first_name: str last_name: str + attributes: Optional[dict[str, Any]] class GuestTokenResourceType(StrEnum): diff --git a/tests/integration_tests/security/api_tests.py b/tests/integration_tests/security/api_tests.py index 2b9d4ef0231..4f7e67f7ffd 100644 --- a/tests/integration_tests/security/api_tests.py +++ b/tests/integration_tests/security/api_tests.py @@ -195,6 +195,174 @@ class TestSecurityGuestTokenApi(SupersetTestCase): 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) -> None: + """ + 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) -> None: + """ + 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) -> None: + """ + 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) -> None: + """ + 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 + assert user == decoded_token["user"] + assert "attributes" not in decoded_token["user"] + assert resource == decoded_token["resources"][0] + @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") def test_post_guest_token_bad_resources(self): self.login(ADMIN_USERNAME) diff --git a/tests/integration_tests/security_tests.py b/tests/integration_tests/security_tests.py index 3cb2b1e2099..b21973ed30a 100644 --- a/tests/integration_tests/security_tests.py +++ b/tests/integration_tests/security_tests.py @@ -37,6 +37,12 @@ from superset.errors import ErrorLevel, SupersetError, SupersetErrorType from superset.exceptions import SupersetSecurityException from superset.models.core import Database from superset.models.slice import Slice +from superset.security.guest_token import ( + GuestTokenResource, + GuestTokenResourceType, + GuestTokenRlsRule, + GuestTokenUser, +) from superset.sql.parse import Table from superset.utils.core import ( DatasourceType, @@ -2331,6 +2337,182 @@ class TestGuestTokens(SupersetTestCase): assert guest_user is not None assert "test_guest" == guest_user.username + def create_guest_token_with_attributes(self) -> bytes: + user: GuestTokenUser = { + "username": "test_guest_with_attrs", + "first_name": "Test", + "last_name": "Guest", + "attributes": { + "department": "Engineering", + "region": "US", + "role": "developer", + "team": "data-platform", + }, + } + resources: list[GuestTokenResource] = [ + {"type": GuestTokenResourceType.DASHBOARD, "id": "test-dashboard"} + ] + rls: list[GuestTokenRlsRule] = [{"dataset": "1", "clause": "access = 1"}] + return security_manager.create_guest_access_token(user, resources, rls) + + def test_create_guest_access_token_with_attributes(self) -> None: + """Test creating guest access token with user attributes.""" + user_with_attributes: GuestTokenUser = { + "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: list[GuestTokenResource] = [ + {"type": GuestTokenResourceType.DASHBOARD, "id": "test-dashboard"} + ] + rls: list[GuestTokenRlsRule] = [{"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) -> None: + """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 + token_attributes = token_user["attributes"] + assert token_attributes is not None + assert token_attributes["department"] == "Engineering" + assert token_attributes["region"] == "US" + assert token_attributes["role"] == "developer" + assert token_attributes["team"] == "data-platform" + + def test_create_guest_access_token_without_attributes(self) -> None: + """Test creating guest access token without user attributes. + + This test ensures backward compatibility. + """ + user_without_attributes: GuestTokenUser = { + "username": "test_guest_no_attrs", + "first_name": "Test", + "last_name": "Guest", + } + resources: list[GuestTokenResource] = [ + {"type": GuestTokenResourceType.DASHBOARD, "id": "test-dashboard"} + ] + rls: list[GuestTokenRlsRule] = [{"dataset": "1", "clause": "id = 1"}] + + token = security_manager.create_guest_access_token( + user_without_attributes, resources, rls + ) + + # Decode and verify the token works without 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" not in user + assert user["username"] == "test_guest_no_attrs" + + def test_create_guest_access_token_with_empty_attributes(self) -> None: + """Test creating guest access token with empty attributes.""" + user_with_empty_attributes: GuestTokenUser = { + "username": "test_guest_empty_attrs", + "first_name": "Test", + "last_name": "Guest", + "attributes": {}, + } + resources: list[GuestTokenResource] = [ + {"type": GuestTokenResourceType.DASHBOARD, "id": "test-dashboard"} + ] + rls: list[GuestTokenRlsRule] = [{"dataset": "1", "clause": "id = 1"}] + + token = security_manager.create_guest_access_token( + user_with_empty_attributes, resources, rls + ) + + # Decode and verify the token contains empty 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"] == {} + + def test_create_guest_access_token_with_null_attributes(self) -> None: + """Test creating guest access token with null attributes.""" + user_with_null_attributes: GuestTokenUser = { + "username": "test_guest_null_attrs", + "first_name": "Test", + "last_name": "Guest", + "attributes": None, + } + resources: list[GuestTokenResource] = [ + {"type": GuestTokenResourceType.DASHBOARD, "id": "test-dashboard"} + ] + rls: list[GuestTokenRlsRule] = [{"dataset": "1", "clause": "id = 1"}] + + token = security_manager.create_guest_access_token( + user_with_null_attributes, resources, rls + ) + + # Decode and verify the token contains null 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"] is None + def test_get_guest_user_with_request_form(self): token = self.create_guest_token() fake_request = FakeRequest() diff --git a/tests/unit_tests/connectors/sqla/models_test.py b/tests/unit_tests/connectors/sqla/models_test.py index 6543c69388c..f1abb10bf47 100644 --- a/tests/unit_tests/connectors/sqla/models_test.py +++ b/tests/unit_tests/connectors/sqla/models_test.py @@ -1264,3 +1264,40 @@ def test_validate_stored_expression_rejects_subquery_around_jinja( None, "(SELECT password FROM ab_user LIMIT 1) {# x #}", ) + + +def test_has_extra_cache_key_calls_scans_guest_token_rls( + mocker: MockerFixture, +) -> None: + """ + Guest-token RLS clauses are templated when the query is built, so a macro + appearing only in a guest-token RLS clause must still trigger extra cache + key extraction; otherwise its value never reaches the cache key and two + guests can share a cache entry. + """ + mocker.patch( + "superset.connectors.sqla.models.is_feature_enabled", + side_effect=lambda flag: flag == "EMBEDDED_SUPERSET", + ) + mocker.patch( + "superset.connectors.sqla.models.security_manager.get_rls_filters", + return_value=[], + ) + get_guest_rls = mocker.patch( + "superset.connectors.sqla.models.security_manager.get_guest_rls_filters", + return_value=[ + {"clause": "tenant = '{{ get_guest_user_attribute(\"tenant\") }}'"} + ], + ) + + table = SqlaTable( + table_name="tenanted", + sql="SELECT 1 AS tenant", + database=Database(database_name="db", sqlalchemy_uri="sqlite://"), + ) + query_obj: QueryObjectDict = {"metrics": [], "columns": [], "extras": {}} + + assert table.has_extra_cache_key_calls(query_obj) is True + + get_guest_rls.return_value = [{"clause": "tenant = 'acme'"}] + assert table.has_extra_cache_key_calls(query_obj) is False diff --git a/tests/unit_tests/jinja_context_test.py b/tests/unit_tests/jinja_context_test.py index a5d63ac6dc3..4b067ddfb29 100644 --- a/tests/unit_tests/jinja_context_test.py +++ b/tests/unit_tests/jinja_context_test.py @@ -43,6 +43,7 @@ from superset.jinja_context import ( dataset_macro, ExtraCache, get_template_processor, + JsonValue, metric_macro, safe_proxy, TimeFilter, @@ -576,6 +577,19 @@ def test_url_param_unescaped_request_args() -> None: assert cache.url_param("foo", escape_result=False) == "O'Brien" +def test_url_param_postgres_backslash_preserved() -> None: + """ + Test that backslashes are left untouched on PostgreSQL. Every supported + PostgreSQL version has ``standard_conforming_strings`` on by default, so + the backslash is not an escape character there; doubling it would rewrite + a value like ``C:\\Users`` to ``C:\\\\Users`` and silently fail to match + the original value. + """ + with current_app.test_request_context(query_string={"foo": r"C:\Users"}): + cache = ExtraCache(dialect=dialect()) + assert cache.url_param("foo") == r"C:\Users" + + def test_safe_proxy_primitive() -> None: """ Test the ``safe_proxy`` helper with a function returning a ``str``. @@ -883,6 +897,20 @@ def test_where_in_empty_list() -> None: assert where_in([], default_to_none=True) is None +def test_where_in_postgres_backslash_preserved() -> None: + """ + Test that ``where_in`` leaves backslashes untouched on PostgreSQL. A + dialect instance built without a live connection (as + ``Database.get_dialect()`` does) defaults ``_backslash_escapes`` to + ``True``, which would double every backslash even though every + supported PostgreSQL version treats the backslash as a plain character + by default; doubling it would rewrite a value like ``C:\\Users`` to + ``C:\\\\Users`` and silently fail to match the original value. + """ + where_in = WhereInMacro(dialect()) + assert where_in([r"C:\Users"]) == r"('C:\Users')" + + @pytest.mark.parametrize( "value,format,output", [ @@ -2104,6 +2132,7 @@ def test_undefined_template_variable_not_function(mocker: MockerFixture) -> None ("SELECT {{ current_user_email() }}", True), ("SELECT {{ current_user_roles() }}", True), ("SELECT {{ current_user_rls_rules() }}", True), + ("SELECT {{ get_guest_user_attribute('department') }}", True), ("SELECT 'cache_key_wrapper(abc)' AS false_positive", False), ("SELECT 1", False), ("SELECT '{{ 1 + 1 }}'", False), @@ -2113,6 +2142,1109 @@ def test_extra_cache_regex(sql: str, expected: bool) -> None: assert bool(ExtraCache.regex.search(sql)) is expected +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.security_manager.is_guest_user", return_value=False) + + 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.security_manager.is_guest_user", 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_escaped(mocker: MockerFixture) -> None: + """ + Test that string attribute values are dialect-escaped by default so they are + safe to interpolate directly into SQL, mirroring ``url_param``. + """ + mocker.patch("superset.security_manager.is_guest_user", return_value=True) + mock_g = mocker.patch("superset.jinja_context.g") + guest_user = mocker.Mock() + guest_user.is_guest_user = True + guest_user.guest_token = { + "user": {"username": "test_guest", "attributes": {"region": "O'Brien"}}, + "resources": [{"type": "dashboard", "id": "test-id"}], + "rls_rules": [], + } + mock_g.user = guest_user + + cache = ExtraCache(dialect=dialect()) + assert cache.get_guest_user_attribute("region") == "O''Brien" + + +def test_get_guest_user_attribute_unescaped(mocker: MockerFixture) -> None: + """ + Test that ``escape_result=False`` returns the raw attribute value. + """ + mocker.patch("superset.security_manager.is_guest_user", return_value=True) + mock_g = mocker.patch("superset.jinja_context.g") + guest_user = mocker.Mock() + guest_user.is_guest_user = True + guest_user.guest_token = { + "user": {"username": "test_guest", "attributes": {"region": "O'Brien"}}, + "resources": [{"type": "dashboard", "id": "test-id"}], + "rls_rules": [], + } + mock_g.user = guest_user + + cache = ExtraCache(dialect=dialect()) + assert cache.get_guest_user_attribute("region", escape_result=False) == "O'Brien" + + +def test_get_guest_user_attribute_mysql_backslash_escaped( + mocker: MockerFixture, +) -> None: + """ + Test that backslashes are escaped on MySQL, where the backslash is an + escape character. Without doubling it, a value like ``x\\' OR 1=1 -- `` + would render to ``'x\\'' OR 1=1 -- '``, which MySQL parses as the string + ``x'`` followed by an injected predicate. + """ + mocker.patch("superset.security_manager.is_guest_user", return_value=True) + mock_g = mocker.patch("superset.jinja_context.g") + guest_user = mocker.Mock() + guest_user.is_guest_user = True + guest_user.guest_token = { + "user": {"username": "test_guest", "attributes": {"attr": "x\\' OR 1=1 -- "}}, + "resources": [{"type": "dashboard", "id": "test-id"}], + "rls_rules": [], + } + mock_g.user = guest_user + + cache = ExtraCache(dialect=mysql.dialect()) + # Both the backslash and the quote are doubled, so the rendered literal + # stays a single string on MySQL + assert cache.get_guest_user_attribute("attr") == "x\\\\'' OR 1=1 -- " + + +def test_get_guest_user_attribute_postgres_backslash_preserved( + mocker: MockerFixture, +) -> None: + """ + Test that backslashes are left untouched on PostgreSQL, where the + backslash is not an escape character by default (every supported version + has ``standard_conforming_strings`` on). A dialect instance built without + a live connection defaults to the opposite assumption, which would double + the backslash and silently corrupt the value. + """ + mocker.patch("superset.security_manager.is_guest_user", return_value=True) + mock_g = mocker.patch("superset.jinja_context.g") + guest_user = mocker.Mock() + guest_user.is_guest_user = True + guest_user.guest_token = { + "user": {"username": "test_guest", "attributes": {"attr": r"C:\Users"}}, + "resources": [{"type": "dashboard", "id": "test-id"}], + "rls_rules": [], + } + mock_g.user = guest_user + + cache = ExtraCache(dialect=dialect()) + assert cache.get_guest_user_attribute("attr") == r"C:\Users" + + +def test_get_guest_user_attribute_default_escaped(mocker: MockerFixture) -> None: + """ + Test that a caller-supplied default is routed through the same escaping as + attribute values, so the macro's contract holds on every branch. + """ + mocker.patch("superset.security_manager.is_guest_user", return_value=False) + + cache = ExtraCache(dialect=dialect()) + assert cache.get_guest_user_attribute("attr", "O'Brien") == "O''Brien" + assert ( + cache.get_guest_user_attribute("attr", "O'Brien", escape_result=False) + == "O'Brien" + ) + + +def test_get_guest_user_attribute_nested_strings_escaped( + mocker: MockerFixture, +) -> None: + """ + Test that strings nested inside lists and dict values are escaped, while + dict keys (used for member lookups, not interpolation) are left untouched. + """ + mocker.patch("superset.security_manager.is_guest_user", return_value=True) + mock_g = mocker.patch("superset.jinja_context.g") + guest_user = mocker.Mock() + guest_user.is_guest_user = True + guest_user.guest_token = { + "user": { + "username": "test_guest", + "attributes": { + "tenant": {"id": "foo' OR 1=1 --", "names": ["O'Brien", 42]}, + }, + }, + "resources": [{"type": "dashboard", "id": "test-id"}], + "rls_rules": [], + } + mock_g.user = guest_user + + cache = ExtraCache(dialect=dialect()) + assert cache.get_guest_user_attribute("tenant") == { + "id": "foo'' OR 1=1 --", + "names": ["O''Brien", 42], + } + + +def test_get_guest_user_attribute_cache_keys_collision_free( + mocker: MockerFixture, +) -> None: + """ + Test that the resolved value is keyed on every branch, so principals whose + tokens render different SQL never produce the same extra cache keys. + + Mirrors the collision-free guarantees proven for the ``current_user_*`` + macros: distinct attribute values, a null attribute, a missing attribute + falling back to the default, and the non-guest branch must all key + distinctly from one another. + """ + mock_g = mocker.patch("superset.jinja_context.g") + + def keys_for(is_guest: bool, attributes: Any) -> list[Any]: + mocker.patch("superset.security_manager.is_guest_user", return_value=is_guest) + guest_user = mocker.Mock() + guest_user.guest_token = { + "user": {"username": "test_guest", "attributes": attributes}, + "resources": [{"type": "dashboard", "id": "test-id"}], + "rls_rules": [], + } + mock_g.user = guest_user + cache = ExtraCache(extra_cache_keys=[]) + cache.get_guest_user_attribute("tenant", "fallback") + return cache.extra_cache_keys or [] + + scenarios = [ + keys_for(True, {"tenant": "acme"}), + keys_for(True, {"tenant": "initech"}), + keys_for(True, {"tenant": None}), + keys_for(True, {}), # falls back to the default + keys_for(False, None), # non-guest branch, also the default + ] + # Every branch contributes a key + assert all(len(keys) == 1 for keys in scenarios) + # Distinct resolved values yield distinct keys + assert len({keys[0] for keys in scenarios[:3]}) == 3 + # The default-resolving branches key identically (identical rendered SQL) + # but differently from any set or null attribute + assert scenarios[3] == scenarios[4] + assert scenarios[3][0] not in {keys[0] for keys in scenarios[:3]} + + +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.security_manager.is_guest_user", 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.security_manager.is_guest_user", 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.security_manager.is_guest_user", 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.security_manager.is_guest_user", 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() + cache.cache_key_wrapper = mock_cache_wrapper # type: ignore + + # Test with add_to_cache_keys=True (default) + result = cache.get_guest_user_attribute("department") + assert result == "Engineering" + mock_cache_wrapper.assert_called_once_with( + 'guest_user_attribute:department:"Engineering"' + ) + + # Reset mock + mock_cache_wrapper.reset_mock() + + # A missing attribute resolving to the default is still keyed, so a guest + # with the attribute set never shares a cache entry with one without it + result = cache.get_guest_user_attribute("missing", "fallback") + assert result == "fallback" + mock_cache_wrapper.assert_called_once_with( + 'guest_user_attribute:missing:"fallback"' + ) + + # Reset mock + mock_cache_wrapper.reset_mock() + + # Test with add_to_cache_keys=False + result = cache.get_guest_user_attribute("region", add_to_cache_keys=False) + assert result == "US" + mock_cache_wrapper.assert_not_called() + + +def test_get_guest_user_attribute_none_value_cached(mocker: MockerFixture) -> None: + """ + Test that None values are added to cache keys, so a guest whose attribute + is null does not collide with a guest whose attribute is set. + """ + mocker.patch("superset.security_manager.is_guest_user", return_value=True) + mock_g = mocker.patch("superset.jinja_context.g") + + # Create guest user with attributes including None value + guest_user = mocker.Mock() + guest_user.is_guest_user = True + guest_user.guest_token = { + "user": { + "username": "test_guest", + "attributes": {"department": "Engineering", "nullable_field": None}, + }, + "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() + cache.cache_key_wrapper = mock_cache_wrapper # type: ignore + + # Test None value gets keyed + result = cache.get_guest_user_attribute("nullable_field") + assert result is None + mock_cache_wrapper.assert_called_once_with( + "guest_user_attribute:nullable_field:null" + ) + + +def test_get_guest_user_attribute_various_data_types(mocker: MockerFixture) -> None: + """ + Test that get_guest_user_attribute handles various data types in attributes. + """ + mocker.patch("superset.security_manager.is_guest_user", return_value=True) + mock_g = mocker.patch("superset.jinja_context.g") + + # Create guest user with various data types in attributes + guest_user = mocker.Mock() + guest_user.is_guest_user = True + guest_user.guest_token = { + "user": { + "username": "test_guest", + "attributes": { + "string_attr": "text_value", + "int_attr": 42, + "float_attr": 3.14, + "bool_attr": True, + "list_attr": ["item1", "item2"], + "dict_attr": {"nested": "value"}, + }, + }, + "resources": [{"type": "dashboard", "id": "test-id"}], + "rls_rules": [], + } + mock_g.user = guest_user + + cache = ExtraCache() + + # Test different data types + assert cache.get_guest_user_attribute("string_attr") == "text_value" + assert cache.get_guest_user_attribute("int_attr") == 42 + assert cache.get_guest_user_attribute("float_attr") == 3.14 + assert cache.get_guest_user_attribute("bool_attr") is True + assert cache.get_guest_user_attribute("list_attr") == ["item1", "item2"] + assert cache.get_guest_user_attribute("dict_attr") == {"nested": "value"} + + +def test_guest_token_attributes_support(mocker: MockerFixture) -> None: + """ + Test that guest tokens properly support the attributes field. + """ + from typing import cast + + from superset.security.guest_token import ( + GuestToken, + GuestTokenResourceType, + GuestUser, + ) + + # Create a guest token with attributes + guest_token_with_attributes = cast( + GuestToken, + { + "user": { + "username": "test_guest", + "first_name": "Test", + "last_name": "Guest", + "attributes": { + "department": "Engineering", + "region": "US", + "role": "developer", + "team": "data-platform", + "clearance_level": "standard", + }, + }, + "resources": [ + {"type": GuestTokenResourceType.DASHBOARD, "id": "test-dashboard-id"} + ], + "rls_rules": [], + "iat": 1234567890, + "exp": 1234567890 + 3600, + }, + ) + + mock_role = mocker.Mock() + guest_user = GuestUser(token=guest_token_with_attributes, roles=[mock_role]) + + # Verify guest user has access to the original token + assert hasattr(guest_user, "guest_token") + assert guest_user.guest_token == guest_token_with_attributes + + # Verify attributes are accessible through the token + token_user = guest_user.guest_token["user"] + assert "attributes" in token_user + user_attributes = token_user["attributes"] + assert user_attributes is not None + assert user_attributes["department"] == "Engineering" + assert user_attributes["region"] == "US" + assert user_attributes["role"] == "developer" + + +def test_guest_token_without_attributes(mocker: MockerFixture) -> None: + """ + Test that guest tokens work properly when no attributes are provided. + """ + from typing import cast + + from superset.security.guest_token import ( + GuestToken, + GuestTokenResourceType, + GuestUser, + ) + + # Create a guest token without attributes + guest_token_without_attributes = cast( + GuestToken, + { + "user": { + "username": "test_guest", + "first_name": "Test", + "last_name": "Guest", + }, + "resources": [ + {"type": GuestTokenResourceType.DASHBOARD, "id": "test-dashboard-id"} + ], + "rls_rules": [], + "iat": 1234567890, + "exp": 1234567890 + 3600, + }, + ) + + mock_role = mocker.Mock() + guest_user = GuestUser(token=guest_token_without_attributes, roles=[mock_role]) + + # Verify guest user is created successfully + assert hasattr(guest_user, "guest_token") + assert guest_user.guest_token == guest_token_without_attributes + + # Verify attributes field is not present + token_user = guest_user.guest_token["user"] + assert "attributes" not in token_user + + +def test_guest_token_with_empty_attributes(mocker: MockerFixture) -> None: + """ + Test that guest tokens handle empty attributes gracefully. + """ + from typing import cast + + from superset.security.guest_token import ( + GuestToken, + GuestTokenResourceType, + GuestUser, + ) + + # Create a guest token with empty attributes + guest_token_with_empty_attributes = cast( + GuestToken, + { + "user": { + "username": "test_guest", + "first_name": "Test", + "last_name": "Guest", + "attributes": {}, + }, + "resources": [ + {"type": GuestTokenResourceType.DASHBOARD, "id": "test-dashboard-id"} + ], + "rls_rules": [], + "iat": 1234567890, + "exp": 1234567890 + 3600, + }, + ) + + mock_role = mocker.Mock() + guest_user = GuestUser(token=guest_token_with_empty_attributes, roles=[mock_role]) + + # Verify guest user is created successfully + assert hasattr(guest_user, "guest_token") + + # Verify empty attributes are handled + token_user = guest_user.guest_token["user"] + assert "attributes" in token_user + assert token_user["attributes"] == {} + + +def test_guest_token_with_null_attributes(mocker: MockerFixture) -> None: + """ + Test that guest tokens handle null attributes gracefully. + """ + from typing import cast + + from superset.security.guest_token import ( + GuestToken, + GuestTokenResourceType, + GuestUser, + ) + + # Create a guest token with null attributes + guest_token_with_null_attributes = cast( + GuestToken, + { + "user": { + "username": "test_guest", + "first_name": "Test", + "last_name": "Guest", + "attributes": None, + }, + "resources": [ + {"type": GuestTokenResourceType.DASHBOARD, "id": "test-dashboard-id"} + ], + "rls_rules": [], + "iat": 1234567890, + "exp": 1234567890 + 3600, + }, + ) + + mock_role = mocker.Mock() + guest_user = GuestUser(token=guest_token_with_null_attributes, roles=[mock_role]) + + # Verify guest user is created successfully + assert hasattr(guest_user, "guest_token") + + # Verify null attributes are handled + token_user = guest_user.guest_token["user"] + assert "attributes" in token_user + assert token_user["attributes"] is None + + +def test_get_guest_user_attribute_integration(mocker: MockerFixture) -> None: + """ + Integration test for get_guest_user_attribute with real GuestUser object. + """ + from typing import cast + + from superset.security.guest_token import ( + GuestToken, + GuestTokenResourceType, + GuestUser, + ) + + mocker.patch("superset.security_manager.is_guest_user", return_value=True) + mock_g = mocker.patch("superset.jinja_context.g") + + # Create a real GuestUser object with attributes + guest_token_with_attributes = cast( + GuestToken, + { + "user": { + "username": "integration_test_user", + "first_name": "Integration", + "last_name": "Test", + "attributes": { + "department": "Data Science", + "region": "EU", + "access_level": "premium", + "team_lead": True, + "projects": ["analytics", "ml-platform"], + }, + }, + "resources": [ + { + "type": GuestTokenResourceType.DASHBOARD, + "id": "integration-test-dashboard", + } + ], + "rls_rules": [], + "iat": 1234567890, + "exp": 1234567890 + 3600, + }, + ) + + mock_role = mocker.Mock() + guest_user = GuestUser(token=guest_token_with_attributes, roles=[mock_role]) + mock_g.user = guest_user + + cache = ExtraCache() + mock_cache_wrapper = mocker.Mock() + cache.cache_key_wrapper = mock_cache_wrapper # type: ignore + + # Test various attribute types + assert cache.get_guest_user_attribute("department") == "Data Science" + assert cache.get_guest_user_attribute("region") == "EU" + assert cache.get_guest_user_attribute("access_level") == "premium" + assert cache.get_guest_user_attribute("team_lead") is True + assert cache.get_guest_user_attribute("projects") == ["analytics", "ml-platform"] + + # Test non-existing attribute with default + assert cache.get_guest_user_attribute("non_existing", "default") == "default" + + # Test cache key behavior + mock_cache_wrapper.assert_any_call('guest_user_attribute:department:"Data Science"') + mock_cache_wrapper.assert_any_call('guest_user_attribute:region:"EU"') + mock_cache_wrapper.assert_any_call('guest_user_attribute:access_level:"premium"') + mock_cache_wrapper.assert_any_call("guest_user_attribute:team_lead:true") + + +def test_get_guest_user_attribute_json_value_types(mocker: MockerFixture) -> None: + """ + Test that get_guest_user_attribute properly handles all JSON-native types. + """ + mocker.patch("superset.security_manager.is_guest_user", return_value=True) + mock_g = mocker.patch("superset.jinja_context.g") + + # Create guest user with attributes of various JSON-native types + guest_user = mocker.Mock() + guest_user.is_guest_user = True + guest_user.guest_token = { + "user": { + "username": "test_guest", + "attributes": { + "string_attr": "hello world", + "int_attr": 42, + "float_attr": 3.14159, + "bool_true": True, + "bool_false": False, + "null_attr": None, + "list_attr": [1, "two", 3.0, True, None], + "dict_attr": { + "nested_string": "value", + "nested_int": 123, + "nested_bool": False, + "nested_list": ["a", "b", "c"], + "nested_dict": {"deep": "value"}, + }, + "empty_list": [], + "empty_dict": {}, + }, + }, + "resources": [{"type": "dashboard", "id": "test-id"}], + "rls_rules": [], + } + mock_g.user = guest_user + + cache = ExtraCache() + + # Test string type + result = cache.get_guest_user_attribute("string_attr") + assert result == "hello world" + assert isinstance(result, str) + + # Test integer type + result = cache.get_guest_user_attribute("int_attr") + assert result == 42 + assert isinstance(result, int) + + # Test float type + result = cache.get_guest_user_attribute("float_attr") + assert result == 3.14159 + assert isinstance(result, float) + + # Test boolean types + result = cache.get_guest_user_attribute("bool_true") + assert result is True + assert isinstance(result, bool) + + result = cache.get_guest_user_attribute("bool_false") + assert result is False + assert isinstance(result, bool) + + # Test null/None type + result = cache.get_guest_user_attribute("null_attr") + assert result is None + + # Test list type + result = cache.get_guest_user_attribute("list_attr") + expected_list = [1, "two", 3.0, True, None] + assert result == expected_list + assert isinstance(result, list) + + # Test dict type + result = cache.get_guest_user_attribute("dict_attr") + expected_dict = { + "nested_string": "value", + "nested_int": 123, + "nested_bool": False, + "nested_list": ["a", "b", "c"], + "nested_dict": {"deep": "value"}, + } + assert result == expected_dict + assert isinstance(result, dict) + + # Test empty collections + result = cache.get_guest_user_attribute("empty_list") + assert result == [] + assert isinstance(result, list) + + result = cache.get_guest_user_attribute("empty_dict") + assert result == {} + assert isinstance(result, dict) + + +def test_get_guest_user_attribute_json_value_defaults(mocker: MockerFixture) -> None: + """ + Test that get_guest_user_attribute properly handles default values of all + JSON-native types. + """ + mocker.patch("superset.security_manager.is_guest_user", 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() + + # Test default string + result = cache.get_guest_user_attribute("missing_attr", default="default_string") + assert result == "default_string" + assert isinstance(result, str) + + # Test default integer + result = cache.get_guest_user_attribute("missing_attr", default=100) + assert result == 100 + assert isinstance(result, int) + + # Test default float + result = cache.get_guest_user_attribute("missing_attr", default=2.71) + assert result == 2.71 + assert isinstance(result, float) + + # Test default boolean + result = cache.get_guest_user_attribute("missing_attr", default=True) + assert result is True + assert isinstance(result, bool) + + result = cache.get_guest_user_attribute("missing_attr", default=False) + assert result is False + assert isinstance(result, bool) + + # Test default None + result = cache.get_guest_user_attribute("missing_attr", default=None) + assert result is None + + # Test default list - using JsonValue compatible types + default_list: list[JsonValue] = ["default", "values"] + result = cache.get_guest_user_attribute("missing_attr", default=default_list) + assert result == default_list + assert isinstance(result, list) + + # Test default dict - using JsonValue compatible types + default_dict: dict[str, JsonValue] = {"key": "value", "nested": {"deep": "data"}} + result = cache.get_guest_user_attribute("missing_attr", default=default_dict) + assert result == default_dict + assert isinstance(result, dict) + + # Test default empty collections + empty_list: list[JsonValue] = [] + result = cache.get_guest_user_attribute("missing_attr", default=empty_list) + assert result == [] + assert isinstance(result, list) + + empty_dict: dict[str, JsonValue] = {} + result = cache.get_guest_user_attribute("missing_attr", default=empty_dict) + assert result == {} + assert isinstance(result, dict) + + +def test_get_guest_user_attribute_json_cache_key_serialization( + mocker: MockerFixture, +) -> None: + """ + Test that get_guest_user_attribute properly serializes different JSON types + for cache keys. + """ + mocker.patch("superset.security_manager.is_guest_user", return_value=True) + mock_g = mocker.patch("superset.jinja_context.g") + + # Create guest user with various attribute types + guest_user = mocker.Mock() + guest_user.is_guest_user = True + guest_user.guest_token = { + "user": { + "username": "test_guest", + "attributes": { + "string_attr": "test_string", + "int_attr": 42, + "float_attr": 3.14, + "bool_attr": True, + "list_attr": ["item1", "item2"], + "dict_attr": { + "key2": "value2", + "key1": "value1", + }, # Unsorted to test sort_keys + "null_attr": None, + "nested_dict": {"level1": {"level2": ["array", "items"]}}, + }, + }, + "resources": [{"type": "dashboard", "id": "test-id"}], + "rls_rules": [], + } + mock_g.user = guest_user + + cache = ExtraCache() + mock_cache_wrapper = mocker.Mock() + cache.cache_key_wrapper = mock_cache_wrapper # type: ignore + + # Test string serialization + cache.get_guest_user_attribute("string_attr") + mock_cache_wrapper.assert_called_with( + 'guest_user_attribute:string_attr:"test_string"' + ) + + # Test integer serialization + mock_cache_wrapper.reset_mock() + cache.get_guest_user_attribute("int_attr") + mock_cache_wrapper.assert_called_with("guest_user_attribute:int_attr:42") + + # Test float serialization + mock_cache_wrapper.reset_mock() + cache.get_guest_user_attribute("float_attr") + mock_cache_wrapper.assert_called_with("guest_user_attribute:float_attr:3.14") + + # Test boolean serialization + mock_cache_wrapper.reset_mock() + cache.get_guest_user_attribute("bool_attr") + mock_cache_wrapper.assert_called_with("guest_user_attribute:bool_attr:true") + + # Test list serialization + mock_cache_wrapper.reset_mock() + cache.get_guest_user_attribute("list_attr") + mock_cache_wrapper.assert_called_with( + 'guest_user_attribute:list_attr:["item1", "item2"]' + ) + + # Test dict serialization (with sorted keys) + mock_cache_wrapper.reset_mock() + cache.get_guest_user_attribute("dict_attr") + mock_cache_wrapper.assert_called_with( + 'guest_user_attribute:dict_attr:{"key1": "value1", "key2": "value2"}' + ) + + # Test None value serialization + mock_cache_wrapper.reset_mock() + cache.get_guest_user_attribute("null_attr") + mock_cache_wrapper.assert_called_with("guest_user_attribute:null_attr:null") + + # Test nested dict serialization + mock_cache_wrapper.reset_mock() + cache.get_guest_user_attribute("nested_dict") + expected_json = '{"level1": {"level2": ["array", "items"]}}' + mock_cache_wrapper.assert_called_with( + f"guest_user_attribute:nested_dict:{expected_json}" + ) + + +def test_get_guest_user_attribute_json_cache_key_consistency( + mocker: MockerFixture, +) -> None: + """ + Test that identical data structures produce identical cache keys regardless + of order. + """ + mocker.patch("superset.security_manager.is_guest_user", return_value=True) + mock_g = mocker.patch("superset.jinja_context.g") + + # Create two guest users with identical dict data but different key order + guest_user1 = mocker.Mock() + guest_user1.is_guest_user = True + guest_user1.guest_token = { + "user": { + "username": "test_guest1", + "attributes": { + "config": {"theme": "dark", "notifications": True, "lang": "en"} + }, + }, + "resources": [{"type": "dashboard", "id": "test-id"}], + "rls_rules": [], + } + + guest_user2 = mocker.Mock() + guest_user2.is_guest_user = True + guest_user2.guest_token = { + "user": { + "username": "test_guest2", + "attributes": { + "config": { + "lang": "en", + "theme": "dark", + "notifications": True, + } # Different order + }, + }, + "resources": [{"type": "dashboard", "id": "test-id"}], + "rls_rules": [], + } + + cache = ExtraCache() + mock_cache_wrapper = mocker.Mock() + cache.cache_key_wrapper = mock_cache_wrapper # type: ignore + + # Test first user + mock_g.user = guest_user1 + cache.get_guest_user_attribute("config") + first_call_args = mock_cache_wrapper.call_args[0][0] + + # Test second user + mock_cache_wrapper.reset_mock() + mock_g.user = guest_user2 + cache.get_guest_user_attribute("config") + second_call_args = mock_cache_wrapper.call_args[0][0] + + # Both should produce the same cache key due to sort_keys=True + assert first_call_args == second_call_args + expected_json = '{"lang": "en", "notifications": true, "theme": "dark"}' + assert first_call_args == f"guest_user_attribute:config:{expected_json}" + + +def test_get_guest_user_attribute_json_edge_cases(mocker: MockerFixture) -> None: + """ + Test edge cases for JSON value handling in get_guest_user_attribute. + """ + mocker.patch("superset.security_manager.is_guest_user", return_value=True) + mock_g = mocker.patch("superset.jinja_context.g") + + # Create guest user with edge case attributes + guest_user = mocker.Mock() + guest_user.is_guest_user = True + guest_user.guest_token = { + "user": { + "username": "test_guest", + "attributes": { + "zero_int": 0, + "zero_float": 0.0, + "false_bool": False, + "empty_string": "", + "whitespace_string": " ", + "special_chars": "Hello \"World\" with 'quotes' and \n newlines", + "unicode_string": "Hello 🌍 World", + "large_number": 9007199254740991, # JavaScript MAX_SAFE_INTEGER + "scientific_notation": 1.23e-4, + "deeply_nested": { + "level1": {"level2": {"level3": {"level4": "deep_value"}}} + }, + }, + }, + "resources": [{"type": "dashboard", "id": "test-id"}], + "rls_rules": [], + } + mock_g.user = guest_user + + cache = ExtraCache() + + # Test zero values (should not be confused with falsy defaults) + assert cache.get_guest_user_attribute("zero_int") == 0 + assert cache.get_guest_user_attribute("zero_float") == 0.0 + assert cache.get_guest_user_attribute("false_bool") is False + + # Test empty string (should not be confused with None) + assert cache.get_guest_user_attribute("empty_string") == "" + assert cache.get_guest_user_attribute("whitespace_string") == " " + + # Test special characters + result = cache.get_guest_user_attribute("special_chars") + assert result == "Hello \"World\" with 'quotes' and \n newlines" + + # Test unicode + result = cache.get_guest_user_attribute("unicode_string") + assert result == "Hello 🌍 World" + + # Test large numbers + result = cache.get_guest_user_attribute("large_number") + assert result == 9007199254740991 + + # Test scientific notation + result = cache.get_guest_user_attribute("scientific_notation") + assert result == 1.23e-4 + + # Test deeply nested structure + result = cache.get_guest_user_attribute("deeply_nested") + expected = {"level1": {"level2": {"level3": {"level4": "deep_value"}}}} + assert result == expected + + # Test accessing nested values works + if isinstance(result, dict): + level1 = result["level1"] + if isinstance(level1, dict): + level2 = level1["level2"] + if isinstance(level2, dict): + level3 = level2["level3"] + if isinstance(level3, dict): + assert level3["level4"] == "deep_value" + + +def test_guest_token_serialization_with_attributes() -> None: + """ + Test that guest tokens with attributes can be serialized/deserialized. + """ + guest_token_data: dict[str, Any] = { + "user": { + "username": "test_user", + "first_name": "Test", + "last_name": "User", + "attributes": { + "department": "Engineering", + "region": "US", + "roles": ["admin", "user"], + "metadata": {"team": "platform", "manager": "jane.doe"}, + }, + }, + "resources": [{"type": "dashboard", "id": "test-id"}], + "rls_rules": [], + "iat": 1234567890, + "exp": 1234567890 + 3600, + } + + # Test serialization + serialized = json.dumps(guest_token_data) + assert serialized is not None + + # Test deserialization + deserialized = json.loads(serialized) + assert deserialized["user"]["attributes"]["department"] == "Engineering" + assert deserialized["user"]["attributes"]["region"] == "US" + assert deserialized["user"]["attributes"]["roles"] == ["admin", "user"] + assert deserialized["user"]["attributes"]["metadata"]["team"] == "platform" + assert deserialized["user"]["attributes"]["metadata"]["manager"] == "jane.doe" + + def test_get_rendered_sql_filter_values_index_error_on_empty_list() -> None: """ A virtual dataset template that indexes into ``filter_values()`` (e.g.