From c83dcce1f7fd12883270a69b55d9afd01628dbbc Mon Sep 17 00:00:00 2001 From: wangxiaolei Date: Mon, 22 Jun 2026 14:57:56 +0800 Subject: [PATCH] chore: remove duplicate code (#37724) --- api/controllers/console/workspace/rbac.py | 36 +------- api/services/enterprise/rbac_service.py | 82 ++++++++++++++++--- .../console/workspace/test_rbac.py | 21 +---- .../services/enterprise/test_rbac_service.py | 37 ++++++++- 4 files changed, 109 insertions(+), 67 deletions(-) diff --git a/api/controllers/console/workspace/rbac.py b/api/controllers/console/workspace/rbac.py index 1b213a4f741..f672833061a 100644 --- a/api/controllers/console/workspace/rbac.py +++ b/api/controllers/console/workspace/rbac.py @@ -211,7 +211,7 @@ def _legacy_workspace_roles( name=role_name, description="", is_builtin=True, - permission_keys=list(_LEGACY_ROLE_PERMISSION_KEYS[role_name]), + permission_keys=list(dict.fromkeys(_LEGACY_ROLE_PERMISSION_KEYS[role_name])), role_tag="owner" if role_name == "owner" else "", ) for role_name in ("owner", "admin", "editor", "normal", "dataset_operator") @@ -244,11 +244,6 @@ def _legacy_workspace_roles( ) -# --------------------------------------------------------------------------- -# Permission catalogs. -# --------------------------------------------------------------------------- - - @console_ns.route("/workspaces/current/rbac/role-permissions/catalog") class RBACWorkspaceCatalogApi(Resource): @login_required @@ -375,30 +370,6 @@ class RBACRoleCopyApi(Resource): return _dump(role), 201 -@console_ns.route("/workspaces/current/rbac/roles//members") -class RBACRoleMembersApi(Resource): - @login_required - @rbac_permission_required( - RBACResourceScope.WORKSPACE, RBACPermission.WORKSPACE_ROLE_MANAGE, resource_required=False - ) - @console_ns.response(200, "Success", console_ns.models[_RBACRoleAccountList.__name__]) - def get(self, role_id): - tenant_id, account_id = _current_ids() - return _dump( - svc.RBACService.Roles.members( - tenant_id, - account_id, - str(role_id), - options=_pagination_options(), - ) - ) - - -# --------------------------------------------------------------------------- -# Access policies (tenant-level permission sets). -# --------------------------------------------------------------------------- - - class _AccessPolicyCreateRequest(BaseModel): name: str resource_type: svc.RBACResourceType @@ -788,11 +759,6 @@ class RBACDatasetMemberBindingsApi(Resource): return {"result": "success"} -# --------------------------------------------------------------------------- -# Workspace-level access (Settings > Access Rules). -# --------------------------------------------------------------------------- - - @console_ns.route("/workspaces/current/rbac/workspace/apps/access-policy") class RBACWorkspaceAppMatrixApi(Resource): @login_required diff --git a/api/services/enterprise/rbac_service.py b/api/services/enterprise/rbac_service.py index c32a5759105..32cf69ce99e 100644 --- a/api/services/enterprise/rbac_service.py +++ b/api/services/enterprise/rbac_service.py @@ -313,12 +313,20 @@ _LEGACY_WORKSPACE_OWNER_KEYS: list[str] = [ "plugin.debug", "credential.use", "credential.manage", + "billing.view", + "billing.subscription.manage", + "billing.manage", + "app.acl.preview", "app_library.access", "app.create_and_management", "app.tag.manage", + "dataset.acl.preview", "dataset.create_and_management", "dataset.tag.manage", "dataset.external.connect", + "dataset.api_key.manage", + "snippets.create_and_modify", + "snippets.management", "tool.manage", "mcp.manage", "snippets.create_and_modify", @@ -337,12 +345,18 @@ _LEGACY_WORKSPACE_ADMIN_KEYS: list[str] = [ "plugin.debug", "credential.use", "credential.manage", + "billing.view", + "billing.subscription.manage", + "billing.manage", "app_library.access", "app.create_and_management", "app.tag.manage", "dataset.create_and_management", "dataset.tag.manage", "dataset.external.connect", + "dataset.api_key.manage", + "snippets.create_and_modify", + "snippets.management", "tool.manage", "mcp.manage", "snippets.create_and_modify", @@ -360,6 +374,7 @@ _LEGACY_WORKSPACE_EDITOR_KEYS: list[str] = [ "dataset.create_and_management", "dataset.tag.manage", "dataset.external.connect", + "snippets.create_and_modify", "tool.manage", "snippets.create_and_modify", ] @@ -378,6 +393,7 @@ _LEGACY_WORKSPACE_DATASET_OPERATOR_KEYS: list[str] = [ ] _LEGACY_APP_OWNER_KEYS: list[str] = [ + "app.acl.preview", "app.acl.view_layout", "app.acl.test_and_run", "app.acl.edit", @@ -389,6 +405,7 @@ _LEGACY_APP_OWNER_KEYS: list[str] = [ ] _LEGACY_APP_ADMIN_KEYS: list[str] = [ + "app.acl.preview", "app.acl.view_layout", "app.acl.test_and_run", "app.acl.edit", @@ -400,6 +417,7 @@ _LEGACY_APP_ADMIN_KEYS: list[str] = [ ] _LEGACY_APP_EDITOR_KEYS: list[str] = [ + "app.acl.preview", "app.acl.view_layout", "app.acl.test_and_run", "app.acl.edit", @@ -411,12 +429,14 @@ _LEGACY_APP_EDITOR_KEYS: list[str] = [ ] _LEGACY_APP_NORMAL_KEYS: list[str] = [ + "app.acl.preview", "app.acl.view_layout", "app.acl.test_and_run", "app.acl.monitor", ] _LEGACY_DATASET_OWNER_KEYS: list[str] = [ + "dataset.acl.preview", "dataset.acl.readonly", "dataset.acl.edit", "dataset.acl.import_export_dsl", @@ -432,6 +452,7 @@ _LEGACY_DATASET_OWNER_KEYS: list[str] = [ ] _LEGACY_DATASET_ADMIN_KEYS: list[str] = [ + "dataset.acl.preview", "dataset.acl.readonly", "dataset.acl.edit", "dataset.acl.import_export_dsl", @@ -447,6 +468,7 @@ _LEGACY_DATASET_ADMIN_KEYS: list[str] = [ ] _LEGACY_DATASET_EDITOR_KEYS: list[str] = [ + "dataset.acl.preview", "dataset.acl.readonly", "dataset.acl.edit", "dataset.acl.import_export_dsl", @@ -497,6 +519,19 @@ _LEGACY_MY_PERMISSIONS: dict[TenantAccountRole, dict[str, list[str]]] = { } +def _legacy_role_permission_keys(role: TenantAccountRole) -> list[str]: + permissions = _LEGACY_MY_PERMISSIONS.get(role, {}) + return list( + dict.fromkeys( + [ + *permissions.get("workspace", []), + *permissions.get("app", []), + *permissions.get("dataset", []), + ] + ) + ) + + def _legacy_my_permissions(tenant_id: str, account_id: str | None) -> MyPermissionsResponse: if not account_id: return MyPermissionsResponse() @@ -1523,21 +1558,44 @@ class RBACService: ) return AccessMatrixItem.model_validate(data or {}) - # ------------------------------------------------------------------ - # Member ↔ role bindings (screenshot 3: Settings > Members > Assign roles). - # ------------------------------------------------------------------ class MemberRoles: @staticmethod def get(tenant_id: str, account_id: str | None, member_account_id: str) -> MemberRolesResponse: - data = _inner_call( - "GET", - f"{_INNER_PREFIX}/members/rbac-roles", - tenant_id=tenant_id, - account_id=account_id, - params={"account_id": member_account_id}, - ) - rst = MemberRolesResponse.model_validate(data or {}) - return rst + if dify_config.RBAC_ENABLED: + data = _inner_call( + "GET", + f"{_INNER_PREFIX}/members/rbac-roles", + tenant_id=tenant_id, + account_id=account_id, + params={"account_id": member_account_id}, + ) + rst = MemberRolesResponse.model_validate(data or {}) + return rst + else: + with session_factory.create_session() as session: + role = session.scalar( + select(TenantAccountJoin.role).where( + TenantAccountJoin.tenant_id == tenant_id, + TenantAccountJoin.account_id == member_account_id, + ) + ) + return MemberRolesResponse( + account_id=member_account_id, + roles=[ + RBACRole( + id="", + name=role, + description="", + is_builtin=True, + type="", + permission_keys=_legacy_role_permission_keys(role), + role_tag="owner" if role == "owner" else role, + tenant_id=tenant_id, + ) + ] + if role + else [], + ) @staticmethod def batch_get( diff --git a/api/tests/unit_tests/controllers/console/workspace/test_rbac.py b/api/tests/unit_tests/controllers/console/workspace/test_rbac.py index 1ad9637b7bb..d78bc1fc6dd 100644 --- a/api/tests/unit_tests/controllers/console/workspace/test_rbac.py +++ b/api/tests/unit_tests/controllers/console/workspace/test_rbac.py @@ -185,7 +185,7 @@ class TestPaginationMapping: "name": "owner", "description": "", "is_builtin": True, - "permission_keys": list(rbac_mod._LEGACY_ROLE_PERMISSION_KEYS["owner"]), + "permission_keys": list(dict.fromkeys(rbac_mod._LEGACY_ROLE_PERMISSION_KEYS["owner"])), "role_tag": "owner", }, { @@ -196,7 +196,7 @@ class TestPaginationMapping: "name": "admin", "description": "", "is_builtin": True, - "permission_keys": list(rbac_mod._LEGACY_ROLE_PERMISSION_KEYS["admin"]), + "permission_keys": list(dict.fromkeys(rbac_mod._LEGACY_ROLE_PERMISSION_KEYS["admin"])), "role_tag": "", }, ] @@ -336,23 +336,6 @@ class TestResourceAccessScopeBindings: class TestPaginationForwarding: - def test_role_members_get_forwards_outer_pagination_params(self, app): - with ( - app.test_request_context("/workspaces/current/rbac/roles/role-1/members?page=2&limit=50&reverse=true"), - patch("controllers.console.workspace.rbac._current_ids", return_value=("tenant-1", "acct-1")), - patch("controllers.console.workspace.rbac.svc.RBACService.Roles.members") as mock_members, - patch("controllers.console.workspace.rbac._dump", return_value={}), - ): - inspect.unwrap(rbac_mod.RBACRoleMembersApi.get)(rbac_mod.RBACRoleMembersApi(), "role-1") - - _, _, role_id = mock_members.call_args.args - _, kwargs = mock_members.call_args - assert role_id == "role-1" - options = kwargs["options"] - assert options.page_number == 2 - assert options.results_per_page == 50 - assert options.reverse is True - def test_access_policies_get_forwards_outer_pagination_params(self, app): with ( app.test_request_context( 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 dfd5662cf3e..35f0d3ac674 100644 --- a/api/tests/unit_tests/services/enterprise/test_rbac_service.py +++ b/api/tests/unit_tests/services/enterprise/test_rbac_service.py @@ -620,6 +620,13 @@ class TestMyPermissions: assert out.dataset.default_permission_keys == dataset_keys assert out.app.overrides == [] assert out.dataset.overrides == [] + if role == "owner": + assert "billing.view" in out.workspace.permission_keys + assert "snippets.management" in out.workspace.permission_keys + assert "app.acl.preview" in out.workspace.permission_keys + assert "dataset.acl.preview" in out.workspace.permission_keys + assert "app.acl.preview" in out.app.default_permission_keys + assert "dataset.acl.preview" in out.dataset.default_permission_keys @pytest.mark.parametrize( ("role", "expected_snippet_keys"), @@ -700,7 +707,8 @@ class TestMemberRoles: } ], } - out = svc.RBACService.MemberRoles.get("tenant-1", "acct-1", "acct-2") + with patch(f"{MODULE}.dify_config.RBAC_ENABLED", True): + out = svc.RBACService.MemberRoles.get("tenant-1", "acct-1", "acct-2") call = _call_args(mock_send) assert call.method == "GET" assert call.endpoint == "/rbac/members/rbac-roles" @@ -708,6 +716,33 @@ 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 + + with ( + patch(f"{MODULE}.dify_config.RBAC_ENABLED", False), + patch(f"{MODULE}.session_factory.create_session") as create_session, + ): + create_session.return_value.__enter__.return_value = session + out = svc.RBACService.MemberRoles.get("tenant-1", "acct-1", "acct-2") + + mock_send.assert_not_called() + assert out.account_id == "acct-2" + assert out.roles[0].name == "editor" + assert out.roles[0].permission_keys == list( + dict.fromkeys( + [ + *svc._LEGACY_WORKSPACE_EDITOR_KEYS, + *svc._LEGACY_APP_EDITOR_KEYS, + *svc._LEGACY_DATASET_EDITOR_KEYS, + ] + ) + ) + assert "snippets.create_and_modify" in out.roles[0].permission_keys + 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): mock_send.return_value = {"account_id": "acct-2", "roles": []} svc.RBACService.MemberRoles.replace(