From cb2b36f1aac5a1c6b44d9b00cd4cd073366e64f8 Mon Sep 17 00:00:00 2001 From: Asuka Minato Date: Wed, 22 Jul 2026 11:13:52 +0900 Subject: [PATCH] test: use SQLite sessions in services enterprise (#39113) --- .../services/enterprise/test_rbac_service.py | 163 +++++++++++------- 1 file changed, 98 insertions(+), 65 deletions(-) diff --git a/api/tests/unit_tests/services/enterprise/test_rbac_service.py b/api/tests/unit_tests/services/enterprise/test_rbac_service.py index 4c8b779491c..27f240797c1 100644 --- a/api/tests/unit_tests/services/enterprise/test_rbac_service.py +++ b/api/tests/unit_tests/services/enterprise/test_rbac_service.py @@ -1,11 +1,9 @@ """Unit tests for services.enterprise.rbac_service. -The enterprise RBAC client is almost pure glue: each method turns a single -``EnterpriseRequest.send_inner_rbac_request`` call into a pydantic response -model. Rather than spinning up an HTTP server we monkeypatch that helper and -assert on the arguments it received; that catches both routing regressions -(wrong method / wrong path / wrong params) and model-shape regressions in -one place. +Most enterprise RBAC methods turn a single ``EnterpriseRequest.send_inner_rbac_request`` +call into a pydantic response model. Rather than spinning up an HTTP server, these tests +monkeypatch that helper and assert on the request arguments and response shape. The legacy +fallbacks use SQLite to verify their database reads and committed role updates. """ from __future__ import annotations @@ -15,7 +13,10 @@ from unittest.mock import MagicMock, patch import pytest from flask import Flask +from sqlalchemy import select +from sqlalchemy.orm import Session +from models import TenantAccountJoin from services.enterprise import rbac_service as svc MODULE = "services.enterprise.rbac_service" @@ -533,8 +534,9 @@ class TestWorkspaceAccess: assert call.params == {"language": "en"} +@pytest.mark.parametrize("sqlite_session", [(TenantAccountJoin,)], indirect=True) class TestMyPermissions: - def test_resource_snapshot_maps_defaults_and_overrides(self): + def test_resource_snapshot_maps_defaults_and_overrides(self, sqlite_session: Session): snapshot = svc.ResourcePermissionSnapshot( default_permission_keys=["app.acl.view_layout"], overrides=[ @@ -550,7 +552,7 @@ class TestMyPermissions: "app-2": ["app.acl.view_layout", "app.acl.edit"], } - def test_get_without_payload_uses_get(self, mock_send: MagicMock): + def test_get_without_payload_uses_get(self, mock_send: MagicMock, sqlite_session: Session): mock_send.return_value = { "workspace": {"permission_keys": ["workspace.member.manage"]}, "app": {"default_permission_keys": ["app.acl.view_layout", "app.acl.test_and_run"], "overrides": []}, @@ -558,7 +560,7 @@ class TestMyPermissions: } with patch(f"{MODULE}.dify_config.RBAC_ENABLED", True): - out = svc.RBACService.MyPermissions.get("tenant-1", "acct-1", session=MagicMock()) + out = svc.RBACService.MyPermissions.get("tenant-1", "acct-1", session=sqlite_session) call = _call_args(mock_send) assert call.method == "GET" @@ -609,12 +611,14 @@ class TestMyPermissions: workspace_keys: list[str], app_keys: list[str], dataset_keys: list[str], + sqlite_session: Session, ): - mock_session = MagicMock() - mock_session.__enter__.return_value = mock_session - mock_session.scalar.return_value = role + sqlite_session.add( + TenantAccountJoin(tenant_id="tenant-1", account_id="acct-1", role=svc.TenantAccountRole(role)) + ) + sqlite_session.commit() with patch(f"{MODULE}.dify_config.RBAC_ENABLED", False): - out = svc.RBACService.MyPermissions.get("tenant-1", "acct-1", session=mock_session) + out = svc.RBACService.MyPermissions.get("tenant-1", "acct-1", session=sqlite_session) mock_send.assert_not_called() assert out.workspace.permission_keys == workspace_keys @@ -648,12 +652,14 @@ class TestMyPermissions: mock_send: MagicMock, role: str, expected_snippet_keys: set[str], + sqlite_session: Session, ): - mock_session = MagicMock() - mock_session.__enter__.return_value = mock_session - mock_session.scalar.return_value = role + sqlite_session.add( + TenantAccountJoin(tenant_id="tenant-1", account_id="acct-1", role=svc.TenantAccountRole(role)) + ) + sqlite_session.commit() with patch(f"{MODULE}.dify_config.RBAC_ENABLED", False): - out = svc.RBACService.MyPermissions.get("tenant-1", "acct-1", session=mock_session) + out = svc.RBACService.MyPermissions.get("tenant-1", "acct-1", session=sqlite_session) actual_snippet_keys = { permission_key for permission_key in out.workspace.permission_keys if permission_key.startswith("snippets.") @@ -662,19 +668,16 @@ class TestMyPermissions: mock_send.assert_not_called() assert actual_snippet_keys == expected_snippet_keys - def test_get_returns_empty_when_role_missing_and_rbac_disabled(self, mock_send: MagicMock): - mock_session = MagicMock() - mock_session.__enter__.return_value = mock_session - mock_session.scalar.return_value = None + def test_get_returns_empty_when_role_missing_and_rbac_disabled(self, mock_send: MagicMock, sqlite_session: Session): with patch(f"{MODULE}.dify_config.RBAC_ENABLED", False): - out = svc.RBACService.MyPermissions.get("tenant-1", "acct-1", session=mock_session) + out = svc.RBACService.MyPermissions.get("tenant-1", "acct-1", session=sqlite_session) mock_send.assert_not_called() assert out.workspace.permission_keys == [] assert out.app.default_permission_keys == [] assert out.dataset.default_permission_keys == [] - def test_get_with_single_resource_filters(self, mock_send: MagicMock): + def test_get_with_single_resource_filters(self, mock_send: MagicMock, sqlite_session: Session): mock_send.return_value = { "workspace": {"permission_keys": []}, "app": { @@ -685,7 +688,7 @@ class TestMyPermissions: } with patch(f"{MODULE}.dify_config.RBAC_ENABLED", True): - out = svc.RBACService.MyPermissions.get("tenant-1", "acct-1", app_id="app-1", session=MagicMock()) + out = svc.RBACService.MyPermissions.get("tenant-1", "acct-1", app_id="app-1", session=sqlite_session) call = _call_args(mock_send) assert call.method == "GET" @@ -694,8 +697,9 @@ class TestMyPermissions: assert out.app.overrides[0].resource_id == "app-1" +@pytest.mark.parametrize("sqlite_session", [(TenantAccountJoin,)], indirect=True) class TestMemberRoles: - def test_get(self, mock_send: MagicMock): + def test_get(self, mock_send: MagicMock, sqlite_session: Session): mock_send.return_value = { "account_id": "acct-2", "roles": [ @@ -707,7 +711,7 @@ class TestMemberRoles: ], } with patch(f"{MODULE}.dify_config.RBAC_ENABLED", True): - out = svc.RBACService.MemberRoles.get("tenant-1", "acct-1", "acct-2", session=MagicMock()) + out = svc.RBACService.MemberRoles.get("tenant-1", "acct-1", "acct-2", session=sqlite_session) call = _call_args(mock_send) assert call.method == "GET" assert call.endpoint == "/rbac/members/rbac-roles" @@ -715,12 +719,14 @@ class TestMemberRoles: assert out.account_id == "acct-2" assert out.roles[0].name == "Member" - def test_get_legacy_role_includes_permission_keys(self, mock_send: MagicMock): - session = MagicMock() - session.scalar.return_value = svc.TenantAccountRole.EDITOR + def test_get_legacy_role_includes_permission_keys(self, mock_send: MagicMock, sqlite_session: Session): + sqlite_session.add( + TenantAccountJoin(tenant_id="tenant-1", account_id="acct-2", role=svc.TenantAccountRole.EDITOR) + ) + sqlite_session.commit() with patch(f"{MODULE}.dify_config.RBAC_ENABLED", False): - out = svc.RBACService.MemberRoles.get("tenant-1", "acct-1", "acct-2", session=session) + out = svc.RBACService.MemberRoles.get("tenant-1", "acct-1", "acct-2", session=sqlite_session) mock_send.assert_not_called() assert out.account_id == "acct-2" @@ -738,7 +744,7 @@ class TestMemberRoles: assert "app.acl.preview" in out.roles[0].permission_keys assert "dataset.acl.preview" in out.roles[0].permission_keys - def test_replace(self, mock_send: MagicMock): + def test_replace(self, mock_send: MagicMock, sqlite_session: Session): mock_send.return_value = {"account_id": "acct-2", "roles": []} with patch(f"{MODULE}.dify_config.RBAC_ENABLED", True): svc.RBACService.MemberRoles.replace( @@ -746,7 +752,7 @@ class TestMemberRoles: "acct-1", "acct-2", role_ids=["workspace.owner", "workspace.editor"], - session=MagicMock(), + session=sqlite_session, ) call = _call_args(mock_send) assert call.method == "PUT" @@ -754,43 +760,59 @@ class TestMemberRoles: assert call.params == {"account_id": "acct-2"} assert call.json == {"role_ids": ["workspace.owner", "workspace.editor"]} - def test_replace_updates_legacy_join_role_when_rbac_disabled(self, mock_send: MagicMock): - session = MagicMock() - session.__enter__.return_value = session - target_join = SimpleNamespace(role=svc.TenantAccountRole.NORMAL, account_id="acct-2") - session.scalar.return_value = target_join + def test_replace_commits_legacy_join_role_when_rbac_disabled(self, mock_send: MagicMock, sqlite_session: Session): + target_join = TenantAccountJoin(tenant_id="tenant-1", account_id="acct-2", role=svc.TenantAccountRole.NORMAL) + sqlite_session.add(target_join) + sqlite_session.commit() + target_join_id = target_join.id + engine = sqlite_session.get_bind() with patch(f"{MODULE}.dify_config.RBAC_ENABLED", False): out = svc.RBACService.MemberRoles.replace( - "tenant-1", "acct-1", "acct-2", role_ids=["editor"], session=session + "tenant-1", "acct-1", "acct-2", role_ids=["editor"], session=sqlite_session ) mock_send.assert_not_called() - session.commit.assert_called_once() - assert target_join.role == svc.TenantAccountRole.EDITOR + # Closing the writer rolls back any uncommitted update and prevents its identity map + # from satisfying the verification query. + sqlite_session.close() + with Session(engine) as verification_session: + persisted_join = verification_session.scalar( + select(TenantAccountJoin).where(TenantAccountJoin.id == target_join_id) + ) + assert persisted_join is not None + assert persisted_join.role == svc.TenantAccountRole.EDITOR assert out.account_id == "acct-2" assert out.roles[0].id == "editor" assert "app.acl.preview" in out.roles[0].permission_keys - def test_replace_legacy_owner_demotes_current_owner_when_rbac_disabled(self, mock_send: MagicMock): - session = MagicMock() - session.__enter__.return_value = session - target_join = SimpleNamespace(role=svc.TenantAccountRole.NORMAL, account_id="acct-2") - owner_join = SimpleNamespace(role=svc.TenantAccountRole.OWNER, account_id="acct-owner") - session.scalar.side_effect = [target_join, owner_join] + def test_replace_legacy_owner_demotes_current_owner_when_rbac_disabled( + self, mock_send: MagicMock, sqlite_session: Session + ): + target_join = TenantAccountJoin(tenant_id="tenant-1", account_id="acct-2", role=svc.TenantAccountRole.NORMAL) + owner_join = TenantAccountJoin(tenant_id="tenant-1", account_id="acct-owner", role=svc.TenantAccountRole.OWNER) + sqlite_session.add_all([target_join, owner_join]) + sqlite_session.commit() with patch(f"{MODULE}.dify_config.RBAC_ENABLED", False): out = svc.RBACService.MemberRoles.replace( - "tenant-1", "acct-1", "acct-2", role_ids=["owner"], session=session + "tenant-1", "acct-1", "acct-2", role_ids=["owner"], session=sqlite_session ) mock_send.assert_not_called() - session.commit.assert_called_once() - assert target_join.role == svc.TenantAccountRole.OWNER - assert owner_join.role == svc.TenantAccountRole.ADMIN + persisted_joins = { + join.account_id: join.role + for join in sqlite_session.scalars( + select(TenantAccountJoin).where(TenantAccountJoin.tenant_id == "tenant-1") + ) + } + assert persisted_joins == { + "acct-2": svc.TenantAccountRole.OWNER, + "acct-owner": svc.TenantAccountRole.ADMIN, + } assert out.roles[0].id == "owner" - def test_batch_get(self, mock_send: MagicMock): + def test_batch_get(self, mock_send: MagicMock, sqlite_session: Session): mock_send.return_value = { "acct-2": [ {"id": "role-1", "name": "Admin", "type": "workspace"}, @@ -811,8 +833,9 @@ class TestMemberRoles: assert out[1].roles == [] +@pytest.mark.parametrize("sqlite_session", [(TenantAccountJoin,)], indirect=True) class TestResourcePermissions: - def test_app_permissions_batch_get(self, mock_send: MagicMock): + def test_app_permissions_batch_get(self, mock_send: MagicMock, sqlite_session: Session): mock_send.return_value = { "data": [ {"resource_id": "app-1", "permission_keys": ["app.acl.view_layout", "app.acl.edit"]}, @@ -822,7 +845,7 @@ class TestResourcePermissions: with patch(f"{MODULE}.dify_config.RBAC_ENABLED", True): out = svc.RBACService.AppPermissions.batch_get( - "tenant-1", "acct-1", ["app-1", "app-2"], session=MagicMock() + "tenant-1", "acct-1", ["app-1", "app-2"], session=sqlite_session ) call = _call_args(mock_send) @@ -834,13 +857,16 @@ class TestResourcePermissions: "app-2": [], } - def test_app_permissions_batch_get_uses_legacy_role_permissions_when_rbac_disabled(self, mock_send: MagicMock): - mock_session = MagicMock() - mock_session.__enter__.return_value = mock_session - mock_session.scalar.return_value = "editor" + def test_app_permissions_batch_get_uses_legacy_role_permissions_when_rbac_disabled( + self, mock_send: MagicMock, sqlite_session: Session + ): + sqlite_session.add( + TenantAccountJoin(tenant_id="tenant-1", account_id="acct-1", role=svc.TenantAccountRole.EDITOR) + ) + sqlite_session.commit() with patch(f"{MODULE}.dify_config.RBAC_ENABLED", False): out = svc.RBACService.AppPermissions.batch_get( - "tenant-1", "acct-1", ["app-1", "app-2"], session=mock_session + "tenant-1", "acct-1", ["app-1", "app-2"], session=sqlite_session ) mock_send.assert_not_called() @@ -849,7 +875,7 @@ class TestResourcePermissions: "app-2": svc._LEGACY_APP_EDITOR_KEYS, } - def test_dataset_permissions_batch_get(self, mock_send: MagicMock): + def test_dataset_permissions_batch_get(self, mock_send: MagicMock, sqlite_session: Session): mock_send.return_value = { "data": [ {"resource_id": "ds-1", "permission_keys": ["dataset.acl.readonly"]}, @@ -859,7 +885,7 @@ class TestResourcePermissions: with patch(f"{MODULE}.dify_config.RBAC_ENABLED", True): out = svc.RBACService.DatasetPermissions.batch_get( - "tenant-1", "acct-1", ["ds-1", "ds-2"], session=MagicMock() + "tenant-1", "acct-1", ["ds-1", "ds-2"], session=sqlite_session ) call = _call_args(mock_send) @@ -871,13 +897,20 @@ class TestResourcePermissions: "ds-2": ["dataset.acl.edit"], } - def test_dataset_permissions_batch_get_uses_legacy_role_permissions_when_rbac_disabled(self, mock_send: MagicMock): - mock_session = MagicMock() - mock_session.__enter__.return_value = mock_session - mock_session.scalar.return_value = "dataset_operator" + def test_dataset_permissions_batch_get_uses_legacy_role_permissions_when_rbac_disabled( + self, mock_send: MagicMock, sqlite_session: Session + ): + sqlite_session.add( + TenantAccountJoin( + tenant_id="tenant-1", + account_id="acct-1", + role=svc.TenantAccountRole.DATASET_OPERATOR, + ) + ) + sqlite_session.commit() with patch(f"{MODULE}.dify_config.RBAC_ENABLED", False): out = svc.RBACService.DatasetPermissions.batch_get( - "tenant-1", "acct-1", ["ds-1", "ds-2"], session=mock_session + "tenant-1", "acct-1", ["ds-1", "ds-2"], session=sqlite_session ) mock_send.assert_not_called()