diff --git a/superset/config.py b/superset/config.py index b4ce659493e9c..b714c2e601e57 100644 --- a/superset/config.py +++ b/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., # diff --git a/superset/security/api.py b/superset/security/api.py index 61fd68e6f0e7c..02bf6b7101ea8 100644 --- a/superset/security/api.py +++ b/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( diff --git a/tests/integration_tests/security/api_tests.py b/tests/integration_tests/security/api_tests.py index c2d9b130cdc58..67aecd73b0912 100644 --- a/tests/integration_tests/security/api_tests.py +++ b/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