Skip to content

Commit

Permalink
fix(security manager): Users should not have access to all draft dash…
Browse files Browse the repository at this point in the history
…boards (apache#27015)

(cherry picked from commit 01e2f8a)
  • Loading branch information
Vitor-Avila authored and sadpandajoe committed Feb 12, 2024
1 parent 2bd03cf commit 5d4607b
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 23 deletions.
31 changes: 17 additions & 14 deletions superset/security/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -1975,25 +1975,28 @@ def raise_for_access(
if self.is_admin() or self.is_owner(dashboard):
return

# RBAC and legacy (datasource inferred) access controls.
# TODO: Once a better sharing flow is in place, we should move the
# dashboard.published check here so that it's applied to both
# regular RBAC and DASHBOARD_RBAC

# DASHBOARD_RBAC logic - Manage dashboard access through roles.
# Only applicable in case the dashboard has roles set.
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
)

# REGULAR RBAC logic
# User can only acess the dashboard in case:
# It doesn't have any datasets; OR
# They have access to at least one dataset used.
# We currently don't check if the dashboard is published,
# to allow creators to share a WIP dashboard with a viewer
# to collect feedback.
elif not dashboard.datasources or any(
self.can_access_datasource(datasource)
for datasource in dashboard.datasources
):
return

Expand Down
24 changes: 15 additions & 9 deletions tests/integration_tests/dashboards/security/security_rbac_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -404,22 +404,28 @@ def test_get_dashboards_api__public_user_get_only_published_permitted_dashboards
for dash in published_dashboards + draft_dashboards:
revoke_access_to_dashboard(dash, "Public")

def test_get_draft_dashboard_without_roles_by_uuid(self):
def test_cannot_get_draft_dashboard_without_roles_by_uuid(self):
"""
Dashboard API: Test get draft dashboard without roles by uuid
"""
admin = self.get_user("admin")
dashboard = self.insert_dashboard("title", "slug1", [admin.id])
assert not dashboard.published
assert dashboard.roles == []

database = create_database_to_db(name="test_db_rbac")
table = create_datasource_table_to_db(
name="test_datasource_rbac", db_id=database.id, owners=[admin]
)
dashboard_to_access = create_dashboard_to_db(
dashboard_title="test_dashboard_rbac",
owners=[admin],
slices=[create_slice_to_db(datasource_id=table.id)],
)
assert not dashboard_to_access.published
assert dashboard_to_access.roles == []

self.login(username="gamma")
uri = f"api/v1/dashboard/{dashboard.uuid}"
uri = f"api/v1/dashboard/{dashboard_to_access.uuid}"
rv = self.client.get(uri)
assert rv.status_code == 200
# rollback changes
db.session.delete(dashboard)
db.session.commit()
assert rv.status_code == 403

def test_cannot_get_draft_dashboard_with_roles_by_uuid(self):
"""
Expand Down

0 comments on commit 5d4607b

Please sign in to comment.