feat: security, user group support

pyproject.toml

@@ -44,7 +44,7 @@ dependencies = [
     "cryptography>=42.0.4, <45.0.0",
     "deprecation>=2.1.0, <2.2.0",
     "flask>=2.2.5, <3.0.0",
-    "flask-appbuilder>=4.5.4, <5.0.0",
+    "flask-appbuilder>=4.6.0, <5.0.0",
     "flask-caching>=2.1.0, <3",
     "flask-compress>=1.13, <2.0",
     "flask-talisman>=1.0.0, <2.0",

requirements/base.txt

@@ -108,7 +108,7 @@ flask==2.3.3
     #   flask-session
     #   flask-sqlalchemy
     #   flask-wtf
-flask-appbuilder==4.5.4
+flask-appbuilder==4.6.0
     # via apache-superset (pyproject.toml)
 flask-babel==2.0.0
     # via flask-appbuilder

requirements/development.txt

@@ -190,7 +190,7 @@ flask==2.3.3
     #   flask-sqlalchemy
     #   flask-testing
     #   flask-wtf
-flask-appbuilder==4.5.4
+flask-appbuilder==4.6.0
     # via
     #   -c requirements/base.txt
     #   apache-superset

superset/security/guest_token.py

@@ -16,7 +16,7 @@
 # under the License.
 from typing import Optional, TypedDict, Union
 
-from flask_appbuilder.security.sqla.models import Role
+from flask_appbuilder.security.sqla.models import Group, Role
 from flask_login import AnonymousUserMixin
 
 from superset.utils.backports import StrEnum
@@ -84,5 +84,6 @@ def __init__(self, token: GuestToken, roles: list[Role]):
         self.first_name = user.get("first_name", "Guest")
         self.last_name = user.get("last_name", "User")
         self.roles = roles
+        self.groups: list[Group] = []  # Guest users don't belong to any groups
         self.resources = token["resources"]
         self.rls = token.get("rls_rules", [])

superset/security/manager.py

@@ -27,7 +27,9 @@
 from flask_appbuilder import Model
 from flask_appbuilder.security.sqla.manager import SecurityManager
 from flask_appbuilder.security.sqla.models import (
+    assoc_group_role,
     assoc_permissionview_role,
+    assoc_user_group,
     assoc_user_role,
     Permission,
     PermissionView,
@@ -51,6 +53,7 @@
 from sqlalchemy.orm import eagerload
 from sqlalchemy.orm.mapper import Mapper
 from sqlalchemy.orm.query import Query as SqlaQuery
+from sqlalchemy.sql import exists
 
 from superset.constants import RouteMethod
 from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
@@ -236,8 +239,10 @@ class SupersetSecurityManager(  # pylint: disable=too-many-public-methods
         "Log",
         "List Users",
         "List Roles",
+        "List Groups",
         "ResetPasswordView",
         "RoleModelView",
+        "UserGroupModelView",
         "Row Level Security",
         "Row Level Security Filters",
         "RowLevelSecurityFiltersModelView",
@@ -733,13 +738,24 @@ def user_view_menu_names(self, permission_name: str) -> set[str]:
         )
 
         if not g.user.is_anonymous:
-            # filter by user id
-            view_menu_names = (
-                base_query.join(assoc_user_role)
-                .join(self.user_model)
-                .filter(self.user_model.id == get_user_id())
-                .filter(self.permission_model.name == permission_name)
-            ).all()
+            user_id = get_user_id()
+
+            user_roles_filter = or_(
+                exists().where(
+                    (assoc_user_role.c.user_id == user_id)
+                    & (assoc_user_role.c.role_id == self.role_model.id)
+                    & (self.permission_model.name == permission_name)
+                ),
+                exists().where(
+                    (assoc_user_group.c.user_id == user_id)
+                    & (assoc_user_group.c.group_id == self.group_model.id)
+                    & (assoc_group_role.c.group_id == self.group_model.id)
+                    & (assoc_group_role.c.role_id == self.role_model.id)
+                    & (self.permission_model.name == permission_name)
+                ),
+            )
+
+            view_menu_names = base_query.filter(user_roles_filter).all()
             return {s.name for s in view_menu_names}
 
         # Properly treat anonymous user
@@ -2424,7 +2440,7 @@ def get_user_roles(self, user: Optional[User] = None) -> list[Role]:
         if user.is_anonymous:
             public_role = current_app.config.get("AUTH_ROLE_PUBLIC")
             return [self.get_public_role()] if public_role else []
-        return user.roles
+        return super().get_user_roles(user)
 
     def get_guest_rls_filters(
         self, dataset: "BaseDatasource"

superset/views/utils.py

@@ -103,8 +103,8 @@ def bootstrap_user_data(user: User, include_perms: bool = False) -> dict[str, An
 def get_permissions(
     user: User,
 ) -> tuple[dict[str, list[tuple[str]]], DefaultDict[str, list[str]]]:
-    if not user.roles:
-        raise AttributeError("User object does not have roles")
+    if not user.roles and not user.groups:
+        raise AttributeError("User object does not have roles or groups")
 
     data_permissions = defaultdict(set)
     roles_permissions = security_manager.get_user_roles_permissions(user)

tests/integration_tests/charts/commands_tests.py

@@ -104,11 +104,13 @@ def test_export_chart_command(self, mock_g):
             "query_context": None,
         }
 
+    @patch("superset.utils.core.g")
     @patch("superset.security.manager.g")
     @pytest.mark.usefixtures("load_energy_table_with_slice")
-    def test_export_chart_command_no_access(self, mock_g):
+    def test_export_chart_command_no_access(self, utils_mock_g, manager_mock_g):
         """Test that users can't export datasets they don't have access to"""
-        mock_g.user = security_manager.find_user("gamma")
+        manager_mock_g.user = security_manager.find_user("gamma")
+        utils_mock_g.user = manager_mock_g.user
 
         example_chart = db.session.query(Slice).all()[0]
         command = ExportChartsCommand([example_chart.id])

tests/integration_tests/databases/api_tests.py

@@ -78,6 +78,9 @@
     load_unicode_dashboard_with_position,  # noqa: F401
     load_unicode_data,  # noqa: F401
 )
+from tests.integration_tests.fixtures.users import (
+    create_gamma_user_group_with_all_database,  # noqa: F401
+)
 from tests.integration_tests.test_app import app
 
 
@@ -259,6 +262,18 @@ def test_get_items_not_allowed(self):
         response = json.loads(rv.data.decode("utf-8"))
         assert response["count"] == 0
 
+    @pytest.mark.usefixtures("create_gamma_user_group_with_all_database")
+    def test_get_items_gamma_group(self):
+        """
+        Database API: Test get items gamma with group
+        """
+        self.login("gamma_with_groups", "password1")
+        uri = "api/v1/database/"
+        rv = self.client.get(uri)
+        assert rv.status_code == 200
+        response = json.loads(rv.data.decode("utf-8"))
+        assert response["count"] > 0
+
     def test_create_database(self):
         """
         Database API: Test create
@@ -2340,6 +2355,17 @@ def test_get_database_related_objects_not_found(self):
         rv = self.get_assert_metric(uri, "related_objects")
         assert rv.status_code == 404
 
+    @pytest.mark.usefixtures("create_gamma_user_group_with_all_database")
+    def test_get_database_related_objects_gamma_group(self):
+        """
+        Database API: Test related objects with gamma group with role all database
+        """
+        database = get_example_database()
+        self.login("gamma_with_groups", "password1")
+        uri = f"api/v1/database/{database.id}/related_objects/"
+        rv = self.get_assert_metric(uri, "related_objects")
+        assert rv.status_code == 200
+
     def test_export_database(self):
         """
         Database API: Test export database

tests/integration_tests/fixtures/users.py

@@ -22,6 +22,86 @@
 from tests.integration_tests.constants import GAMMA_SQLLAB_NO_DATA_USERNAME
 
 
+def create_role_with_permissions(role_name: str, permissions: list[tuple[str, str]]):
+    pvm_list = [
+        security_manager.add_permission_view_menu(p[0], p[1]) for p in permissions
+    ]
+    return security_manager.add_role(role_name, pvm_list)
+
+
+def create_user_and_group(
+    group_name: str,
+    username: str,
+    roles: list[Role],
+    password: str = "password1",  # noqa: S107
+):
+    group = security_manager.add_group(group_name, "", "", roles=roles)
+    user = security_manager.add_user(
+        username,
+        "gamma",
+        "user",
+        username,
+        password=password,  # noqa: S106
+        role=[],
+        groups=[group],
+    )
+    return user, group
+
+
+def cleanup(user, group):
+    security_manager.get_session.delete(user)
+    security_manager.get_session.delete(group)
+    security_manager.get_session.commit()
+
+
+@pytest.fixture
+def create_gamma_user_group(app_context: AppContext):
+    gamma_role = security_manager.find_role("Gamma")
+    user, group = create_user_and_group("group1", "gamma_with_groups", [gamma_role])
+    yield
+    cleanup(user, group)
+
+
+@pytest.fixture
+def create_user_group_with_dar(app_context: AppContext):
+    dar_role = create_role_with_permissions(
+        "dar", [("datasource_access", "[examples].[birth_names](id:1)]")]
+    )
+    user, group = create_user_and_group("group1", "gamma_with_groups", [dar_role])
+    yield
+    cleanup(user, group)
+
+
+@pytest.fixture
+def create_gamma_user_group_with_dar(app_context: AppContext):
+    dar_role = create_role_with_permissions(
+        "dar",
+        [
+            ("datasource_access", "[examples].[birth_names](id:1)]"),
+            ("all_database_access", "all_database_access"),
+        ],
+    )
+    gamma_role = security_manager.find_role("Gamma")
+    user, group = create_user_and_group(
+        "group1", "gamma_with_groups", [dar_role, gamma_role]
+    )
+    yield
+    cleanup(user, group)
+
+
+@pytest.fixture
+def create_gamma_user_group_with_all_database(app_context: AppContext):
+    dar_role = create_role_with_permissions(
+        "dar", [("all_database_access", "all_database_access")]
+    )
+    gamma_role = security_manager.find_role("Gamma")
+    user, group = create_user_and_group(
+        "group1", "gamma_with_groups", [dar_role, gamma_role]
+    )
+    yield
+    cleanup(user, group)
+
+
 @pytest.fixture
 def create_gamma_sqllab_no_data(app_context: AppContext):
     gamma_role = db.session.query(Role).filter(Role.name == "Gamma").one_or_none()

tests/integration_tests/security_tests.py

@@ -63,6 +63,11 @@
     load_world_bank_dashboard_with_slices,  # noqa: F401
     load_world_bank_data,  # noqa: F401
 )
+from tests.integration_tests.fixtures.users import (
+    create_gamma_user_group,  # noqa: F401
+    create_user_group_with_dar,  # noqa: F401
+    create_gamma_user_group_with_dar,  # noqa: F401
+)
 
 NEW_SECURITY_CONVERGE_VIEWS = (
     "Annotation",
@@ -1871,12 +1876,58 @@ def test_raise_for_access_rbac(
                     }
                 )
 
-    def test_get_user_roles(self):
+    def test_get_admin_user_roles(self):
         admin = security_manager.find_user("admin")
         with override_user(admin):
             roles = security_manager.get_user_roles()
             assert admin.roles == roles
 
+    def test_get_gamma_user_roles(self):
+        admin = security_manager.find_user("gamma")
+        with override_user(admin):
+            roles = security_manager.get_user_roles()
+            assert admin.roles == roles
+
+    @pytest.mark.usefixtures("create_gamma_user_group")
+    def test_get_user_roles_with_groups(self):
+        user = security_manager.find_user("gamma_with_groups")
+        with override_user(user):
+            roles = security_manager.get_user_roles()
+            assert user.groups[0].roles == roles
+
+    @pytest.mark.usefixtures("create_gamma_user_group_with_dar")
+    def test_get_user_roles_with_groups_dar(self):
+        user = security_manager.find_user("gamma_with_groups")
+        with override_user(user):
+            role_names = [role.name for role in security_manager.get_user_roles()]
+            assert "Gamma" in role_names
+            assert "dar" in role_names
+            assert len(role_names) == 2
+
+    @pytest.mark.usefixtures("create_user_group_with_dar")
+    def test_user_view_menu_names_with_groups_dar(self):
+        user = security_manager.find_user("gamma_with_groups")
+        with override_user(user):
+            assert security_manager.user_view_menu_names("datasource_access") == {
+                "[examples].[birth_names](id:1)]"
+            }
+
+    @pytest.mark.usefixtures("create_gamma_user_group_with_dar")
+    def test_gamma_user_view_menu_names_with_groups_dar(self):
+        user = security_manager.find_user("gamma_with_groups")
+        with override_user(user):
+            # assert pvm for dar role
+            assert security_manager.user_view_menu_names("datasource_access") == {
+                "[examples].[birth_names](id:1)]"
+            }
+            # assert pvm for gamma role
+            assert security_manager.user_view_menu_names("can_external_metadata") == {
+                "Datasource"
+            }
+            assert security_manager.user_view_menu_names("can_recent_activity") == {
+                "Log"
+            }
+
     def test_get_anonymous_roles(self):
         with override_user(security_manager.get_anonymous_user()):
             roles = security_manager.get_user_roles()