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(security manager): Users should not have access to all draft dashboards #27015

Merged
merged 4 commits into from
Feb 7, 2024
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
31 changes: 17 additions & 14 deletions superset/security/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -2018,25 +2018,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
Loading