fix: Don't let users see dashboards only because it's favorited (#24991)

superset/dashboards/filters.py

@@ -24,7 +24,7 @@
 
 from superset import db, is_feature_enabled, security_manager
 from superset.connectors.sqla.models import SqlaTable
-from superset.models.core import Database, FavStar
+from superset.models.core import Database
 from superset.models.dashboard import Dashboard, is_uuid
 from superset.models.embedded_dashboard import EmbeddedDashboard
 from superset.models.slice import Slice
@@ -92,8 +92,8 @@ class DashboardAccessFilter(BaseFilter):  # pylint: disable=too-few-public-metho
     """
     List dashboards with the following criteria:
         1. Those which the user owns
-        2. Those which the user has favorited
-        3. Those which have been published (if they have access to at least one slice)
+        2. Those which have been published (if they have access to at least one slice)
+        3. Those that they have access to via a role (if `DASHBOARD_RBAC` is enabled)
 
     If the user is an admin then show all dashboards.
     This means they do not get curation but can still sort by "published"
@@ -126,12 +126,6 @@ def apply(self, query: Query, value: Any) -> Query:
             )
         )
 
-        users_favorite_dash_query = db.session.query(FavStar.obj_id).filter(
-            and_(
-                FavStar.user_id == get_user_id(),
-                FavStar.class_name == "Dashboard",
-            )
-        )
         owner_ids_query = (
             db.session.query(Dashboard.id)
             .join(Dashboard.owners)
@@ -179,7 +173,6 @@ def apply(self, query: Query, value: Any) -> Query:
             or_(
                 Dashboard.id.in_(owner_ids_query),
                 Dashboard.id.in_(datasource_perm_query),
-                Dashboard.id.in_(users_favorite_dash_query),
                 *feature_flagged_filters,
             )
         )

tests/integration_tests/dashboard_tests.py

@@ -27,7 +27,6 @@
 from tests.integration_tests.test_app import app
 from superset import db, security_manager
 from superset.connectors.sqla.models import SqlaTable
-from superset.models import core as models
 from superset.models.dashboard import Dashboard
 from superset.models.slice import Slice
 from tests.integration_tests.fixtures.birth_names_dashboard import (
@@ -227,44 +226,6 @@ def test_users_can_view_own_dashboard(self):
         self.assertIn(f"/superset/dashboard/{my_dash_slug}/", resp)
         self.assertNotIn(f"/superset/dashboard/{not_my_dash_slug}/", resp)
 
-    def test_users_can_view_favorited_dashboards(self):
-        user = security_manager.find_user("gamma")
-        fav_dash_slug = f"my_favorite_dash_{random()}"
-        regular_dash_slug = f"regular_dash_{random()}"
-
-        favorite_dash = Dashboard()
-        favorite_dash.dashboard_title = "My Favorite Dashboard"
-        favorite_dash.slug = fav_dash_slug
-
-        regular_dash = Dashboard()
-        regular_dash.dashboard_title = "A Plain Ol Dashboard"
-        regular_dash.slug = regular_dash_slug
-
-        db.session.add(favorite_dash)
-        db.session.add(regular_dash)
-        db.session.commit()
-
-        dash = db.session.query(Dashboard).filter_by(slug=fav_dash_slug).first()
-
-        favorites = models.FavStar()
-        favorites.obj_id = dash.id
-        favorites.class_name = "Dashboard"
-        favorites.user_id = user.id
-
-        db.session.add(favorites)
-        db.session.commit()
-
-        self.login(user.username)
-
-        resp = self.get_resp("/api/v1/dashboard/")
-
-        db.session.delete(favorites)
-        db.session.delete(regular_dash)
-        db.session.delete(favorite_dash)
-        db.session.commit()
-
-        self.assertIn(f"/superset/dashboard/{fav_dash_slug}/", resp)
-
     def test_user_can_not_view_unpublished_dash(self):
         admin_user = security_manager.find_user("admin")
         gamma_user = security_manager.find_user("gamma")

tests/integration_tests/dashboards/security/security_dataset_tests.py

@@ -23,7 +23,6 @@
 
 from superset import app
 from superset.daos.dashboard import DashboardDAO
-from superset.models import core as models
 from tests.integration_tests.dashboards.base_case import DashboardTestCase
 from tests.integration_tests.dashboards.consts import *
 from tests.integration_tests.dashboards.dashboard_test_utils import *
@@ -124,48 +123,6 @@ def test_get_dashboards__owners_can_view_empty_dashboard(self):
         # assert
         self.assertNotIn(dashboard_url, get_dashboards_response)
 
-    def test_get_dashboards__users_can_view_favorites_dashboards(self):
-        # arrange
-        user = security_manager.find_user("gamma")
-        fav_dash_slug = f"my_favorite_dash_{random_slug()}"
-        regular_dash_slug = f"regular_dash_{random_slug()}"
-
-        favorite_dash = Dashboard()
-        favorite_dash.dashboard_title = "My Favorite Dashboard"
-        favorite_dash.slug = fav_dash_slug
-
-        regular_dash = Dashboard()
-        regular_dash.dashboard_title = "A Plain Ol Dashboard"
-        regular_dash.slug = regular_dash_slug
-
-        db.session.add(favorite_dash)
-        db.session.add(regular_dash)
-        db.session.commit()
-
-        dash = db.session.query(Dashboard).filter_by(slug=fav_dash_slug).first()
-
-        favorites = models.FavStar()
-        favorites.obj_id = dash.id
-        favorites.class_name = "Dashboard"
-        favorites.user_id = user.id
-
-        db.session.add(favorites)
-        db.session.commit()
-
-        self.login(user.username)
-
-        # act
-        get_dashboards_response = self.get_resp(DASHBOARDS_API_URL)
-
-        # cleanup
-        db.session.delete(favorites)
-        db.session.delete(favorite_dash)
-        db.session.delete(regular_dash)
-        db.session.commit()
-
-        # assert
-        self.assertIn(f"/superset/dashboard/{fav_dash_slug}/", get_dashboards_response)
-
     def test_get_dashboards__user_can_not_view_unpublished_dash(self):
         # arrange
         admin_user = security_manager.find_user(ADMIN_USERNAME)