diff --git a/superset/security/manager.py b/superset/security/manager.py index 72d3be27d33cf..f1b1b2bcecef5 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -2050,25 +2050,28 @@ def raise_for_access( if self.is_admin() or self.is_owner(dashboard): 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 dashboard.published and {role.id for role in dashboard.roles} & { role.id for role in self.get_user_roles() }: return - elif ( - # To understand why we rely on status and give access to draft dashboards - # without roles take a look at: - # - # - https://github.com/apache/superset/pull/24350#discussion_r1225061550 - # - https://github.com/apache/superset/pull/17511#issuecomment-975870169 - # - not dashboard.published - or not dashboard.datasources - or any( - self.can_access_datasource(datasource) - for datasource in dashboard.datasources - ) + + # REGULAR RBAC logic + # User can only acess the dashboard in case: + # It doesn't have any datasets; OR + # They have access to at least one dataset used. + # We currently don't check if the dashboard is published, + # to allow creators to share a WIP dashboard with a viewer + # to collect feedback. + elif not dashboard.datasources or any( + self.can_access_datasource(datasource) + for datasource in dashboard.datasources ): return diff --git a/tests/integration_tests/dashboards/security/security_rbac_tests.py b/tests/integration_tests/dashboards/security/security_rbac_tests.py index 792c9d1716d31..4f1cc5b221196 100644 --- a/tests/integration_tests/dashboards/security/security_rbac_tests.py +++ b/tests/integration_tests/dashboards/security/security_rbac_tests.py @@ -404,22 +404,28 @@ def test_get_dashboards_api__public_user_get_only_published_permitted_dashboards for dash in published_dashboards + draft_dashboards: 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 """ admin = self.get_user("admin") - dashboard = self.insert_dashboard("title", "slug1", [admin.id]) - assert not dashboard.published - assert dashboard.roles == [] + + database = create_database_to_db(name="test_db_rbac") + 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") - uri = f"api/v1/dashboard/{dashboard.uuid}" + uri = f"api/v1/dashboard/{dashboard_to_access.uuid}" rv = self.client.get(uri) - assert rv.status_code == 200 - # rollback changes - db.session.delete(dashboard) - db.session.commit() + assert rv.status_code == 403 def test_cannot_get_draft_dashboard_with_roles_by_uuid(self): """