Skip to content

Commit

Permalink
feat(dashboard-rbac): dashboard lists (#12680)
Browse files Browse the repository at this point in the history
  • Loading branch information
amitmiran137 authored Jan 31, 2021
1 parent 2b9d579 commit 9a7fba8
Show file tree
Hide file tree
Showing 15 changed files with 1,345 additions and 30 deletions.
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
35 changes: 30 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,28 @@ 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)

@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):
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

0 comments on commit 9a7fba8

Please sign in to comment.