feat(dashboard_rbac): dashboards API support for roles create/update + roles validation

superset/commands/exceptions.py

@@ -85,6 +85,13 @@ def __init__(self) -> None:
         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
 

superset/commands/utils.py

@@ -16,38 +16,53 @@
 # 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
 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()
         owners.append(owner)
     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(

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",

superset/dashboards/commands/create.py

@@ -22,7 +22,7 @@
 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 @@ def run(self) -> Model:
     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 @@ def validate(self) -> None:
             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

superset/dashboards/commands/update.py

@@ -22,7 +22,7 @@
 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 @@ def run(self) -> Model:
 
     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,14 +77,27 @@ def validate(self) -> None:
             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)
         if exceptions:
             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

superset/dashboards/schemas.py

@@ -38,6 +38,12 @@
     "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
     )

superset/security/manager.py

@@ -27,6 +27,7 @@
     assoc_permissionview_role,
     assoc_user_role,
     PermissionView,
+    Role,
     User,
 )
 from flask_appbuilder.security.views import (
@@ -59,6 +60,7 @@
     from superset.sql_parse import Table
     from superset.viz import BaseViz
 
+
 logger = logging.getLogger(__name__)
 
 
@@ -656,6 +658,13 @@ def _get_pvms_from_builtin_role(self, role_name: str) -> List[PermissionView]:
                         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:

tests/base_tests.py

@@ -182,6 +182,15 @@ def get_user(username: str) -> ab_models.User:
         )
         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

tests/dashboards/api_tests.py

@@ -71,6 +71,7 @@ def insert_dashboard(
         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 @@ def insert_dashboard(
         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 @@ def test_get_dashboard(self):
         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 @@ def test_get_dashboard(self):
                     "last_name": "user",
                 }
             ],
+            "roles": [],
             "position_json": "",
             "published": False,
             "url": "/superset/dashboard/slug1/",
@@ -863,6 +872,19 @@ def test_create_dashboard_validate_owners(self):
         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
@@ -893,7 +915,10 @@ def test_update_dashboard(self):
         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")
@@ -906,6 +931,7 @@ def test_update_dashboard(self):
         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()