feat(dashboard_rbac): dashboards API support for roles create/update + roles validation (#12865)
This commit is contained in:
parent
1bb305323e
commit
8ccf2e8f1e
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
)
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
Loading…
Reference in New Issue