fix: Dashboard access when DASHBOARD_RBAC is disabled (#17511)

* fix: Dashboard access when RBAC is disabled

* Sends 403 when forbidden

* Fixes issort

* Changes assertion

* Allow access to unpublished dashboards that don't have roles

* Fixes the test_get_dashboard_changed_on test

superset/dashboards/api.py

@@ -39,6 +39,7 @@
 from superset.dashboards.commands.create import CreateDashboardCommand
 from superset.dashboards.commands.delete import DeleteDashboardCommand
 from superset.dashboards.commands.exceptions import (
+    DashboardAccessDeniedError,
     DashboardBulkDeleteFailedError,
     DashboardCreateFailedError,
     DashboardDeleteFailedError,
@@ -267,6 +268,8 @@ def get(self, id_or_slug: str) -> Response:
               $ref: '#/components/responses/400'
             401:
               $ref: '#/components/responses/401'
+            403:
+              $ref: '#/components/responses/403'
             404:
               $ref: '#/components/responses/404'
         """
@@ -275,6 +278,8 @@ def get(self, id_or_slug: str) -> Response:
             dash = DashboardDAO.get_by_id_or_slug(id_or_slug)
             result = self.dashboard_get_response_schema.dump(dash)
             return self.response(200, result=result)
+        except DashboardAccessDeniedError:
+            return self.response_403()
         except DashboardNotFoundError:
             return self.response_404()
 
@@ -327,6 +332,8 @@ def get_datasets(self, id_or_slug: str) -> Response:
               $ref: '#/components/responses/400'
             401:
               $ref: '#/components/responses/401'
+            403:
+              $ref: '#/components/responses/403'
             404:
               $ref: '#/components/responses/404'
         """
@@ -336,6 +343,8 @@ def get_datasets(self, id_or_slug: str) -> Response:
                 self.dashboard_dataset_schema.dump(dataset) for dataset in datasets
             ]
             return self.response(200, result=result)
+        except DashboardAccessDeniedError:
+            return self.response_403()
         except DashboardNotFoundError:
             return self.response_404()
 
@@ -386,6 +395,8 @@ def get_charts(self, id_or_slug: str) -> Response:
               $ref: '#/components/responses/400'
             401:
               $ref: '#/components/responses/401'
+            403:
+              $ref: '#/components/responses/403'
             404:
               $ref: '#/components/responses/404'
         """
@@ -401,6 +412,8 @@ def get_charts(self, id_or_slug: str) -> Response:
                     form_data.pop("label_colors", None)
 
             return self.response(200, result=result)
+        except DashboardAccessDeniedError:
+            return self.response_403()
         except DashboardNotFoundError:
             return self.response_404()
 

superset/security/manager.py

@@ -1174,19 +1174,23 @@ def raise_for_dashboard_access(dashboard: "Dashboard") -> None:
         from superset.views.base import get_user_roles, is_user_admin
         from superset.views.utils import is_owner
 
+        has_rbac_access = True
+
         if is_feature_enabled("DASHBOARD_RBAC"):
             has_rbac_access = any(
                 dashboard_role.id in [user_role.id for user_role in get_user_roles()]
                 for dashboard_role in dashboard.roles
             )
-            can_access = (
-                is_user_admin()
-                or is_owner(dashboard, g.user)
-                or (dashboard.published and has_rbac_access)
-            )
 
-            if not can_access:
-                raise DashboardAccessDeniedError()
+        can_access = (
+            is_user_admin()
+            or is_owner(dashboard, g.user)
+            or (dashboard.published and has_rbac_access)
+            or (not dashboard.published and not dashboard.roles)
+        )
+
+        if not can_access:
+            raise DashboardAccessDeniedError()
 
     @staticmethod
     def can_access_based_on_dashboard(datasource: "BaseDatasource") -> bool:

tests/integration_tests/dashboards/api_tests.py

@@ -395,7 +395,7 @@ def test_get_dashboard_no_data_access(self):
         self.login(username="gamma")
         uri = f"api/v1/dashboard/{dashboard.id}"
         rv = self.client.get(uri)
-        self.assertEqual(rv.status_code, 200)
+        assert rv.status_code == 200
         # rollback changes
         db.session.delete(dashboard)
         db.session.commit()

tests/integration_tests/dashboards/dao_tests.py

@@ -85,6 +85,7 @@ def test_set_dash_metadata(self):
 
     @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices")
     def test_get_dashboard_changed_on(self):
+        self.login(username="admin")
         session = db.session()
         dashboard = session.query(Dashboard).filter_by(slug="world_health").first()