You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by en...@apache.org on 2026/08/21 16:04:54 UTC
(superset) branch enxdev/fix/dashboard-list created (now 5aa2b451f15)
This is an automated email from the ASF dual-hosted git repository.
EnxDev pushed a change to branch enxdev/fix/dashboard-list
in repository https://gitbox.apache.org/repos/asf/superset.git
at 5aa2b451f15 fix(security): honor extra_editors in dashboard/chart list responses
This branch includes the following new commits:
new 5aa2b451f15 fix(security): honor extra_editors in dashboard/chart list responses
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(security): honor extra_editors in dashboard/chart list responses
Posted by en...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
EnxDev pushed a commit to branch enxdev/fix/dashboard-list
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 5aa2b451f15ca6ab3845aa5ebbcdd41984c2f693
Author: Enzo Martellucci <en...@gmail.com>
AuthorDate: Fri Aug 21 18:04:06 2026 +0200
fix(security): honor extra_editors in dashboard/chart list responses
---
.../src/dashboard/util/permissionUtils.test.ts | 15 ++++
.../src/dashboard/util/permissionUtils.ts | 7 +-
.../src/features/charts/ChartCard.tsx | 6 +-
.../src/features/dashboards/DashboardCard.tsx | 6 +-
superset-frontend/src/pages/ChartList/index.tsx | 6 +-
.../src/pages/DashboardList/index.tsx | 8 ++-
superset-frontend/src/types/Chart.ts | 2 +
superset-frontend/src/views/CRUD/types.ts | 2 +
superset/charts/api.py | 14 +++-
superset/dashboards/api.py | 14 +++-
superset/security/manager.py | 30 ++++++++
tests/integration_tests/dashboards/api_tests.py | 84 ++++++++++++++++++++++
12 files changed, 184 insertions(+), 10 deletions(-)
diff --git a/superset-frontend/src/dashboard/util/permissionUtils.test.ts b/superset-frontend/src/dashboard/util/permissionUtils.test.ts
index 04d7b239450..d3cf09b5c7d 100644
--- a/superset-frontend/src/dashboard/util/permissionUtils.test.ts
+++ b/superset-frontend/src/dashboard/util/permissionUtils.test.ts
@@ -259,6 +259,21 @@ describe('isUserEditorOrAdmin', () => {
test('returns false when editors is omitted', () => {
expect(isUserEditorOrAdmin(outsiderUser)).toEqual(false);
});
+
+ test('returns true when the user is granted editorship only through extra_editors', () => {
+ expect(isUserEditorOrAdmin(editorUser, [], [10])).toEqual(true);
+ });
+
+ test('unions editors and extra_editors rather than preferring one', () => {
+ const nonMatchingSubject: Subject = { id: 999, label: 'Other', type: 1 };
+ expect(isUserEditorOrAdmin(editorUser, [nonMatchingSubject], [10])).toEqual(
+ true,
+ );
+ });
+
+ test('returns false when extra_editors names other subjects', () => {
+ expect(isUserEditorOrAdmin(editorUser, [], [999])).toEqual(false);
+ });
});
// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from describe blocks
diff --git a/superset-frontend/src/dashboard/util/permissionUtils.ts b/superset-frontend/src/dashboard/util/permissionUtils.ts
index 448fd60b502..bf746dec402 100644
--- a/superset-frontend/src/dashboard/util/permissionUtils.ts
+++ b/superset-frontend/src/dashboard/util/permissionUtils.ts
@@ -55,9 +55,6 @@ export const isUserInSubjects = (
);
};
-const isUserInEditors = (editors: Subject[] = []): boolean =>
- isUserInSubjects(editors);
-
export const isUserAdmin = (
user?: UserWithPermissionsAndRoles | UndefinedUser,
) =>
@@ -66,10 +63,12 @@ export const isUserAdmin = (
role => role.toLowerCase() === ADMIN_ROLE_NAME.toLowerCase(),
);
+/** `extraEditors` is editorship granted via a deployment's EXTRA_EDITORS_RESOLVER. */
export const isUserEditorOrAdmin = (
user?: UserWithPermissionsAndRoles | UndefinedUser,
editors: Subject[] = [],
-): boolean => isUserInEditors(editors) || isUserAdmin(user);
+ extraEditors?: SubjectRef[] | null,
+): boolean => isUserInSubjects(editors, extraEditors) || isUserAdmin(user);
/**
* Editorship of *dashboard*, matching the server's `is_editor`: the explicit
diff --git a/superset-frontend/src/features/charts/ChartCard.tsx b/superset-frontend/src/features/charts/ChartCard.tsx
index a4c4d47bf79..494d6a3c052 100644
--- a/superset-frontend/src/features/charts/ChartCard.tsx
+++ b/superset-frontend/src/features/charts/ChartCard.tsx
@@ -96,7 +96,11 @@ export default function ChartCard({
const canEdit = hasPerm('can_write');
const canDelete = hasPerm('can_write');
const canExport = hasPerm('can_export');
- const allowEdit = isUserEditorOrAdmin(user, chart.editors);
+ const allowEdit = isUserEditorOrAdmin(
+ user,
+ chart.editors,
+ chart.extra_editors,
+ );
const menuItems: MenuItem[] = [];
if (canEdit) {
diff --git a/superset-frontend/src/features/dashboards/DashboardCard.tsx b/superset-frontend/src/features/dashboards/DashboardCard.tsx
index 63e854a388a..02b8171074e 100644
--- a/superset-frontend/src/features/dashboards/DashboardCard.tsx
+++ b/superset-frontend/src/features/dashboards/DashboardCard.tsx
@@ -83,7 +83,11 @@ function DashboardCard({
const canEdit = hasPerm('can_write');
const canDelete = hasPerm('can_write');
const canExport = hasPerm('can_export');
- const allowEdit = isUserEditorOrAdmin(user, dashboard.editors);
+ const allowEdit = isUserEditorOrAdmin(
+ user,
+ dashboard.editors,
+ dashboard.extra_editors,
+ );
const digest = dashboard.changed_on_utc || dashboard.changed_on;
const thumbnailUrl =
isFeatureEnabled(FeatureFlag.Thumbnails) && dashboard.id && digest
diff --git a/superset-frontend/src/pages/ChartList/index.tsx b/superset-frontend/src/pages/ChartList/index.tsx
index 09666a74dc5..c4e192882ab 100644
--- a/superset-frontend/src/pages/ChartList/index.tsx
+++ b/superset-frontend/src/pages/ChartList/index.tsx
@@ -527,7 +527,11 @@ function ChartList(props: ChartListProps) {
},
{
Cell: ({ row: { original } }: CellProps<Chart>) => {
- const allowEdit = isUserEditorOrAdmin(user, original.editors);
+ const allowEdit = isUserEditorOrAdmin(
+ user,
+ original.editors,
+ original.extra_editors,
+ );
const handleDelete = () =>
handleChartDelete(
original,
diff --git a/superset-frontend/src/pages/DashboardList/index.tsx b/superset-frontend/src/pages/DashboardList/index.tsx
index a5513422098..6bc23634086 100644
--- a/superset-frontend/src/pages/DashboardList/index.tsx
+++ b/superset-frontend/src/pages/DashboardList/index.tsx
@@ -122,6 +122,8 @@ export interface Dashboard {
description?: string;
thumbnail_url?: string | null;
editors?: Subject[];
+ // Bare subject ids from a deployment's EXTRA_EDITORS_RESOLVER.
+ extra_editors?: number[];
viewers?: Subject[];
tags: TagType[];
created_by: object;
@@ -505,7 +507,11 @@ function DashboardList(props: DashboardListProps) {
},
{
Cell: ({ row: { original } }: CellProps<Dashboard>) => {
- const allowEdit = isUserEditorOrAdmin(user, original.editors);
+ const allowEdit = isUserEditorOrAdmin(
+ user,
+ original.editors,
+ original.extra_editors,
+ );
const handleDelete = () =>
handleDashboardDelete(
original,
diff --git a/superset-frontend/src/types/Chart.ts b/superset-frontend/src/types/Chart.ts
index 499d5115dd2..cc5f369be49 100644
--- a/superset-frontend/src/types/Chart.ts
+++ b/superset-frontend/src/types/Chart.ts
@@ -45,6 +45,8 @@ export interface Chart {
cache_timeout: number | null;
thumbnail_url?: string;
editors?: Subject[];
+ // Bare subject ids from a deployment's EXTRA_EDITORS_RESOLVER.
+ extra_editors?: number[];
viewers?: Subject[];
tags?: TagType[];
last_saved_at?: string;
diff --git a/superset-frontend/src/views/CRUD/types.ts b/superset-frontend/src/views/CRUD/types.ts
index 83fef7ab1f6..08206bb0382 100644
--- a/superset-frontend/src/views/CRUD/types.ts
+++ b/superset-frontend/src/views/CRUD/types.ts
@@ -67,6 +67,8 @@ export interface Dashboard {
url: string;
thumbnail_url?: string | null;
editors?: Subject[];
+ // Bare subject ids from a deployment's EXTRA_EDITORS_RESOLVER.
+ extra_editors?: number[];
viewers?: Subject[];
loading?: boolean;
}
diff --git a/superset/charts/api.py b/superset/charts/api.py
index 1e3626bd77d..7f19a9136aa 100644
--- a/superset/charts/api.py
+++ b/superset/charts/api.py
@@ -92,7 +92,10 @@ from superset.exceptions import (
)
from superset.extensions import event_logger, security_manager
from superset.models.slice import Slice
-from superset.security.manager import get_extra_editor_subject_ids
+from superset.security.manager import (
+ get_extra_editor_subject_ids,
+ get_extra_editors_by_pk,
+)
from superset.subjects.filters import (
FilterRelatedSubjects,
subject_type_filter,
@@ -410,6 +413,15 @@ class ChartRestApi(SoftDeleteApiMixin, BaseSupersetModelRestApi):
except ChartNotFoundError:
return self.response_404()
+ def pre_get_list(self, data: dict[str, Any]) -> None:
+ """Attach ``extra_editors`` to each row, matching the single-object GET."""
+ super().pre_get_list(data)
+ ids = data.get("ids", [])
+ extra_editors_by_id = get_extra_editors_by_pk(Slice, ids)
+ for row, row_id in zip(data.get("result", []), ids, strict=False):
+ if row_id in extra_editors_by_id:
+ row["extra_editors"] = extra_editors_by_id[row_id]
+
@expose("/<pk>/deck_layers/", methods=("GET",))
@protect()
@safe
diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py
index cbde7183106..b51d6cc4ee0 100644
--- a/superset/dashboards/api.py
+++ b/superset/dashboards/api.py
@@ -140,7 +140,10 @@ from superset.extensions import event_logger, security_manager
from superset.models.dashboard import Dashboard
from superset.models.embedded_dashboard import EmbeddedDashboard
from superset.security.guest_token import GuestUser
-from superset.security.manager import get_extra_editor_subject_ids
+from superset.security.manager import (
+ get_extra_editor_subject_ids,
+ get_extra_editors_by_pk,
+)
from superset.subjects.filters import (
FilterRelatedSubjects,
subject_type_filter,
@@ -431,6 +434,15 @@ class DashboardRestApi(
"""
return super().get_list(**kwargs)
+ def pre_get_list(self, data: dict[str, Any]) -> None:
+ """Attach ``extra_editors`` to each row, matching the single-object GET."""
+ super().pre_get_list(data)
+ ids = data.get("ids", [])
+ extra_editors_by_id = get_extra_editors_by_pk(Dashboard, ids)
+ for row, row_id in zip(data.get("result", []), ids, strict=False):
+ if row_id in extra_editors_by_id:
+ row["extra_editors"] = extra_editors_by_id[row_id]
+
list_select_columns = list_columns + ["changed_on", "created_on", "changed_by_fk"]
order_columns = [
"changed_by.first_name",
diff --git a/superset/security/manager.py b/superset/security/manager.py
index 79c262dfddc..609d9651ca0 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -170,6 +170,36 @@ def get_extra_editor_subject_ids(resource: Model) -> list[int]:
return subject_ids
+def get_extra_editors_by_pk(
+ model_cls: type[Model], primary_keys: list[Any]
+) -> dict[Any, list[int]]:
+ """
+ Resolve extra editor subject IDs for a batch of resources, keyed by
+ primary key. List responses only have serialized rows, not model
+ instances, so this re-queries the page's rows in one batched query.
+ """
+ if not primary_keys or not (
+ has_app_context() and current_app.config.get("EXTRA_EDITORS_RESOLVER")
+ ):
+ return {}
+
+ # pylint: disable=import-outside-toplevel
+ from superset import db
+ from superset.models.helpers import SKIP_VISIBILITY_FILTER_CLASSES
+
+ pk_col = inspect(model_cls).primary_key[0]
+ resources = (
+ db.session.query(model_cls)
+ .execution_options(**{SKIP_VISIBILITY_FILTER_CLASSES: {model_cls}})
+ .filter(pk_col.in_(primary_keys))
+ .all()
+ )
+ return {
+ getattr(resource, pk_col.name): get_extra_editor_subject_ids(resource)
+ for resource in resources
+ }
+
+
def _render_permission_instructions_link(
*,
datasource_id: str = "",
diff --git a/tests/integration_tests/dashboards/api_tests.py b/tests/integration_tests/dashboards/api_tests.py
index 52857d5f572..456e634071c 100644
--- a/tests/integration_tests/dashboards/api_tests.py
+++ b/tests/integration_tests/dashboards/api_tests.py
@@ -921,6 +921,55 @@ class TestDashboardApi(ApiEditorsTestCaseMixin, InsertChartMixin, SupersetTestCa
db.session.delete(dashboard)
db.session.commit()
+ def test_get_dashboards_list_omits_extra_editors_by_default(self):
+ """No EXTRA_EDITORS_RESOLVER configured: list rows omit extra_editors."""
+ admin = self.get_user("admin")
+ dashboard = self.insert_dashboard(
+ "no_extra_editors_list_dashboard",
+ "no-extra-editors-list-dashboard",
+ [admin.id],
+ )
+ try:
+ self.login(ADMIN_USERNAME)
+ rv = self.client.get("api/v1/dashboard/")
+ assert rv.status_code == 200
+ data = json.loads(rv.data.decode("utf-8"))
+ row = next(
+ d
+ for d in data["result"]
+ if d["dashboard_title"] == dashboard.dashboard_title
+ )
+ assert "extra_editors" not in row
+ finally:
+ db.session.delete(dashboard)
+ db.session.commit()
+
+ @with_config({"EXTRA_EDITORS_RESOLVER": lambda resource: [123]})
+ def test_get_dashboards_list_includes_extra_editors_when_resolver_configured(
+ self,
+ ):
+ """List rows get extra_editors too, mirroring the single-object GET."""
+ admin = self.get_user("admin")
+ dashboard = self.insert_dashboard(
+ "extra_editors_list_dashboard",
+ "extra-editors-list-dashboard",
+ [admin.id],
+ )
+ try:
+ self.login(ADMIN_USERNAME)
+ rv = self.client.get("api/v1/dashboard/")
+ assert rv.status_code == 200
+ data = json.loads(rv.data.decode("utf-8"))
+ row = next(
+ d
+ for d in data["result"]
+ if d["dashboard_title"] == dashboard.dashboard_title
+ )
+ assert row["extra_editors"] == [123]
+ finally:
+ db.session.delete(dashboard)
+ db.session.commit()
+
def test_get_charts_admin_sees_existing_charts(self):
"""Regression for #25890: GET /api/v1/chart/ as an Admin user should
return existing charts, not an empty list."""
@@ -943,6 +992,41 @@ class TestDashboardApi(ApiEditorsTestCaseMixin, InsertChartMixin, SupersetTestCa
db.session.delete(chart)
db.session.commit()
+ def test_get_charts_list_omits_extra_editors_by_default(self):
+ """No EXTRA_EDITORS_RESOLVER configured: list rows omit extra_editors."""
+ admin = self.get_user("admin")
+ chart = self.insert_chart(
+ "no_extra_editors_list_chart", [admin.id], 1, params="{}"
+ )
+ try:
+ self.login(ADMIN_USERNAME)
+ rv = self.client.get("api/v1/chart/")
+ assert rv.status_code == 200
+ data = json.loads(rv.data.decode("utf-8"))
+ row = next(c for c in data["result"] if c["slice_name"] == chart.slice_name)
+ assert "extra_editors" not in row
+ finally:
+ db.session.delete(chart)
+ db.session.commit()
+
+ @with_config({"EXTRA_EDITORS_RESOLVER": lambda resource: [123]})
+ def test_get_charts_list_includes_extra_editors_when_resolver_configured(self):
+ """List rows get extra_editors too, mirroring the single-object GET."""
+ admin = self.get_user("admin")
+ chart = self.insert_chart(
+ "extra_editors_list_chart", [admin.id], 1, params="{}"
+ )
+ try:
+ self.login(ADMIN_USERNAME)
+ rv = self.client.get("api/v1/chart/")
+ assert rv.status_code == 200
+ data = json.loads(rv.data.decode("utf-8"))
+ row = next(c for c in data["result"] if c["slice_name"] == chart.slice_name)
+ assert row["extra_editors"] == [123]
+ finally:
+ db.session.delete(chart)
+ db.session.commit()
+
def test_get_dashboards_filter(self):
"""
Dashboard API: Test get dashboards filter