This is an automated email from the ASF dual-hosted git repository.
EnxDev 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 069605828d1 feat(subjects): scope principal listings and default new
assets to creator groups (#42472)
069605828d1 is described below
commit 069605828d109cfc73b38f0424128eb446fc434c
Author: Enzo Martellucci <[email protected]>
AuthorDate: Wed Jul 29 14:36:45 2026 +0200
feat(subjects): scope principal listings and default new assets to creator
groups (#42472)
Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
---
.github/workflows/superset-frontend.yml | 6 +
UPDATING.md | 24 +
docs/admin_docs/security/security.mdx | 45 ++
docs/static/feature-flags.json | 7 +
scripts/change_detector.py | 47 +-
superset/commands/chart/importers/v1/__init__.py | 9 +-
superset/commands/chart/importers/v1/utils.py | 23 +-
.../commands/dashboard/importers/v1/__init__.py | 15 +-
superset/commands/dashboard/importers/v1/utils.py | 17 +-
superset/commands/dataset/create.py | 4 +-
superset/commands/importers/v1/assets.py | 13 +-
superset/commands/importers/v1/examples.py | 7 +
superset/commands/utils.py | 8 +-
superset/config.py | 9 +
superset/daos/dashboard.py | 24 +-
.../dashboard/tool/generate_dashboard.py | 8 +-
superset/models/dashboard.py | 10 +-
superset/models/helpers.py | 11 +-
superset/security/manager.py | 10 +-
superset/subjects/api.py | 3 +-
superset/subjects/filters.py | 16 +
superset/subjects/utils.py | 206 ++++++++-
superset/views/base_api.py | 11 +-
superset/views/core.py | 20 +-
superset/views/dashboard/views.py | 6 +-
superset/views/users/schemas.py | 8 +
tests/integration_tests/subjects/api_tests.py | 22 +
tests/integration_tests/users/api_tests.py | 9 +
.../commands/dataset/test_create_subjects.py | 71 +++
.../dashboard/tool/test_dashboard_generation.py | 32 ++
tests/unit_tests/scripts/change_detector_test.py | 63 +++
.../security/test_audit_event_metrics.py | 55 +++
tests/unit_tests/subjects/test_api_filters.py | 84 ++++
.../subjects/test_creator_group_call_sites.py | 508 +++++++++++++++++++++
.../subjects/test_creator_group_defaults.py | 293 ++++++++++++
tests/unit_tests/subjects/test_utils.py | 190 ++++++++
tests/unit_tests/views/test_related_include_ids.py | 124 +++++
tests/unit_tests/views/test_user_schemas.py | 52 +++
38 files changed, 2035 insertions(+), 35 deletions(-)
diff --git a/.github/workflows/superset-frontend.yml
b/.github/workflows/superset-frontend.yml
index 46cb237c4d8..a7e20ab8235 100644
--- a/.github/workflows/superset-frontend.yml
+++ b/.github/workflows/superset-frontend.yml
@@ -23,6 +23,12 @@ jobs:
frontend-build:
runs-on: ubuntu-26.04
timeout-minutes: 30
+ # The change detector reads the PR's file list, which needs
+ # `pull-requests: read`. Public repos serve that endpoint without it;
+ # private forks return 403.
+ permissions:
+ contents: read
+ pull-requests: read
outputs:
should-run: ${{ steps.check.outputs.frontend }}
steps:
diff --git a/UPDATING.md b/UPDATING.md
index aac6792c8fb..c5eb722118f 100644
--- a/UPDATING.md
+++ b/UPDATING.md
@@ -24,6 +24,30 @@ assists people when migrating to a new version.
## Next
+### Principal listing APIs now honour related-field filters
+
+Two authorization-related listing behaviors changed for API clients. Neither
+affects the Superset UI — `include_ids` and `security/subject` have no frontend
+callers — but external clients will notice:
+
+- `GET /api/v1/<resource>/related/<column>?include_ids=...` now applies the
+ endpoint's `base_related_field_filters`. Previously the forced IDs were
fetched
+ with no filtering, so a caller could resolve principals the related-field
+ filters deliberately hide; those IDs are now omitted from the response.
+- `GET /api/v1/security/subject/` now honours `EXCLUDE_USERS_FROM_LISTS` and
+ `EXTRA_RELATED_QUERY_FILTERS`, matching every other principal-listing
endpoint.
+ List counts may drop, and fetching an excluded principal via
+ `GET /api/v1/security/subject/<id>` now returns `404`.
+
+### v1 chart import no longer re-adds the importer as editor on overwrite
+
+Re-importing over an existing chart (an overwrite or a soft-delete restore) no
+longer appends the importing user to the chart's `editors`, matching the
+dashboard importer's behavior. This changes anything only for a user who was
not
+already an editor of that chart — typically an admin overwriting a chart they
do
+not own, who was previously added as an editor as a side effect of the import.
+Newly-created charts are unaffected.
+
### Dashboard "Export Data to Excel" requires a Celery worker and S3 bucket
A new dashboard action exports every chart's data to a single multi-sheet
diff --git a/docs/admin_docs/security/security.mdx
b/docs/admin_docs/security/security.mdx
index e6c11a7ae85..0045f25241e 100644
--- a/docs/admin_docs/security/security.mdx
+++ b/docs/admin_docs/security/security.mdx
@@ -247,6 +247,51 @@ This feature is particularly useful for:
- Granting access to dashboards without exposing the underlying datasets for
other uses
- Creating dashboard-specific access patterns that don't align with dataset
permissions
+#### Assigning the creator's groups automatically
+
+```python
+FEATURE_FLAGS = {
+ "ENABLE_VIEWERS": True,
+ "ASSIGN_CREATOR_GROUPS_AS_VIEWERS": True,
+}
+```
+
+This requires `ENABLE_VIEWERS` — it has no effect on its own, since it works
by populating the
+`viewers` relationship that `ENABLE_VIEWERS` governs.
+
+With this enabled, a newly created dashboard or chart is shared read-only with
every group its
+creator belongs to, unless the create payload names viewers explicitly. It
applies to every path
+that creates an asset: the REST API, `/dashboard/new/`, save-as from Explore,
dashboard copy,
+the import commands, and the MCP tools. Datasets are unaffected, as they have
editors but no
+viewers.
+
+For dashboards, viewer access additionally requires the dashboard to be
**published** — the same
+rule that governs every dashboard viewer. Until a new dashboard is published
its creator's groups
+see nothing, and because it now has viewers the dataset-based fallback no
longer applies to it
+either; group members gain access only once it is published. Charts have no
such publish gate, so a
+new chart's viewer grant takes effect immediately. Note, though, that the
viewer relationship
+governs access to the chart itself; rendering its **data** still runs the
normal datasource
+permission check unless `VIEWER_PROMISCUOUS_MODE` is enabled — a group member
without access to the
+underlying dataset can open the chart but won't load its data.
+
+Note that this **narrows** access rather than widening it. Because an asset
with no viewers falls
+back to dataset-based access, giving new assets a viewer makes that fallback
unreachable for them:
+a colleague who could previously open a new dashboard by virtue of dataset
access can no longer do
+so unless they share one of the creator's groups. Existing assets are
untouched — only assets
+created after the feature flag is enabled are affected.
+
+The same narrowing affects background execution as a fixed identity. Alerts,
reports, and
+thumbnails that run as a `FixedExecutor` service account (via
`ALERT_REPORTS_EXECUTE_AS` or
+`THUMBNAIL_EXECUTE_AS`) reach a chart or dashboard through the dataset
fallback when that account
+is neither an editor nor a viewer. Once a new asset is scoped to the creator's
groups that
+fallback is gone, so rendering fails `raise_for_access` unless the executor
account is an admin,
+an editor, or shares one of those groups. If a fixed executor must render
assets it does not own,
+grant it the needed access (make it an admin or add it to the relevant groups)
— or leave the
+feature flag off.
+
+The setting assigns groups as *viewers*, not editors, so group members get
read-only access and
+cannot modify or delete the asset.
+
### SQL Execution Security Considerations
Apache Superset includes features designed to provide safeguards when
interacting with connected databases, such as the `DISALLOWED_SQL_FUNCTIONS`
configuration setting. This aims to prevent the execution of potentially
harmful database functions or system variables directly from Superset
interfaces like SQL Lab.
diff --git a/docs/static/feature-flags.json b/docs/static/feature-flags.json
index 5646315efd9..7a7bcad85e5 100644
--- a/docs/static/feature-flags.json
+++ b/docs/static/feature-flags.json
@@ -138,6 +138,13 @@
"lifecycle": "testing",
"description": "Allow users to export full CSV of table viz type.
Warning: Could cause server memory/compute issues with large datasets."
},
+ {
+ "name": "ASSIGN_CREATOR_GROUPS_AS_VIEWERS",
+ "default": false,
+ "lifecycle": "testing",
+ "description": "Share a newly created dashboard or chart read-only
with every group its creator belongs to, unless the create payload names
viewers explicitly. Requires ENABLE_VIEWERS. Narrows access: an asset with no
viewers falls back to datasource permissions; one with viewers is limited to
its editors and viewers, and dashboards must also be published before those
viewers gain access.",
+ "category": "security"
+ },
{
"name": "AWS_DATABASE_IAM_AUTH",
"default": false,
diff --git a/scripts/change_detector.py b/scripts/change_detector.py
index fac6caa3ba4..5e96078327b 100755
--- a/scripts/change_detector.py
+++ b/scripts/change_detector.py
@@ -31,9 +31,12 @@ from urllib.request import Request, urlopen
MAX_RETRIES: int = 4
RETRY_BACKOFF_SECONDS: int = 2
REQUEST_TIMEOUT_SECONDS: int = 30
-# GitHub returns 429 (and 403 for secondary rate limits) when throttling, which
-# is transient and worth retrying alongside 5xx server errors.
-RETRYABLE_STATUS_CODES: frozenset[int] = frozenset({403, 429})
+# GitHub returns 429 when throttling, which is transient and worth retrying
+# alongside 5xx server errors. It also returns 403 for two very different
+# reasons — a secondary rate limit, and a token missing the required scope —
+# so 403 is only retried when the response headers show throttling. Retrying a
+# scope failure just delays the error and buries its cause.
+RETRYABLE_STATUS_CODES: frozenset[int] = frozenset({429})
# Define patterns for each group of files you're interested in
PATTERNS = {
@@ -69,6 +72,37 @@ PATTERNS = {
GITHUB_TOKEN = os.environ.get("GITHUB_TOKEN")
+def _is_rate_limited(err: HTTPError) -> bool:
+ """Whether a 403 is GitHub throttling rather than a missing token scope."""
+ headers = getattr(err, "headers", None)
+ if headers is None:
+ return False
+ try:
+ return (
+ headers.get("x-ratelimit-remaining") == "0"
+ or headers.get("retry-after") is not None
+ )
+ except AttributeError:
+ return False
+
+
+def _api_error_detail(err: object) -> str:
+ """The API's own explanation, e.g. ``Resource not accessible by
integration``.
+
+ Without this a 403 is indistinguishable from a rate limit in the CI log.
+ """
+ try:
+ body = err.read().decode("utf-8", "replace") # type:
ignore[attr-defined]
+ except Exception: # noqa: BLE001 # pylint: disable=broad-except
+ return ""
+ try:
+ detail = json.loads(body).get("message") or body
+ except (ValueError, AttributeError):
+ detail = body
+ # Bound the output so a large/HTML error page can't flood the CI log.
+ return detail[:500]
+
+
def fetch_files_github_api(url: str): # type: ignore
"""Fetches data using GitHub API, retrying on transient failures."""
req = Request(url) # noqa: S310
@@ -88,9 +122,14 @@ def fetch_files_github_api(url: str): # type: ignore
# re-raise once the retry budget is exhausted.
status = getattr(err, "code", None)
is_transient = (
- status is None or status >= 500 or status in
RETRYABLE_STATUS_CODES
+ status is None
+ or status >= 500
+ or status in RETRYABLE_STATUS_CODES
+ or (status == 403 and _is_rate_limited(err)) # type:
ignore[arg-type]
)
if not is_transient or attempt == MAX_RETRIES:
+ if detail := _api_error_detail(err):
+ print(f"GitHub API error {status}: {detail}")
raise
wait = RETRY_BACKOFF_SECONDS * 2 ** (attempt - 1)
print(
diff --git a/superset/commands/chart/importers/v1/__init__.py
b/superset/commands/chart/importers/v1/__init__.py
index 1b28e19df5a..3f0ea68608a 100644
--- a/superset/commands/chart/importers/v1/__init__.py
+++ b/superset/commands/chart/importers/v1/__init__.py
@@ -35,6 +35,7 @@ from superset.daos.chart import ChartDAO
from superset.databases.schemas import ImportV1DatabaseSchema
from superset.datasets.schemas import ImportV1DatasetSchema
from superset.extensions import feature_flag_manager
+from superset.subjects.utils import get_default_viewers_for_current_user
class ImportChartsCommand(ImportModelsCommand):
@@ -88,6 +89,10 @@ class ImportChartsCommand(ImportModelsCommand):
dataset = import_dataset(config, overwrite=False)
datasets[str(dataset.uuid)] = dataset
+ # Resolve the creator's default viewers once for the whole bundle
+ # rather than once per chart (a membership query each).
+ default_viewers = get_default_viewers_for_current_user()
+
# import charts with the correct parent ref
for file_name, config in configs.items():
if file_name.startswith("charts/") and config["dataset_uuid"] in
datasets:
@@ -103,7 +108,9 @@ class ImportChartsCommand(ImportModelsCommand):
"datasource_name": dataset.table_name,
}
config = update_chart_config_dataset(config, dataset_dict)
- chart = import_chart(config, overwrite=overwrite)
+ chart = import_chart(
+ config, overwrite=overwrite,
default_viewers=default_viewers
+ )
# Handle tags using import_tag function
if feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM"):
diff --git a/superset/commands/chart/importers/v1/utils.py
b/superset/commands/chart/importers/v1/utils.py
index a4e0bba2e1b..5488777a8a7 100644
--- a/superset/commands/chart/importers/v1/utils.py
+++ b/superset/commands/chart/importers/v1/utils.py
@@ -25,6 +25,7 @@ from superset.commands.importers.v1.utils import
find_existing_for_import
from superset.migrations.shared.migrate_viz import processors
from superset.migrations.shared.migrate_viz.base import MigrateViz
from superset.models.slice import Slice
+from superset.subjects.models import Subject
from superset.utils import json
from superset.utils.core import AnnotationType, get_user
@@ -137,6 +138,7 @@ def import_chart(
config: dict[str, Any],
overwrite: bool = False,
ignore_permissions: bool = False,
+ default_viewers: list[Subject] | None = None,
) -> Slice:
"""Import a chart from a config dict, handling existing matches.
@@ -201,12 +203,29 @@ def import_chart(
if chart.id is None:
db.session.flush()
- if user:
- from superset.subjects.utils import get_user_subject
+ # Only newly created charts inherit the creator's editor/viewer defaults;
+ # re-importing over an existing chart (overwrite or soft-delete restore)
+ # must not silently grant the importer's groups access. Mirrors the
+ # dashboard importer's ``not existing`` guard.
+ if not existing and user:
+ from superset.subjects.utils import (
+ get_default_viewers_for_new_asset,
+ get_user_subject,
+ )
subj = get_user_subject(user.id)
if subj and subj not in chart.editors:
chart.editors.append(subj)
+ # Resolved once by bulk importers and passed in; recomputed here only
+ # for direct callers that omit it (one membership query per asset).
+ viewers = (
+ default_viewers
+ if default_viewers is not None
+ else get_default_viewers_for_new_asset(user.id)
+ )
+ for viewer in viewers:
+ if viewer not in chart.viewers:
+ chart.viewers.append(viewer)
return chart
diff --git a/superset/commands/dashboard/importers/v1/__init__.py
b/superset/commands/dashboard/importers/v1/__init__.py
index 53a253a6953..787ea74a8e0 100644
--- a/superset/commands/dashboard/importers/v1/__init__.py
+++ b/superset/commands/dashboard/importers/v1/__init__.py
@@ -49,6 +49,7 @@ from superset.extensions import feature_flag_manager
from superset.migrations.shared.native_filters import migrate_dashboard
from superset.models.dashboard import Dashboard, dashboard_slices
from superset.models.slice import Slice
+from superset.subjects.utils import get_default_viewers_for_current_user
from superset.themes.schemas import ImportV1ThemeSchema
from superset.utils.decorators import transaction
@@ -163,6 +164,10 @@ class ImportDashboardsCommand(ImportModelsCommand):
"datasource_name": dataset.table_name,
}
+ # Resolve the creator's default viewers once for the whole bundle
+ # rather than once per chart/dashboard (a membership query each).
+ default_viewers = get_default_viewers_for_current_user()
+
# import charts with the correct parent ref
charts = []
chart_ids: dict[str, int] = {}
@@ -175,7 +180,11 @@ class ImportDashboardsCommand(ImportModelsCommand):
dataset_dict = dataset_info[config["dataset_uuid"]]
config = update_chart_config_dataset(config, dataset_dict)
- chart = import_chart(config, overwrite=overwrite_assets)
+ chart = import_chart(
+ config,
+ overwrite=overwrite_assets,
+ default_viewers=default_viewers,
+ )
charts.append(chart)
chart_ids[str(chart.uuid)] = chart.id
@@ -221,7 +230,9 @@ class ImportDashboardsCommand(ImportModelsCommand):
# Theme not found, set to None for graceful fallback
config["theme_id"] = None
del config["theme_uuid"]
- dashboard = import_dashboard(config, overwrite=overwrite)
+ dashboard = import_dashboard(
+ config, overwrite=overwrite,
default_viewers=default_viewers
+ )
dashboards.append(dashboard)
# Resolve the dashboard's chart membership from the imported
diff --git a/superset/commands/dashboard/importers/v1/utils.py
b/superset/commands/dashboard/importers/v1/utils.py
index e556674d8bd..9941eb123b6 100644
--- a/superset/commands/dashboard/importers/v1/utils.py
+++ b/superset/commands/dashboard/importers/v1/utils.py
@@ -23,6 +23,7 @@ from superset.commands.exceptions import ImportFailedError
from superset.commands.importers.v1.utils import find_existing_for_import
from superset.daos.dashboard import DashboardDAO
from superset.models.dashboard import Dashboard
+from superset.subjects.models import Subject
from superset.utils import json
from superset.utils.core import get_user
@@ -277,6 +278,7 @@ def import_dashboard( # noqa: C901
config: dict[str, Any],
overwrite: bool = False,
ignore_permissions: bool = False,
+ default_viewers: list[Subject] | None = None,
) -> Dashboard:
"""Import a dashboard from a config dict, handling existing matches.
@@ -449,10 +451,23 @@ def import_dashboard( # noqa: C901
db.session.flush()
if not existing and user:
- from superset.subjects.utils import get_user_subject
+ from superset.subjects.utils import (
+ get_default_viewers_for_new_asset,
+ get_user_subject,
+ )
subj = get_user_subject(user.id)
if subj and subj not in dashboard.editors:
dashboard.editors.append(subj)
+ # Resolved once by bulk importers and passed in; recomputed here only
+ # for direct callers that omit it (one membership query per asset).
+ viewers = (
+ default_viewers
+ if default_viewers is not None
+ else get_default_viewers_for_new_asset(user.id)
+ )
+ for viewer in viewers:
+ if viewer not in dashboard.viewers:
+ dashboard.viewers.append(viewer)
return dashboard
diff --git a/superset/commands/dataset/create.py
b/superset/commands/dataset/create.py
index 7289f168806..310fa9b24d8 100644
--- a/superset/commands/dataset/create.py
+++ b/superset/commands/dataset/create.py
@@ -120,7 +120,9 @@ class CreateDatasetCommand(CreateMixin, BaseCommand):
except SupersetSecurityException as ex:
exceptions.append(DatasetDataAccessIsNotAllowed(ex.error.message))
- populate_subjects(self._properties, exceptions)
+ # Datasets have editors only — there is no ``sqlatable_viewers`` table,
+ # so a ``viewers`` key would be dropped by the DAO's ``setattr`` loop.
+ populate_subjects(self._properties, exceptions, include_viewers=False)
if exceptions:
raise DatasetInvalidError(exceptions=exceptions)
diff --git a/superset/commands/importers/v1/assets.py
b/superset/commands/importers/v1/assets.py
index 97f394b920b..25cd33c7d25 100644
--- a/superset/commands/importers/v1/assets.py
+++ b/superset/commands/importers/v1/assets.py
@@ -52,6 +52,7 @@ from superset.models.dashboard import Dashboard
from superset.models.slice import Slice
from superset.models.sql_lab import SavedQuery
from superset.queries.saved_queries.schemas import ImportV1SavedQuerySchema
+from superset.subjects.utils import get_default_viewers_for_current_user
from superset.utils.decorators import on_error, transaction
@@ -140,13 +141,19 @@ class ImportAssetsCommand(BaseCommand):
"datasource_name": dataset.table_name,
}
+ # Resolve the creator's default viewers once for the whole bundle
+ # rather than once per chart/dashboard (a membership query each).
+ default_viewers = get_default_viewers_for_current_user()
+
# import charts
charts = []
for file_name, config in configs.items():
if file_name.startswith("charts/"):
dataset_dict = dataset_info[config["dataset_uuid"]]
config = update_chart_config_dataset(config, dataset_dict)
- chart = import_chart(config, overwrite=overwrite)
+ chart = import_chart(
+ config, overwrite=overwrite,
default_viewers=default_viewers
+ )
charts.append(chart)
chart_ids[str(chart.uuid)] = chart.id
@@ -161,7 +168,9 @@ class ImportAssetsCommand(BaseCommand):
for file_name, config in configs.items():
if file_name.startswith("dashboards/"):
config = update_id_refs(config, chart_ids, dataset_info)
- dashboard = import_dashboard(config, overwrite=overwrite)
+ dashboard = import_dashboard(
+ config, overwrite=overwrite,
default_viewers=default_viewers
+ )
# set ref in the dashboard_slices table
# Use ORM-level reassignment instead of Core
diff --git a/superset/commands/importers/v1/examples.py
b/superset/commands/importers/v1/examples.py
index 19fe811044a..773fc3c1bcf 100644
--- a/superset/commands/importers/v1/examples.py
+++ b/superset/commands/importers/v1/examples.py
@@ -46,6 +46,7 @@ from superset.datasets.schemas import ImportV1DatasetSchema
from superset.exceptions import QueryClauseValidationException
from superset.models.core import Database
from superset.sql.parse import transpile_to_dialect
+from superset.subjects.utils import get_default_viewers_for_current_user
from superset.utils.core import get_example_default_schema
from superset.utils.decorators import transaction
@@ -199,6 +200,10 @@ class ImportExamplesCommand(ImportModelsCommand):
"datasource_name": dataset.table_name,
}
+ # Resolve the creator's default viewers once for the whole bundle
+ # rather than once per chart/dashboard (a membership query each).
+ default_viewers = get_default_viewers_for_current_user()
+
# import charts
chart_ids: dict[str, int] = {}
for file_name, config in configs.items():
@@ -212,6 +217,7 @@ class ImportExamplesCommand(ImportModelsCommand):
config,
overwrite=overwrite,
ignore_permissions=True,
+ default_viewers=default_viewers,
)
chart_ids[str(chart.uuid)] = chart.id
@@ -228,6 +234,7 @@ class ImportExamplesCommand(ImportModelsCommand):
config,
overwrite=overwrite,
ignore_permissions=True,
+ default_viewers=default_viewers,
)
dashboard.published = True
diff --git a/superset/commands/utils.py b/superset/commands/utils.py
index 4caae03761f..d5325a0c9af 100644
--- a/superset/commands/utils.py
+++ b/superset/commands/utils.py
@@ -35,6 +35,7 @@ from superset.daos.tag import TagDAO
from superset.subjects.exceptions import SubjectsNotFoundValidationError
from superset.subjects.models import Subject
from superset.subjects.utils import (
+ get_default_viewers_for_new_asset,
get_or_create_user_subject as get_user_subject,
get_subject,
get_user_subject_ids,
@@ -146,7 +147,10 @@ def populate_subjects(
except ValidationError as ex:
exceptions.append(ex)
- if include_viewers and properties.get("viewers") is not None:
+ if not include_viewers:
+ return
+
+ if properties.get("viewers") is not None:
try:
properties["viewers"] = populate_subject_list(
properties["viewers"],
@@ -155,6 +159,8 @@ def populate_subjects(
)
except ValidationError as ex:
exceptions.append(ex)
+ elif default_viewers := get_default_viewers_for_new_asset(get_user_id()):
+ properties["viewers"] = default_viewers
def compute_subjects(
diff --git a/superset/config.py b/superset/config.py
index b7c71f6b610..d77c26c1002 100644
--- a/superset/config.py
+++ b/superset/config.py
@@ -849,6 +849,15 @@ DEFAULT_FEATURE_FLAGS: dict[str, bool] = {
# @lifecycle: testing
# @category: security
"ENABLE_VIEWERS": False,
+ # Share a newly created dashboard or chart read-only with every group its
+ # creator belongs to, unless the create payload names viewers explicitly.
+ # Requires ENABLE_VIEWERS. Narrows access: an asset with no viewers falls
+ # back to datasource permissions; one with viewers is limited to its
editors
+ # and viewers, and dashboards must also be published before those viewers
+ # gain access.
+ # @lifecycle: testing
+ # @category: security
+ "ASSIGN_CREATOR_GROUPS_AS_VIEWERS": False,
# Supports simultaneous data and dashboard virtualization for backend
performance
# @lifecycle: stable
# @category: runtime_config
diff --git a/superset/daos/dashboard.py b/superset/daos/dashboard.py
index 40267df21ca..336176a66f8 100644
--- a/superset/daos/dashboard.py
+++ b/superset/daos/dashboard.py
@@ -429,11 +429,22 @@ class DashboardDAO(BaseDAO[Dashboard]):
raise DashboardForbiddenError()
dash = Dashboard()
+ # The copied dashboard and every chart cloned below share one creator,
+ # so both lookups are resolved here rather than inside the loop, where
+ # they would cost two extra queries for each chart in the dashboard.
+ creator_editors: list[Any] = []
+ creator_viewers: list[Any] = []
if g.user:
- from superset.subjects.utils import get_user_subject
+ from superset.subjects.utils import (
+ get_default_viewers_for_new_asset,
+ get_user_subject,
+ )
user_subject = get_user_subject(g.user.id)
- dash.editors = [user_subject] if user_subject else []
+ creator_editors = [user_subject] if user_subject else []
+ creator_viewers = get_default_viewers_for_new_asset(g.user.id)
+ dash.editors = creator_editors
+ dash.viewers = creator_viewers
dash.dashboard_title = data["dashboard_title"]
dash.css = data.get("css")
@@ -443,11 +454,10 @@ class DashboardDAO(BaseDAO[Dashboard]):
# Duplicating slices as well, mapping old ids to new ones
for slc in original_dash.slices:
new_slice = slc.clone()
- if g.user:
- from superset.subjects.utils import get_user_subject
-
- user_subject = get_user_subject(g.user.id)
- new_slice.editors = [user_subject] if user_subject else []
+ # ``Slice.clone()`` carries over no subjects, so both
+ # collections start empty on the new chart.
+ new_slice.editors = list(creator_editors)
+ new_slice.viewers = list(creator_viewers)
db.session.add(new_slice)
db.session.flush()
new_slice.dashboards.append(dash)
diff --git a/superset/mcp_service/dashboard/tool/generate_dashboard.py
b/superset/mcp_service/dashboard/tool/generate_dashboard.py
index a244edbd4c8..71319199279 100644
--- a/superset/mcp_service/dashboard/tool/generate_dashboard.py
+++ b/superset/mcp_service/dashboard/tool/generate_dashboard.py
@@ -347,11 +347,17 @@ def generate_dashboard( # noqa: C901
.first()
)
if current_user:
- from superset.subjects.utils import get_user_subject
+ from superset.subjects.utils import (
+ get_default_viewers_for_new_asset,
+ get_user_subject,
+ )
subj = get_user_subject(current_user.id)
if subj:
dashboard.editors = [subj]
+ dashboard.viewers = get_default_viewers_for_new_asset(
+ current_user.id
+ )
fresh_charts = (
db.session.query(Slice)
diff --git a/superset/models/dashboard.py b/superset/models/dashboard.py
index 7ef6d16db07..dbddea06aea 100644
--- a/superset/models/dashboard.py
+++ b/superset/models/dashboard.py
@@ -71,7 +71,10 @@ def copy_dashboard(_mapper: Mapper, _connection: Connection,
target: Dashboard)
if dashboard_id is None:
return
- from superset.subjects.utils import get_user_subject
+ from superset.subjects.utils import (
+ get_default_viewers_for_groups,
+ get_user_subject,
+ )
session = sqla.inspect(target).session # pylint: disable=disallowed-name
new_user = session.query(User).filter_by(id=target.id).first()
@@ -91,6 +94,11 @@ def copy_dashboard(_mapper: Mapper, _connection: Connection,
target: Dashboard)
json_metadata=template.json_metadata,
slices=template.slices,
editors=editors,
+ # Resolved from the in-memory collection: this runs in ``after_insert``
+ # for the user, before their ``ab_user_group`` rows are written.
+ viewers=get_default_viewers_for_groups(
+ list(getattr(new_user, "groups", []) or [])
+ ),
)
session.add(dashboard)
diff --git a/superset/models/helpers.py b/superset/models/helpers.py
index 1376a6bfbb6..5748c04e097 100644
--- a/superset/models/helpers.py
+++ b/superset/models/helpers.py
@@ -646,7 +646,10 @@ class ImportExportMixin(UUIDMixin):
def reset_ownership(self) -> None:
"""object will belong to the current user"""
- from superset.subjects.utils import get_user_subject
+ from superset.subjects.utils import (
+ get_default_viewers_for_new_asset,
+ get_user_subject,
+ )
# Reset the audit pointers. When a Flask request context is
# available we explicitly stamp the current user, otherwise we
@@ -664,6 +667,12 @@ class ImportExportMixin(UUIDMixin):
user_subject = get_user_subject(g.user.id)
if user_subject:
self.editors = [user_subject]
+ # Only dashboards and charts have ``viewers``; this mixin also
+ # serves datasets, which have editors only. Defaults never replace
+ # viewers already present on the instance, matching the create
+ # commands' "explicit viewers win" rule.
+ if hasattr(self, "viewers") and not self.viewers: # type:
ignore[has-type]
+ self.viewers = get_default_viewers_for_new_asset(g.user.id)
else:
self.created_by = None
self.changed_by = None
diff --git a/superset/security/manager.py b/superset/security/manager.py
index ff214c50908..803c65bcd90 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -193,10 +193,16 @@ def _log_audit_event(action: str, payload: dict[str,
Any]) -> None:
configured implementation (DBEventLogger, S3EventLogger, etc.)
receives these security audit events.
"""
- from superset.extensions import (
- event_logger, # pylint: disable=import-outside-toplevel
+ from superset.extensions import ( # pylint:
disable=import-outside-toplevel
+ event_logger,
+ stats_logger_manager,
)
+ try:
+ stats_logger_manager.instance.incr(f"security.{action}")
+ except Exception: # pylint: disable=broad-except
+ logger.warning("Failed to emit audit metric: %s", action,
exc_info=True)
+
user_id = get_user_id()
try:
event_logger.log(
diff --git a/superset/subjects/api.py b/superset/subjects/api.py
index 2984dddc3df..dfd9da9de0f 100644
--- a/superset/subjects/api.py
+++ b/superset/subjects/api.py
@@ -19,7 +19,7 @@ import logging
from flask_appbuilder.models.sqla.interface import SQLAInterface
from superset.constants import RouteMethod
-from superset.subjects.filters import SubjectAllTextFilter
+from superset.subjects.filters import SubjectAllTextFilter, SubjectListFilter
from superset.subjects.models import Subject
from superset.subjects.schemas import openapi_spec_methods_override
from superset.views.base_api import BaseSupersetModelRestApi
@@ -52,6 +52,7 @@ class SubjectRestApi(BaseSupersetModelRestApi):
resource_name = "security/subject"
allow_browser_login = True
+ base_filters = [["id", SubjectListFilter, lambda: []]]
openapi_spec_tag = "Security Subjects"
openapi_spec_methods = openapi_spec_methods_override
diff --git a/superset/subjects/filters.py b/superset/subjects/filters.py
index c99fd7df972..8305a07e470 100644
--- a/superset/subjects/filters.py
+++ b/superset/subjects/filters.py
@@ -101,6 +101,22 @@ def _apply_subject_list_filters(query: Query) -> Query:
return _apply_extra_related_query_filters(_apply_excluded_users(query))
+class SubjectListFilter(BaseFilter): # pylint: disable=too-few-public-methods
+ """Scope a Subject query to the principals a deployment chooses to expose.
+
+ The related-item endpoints backing the editor/viewer pickers already apply
+ ``EXCLUDE_USERS_FROM_LISTS`` and ``EXTRA_RELATED_QUERY_FILTERS``.
+ ``SubjectRestApi`` serves the same principal directory — including
+ ``user.email`` on its show endpoint — so it applies them too.
+ """
+
+ name = _("Subject list")
+ arg_name = "subject_list"
+
+ def apply(self, query: Query, value: Any) -> Query:
+ return _apply_subject_list_filters(query)
+
+
def subject_type_filter(entity_config_key: str | None = None) ->
type[BaseFilter]:
"""Return a BaseFilter subclass scoped to a specific entity config.
diff --git a/superset/subjects/utils.py b/superset/subjects/utils.py
index f19bafafc50..b6cccc8150c 100644
--- a/superset/subjects/utils.py
+++ b/superset/subjects/utils.py
@@ -19,9 +19,10 @@ from __future__ import annotations
from typing import TYPE_CHECKING
if TYPE_CHECKING:
- from flask_appbuilder.security.sqla.models import Role, User
- from sqlalchemy.sql import CompoundSelect
+ from flask_appbuilder.security.sqla.models import Group, Role, User
+ from sqlalchemy.sql import CompoundSelect, Select
+from flask import has_app_context
from sqlalchemy import select, union_all
from superset import db
@@ -79,6 +80,125 @@ def get_user_subject_ids_subquery(user_id: int) ->
CompoundSelect:
return union_all(user_subj, role_subj, group_subj, group_role_subj)
+def get_user_group_subject_ids_subquery(user_id: int) -> Select:
+ """Return a Select of GROUP-type Subject IDs for a user's groups.
+
+ Unlike :func:`get_user_subject_ids_subquery` this covers group membership
+ only — it excludes the user's own subject and any roles.
+ """
+ from flask_appbuilder.security.sqla.models import assoc_user_group
+
+ return (
+ select(Subject.id)
+ .join(assoc_user_group, Subject.group_id ==
assoc_user_group.c.group_id)
+ .where(
+ assoc_user_group.c.user_id == user_id,
+ Subject.type == SubjectType.GROUP,
+ )
+ )
+
+
+def get_user_group_subjects(user_id: int) -> list[Subject]:
+ """Return the GROUP-type Subjects for every group a user belongs to.
+
+ Any group whose Subject row is missing is backfilled on demand (see
+ :func:`get_or_create_group_subject`) rather than skipped: a single viewer
+ disables an asset's datasource-access fallback, so a partial viewer set
+ would silently lock out the members of an unsynced group.
+ """
+ from flask_appbuilder.security.sqla.models import assoc_user_group
+
+ subjects = (
+ db.session.query(Subject)
+ .filter(Subject.id.in_(get_user_group_subject_ids_subquery(user_id)))
+ .all()
+ )
+ member_group_ids = {
+ row[0]
+ for row in db.session.execute(
+ select(assoc_user_group.c.group_id).where(
+ assoc_user_group.c.user_id == user_id
+ )
+ )
+ }
+ for group_id in member_group_ids - {subject.group_id for subject in
subjects}:
+ if subject := get_or_create_group_subject(group_id):
+ subjects.append(subject)
+ return subjects
+
+
+def _assigns_creator_groups_as_viewers() -> bool:
+ from superset import is_feature_enabled
+
+ # Also requires ENABLE_VIEWERS. ``raise_for_access`` enforces any non-empty
+ # ``viewers`` relationship regardless of that flag (a viewer list removes
the
+ # datasource fallback), so assigning viewers while the viewers feature is
off
+ # would silently restrict new assets to their editors/viewers.
+ return (
+ has_app_context()
+ and is_feature_enabled("ENABLE_VIEWERS")
+ and is_feature_enabled("ASSIGN_CREATOR_GROUPS_AS_VIEWERS")
+ )
+
+
+def get_default_viewers_for_new_asset(user_id: int | None) -> list[Subject]:
+ """Return the viewers a newly created asset should default to.
+
+ The single place deciding how a creator's group membership propagates to a
+ new asset, so the paths that build assets outside the create commands
+ (save-as, importers, MCP tools) stay consistent with them. Empty unless
+ ``ASSIGN_CREATOR_GROUPS_AS_VIEWERS`` is enabled.
+
+ Only assets with a ``viewers`` relationship apply — datasets have editors
+ only. Callers inside a flush event use
+ :func:`get_default_viewers_for_groups` instead.
+ """
+ if user_id is None or not _assigns_creator_groups_as_viewers():
+ return []
+ return get_user_group_subjects(user_id)
+
+
+def get_default_viewers_for_groups(groups: list[Group]) -> list[Subject]:
+ """Like :func:`get_default_viewers_for_new_asset`, for in-memory groups.
+
+ Callers inside a SQLAlchemy flush event cannot resolve membership by
+ querying ``ab_user_group``: for a user being inserted, the association rows
+ are written *after* the user's own INSERT, so the query returns nothing.
+ Those callers already hold the group objects on the instance and pass them
+ here instead.
+
+ Resolution is all-or-nothing: if any group lacks a Subject row (an unsynced
+ group is a real deployment state — see the ``superset sync-subjects`` CLI),
+ the whole result is discarded and ``[]`` returned. A *partial* viewer set
+ would switch off the asset's datasource-access fallback
+ (``security_manager.raise_for_access``) while granting the unsynced group's
+ members nothing, locking them out; no viewers keeps the fallback intact and
+ is strictly safer. On-demand backfill isn't an option here — this runs in a
+ flush event, where :func:`get_or_create_group_subject`'s flush is unsafe.
+ """
+ if not groups or not _assigns_creator_groups_as_viewers():
+ return []
+ subjects = subjects_from_groups(groups)
+ if {subject.group_id for subject in subjects} != {group.id for group in
groups}:
+ return []
+ return subjects
+
+
+def get_default_viewers_for_current_user() -> list[Subject]:
+ """Default viewers for the acting user, resolved once for bulk callers.
+
+ The v1 importers create many assets in a single request; resolving the
+ creator's groups once here and passing the result down avoids one
+ membership query per imported asset. Mirrors the ``user = get_user()``
+ lookup the importers do internally, so the result is identical to letting
+ each call resolve it. Returns ``[]`` when there is no request user.
+ """
+ from superset.utils.core import get_user
+
+ user = get_user()
+ return get_default_viewers_for_new_asset(user.id) if user else []
+
+
def get_user_subject_ids(user_id: int) -> list[int]:
"""Return all Subject IDs that a user represents.
@@ -176,6 +296,88 @@ def get_or_create_role_subject(role_id: int) -> Subject |
None:
return get_role_subject(role_id)
+def get_group_subject(group_id: int) -> Subject | None:
+ """Get the GROUP-type Subject for a given group ID."""
+ return (
+ db.session.query(Subject)
+ .filter_by(
+ group_id=group_id,
+ type=SubjectType.GROUP,
+ )
+ .first()
+ )
+
+
+def get_or_create_group_subject(group_id: int) -> Subject | None:
+ """Get or create the GROUP-type Subject for a given group ID.
+
+ Mirrors ``get_or_create_role_subject``: a group that exists but has no
+ Subject row yet (e.g. created before the sync hooks were installed and not
+ yet backfilled) is synced on demand rather than skipped. Returns ``None``
+ only when no such group exists.
+
+ The insert runs inside a SAVEPOINT. ``Subject.group_id`` is unique, so a
+ concurrent request backfilling the same group would otherwise fail this
+ flush with an ``IntegrityError`` and poison the surrounding transaction. On
+ that conflict the savepoint rolls back and the row the other request wrote
+ is reloaded. Any ``IntegrityError`` that did *not* leave a matching row is
+ re-raised rather than swallowed — it isn't the expected unique-key race, so
+ masking it would silently drop the group from the resulting viewer set.
+
+ Unrelated pending work is flushed *before* the SAVEPOINT, so entering it
+ doesn't flush the caller's other objects into the ``except`` below where an
+ unrelated integrity failure could be mistaken for this group's conflict.
+ """
+ if subject := get_group_subject(group_id):
+ return subject
+
+ from sqlalchemy.exc import IntegrityError
+
+ from superset import security_manager
+ from superset.subjects.sync import sync_group_subject
+
+ group = db.session.get(security_manager.group_model, group_id)
+ if not group:
+ return None
+
+ # Surface any unrelated pending failure here, outside the SAVEPOINT, so the
+ # savepoint's own flush carries only the Subject insert and the ``except``
+ # sees only this group's uniqueness conflict.
+ db.session.flush()
+ try:
+ with db.session.begin_nested():
+ sync_group_subject(group)
+ except IntegrityError:
+ if subject := get_group_subject(group_id):
+ return subject
+ raise
+ return get_group_subject(group_id)
+
+
+def subjects_from_groups(groups: list[Group | int]) -> list[Subject]:
+ """Convert a list of Group objects or group IDs to GROUP-type Subjects.
+
+ Mirrors :func:`subjects_from_roles`, but resolves the whole list in one
+ query rather than one per group. Groups without a matching Subject row are
+ omitted from the result; the viewer-defaulting caller
+ (:func:`get_default_viewers_for_groups`) detects that shortfall and
discards
+ the partial result rather than applying it, so no partial viewer set leaks
+ out. On-demand backfill happens instead in :func:`get_user_group_subjects`,
+ the path that does not run inside a flush event.
+ """
+ group_ids = [group if isinstance(group, int) else group.id for group in
groups]
+ if not group_ids:
+ return []
+ return (
+ db.session.query(Subject)
+ .filter(
+ Subject.group_id.in_(group_ids),
+ Subject.type == SubjectType.GROUP,
+ )
+ .all()
+ )
+
+
def subjects_from_roles(roles: list[Role | int]) -> list[Subject]:
"""Convert a list of Role objects or role IDs to role-type Subjects.
diff --git a/superset/views/base_api.py b/superset/views/base_api.py
index 852602c700f..d556654c58b 100644
--- a/superset/views/base_api.py
+++ b/superset/views/base_api.py
@@ -501,8 +501,15 @@ class BaseSupersetModelRestApi(BaseSupersetApiMixin,
ModelRestApi):
values = [row["value"] for row in result]
ids = [id_ for id_ in ids if id_ not in values]
pk_col = datamodel.get_pk()
- # Fetch requested values from ids
- extra_rows =
db.session.query(datamodel.obj).filter(pk_col.in_(ids)).all()
+ # Fetch requested values from ids, applying the same scoping as the
+ # unforced query so ``include_ids`` cannot resolve rows the
+ # related-field filters deliberately hide.
+ query = db.session.query(datamodel.obj).filter(pk_col.in_(ids))
+ if base_filters :=
self.base_related_field_filters.get(column_name):
+ query = datamodel.apply_filters(
+ query,
datamodel.get_filters().add_filter_list(base_filters)
+ )
+ extra_rows = query.all()
result += self._get_result_from_rows(datamodel, extra_rows,
column_name)
@event_logger.log_this_with_context(
diff --git a/superset/views/core.py b/superset/views/core.py
index 37d13f0a1c2..980eec9ce33 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -626,14 +626,22 @@ class Superset(BaseSupersetView):
if action == "saveas":
if "slice_id" in form_data:
form_data.pop("slice_id") # don't save old slice_id
- from superset.subjects.utils import get_user_subject
+ from superset.subjects.utils import (
+ get_default_viewers_for_new_asset,
+ get_user_subject,
+ )
editors = []
if g.user:
subj = get_user_subject(g.user.id)
if subj:
editors.append(subj)
- slc = Slice(editors=editors)
+ slc = Slice(
+ editors=editors,
+ viewers=get_default_viewers_for_new_asset(
+ g.user.id if g.user else None
+ ),
+ )
utils.remove_extra_adhoc_filters(form_data)
@@ -685,7 +693,10 @@ class Superset(BaseSupersetView):
status=403,
)
- from superset.subjects.utils import get_user_subject
+ from superset.subjects.utils import (
+ get_default_viewers_for_new_asset,
+ get_user_subject,
+ )
editors = []
if g.user:
@@ -695,6 +706,9 @@ class Superset(BaseSupersetView):
dash = Dashboard(
dashboard_title=request.args.get("new_dashboard_name"),
editors=editors,
+ viewers=get_default_viewers_for_new_asset(
+ g.user.id if g.user else None
+ ),
)
if dash and slc not in dash.slices:
diff --git a/superset/views/dashboard/views.py
b/superset/views/dashboard/views.py
index b5e788c5dde..6bbe30da1ac 100644
--- a/superset/views/dashboard/views.py
+++ b/superset/views/dashboard/views.py
@@ -78,7 +78,10 @@ class Dashboard(BaseSupersetView):
@expose("/new/")
def new(self) -> FlaskResponse:
"""Creates a new, blank dashboard and redirects to it in edit mode"""
- from superset.subjects.utils import get_user_subject
+ from superset.subjects.utils import (
+ get_default_viewers_for_new_asset,
+ get_user_subject,
+ )
editors = []
if g.user:
@@ -88,6 +91,7 @@ class Dashboard(BaseSupersetView):
new_dashboard = DashboardModel(
dashboard_title="[ untitled dashboard ]",
editors=editors,
+ viewers=get_default_viewers_for_new_asset(g.user.id if g.user else
None),
)
db.session.add(new_dashboard)
db.session.commit() # pylint: disable=consider-using-transaction
diff --git a/superset/views/users/schemas.py b/superset/views/users/schemas.py
index f74d1cbca50..6493ba24177 100644
--- a/superset/views/users/schemas.py
+++ b/superset/views/users/schemas.py
@@ -27,6 +27,13 @@ last_name_description = "The current user's last name"
password_description = "The current user's password for authentication" #
noqa: S105
+class UserGroupSchema(Schema):
+ """A group the current user belongs to."""
+
+ id = Integer()
+ name = String()
+
+
class UserResponseSchema(Schema):
id = Integer()
username = String()
@@ -36,6 +43,7 @@ class UserResponseSchema(Schema):
is_active = Boolean()
is_anonymous = Boolean()
login_count = Integer()
+ groups = fields.List(fields.Nested(UserGroupSchema))
class CurrentUserPutSchema(Schema):
diff --git a/tests/integration_tests/subjects/api_tests.py
b/tests/integration_tests/subjects/api_tests.py
index b2e2a698ba2..7bc021707ae 100644
--- a/tests/integration_tests/subjects/api_tests.py
+++ b/tests/integration_tests/subjects/api_tests.py
@@ -24,6 +24,7 @@ from superset import db
from superset.subjects.models import Subject
from superset.subjects.types import SubjectType
from superset.utils import json
+from tests.conftest import with_config
from tests.integration_tests.base_tests import SupersetTestCase
from tests.integration_tests.constants import ADMIN_USERNAME, GAMMA_USERNAME
@@ -140,3 +141,24 @@ class TestSubjectApi(SupersetTestCase):
assert self.client.post("api/v1/security/subject/",
json={}).status_code == 405
assert self.client.put("api/v1/security/subject/1",
json={}).status_code == 405
assert self.client.delete("api/v1/security/subject/1").status_code ==
405
+
+ @pytest.mark.usefixtures("create_subjects")
+ @with_config({"EXCLUDE_USERS_FROM_LISTS": ["admin"]})
+ def test_get_list_subject_excludes_configured_users(self) -> None:
+ """Subject API: excluded users are absent from the list"""
+ self.login(ADMIN_USERNAME)
+
+ rv = self.get_assert_metric("api/v1/security/subject/", "get_list")
+
+ assert rv.status_code == 200
+ data = json.loads(rv.data.decode("utf-8"))
+ excluded = (
+ db.session.query(Subject)
+ .filter(Subject.type == SubjectType.USER)
+ .join(Subject.user)
+ .filter_by(username="admin")
+ .one_or_none()
+ )
+ assert excluded is not None, "expected an admin USER subject to exist"
+ assert excluded.id not in {row["id"] for row in data["result"]}
+ assert data["count"] == db.session.query(Subject).count() - 1
diff --git a/tests/integration_tests/users/api_tests.py
b/tests/integration_tests/users/api_tests.py
index 85985f9f35b..184a76a6660 100644
--- a/tests/integration_tests/users/api_tests.py
+++ b/tests/integration_tests/users/api_tests.py
@@ -42,6 +42,15 @@ class TestCurrentUserApi(SupersetTestCase):
assert True is response["result"]["is_active"]
assert False is response["result"]["is_anonymous"]
+ def test_get_me_includes_groups(self):
+ self.login(ADMIN_USERNAME)
+
+ rv = self.client.get(meUri)
+
+ assert 200 == rv.status_code
+ response = json.loads(rv.data.decode("utf-8"))
+ assert isinstance(response["result"]["groups"], list)
+
def test_get_me_with_roles(self):
self.login(ADMIN_USERNAME)
diff --git a/tests/unit_tests/commands/dataset/test_create_subjects.py
b/tests/unit_tests/commands/dataset/test_create_subjects.py
new file mode 100644
index 00000000000..777a2d4e307
--- /dev/null
+++ b/tests/unit_tests/commands/dataset/test_create_subjects.py
@@ -0,0 +1,71 @@
+# 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.
+"""Datasets must never be given viewers.
+
+``SqlaTable`` has ``sqlatable_editors`` but no viewers table, so a ``viewers``
+key in the create properties would be handed to ``setattr`` by the DAO and
+silently dropped instead of granting anything.
+"""
+
+from unittest.mock import Mock, patch
+
+from superset.commands.dataset.create import CreateDatasetCommand
+from superset.models.core import Database
+from superset.subjects.models import Subject
+from superset.subjects.types import SubjectType
+
+
+def _group_subject(id_: int) -> Subject:
+ subject = Subject()
+ subject.id = id_
+ subject.type = SubjectType.GROUP
+ return subject
+
+
+def test_dataset_create_never_populates_viewers(app_context) -> None:
+ database = Mock(spec=Database)
+ database.id = 1
+ database.get_default_catalog.return_value = None
+
+ command = CreateDatasetCommand(
+ {"database": 1, "table_name": "some_table", "sql": "SELECT 1"}
+ )
+
+ with (
+ patch(
+ "superset.commands.dataset.create.DatasetDAO.get_database_by_id",
+ return_value=database,
+ ),
+ patch(
+ "superset.commands.dataset.create.DatasetDAO.validate_uniqueness",
+ return_value=True,
+ ),
+
patch("superset.commands.dataset.create.security_manager.raise_for_access"),
+ patch("superset.commands.utils.populate_subject_list",
return_value=[]),
+ patch("superset.commands.utils.get_user_id", return_value=5),
+ patch(
+ "superset.subjects.utils.get_user_group_subjects",
+ return_value=[_group_subject(11)],
+ ),
+ patch(
+ "superset.subjects.utils._assigns_creator_groups_as_viewers",
+ return_value=True,
+ ),
+ ):
+ command.validate()
+
+ assert "viewers" not in command._properties # noqa: SLF001
diff --git
a/tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_generation.py
b/tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_generation.py
index b401665657a..9271d7d070f 100644
--- a/tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_generation.py
+++ b/tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_generation.py
@@ -2140,3 +2140,35 @@ class TestDashboardSerializationEagerLoading:
assert "editors" not in dash
assert dash["tags"] == []
assert dash["charts"] == []
+
+
+class TestGenerateDashboardCreatorGroups:
+ """The tool creates dashboards outside CreateDashboardCommand, so it has to
+ apply the creator's default viewers itself."""
+
+ @patch("superset.models.dashboard.Dashboard")
+ @patch("superset.daos.dashboard.DashboardDAO.find_by_id")
+ @patch("superset.db.session")
+ @pytest.mark.asyncio
+ async def test_generated_dashboard_gets_the_creators_default_viewers(
+ self, mock_db_session, mock_find_by_id, mock_dashboard_cls, mcp_server
+ ):
+ charts = [_mock_chart(id=1, slice_name="Sales Chart")]
+ mock_dashboard = _mock_dashboard(id=10, title="Analytics Dashboard")
+ mock_dashboard.viewers = []
+ _setup_generate_dashboard_mocks(
+ mock_db_session, mock_find_by_id, mock_dashboard_cls, charts,
mock_dashboard
+ )
+ viewer = Mock()
+
+ with patch(
+ "superset.subjects.utils.get_default_viewers_for_new_asset",
+ return_value=[viewer],
+ ):
+ async with Client(mcp_server) as client:
+ await client.call_tool(
+ "generate_dashboard",
+ {"request": {"chart_ids": [1], "dashboard_title":
"Analytics"}},
+ )
+
+ assert mock_dashboard.viewers == [viewer]
diff --git a/tests/unit_tests/scripts/change_detector_test.py
b/tests/unit_tests/scripts/change_detector_test.py
index af60b1eb4a4..18ca457008b 100644
--- a/tests/unit_tests/scripts/change_detector_test.py
+++ b/tests/unit_tests/scripts/change_detector_test.py
@@ -14,6 +14,7 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
+import io
from unittest import mock
from urllib.error import HTTPError, URLError
@@ -96,6 +97,68 @@ def test_fetch_gives_up_after_max_retries() -> None:
assert urlopen_mock.call_count == change_detector.MAX_RETRIES
+def _http_error(
+ status: int, headers: dict[str, str] | None = None, body: bytes = b""
+) -> HTTPError:
+ """Builds an HTTPError carrying headers and a readable body, as urllib
does."""
+ return HTTPError(
+ "http://api";,
+ status,
+ "Forbidden",
+ headers or {}, # type: ignore[arg-type]
+ io.BytesIO(body),
+ )
+
+
+def test_fetch_does_not_retry_a_permission_denied_403() -> None:
+ """A missing token scope is deterministic; retrying only delays the
failure."""
+ error = _http_error(
+ 403,
+ headers={"x-ratelimit-remaining": "4998"},
+ body=b'{"message": "Resource not accessible by integration"}',
+ )
+ with (
+ mock.patch.object(
+ change_detector, "urlopen", side_effect=error
+ ) as urlopen_mock,
+ mock.patch.object(change_detector.time, "sleep") as sleep_mock,
+ ):
+ with pytest.raises(HTTPError):
+ change_detector.fetch_files_github_api("http://api";)
+
+ assert urlopen_mock.call_count == 1
+ sleep_mock.assert_not_called()
+
+
+def test_fetch_retries_a_rate_limited_403() -> None:
+ """GitHub signals rate limiting with 403 and an exhausted remaining
count."""
+ side_effects: list[object] = [
+ _http_error(403, headers={"x-ratelimit-remaining": "0"}),
+ _make_response(b'[{"filename": "superset/foo.py"}]'),
+ ]
+ with (
+ mock.patch.object(change_detector, "urlopen",
side_effect=side_effects),
+ mock.patch.object(change_detector.time, "sleep"),
+ ):
+ result = change_detector.fetch_files_github_api("http://api";)
+
+ assert result == [{"filename": "superset/foo.py"}]
+
+
+def test_fetch_reports_the_api_error_message(capsys) -> None:
+ """The response body names the cause; without it a 403 is unactionable."""
+ error = _http_error(
+ 403,
+ headers={"x-ratelimit-remaining": "4998"},
+ body=b'{"message": "Resource not accessible by integration"}',
+ )
+ with mock.patch.object(change_detector, "urlopen", side_effect=error):
+ with pytest.raises(HTTPError):
+ change_detector.fetch_files_github_api("http://api";)
+
+ assert "Resource not accessible by integration" in capsys.readouterr().out
+
+
def test_image_tag_compose_changes_trigger_python_tests() -> None:
assert change_detector.detect_changes(
["docker-compose-image-tag.yml"],
diff --git a/tests/unit_tests/security/test_audit_event_metrics.py
b/tests/unit_tests/security/test_audit_event_metrics.py
new file mode 100644
index 00000000000..858d57f99f8
--- /dev/null
+++ b/tests/unit_tests/security/test_audit_event_metrics.py
@@ -0,0 +1,55 @@
+# 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.
+"""Tests that role/group audit events are also observable as StatsD metrics.
+
+The group and role write endpoints record audit events through
+``_log_audit_event``. Operators need the same activity as counters, so that
+group management is visible on dashboards without parsing the event log.
+"""
+
+from unittest.mock import MagicMock
+
+from pytest_mock import MockerFixture
+
+from superset.security.manager import _log_audit_event
+
+
+def test_audit_event_increments_a_statsd_counter(
+ mocker: MockerFixture,
+ app_context,
+) -> None:
+ stats = MagicMock()
+ mocker.patch("superset.extensions.stats_logger_manager", stats)
+ mocker.patch("superset.extensions.event_logger")
+
+ _log_audit_event("GroupCreated", {"group_id": 1, "group_name": "tenant-a"})
+
+ stats.instance.incr.assert_called_once_with("security.GroupCreated")
+
+
+def test_audit_event_is_still_logged_when_statsd_fails(
+ mocker: MockerFixture,
+ app_context,
+) -> None:
+ stats = MagicMock()
+ stats.instance.incr.side_effect = RuntimeError("statsd down")
+ mocker.patch("superset.extensions.stats_logger_manager", stats)
+ event_logger = mocker.patch("superset.extensions.event_logger")
+
+ _log_audit_event("GroupDeleted", {"group_id": 1, "group_name": "tenant-a"})
+
+ event_logger.log.assert_called_once()
diff --git a/tests/unit_tests/subjects/test_api_filters.py
b/tests/unit_tests/subjects/test_api_filters.py
new file mode 100644
index 00000000000..c30d10ecaf6
--- /dev/null
+++ b/tests/unit_tests/subjects/test_api_filters.py
@@ -0,0 +1,84 @@
+# 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.
+"""Tests that ``SubjectRestApi`` scopes its own list/show queries.
+
+``SubjectRestApi`` exposes the principal directory (including ``user.email``),
+so the ``EXCLUDE_USERS_FROM_LISTS`` and ``EXTRA_RELATED_QUERY_FILTERS``
+deployment settings must apply to it, exactly as they do to the related-item
+endpoints that feed the editor/viewer pickers.
+"""
+
+from unittest.mock import MagicMock
+
+from flask_appbuilder.models.sqla.interface import SQLAInterface
+from pytest_mock import MockerFixture
+from sqlalchemy import create_engine
+from sqlalchemy.orm import sessionmaker
+
+from superset.subjects.filters import SubjectListFilter
+from superset.subjects.models import Subject
+
+
+def _compiled(query) -> str:
+ engine = create_engine("sqlite://")
+ return str(
+ query.statement.compile(engine, compile_kwargs={"literal_binds":
True}),
+ )
+
+
+def _subject_query():
+ session = sessionmaker(bind=create_engine("sqlite://"))()
+ return session.query(Subject)
+
+
+def test_subject_list_filter_excludes_configured_users(
+ mocker: MockerFixture,
+) -> None:
+ mock_current_app = MagicMock()
+ mock_current_app.config = {
+ "EXCLUDE_USERS_FROM_LISTS": ["service_account"],
+ "EXTRA_RELATED_QUERY_FILTERS": {},
+ }
+ mocker.patch("superset.subjects.filters.current_app", mock_current_app)
+
+ filter_ = SubjectListFilter("id", SQLAInterface(Subject))
+ filtered = filter_.apply(_subject_query(), None)
+
+ # Assert on the NOT IN predicate specifically: a plain "service_account"
+ # membership check would pass for an (incorrect) IN filter just as well.
+ assert "NOT IN ('service_account')" in _compiled(filtered)
+
+
+def test_subject_list_filter_is_a_noop_without_configuration(
+ mocker: MockerFixture,
+) -> None:
+ mock_current_app = MagicMock()
+ mock_current_app.config = {}
+ mocker.patch("superset.subjects.filters.current_app", mock_current_app)
+
+ query = _subject_query()
+ filter_ = SubjectListFilter("id", SQLAInterface(Subject))
+
+ assert filter_.apply(query, None) is query
+
+
+def test_subject_rest_api_registers_the_scoping_base_filter() -> None:
+ from superset.subjects.api import SubjectRestApi
+
+ registered = [spec[1] for spec in SubjectRestApi.base_filters]
+
+ assert SubjectListFilter in registered
diff --git a/tests/unit_tests/subjects/test_creator_group_call_sites.py
b/tests/unit_tests/subjects/test_creator_group_call_sites.py
new file mode 100644
index 00000000000..2d2f6b13080
--- /dev/null
+++ b/tests/unit_tests/subjects/test_creator_group_call_sites.py
@@ -0,0 +1,508 @@
+# 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.
+"""Tests that asset-creation paths attach the creator's default viewers.
+
+``get_default_viewers_for_new_asset`` is covered on its own elsewhere; these
+tests pin the wiring at the call sites that build an asset outside the create
+commands, so removing one fails the suite. Covered here: both v1 importers,
+the template copy, ``/dashboard/new/``, the dashboard-copy DAO (including its
+cloned charts), the legacy explore save paths, and ``reset_ownership`` (used
+by the v0 importers). The MCP ``generate_dashboard`` tool is covered by
+``tests/unit_tests/mcp_service/dashboard/tool/test_dashboard_generation.py``.
+"""
+
+from types import SimpleNamespace
+from typing import Any
+from unittest.mock import MagicMock, patch
+
+import pytest
+
+from superset.subjects.models import Subject
+from superset.subjects.types import SubjectType
+
+
+def _group_subject(id_: int = 11) -> Subject:
+ subject = Subject()
+ subject.id = id_
+ subject.type = SubjectType.GROUP
+ return subject
+
+
+def _user(user_id: int = 5) -> SimpleNamespace:
+ return SimpleNamespace(id=user_id)
+
+
+def test_chart_importer_attaches_default_viewers(app_context) -> None:
+ from superset.commands.chart.importers.v1 import utils as chart_utils
+
+ viewer = _group_subject()
+ chart = SimpleNamespace(id=1, editors=[], viewers=[])
+
+ with (
+ patch.object(chart_utils, "find_existing_for_import",
return_value=None),
+ patch.object(chart_utils, "get_user", return_value=_user()),
+ patch.object(chart_utils, "filter_chart_annotations"),
+ patch.object(chart_utils, "migrate_chart", side_effect=lambda config:
config),
+ patch.object(chart_utils.Slice, "import_from_dict",
return_value=chart),
+ patch("superset.subjects.utils.get_user_subject", return_value=None),
+ patch(
+ "superset.subjects.utils.get_default_viewers_for_new_asset",
+ return_value=[viewer],
+ ),
+ ):
+ result = chart_utils.import_chart(
+ {"uuid": "x", "params": {}}, ignore_permissions=True
+ )
+
+ assert result.viewers == [viewer]
+
+
+def test_chart_importer_adds_missing_viewers_without_duplicating_present_ones(
+ app_context,
+) -> None:
+ from superset.commands.chart.importers.v1 import utils as chart_utils
+
+ already_there = _group_subject(11)
+ newly_added = _group_subject(12)
+ chart = SimpleNamespace(id=1, editors=[], viewers=[already_there])
+
+ with (
+ patch.object(chart_utils, "find_existing_for_import",
return_value=None),
+ patch.object(chart_utils, "get_user", return_value=_user()),
+ patch.object(chart_utils, "filter_chart_annotations"),
+ patch.object(chart_utils, "migrate_chart", side_effect=lambda config:
config),
+ patch.object(chart_utils.Slice, "import_from_dict",
return_value=chart),
+ patch("superset.subjects.utils.get_user_subject", return_value=None),
+ patch(
+ "superset.subjects.utils.get_default_viewers_for_new_asset",
+ return_value=[already_there, newly_added],
+ ),
+ ):
+ result = chart_utils.import_chart(
+ {"uuid": "x", "params": {}}, ignore_permissions=True
+ )
+
+ assert result.viewers == [already_there, newly_added]
+
+
+def test_chart_importer_skips_default_viewers_on_reimport(app_context) -> None:
+ """Re-importing over an existing chart must not grant the importer's
groups.
+
+ Unlike a fresh import, an overwrite / soft-delete restore falls through to
+ an in-place update; applying "new asset" defaults there would silently
+ widen an existing chart's read access.
+ """
+ from superset.commands.chart.importers.v1 import utils as chart_utils
+
+ chart = SimpleNamespace(id=1, editors=[], viewers=[])
+
+ with (
+ # A matching row that falls through to an in-place update (return
None).
+ patch.object(chart_utils, "find_existing_for_import",
return_value=MagicMock()),
+ patch.object(
+ chart_utils, "_prepare_existing_chart_for_import",
return_value=None
+ ),
+ patch.object(chart_utils, "get_user", return_value=_user()),
+ patch.object(chart_utils, "filter_chart_annotations"),
+ patch.object(chart_utils, "migrate_chart", side_effect=lambda config:
config),
+ patch.object(chart_utils.Slice, "import_from_dict",
return_value=chart),
+ patch("superset.subjects.utils.get_user_subject", return_value=None),
+ patch(
+ "superset.subjects.utils.get_default_viewers_for_new_asset",
+ return_value=[_group_subject()],
+ ) as mock_viewers,
+ ):
+ result = chart_utils.import_chart(
+ {"uuid": "x", "params": {}}, overwrite=True,
ignore_permissions=True
+ )
+
+ assert result.viewers == []
+ mock_viewers.assert_not_called()
+
+
+def test_chart_importer_uses_passed_default_viewers_without_recomputing(
+ app_context,
+) -> None:
+ """Bulk importers resolve the creator's viewers once and pass them down;
+ a per-asset call must reuse that list rather than re-query membership."""
+ from superset.commands.chart.importers.v1 import utils as chart_utils
+
+ passed = _group_subject(7)
+ chart = SimpleNamespace(id=1, editors=[], viewers=[])
+
+ with (
+ patch.object(chart_utils, "find_existing_for_import",
return_value=None),
+ patch.object(chart_utils, "get_user", return_value=_user()),
+ patch.object(chart_utils, "filter_chart_annotations"),
+ patch.object(chart_utils, "migrate_chart", side_effect=lambda config:
config),
+ patch.object(chart_utils.Slice, "import_from_dict",
return_value=chart),
+ patch("superset.subjects.utils.get_user_subject", return_value=None),
+ patch(
+ "superset.subjects.utils.get_default_viewers_for_new_asset",
+ ) as mock_compute,
+ ):
+ result = chart_utils.import_chart(
+ {"uuid": "x", "params": {}},
+ ignore_permissions=True,
+ default_viewers=[passed],
+ )
+
+ assert result.viewers == [passed]
+ mock_compute.assert_not_called()
+
+
+def test_copy_dashboard_attaches_viewers_from_the_users_in_memory_groups(
+ app_context,
+) -> None:
+ """The template copy runs in ``after_insert``, before ``ab_user_group``
exists."""
+ from superset.models import dashboard as dashboard_module
+
+ viewer = _group_subject()
+ group = MagicMock()
+ new_user = MagicMock()
+ new_user.id = 5
+ new_user.groups = [group]
+ session = MagicMock()
+ # Both lookups (the user, then the template dashboard) go through the same
+ # mocked chain; a MagicMock stands in for either.
+ session.query.return_value.filter_by.return_value.first.return_value =
new_user
+
+ created: dict[str, Any] = {}
+
+ def _capture(**kwargs):
+ created.update(kwargs)
+ return MagicMock()
+
+ with (
+ patch.dict(dashboard_module.app.config, {"DASHBOARD_TEMPLATE_ID": 1}),
+ patch.object(
+ dashboard_module.sqla, "inspect",
return_value=MagicMock(session=session)
+ ),
+ patch.object(dashboard_module, "Dashboard", side_effect=_capture),
+ patch("superset.subjects.utils.get_user_subject", return_value=None),
+ patch(
+ "superset.subjects.utils.get_default_viewers_for_groups",
+ return_value=[viewer],
+ ) as mock_for_groups,
+ ):
+ dashboard_module.copy_dashboard(
+ MagicMock(),
+ MagicMock(),
+ SimpleNamespace(id=5), # type: ignore[arg-type]
+ )
+
+ assert created["viewers"] == [viewer]
+ # Resolved from the in-memory collection, never by querying membership.
+ mock_for_groups.assert_called_once_with([group])
+
+
+def test_dashboard_importer_attaches_default_viewers(app_context) -> None:
+ from superset.commands.dashboard.importers.v1 import utils as
dashboard_utils
+
+ viewer = _group_subject()
+ dashboard = SimpleNamespace(id=1, editors=[], viewers=[])
+
+ with (
+ patch.object(dashboard_utils, "find_existing_for_import",
return_value=None),
+ patch.object(dashboard_utils, "get_user", return_value=_user()),
+ patch.object(
+ dashboard_utils.Dashboard, "import_from_dict",
return_value=dashboard
+ ),
+ patch("superset.subjects.utils.get_user_subject", return_value=None),
+ patch(
+ "superset.subjects.utils.get_default_viewers_for_new_asset",
+ return_value=[viewer],
+ ),
+ ):
+ result = dashboard_utils.import_dashboard(
+ {"uuid": "x", "metadata": {}}, ignore_permissions=True
+ )
+
+ assert result.viewers == [viewer]
+
+
+def test_new_dashboard_view_attaches_default_viewers(app_context) -> None:
+ from superset.views.dashboard import views as dashboard_views
+
+ viewer = _group_subject()
+ created: dict[str, Any] = {}
+
+ def _capture(**kwargs):
+ created.update(kwargs)
+ return MagicMock(id=1)
+
+ # Strip ``@has_access``/``@expose`` so the handler body runs directly.
+ handler = dashboard_views.Dashboard.new
+ while hasattr(handler, "__wrapped__"):
+ handler = handler.__wrapped__
+
+ with (
+ patch.object(dashboard_views, "DashboardModel", side_effect=_capture),
+ patch.object(dashboard_views, "db"),
+ patch.object(dashboard_views, "redirect"),
+ patch.object(dashboard_views, "url_for"),
+ patch.object(dashboard_views, "g", MagicMock(user=_user())),
+ patch("superset.subjects.utils.get_user_subject", return_value=None),
+ patch(
+ "superset.subjects.utils.get_default_viewers_for_new_asset",
+ return_value=[viewer],
+ ),
+ ):
+ handler(MagicMock())
+
+ assert created["viewers"] == [viewer]
+
+
+def test_dashboard_copy_dao_attaches_default_viewers(app_context) -> None:
+ from superset.daos import dashboard as dashboard_dao
+
+ viewer = _group_subject()
+ original = MagicMock()
+ original.slices = []
+ created: list[Any] = []
+
+ with (
+ patch.object(dashboard_dao.security_manager, "is_editor",
return_value=True),
+ patch.object(dashboard_dao, "g", MagicMock(user=_user())),
+ patch.object(dashboard_dao, "db"),
+ patch.object(dashboard_dao.DashboardDAO, "set_dash_metadata"),
+ patch.object(
+ dashboard_dao,
+ "Dashboard",
+ side_effect=lambda: created.append(MagicMock()) # type:
ignore[func-returns-value]
+ or created[-1],
+ ),
+ patch("superset.subjects.utils.get_user_subject", return_value=None),
+ patch(
+ "superset.subjects.utils.get_default_viewers_for_new_asset",
+ return_value=[viewer],
+ ),
+ ):
+ dashboard_dao.DashboardDAO.copy_dashboard(
+ original, {"dashboard_title": "copy", "json_metadata": "{}"}
+ )
+
+ assert created[-1].viewers == [viewer]
+
+
+def test_dashboard_copy_dao_attaches_default_viewers_to_cloned_charts(
+ app_context,
+) -> None:
+ from superset.daos import dashboard as dashboard_dao
+
+ viewer = _group_subject()
+ clone = MagicMock()
+ source_slice = MagicMock()
+ source_slice.clone.return_value = clone
+ original = MagicMock()
+ original.slices = [source_slice]
+
+ with (
+ patch.object(dashboard_dao.security_manager, "is_editor",
return_value=True),
+ patch.object(dashboard_dao, "g", MagicMock(user=_user())),
+ patch.object(dashboard_dao, "db"),
+ patch.object(dashboard_dao.DashboardDAO, "set_dash_metadata"),
+ patch.object(dashboard_dao, "Dashboard", return_value=MagicMock()),
+ patch("superset.subjects.utils.get_user_subject", return_value=None),
+ patch(
+ "superset.subjects.utils.get_default_viewers_for_new_asset",
+ return_value=[viewer],
+ ),
+ ):
+ dashboard_dao.DashboardDAO.copy_dashboard(
+ original,
+ {
+ "dashboard_title": "copy",
+ "json_metadata": '{"positions": {}}',
+ "duplicate_slices": True,
+ },
+ )
+
+ assert clone.viewers == [viewer]
+
+
+def test_reset_ownership_attaches_default_viewers(app_context) -> None:
+ """Used by the v0 importers, which never went through the v1 wiring."""
+ from superset.models.helpers import ImportExportMixin
+
+ viewer = _group_subject()
+
+ class _Asset(ImportExportMixin):
+ def __init__(self) -> None:
+ self.editors: list[Any] = []
+ self.viewers: list[Any] = []
+
+ asset = _Asset()
+
+ with (
+ patch("superset.models.helpers.g", MagicMock(user=_user())),
+ patch("superset.subjects.utils.get_user_subject", return_value=None),
+ patch(
+ "superset.subjects.utils.get_default_viewers_for_new_asset",
+ return_value=[viewer],
+ ),
+ ):
+ asset.reset_ownership()
+
+ assert asset.viewers == [viewer]
+
+
+def test_reset_ownership_preserves_explicitly_set_viewers(app_context) -> None:
+ """A viewer list already on the instance is not replaced by group
defaults."""
+ from superset.models.helpers import ImportExportMixin
+
+ explicit = _group_subject(99)
+
+ class _Asset(ImportExportMixin):
+ def __init__(self) -> None:
+ self.editors: list[Any] = []
+ self.viewers: list[Any] = [explicit]
+
+ asset = _Asset()
+
+ with (
+ patch("superset.models.helpers.g", MagicMock(user=_user())),
+ patch("superset.subjects.utils.get_user_subject", return_value=None),
+ patch(
+ "superset.subjects.utils.get_default_viewers_for_new_asset",
+ return_value=[_group_subject(11)],
+ ) as mock_viewers,
+ ):
+ asset.reset_ownership()
+
+ assert asset.viewers == [explicit]
+ mock_viewers.assert_not_called()
+
+
+class _ConstructedError(Exception):
+ """Raised once the asset under test has been constructed."""
+
+
+def _capturing(store: dict[str, Any]):
+ """Record constructor kwargs, then abort the surrounding request handler.
+
+ ``save_or_overwrite_slice`` does a lot of work after building the asset;
+ stopping at construction keeps these tests focused on the wiring.
+ """
+
+ def _factory(**kwargs):
+ store.update(kwargs)
+ raise _ConstructedError
+
+ return _factory
+
+
+def test_legacy_explore_saveas_attaches_default_viewers(app_context) -> None:
+ """The ``@deprecated`` explore save route is still mounted and
reachable."""
+ from superset.views import core as core_views
+
+ viewer = _group_subject()
+ created: dict[str, Any] = {}
+ request = MagicMock()
+ request.args.get.side_effect = lambda key, *a: {
+ "slice_name": "n",
+ "action": "saveas",
+ }.get(key)
+
+ with (
+ patch.object(core_views, "request", request),
+ patch.object(
+ core_views, "get_form_data", return_value=({"viz_type": "table"},
None)
+ ),
+ patch.object(core_views, "Slice", side_effect=_capturing(created)),
+ patch.object(core_views, "g", MagicMock(user=_user())),
+ patch("superset.subjects.utils.get_user_subject", return_value=None),
+ patch(
+ "superset.subjects.utils.get_default_viewers_for_new_asset",
+ return_value=[viewer],
+ ),
+ pytest.raises(_ConstructedError),
+ ):
+ core_views.Superset.save_or_overwrite_slice(
+ None, True, False, False, 1, "table", "t"
+ )
+
+ assert created["viewers"] == [viewer]
+
+
+def test_legacy_explore_new_dashboard_attaches_default_viewers(app_context) ->
None:
+ """Saving a chart to a brand new dashboard from legacy explore."""
+ from superset.views import core as core_views
+
+ viewer = _group_subject()
+ created: dict[str, Any] = {}
+ request = MagicMock()
+ request.args.get.side_effect = lambda key, *a: {
+ "slice_name": "n",
+ "new_dashboard_name": "d",
+ }.get(key)
+
+ with (
+ patch.object(core_views, "request", request),
+ patch.object(
+ core_views, "get_form_data", return_value=({"viz_type": "table"},
None)
+ ),
+ patch.object(core_views, "Dashboard", side_effect=_capturing(created)),
+ patch.object(core_views, "g", MagicMock(user=_user())),
+ patch.object(core_views.security_manager, "can_access",
return_value=True),
+ patch.object(core_views.utils, "remove_extra_adhoc_filters"),
+ patch("superset.subjects.utils.get_user_subject", return_value=None),
+ patch(
+ "superset.subjects.utils.get_default_viewers_for_new_asset",
+ return_value=[viewer],
+ ),
+ pytest.raises(_ConstructedError),
+ ):
+ core_views.Superset.save_or_overwrite_slice(
+ MagicMock(), False, False, False, 1, "table", "t"
+ )
+
+ assert created["viewers"] == [viewer]
+
+
+def test_dashboard_copy_dao_resolves_creator_subjects_once(app_context) ->
None:
+ """Both lookups are loop-invariant: N cloned charts must not mean N
queries."""
+ from superset.daos import dashboard as dashboard_dao
+
+ original = MagicMock()
+ original.slices = [MagicMock(), MagicMock(), MagicMock()]
+
+ with (
+ patch.object(dashboard_dao.security_manager, "is_editor",
return_value=True),
+ patch.object(dashboard_dao, "g", MagicMock(user=_user())),
+ patch.object(dashboard_dao, "db"),
+ patch.object(dashboard_dao.DashboardDAO, "set_dash_metadata"),
+ patch.object(dashboard_dao, "Dashboard", return_value=MagicMock()),
+ patch(
+ "superset.subjects.utils.get_user_subject", return_value=None
+ ) as mock_user_subject,
+ patch(
+ "superset.subjects.utils.get_default_viewers_for_new_asset",
+ return_value=[_group_subject()],
+ ) as mock_viewers,
+ ):
+ dashboard_dao.DashboardDAO.copy_dashboard(
+ original,
+ {
+ "dashboard_title": "copy",
+ "json_metadata": '{"positions": {}}',
+ "duplicate_slices": True,
+ },
+ )
+
+ assert mock_user_subject.call_count == 1
+ assert mock_viewers.call_count == 1
diff --git a/tests/unit_tests/subjects/test_creator_group_defaults.py
b/tests/unit_tests/subjects/test_creator_group_defaults.py
new file mode 100644
index 00000000000..a2951a473b6
--- /dev/null
+++ b/tests/unit_tests/subjects/test_creator_group_defaults.py
@@ -0,0 +1,293 @@
+# 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.
+"""Tests for defaulting a new asset's viewers to its creator's groups.
+
+Assigning the creator's groups as *viewers* (rather than editors) shares the
+asset read-only, and is gated behind ``ASSIGN_CREATOR_GROUPS_AS_VIEWERS``
+because populating ``viewers`` changes how access is enforced: once an asset
+has any viewer, the datasource-permission fallback no longer applies to it.
+"""
+
+from typing import Any
+from unittest.mock import MagicMock, patch
+
+from superset.commands.utils import populate_subjects
+from superset.subjects.models import Subject
+from superset.subjects.types import SubjectType
+from superset.subjects.utils import (
+ _assigns_creator_groups_as_viewers,
+ get_default_viewers_for_groups,
+ get_default_viewers_for_new_asset,
+ get_user_group_subject_ids_subquery,
+)
+
+
+def _group_subject(id_: int) -> Subject:
+ subject = Subject()
+ subject.id = id_
+ subject.type = SubjectType.GROUP
+ return subject
+
+
+def test_gate_resolves_the_assign_creator_groups_feature_flag(app_context) ->
None:
+ """The gate is a feature flag read via ``is_feature_enabled`` and
additionally
+ requires ``ENABLE_VIEWERS``."""
+ with patch("superset.is_feature_enabled", return_value=True) as mock_flag:
+ assert _assigns_creator_groups_as_viewers() is True
+ mock_flag.assert_any_call("ENABLE_VIEWERS")
+ mock_flag.assert_any_call("ASSIGN_CREATOR_GROUPS_AS_VIEWERS")
+
+ with patch("superset.is_feature_enabled", return_value=False):
+ assert _assigns_creator_groups_as_viewers() is False
+
+ # ASSIGN on but ENABLE_VIEWERS off → the gate stays closed.
+ with patch(
+ "superset.is_feature_enabled",
+ side_effect=lambda flag: flag != "ENABLE_VIEWERS",
+ ):
+ assert _assigns_creator_groups_as_viewers() is False
+
+
+def test_user_group_subject_ids_subquery_joins_group_membership(app_context)
-> None:
+ sql = str(
+ get_user_group_subject_ids_subquery(7).compile(
+ compile_kwargs={"literal_binds": True}
+ )
+ )
+
+ assert "ab_user_group" in sql
+ assert "user_id = 7" in sql
+
+
+@patch("superset.commands.utils.get_user_id", return_value=5)
+@patch("superset.commands.utils.populate_subject_list")
+@patch("superset.subjects.utils.get_user_group_subjects")
+def test_new_asset_viewers_default_to_creator_groups_when_enabled(
+ mock_groups: MagicMock,
+ mock_populate: MagicMock,
+ mock_user_id: MagicMock,
+ app_context,
+) -> None:
+ groups = [_group_subject(11), _group_subject(12)]
+ mock_groups.return_value = groups
+ mock_populate.return_value = [_group_subject(1)]
+ properties: dict[str, Any] = {}
+
+ with patch(
+ "superset.subjects.utils._assigns_creator_groups_as_viewers",
+ return_value=True,
+ ):
+ populate_subjects(properties, [])
+
+ assert properties["viewers"] == groups
+
+
+@patch("superset.commands.utils.get_user_id", return_value=5)
+@patch("superset.commands.utils.populate_subject_list")
+@patch("superset.subjects.utils.get_user_group_subjects")
+def test_new_asset_gets_no_default_viewers_when_disabled(
+ mock_groups: MagicMock,
+ mock_populate: MagicMock,
+ mock_user_id: MagicMock,
+ app_context,
+) -> None:
+ mock_groups.return_value = [_group_subject(11)]
+ mock_populate.return_value = [_group_subject(1)]
+ properties: dict[str, Any] = {}
+
+ with patch(
+ "superset.subjects.utils._assigns_creator_groups_as_viewers",
+ return_value=False,
+ ):
+ populate_subjects(properties, [])
+
+ assert "viewers" not in properties
+
+
+@patch("superset.subjects.utils.get_user_group_subjects")
+def test_default_viewers_are_empty_for_an_anonymous_creator(
+ mock_groups: MagicMock,
+ app_context,
+) -> None:
+ with patch(
+ "superset.subjects.utils._assigns_creator_groups_as_viewers",
+ return_value=True,
+ ):
+ assert get_default_viewers_for_new_asset(None) == []
+
+ mock_groups.assert_not_called()
+
+
+@patch("superset.subjects.utils.get_user_group_subjects")
+def test_default_viewers_are_empty_when_the_setting_is_off(
+ mock_groups: MagicMock,
+ app_context,
+) -> None:
+ with patch(
+ "superset.subjects.utils._assigns_creator_groups_as_viewers",
+ return_value=False,
+ ):
+ assert get_default_viewers_for_new_asset(5) == []
+
+ mock_groups.assert_not_called()
+
+
+@patch("superset.subjects.utils.get_user_group_subjects")
+def test_default_viewers_are_the_creators_groups_when_enabled(
+ mock_groups: MagicMock,
+ app_context,
+) -> None:
+ groups = [_group_subject(11)]
+ mock_groups.return_value = groups
+
+ with patch(
+ "superset.subjects.utils._assigns_creator_groups_as_viewers",
+ return_value=True,
+ ):
+ assert get_default_viewers_for_new_asset(5) == groups
+
+
+@patch("superset.commands.utils.get_user_id", return_value=5)
+@patch("superset.commands.utils.populate_subject_list")
+@patch("superset.subjects.utils.get_user_group_subjects")
+def test_explicit_viewers_are_not_replaced_by_creator_groups(
+ mock_groups: MagicMock,
+ mock_populate: MagicMock,
+ mock_user_id: MagicMock,
+ app_context,
+) -> None:
+ chosen = [_group_subject(99)]
+ mock_groups.return_value = [_group_subject(11)]
+ mock_populate.side_effect = [[_group_subject(1)], chosen]
+ properties: dict[str, Any] = {"viewers": [99]}
+
+ with patch(
+ "superset.subjects.utils._assigns_creator_groups_as_viewers",
+ return_value=True,
+ ):
+ populate_subjects(properties, [])
+
+ assert properties["viewers"] == chosen
+ mock_groups.assert_not_called()
+
+
+@patch("superset.subjects.utils.subjects_from_groups")
+def test_default_viewers_for_groups_resolves_in_memory_groups(
+ mock_from_groups: MagicMock,
+ app_context,
+) -> None:
+ """Flush-time callers hold groups in memory before ab_user_group is
written."""
+ group = MagicMock()
+ group.id = 11
+ subject = _group_subject(11)
+ subject.group_id = 11
+ mock_from_groups.return_value = [subject]
+
+ with patch(
+ "superset.subjects.utils._assigns_creator_groups_as_viewers",
+ return_value=True,
+ ):
+ assert get_default_viewers_for_groups([group]) == [subject]
+
+
+@patch("superset.subjects.utils.subjects_from_groups")
+def test_default_viewers_for_groups_is_empty_when_the_setting_is_off(
+ mock_from_groups: MagicMock,
+ app_context,
+) -> None:
+ with patch(
+ "superset.subjects.utils._assigns_creator_groups_as_viewers",
+ return_value=False,
+ ):
+ assert get_default_viewers_for_groups([MagicMock()]) == []
+
+ mock_from_groups.assert_not_called()
+
+
+@patch("superset.subjects.utils.get_user_group_subjects")
+def test_user_id_zero_is_treated_as_a_real_principal(
+ mock_groups: MagicMock,
+ app_context,
+) -> None:
+ """A falsy-but-valid id must not be conflated with 'no user'."""
+ groups = [_group_subject(11)]
+ mock_groups.return_value = groups
+
+ with patch(
+ "superset.subjects.utils._assigns_creator_groups_as_viewers",
+ return_value=True,
+ ):
+ assert get_default_viewers_for_new_asset(0) == groups
+
+
+def test_user_group_subject_ids_subquery_restricts_to_group_subjects(
+ app_context,
+) -> None:
+ """Defense in depth: don't rely on ``group_id`` alone to imply the type."""
+ sql = str(
+ get_user_group_subject_ids_subquery(7).compile(
+ compile_kwargs={"literal_binds": True}
+ )
+ )
+
+ assert "subjects.type = 3" in sql
+
+
+def test_subjects_from_groups_issues_a_single_query(app_context) -> None:
+ """One query for the whole list, not one per group."""
+ from superset.subjects import utils as subjects_utils
+
+ with patch.object(subjects_utils, "db") as mock_db:
+
mock_db.session.query.return_value.filter.return_value.all.return_value = []
+ subjects_utils.subjects_from_groups([MagicMock(id=1), MagicMock(id=2)])
+
+ assert mock_db.session.query.call_count == 1
+
+
+@patch("superset.commands.utils.get_user_id", return_value=5)
+@patch("superset.commands.utils.populate_subject_list", return_value=[])
+@patch("superset.subjects.utils.get_user_group_subjects")
+def test_an_explicit_empty_viewers_list_suppresses_the_group_default(
+ mock_groups: MagicMock,
+ mock_populate: MagicMock,
+ mock_user_id: MagicMock,
+ app_context,
+) -> None:
+ """``viewers: []`` means "no viewers", not "fall back to my groups"."""
+ properties: dict[str, Any] = {"viewers": []}
+
+ with patch(
+ "superset.subjects.utils._assigns_creator_groups_as_viewers",
+ return_value=True,
+ ):
+ populate_subjects(properties, [])
+
+ assert properties["viewers"] == []
+ mock_groups.assert_not_called()
+
+
+@patch("superset.subjects.utils.get_user_group_subjects", return_value=[])
+def test_a_creator_without_groups_leaves_the_dataset_fallback_intact(
+ mock_groups: MagicMock,
+ app_context,
+) -> None:
+ """No groups means no viewers, so the asset keeps dataset-based access."""
+ with patch(
+ "superset.subjects.utils._assigns_creator_groups_as_viewers",
+ return_value=True,
+ ):
+ assert get_default_viewers_for_new_asset(5) == []
diff --git a/tests/unit_tests/subjects/test_utils.py
b/tests/unit_tests/subjects/test_utils.py
index 285b87707cf..e5ebf4323e5 100644
--- a/tests/unit_tests/subjects/test_utils.py
+++ b/tests/unit_tests/subjects/test_utils.py
@@ -30,7 +30,10 @@ from superset.subjects.models import Subject
from superset.subjects.types import SubjectType
from superset.subjects.utils import (
get_current_user_subject_ids,
+ get_default_viewers_for_groups,
+ get_or_create_group_subject,
get_or_create_role_subject,
+ get_user_group_subjects,
get_user_subject_ids_subquery,
)
@@ -143,6 +146,193 @@ def
test_get_or_create_role_subject_missing_role_returns_none() -> None:
mock_sync.assert_not_called()
+# --------------------------------------------------------------------------
+# get_or_create_group_subject / get_user_group_subjects backfill
+# --------------------------------------------------------------------------
+
+
+@patch("superset.subjects.utils.get_group_subject")
+def test_get_or_create_group_subject_returns_existing(
+ mock_get_group_subject: MagicMock,
+) -> None:
+ """An already-synced group resolves to its Subject with no sync."""
+ existing = _make_subject(10, SubjectType.GROUP)
+ mock_get_group_subject.return_value = existing
+
+ assert get_or_create_group_subject(5) is existing
+ mock_get_group_subject.assert_called_once_with(5)
+
+
+def test_get_or_create_group_subject_syncs_unsynced_group() -> None:
+ """A group that exists but has no Subject row yet is synced on demand
+ rather than treated as nonexistent."""
+ created = _make_subject(11, SubjectType.GROUP)
+ group = MagicMock()
+ group.id = 5
+
+ with (
+ patch(
+ "superset.subjects.utils.get_group_subject",
+ side_effect=[None, created],
+ ) as mock_get_group_subject,
+ patch("superset.subjects.utils.db") as mock_db,
+ patch("superset.subjects.sync.sync_group_subject") as mock_sync,
+ ):
+ mock_db.session.get.return_value = group
+
+ assert get_or_create_group_subject(5) is created
+
+ mock_sync.assert_called_once_with(group)
+ # The insert is wrapped in a SAVEPOINT so a concurrent conflict can't
+ # poison the surrounding transaction.
+ mock_db.session.begin_nested.assert_called_once()
+ assert mock_get_group_subject.call_count == 2
+
+
+def test_get_or_create_group_subject_handles_concurrent_insert() -> None:
+ """A concurrent backfill (IntegrityError in the savepoint) is swallowed and
+ the row the other request wrote is reloaded, keeping the transaction
usable."""
+ from sqlalchemy.exc import IntegrityError
+
+ winner = _make_subject(12, SubjectType.GROUP)
+ group = MagicMock()
+ group.id = 7
+
+ with (
+ patch(
+ "superset.subjects.utils.get_group_subject",
+ side_effect=[None, winner],
+ ) as mock_get_group_subject,
+ patch("superset.subjects.utils.db") as mock_db,
+ patch(
+ "superset.subjects.sync.sync_group_subject",
+ side_effect=IntegrityError("duplicate group_id", None,
Exception()),
+ ) as mock_sync,
+ ):
+ mock_db.session.get.return_value = group
+ # The mocked savepoint must not suppress the error raised inside its
body.
+ mock_db.session.begin_nested.return_value.__exit__.return_value = False
+
+ assert get_or_create_group_subject(7) is winner
+
+ # The insert (via ``sync``) ran and conflicted; the reload returns the
winner.
+ mock_sync.assert_called_once_with(group)
+ assert mock_get_group_subject.call_count == 2
+
+
+def test_get_or_create_group_subject_reraises_unrelated_integrity_error() ->
None:
+ """An IntegrityError that left no matching row isn't the expected
unique-key
+ race, so it propagates instead of silently dropping the group."""
+ from sqlalchemy.exc import IntegrityError
+
+ group = MagicMock()
+ group.id = 7
+
+ with (
+ patch("superset.subjects.utils.get_group_subject", return_value=None),
+ patch("superset.subjects.utils.db") as mock_db,
+ patch(
+ "superset.subjects.sync.sync_group_subject",
+ side_effect=IntegrityError("unrelated failure", None, Exception()),
+ ) as mock_sync,
+ ):
+ mock_db.session.get.return_value = group
+ mock_db.session.begin_nested.return_value.__exit__.return_value = False
+
+ with pytest.raises(IntegrityError):
+ get_or_create_group_subject(7)
+
+ mock_sync.assert_called_once_with(group)
+
+
+def test_get_or_create_group_subject_missing_group_returns_none() -> None:
+ """A group ID with no matching group at all resolves to None."""
+ with (
+ patch("superset.subjects.utils.get_group_subject", return_value=None),
+ patch("superset.subjects.utils.db") as mock_db,
+ patch("superset.subjects.sync.sync_group_subject") as mock_sync,
+ ):
+ mock_db.session.get.return_value = None
+
+ assert get_or_create_group_subject(999) is None
+
+ mock_sync.assert_not_called()
+
+
+def test_get_user_group_subjects_backfills_missing_group(app_context) -> None:
+ """A group the user belongs to but which has no Subject row is backfilled,
+ not skipped: a partial viewer set would silently lock out its members."""
+ synced = _make_subject(10, SubjectType.GROUP)
+ synced.group_id = 1
+ backfilled = _make_subject(11, SubjectType.GROUP)
+ backfilled.group_id = 2
+
+ with (
+ patch("superset.subjects.utils.db") as mock_db,
+ patch(
+ "superset.subjects.utils.get_or_create_group_subject",
+ return_value=backfilled,
+ ) as mock_get_or_create,
+ ):
+
mock_db.session.query.return_value.filter.return_value.all.return_value = [
+ synced
+ ]
+ # The user belongs to groups 1 (synced) and 2 (missing a Subject row).
+ mock_db.session.execute.return_value = [(1,), (2,)]
+
+ result = get_user_group_subjects(5)
+
+ assert result == [synced, backfilled]
+ mock_get_or_create.assert_called_once_with(2)
+
+
+def test_get_default_viewers_for_groups_discards_partial_result(app_context)
-> None:
+ """If any group lacks a Subject row, return [] so the datasource fallback
+ stays intact rather than a partial viewer set that locks its members
out."""
+ g1 = MagicMock()
+ g1.id = 1
+ g2 = MagicMock()
+ g2.id = 2
+ subject_for_g1 = _make_subject(10, SubjectType.GROUP)
+ subject_for_g1.group_id = 1
+
+ with (
+ patch(
+ "superset.subjects.utils._assigns_creator_groups_as_viewers",
+ return_value=True,
+ ),
+ patch(
+ "superset.subjects.utils.subjects_from_groups",
+ return_value=[subject_for_g1],
+ ),
+ ):
+ assert get_default_viewers_for_groups([g1, g2]) == []
+
+
+def test_get_default_viewers_for_groups_applies_when_all_resolve(app_context)
-> None:
+ """When every group resolves to a Subject, all are returned as viewers."""
+ g1 = MagicMock()
+ g1.id = 1
+ g2 = MagicMock()
+ g2.id = 2
+ s1 = _make_subject(10, SubjectType.GROUP)
+ s1.group_id = 1
+ s2 = _make_subject(11, SubjectType.GROUP)
+ s2.group_id = 2
+
+ with (
+ patch(
+ "superset.subjects.utils._assigns_creator_groups_as_viewers",
+ return_value=True,
+ ),
+ patch(
+ "superset.subjects.utils.subjects_from_groups",
+ return_value=[s1, s2],
+ ),
+ ):
+ assert get_default_viewers_for_groups([g1, g2]) == [s1, s2]
+
+
# --------------------------------------------------------------------------
# populate_subject_list
# --------------------------------------------------------------------------
diff --git a/tests/unit_tests/views/test_related_include_ids.py
b/tests/unit_tests/views/test_related_include_ids.py
new file mode 100644
index 00000000000..dc9e42802a8
--- /dev/null
+++ b/tests/unit_tests/views/test_related_include_ids.py
@@ -0,0 +1,124 @@
+# 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.
+"""Tests that ``?include_ids=`` cannot bypass related-field scoping.
+
+The ``/related/<column>`` endpoints scope their results with
+``base_related_field_filters``. The ``include_ids`` argument force-fetches
+specific rows, and must not be able to resolve principals that those filters
+deliberately hide.
+
+``include_ids`` exists so an edit form can render a value that is already
+associated with a record but falls outside the current page or search of the
+picker. Scoping it has an accepted consequence: a value that is already
+associated but has since been hidden by ``EXCLUDE_USERS_FROM_LISTS`` or
+``EXTRA_RELATED_QUERY_FILTERS`` stops resolving, so the form shows the bare id
+rather than a label. That is deliberate — a principal a deployment has chosen
+to hide must not be resolvable through a side door, and the association itself
+is untouched in the database.
+"""
+
+from typing import Any
+
+from flask_appbuilder.models.filters import BaseFilter
+from flask_appbuilder.models.sqla.interface import SQLAInterface
+from pytest_mock import MockerFixture
+from sqlalchemy import Column, create_engine, Integer, String
+from sqlalchemy.orm import declarative_base, sessionmaker
+
+from superset.views.base_api import BaseSupersetModelRestApi
+
+Base = declarative_base()
+
+
+class Principal(Base): # type: ignore[misc, valid-type]
+ __tablename__ = "principal"
+
+ id = Column(Integer, primary_key=True)
+ name = Column(String(50))
+
+ def __repr__(self) -> str:
+ return str(self.name)
+
+
+class ExcludeHiddenFilter(BaseFilter): # pylint:
disable=too-few-public-methods
+ """Stand-in for a deployment's principal-scoping filter."""
+
+ name = "Exclude hidden"
+ arg_name = "exclude_hidden"
+
+ def apply(self, query: Any, value: Any) -> Any:
+ return query.filter(Principal.name != "hidden")
+
+
+def _session():
+ engine = create_engine("sqlite://")
+ Base.metadata.create_all(engine)
+ session = sessionmaker(bind=engine)()
+ session.add_all(
+ [Principal(id=1, name="visible"), Principal(id=2, name="hidden")],
+ )
+ session.commit()
+ return session
+
+
+def _api(**overrides: Any) -> BaseSupersetModelRestApi:
+ api = BaseSupersetModelRestApi.__new__(BaseSupersetModelRestApi)
+ api.text_field_rel_fields = {}
+ api.extra_fields_rel_fields = {}
+ api.base_related_field_filters = {}
+ for key, value in overrides.items():
+ setattr(api, key, value)
+ return api
+
+
+def test_include_ids_cannot_resolve_a_filtered_out_principal(
+ mocker: MockerFixture,
+) -> None:
+ """Holds even when the principal is already associated with the record.
+
+ This is the case an edit form hits: the client sends the id of a current
+ owner, and a hidden principal must still not resolve to a label.
+ """
+ session = _session()
+ mocker.patch("superset.views.base_api.db").session = session
+ api = _api(
+ base_related_field_filters={
+ "owners": [["name", ExcludeHiddenFilter, None]],
+ },
+ )
+ result: list[dict[str, Any]] = []
+
+ api._add_extra_ids_to_result( # noqa: SLF001
+ SQLAInterface(Principal, session), "owners", [1, 2], result
+ )
+
+ assert [row["value"] for row in result] == [1]
+
+
+def test_include_ids_still_resolves_when_no_filters_are_configured(
+ mocker: MockerFixture,
+) -> None:
+ session = _session()
+ mocker.patch("superset.views.base_api.db").session = session
+ api = _api()
+ result: list[dict[str, Any]] = []
+
+ api._add_extra_ids_to_result( # noqa: SLF001
+ SQLAInterface(Principal, session), "owners", [1, 2], result
+ )
+
+ assert sorted(row["value"] for row in result) == [1, 2]
diff --git a/tests/unit_tests/views/test_user_schemas.py
b/tests/unit_tests/views/test_user_schemas.py
new file mode 100644
index 00000000000..6fafc344d9c
--- /dev/null
+++ b/tests/unit_tests/views/test_user_schemas.py
@@ -0,0 +1,52 @@
+# 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.
+"""Unit tests for the current-user (``/api/v1/me/``) response schema."""
+
+from types import SimpleNamespace
+
+from superset.views.users.schemas import UserResponseSchema
+
+
+def _user(**overrides):
+ """Build a stand-in for a FAB ``User`` row."""
+ attrs = {
+ "id": 1,
+ "username": "alice",
+ "email": "[email protected]",
+ "first_name": "Alice",
+ "last_name": "Doe",
+ "is_active": True,
+ "is_anonymous": False,
+ "login_count": 3,
+ "groups": [],
+ }
+ attrs.update(overrides)
+ return SimpleNamespace(**attrs)
+
+
+def test_user_response_includes_the_callers_groups():
+ groups = [SimpleNamespace(id=7, name="tenant-a")]
+
+ result = UserResponseSchema().dump(_user(groups=groups))
+
+ assert result["groups"] == [{"id": 7, "name": "tenant-a"}]
+
+
+def test_user_response_groups_is_empty_list_when_user_has_no_groups():
+ result = UserResponseSchema().dump(_user(groups=[]))
+
+ assert result["groups"] == []
