You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by vi...@apache.org on 2021/02/07 14:17:10 UTC
[superset] branch master updated: feat(dashboard_rbac): dashboards
API support for roles create/update + roles validation (#12865)
This is an automated email from the ASF dual-hosted git repository.
villebro 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 8ccf2e8 feat(dashboard_rbac): dashboards API support for roles create/update + roles validation (#12865)
8ccf2e8 is described below
commit 8ccf2e8f1e33a95c03b9d391068f615efa22570e
Author: Amit Miran <47...@users.noreply.github.com>
AuthorDate: Sun Feb 7 16:16:19 2021 +0200
feat(dashboard_rbac): dashboards API support for roles create/update + roles validation (#12865)
---
superset/commands/exceptions.py | 7 +++++++
superset/commands/utils.py | 27 +++++++++++++++++++++------
superset/dashboards/api.py | 6 ++++++
superset/dashboards/commands/create.py | 13 ++++++++++++-
superset/dashboards/commands/update.py | 24 +++++++++++++++++++-----
superset/dashboards/schemas.py | 8 ++++++++
superset/security/manager.py | 9 +++++++++
tests/base_tests.py | 9 +++++++++
tests/dashboards/api_tests.py | 30 ++++++++++++++++++++++++++++--
9 files changed, 119 insertions(+), 14 deletions(-)
diff --git a/superset/commands/exceptions.py b/superset/commands/exceptions.py
index fdc7bee..bb8992a 100644
--- a/superset/commands/exceptions.py
+++ b/superset/commands/exceptions.py
@@ -85,6 +85,13 @@ class OwnersNotFoundValidationError(ValidationError):
super().__init__([_("Owners are invalid")], field_name="owners")
+class RolesNotFoundValidationError(ValidationError):
+ status = 422
+
+ def __init__(self) -> None:
+ super().__init__([_("Some roles do not exist")], field_name="roles")
+
+
class DatasourceNotFoundValidationError(ValidationError):
status = 404
diff --git a/superset/commands/utils.py b/superset/commands/utils.py
index 874ea4b..b39cf64 100644
--- a/superset/commands/utils.py
+++ b/superset/commands/utils.py
@@ -16,11 +16,12 @@
# under the License.
from typing import List, Optional
-from flask_appbuilder.security.sqla.models import User
+from flask_appbuilder.security.sqla.models import Role, User
from superset.commands.exceptions import (
DatasourceNotFoundValidationError,
OwnersNotFoundValidationError,
+ RolesNotFoundValidationError,
)
from superset.connectors.base.models import BaseDatasource
from superset.connectors.connector_registry import ConnectorRegistry
@@ -28,19 +29,19 @@ from superset.datasets.commands.exceptions import DatasetNotFoundError
from superset.extensions import db, security_manager
-def populate_owners(user: User, owners_ids: Optional[List[int]] = None) -> List[User]:
+def populate_owners(user: User, owner_ids: Optional[List[int]] = None) -> List[User]:
"""
Helper function for commands, will fetch all users from owners id's
Can raise ValidationError
:param user: The current user
- :param owners_ids: A List of owners by id's
+ :param owner_ids: A List of owners by id's
"""
owners = list()
- if not owners_ids:
+ if not owner_ids:
return [user]
- if user.id not in owners_ids:
+ if user.id not in owner_ids:
owners.append(user)
- for owner_id in owners_ids:
+ for owner_id in owner_ids:
owner = security_manager.get_user_by_id(owner_id)
if not owner:
raise OwnersNotFoundValidationError()
@@ -48,6 +49,20 @@ def populate_owners(user: User, owners_ids: Optional[List[int]] = None) -> List[
return owners
+def populate_roles(role_ids: Optional[List[int]] = None) -> List[Role]:
+ """
+ Helper function for commands, will fetch all roles from roles id's
+ :raises RolesNotFoundValidationError: If a role in the input list is not found
+ :param role_ids: A List of roles by id's
+ """
+ roles: List[Role] = []
+ if role_ids:
+ roles = security_manager.find_roles_by_id(role_ids)
+ if len(roles) != len(role_ids):
+ raise RolesNotFoundValidationError()
+ return roles
+
+
def get_datasource_by_id(datasource_id: int, datasource_type: str) -> BaseDatasource:
try:
return ConnectorRegistry.get_datasource(
diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py
index 61b56b1..dbde513 100644
--- a/superset/dashboards/api.py
+++ b/superset/dashboards/api.py
@@ -106,6 +106,8 @@ class DashboardRestApi(BaseSupersetModelRestApi):
"owners.username",
"owners.first_name",
"owners.last_name",
+ "roles.id",
+ "roles.name",
"changed_by_name",
"changed_by_url",
"changed_by.username",
@@ -142,6 +144,8 @@ class DashboardRestApi(BaseSupersetModelRestApi):
"owners.username",
"owners.first_name",
"owners.last_name",
+ "roles.id",
+ "roles.name",
]
list_select_columns = list_columns + ["changed_on", "changed_by_fk"]
order_columns = [
@@ -156,6 +160,7 @@ class DashboardRestApi(BaseSupersetModelRestApi):
"dashboard_title",
"slug",
"owners",
+ "roles",
"position_json",
"css",
"json_metadata",
@@ -168,6 +173,7 @@ class DashboardRestApi(BaseSupersetModelRestApi):
"dashboard_title",
"id",
"owners",
+ "roles",
"published",
"slug",
"changed_by",
diff --git a/superset/dashboards/commands/create.py b/superset/dashboards/commands/create.py
index dff25d6..e351d67 100644
--- a/superset/dashboards/commands/create.py
+++ b/superset/dashboards/commands/create.py
@@ -22,7 +22,7 @@ from flask_appbuilder.security.sqla.models import User
from marshmallow import ValidationError
from superset.commands.base import BaseCommand
-from superset.commands.utils import populate_owners
+from superset.commands.utils import populate_owners, populate_roles
from superset.dao.exceptions import DAOCreateFailedError
from superset.dashboards.commands.exceptions import (
DashboardCreateFailedError,
@@ -52,6 +52,7 @@ class CreateDashboardCommand(BaseCommand):
def validate(self) -> None:
exceptions: List[ValidationError] = list()
owner_ids: Optional[List[int]] = self._properties.get("owners")
+ role_ids: Optional[List[int]] = self._properties.get("roles")
slug: str = self._properties.get("slug", "")
# Validate slug uniqueness
@@ -67,3 +68,13 @@ class CreateDashboardCommand(BaseCommand):
exception = DashboardInvalidError()
exception.add_list(exceptions)
raise exception
+
+ try:
+ roles = populate_roles(role_ids)
+ self._properties["roles"] = roles
+ except ValidationError as ex:
+ exceptions.append(ex)
+ if exceptions:
+ exception = DashboardInvalidError()
+ exception.add_list(exceptions)
+ raise exception
diff --git a/superset/dashboards/commands/update.py b/superset/dashboards/commands/update.py
index 54c5de1..9c885c6 100644
--- a/superset/dashboards/commands/update.py
+++ b/superset/dashboards/commands/update.py
@@ -22,7 +22,7 @@ from flask_appbuilder.security.sqla.models import User
from marshmallow import ValidationError
from superset.commands.base import BaseCommand
-from superset.commands.utils import populate_owners
+from superset.commands.utils import populate_owners, populate_roles
from superset.dao.exceptions import DAOUpdateFailedError
from superset.dashboards.commands.exceptions import (
DashboardForbiddenError,
@@ -58,7 +58,8 @@ class UpdateDashboardCommand(BaseCommand):
def validate(self) -> None:
exceptions: List[ValidationError] = []
- owner_ids: Optional[List[int]] = self._properties.get("owners")
+ owners_ids: Optional[List[int]] = self._properties.get("owners")
+ roles_ids: Optional[List[int]] = self._properties.get("roles")
slug: Optional[str] = self._properties.get("slug")
# Validate/populate model exists
@@ -76,10 +77,10 @@ class UpdateDashboardCommand(BaseCommand):
exceptions.append(DashboardSlugExistsValidationError())
# Validate/Populate owner
- if owner_ids is None:
- owner_ids = [owner.id for owner in self._model.owners]
+ if owners_ids is None:
+ owners_ids = [owner.id for owner in self._model.owners]
try:
- owners = populate_owners(self._actor, owner_ids)
+ owners = populate_owners(self._actor, owners_ids)
self._properties["owners"] = owners
except ValidationError as ex:
exceptions.append(ex)
@@ -87,3 +88,16 @@ class UpdateDashboardCommand(BaseCommand):
exception = DashboardInvalidError()
exception.add_list(exceptions)
raise exception
+
+ # Validate/Populate role
+ if roles_ids is None:
+ roles_ids = [role.id for role in self._model.roles]
+ try:
+ roles = populate_roles(roles_ids)
+ self._properties["roles"] = roles
+ except ValidationError as ex:
+ exceptions.append(ex)
+ if exceptions:
+ exception = DashboardInvalidError()
+ exception.add_list(exceptions)
+ raise exception
diff --git a/superset/dashboards/schemas.py b/superset/dashboards/schemas.py
index e3c4313..17e46d0 100644
--- a/superset/dashboards/schemas.py
+++ b/superset/dashboards/schemas.py
@@ -38,6 +38,12 @@ owners_description = (
"Owner are users ids allowed to delete or change this dashboard. "
"If left empty you will be one of the owners of the dashboard."
)
+roles_description = (
+ "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."
+)
position_json_description = (
"This json object describes the positioning of the widgets "
"in the dashboard. It is dynamically generated when "
@@ -136,6 +142,7 @@ class DashboardPostSchema(BaseDashboardSchema):
description=slug_description, allow_none=True, validate=[Length(1, 255)]
)
owners = fields.List(fields.Integer(description=owners_description))
+ roles = fields.List(fields.Integer(description=roles_description))
position_json = fields.String(
description=position_json_description, validate=validate_json
)
@@ -158,6 +165,7 @@ class DashboardPutSchema(BaseDashboardSchema):
owners = fields.List(
fields.Integer(description=owners_description, allow_none=True)
)
+ roles = fields.List(fields.Integer(description=roles_description, allow_none=True))
position_json = fields.String(
description=position_json_description, allow_none=True, validate=validate_json
)
diff --git a/superset/security/manager.py b/superset/security/manager.py
index 8f05358..2bf8cd0 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -27,6 +27,7 @@ from flask_appbuilder.security.sqla.models import (
assoc_permissionview_role,
assoc_user_role,
PermissionView,
+ Role,
User,
)
from flask_appbuilder.security.views import (
@@ -59,6 +60,7 @@ if TYPE_CHECKING:
from superset.sql_parse import Table
from superset.viz import BaseViz
+
logger = logging.getLogger(__name__)
@@ -656,6 +658,13 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
role_from_permissions.append(pvm)
return role_from_permissions
+ def find_roles_by_id(self, role_ids: List[int]) -> List[Role]:
+ """
+ Find a List of models by a list of ids, if defined applies `base_filter`
+ """
+ query = self.get_session.query(Role).filter(Role.id.in_(role_ids))
+ return query.all()
+
def copy_role(
self, role_from_name: str, role_to_name: str, merge: bool = True
) -> None:
diff --git a/tests/base_tests.py b/tests/base_tests.py
index a5fd9d5..a55302d 100644
--- a/tests/base_tests.py
+++ b/tests/base_tests.py
@@ -182,6 +182,15 @@ class SupersetTestCase(TestCase):
)
return user
+ @staticmethod
+ def get_role(name: str) -> Optional[ab_models.User]:
+ user = (
+ db.session.query(security_manager.role_model)
+ .filter_by(name=name)
+ .one_or_none()
+ )
+ return user
+
@classmethod
def create_druid_test_objects(cls):
# create druid cluster and druid datasources
diff --git a/tests/dashboards/api_tests.py b/tests/dashboards/api_tests.py
index 0baea27..232c680 100644
--- a/tests/dashboards/api_tests.py
+++ b/tests/dashboards/api_tests.py
@@ -71,6 +71,7 @@ class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin):
dashboard_title: str,
slug: Optional[str],
owners: List[int],
+ roles: List[int] = [],
created_by=None,
slices: Optional[List[Slice]] = None,
position_json: str = "",
@@ -79,14 +80,19 @@ class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin):
published: bool = False,
) -> Dashboard:
obj_owners = list()
+ obj_roles = list()
slices = slices or []
for owner in owners:
user = db.session.query(security_manager.user_model).get(owner)
obj_owners.append(user)
+ for role in roles:
+ role_obj = db.session.query(security_manager.role_model).get(role)
+ obj_roles.append(role_obj)
dashboard = Dashboard(
dashboard_title=dashboard_title,
slug=slug,
owners=obj_owners,
+ roles=obj_roles,
position_json=position_json,
css=css,
json_metadata=json_metadata,
@@ -151,7 +157,9 @@ class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin):
Dashboard API: Test get dashboard
"""
admin = self.get_user("admin")
- dashboard = self.insert_dashboard("title", "slug1", [admin.id], admin)
+ dashboard = self.insert_dashboard(
+ "title", "slug1", [admin.id], created_by=admin
+ )
self.login(username="admin")
uri = f"api/v1/dashboard/{dashboard.id}"
rv = self.get_assert_metric(uri, "get")
@@ -174,6 +182,7 @@ class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin):
"last_name": "user",
}
],
+ "roles": [],
"position_json": "",
"published": False,
"url": "/superset/dashboard/slug1/",
@@ -842,6 +851,19 @@ class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin):
expected_response = {"message": {"owners": ["Owners are invalid"]}}
self.assertEqual(response, expected_response)
+ def test_create_dashboard_validate_roles(self):
+ """
+ Dashboard API: Test create validate roles
+ """
+ dashboard_data = {"dashboard_title": "title1", "roles": [1000]}
+ self.login(username="admin")
+ uri = "api/v1/dashboard/"
+ rv = self.client.post(uri, json=dashboard_data)
+ self.assertEqual(rv.status_code, 422)
+ response = json.loads(rv.data.decode("utf-8"))
+ expected_response = {"message": {"roles": ["Some roles do not exist"]}}
+ self.assertEqual(response, expected_response)
+
def test_create_dashboard_validate_json(self):
"""
Dashboard API: Test create validate json
@@ -872,7 +894,10 @@ class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin):
Dashboard API: Test update
"""
admin = self.get_user("admin")
- dashboard_id = self.insert_dashboard("title1", "slug1", [admin.id]).id
+ admin_role = self.get_role("Admin")
+ dashboard_id = self.insert_dashboard(
+ "title1", "slug1", [admin.id], roles=[admin_role.id]
+ ).id
self.login(username="admin")
uri = f"api/v1/dashboard/{dashboard_id}"
rv = self.put_assert_metric(uri, self.dashboard_data, "put")
@@ -885,6 +910,7 @@ class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin):
self.assertEqual(model.json_metadata, self.dashboard_data["json_metadata"])
self.assertEqual(model.published, self.dashboard_data["published"])
self.assertEqual(model.owners, [admin])
+ self.assertEqual(model.roles, [admin_role])
db.session.delete(model)
db.session.commit()