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

feat(dashboard_rbac): dashboard lists #12680

Merged
merged 50 commits into from
Jan 31, 2021
Merged
Show file tree
Hide file tree
Changes from 47 commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
ca7e89f
feat: add dashboard roles to support dashboard level access
Jan 14, 2021
0f1a6db
fix: dashboard filter logic
Jan 15, 2021
39b6b2e
fix: negate dashboard has any roles
Jan 15, 2021
9884bc3
fix: pre-commit
Jan 16, 2021
3eb61b3
fix: unused imports
Jan 16, 2021
47235a4
fix: unused imports and invalid naming
Jan 16, 2021
2f13075
fix: hopefully last lint issue
Jan 16, 2021
9d26d30
fix: adapt to changes
Jan 22, 2021
55199ed
fix: fix pre-commit
Jan 22, 2021
dcc8e66
fix: extract id from role
Jan 22, 2021
c560de8
fix: extract hasNoRoleBasedAccess
Jan 22, 2021
0b869a2
fix: pylint bad method name casing :)
Jan 22, 2021
27f8fb9
fix: pre-commit non typed method
Jan 22, 2021
0223844
fix: remove redundant pass
amitmiran137 Jan 22, 2021
aa2e53e
feat: allow managing dashboard roles from the viewbased dashboard ed…
Jan 23, 2021
58b680b
fix: pre-commit :)
Jan 23, 2021
8714b8b
test: dashboard lists API and view endpoints
Jan 26, 2021
1d3e3ae
Merge branch 'master' into feat/dashboard_access_roles
Jan 26, 2021
60e92a3
chore: update down revision
Jan 26, 2021
ff92508
fix: license check and pre-commit
Jan 26, 2021
c8fadb9
fix: mypy pre-commit issues
Jan 26, 2021
061ce2f
test: add security_dataset_tests.py and removed non fixture test that…
Jan 26, 2021
af5d30f
fix: decouple test
Jan 27, 2021
b4d2cbb
fix: add_role if not exists
Jan 27, 2021
906fb50
fix: decouple another test
Jan 27, 2021
16d4061
fix: decouple another test
Jan 27, 2021
0c11ffc
fix: pre-commit :)
Jan 27, 2021
9c0b6a6
fix: make user creation with roles to copy role form gamma and and to…
Jan 27, 2021
2b8cd8e
fix: make copy_role optional and False by default
Jan 27, 2021
83d76ce
fix: pre commit ;)
Jan 27, 2021
90a32fd
fix: pre commit ;)
Jan 27, 2021
e7a4770
fix: hopfully last decoupling
Jan 27, 2021
7036216
fix: remove dependency from gamma user
Jan 27, 2021
14a7d74
fix: another try
Jan 27, 2021
2c2ade8
fix: after cr comments
Jan 28, 2021
903e3ba
Update roles description
amitmiran137 Jan 29, 2021
63051e7
chore(after CR) : dashboard_has_roles variable and usages in queries
Jan 29, 2021
7b7da44
chore(after CR) : move to dashboard util
Jan 29, 2021
f6c7cbe
chore(after CR) : remove irrelevant slugs
Jan 29, 2021
225493d
chore(after CR) : remove unused method
Jan 29, 2021
60d2d91
chore(after CR) : remove const
Jan 29, 2021
84bb61e
chore: changes after CR
Jan 29, 2021
cae1556
feat: DASHBOARD_RBAC ff support
Jan 29, 2021
2826b77
fix: pre-commit
Jan 29, 2021
d337209
chore: is_feature_enabled("DASHBOARD_RBAC") on every usage
Jan 30, 2021
29a5bc5
fix: after CR
Jan 30, 2021
ac45a58
fix: roles_based_query inside the if is_feature_enabled("DASHBOARD_RB…
Jan 30, 2021
594e9f2
fix: replace assert actions with the common assert pattern
Jan 30, 2021
80c26ae
feat: FF the dashboard CRUD view with the DASHBOARD_RBAC
Jan 30, 2021
812bb87
Revert "feat: FF the dashboard CRUD view with the DASHBOARD_RBAC"
Jan 30, 2021
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
1 change: 1 addition & 0 deletions superset/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,7 @@ def _try_json_readsha( # pylint: disable=unused-argument
"ALERT_REPORTS": False,
# Enable experimental feature to search for other dashboards
"OMNIBAR": False,
"DASHBOARD_RBAC": False,
}

# Set the default view to card/grid view if thumbnail support is enabled.
Expand Down
36 changes: 31 additions & 5 deletions superset/dashboards/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,16 @@
# under the License.
from typing import Any

from flask_appbuilder.security.sqla.models import Role
from flask_babel import lazy_gettext as _
from sqlalchemy import and_, or_
from sqlalchemy.orm.query import Query

from superset import db, security_manager
from superset import db, is_feature_enabled, security_manager
from superset.models.core import FavStar
from superset.models.dashboard import Dashboard
from superset.models.slice import Slice
from superset.views.base import BaseFilter, is_user_admin
from superset.views.base import BaseFilter, get_user_roles, is_user_admin
from superset.views.base_api import BaseFavoriteFilter


Expand Down Expand Up @@ -74,12 +75,19 @@ def apply(self, query: Query, value: Any) -> Query:

datasource_perms = security_manager.user_view_menu_names("datasource_access")
schema_perms = security_manager.user_view_menu_names("schema_access")
published_dash_query = (

is_rbac_disabled_filter = []
dashboard_has_roles = Dashboard.roles.any()
if is_feature_enabled("DASHBOARD_RBAC"):
is_rbac_disabled_filter.append(~dashboard_has_roles)

datasource_perm_query = (
db.session.query(Dashboard.id)
.join(Dashboard.slices)
.filter(
and_(
Dashboard.published == True, # pylint: disable=singleton-comparison
Dashboard.published.is_(True),
*is_rbac_disabled_filter,
or_(
Slice.perm.in_(datasource_perms),
Slice.schema_perm.in_(schema_perms),
Expand All @@ -104,11 +112,29 @@ def apply(self, query: Query, value: Any) -> Query:
)
)

dashboard_rbac_or_filters = []
if is_feature_enabled("DASHBOARD_RBAC"):
roles_based_query = (
db.session.query(Dashboard.id)
.join(Dashboard.roles)
.filter(
and_(
Dashboard.published.is_(True),
dashboard_has_roles,
Role.id.in_([x.id for x in get_user_roles()]),
),
)
)

dashboard_rbac_or_filters.append(Dashboard.id.in_(roles_based_query))


query = query.filter(
or_(
Dashboard.id.in_(owner_ids_query),
Dashboard.id.in_(published_dash_query),
Dashboard.id.in_(datasource_perm_query),
Dashboard.id.in_(users_favorite_dash_query),
*dashboard_rbac_or_filters,
)
)

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
"""add roles relationship to dashboard
Revision ID: e11ccdd12658
Revises: 260bf0649a77
Create Date: 2021-01-14 19:12:43.406230
"""
# revision identifiers, used by Alembic.
revision = "e11ccdd12658"
down_revision = "260bf0649a77"
import sqlalchemy as sa
from alembic import op


def upgrade():
op.create_table(
"dashboard_roles",
sa.Column("id", sa.Integer(), nullable=False),
sa.Column("role_id", sa.Integer(), nullable=False),
sa.Column("dashboard_id", sa.Integer(), nullable=True),
sa.ForeignKeyConstraint(["dashboard_id"], ["dashboards.id"]),
sa.ForeignKeyConstraint(["role_id"], ["ab_role.id"]),
sa.PrimaryKeyConstraint("id"),
)


def downgrade():
op.drop_table("dashboard_roles")
11 changes: 10 additions & 1 deletion superset/models/dashboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,15 @@ def copy_dashboard(
)


DashboardRoles = Table(
"dashboard_roles",
metadata,
Column("id", Integer, primary_key=True),
Column("dashboard_id", Integer, ForeignKey("dashboards.id"), nullable=False),
Column("role_id", Integer, ForeignKey("ab_role.id"), nullable=False),
)


class Dashboard( # pylint: disable=too-many-instance-attributes
Model, AuditMixinNullable, ImportExportMixin
):
Expand All @@ -132,7 +141,7 @@ class Dashboard( # pylint: disable=too-many-instance-attributes
slices = relationship(Slice, secondary=dashboard_slices, backref="dashboards")
owners = relationship(security_manager.user_model, secondary=dashboard_user)
published = Column(Boolean, default=False)

roles = relationship(security_manager.role_model, secondary=DashboardRoles)
export_fields = [
"dashboard_title",
"position_json",
Expand Down
8 changes: 8 additions & 0 deletions superset/views/dashboard/mixin.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ class DashboardMixin: # pylint: disable=too-few-public-methods
"dashboard_title",
"slug",
"owners",
"roles",
"position_json",
"css",
"json_metadata",
Expand Down Expand Up @@ -62,6 +63,12 @@ class DashboardMixin: # pylint: disable=too-few-public-methods
"want to alter specific parameters."
),
"owners": _("Owners is a list of users who can alter the dashboard."),
"roles": _(
"Roles is a list which defines access to the dashboard. "
"These roles are always applied in addition to restrictions on dataset "
"level access. "
"If no roles defined then the dashboard is available to all roles."
),
"published": _(
"Determines whether or not this dashboard is "
"visible in the list of all dashboards"
Expand All @@ -74,6 +81,7 @@ class DashboardMixin: # pylint: disable=too-few-public-methods
"slug": _("Slug"),
"charts": _("Charts"),
"owners": _("Owners"),
"roles": _("Roles"),
"published": _("Published"),
"creator": _("Creator"),
"modified": _("Modified"),
Expand Down
26 changes: 20 additions & 6 deletions tests/base_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,6 @@ def logged_in_admin():


class SupersetTestCase(TestCase):

default_schema_backend_map = {
"sqlite": "main",
"mysql": "superset",
Expand All @@ -135,7 +134,9 @@ def get_birth_names_dataset():
)

@staticmethod
def create_user_with_roles(username: str, roles: List[str]):
def create_user_with_roles(
username: str, roles: List[str], should_create_roles: bool = False
):
user_to_create = security_manager.find_user(username)
if not user_to_create:
security_manager.add_user(
Expand All @@ -149,7 +150,12 @@ def create_user_with_roles(username: str, roles: List[str]):
db.session.commit()
user_to_create = security_manager.find_user(username)
assert user_to_create
user_to_create.roles = [security_manager.find_role(r) for r in roles]
user_to_create.roles = []
for chosen_user_role in roles:
if should_create_roles:
## copy role from gamma but without data permissions
security_manager.copy_role("Gamma", chosen_user_role, merge=False)
user_to_create.roles.append(security_manager.find_role(chosen_user_role))
db.session.commit()
return user_to_create

Expand Down Expand Up @@ -290,18 +296,26 @@ def logout(self):
self.client.get("/logout/", follow_redirects=True)

def grant_public_access_to_table(self, table):
public_role = security_manager.find_role("Public")
role_name = "Public"
self.grant_role_access_to_table(table, role_name)

def grant_role_access_to_table(self, table, role_name):
role = security_manager.find_role(role_name)
perms = db.session.query(ab_models.PermissionView).all()
for perm in perms:
if (
perm.permission.name == "datasource_access"
and perm.view_menu
and table.perm in perm.view_menu.name
):
security_manager.add_permission_role(public_role, perm)
security_manager.add_permission_role(role, perm)

def revoke_public_access_to_table(self, table):
public_role = security_manager.find_role("Public")
role_name = "Public"
self.revoke_role_access_to_table(role_name, table)

def revoke_role_access_to_table(self, role_name, table):
public_role = security_manager.find_role(role_name)
perms = db.session.query(ab_models.PermissionView).all()
for perm in perms:
if (
Expand Down
18 changes: 0 additions & 18 deletions tests/dashboard_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -453,24 +453,6 @@ def test_only_owners_can_save(self):
db.session.commit()
self.test_save_dash("alpha")

def test_owners_can_view_empty_dashboard(self):
dash = db.session.query(Dashboard).filter_by(slug="empty_dashboard").first()
if not dash:
dash = Dashboard()
dash.dashboard_title = "Empty Dashboard"
dash.slug = "empty_dashboard"
else:
dash.slices = []
dash.owners = []
db.session.merge(dash)
db.session.commit()

gamma_user = security_manager.find_user("gamma")
self.login(gamma_user.username)

resp = self.get_resp("/api/v1/dashboard/")
self.assertNotIn("/superset/dashboard/empty_dashboard/", resp)

amitmiran137 marked this conversation as resolved.
Show resolved Hide resolved
@pytest.mark.usefixtures("load_energy_table_with_slice", "load_dashboard")
def test_users_can_view_published_dashboard(self):
resp = self.get_resp("/api/v1/dashboard/")
Expand Down
114 changes: 114 additions & 0 deletions tests/dashboards/base_case.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
import json
from typing import Any, Dict, Union

import prison
from flask import Response

from superset import app, security_manager
from tests.base_tests import SupersetTestCase
from tests.dashboards.consts import *
from tests.dashboards.dashboard_test_utils import build_save_dash_parts
from tests.dashboards.superset_factory_util import delete_all_inserted_objects


class DashboardTestCase(SupersetTestCase):
amitmiran137 marked this conversation as resolved.
Show resolved Hide resolved
def get_dashboard_via_api_by_id(self, dashboard_id: int) -> Response:
uri = DASHBOARD_API_URL_FORMAT.format(dashboard_id)
return self.get_assert_metric(uri, "get")

def get_dashboard_view_response(self, dashboard_to_access) -> Response:
return self.client.get(dashboard_to_access.url)

def get_dashboard_api_response(self, dashboard_to_access) -> Response:
return self.client.get(DASHBOARD_API_URL_FORMAT.format(dashboard_to_access.id))

def get_dashboards_list_response(self) -> Response:
return self.client.get(GET_DASHBOARDS_LIST_VIEW)

def get_dashboards_api_response(self) -> Response:
return self.client.get(DASHBOARDS_API_URL)

def save_dashboard_via_view(
self, dashboard_id: Union[str, int], dashboard_data: Dict[str, Any]
) -> Response:
save_dash_url = SAVE_DASHBOARD_URL_FORMAT.format(dashboard_id)
return self.get_resp(save_dash_url, data=dict(data=json.dumps(dashboard_data)))

def save_dashboard(
self, dashboard_id: Union[str, int], dashboard_data: Dict[str, Any]
) -> Response:
return self.save_dashboard_via_view(dashboard_id, dashboard_data)

def delete_dashboard_via_view(self, dashboard_id: int) -> Response:
delete_dashboard_url = DELETE_DASHBOARD_VIEW_URL_FORMAT.format(dashboard_id)
return self.get_resp(delete_dashboard_url, {})

def delete_dashboard_via_api(self, dashboard_id):
uri = DASHBOARD_API_URL_FORMAT.format(dashboard_id)
return self.delete_assert_metric(uri, "delete")

def bulk_delete_dashboard_via_api(self, dashboard_ids):
uri = DASHBOARDS_API_URL_WITH_QUERY_FORMAT.format(prison.dumps(dashboard_ids))
return self.delete_assert_metric(uri, "bulk_delete")

def delete_dashboard(self, dashboard_id: int) -> Response:
return self.delete_dashboard_via_view(dashboard_id)

def assert_permission_was_created(self, dashboard):
view_menu = security_manager.find_view_menu(dashboard.view_name)
self.assertIsNotNone(view_menu)
self.assertEqual(len(security_manager.find_permissions_view_menu(view_menu)), 1)

def assert_permission_kept_and_changed(self, updated_dashboard, excepted_view_id):
view_menu_after_title_changed = security_manager.find_view_menu(
updated_dashboard.view_name
)
self.assertIsNotNone(view_menu_after_title_changed)
self.assertEqual(view_menu_after_title_changed.id, excepted_view_id)

def assert_permissions_were_deleted(self, deleted_dashboard):
view_menu = security_manager.find_view_menu(deleted_dashboard.view_name)
self.assertIsNone(view_menu)

def save_dash_basic_case(self, username=ADMIN_USERNAME):
# arrange
self.login(username=username)
(
dashboard_to_save,
data_before_change,
data_after_change,
) = build_save_dash_parts()

# act
save_dash_response = self.save_dashboard_via_view(
dashboard_to_save.id, data_after_change
)

# assert
self.assertIn("SUCCESS", save_dash_response)

# post test
self.save_dashboard(dashboard_to_save.id, data_before_change)

def clean_created_objects(self):
with app.test_request_context():
self.logout()
self.login("admin")
delete_all_inserted_objects()
self.logout()
Loading