You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ms...@apache.org on 2026/06/05 21:58:07 UTC
(superset) branch msyavuz/fix/matrixify-rls-dimension-values created (now 498adc9722e)
This is an automated email from the ASF dual-hosted git repository.
msyavuz pushed a change to branch msyavuz/fix/matrixify-rls-dimension-values
in repository https://gitbox.apache.org/repos/asf/superset.git
at 498adc9722e fix(explore): apply RLS to matrixify dimension values at render
This branch includes the following new commits:
new 498adc9722e fix(explore): apply RLS to matrixify dimension values at render
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(superset) 01/01: fix(explore): apply RLS to matrixify dimension values at render
Posted by ms...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
msyavuz pushed a commit to branch msyavuz/fix/matrixify-rls-dimension-values
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 498adc9722ee9c9c4742df31e330cddb38c6972c
Author: Mehmet Salih Yavuz <sa...@proton.me>
AuthorDate: Sat Jun 6 00:57:54 2026 +0300
fix(explore): apply RLS to matrixify dimension values at render
---
.../Matrixify/MatrixifyGridRenderer.test.tsx | 88 +++++++++++++
.../components/Matrixify/MatrixifyGridRenderer.tsx | 79 ++++++++++-
.../Matrixify/useMatrixifyAllowedValues.test.ts | 105 +++++++++++++++
.../Matrixify/useMatrixifyAllowedValues.ts | 145 +++++++++++++++++++++
4 files changed, 414 insertions(+), 3 deletions(-)
diff --git a/superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/MatrixifyGridRenderer.test.tsx b/superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/MatrixifyGridRenderer.test.tsx
index 588585372da..ed5330464c7 100644
--- a/superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/MatrixifyGridRenderer.test.tsx
+++ b/superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/MatrixifyGridRenderer.test.tsx
@@ -24,12 +24,21 @@ import { supersetTheme } from '@apache-superset/core/theme';
import MatrixifyGridRenderer from './MatrixifyGridRenderer';
import type { MatrixifyMode } from '../../types/matrixify';
import { generateMatrixifyGrid } from './MatrixifyGridGenerator';
+import { useMatrixifyAllowedValues } from './useMatrixifyAllowedValues';
// Mock the MatrixifyGridGenerator
jest.mock('./MatrixifyGridGenerator', () => ({
generateMatrixifyGrid: jest.fn(),
}));
+// Mock the RLS allow-list hook so the renderer can be tested without network
+jest.mock('./useMatrixifyAllowedValues', () => ({
+ useMatrixifyAllowedValues: jest.fn(() => ({
+ status: 'success',
+ allowedByColumn: {},
+ })),
+}));
+
// Mock MatrixifyGridCell component
jest.mock('./MatrixifyGridCell', () =>
// eslint-disable-next-line react/display-name, @typescript-eslint/no-unused-vars
@@ -41,12 +50,20 @@ jest.mock('./MatrixifyGridCell', () =>
const mockGenerateMatrixifyGrid = generateMatrixifyGrid as jest.MockedFunction<
typeof generateMatrixifyGrid
>;
+const mockUseMatrixifyAllowedValues =
+ useMatrixifyAllowedValues as jest.MockedFunction<
+ typeof useMatrixifyAllowedValues
+ >;
const renderWithTheme = (component: React.ReactElement) =>
render(<ThemeProvider theme={supersetTheme}>{component}</ThemeProvider>);
beforeEach(() => {
jest.clearAllMocks();
+ mockUseMatrixifyAllowedValues.mockReturnValue({
+ status: 'success',
+ allowedByColumn: {},
+ });
});
test('should create single group when fitting columns dynamically', () => {
@@ -417,3 +434,74 @@ test('should use default values for missing configuration', () => {
const gridCells = container.querySelectorAll('[data-testid^="grid-cell-"]');
expect(gridCells).toHaveLength(2);
});
+
+test('shows a loading indicator while the RLS allow-list resolves', () => {
+ mockUseMatrixifyAllowedValues.mockReturnValue({
+ status: 'loading',
+ allowedByColumn: {},
+ });
+
+ const formData = {
+ viz_type: 'test_chart',
+ matrixify_enable: true,
+ matrixify_mode_rows: 'dimensions' as MatrixifyMode,
+ matrixify_dimension_rows: { dimension: 'region', values: ['US'] },
+ };
+
+ const { container } = renderWithTheme(
+ <MatrixifyGridRenderer formData={formData} />,
+ );
+
+ // The grid is never built while resolution is in flight
+ expect(mockGenerateMatrixifyGrid).not.toHaveBeenCalled();
+ const gridCells = container.querySelectorAll('[data-testid^="grid-cell-"]');
+ expect(gridCells).toHaveLength(0);
+});
+
+test('fails closed and renders no cells when the allow-list errors', () => {
+ mockUseMatrixifyAllowedValues.mockReturnValue({
+ status: 'error',
+ allowedByColumn: {},
+ });
+
+ const formData = {
+ viz_type: 'test_chart',
+ matrixify_enable: true,
+ matrixify_mode_rows: 'dimensions' as MatrixifyMode,
+ matrixify_dimension_rows: { dimension: 'region', values: ['US'] },
+ };
+
+ const { container } = renderWithTheme(
+ <MatrixifyGridRenderer formData={formData} />,
+ );
+
+ expect(mockGenerateMatrixifyGrid).not.toHaveBeenCalled();
+ const gridCells = container.querySelectorAll('[data-testid^="grid-cell-"]');
+ expect(gridCells).toHaveLength(0);
+});
+
+test('filters stored dimension values to the RLS allow-list before building the grid', () => {
+ mockUseMatrixifyAllowedValues.mockReturnValue({
+ status: 'success',
+ allowedByColumn: { region: new Set(['US']) },
+ });
+ mockGenerateMatrixifyGrid.mockReturnValue({
+ rowHeaders: [],
+ colHeaders: [],
+ cells: [],
+ } as any);
+
+ const formData = {
+ viz_type: 'test_chart',
+ matrixify_enable: true,
+ matrixify_mode_rows: 'dimensions' as MatrixifyMode,
+ // 'EU' is not in the viewer's allow-list and must be dropped
+ matrixify_dimension_rows: { dimension: 'region', values: ['US', 'EU'] },
+ };
+
+ renderWithTheme(<MatrixifyGridRenderer formData={formData} />);
+
+ expect(mockGenerateMatrixifyGrid).toHaveBeenCalledTimes(1);
+ const passedFormData = mockGenerateMatrixifyGrid.mock.calls[0][0] as any;
+ expect(passedFormData.matrixify_dimension_rows.values).toEqual(['US']);
+});
diff --git a/superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/MatrixifyGridRenderer.tsx b/superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/MatrixifyGridRenderer.tsx
index fc4b6422459..934b452c97f 100644
--- a/superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/MatrixifyGridRenderer.tsx
+++ b/superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/MatrixifyGridRenderer.tsx
@@ -19,8 +19,11 @@
import { useMemo } from 'react';
import { styled } from '@apache-superset/core/theme';
+import { t } from '@apache-superset/core/translation';
+import { Loading } from '../../../components/Loading';
import { MatrixifyFormData } from '../../types/matrixify';
import { generateMatrixifyGrid } from './MatrixifyGridGenerator';
+import { useMatrixifyAllowedValues } from './useMatrixifyAllowedValues';
import MatrixifyGridCell from './MatrixifyGridCell';
// Layout constants
@@ -74,6 +77,17 @@ const GridGroup = styled.div<{ isLast: boolean }>`
margin-bottom: ${({ isLast }) => (isLast ? 0 : GROUP_SPACING)}px;
`;
+const GridMessage = styled.div`
+ display: flex;
+ align-items: center;
+ justify-content: center;
+ width: 100%;
+ height: 100%;
+ padding: ${({ theme }) => theme.sizeUnit * 4}px;
+ text-align: center;
+ color: ${({ theme }) => theme.colorTextTertiary};
+`;
+
const GridHeader = styled.div`
background-color: ${({ theme }) => theme.colorFillAlter};
padding: ${({ theme }) => theme.sizeUnit / 2}px; /* Reduced padding */
@@ -122,10 +136,47 @@ function MatrixifyGridRenderer({
height,
hooks,
}: MatrixifyGridRendererProps) {
- // Generate grid structure from form data
+ // Resolve which dimension values the current viewer is allowed to see, with
+ // row-level security applied server-side. Matrixify axis values are frozen
+ // into formData at design time, so without this the grid would be built from
+ // the chart author's RLS context and leak values to restricted viewers.
+ const { status: allowedStatus, allowedByColumn } =
+ useMatrixifyAllowedValues(formData);
+
+ // Drop any stored axis values the viewer is not entitled to before building
+ // the grid, so forbidden subplots (and their value labels) are never emitted.
+ const sanitizedFormData = useMemo(() => {
+ if (allowedStatus !== 'success') {
+ return formData;
+ }
+ const next: any = { ...formData };
+ (
+ ['matrixify_dimension_rows', 'matrixify_dimension_columns'] as const
+ ).forEach(key => {
+ const dimension = next[key];
+ const allowed =
+ dimension?.dimension && allowedByColumn[dimension.dimension];
+ if (allowed && Array.isArray(dimension.values)) {
+ next[key] = {
+ ...dimension,
+ values: dimension.values.filter((value: any) =>
+ allowed.has(String(value)),
+ ),
+ };
+ }
+ });
+ return next;
+ }, [formData, allowedStatus, allowedByColumn]);
+
+ // Generate grid structure from the RLS-filtered form data. Never build it
+ // until the allow-list resolves, so a grid is never derived from unfiltered
+ // (potentially leaking) values.
const grid = useMemo(
- () => generateMatrixifyGrid(formData as any),
- [formData],
+ () =>
+ allowedStatus === 'success'
+ ? generateMatrixifyGrid(sanitizedFormData as any)
+ : null,
+ [sanitizedFormData, allowedStatus],
);
// Determine layout parameters - only show headers/labels if layout is enabled
@@ -165,6 +216,28 @@ function MatrixifyGridRenderer({
return groups;
}, [grid, fitColumnsDynamically, chartsPerRow]);
+ // Wait for the allow-list before rendering anything, so unfiltered values are
+ // never shown — even briefly — while resolution is in flight.
+ if (allowedStatus === 'loading') {
+ return (
+ <GridContainer height={height}>
+ <Loading />
+ </GridContainer>
+ );
+ }
+
+ // Fail closed: if the allow-list could not be resolved, render nothing rather
+ // than falling back to the unfiltered (leaking) value list.
+ if (allowedStatus === 'error') {
+ return (
+ <GridContainer height={height}>
+ <GridMessage>
+ {t('Unable to verify access to dimension values.')}
+ </GridMessage>
+ </GridContainer>
+ );
+ }
+
if (!grid) {
return null;
}
diff --git a/superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/useMatrixifyAllowedValues.test.ts b/superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/useMatrixifyAllowedValues.test.ts
new file mode 100644
index 00000000000..d04805516ea
--- /dev/null
+++ b/superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/useMatrixifyAllowedValues.test.ts
@@ -0,0 +1,105 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { renderHook, waitFor } from '@testing-library/react';
+import { SupersetClient } from '../../..';
+import { useMatrixifyAllowedValues } from './useMatrixifyAllowedValues';
+
+jest.mock('../../..', () => ({
+ SupersetClient: { get: jest.fn() },
+}));
+
+const mockGet = SupersetClient.get as jest.Mock;
+
+beforeEach(() => {
+ jest.clearAllMocks();
+});
+
+test('resolves immediately without fetching for metrics-only matrixify', async () => {
+ const { result } = renderHook(() =>
+ useMatrixifyAllowedValues({
+ datasource: '1__table',
+ matrixify_enable: true,
+ matrixify_mode_rows: 'metrics',
+ matrixify_mode_columns: 'metrics',
+ } as any),
+ );
+
+ expect(result.current.status).toBe('success');
+ expect(result.current.allowedByColumn).toEqual({});
+ expect(mockGet).not.toHaveBeenCalled();
+});
+
+test('fetches RLS-allowed values for each dimension axis', async () => {
+ mockGet.mockImplementation(({ endpoint }: { endpoint: string }) => {
+ if (endpoint.includes('/column/region/')) {
+ return Promise.resolve({ json: { result: ['US', 'CA'] } });
+ }
+ return Promise.resolve({ json: { result: [2024, 2025] } });
+ });
+
+ const { result } = renderHook(() =>
+ useMatrixifyAllowedValues({
+ datasource: '7__table',
+ matrixify_enable: true,
+ matrixify_mode_rows: 'dimensions',
+ matrixify_dimension_rows: { dimension: 'region', values: ['US', 'EU'] },
+ matrixify_mode_columns: 'dimensions',
+ matrixify_dimension_columns: { dimension: 'year', values: [2024] },
+ } as any),
+ );
+
+ await waitFor(() => expect(result.current.status).toBe('success'));
+
+ expect(mockGet).toHaveBeenCalledTimes(2);
+ expect(result.current.allowedByColumn.region).toEqual(new Set(['US', 'CA']));
+ // values are string-normalized so they can be compared regardless of type
+ expect(result.current.allowedByColumn.year).toEqual(
+ new Set(['2024', '2025']),
+ );
+});
+
+test('fails closed when the values endpoint errors', async () => {
+ mockGet.mockRejectedValue(new Error('boom'));
+
+ const { result } = renderHook(() =>
+ useMatrixifyAllowedValues({
+ datasource: '7__table',
+ matrixify_enable: true,
+ matrixify_mode_rows: 'dimensions',
+ matrixify_dimension_rows: { dimension: 'region', values: ['US'] },
+ } as any),
+ );
+
+ await waitFor(() => expect(result.current.status).toBe('error'));
+ expect(result.current.allowedByColumn).toEqual({});
+});
+
+test('fails closed when datasource is missing', async () => {
+ const { result } = renderHook(() =>
+ useMatrixifyAllowedValues({
+ matrixify_enable: true,
+ matrixify_mode_rows: 'dimensions',
+ matrixify_dimension_rows: { dimension: 'region', values: ['US'] },
+ } as any),
+ );
+
+ await waitFor(() => expect(result.current.status).toBe('error'));
+ expect(mockGet).not.toHaveBeenCalled();
+});
diff --git a/superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/useMatrixifyAllowedValues.ts b/superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/useMatrixifyAllowedValues.ts
new file mode 100644
index 00000000000..7d2e4d7e504
--- /dev/null
+++ b/superset-frontend/packages/superset-ui-core/src/chart/components/Matrixify/useMatrixifyAllowedValues.ts
@@ -0,0 +1,145 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { useEffect, useMemo, useState } from 'react';
+import { SupersetClient } from '../../..';
+import { getMatrixifyConfig, MatrixifyFormData } from '../../types/matrixify';
+
+export type MatrixifyAllowedValuesStatus = 'success' | 'loading' | 'error';
+
+export interface MatrixifyAllowedValuesState {
+ status: MatrixifyAllowedValuesStatus;
+ /**
+ * Map of dimension column name -> set of string-normalized values the
+ * current viewer is allowed to see (RLS applied by the backend).
+ */
+ allowedByColumn: Record<string, Set<string>>;
+}
+
+/**
+ * Collect the distinct dimension columns referenced by the matrixify axes.
+ */
+function getDimensionColumns(formData: MatrixifyFormData): string[] {
+ const config = getMatrixifyConfig(formData as any);
+ if (!config) {
+ return [];
+ }
+ const columns = new Set<string>();
+ [config.rows, config.columns].forEach(axis => {
+ if (axis.mode === 'dimensions' && axis.dimension?.dimension) {
+ columns.add(axis.dimension.dimension);
+ }
+ });
+ return Array.from(columns);
+}
+
+/**
+ * Fetch the distinct values a viewer is permitted to see for a column. This
+ * reuses the datasource ``/column/<col>/values/`` endpoint, which applies the
+ * requesting user's row-level security filters server-side.
+ */
+async function fetchAllowedValues(
+ datasource: string,
+ column: string,
+ signal: AbortSignal,
+): Promise<any[]> {
+ const [id, type] = String(datasource).split('__');
+ const endpoint = `/api/v1/datasource/${type}/${id}/column/${encodeURIComponent(
+ column,
+ )}/values/`;
+ const { json } = await SupersetClient.get({ endpoint, signal });
+ return json?.result || [];
+}
+
+/**
+ * Resolve, per render, which dimension values the current viewer is allowed to
+ * see. Matrixify axis values are frozen into ``formData`` at design time, so
+ * without this the grid would be built from the chart author's RLS context and
+ * leak values (as subplot headers + empty cells) to restricted viewers. The
+ * renderer intersects the stored values against the returned allow-list.
+ *
+ * Fails closed: while loading the grid must not render, and on error the
+ * allow-list is treated as empty rather than falling back to the unfiltered
+ * (leaking) list.
+ */
+export function useMatrixifyAllowedValues(
+ formData: MatrixifyFormData,
+): MatrixifyAllowedValuesState {
+ const datasource = (formData as any)?.datasource as string | undefined;
+ // Serialize the dimension columns to a primitive key. ``formData`` is a fresh
+ // object on most renders, so the effect must depend on this stable string
+ // rather than the (always-new) array, otherwise it would refetch in a loop.
+ const columnsKey = useMemo(
+ () => JSON.stringify([...getDimensionColumns(formData)].sort()),
+ [formData],
+ );
+
+ const [state, setState] = useState<MatrixifyAllowedValuesState>({
+ status: columnsKey === '[]' ? 'success' : 'loading',
+ allowedByColumn: {},
+ });
+
+ useEffect(() => {
+ const dimensionColumns: string[] = JSON.parse(columnsKey);
+
+ // Metrics-only matrixify has no dimension axes: nothing to resolve.
+ if (dimensionColumns.length === 0) {
+ setState({ status: 'success', allowedByColumn: {} });
+ return undefined;
+ }
+
+ if (!datasource) {
+ setState({ status: 'error', allowedByColumn: {} });
+ return undefined;
+ }
+
+ const controller = new AbortController();
+ setState(prev => ({ ...prev, status: 'loading' }));
+
+ (async () => {
+ try {
+ const results = await Promise.all(
+ dimensionColumns.map(column =>
+ fetchAllowedValues(datasource, column, controller.signal).then(
+ values => [column, values] as const,
+ ),
+ ),
+ );
+ if (controller.signal.aborted) {
+ return;
+ }
+ const allowedByColumn: Record<string, Set<string>> = {};
+ results.forEach(([column, values]) => {
+ allowedByColumn[column] = new Set(values.map(v => String(v)));
+ });
+ setState({ status: 'success', allowedByColumn });
+ } catch (error) {
+ if (!controller.signal.aborted) {
+ setState({ status: 'error', allowedByColumn: {} });
+ }
+ }
+ })();
+
+ return () => {
+ controller.abort();
+ };
+ }, [datasource, columnsKey]);
+
+ return state;
+}