feat(rbac): agent RBAC/ACL scenes and resource-locator check bundles (#41768)

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

api/tests/unit_tests/controllers/console/workspace/test_endpoint.py

@@ -6,6 +6,7 @@ from unittest.mock import patch
 import pytest
 from flask import Flask
 
+from controllers.common.rbac import Workspace
 from controllers.console import console_ns
 from controllers.console.workspace.endpoint import (
     DeprecatedEndpointCreateApi,
@@ -24,10 +25,11 @@ from controllers.console.workspace.endpoint import (
     EndpointUpdatePayload,
     LegacyEndpointUpdatePayload,
 )
-from controllers.console.wraps import RBACPermission, RBACResourceScope
+from controllers.console.wraps import RBACPermission
 from core.entities.provider_entities import ProviderConfig, ProviderConfigType
 from core.plugin.entities.endpoint import EndpointEntityWithInstance, EndpointProviderDeclaration
 from core.plugin.impl.exc import PluginPermissionDeniedError
+from tests.unit_tests.controllers.rbac_introspection import rbac_checks
 
 
 def _endpoint_entity() -> EndpointEntityWithInstance:
@@ -65,13 +67,9 @@ def test_endpoint_lists_require_management_permission(method: FunctionType) -> N
     )
     assert "is_admin_or_owner_required" in legacy_wrapper.__code__.co_qualname
 
-    rbac_wrapper = inspect.unwrap(
-        method, stop=lambda wrapper: "rbac_permission_required" in wrapper.__code__.co_qualname
-    )
-    rbac_config = inspect.getclosurevars(rbac_wrapper).nonlocals
-    assert rbac_config["resource_type"] == RBACResourceScope.WORKSPACE
-    assert rbac_config["scene"] == RBACPermission.PLUGIN_MODEL_CONFIG
-    assert rbac_config["resource_required"] is False
+    [check] = rbac_checks(method)
+    assert check.scene == RBACPermission.PLUGIN_MODEL_CONFIG
+    assert isinstance(check.locator, Workspace)
 
 
 class TestEndpointCollectionApi:

api/tests/unit_tests/controllers/console/workspace/test_rbac.py

@@ -85,6 +85,25 @@ class TestMyPermissions:
         assert response["app"]["default_permission_keys"] == ["app.acl.deploy"]
         mock_get.assert_called_once()
 
+    def test_forwards_agent_id_query_param(self, app):
+        permissions = rbac_mod.svc.MyPermissionsResponse(
+            agent=rbac_mod.svc.ResourcePermissionSnapshot(
+                default_permission_keys=["agent.acl.preview"],
+            )
+        )
+        with (
+            app.test_request_context("/workspaces/current/rbac/my-permissions?agent_id=agent-1"),
+            patch("controllers.console.workspace.rbac._current_ids", return_value=("tenant-1", "acct-1")),
+            patch(
+                "controllers.console.workspace.rbac.svc.RBACService.MyPermissions.get",
+                return_value=permissions,
+            ) as mock_get,
+        ):
+            response = inspect.unwrap(rbac_mod.RBACMyPermissionsApi.get)(rbac_mod.RBACMyPermissionsApi())
+
+        assert mock_get.call_args.kwargs["agent_id"] == "agent-1"
+        assert response["agent"]["default_permission_keys"] == ["agent.acl.preview"]
+
 
 class TestAccessMatrixAccountNames:
     def test_hydrates_missing_account_names(self):
@@ -332,172 +351,6 @@ class TestPaginationMapping:
         assert options.reverse is True
 
 
-class TestResourceAccessScopeBindings:
-    def test_app_whitelist_all_schedules_member_policy_sync(self, app):
-        with (
-            app.test_request_context(
-                "/workspaces/current/rbac/apps/app-1/whitelist",
-                method="PUT",
-                json={"automatic_include_workspace_members": True},
-            ),
-            patch("controllers.console.workspace.rbac._current_ids", return_value=("tenant-1", "acct-actor")),
-            patch(
-                "controllers.console.workspace.rbac.svc.RBACService.AppAccess.replace_whitelist",
-                return_value=rbac_mod.svc.ResourceWhitelist(automatic_include_workspace_members=True),
-            ),
-            patch("controllers.console.workspace.rbac.initialize_created_app_rbac_access_task") as mock_sync_task,
-        ):
-            inspect.unwrap(rbac_mod.RBACAppWhitelistApi.put)(rbac_mod.RBACAppWhitelistApi(), "app-1")
-
-        mock_sync_task.delay.assert_called_once_with("tenant-1", "acct-actor", app_id="app-1")
-
-    def test_dataset_whitelist_all_schedules_member_policy_sync(self, app):
-        # Widening a dataset to the whole workspace only records the scope; without granting the
-        # default policy to the current members nobody actually gains access.
-        with (
-            app.test_request_context(
-                "/workspaces/current/rbac/datasets/dataset-1/whitelist",
-                method="PUT",
-                json={"automatic_include_workspace_members": True},
-            ),
-            patch("controllers.console.workspace.rbac._current_ids", return_value=("tenant-1", "acct-actor")),
-            patch(
-                "controllers.console.workspace.rbac.svc.RBACService.DatasetAccess.replace_whitelist",
-                return_value=rbac_mod.svc.ResourceWhitelist(automatic_include_workspace_members=True),
-            ),
-            patch("controllers.console.workspace.rbac.initialize_created_app_rbac_access_task") as mock_sync_task,
-        ):
-            inspect.unwrap(rbac_mod.RBACDatasetWhitelistApi.put)(rbac_mod.RBACDatasetWhitelistApi(), "dataset-1")
-
-        mock_sync_task.delay.assert_called_once_with("tenant-1", "acct-actor", dataset_id="dataset-1")
-
-    def test_dataset_whitelist_specific_does_not_schedule_member_policy_sync(self, app):
-        with (
-            app.test_request_context(
-                "/workspaces/current/rbac/datasets/dataset-1/whitelist",
-                method="PUT",
-                json={"automatic_include_workspace_members": False},
-            ),
-            patch("controllers.console.workspace.rbac._current_ids", return_value=("tenant-1", "acct-actor")),
-            patch(
-                "controllers.console.workspace.rbac.svc.RBACService.DatasetAccess.replace_whitelist",
-                return_value=rbac_mod.svc.ResourceWhitelist(automatic_include_workspace_members=False),
-            ),
-            patch("controllers.console.workspace.rbac.initialize_created_app_rbac_access_task") as mock_sync_task,
-        ):
-            inspect.unwrap(rbac_mod.RBACDatasetWhitelistApi.put)(rbac_mod.RBACDatasetWhitelistApi(), "dataset-1")
-
-        mock_sync_task.delay.assert_not_called()
-
-    def test_app_whitelist_config_returns_switch_state_only(self, app):
-        result = rbac_mod.svc.ResourceWhitelistConfig(automatic_include_workspace_members=True)
-        with (
-            app.test_request_context("/workspaces/current/rbac/apps/app-1/whitelist_config"),
-            patch("controllers.console.workspace.rbac._current_ids", return_value=("tenant-1", "acct-1")),
-            patch(
-                "controllers.console.workspace.rbac.svc.RBACService.AppAccess.whitelist_config",
-                return_value=result,
-            ) as mock_get,
-        ):
-            response = inspect.unwrap(rbac_mod.RBACAppWhitelistConfigApi.get)(
-                rbac_mod.RBACAppWhitelistConfigApi(),
-                "app-1",
-            )
-
-        assert response == {"automatic_include_workspace_members": True}
-        mock_get.assert_called_once_with("tenant-1", "acct-1", "app-1")
-
-    def test_dataset_whitelist_config_returns_switch_state_only(self, app):
-        result = rbac_mod.svc.ResourceWhitelistConfig(automatic_include_workspace_members=False)
-        with (
-            app.test_request_context("/workspaces/current/rbac/datasets/dataset-1/whitelist_config"),
-            patch("controllers.console.workspace.rbac._current_ids", return_value=("tenant-1", "acct-1")),
-            patch(
-                "controllers.console.workspace.rbac.svc.RBACService.DatasetAccess.whitelist_config",
-                return_value=result,
-            ) as mock_get,
-        ):
-            response = inspect.unwrap(rbac_mod.RBACDatasetWhitelistConfigApi.get)(
-                rbac_mod.RBACDatasetWhitelistConfigApi(),
-                "dataset-1",
-            )
-
-        assert response == {"automatic_include_workspace_members": False}
-        mock_get.assert_called_once_with("tenant-1", "acct-1", "dataset-1")
-
-    def test_app_user_access_policy_assignment_forwards_ids(self, app):
-        with (
-            app.test_request_context(
-                "/workspaces/current/rbac/apps/app-1/users/acct-target/access-policies",
-                method="PUT",
-                json={"access_policy_ids": ["policy-1", "policy-2"]},
-            ),
-            patch("controllers.console.workspace.rbac._current_ids", return_value=("tenant-1", "acct-actor")),
-            patch(
-                "controllers.console.workspace.rbac.svc.RBACService.AppAccess.replace_user_access_policies"
-            ) as mock_replace,
-            patch("controllers.console.workspace.rbac._dump", return_value={}),
-        ):
-            inspect.unwrap(rbac_mod.RBACAppUserAccessPolicyAssignmentApi.put)(
-                rbac_mod.RBACAppUserAccessPolicyAssignmentApi(),
-                "app-1",
-                "acct-target",
-            )
-
-        tenant_id, actor_id, app_id, target_id, payload = mock_replace.call_args.args
-        assert (tenant_id, actor_id, app_id, target_id) == (
-            "tenant-1",
-            "acct-actor",
-            "app-1",
-            "acct-target",
-        )
-        assert payload.access_policy_ids == ["policy-1", "policy-2"]
-
-    def test_app_member_bindings_delete_forwards_account_ids(self, app):
-        with (
-            app.test_request_context(
-                "/workspaces/current/rbac/apps/app-1/access-policies/policy-1/member-bindings",
-                method="DELETE",
-                json={"account_ids": ["acct-2", "acct-3"]},
-            ),
-            patch("controllers.console.workspace.rbac._current_ids", return_value=("tenant-1", "acct-actor")),
-            patch("controllers.console.workspace.rbac.svc.RBACService.AppAccess.delete_member_bindings") as mock_delete,
-        ):
-            response = inspect.unwrap(rbac_mod.RBACAppMemberBindingsApi.delete)(
-                rbac_mod.RBACAppMemberBindingsApi(),
-                "app-1",
-                "policy-1",
-            )
-
-        assert response == {"result": "success"}
-        tenant_id, actor_id, app_id, policy_id, payload = mock_delete.call_args.args
-        assert (tenant_id, actor_id, app_id, policy_id) == ("tenant-1", "acct-actor", "app-1", "policy-1")
-        assert payload.account_ids == ["acct-2", "acct-3"]
-
-    def test_dataset_member_bindings_delete_forwards_account_ids(self, app):
-        with (
-            app.test_request_context(
-                "/workspaces/current/rbac/datasets/dataset-1/access-policies/policy-1/member-bindings",
-                method="DELETE",
-                json={"account_ids": ["acct-2"]},
-            ),
-            patch("controllers.console.workspace.rbac._current_ids", return_value=("tenant-1", "acct-actor")),
-            patch(
-                "controllers.console.workspace.rbac.svc.RBACService.DatasetAccess.delete_member_bindings"
-            ) as mock_delete,
-        ):
-            response = inspect.unwrap(rbac_mod.RBACDatasetMemberBindingsApi.delete)(
-                rbac_mod.RBACDatasetMemberBindingsApi(),
-                "dataset-1",
-                "policy-1",
-            )
-
-        assert response == {"result": "success"}
-        tenant_id, actor_id, dataset_id, policy_id, payload = mock_delete.call_args.args
-        assert (tenant_id, actor_id, dataset_id, policy_id) == ("tenant-1", "acct-actor", "dataset-1", "policy-1")
-        assert payload.account_ids == ["acct-2"]
-
-
 class TestPaginationForwarding:
     def test_access_policies_get_forwards_outer_pagination_params(self, app):
         with (
@@ -517,88 +370,6 @@ class TestPaginationForwarding:
         assert options.results_per_page == 25
         assert options.reverse is False
 
-    def test_workspace_app_matrix_forwards_outer_pagination_params(self, app):
-        with (
-            app.test_request_context("/workspaces/current/rbac/workspace/apps/access-policy?page=4&limit=10"),
-            patch("controllers.console.workspace.rbac._current_ids", return_value=("tenant-1", "acct-1")),
-            patch("controllers.console.workspace.rbac.svc.RBACService.WorkspaceAccess.app_matrix") as mock_list,
-            patch("controllers.console.workspace.rbac._dump", return_value={}),
-        ):
-            inspect.unwrap(rbac_mod.RBACWorkspaceAppMatrixApi.get)(rbac_mod.RBACWorkspaceAppMatrixApi())
-
-        _, kwargs = mock_list.call_args
-        options = kwargs["options"]
-        assert options.page_number == 4
-        assert options.results_per_page == 10
-        assert options.reverse is None
-
-    def test_workspace_dataset_matrix_forwards_outer_pagination_params(self, app):
-        with (
-            app.test_request_context(
-                "/workspaces/current/rbac/workspace/datasets/access-policy?page=5&limit=15&reverse=true"
-            ),
-            patch("controllers.console.workspace.rbac._current_ids", return_value=("tenant-1", "acct-1")),
-            patch("controllers.console.workspace.rbac.svc.RBACService.WorkspaceAccess.dataset_matrix") as mock_list,
-            patch("controllers.console.workspace.rbac._dump", return_value={}),
-        ):
-            inspect.unwrap(rbac_mod.RBACWorkspaceDatasetMatrixApi.get)(rbac_mod.RBACWorkspaceDatasetMatrixApi())
-
-        _, kwargs = mock_list.call_args
-        options = kwargs["options"]
-        assert options.page_number == 5
-        assert options.results_per_page == 15
-        assert options.reverse is True
-
-    def test_app_user_access_policies_forwards_outer_pagination_params(self, app):
-        result = rbac_mod.svc.ResourceUserAccessPoliciesResponse()
-        with (
-            app.test_request_context(
-                "/workspaces/current/rbac/apps/app-1/user-access-policies?page=2&limit=30&reverse=false"
-            ),
-            patch("controllers.console.workspace.rbac._current_ids", return_value=("tenant-1", "acct-1")),
-            patch(
-                "controllers.console.workspace.rbac.svc.RBACService.AppAccess.user_access_policies",
-                return_value=result,
-            ) as mock_list,
-            patch("controllers.console.workspace.rbac._dump", return_value={}),
-        ):
-            inspect.unwrap(rbac_mod.RBACAppUserAccessPoliciesApi.get)(
-                rbac_mod.RBACAppUserAccessPoliciesApi(),
-                "app-1",
-            )
-
-        args, kwargs = mock_list.call_args
-        assert args == ("tenant-1", "acct-1", "app-1")
-        options = kwargs["options"]
-        assert options.page_number == 2
-        assert options.results_per_page == 30
-        assert options.reverse is False
-
-    def test_dataset_user_access_policies_forwards_outer_pagination_params(self, app):
-        result = rbac_mod.svc.ResourceUserAccessPoliciesResponse()
-        with (
-            app.test_request_context(
-                "/workspaces/current/rbac/datasets/dataset-1/user-access-policies?page=3&limit=40&reverse=true"
-            ),
-            patch("controllers.console.workspace.rbac._current_ids", return_value=("tenant-1", "acct-1")),
-            patch(
-                "controllers.console.workspace.rbac.svc.RBACService.DatasetAccess.user_access_policies",
-                return_value=result,
-            ) as mock_list,
-            patch("controllers.console.workspace.rbac._dump", return_value={}),
-        ):
-            inspect.unwrap(rbac_mod.RBACDatasetUserAccessPoliciesApi.get)(
-                rbac_mod.RBACDatasetUserAccessPoliciesApi(),
-                "dataset-1",
-            )
-
-        args, kwargs = mock_list.call_args
-        assert args == ("tenant-1", "acct-1", "dataset-1")
-        options = kwargs["options"]
-        assert options.page_number == 3
-        assert options.results_per_page == 40
-        assert options.reverse is True
-
 
 class TestAccessPolicyBindingLockUnlock:
     def test_lock_forwards_binding_id(self, app):
@@ -653,7 +424,7 @@ class TestWorkspaceRbacGuards:
                 "controllers.common.wraps.current_account_with_tenant",
                 return_value=(_account(), "tenant-1"),
             ),
-            patch("controllers.common.wraps.RBACService.CheckAccess.check", return_value=False),
+            patch("controllers.common.rbac.checks.RBACService.CheckAccess.check", return_value=False),
             patch("controllers.console.workspace.rbac.svc.RBACService.Roles.create") as mock_create,
         ):
             with pytest.raises(Forbidden):
@@ -672,7 +443,7 @@ class TestWorkspaceRbacGuards:
                 "controllers.common.wraps.current_account_with_tenant",
                 return_value=(_account(), "tenant-1"),
             ),
-            patch("controllers.common.wraps.RBACService.CheckAccess.check", return_value=False),
+            patch("controllers.common.rbac.checks.RBACService.CheckAccess.check", return_value=False),
             patch("controllers.console.workspace.rbac.svc.RBACService.AccessPolicies.create") as mock_create,
         ):
             with pytest.raises(Forbidden):

api/tests/unit_tests/controllers/console/workspace/test_rbac_resource_access_routes.py

@@ -0,0 +1,167 @@
+"""Contract tests for the generated app / dataset / agent access-permission routes.
+
+``controllers.console.workspace.rbac`` builds the same twelve endpoints for every
+resource kind from ``_RESOURCE_ACCESS_ROUTES``. These tests pin the two things the
+generation must not get wrong: the registered URLs (with their class names and HTTP
+methods) and which inner-API client each handler reaches for.
+"""
+
+from __future__ import annotations
+
+import inspect
+from collections.abc import Callable, Generator
+from unittest.mock import MagicMock, patch
+
+import pytest
+from flask import Flask
+
+from controllers.console import console_ns
+from controllers.console.workspace import rbac as rbac_mod
+from enums import DeploymentEdition
+from services.enterprise import rbac_service as svc
+
+RESOURCE_ID = {
+    svc.RBACResourceType.APP: "app-1",
+    svc.RBACResourceType.DATASET: "dataset-1",
+    svc.RBACResourceType.AGENT: "agent-1",
+}
+
+# The URL segment and path parameter of each resource kind, spelled out here instead of read
+# back from the same enum the controller builds its URLs from, so a rename fails this test.
+RESOURCE_URL_PARTS = {
+    svc.RBACResourceType.APP: ("apps", "app_id"),
+    svc.RBACResourceType.DATASET: ("datasets", "dataset_id"),
+    svc.RBACResourceType.AGENT: ("agents", "agent_id"),
+}
+
+
+def _segment(spec: rbac_mod._ResourceAccessRoutes) -> str:
+    return RESOURCE_URL_PARTS[spec.resource_type][0]
+
+
+def _id_param(spec: rbac_mod._ResourceAccessRoutes) -> str:
+    return RESOURCE_URL_PARTS[spec.resource_type][1]
+
+
+@pytest.fixture
+def app() -> Flask:
+    flask_app = Flask(__name__)
+    flask_app.config["TESTING"] = True
+    return flask_app
+
+
+@pytest.fixture(autouse=True)
+def _rbac_config(config_overrides: Callable[..., None]) -> None:
+    config_overrides(
+        DEPLOYMENT_EDITION=DeploymentEdition.ENTERPRISE,
+        RBAC_ENABLED=True,
+        LOGIN_DISABLED=True,
+    )
+
+
+@pytest.fixture(params=rbac_mod._RESOURCE_ACCESS_ROUTES, ids=lambda spec: spec.resource_type.value)
+def spec(request: pytest.FixtureRequest) -> rbac_mod._ResourceAccessRoutes:
+    return request.param
+
+
+@pytest.fixture
+def apis(spec: rbac_mod._ResourceAccessRoutes) -> rbac_mod._ResourceAccessApis:
+    return rbac_mod._RESOURCE_ACCESS_APIS[spec.resource_type]
+
+
+@pytest.fixture
+def resource_id(spec: rbac_mod._ResourceAccessRoutes) -> str:
+    return RESOURCE_ID[spec.resource_type]
+
+
+@pytest.fixture(autouse=True)
+def _patched_current_ids() -> Generator[None]:
+    with patch("controllers.console.workspace.rbac._current_ids", return_value=("tenant-1", "acct-actor")):
+        yield
+
+
+def _expected_routes(spec: rbac_mod._ResourceAccessRoutes) -> dict[str, tuple[str, set[str]]]:
+    resource = f"/workspaces/current/rbac/{_segment(spec)}/<uuid:{_id_param(spec)}>"
+    workspace = f"/workspaces/current/rbac/workspace/{_segment(spec)}"
+    prefix = spec.class_prefix
+    return {
+        f"/workspaces/current/rbac/role-permissions/catalog/{spec.resource_type.value}": (
+            f"RBAC{prefix}CatalogApi",
+            {"GET"},
+        ),
+        f"{resource}/access-policy": (f"RBAC{prefix}MatrixApi", {"GET"}),
+        f"{resource}/whitelist": (f"RBAC{prefix}WhitelistApi", {"GET", "PUT"}),
+        f"{resource}/whitelist_config": (f"RBAC{prefix}WhitelistConfigApi", {"GET"}),
+        f"{resource}/user-access-policies": (f"RBAC{prefix}UserAccessPoliciesApi", {"GET"}),
+        f"{resource}/users/<uuid:target_account_id>/access-policies": (
+            f"RBAC{prefix}UserAccessPolicyAssignmentApi",
+            {"PUT"},
+        ),
+        f"{resource}/access-policies/<uuid:policy_id>/role-bindings": (f"RBAC{prefix}RoleBindingsApi", {"GET"}),
+        f"{resource}/access-policies/<string:policy_id>/member-bindings": (
+            f"RBAC{prefix}MemberBindingsApi",
+            {"GET", "DELETE"},
+        ),
+        f"{workspace}/access-policy": (f"RBACWorkspace{prefix}MatrixApi", {"GET"}),
+        f"{workspace}/access-policies/<uuid:policy_id>/role-bindings": (
+            f"RBACWorkspace{prefix}RoleBindingsApi",
+            {"GET"},
+        ),
+        f"{workspace}/access-policies/<uuid:policy_id>/bindings": (f"RBACWorkspace{prefix}BindingsApi", {"PUT"}),
+        f"{workspace}/access-policies/<uuid:policy_id>/member-bindings": (
+            f"RBACWorkspace{prefix}MemberBindingsApi",
+            {"GET"},
+        ),
+    }
+
+
+def _registered_resource_by_url() -> dict[str, type]:
+    registered: dict[str, type] = {}
+    for route in console_ns.resources:
+        for url in route.urls:
+            registered[url] = route.resource
+    return registered
+
+
+def test_every_resource_kind_registers_the_same_twelve_routes(spec: rbac_mod._ResourceAccessRoutes) -> None:
+    registered = _registered_resource_by_url()
+
+    for url, (class_name, methods) in _expected_routes(spec).items():
+        assert url in registered, f"{url} is not registered"
+        resource = registered[url]
+        assert resource.__name__ == class_name
+        assert set(resource.methods or ()) == methods
+
+
+@pytest.mark.parametrize("automatic_include_workspace_members", [True, False])
+def test_whitelist_put_queues_the_seed_task_only_when_auto_including(
+    app: Flask,
+    apis: rbac_mod._ResourceAccessApis,
+    spec: rbac_mod._ResourceAccessRoutes,
+    resource_id: str,
+    automatic_include_workspace_members: bool,
+) -> None:
+    replace = MagicMock(
+        return_value=svc.ResourceWhitelist(
+            automatic_include_workspace_members=automatic_include_workspace_members,
+        )
+    )
+    with (
+        app.test_request_context(
+            f"/{_segment(spec)}/{resource_id}/whitelist",
+            method="PUT",
+            json={"automatic_include_workspace_members": automatic_include_workspace_members},
+        ),
+        patch.object(spec.access, "replace_whitelist", replace),
+        patch("controllers.console.workspace.rbac.initialize_created_app_rbac_access_task") as seed_task,
+    ):
+        inspect.unwrap(vars(apis.whitelist)["put"])(apis.whitelist(), **{_id_param(spec): resource_id})
+
+    tenant_id, actor_id, target_id, payload = replace.call_args.args
+    assert (tenant_id, actor_id, target_id) == ("tenant-1", "acct-actor", resource_id)
+    assert payload.automatic_include_workspace_members is automatic_include_workspace_members
+
+    if automatic_include_workspace_members:
+        seed_task.delay.assert_called_once_with("tenant-1", "acct-actor", **{_id_param(spec): resource_id})
+    else:
+        seed_task.delay.assert_not_called()