This is an automated email from the ASF dual-hosted git repository.
msyavuz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 95f61bd223e fix: add parent_slice_id for multilayer charts to embed
(#38243)
95f61bd223e is described below
commit 95f61bd223ee7f72893197c77ec73a06d225443b
Author: Mehmet Salih Yavuz <[email protected]>
AuthorDate: Thu Mar 12 21:21:43 2026 +0300
fix: add parent_slice_id for multilayer charts to embed (#38243)
---
.../src/Multi/Multi.test.tsx | 137 +++++++++++++++++++++
.../legacy-preset-chart-deckgl/src/Multi/Multi.tsx | 2 +
.../src/components/Chart/chartReducers.test.ts | 45 ++++---
superset/security/manager.py | 58 ++++++++-
tests/unit_tests/security/manager_test.py | 114 +++++++++++++++++
5 files changed, 328 insertions(+), 28 deletions(-)
diff --git
a/superset-frontend/plugins/legacy-preset-chart-deckgl/src/Multi/Multi.test.tsx
b/superset-frontend/plugins/legacy-preset-chart-deckgl/src/Multi/Multi.test.tsx
index f1e50c5b589..abce3761ad3 100644
---
a/superset-frontend/plugins/legacy-preset-chart-deckgl/src/Multi/Multi.test.tsx
+++
b/superset-frontend/plugins/legacy-preset-chart-deckgl/src/Multi/Multi.test.tsx
@@ -605,3 +605,140 @@ describe('DeckMulti Component Rendering', () => {
});
});
});
+
+test('includes parent_slice_id in child slice requests when parent has
slice_id', async () => {
+ jest.clearAllMocks();
+ const mockGet = jest.fn().mockResolvedValue({
+ json: {
+ result: {
+ form_data: {
+ viz_type: 'deck_scatter',
+ datasource: '1__table',
+ },
+ },
+ data: {
+ features: [],
+ },
+ },
+ });
+ (SupersetClient.get as jest.Mock) = mockGet;
+ const parentSliceId = 99;
+ const dashboardId = 5;
+
+ const props = {
+ ...baseMockProps,
+ formData: {
+ ...baseMockProps.formData,
+ slice_id: parentSliceId,
+ dashboardId,
+ },
+ };
+
+ renderWithProviders(<DeckMulti {...props} />);
+
+ await waitFor(() => {
+ expect(mockGet).toHaveBeenCalled();
+ });
+
+ // Check that the child slice requests include parent_slice_id
+ const { calls } = mockGet.mock;
+ calls.forEach(call => {
+ const { endpoint } = call[0];
+ if (endpoint.includes('api/v1/explore/form_data')) {
+ const body = JSON.parse(call[0].body);
+ expect(body.form_data).toMatchObject({
+ dashboardId,
+ parent_slice_id: parentSliceId,
+ });
+ }
+ });
+});
+
+test('includes parent_slice_id in embedded mode', async () => {
+ jest.clearAllMocks();
+ const mockGet = jest.fn().mockResolvedValue({
+ json: {
+ result: {
+ form_data: {
+ viz_type: 'deck_scatter',
+ datasource: '1__table',
+ },
+ },
+ data: {
+ features: [],
+ },
+ },
+ });
+ (SupersetClient.get as jest.Mock) = mockGet;
+ const parentSliceId = 200;
+ const dashboardId = 10;
+
+ const props = {
+ ...baseMockProps,
+ formData: {
+ ...baseMockProps.formData,
+ slice_id: parentSliceId,
+ dashboardId,
+ embedded: true,
+ },
+ };
+
+ renderWithProviders(<DeckMulti {...props} />);
+
+ await waitFor(() => {
+ expect(mockGet).toHaveBeenCalled();
+ });
+
+ // Verify parent_slice_id is included in embedded mode
+ const { calls } = mockGet.mock;
+ calls.forEach(call => {
+ const { endpoint } = call[0];
+ if (endpoint.includes('api/v1/explore/form_data')) {
+ const body = JSON.parse(call[0].body);
+ expect(body.form_data.parent_slice_id).toBe(parentSliceId);
+ }
+ });
+});
+
+test('does not include parent_slice_id when parent has no slice_id', async ()
=> {
+ jest.clearAllMocks();
+ const mockGet = jest.fn().mockResolvedValue({
+ json: {
+ result: {
+ form_data: {
+ viz_type: 'deck_scatter',
+ datasource: '1__table',
+ },
+ },
+ data: {
+ features: [],
+ },
+ },
+ });
+ (SupersetClient.get as jest.Mock) = mockGet;
+
+ const props = {
+ ...baseMockProps,
+ formData: {
+ ...baseMockProps.formData,
+ // No slice_id in parent
+ dashboardId: 5,
+ },
+ };
+
+ renderWithProviders(<DeckMulti {...props} />);
+
+ await waitFor(() => {
+ expect(mockGet).toHaveBeenCalled();
+ });
+
+ // Verify parent_slice_id is not included when parent has no slice_id
+ const { calls } = mockGet.mock;
+ calls.forEach(call => {
+ const { endpoint } = call[0];
+ if (endpoint.includes('api/v1/explore/form_data')) {
+ const body = JSON.parse(call[0].body);
+ expect(body.form_data.parent_slice_id).toBeUndefined();
+ }
+ });
+});
diff --git
a/superset-frontend/plugins/legacy-preset-chart-deckgl/src/Multi/Multi.tsx
b/superset-frontend/plugins/legacy-preset-chart-deckgl/src/Multi/Multi.tsx
index c361eb755c8..3c52464a247 100644
--- a/superset-frontend/plugins/legacy-preset-chart-deckgl/src/Multi/Multi.tsx
+++ b/superset-frontend/plugins/legacy-preset-chart-deckgl/src/Multi/Multi.tsx
@@ -289,6 +289,8 @@ const DeckMulti = (props: DeckMultiProps) => {
adhoc_filters: adhocFilters,
// Preserve dashboard context for embedded mode permissions
...(formData.dashboardId && { dashboardId: formData.dashboardId }),
+ // Include parent multilayer chart ID for security checks
+ ...(formData.slice_id && { parent_slice_id: formData.slice_id }),
},
} as any as JsonObject & { slice_id: number };
diff --git a/superset-frontend/src/components/Chart/chartReducers.test.ts
b/superset-frontend/src/components/Chart/chartReducers.test.ts
index ee5010358cb..d5b660dd810 100644
--- a/superset-frontend/src/components/Chart/chartReducers.test.ts
+++ b/superset-frontend/src/components/Chart/chartReducers.test.ts
@@ -35,36 +35,35 @@ describe('chart reducers', () => {
});
test('should update endtime on fail', () => {
- const controller = new AbortController();
- charts[chartKey] = {
- ...charts[chartKey],
- queryController: controller,
- };
- const newState = chartReducer(
- charts,
- actions.chartUpdateStopped(chartKey, controller),
- );
+ const newState = chartReducer(charts,
actions.chartUpdateStopped(chartKey));
expect(newState[chartKey].chartUpdateEndTime).toBeGreaterThan(0);
expect(newState[chartKey].chartStatus).toEqual('stopped');
});
- test('should ignore stopped updates from stale controllers', () => {
- const controller = new AbortController();
- const staleController = new AbortController();
- charts[chartKey] = {
- ...charts[chartKey],
- chartStatus: 'loading',
- queryController: controller,
- };
+ test('should handle chartUpdateStopped without queryController', () => {
+ const newState = chartReducer(charts,
actions.chartUpdateStopped(chartKey));
+ expect(newState[chartKey].chartStatus).toEqual('stopped');
+ expect(newState[chartKey].chartAlert).toContain(
+ 'Updating chart was stopped',
+ );
+ expect(newState[chartKey].chartUpdateEndTime).toBeGreaterThan(0);
+ });
+ test('chartUpdateStopped sets state correctly', () => {
+ const chartsWithController = {
+ [chartKey]: {
+ ...testChart,
+ queryController: new AbortController(),
+ },
+ };
const newState = chartReducer(
- charts,
- actions.chartUpdateStopped(chartKey, staleController),
+ chartsWithController,
+ actions.chartUpdateStopped(chartKey),
);
-
- expect(newState[chartKey].chartStatus).toEqual('loading');
- expect(newState[chartKey].chartUpdateEndTime).toEqual(
- charts[chartKey].chartUpdateEndTime,
+ // Verify the chart status and alert are set
+ expect(newState[chartKey].chartStatus).toEqual('stopped');
+ expect(newState[chartKey].chartAlert).toContain(
+ 'Updating chart was stopped',
);
});
diff --git a/superset/security/manager.py b/superset/security/manager.py
index 2e2ca65017c..32bc75e079e 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -695,6 +695,32 @@ class SupersetSecurityManager( # pylint:
disable=too-many-public-methods
return False
+ def _validate_child_in_parent_multilayer(
+ self, child_slice_id: int, parent_slice: "Slice"
+ ) -> bool:
+ """
+ Validate that a child slice ID is actually configured in the parent
+ multi-layer chart's deck_slices configuration.
+
+ This prevents attackers from forging a parent_slice_id to gain
+ unauthorized access to arbitrary charts.
+ """
+ try:
+ # Parse the parent chart's configuration
+ parent_form_data = json.loads(parent_slice.params or "{}")
+
+ # Check if this is actually a multi-layer deck.gl chart
+ if parent_form_data.get("viz_type") != "deck_multi":
+ return False
+
+ # Get the configured child slices
+ deck_slices = parent_form_data.get("deck_slices", [])
+
+ # Validate the child is in the parent's configuration
+ return child_slice_id in deck_slices
+ except (json.JSONDecodeError, TypeError):
+ return False
+
def has_drill_by_access(
self,
form_data: dict[str, Any],
@@ -2604,12 +2630,34 @@ class SupersetSecurityManager( # pylint:
disable=too-many-public-methods
form_data.get("type") != "NATIVE_FILTER"
and (slice_id := form_data.get("slice_id"))
and (
- slc := self.session.query(Slice)
- .filter(Slice.id == slice_id)
- .one_or_none()
+ # Direct chart access (no parent)
+ (
+ form_data.get("parent_slice_id") is None
+ and (
+ slc := self.session.query(Slice)
+ .filter(Slice.id == slice_id)
+ .one_or_none()
+ )
+ and slc in dashboard_.slices
+ and slc.datasource == datasource
+ )
+ or
+ # Multi-layer chart child access (has parent)
+ (
+ (parent_id :=
form_data.get("parent_slice_id"))
+ and (
+ parent_slc := self.session.query(Slice)
+ .filter(Slice.id == parent_id)
+ .one_or_none()
+ )
+ and parent_slc in dashboard_.slices
+ # Validate child is actually part of
parent's config
+ and
self._validate_child_in_parent_multilayer(
+ child_slice_id=slice_id,
+ parent_slice=parent_slc,
+ )
+ )
)
- and slc in dashboard_.slices
- and slc.datasource == datasource
)
or self.has_drill_by_access(form_data, dashboard_,
datasource)
)
diff --git a/tests/unit_tests/security/manager_test.py
b/tests/unit_tests/security/manager_test.py
index a59b969f1e8..ca4b0b8df8b 100644
--- a/tests/unit_tests/security/manager_test.py
+++ b/tests/unit_tests/security/manager_test.py
@@ -1433,3 +1433,117 @@ def test_prefetch_rls_filters_works_for_guest_user(
# Cache should be populated with (username, table_id) keys and empty lists
assert mock_g._rls_filter_cache[("guest_user", 10)] == []
assert mock_g._rls_filter_cache[("guest_user", 20)] == []
+
+
+def test_validate_child_in_parent_multilayer_valid(
+ app_context: None, mocker: MockerFixture
+) -> None:
+ """Test validation succeeds for valid multi-layer child"""
+ sm = SupersetSecurityManager(appbuilder)
+
+ parent_slice = mocker.MagicMock(spec=Slice)
+ parent_slice.params = json.dumps(
+ {"viz_type": "deck_multi", "deck_slices": [1, 2, 3]}
+ )
+
+ # Child 2 is in parent's deck_slices
+ assert sm._validate_child_in_parent_multilayer(
+ child_slice_id=2, parent_slice=parent_slice
+ )
+
+
+def test_validate_child_in_parent_multilayer_invalid_child(
+ app_context: None, mocker: MockerFixture
+) -> None:
+ """Test validation fails for child not in parent config"""
+ sm = SupersetSecurityManager(appbuilder)
+
+ parent_slice = mocker.MagicMock(spec=Slice)
+ parent_slice.params = json.dumps(
+ {"viz_type": "deck_multi", "deck_slices": [1, 2, 3]}
+ )
+
+ # Child 5 is NOT in parent's deck_slices
+ assert not sm._validate_child_in_parent_multilayer(
+ child_slice_id=5, parent_slice=parent_slice
+ )
+
+
+def test_validate_child_in_parent_multilayer_wrong_viz_type(
+ app_context: None, mocker: MockerFixture
+) -> None:
+ """Test validation fails for non-multilayer charts"""
+ sm = SupersetSecurityManager(appbuilder)
+
+ parent_slice = mocker.MagicMock(spec=Slice)
+ parent_slice.params = json.dumps(
+ {
+ "viz_type": "line", # Not deck_multi
+ "deck_slices": [1, 2, 3],
+ }
+ )
+
+ assert not sm._validate_child_in_parent_multilayer(
+ child_slice_id=2, parent_slice=parent_slice
+ )
+
+
+def test_validate_child_in_parent_multilayer_empty_deck_slices(
+ app_context: None, mocker: MockerFixture
+) -> None:
+ """Test validation fails when deck_slices is empty"""
+ sm = SupersetSecurityManager(appbuilder)
+
+ parent_slice = mocker.MagicMock(spec=Slice)
+ parent_slice.params = json.dumps({"viz_type": "deck_multi", "deck_slices":
[]})
+
+ assert not sm._validate_child_in_parent_multilayer(
+ child_slice_id=1, parent_slice=parent_slice
+ )
+
+
+def test_validate_child_in_parent_multilayer_no_deck_slices(
+ app_context: None, mocker: MockerFixture
+) -> None:
+ """Test validation fails when deck_slices is missing"""
+ sm = SupersetSecurityManager(appbuilder)
+
+ parent_slice = mocker.MagicMock(spec=Slice)
+ parent_slice.params = json.dumps(
+ {
+ "viz_type": "deck_multi"
+ # No deck_slices key
+ }
+ )
+
+ assert not sm._validate_child_in_parent_multilayer(
+ child_slice_id=1, parent_slice=parent_slice
+ )
+
+
+def test_validate_child_in_parent_multilayer_malformed_json(
+ app_context: None, mocker: MockerFixture
+) -> None:
+ """Test validation fails gracefully with malformed JSON"""
+ sm = SupersetSecurityManager(appbuilder)
+
+ parent_slice = mocker.MagicMock(spec=Slice)
+ parent_slice.params = "not valid json {{"
+
+ assert not sm._validate_child_in_parent_multilayer(
+ child_slice_id=1, parent_slice=parent_slice
+ )
+
+
+def test_validate_child_in_parent_multilayer_null_params(
+ app_context: None, mocker: MockerFixture
+) -> None:
+ """Test validation fails gracefully with null params"""
+ sm = SupersetSecurityManager(appbuilder)
+
+ parent_slice = mocker.MagicMock(spec=Slice)
+ parent_slice.params = None
+
+ assert not sm._validate_child_in_parent_multilayer(
+ child_slice_id=1, parent_slice=parent_slice
+ )
