You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by be...@apache.org on 2023/07/13 17:33:19 UTC
[superset] branch master updated: fix: embedded dashboard check (#24690)
This is an automated email from the ASF dual-hosted git repository.
beto 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 9844b15e07 fix: embedded dashboard check (#24690)
9844b15e07 is described below
commit 9844b15e0751c2ffd923f168ad48478d1ca44533
Author: Beto Dealmeida <ro...@dealmeida.net>
AuthorDate: Thu Jul 13 10:33:12 2023 -0700
fix: embedded dashboard check (#24690)
---
superset/security/manager.py | 45 +++++++++++++---------
.../security/guest_token_security_tests.py | 29 ++++++++++++++
2 files changed, 56 insertions(+), 18 deletions(-)
diff --git a/superset/security/manager.py b/superset/security/manager.py
index 942ed66776..28354ac18d 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -2006,28 +2006,37 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
from superset import is_feature_enabled
from superset.dashboards.commands.exceptions import DashboardAccessDeniedError
- if self.is_guest_user() and dashboard.embedded:
+ if self.is_guest_user():
+ # Guest user is currently used for embedded dashboards only. If the guest user
+ # doesn't have access to the dashboard, ignore all other checks.
if self.has_guest_access(dashboard):
return
- else:
- if self.is_admin() or self.is_owner(dashboard):
- return
+ raise DashboardAccessDeniedError()
- # RBAC and legacy (datasource inferred) access controls.
- 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 (
- not dashboard.published
- or not dashboard.datasources
- or any(
- self.can_access_datasource(datasource)
- for datasource in dashboard.datasources
- )
- ):
+ if self.is_admin() or self.is_owner(dashboard):
+ return
+
+ # RBAC and legacy (datasource inferred) access controls.
+ 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
+ )
+ ):
+ return
raise DashboardAccessDeniedError()
diff --git a/tests/integration_tests/security/guest_token_security_tests.py b/tests/integration_tests/security/guest_token_security_tests.py
index 86d02975d0..e0517c9b28 100644
--- a/tests/integration_tests/security/guest_token_security_tests.py
+++ b/tests/integration_tests/security/guest_token_security_tests.py
@@ -204,3 +204,32 @@ class TestGuestUserDashboardAccess(SupersetTestCase):
with self.assertRaises(DashboardAccessDeniedError):
security_manager.raise_for_dashboard_access(self.dash)
+
+ def test_raise_for_dashboard_access_as_guest_no_rbac(self):
+ """
+ Test that guest account has no access to other dashboards.
+
+ A bug in the ``raise_for_dashboard_access`` logic allowed the guest user to
+ fetch data from other dashboards, as long as the other dashboard:
+
+ - was not embedded AND
+ - was not published OR
+ - had at least 1 datasource that the user had access to.
+
+ """
+ g.user = self.unauthorized_guest
+
+ # Create a draft dashboard that is not embedded
+ dash = Dashboard()
+ dash.dashboard_title = "My Dashboard"
+ dash.owners = []
+ dash.slices = []
+ dash.published = False
+ db.session.add(dash)
+ db.session.commit()
+
+ with self.assertRaises(DashboardAccessDeniedError):
+ security_manager.raise_for_dashboard_access(dash)
+
+ db.session.delete(dash)
+ db.session.commit()