From 13af5cc86559e8b6573d6bce007ee1e2096d718e Mon Sep 17 00:00:00 2001 From: Superset Dev Date: Fri, 21 Aug 2026 00:44:50 -0700 Subject: [PATCH] fix(security): make user-registrations API admin-only and read-only The REST counterpart of the "User Registrations" FAB view was never added to ADMIN_ONLY_VIEW_MENUS, so role sync granted its list/get/ delete permissions to stock Gamma and Alpha, exposing pending registrants' PII and letting non-admins cancel registrations. Add "UserRegistrationsRestAPI" to the admin-only allowlist and restrict the API to its GET/GET_LIST/INFO routes so write handlers are never registered at all, even if the allowlist entry regresses later. --- superset/security/api.py | 6 ++++++ superset/security/manager.py | 4 ++++ tests/unit_tests/security/api_test.py | 27 ++++++++++++++++++++++++++- 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/superset/security/api.py b/superset/security/api.py index bb8995d4665..92084a25ced 100644 --- a/superset/security/api.py +++ b/superset/security/api.py @@ -40,6 +40,7 @@ from superset.commands.dashboard.embedded.exceptions import ( EmbeddedDashboardNotFoundError, ) from superset.commands.exceptions import ForbiddenError +from superset.constants import RouteMethod from superset.exceptions import SupersetGenericErrorException from superset.extensions import db, event_logger from superset.security.guest_token import ( @@ -423,6 +424,11 @@ class UserRegistrationsRestAPI(BaseSupersetModelRestApi): resource_name = "security/user_registrations" datamodel = SQLAInterface(RegisterUser) allow_browser_login = True + # This API is read-only by design: restricting the exposed routes keeps + # the FAB default POST/PUT/DELETE handlers from ever being registered, + # so a mis-granted role cannot create, alter, or silently cancel a + # pending registration. + include_route_methods = {RouteMethod.GET, RouteMethod.GET_LIST, RouteMethod.INFO} # NOTE: registration_hash is intentionally excluded from both list_columns # and search_columns. It is a bearer token for the # /register/activation/ flow; exposing it in API responses (and thus diff --git a/superset/security/manager.py b/superset/security/manager.py index 79c262dfddc..21f0729d410 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -1432,6 +1432,10 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods "Security", "SQL Lab", "User Registrations", + # REST counterpart of the FAB "User Registrations" views. FAB derives + # the view-menu name from the class name; without this entry + # _is_gamma_pvm grants its permissions to stock Gamma and Alpha. + "UserRegistrationsRestAPI", "User's Statistics", # Guarding all AB_ADD_SECURITY_API = True REST APIs "RoleRestAPI", diff --git a/tests/unit_tests/security/api_test.py b/tests/unit_tests/security/api_test.py index f2962a82a66..ad3c3c1e2eb 100644 --- a/tests/unit_tests/security/api_test.py +++ b/tests/unit_tests/security/api_test.py @@ -19,8 +19,10 @@ from typing import Any import pytest from marshmallow import ValidationError +from superset.constants import RouteMethod from superset.extensions import csrf -from superset.security.api import RlsRuleSchema +from superset.security.api import RlsRuleSchema, UserRegistrationsRestAPI +from superset.security.manager import SupersetSecurityManager @pytest.mark.parametrize( @@ -172,3 +174,26 @@ def test_rls_rule_schema_rejects_falsy_dataset(dataset: Any) -> None: with pytest.raises(ValidationError) as exc_info: RlsRuleSchema().load({"dataset": dataset, "clause": "tenant_id = 1"}) assert "dataset" in exc_info.value.messages + + +def test_user_registrations_rest_api_is_admin_only() -> None: + """ + The API is documented Admin-only, but the admin gate is membership in + ADMIN_ONLY_VIEW_MENUS keyed by the FAB-derived view-menu name (the class + name). If the entry is missing, ``superset init`` grants the API's + permissions to stock Gamma and Alpha via ``_is_gamma_pvm``, exposing + pending registrants' PII and registration deletion. + """ + assert "UserRegistrationsRestAPI" in SupersetSecurityManager.ADMIN_ONLY_VIEW_MENUS + + +def test_user_registrations_rest_api_routes_are_read_only() -> None: + """ + Only the read routes should be registered; the FAB default POST/PUT/ + DELETE handlers must not exist on this API at all. + """ + assert UserRegistrationsRestAPI.include_route_methods == { + RouteMethod.GET, + RouteMethod.GET_LIST, + RouteMethod.INFO, + }