-
Notifications
You must be signed in to change notification settings - Fork 17.3k
feat(security): add built-in Public role for anonymous dashboard access #36548
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
07fd30f
1ade1a0
495b7d6
6c11df8
71bfd94
306dbdc
130de60
b5988ca
a4fa840
734b8de
cc7c30e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -264,6 +264,7 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods | |
| } | ||
|
|
||
| GAMMA_READ_ONLY_MODEL_VIEWS = { | ||
| "CssTemplate", | ||
| "Dataset", | ||
| "Datasource", | ||
| } | READ_ONLY_MODEL_VIEWS | ||
|
|
@@ -302,7 +303,6 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods | |
| "Annotation", | ||
| "CSS Templates", | ||
| "ColumnarToDatabaseView", | ||
| "CssTemplate", | ||
| "ExcelToDatabaseView", | ||
| "Import dashboards", | ||
| "ImportExportRestApi", | ||
|
|
@@ -389,6 +389,60 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods | |
| ("can_read", "Database"), | ||
| } | ||
|
|
||
| # Permissions for the Public role - minimal read-only access for viewing | ||
| # dashboards without authentication. This is more restrictive than Gamma. | ||
| # Users can set PUBLIC_ROLE_LIKE = "Public" to use these sensible defaults. | ||
| PUBLIC_ROLE_PERMISSIONS = { | ||
| # Core dashboard viewing | ||
| ("can_read", "Dashboard"), | ||
| ("can_read", "Chart"), | ||
| ("can_dashboard", "Superset"), | ||
| ("can_slice", "Superset"), | ||
| ("can_explore_json", "Superset"), | ||
| ("can_dashboard_permalink", "Superset"), | ||
| ("can_read", "DashboardPermalinkRestApi"), | ||
| # Dashboard filter interactions | ||
| ("can_read", "DashboardFilterStateRestApi"), | ||
| ("can_write", "DashboardFilterStateRestApi"), | ||
| # API access for chart rendering | ||
| ("can_time_range", "Api"), | ||
| ("can_query_form_data", "Api"), | ||
| ("can_query", "Api"), | ||
| # CSS for dashboard styling | ||
| ("can_read", "CssTemplate"), | ||
| # Embedded dashboard support | ||
| ("can_read", "EmbeddedDashboard"), | ||
| # Datasource metadata for chart rendering | ||
| ("can_get", "Datasource"), | ||
| ("can_external_metadata", "Datasource"), | ||
| # Annotations on charts | ||
| ("can_read", "Annotation"), | ||
| ("can_read", "AnnotationLayerRestApi"), | ||
| # Chart permalinks (for shared chart links) | ||
| ("can_read", "ExplorePermalinkRestApi"), | ||
| } | ||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have a Public role in production. I set it up around 2022 so many versions ago. Here I compare this PR's list to the list I'm using, both ways. An original artifact of the list I used was this gist: https://gist.github.com/byk0t/bd6e9c3839967b4ac28a8da30f468b2a its comments have a couple of useful clues. Present here but not in my Public role
It surprises me that my Public role works without having these: Present in my Public role but not this PRI think we might want to add:
I'm not sure about these - I have them but it's not obvious to me what they do and I don't remember why I added each:
Probably only a fit for my use case:
|
||
| # View menus that Public role should NOT have access to | ||
| PUBLIC_EXCLUDED_VIEW_MENUS = { | ||
| "SQL Lab", | ||
| "SQL Editor", | ||
| "Saved Queries", | ||
| "Query Search", | ||
| "Queries", | ||
| "Security", | ||
| "List Users", | ||
| "List Roles", | ||
| "Row Level Security", | ||
| "Row Level Security Filters", | ||
| "Access Requests", | ||
| "Action Log", | ||
| "Manage", | ||
| "Import dashboards", | ||
| "Annotation Layers", | ||
| "CSS Templates", | ||
| "Alerts & Report", | ||
| } | ||
|
|
||
| data_access_permissions = ( | ||
| "database_access", | ||
| "schema_access", | ||
|
|
@@ -1205,9 +1259,18 @@ def sync_role_definitions(self) -> None: | |
| self.set_role("sql_lab", self._is_sql_lab_pvm, pvms) | ||
|
|
||
| # Configure public role | ||
| if get_conf()["PUBLIC_ROLE_LIKE"]: | ||
| # If PUBLIC_ROLE_LIKE is "Public", use the built-in Public role with | ||
| # sensible defaults for anonymous dashboard viewing. | ||
| # If set to another role name (e.g., "Gamma"), copy permissions from that role. | ||
| # If not set (None), the Public role remains empty (default/legacy behavior). | ||
| public_role_like = get_conf()["PUBLIC_ROLE_LIKE"] | ||
| if public_role_like == "Public": | ||
| # Use the built-in Public role with minimal read-only permissions | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Role Name Inconsistency
The set_role call uses a hardcoded "Public" string, but to match the AUTH_ROLE_PUBLIC config (which defaults to "Public" but can be customized), it should use self.auth_role_public. This ensures consistency, as the copy_role in the elif branch correctly uses self.auth_role_public. Citations
Code Review Run #8f0e3a Should Bito avoid suggestions like this for future reviews? (Manage Rules)
|
||
| self.set_role("Public", self._is_public_pvm, pvms) | ||
| elif public_role_like: | ||
| # Copy permissions from another role (e.g., "Gamma") to Public | ||
| self.copy_role( | ||
| get_conf()["PUBLIC_ROLE_LIKE"], | ||
| public_role_like, | ||
| self.auth_role_public, | ||
| merge=True, | ||
| ) | ||
|
|
@@ -1427,6 +1490,34 @@ def _is_sql_lab_pvm(self, pvm: PermissionView) -> bool: | |
| in self.SQLLAB_EXTRA_PERMISSION_VIEWS | ||
| ) | ||
|
|
||
| def _is_public_pvm(self, pvm: PermissionView) -> bool: | ||
| """ | ||
| Return True if the FAB permission/view is appropriate for the Public role, | ||
| False otherwise. | ||
|
|
||
| The Public role is designed for anonymous/unauthenticated users who need | ||
| to view dashboards. It provides minimal read-only access - more restrictive | ||
| than Gamma - suitable for public-facing dashboard deployments. | ||
|
|
||
| :param pvm: The FAB permission/view | ||
| :returns: Whether the FAB object is appropriate for Public role | ||
| """ | ||
| # Explicitly allow permissions in the PUBLIC_ROLE_PERMISSIONS set | ||
| if (pvm.permission.name, pvm.view_menu.name) in self.PUBLIC_ROLE_PERMISSIONS: | ||
| return True | ||
|
|
||
| # Exclude any view menus in the excluded list | ||
| if pvm.view_menu.name in self.PUBLIC_EXCLUDED_VIEW_MENUS: | ||
| return False | ||
|
|
||
| # Exclude user-defined permissions (datasource_access, schema_access, etc.) | ||
| # These must be explicitly granted to the Public role | ||
| if self._is_user_defined_permission(pvm): | ||
| return False | ||
|
|
||
| # Exclude all other permissions not explicitly allowed | ||
| return False | ||
|
|
||
| def database_after_insert( | ||
| self, | ||
| mapper: Mapper, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Public is given several permissions that Gamma does not have:
can_read → CssTemplate
can_read → SecurityRestApi
can_get → MenuApi
can_get → OpenApi
can_external_metadata_by_name → Datasource
Because of these, public_perm_set - gamma_perm_set is not empty, so test_public_role_more_restrictive_than_gamma will fail. Either remove these permissions from Public or update the test, but both cannot be true at once.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for catching that... I'll take 'em out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I added CssTemplate can_read to Gamma :D