Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(embedded): add hook to allow superset admins to validate guest token parameters #30132

Merged
merged 16 commits into from
Sep 11, 2024
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions superset/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -1721,6 +1721,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.,
#
Expand Down
18 changes: 15 additions & 3 deletions superset/security/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -148,8 +149,19 @@
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(

Check warning on line 152 in superset/security/api.py

View check run for this annotation

Codecov / codecov/patch

superset/security/api.py#L152

Added line #L152 was not covered by tests
"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")

Check warning on line 159 in superset/security/api.py

View check run for this annotation

Codecov / codecov/patch

superset/security/api.py#L156-L159

Added lines #L156 - L159 were not covered by tests
else:
raise SupersetGenericErrorException(

Check warning on line 161 in superset/security/api.py

View check run for this annotation

Codecov / codecov/patch

superset/security/api.py#L161

Added line #L161 was not covered by tests
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(
Expand Down
79 changes: 79 additions & 0 deletions tests/integration_tests/security/api_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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/" # noqa: F541
giftig marked this conversation as resolved.
Show resolved Hide resolved

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"}
giftig marked this conversation as resolved.
Show resolved Hide resolved
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

Expand Down
Loading