Skip to content

Commit

Permalink
fix(embedded): Hide sensitive payload data from guest users (apache#2…
Browse files Browse the repository at this point in the history
…5878)

(cherry picked from commit 386d4e0)
  • Loading branch information
jfrag1 authored and sadpandajoe committed Dec 11, 2023
1 parent 248586d commit 37b1c4a
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 1 deletion.
1 change: 1 addition & 0 deletions superset-frontend/src/hooks/apiResources/dashboards.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export const useDashboard = (idOrSlug: string | number) =>
(dashboard.json_metadata && JSON.parse(dashboard.json_metadata)) || {},
position_data:
dashboard.position_json && JSON.parse(dashboard.position_json),
owners: dashboard.owners || [],
}),
);

Expand Down
20 changes: 19 additions & 1 deletion superset/dashboards/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@
import re
from typing import Any, Union

from marshmallow import fields, post_load, pre_load, Schema
from marshmallow import fields, post_dump, post_load, pre_load, Schema
from marshmallow.validate import Length, ValidationError

from superset import security_manager
from superset.exceptions import SupersetException
from superset.tags.models import TagType
from superset.utils import core as utils
Expand Down Expand Up @@ -198,6 +199,15 @@ class DashboardGetResponseSchema(Schema):
changed_on_humanized = fields.String(data_key="changed_on_delta_humanized")
is_managed_externally = fields.Boolean(allow_none=True, dump_default=False)

# pylint: disable=unused-argument
@post_dump()
def post_dump(self, serialized: dict[str, Any], **kwargs: Any) -> dict[str, Any]:
if security_manager.is_guest_user():
del serialized["owners"]
del serialized["changed_by_name"]
del serialized["changed_by"]
return serialized


class DatabaseSchema(Schema):
id = fields.Int()
Expand Down Expand Up @@ -247,6 +257,14 @@ class DashboardDatasetSchema(Schema):
normalize_columns = fields.Bool()
always_filter_main_dttm = fields.Bool()

# pylint: disable=unused-argument
@post_dump()
def post_dump(self, serialized: dict[str, Any], **kwargs: Any) -> dict[str, Any]:
if security_manager.is_guest_user():
del serialized["owners"]
del serialized["database"]
return serialized


class BaseDashboardSchema(Schema):
# pylint: disable=unused-argument
Expand Down
43 changes: 43 additions & 0 deletions tests/integration_tests/dashboards/api_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,26 @@ def test_get_dashboard_datasets(self):
expected_values = [0, 1] if backend() == "presto" else [0, 1, 2]
self.assertEqual(result[0]["column_types"], expected_values)

@pytest.mark.usefixtures("load_world_bank_dashboard_with_slices")
@patch("superset.dashboards.schemas.security_manager.has_guest_access")
@patch("superset.dashboards.schemas.security_manager.is_guest_user")
def test_get_dashboard_datasets_as_guest(self, is_guest_user, has_guest_access):
self.login(username="admin")
uri = "api/v1/dashboard/world_health/datasets"
is_guest_user = True
has_guest_access = True
response = self.get_assert_metric(uri, "get_datasets")
self.assertEqual(response.status_code, 200)
data = json.loads(response.data.decode("utf-8"))
dashboard = Dashboard.get("world_health")
expected_dataset_ids = {s.datasource_id for s in dashboard.slices}
result = data["result"]
actual_dataset_ids = {dataset["id"] for dataset in result}
self.assertEqual(actual_dataset_ids, expected_dataset_ids)
for dataset in result:
for excluded_key in ["database", "owners"]:
assert excluded_key not in dataset

@pytest.mark.usefixtures("load_world_bank_dashboard_with_slices")
def test_get_dashboard_datasets_not_found(self):
self.login(username="alpha")
Expand Down Expand Up @@ -409,6 +429,29 @@ def test_get_dashboard(self):
db.session.delete(dashboard)
db.session.commit()

@patch("superset.dashboards.schemas.security_manager.has_guest_access")
@patch("superset.dashboards.schemas.security_manager.is_guest_user")
def test_get_dashboard_as_guest(self, is_guest_user, has_guest_access):
"""
Dashboard API: Test get dashboard as guest
"""
admin = self.get_user("admin")
dashboard = self.insert_dashboard(
"title", "slug1", [admin.id], created_by=admin
)
is_guest_user.return_value = True
has_guest_access.return_value = True
self.login(username="admin")
uri = f"api/v1/dashboard/{dashboard.id}"
rv = self.get_assert_metric(uri, "get")
self.assertEqual(rv.status_code, 200)
data = json.loads(rv.data.decode("utf-8"))
for excluded_key in ["changed_by", "changed_by_name", "owners"]:
assert excluded_key not in data["result"]
# rollback changes
db.session.delete(dashboard)
db.session.commit()

def test_info_dashboard(self):
"""
Dashboard API: Test info
Expand Down

0 comments on commit 37b1c4a

Please sign in to comment.