feat(embedded): add hook to allow superset admins to validate guest t…

…oken parameters (#30132)

Co-authored-by: David Markey <markey@rapidraitngs.com>

superset/config.py

@@ -1723,6 +1723,15 @@ def EMAIL_HEADER_MUTATOR(  # pylint: disable=invalid-name,unused-argument
 # Guest token audience for the embedded superset, either string or callable
 GUEST_TOKEN_JWT_AUDIENCE: Callable[[], str] | str | None = None
 
+# A callable that can be supplied to do extra validation of guest token configuration
+# for example certain RLS parameters:
+# lambda x: len(x['rls']) == 1 and "tenant_id=" in x['rls'][0]['clause']
+#
+# Takes the GuestTokenUser dict as an argument
+# Return False from the callable to return a HTTP 400 to the user.
+
+GUEST_TOKEN_VALIDATOR_HOOK = None
+
 # A SQL dataset health check. Note if enabled it is strongly advised that the callable
 # be memoized to aid with performance, i.e.,
 #

superset/security/api.py

@@ -17,7 +17,7 @@
 import logging
 from typing import Any
 
-from flask import request, Response
+from flask import current_app, request, Response
 from flask_appbuilder import expose
 from flask_appbuilder.api import safe
 from flask_appbuilder.security.decorators import permission_name, protect
@@ -27,6 +27,7 @@
 from superset.commands.dashboard.embedded.exceptions import (
     EmbeddedDashboardNotFoundError,
 )
+from superset.exceptions import SupersetGenericErrorException
 from superset.extensions import event_logger
 from superset.security.guest_token import GuestTokenResourceType
 from superset.views.base_api import BaseSupersetApi, statsd_metrics
@@ -148,8 +149,19 @@ def guest_token(self) -> Response:
         try:
             body = guest_token_create_schema.load(request.json)
             self.appbuilder.sm.validate_guest_token_resources(body["resources"])
-
-            # todo validate stuff:
+            guest_token_validator_hook = current_app.config.get(
+                "GUEST_TOKEN_VALIDATOR_HOOK"
+            )
+            # Run validator to ensure the token parameters are OK.
+            if guest_token_validator_hook is not None:
+                if callable(guest_token_validator_hook):
+                    if not guest_token_validator_hook(body):
+                        raise ValidationError(message="Guest token validation failed")
+                else:
+                    raise SupersetGenericErrorException(
+                        message="Guest token validator hook not callable"
+                    )
+            # TODO: Add generic validation:
             # make sure username doesn't reference an existing user
             # check rls rules for validity?
             token = self.appbuilder.sm.create_guest_access_token(

tests/integration_tests/security/api_tests.py

@@ -81,7 +81,7 @@ def test_login(self):
 
 
 class TestSecurityGuestTokenApi(SupersetTestCase):
-    uri = "api/v1/security/guest_token/"  # noqa: F541
+    uri = "api/v1/security/guest_token/"
 
     def test_post_guest_token_unauthenticated(self):
         """
@@ -138,6 +138,85 @@ def test_post_guest_token_bad_resources(self):
         self.assert400(response)
 
 
+@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices", scope="class")
+class TestSecurityGuestTokenApiTokenValidator(SupersetTestCase):
+    uri = "api/v1/security/guest_token/"
+
+    def _get_guest_token_with_rls(self, rls_rule):
+        dash = db.session.query(Dashboard).filter_by(slug="births").first()
+        self.embedded = EmbeddedDashboardDAO.upsert(dash, [])
+        self.login(ADMIN_USERNAME)
+        user = {"username": "bob", "first_name": "Bob", "last_name": "Also Bob"}
+        resource = {"type": "dashboard", "id": str(self.embedded.uuid)}
+        params = {"user": user, "resources": [resource], "rls": [rls_rule]}
+        return self.client.post(
+            self.uri, data=json.dumps(params), content_type="application/json"
+        )
+
+    @with_config({"GUEST_TOKEN_VALIDATOR_HOOK": lambda x: False})
+    def test_guest_token_validator_hook_denied(self):
+        """
+        Security API: Test False case from validator - should be 400
+        """
+        rls_rule = {"dataset": 1, "clause": "tenant_id=123"}
+        self.assert400(self._get_guest_token_with_rls(rls_rule))
+
+    @with_config({"GUEST_TOKEN_VALIDATOR_HOOK": lambda x: True})
+    def test_guest_token_validator_hook_denied_allowed(self):
+        """
+        Security API: Test True case from validator - should be 200
+        """
+        rls_rule = {"dataset": 1, "clause": "tenant_id=123"}
+        self.assert200(self._get_guest_token_with_rls(rls_rule))
+
+    @with_config({"GUEST_TOKEN_VALIDATOR_HOOK": 123})
+    def test_guest_validator_hook_not_callable(self):
+        """
+        Security API: Test validator throws exception when isn't callable
+         should be 500
+        """
+        rls_rule = {"dataset": 1, "clause": "tenant_id=123"}
+        self.assert500(self._get_guest_token_with_rls(rls_rule))
+
+    @with_config({"GUEST_TOKEN_VALIDATOR_HOOK": lambda x: [][0]})
+    def test_guest_validator_hook_throws_exception(self):
+        """
+        Security API: Test validator throws exception - should be 500
+        """
+        rls_rule = {"dataset": 1, "clause": "tenant_id=123"}
+        self.assert500(self._get_guest_token_with_rls(rls_rule))
+
+    @with_config(
+        {
+            "GUEST_TOKEN_VALIDATOR_HOOK": lambda x: len(x["rls"]) == 1
+            and "tenant_id=" in x["rls"][0]["clause"]
+        }
+    )
+    def test_guest_validator_hook_real_world_example_positive(self):
+        """
+        Security API: Test validator real world example, check tenant_id is in clause
+            Positive case
+        """
+        # Test validator real world example, check tenant_id is in clause
+        # Should be 200.
+        rls_rule = {"dataset": 1, "clause": "tenant_id=123"}
+        self.assert200(self._get_guest_token_with_rls(rls_rule))
+
+    @with_config(
+        {
+            "GUEST_TOKEN_VALIDATOR_HOOK": lambda x: len(x["rls"]) == 1
+            and "tenant_id=" in x["rls"][0]["clause"]
+        }
+    )
+    def test_guest_validator_hook_real_world_example_negative(self):
+        """
+        Security API: Test validator real world example, check tenant_id is in clause
+            Negative case
+        """
+        rls_rule = {}
+        self.assert400(self._get_guest_token_with_rls(rls_rule))
+
+
 class TestSecurityRolesApi(SupersetTestCase):
     uri = "api/v1/security/roles/"  # noqa: F541