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 9 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
74 changes: 74 additions & 0 deletions tests/integration_tests/security/api_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import jwt
import pytest

from flask import current_app
from flask_wtf.csrf import generate_csrf
from superset import db
from superset.daos.dashboard import EmbeddedDashboardDAO
Expand Down Expand Up @@ -137,6 +138,79 @@ def test_post_guest_token_bad_resources(self):

self.assert400(response)

@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
def test_guest_token_validator_hook(self):
"""
Security API: Test various scenarios for the GUEST_TOKEN_VALIDATOR_HOOK
"""

self.dash = db.session.query(Dashboard).filter_by(slug="births").first()
self.embedded = EmbeddedDashboardDAO.upsert(self.dash, [])
self.login(ADMIN_USERNAME)
user = {"username": "bob", "first_name": "Bob", "last_name": "Also Bob"}
resource = {"type": "dashboard", "id": str(self.embedded.uuid)}
rls_rule = {"dataset": 1, "clause": "tenant_id=123"}
params = {"user": user, "resources": [resource], "rls": [rls_rule]}

# Test False case from validator - should raise 400
current_app.config["GUEST_TOKEN_VALIDATOR_HOOK"] = lambda x: False
response = self.client.post(
self.uri, data=json.dumps(params), content_type="application/json"
)

self.assert400(response)

# Test True case from validator - should be 200
current_app.config["GUEST_TOKEN_VALIDATOR_HOOK"] = lambda x: True
response = self.client.post(
self.uri, data=json.dumps(params), content_type="application/json"
)

self.assert200(response)
giftig marked this conversation as resolved.
Show resolved Hide resolved

# Test validator is not callable - should be 500
current_app.config["GUEST_TOKEN_VALIDATOR_HOOK"] = 123

response = self.client.post(
self.uri, data=json.dumps(params), content_type="application/json"
)
self.assert500(response)

# Test validator throws exception - should be 500
current_app.config["GUEST_TOKEN_VALIDATOR_HOOK"] = lambda x: [][0]

response = self.client.post(
self.uri, data=json.dumps(params), content_type="application/json"
)
self.assert500(response)

# Test validator real world example, check tenant_id is in clause
# Should be 200.
current_app.config["GUEST_TOKEN_VALIDATOR_HOOK"] = (
lambda x: len(x["rls"]) == 1 and "tenant_id=" in x["rls"][0]["clause"]
)

response = self.client.post(
self.uri, data=json.dumps(params), content_type="application/json"
)
self.assert200(response)

# Test validator real world example, check tenant_id is in clause
# Should be 400 as there is no RLS clause
params["rls"] = []
response = self.client.post(
self.uri, data=json.dumps(params), content_type="application/json"
)
self.assert400(response)

# Revert/Test that the default configuration has no side effects - 200
current_app.config["GUEST_TOKEN_VALIDATOR_HOOK"] = None

response = self.client.post(
self.uri, data=json.dumps(params), content_type="application/json"
)
self.assert200(response)


class TestSecurityRolesApi(SupersetTestCase):
uri = "api/v1/security/roles/" # noqa: F541
Expand Down
Loading