You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by li...@apache.org on 2023/08/18 00:35:46 UTC

[superset] branch master updated: fix: Don't let users see dashboards only because it's favorited (#24991)

This is an automated email from the ASF dual-hosted git repository.

lilykuang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git


The following commit(s) were added to refs/heads/master by this push:
     new 258e56285a fix: Don't let users see dashboards only because it's favorited (#24991)
258e56285a is described below

commit 258e56285ae13f55ef9c3704c79dcc4714ed3533
Author: Jack Fragassi <jf...@gmail.com>
AuthorDate: Thu Aug 17 17:35:37 2023 -0700

    fix: Don't let users see dashboards only because it's favorited (#24991)
---
 superset/dashboards/filters.py                     | 13 ++-----
 tests/integration_tests/dashboard_tests.py         | 39 --------------------
 .../dashboards/security/security_dataset_tests.py  | 43 ----------------------
 3 files changed, 3 insertions(+), 92 deletions(-)

diff --git a/superset/dashboards/filters.py b/superset/dashboards/filters.py
index 596e97de31..0c7878d508 100644
--- a/superset/dashboards/filters.py
+++ b/superset/dashboards/filters.py
@@ -24,7 +24,7 @@ from sqlalchemy.orm.query import Query
 
 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 @@ class DashboardAccessFilter(BaseFilter):  # pylint: disable=too-few-public-metho
             )
         )
 
-        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 @@ class DashboardAccessFilter(BaseFilter):  # pylint: disable=too-few-public-metho
             or_(
                 Dashboard.id.in_(owner_ids_query),
                 Dashboard.id.in_(datasource_perm_query),
-                Dashboard.id.in_(users_favorite_dash_query),
                 *feature_flagged_filters,
             )
         )
diff --git a/tests/integration_tests/dashboard_tests.py b/tests/integration_tests/dashboard_tests.py
index fef4edd6cc..0df9b22267 100644
--- a/tests/integration_tests/dashboard_tests.py
+++ b/tests/integration_tests/dashboard_tests.py
@@ -27,7 +27,6 @@ from sqlalchemy import func
 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 @@ class TestDashboard(SupersetTestCase):
         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")
diff --git a/tests/integration_tests/dashboards/security/security_dataset_tests.py b/tests/integration_tests/dashboards/security/security_dataset_tests.py
index dffab61a7a..54e8b81442 100644
--- a/tests/integration_tests/dashboards/security/security_dataset_tests.py
+++ b/tests/integration_tests/dashboards/security/security_dataset_tests.py
@@ -23,7 +23,6 @@ from flask import escape
 
 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 @@ class TestDashboardDatasetSecurity(DashboardTestCase):
         # 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)