Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 3 additions & 10 deletions superset/dashboards/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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,
)
)
Expand Down
39 changes: 0 additions & 39 deletions tests/integration_tests/dashboard_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 *
Expand Down Expand Up @@ -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)
Expand Down
Loading