fix(security manager): Users should not have access to all draft dashboards (#27015)

This commit is contained in:
Vitor Avila 2024-02-07 13:20:41 -03:00 committed by GitHub
parent e437356013
commit 01e2f8ace3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 32 additions and 23 deletions

View File

@ -2050,25 +2050,28 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
if self.is_admin() or self.is_owner(dashboard): if self.is_admin() or self.is_owner(dashboard):
return return
# RBAC and legacy (datasource inferred) access controls. # TODO: Once a better sharing flow is in place, we should move the
# dashboard.published check here so that it's applied to both
# regular RBAC and DASHBOARD_RBAC
# DASHBOARD_RBAC logic - Manage dashboard access through roles.
# Only applicable in case the dashboard has roles set.
if is_feature_enabled("DASHBOARD_RBAC") and dashboard.roles: if is_feature_enabled("DASHBOARD_RBAC") and dashboard.roles:
if dashboard.published and {role.id for role in dashboard.roles} & { if dashboard.published and {role.id for role in dashboard.roles} & {
role.id for role in self.get_user_roles() role.id for role in self.get_user_roles()
}: }:
return return
elif (
# To understand why we rely on status and give access to draft dashboards # REGULAR RBAC logic
# without roles take a look at: # User can only acess the dashboard in case:
# # It doesn't have any datasets; OR
# - https://github.com/apache/superset/pull/24350#discussion_r1225061550 # They have access to at least one dataset used.
# - https://github.com/apache/superset/pull/17511#issuecomment-975870169 # We currently don't check if the dashboard is published,
# # to allow creators to share a WIP dashboard with a viewer
not dashboard.published # to collect feedback.
or not dashboard.datasources elif not dashboard.datasources or any(
or any( self.can_access_datasource(datasource)
self.can_access_datasource(datasource) for datasource in dashboard.datasources
for datasource in dashboard.datasources
)
): ):
return return

View File

@ -404,22 +404,28 @@ class TestDashboardRoleBasedSecurity(BaseTestDashboardSecurity):
for dash in published_dashboards + draft_dashboards: for dash in published_dashboards + draft_dashboards:
revoke_access_to_dashboard(dash, "Public") revoke_access_to_dashboard(dash, "Public")
def test_get_draft_dashboard_without_roles_by_uuid(self): def test_cannot_get_draft_dashboard_without_roles_by_uuid(self):
""" """
Dashboard API: Test get draft dashboard without roles by uuid Dashboard API: Test get draft dashboard without roles by uuid
""" """
admin = self.get_user("admin") admin = self.get_user("admin")
dashboard = self.insert_dashboard("title", "slug1", [admin.id])
assert not dashboard.published database = create_database_to_db(name="test_db_rbac")
assert dashboard.roles == [] table = create_datasource_table_to_db(
name="test_datasource_rbac", db_id=database.id, owners=[admin]
)
dashboard_to_access = create_dashboard_to_db(
dashboard_title="test_dashboard_rbac",
owners=[admin],
slices=[create_slice_to_db(datasource_id=table.id)],
)
assert not dashboard_to_access.published
assert dashboard_to_access.roles == []
self.login(username="gamma") self.login(username="gamma")
uri = f"api/v1/dashboard/{dashboard.uuid}" uri = f"api/v1/dashboard/{dashboard_to_access.uuid}"
rv = self.client.get(uri) rv = self.client.get(uri)
assert rv.status_code == 200 assert rv.status_code == 403
# rollback changes
db.session.delete(dashboard)
db.session.commit()
def test_cannot_get_draft_dashboard_with_roles_by_uuid(self): def test_cannot_get_draft_dashboard_with_roles_by_uuid(self):
""" """