betodealmeida commented on code in PR #38581:
URL: https://github.com/apache/superset/pull/38581#discussion_r2990918818
##########
superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/index.tsx:
##########
@@ -231,6 +233,36 @@ export const useExploreAdditionalActionsMenu = (
: undefined;
},
);
+ const canExportImage = useSelector<ExploreState, boolean>(
+ state => state.explore?.can_export_image ?? true,
Review Comment:
This is a good point, are we setting `can_export_image` correctly when the
FF is off?
##########
tests/integration_tests/test_granular_export_api.py:
##########
@@ -0,0 +1,251 @@
+# 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.
+"""Integration tests for Phase 2 granular export controls.
+
+These tests verify that the GRANULAR_EXPORT_CONTROLS feature flag gates
+access to chart/dashboard screenshot endpoints (can_export_image) and
+SQL Lab export endpoints (can_export_data).
+"""
+
+from unittest.mock import patch
+
+import prison
+import pytest
+
+from superset.security import SupersetSecurityManager
+from tests.integration_tests.base_tests import SupersetTestCase
+from tests.integration_tests.conftest import with_feature_flags
+from tests.integration_tests.constants import ADMIN_USERNAME
+from tests.integration_tests.fixtures.birth_names_dashboard import (
+ load_birth_names_dashboard_with_slices, # noqa: F401
+ load_birth_names_data, # noqa: F401
+)
+
+
+def _deny_can_export_image(perm: str, view: str) -> bool:
+ """Return False only for can_export_image on Superset, allow everything
else."""
+ return perm != "can_export_image" or view != "Superset"
+
+
+def _deny_can_export_data(perm: str, view: str) -> bool:
+ """Return False only for can_export_data on Superset, allow everything
else."""
+ return perm != "can_export_data" or view != "Superset"
+
+
+class TestGranularExportChartAPI(SupersetTestCase):
+ """Test granular export controls on chart screenshot endpoints."""
+
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ @with_feature_flags(GRANULAR_EXPORT_CONTROLS=True, THUMBNAILS=True)
+ @patch.object(
+ SupersetSecurityManager,
+ "can_access",
+ side_effect=_deny_can_export_image,
+ )
+ def test_chart_cache_screenshot_403_without_can_export_image(
+ self, mock_can_access
+ ) -> None:
+ """When GRANULAR_EXPORT_CONTROLS is ON and user lacks can_export_image,
+ cache_screenshot should return 403."""
+ self.login(ADMIN_USERNAME)
+ chart = self.get_slice("Girls")
+ uri = f"api/v1/chart/{chart.id}/cache_screenshot/"
+ rison_params = prison.dumps({"force": False})
+ rv = self.client.get(f"{uri}?q={rison_params}")
+ assert rv.status_code == 403
+
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ @with_feature_flags(GRANULAR_EXPORT_CONTROLS=True, THUMBNAILS=True)
+ @patch.object(
+ SupersetSecurityManager,
+ "can_access",
+ side_effect=_deny_can_export_image,
+ )
+ def test_chart_screenshot_403_without_can_export_image(
+ self, mock_can_access
+ ) -> None:
+ """When GRANULAR_EXPORT_CONTROLS is ON and user lacks can_export_image,
+ screenshot should return 403."""
+ self.login(ADMIN_USERNAME)
+ chart = self.get_slice("Girls")
+ uri = f"api/v1/chart/{chart.id}/screenshot/fake_digest/"
+ rv = self.client.get(uri)
+ assert rv.status_code == 403
+
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ @with_feature_flags(GRANULAR_EXPORT_CONTROLS=True, THUMBNAILS=True)
+ @patch.object(
+ SupersetSecurityManager,
+ "can_access",
+ side_effect=_deny_can_export_image,
+ )
+ def test_chart_thumbnail_403_without_can_export_image(
+ self, mock_can_access
+ ) -> None:
+ """When GRANULAR_EXPORT_CONTROLS is ON and user lacks can_export_image,
+ thumbnail should return 403."""
+ self.login(ADMIN_USERNAME)
+ chart = self.get_slice("Girls")
+ uri = f"api/v1/chart/{chart.id}/thumbnail/{chart.digest}/"
+ rv = self.client.get(uri)
+ assert rv.status_code == 403
+
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ @with_feature_flags(GRANULAR_EXPORT_CONTROLS=False)
+ def test_chart_cache_screenshot_allowed_when_flag_disabled(self) -> None:
+ """When GRANULAR_EXPORT_CONTROLS is OFF, no permission check occurs
+ and the request proceeds (may return 404/422 due to missing thumbnails
+ config, but NOT 403)."""
+ self.login(ADMIN_USERNAME)
+ chart = self.get_slice("Girls")
+ uri = f"api/v1/chart/{chart.id}/cache_screenshot/"
+ rison_params = prison.dumps({"force": False})
+ rv = self.client.get(f"{uri}?q={rison_params}")
+ assert rv.status_code != 403
+
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ @with_feature_flags(GRANULAR_EXPORT_CONTROLS=False)
+ def test_chart_screenshot_allowed_when_flag_disabled(self) -> None:
+ """When GRANULAR_EXPORT_CONTROLS is OFF, screenshot endpoint does
+ not enforce granular permission."""
+ self.login(ADMIN_USERNAME)
+ chart = self.get_slice("Girls")
+ uri = f"api/v1/chart/{chart.id}/screenshot/fake_digest/"
+ rv = self.client.get(uri)
+ assert rv.status_code != 403
+
+
+class TestGranularExportDashboardAPI(SupersetTestCase):
+ """Test granular export controls on dashboard screenshot endpoints."""
+
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ @with_feature_flags(
+ GRANULAR_EXPORT_CONTROLS=True,
+ THUMBNAILS=True,
+ ENABLE_DASHBOARD_SCREENSHOT_ENDPOINTS=True,
+ )
+ @patch.object(
+ SupersetSecurityManager,
+ "can_access",
+ side_effect=_deny_can_export_image,
+ )
+ def test_dashboard_cache_screenshot_403_without_can_export_image(
+ self, mock_can_access
+ ) -> None:
+ """When GRANULAR_EXPORT_CONTROLS is ON and user lacks can_export_image,
+ cache_dashboard_screenshot should return 403."""
+ self.login(ADMIN_USERNAME)
+ dashboard = self.get_dash_by_slug("births") or self.get_dash_by_slug(
+ "birth_names"
+ )
+ uri = f"api/v1/dashboard/{dashboard.id}/cache_dashboard_screenshot/"
+ rison_params = prison.dumps({"force": False})
+ rv = self.client.post(
+ f"{uri}?q={rison_params}",
+ json={},
+ )
+ assert rv.status_code == 403
+
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ @with_feature_flags(
+ GRANULAR_EXPORT_CONTROLS=True,
+ THUMBNAILS=True,
+ ENABLE_DASHBOARD_SCREENSHOT_ENDPOINTS=True,
+ )
+ @patch.object(
+ SupersetSecurityManager,
+ "can_access",
+ side_effect=_deny_can_export_image,
+ )
+ def test_dashboard_screenshot_403_without_can_export_image(
+ self, mock_can_access
+ ) -> None:
+ """When GRANULAR_EXPORT_CONTROLS is ON and user lacks can_export_image,
+ screenshot should return 403."""
+ self.login(ADMIN_USERNAME)
+ dashboard = self.get_dash_by_slug("births") or self.get_dash_by_slug(
+ "birth_names"
+ )
+ uri = f"api/v1/dashboard/{dashboard.id}/screenshot/fake_digest/"
+ rv = self.client.get(uri)
+ assert rv.status_code == 403
+
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ @with_feature_flags(GRANULAR_EXPORT_CONTROLS=False)
+ def test_dashboard_screenshot_allowed_when_flag_disabled(self) -> None:
+ """When GRANULAR_EXPORT_CONTROLS is OFF, no granular permission check
+ is enforced on the screenshot endpoint."""
+ self.login(ADMIN_USERNAME)
+ dashboard = self.get_dash_by_slug("births") or self.get_dash_by_slug(
+ "birth_names"
+ )
+ uri = f"api/v1/dashboard/{dashboard.id}/screenshot/fake_digest/"
+ rv = self.client.get(uri)
+ assert rv.status_code != 403
+
+
+class TestGranularExportSqlLabAPI(SupersetTestCase):
+ """Test granular export controls on SQL Lab export endpoints."""
+
+ @with_feature_flags(GRANULAR_EXPORT_CONTROLS=True)
+ @patch.object(
+ SupersetSecurityManager,
+ "can_access",
+ side_effect=_deny_can_export_data,
+ )
+ def test_export_csv_403_without_can_export_data(self, mock_can_access) ->
None:
+ """When GRANULAR_EXPORT_CONTROLS is ON and user lacks can_export_data,
+ export_csv should return 403."""
+ self.login(ADMIN_USERNAME)
+ uri = "api/v1/sqllab/export/fake_client_id/"
+ rv = self.client.get(uri)
+ assert rv.status_code == 403
+
+ @with_feature_flags(GRANULAR_EXPORT_CONTROLS=True)
+ @patch.object(
+ SupersetSecurityManager,
+ "can_access",
+ side_effect=_deny_can_export_data,
+ )
+ def test_export_streaming_csv_403_without_can_export_data(
+ self, mock_can_access
+ ) -> None:
+ """When GRANULAR_EXPORT_CONTROLS is ON and user lacks can_export_data,
+ export_streaming_csv should return 403."""
+ self.login(ADMIN_USERNAME)
+ uri = "api/v1/sqllab/export_streaming/"
+ rv = self.client.post(uri, data={"client_id": "fake_client_id"})
+ assert rv.status_code == 403
+
+ @with_feature_flags(GRANULAR_EXPORT_CONTROLS=False)
+ def test_export_csv_allowed_when_flag_disabled(self) -> None:
+ """When GRANULAR_EXPORT_CONTROLS is OFF, no granular permission check
+ is enforced. The request may fail for other reasons (no query found),
+ but must not return 403."""
+ self.login(ADMIN_USERNAME)
+ uri = "api/v1/sqllab/export/fake_client_id/"
+ rv = self.client.get(uri)
+ assert rv.status_code != 403
+
+ @with_feature_flags(GRANULAR_EXPORT_CONTROLS=False)
+ def test_export_streaming_csv_allowed_when_flag_disabled(self) -> None:
Review Comment:
Let's add this.
##########
tests/integration_tests/test_granular_export_api.py:
##########
@@ -0,0 +1,251 @@
+# 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.
+"""Integration tests for Phase 2 granular export controls.
+
+These tests verify that the GRANULAR_EXPORT_CONTROLS feature flag gates
+access to chart/dashboard screenshot endpoints (can_export_image) and
+SQL Lab export endpoints (can_export_data).
+"""
+
+from unittest.mock import patch
+
+import prison
+import pytest
+
+from superset.security import SupersetSecurityManager
+from tests.integration_tests.base_tests import SupersetTestCase
+from tests.integration_tests.conftest import with_feature_flags
+from tests.integration_tests.constants import ADMIN_USERNAME
+from tests.integration_tests.fixtures.birth_names_dashboard import (
+ load_birth_names_dashboard_with_slices, # noqa: F401
+ load_birth_names_data, # noqa: F401
+)
+
+
+def _deny_can_export_image(perm: str, view: str) -> bool:
+ """Return False only for can_export_image on Superset, allow everything
else."""
+ return perm != "can_export_image" or view != "Superset"
+
+
+def _deny_can_export_data(perm: str, view: str) -> bool:
+ """Return False only for can_export_data on Superset, allow everything
else."""
+ return perm != "can_export_data" or view != "Superset"
+
+
+class TestGranularExportChartAPI(SupersetTestCase):
+ """Test granular export controls on chart screenshot endpoints."""
+
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ @with_feature_flags(GRANULAR_EXPORT_CONTROLS=True, THUMBNAILS=True)
+ @patch.object(
+ SupersetSecurityManager,
+ "can_access",
+ side_effect=_deny_can_export_image,
+ )
+ def test_chart_cache_screenshot_403_without_can_export_image(
+ self, mock_can_access
+ ) -> None:
+ """When GRANULAR_EXPORT_CONTROLS is ON and user lacks can_export_image,
+ cache_screenshot should return 403."""
+ self.login(ADMIN_USERNAME)
+ chart = self.get_slice("Girls")
+ uri = f"api/v1/chart/{chart.id}/cache_screenshot/"
+ rison_params = prison.dumps({"force": False})
+ rv = self.client.get(f"{uri}?q={rison_params}")
+ assert rv.status_code == 403
+
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ @with_feature_flags(GRANULAR_EXPORT_CONTROLS=True, THUMBNAILS=True)
+ @patch.object(
+ SupersetSecurityManager,
+ "can_access",
+ side_effect=_deny_can_export_image,
+ )
+ def test_chart_screenshot_403_without_can_export_image(
+ self, mock_can_access
+ ) -> None:
+ """When GRANULAR_EXPORT_CONTROLS is ON and user lacks can_export_image,
+ screenshot should return 403."""
+ self.login(ADMIN_USERNAME)
+ chart = self.get_slice("Girls")
+ uri = f"api/v1/chart/{chart.id}/screenshot/fake_digest/"
+ rv = self.client.get(uri)
+ assert rv.status_code == 403
+
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ @with_feature_flags(GRANULAR_EXPORT_CONTROLS=True, THUMBNAILS=True)
+ @patch.object(
+ SupersetSecurityManager,
+ "can_access",
+ side_effect=_deny_can_export_image,
+ )
+ def test_chart_thumbnail_403_without_can_export_image(
+ self, mock_can_access
+ ) -> None:
+ """When GRANULAR_EXPORT_CONTROLS is ON and user lacks can_export_image,
+ thumbnail should return 403."""
+ self.login(ADMIN_USERNAME)
+ chart = self.get_slice("Girls")
+ uri = f"api/v1/chart/{chart.id}/thumbnail/{chart.digest}/"
+ rv = self.client.get(uri)
+ assert rv.status_code == 403
+
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ @with_feature_flags(GRANULAR_EXPORT_CONTROLS=False)
+ def test_chart_cache_screenshot_allowed_when_flag_disabled(self) -> None:
+ """When GRANULAR_EXPORT_CONTROLS is OFF, no permission check occurs
+ and the request proceeds (may return 404/422 due to missing thumbnails
+ config, but NOT 403)."""
+ self.login(ADMIN_USERNAME)
+ chart = self.get_slice("Girls")
+ uri = f"api/v1/chart/{chart.id}/cache_screenshot/"
+ rison_params = prison.dumps({"force": False})
+ rv = self.client.get(f"{uri}?q={rison_params}")
+ assert rv.status_code != 403
+
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ @with_feature_flags(GRANULAR_EXPORT_CONTROLS=False)
+ def test_chart_screenshot_allowed_when_flag_disabled(self) -> None:
+ """When GRANULAR_EXPORT_CONTROLS is OFF, screenshot endpoint does
+ not enforce granular permission."""
+ self.login(ADMIN_USERNAME)
+ chart = self.get_slice("Girls")
+ uri = f"api/v1/chart/{chart.id}/screenshot/fake_digest/"
+ rv = self.client.get(uri)
+ assert rv.status_code != 403
+
+
+class TestGranularExportDashboardAPI(SupersetTestCase):
+ """Test granular export controls on dashboard screenshot endpoints."""
+
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ @with_feature_flags(
+ GRANULAR_EXPORT_CONTROLS=True,
+ THUMBNAILS=True,
+ ENABLE_DASHBOARD_SCREENSHOT_ENDPOINTS=True,
+ )
+ @patch.object(
+ SupersetSecurityManager,
+ "can_access",
+ side_effect=_deny_can_export_image,
+ )
+ def test_dashboard_cache_screenshot_403_without_can_export_image(
+ self, mock_can_access
+ ) -> None:
+ """When GRANULAR_EXPORT_CONTROLS is ON and user lacks can_export_image,
+ cache_dashboard_screenshot should return 403."""
+ self.login(ADMIN_USERNAME)
+ dashboard = self.get_dash_by_slug("births") or self.get_dash_by_slug(
+ "birth_names"
+ )
+ uri = f"api/v1/dashboard/{dashboard.id}/cache_dashboard_screenshot/"
+ rison_params = prison.dumps({"force": False})
+ rv = self.client.post(
+ f"{uri}?q={rison_params}",
+ json={},
+ )
+ assert rv.status_code == 403
+
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ @with_feature_flags(
+ GRANULAR_EXPORT_CONTROLS=True,
+ THUMBNAILS=True,
+ ENABLE_DASHBOARD_SCREENSHOT_ENDPOINTS=True,
+ )
+ @patch.object(
+ SupersetSecurityManager,
+ "can_access",
+ side_effect=_deny_can_export_image,
+ )
+ def test_dashboard_screenshot_403_without_can_export_image(
+ self, mock_can_access
+ ) -> None:
+ """When GRANULAR_EXPORT_CONTROLS is ON and user lacks can_export_image,
+ screenshot should return 403."""
+ self.login(ADMIN_USERNAME)
+ dashboard = self.get_dash_by_slug("births") or self.get_dash_by_slug(
+ "birth_names"
+ )
+ uri = f"api/v1/dashboard/{dashboard.id}/screenshot/fake_digest/"
+ rv = self.client.get(uri)
+ assert rv.status_code == 403
+
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ @with_feature_flags(GRANULAR_EXPORT_CONTROLS=False)
+ def test_dashboard_screenshot_allowed_when_flag_disabled(self) -> None:
+ """When GRANULAR_EXPORT_CONTROLS is OFF, no granular permission check
+ is enforced on the screenshot endpoint."""
+ self.login(ADMIN_USERNAME)
+ dashboard = self.get_dash_by_slug("births") or self.get_dash_by_slug(
+ "birth_names"
+ )
+ uri = f"api/v1/dashboard/{dashboard.id}/screenshot/fake_digest/"
+ rv = self.client.get(uri)
+ assert rv.status_code != 403
Review Comment:
I think the bot is right, this could be a 404, while we expect a 2xx. Let's
add the other FFs and check that we have a successful status code here.
##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -361,11 +363,20 @@ const ResultSet = ({
schema: query?.schema,
};
- const canExportData = findPermission(
+ const canExportDataLegacy = findPermission(
'can_export_csv',
'SQLLab',
user?.roles,
);
+ const granularExport = isFeatureEnabled(
+ FeatureFlag.GranularExportControls,
+ );
+ const canExportData = granularExport
+ ? findPermission('can_export_data', 'Superset', user?.roles)
+ : canExportDataLegacy;
+ const canCopyClipboard = granularExport
+ ? findPermission('can_copy_clipboard', 'Superset', user?.roles)
+ : canExportDataLegacy;
Review Comment:
Can't we just reuse the permissions computed in
`superset-frontend/src/hooks/usePermissions.ts`?
##########
docs/docs/security/granular-export-controls.mdx:
##########
@@ -0,0 +1,75 @@
+---
+title: Granular Export Controls
+sidebar_position: 4
+---
+
+# Granular Export Controls
+
+Superset provides granular, permission-based controls for data export, image
export, and clipboard operations. These replace the legacy `can_csv` permission
with three fine-grained permissions that can be assigned independently to roles.
+
+## Feature Flag
+
+Granular export controls are gated behind the `GRANULAR_EXPORT_CONTROLS`
feature flag. When the flag is disabled, the legacy `can_csv` permission
behavior is preserved.
+
+```python
+FEATURE_FLAGS = {
+ "GRANULAR_EXPORT_CONTROLS": True,
+}
+```
+
+## Permissions
+
+| Permission | Resource | Controls |
+|---|---|---|
+| `can_export_data` | `Superset` | CSV, Excel, and JSON data exports from
charts, dashboards, and SQL Lab |
+| `can_export_image` | `Superset` | Screenshot (JPEG/PNG) and PDF exports from
charts and dashboards |
+| `can_copy_clipboard` | `Superset` | Copy-to-clipboard operations in SQL Lab
and the Explore data pane |
+
+## Default Role Assignments
+
+The migration grants all three new permissions (`can_export_data`,
`can_export_image`, `can_copy_clipboard`) to every role that currently has
`can_csv`. This preserves existing behavior — no role loses access during the
upgrade.
+
+After the migration, admins can selectively revoke individual export
permissions from any role to restrict access. For example, to prevent Gamma
users from exporting data or images while still allowing clipboard operations,
revoke `can_export_data` and `can_export_image` from the Gamma role.
+
+## Configuration Steps
+
+1. **Enable the feature flag** in `superset_config.py`:
+ ```python
+ FEATURE_FLAGS = {
+ "GRANULAR_EXPORT_CONTROLS": True,
+ }
+ ```
+
+2. **Run the database migration** to register the new permissions:
+ ```bash
+ superset db upgrade
+ ```
+
+3. **Initialize permissions** so roles are populated:
+ ```bash
+ superset init
+ ```
+
+4. **Verify role assignments** in **Settings > List Roles**. Confirm that each
role has the expected permissions from the table above.
+
+5. **Customize as needed**: Grant or revoke individual export permissions on
any role through the role editor.
+
+## User Experience
+
+When a user lacks a required export permission:
+
+- **Menu items** (CSV, Excel, JSON, screenshot) appear **disabled** with an
info tooltip icon explaining the restriction
+- **Buttons** (SQL Lab download, clipboard copy) appear **disabled** with a
tooltip on hover
+- **API endpoints** return **403 Forbidden** when the corresponding permission
is missing
+
+## API Enforcement
+
+The following API endpoints enforce granular export permissions when the
feature flag is enabled:
+
+| Endpoint | Required Permission |
+|---|---|
+| `GET /api/v1/chart/{id}/data/` (CSV/Excel format) | `can_export_data` |
+| `GET /api/v1/chart/{id}/cache_screenshot/` | `can_export_image` |
+| `GET /api/v1/dashboard/{id}/cache_screenshot/` | `can_export_image` |
Review Comment:
I think the bot is wrong, the line looks good and the endpoint exists:
```bash
superset/charts/api.py: @expose("/<pk>/cache_screenshot/",
methods=("GET",))
```
I'm not sure about what we should do with
`/api/v1/dashboard/{id}/cache_dashboard_screenshot/`. I think it should be fine
to leave it as is, since if a user caches a dashboard screenshot it doesn't
mean they will be able to see it.
##########
superset-frontend/src/explore/components/DataTablesPane/components/DataTableControls.tsx:
##########
@@ -79,9 +84,38 @@ export const TableControls = ({
`}
>
<RowCountLabel rowcount={rowcount} loading={isLoading} />
- {canDownload && (
- <CopyToClipboardButton data={formattedData} columns={columnNames} />
- )}
+ <Tooltip
+ title={
+ !copyEnabled
+ ? t("You don't have permission to copy to clipboard")
+ : undefined
+ }
+ >
+ <div
+ css={
+ !copyEnabled
+ ? css`
+ opacity: 0.3;
+ `
+ : undefined
+ }
+ >
+ <div
+ css={
+ !copyEnabled
+ ? css`
+ pointer-events: none;
+ `
+ : undefined
+ }
+ >
+ <CopyToClipboardButton
+ data={formattedData}
+ columns={columnNames}
+ />
+ </div>
+ </div>
+ </Tooltip>
Review Comment:
We can simplify this, and also let's make sure to disable the
`CopyToClipboardButton` element:
```suggestion
{copyEnabled ? (
<CopyToClipboardButton
data={formattedData}
columns={columnNames}
/>
) : (
<Tooltip title={t("You don't have permission to copy to clipboard")}>
<span css={css`opacity: 0.3; cursor: not-allowed;`}>
<CopyToClipboardButton
data={formattedData}
columns={columnNames}
disabled
/>
</span>
</Tooltip>
)}
```
##########
docs/docs/security/granular-export-controls.mdx:
##########
@@ -0,0 +1,75 @@
+---
+title: Granular Export Controls
+sidebar_position: 4
+---
+
+# Granular Export Controls
+
+Superset provides granular, permission-based controls for data export, image
export, and clipboard operations. These replace the legacy `can_csv` permission
with three fine-grained permissions that can be assigned independently to roles.
+
+## Feature Flag
+
+Granular export controls are gated behind the `GRANULAR_EXPORT_CONTROLS`
feature flag. When the flag is disabled, the legacy `can_csv` permission
behavior is preserved.
+
+```python
+FEATURE_FLAGS = {
+ "GRANULAR_EXPORT_CONTROLS": True,
+}
+```
+
+## Permissions
+
+| Permission | Resource | Controls |
+|---|---|---|
+| `can_export_data` | `Superset` | CSV, Excel, and JSON data exports from
charts, dashboards, and SQL Lab |
+| `can_export_image` | `Superset` | Screenshot (JPEG/PNG) and PDF exports from
charts and dashboards |
+| `can_copy_clipboard` | `Superset` | Copy-to-clipboard operations in SQL Lab
and the Explore data pane |
+
+## Default Role Assignments
+
+The migration grants all three new permissions (`can_export_data`,
`can_export_image`, `can_copy_clipboard`) to every role that currently has
`can_csv`. This preserves existing behavior — no role loses access during the
upgrade.
+
+After the migration, admins can selectively revoke individual export
permissions from any role to restrict access. For example, to prevent Gamma
users from exporting data or images while still allowing clipboard operations,
revoke `can_export_data` and `can_export_image` from the Gamma role.
+
+## Configuration Steps
+
+1. **Enable the feature flag** in `superset_config.py`:
+ ```python
+ FEATURE_FLAGS = {
+ "GRANULAR_EXPORT_CONTROLS": True,
+ }
+ ```
+
+2. **Run the database migration** to register the new permissions:
+ ```bash
+ superset db upgrade
+ ```
+
+3. **Initialize permissions** so roles are populated:
+ ```bash
+ superset init
+ ```
+
+4. **Verify role assignments** in **Settings > List Roles**. Confirm that each
role has the expected permissions from the table above.
+
+5. **Customize as needed**: Grant or revoke individual export permissions on
any role through the role editor.
+
+## User Experience
+
+When a user lacks a required export permission:
+
+- **Menu items** (CSV, Excel, JSON, screenshot) appear **disabled** with an
info tooltip icon explaining the restriction
+- **Buttons** (SQL Lab download, clipboard copy) appear **disabled** with a
tooltip on hover
+- **API endpoints** return **403 Forbidden** when the corresponding permission
is missing
+
+## API Enforcement
+
+The following API endpoints enforce granular export permissions when the
feature flag is enabled:
+
+| Endpoint | Required Permission |
+|---|---|
+| `GET /api/v1/chart/{id}/data/` (CSV/Excel format) | `can_export_data` |
+| `GET /api/v1/chart/{id}/cache_screenshot/` | `can_export_image` |
+| `GET /api/v1/dashboard/{id}/cache_screenshot/` | `can_export_image` |
+| `POST /api/v1/sqllab/export/` | `can_export_data` |
Review Comment:
This one is correct, the right method/endpoint is `GET
/api/v1/sqllab/export/{client_id}/`.
##########
tests/integration_tests/test_granular_export_api.py:
##########
@@ -0,0 +1,251 @@
+# 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.
+"""Integration tests for Phase 2 granular export controls.
+
+These tests verify that the GRANULAR_EXPORT_CONTROLS feature flag gates
+access to chart/dashboard screenshot endpoints (can_export_image) and
+SQL Lab export endpoints (can_export_data).
+"""
+
+from unittest.mock import patch
+
+import prison
+import pytest
+
+from superset.security import SupersetSecurityManager
+from tests.integration_tests.base_tests import SupersetTestCase
+from tests.integration_tests.conftest import with_feature_flags
+from tests.integration_tests.constants import ADMIN_USERNAME
+from tests.integration_tests.fixtures.birth_names_dashboard import (
+ load_birth_names_dashboard_with_slices, # noqa: F401
+ load_birth_names_data, # noqa: F401
+)
+
+
+def _deny_can_export_image(perm: str, view: str) -> bool:
+ """Return False only for can_export_image on Superset, allow everything
else."""
+ return perm != "can_export_image" or view != "Superset"
+
+
+def _deny_can_export_data(perm: str, view: str) -> bool:
+ """Return False only for can_export_data on Superset, allow everything
else."""
+ return perm != "can_export_data" or view != "Superset"
+
+
+class TestGranularExportChartAPI(SupersetTestCase):
+ """Test granular export controls on chart screenshot endpoints."""
+
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ @with_feature_flags(GRANULAR_EXPORT_CONTROLS=True, THUMBNAILS=True)
+ @patch.object(
+ SupersetSecurityManager,
+ "can_access",
+ side_effect=_deny_can_export_image,
+ )
+ def test_chart_cache_screenshot_403_without_can_export_image(
+ self, mock_can_access
+ ) -> None:
+ """When GRANULAR_EXPORT_CONTROLS is ON and user lacks can_export_image,
+ cache_screenshot should return 403."""
+ self.login(ADMIN_USERNAME)
+ chart = self.get_slice("Girls")
+ uri = f"api/v1/chart/{chart.id}/cache_screenshot/"
+ rison_params = prison.dumps({"force": False})
+ rv = self.client.get(f"{uri}?q={rison_params}")
+ assert rv.status_code == 403
+
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ @with_feature_flags(GRANULAR_EXPORT_CONTROLS=True, THUMBNAILS=True)
+ @patch.object(
+ SupersetSecurityManager,
+ "can_access",
+ side_effect=_deny_can_export_image,
+ )
+ def test_chart_screenshot_403_without_can_export_image(
+ self, mock_can_access
+ ) -> None:
+ """When GRANULAR_EXPORT_CONTROLS is ON and user lacks can_export_image,
+ screenshot should return 403."""
+ self.login(ADMIN_USERNAME)
+ chart = self.get_slice("Girls")
+ uri = f"api/v1/chart/{chart.id}/screenshot/fake_digest/"
+ rv = self.client.get(uri)
+ assert rv.status_code == 403
+
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ @with_feature_flags(GRANULAR_EXPORT_CONTROLS=True, THUMBNAILS=True)
+ @patch.object(
+ SupersetSecurityManager,
+ "can_access",
+ side_effect=_deny_can_export_image,
+ )
+ def test_chart_thumbnail_403_without_can_export_image(
+ self, mock_can_access
+ ) -> None:
+ """When GRANULAR_EXPORT_CONTROLS is ON and user lacks can_export_image,
+ thumbnail should return 403."""
+ self.login(ADMIN_USERNAME)
+ chart = self.get_slice("Girls")
+ uri = f"api/v1/chart/{chart.id}/thumbnail/{chart.digest}/"
+ rv = self.client.get(uri)
+ assert rv.status_code == 403
+
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ @with_feature_flags(GRANULAR_EXPORT_CONTROLS=False)
+ def test_chart_cache_screenshot_allowed_when_flag_disabled(self) -> None:
+ """When GRANULAR_EXPORT_CONTROLS is OFF, no permission check occurs
+ and the request proceeds (may return 404/422 due to missing thumbnails
+ config, but NOT 403)."""
+ self.login(ADMIN_USERNAME)
+ chart = self.get_slice("Girls")
+ uri = f"api/v1/chart/{chart.id}/cache_screenshot/"
+ rison_params = prison.dumps({"force": False})
+ rv = self.client.get(f"{uri}?q={rison_params}")
+ assert rv.status_code != 403
+
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ @with_feature_flags(GRANULAR_EXPORT_CONTROLS=False)
+ def test_chart_screenshot_allowed_when_flag_disabled(self) -> None:
+ """When GRANULAR_EXPORT_CONTROLS is OFF, screenshot endpoint does
+ not enforce granular permission."""
+ self.login(ADMIN_USERNAME)
+ chart = self.get_slice("Girls")
+ uri = f"api/v1/chart/{chart.id}/screenshot/fake_digest/"
+ rv = self.client.get(uri)
+ assert rv.status_code != 403
+
+
+class TestGranularExportDashboardAPI(SupersetTestCase):
+ """Test granular export controls on dashboard screenshot endpoints."""
+
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ @with_feature_flags(
+ GRANULAR_EXPORT_CONTROLS=True,
+ THUMBNAILS=True,
+ ENABLE_DASHBOARD_SCREENSHOT_ENDPOINTS=True,
+ )
+ @patch.object(
+ SupersetSecurityManager,
+ "can_access",
+ side_effect=_deny_can_export_image,
+ )
+ def test_dashboard_cache_screenshot_403_without_can_export_image(
+ self, mock_can_access
+ ) -> None:
+ """When GRANULAR_EXPORT_CONTROLS is ON and user lacks can_export_image,
+ cache_dashboard_screenshot should return 403."""
+ self.login(ADMIN_USERNAME)
+ dashboard = self.get_dash_by_slug("births") or self.get_dash_by_slug(
+ "birth_names"
+ )
+ uri = f"api/v1/dashboard/{dashboard.id}/cache_dashboard_screenshot/"
+ rison_params = prison.dumps({"force": False})
+ rv = self.client.post(
+ f"{uri}?q={rison_params}",
+ json={},
+ )
+ assert rv.status_code == 403
+
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ @with_feature_flags(
+ GRANULAR_EXPORT_CONTROLS=True,
+ THUMBNAILS=True,
+ ENABLE_DASHBOARD_SCREENSHOT_ENDPOINTS=True,
+ )
+ @patch.object(
+ SupersetSecurityManager,
+ "can_access",
+ side_effect=_deny_can_export_image,
+ )
+ def test_dashboard_screenshot_403_without_can_export_image(
+ self, mock_can_access
+ ) -> None:
+ """When GRANULAR_EXPORT_CONTROLS is ON and user lacks can_export_image,
+ screenshot should return 403."""
+ self.login(ADMIN_USERNAME)
+ dashboard = self.get_dash_by_slug("births") or self.get_dash_by_slug(
+ "birth_names"
+ )
+ uri = f"api/v1/dashboard/{dashboard.id}/screenshot/fake_digest/"
+ rv = self.client.get(uri)
+ assert rv.status_code == 403
+
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ @with_feature_flags(GRANULAR_EXPORT_CONTROLS=False)
+ def test_dashboard_screenshot_allowed_when_flag_disabled(self) -> None:
+ """When GRANULAR_EXPORT_CONTROLS is OFF, no granular permission check
+ is enforced on the screenshot endpoint."""
+ self.login(ADMIN_USERNAME)
+ dashboard = self.get_dash_by_slug("births") or self.get_dash_by_slug(
+ "birth_names"
+ )
+ uri = f"api/v1/dashboard/{dashboard.id}/screenshot/fake_digest/"
+ rv = self.client.get(uri)
+ assert rv.status_code != 403
+
+
+class TestGranularExportSqlLabAPI(SupersetTestCase):
+ """Test granular export controls on SQL Lab export endpoints."""
+
+ @with_feature_flags(GRANULAR_EXPORT_CONTROLS=True)
+ @patch.object(
+ SupersetSecurityManager,
+ "can_access",
+ side_effect=_deny_can_export_data,
+ )
+ def test_export_csv_403_without_can_export_data(self, mock_can_access) ->
None:
+ """When GRANULAR_EXPORT_CONTROLS is ON and user lacks can_export_data,
+ export_csv should return 403."""
+ self.login(ADMIN_USERNAME)
+ uri = "api/v1/sqllab/export/fake_client_id/"
+ rv = self.client.get(uri)
+ assert rv.status_code == 403
+
+ @with_feature_flags(GRANULAR_EXPORT_CONTROLS=True)
+ @patch.object(
+ SupersetSecurityManager,
+ "can_access",
+ side_effect=_deny_can_export_data,
+ )
+ def test_export_streaming_csv_403_without_can_export_data(
+ self, mock_can_access
+ ) -> None:
+ """When GRANULAR_EXPORT_CONTROLS is ON and user lacks can_export_data,
+ export_streaming_csv should return 403."""
+ self.login(ADMIN_USERNAME)
+ uri = "api/v1/sqllab/export_streaming/"
+ rv = self.client.post(uri, data={"client_id": "fake_client_id"})
+ assert rv.status_code == 403
+
+ @with_feature_flags(GRANULAR_EXPORT_CONTROLS=False)
+ def test_export_csv_allowed_when_flag_disabled(self) -> None:
Review Comment:
Let's add this.
##########
superset/sqllab/api.py:
##########
@@ -376,6 +381,10 @@ def export_streaming_csv(self) -> Response:
500:
$ref: '#/components/responses/500'
"""
+ if is_feature_enabled(
+ "GRANULAR_EXPORT_CONTROLS"
+ ) and not security_manager.can_access("can_export_data", "Superset"):
Review Comment:
We can do this in a separate PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
