diff --git a/api/apps/restful_apis/agent_api.py b/api/apps/restful_apis/agent_api.py index 3745460f42e..c0c6c604af7 100644 --- a/api/apps/restful_apis/agent_api.py +++ b/api/apps/restful_apis/agent_api.py @@ -72,7 +72,27 @@ def _require_canvas_access_sync(func): @wraps(func) def wrapper(*args, **kwargs): if not UserCanvasService.accessible(kwargs.get('agent_id'), kwargs.get('tenant_id')): - return get_json_result(data=False, message="Only owner of canvas authorized for this operation.", code=RetCode.OPERATING_ERROR) + return get_json_result(data=False, message="Make sure you have permission to access the agent.", code=RetCode.OPERATING_ERROR) + return func(*args, **kwargs) + return wrapper + + +def _require_canvas_access_async(func): + @wraps(func) + async def wrapper(*args, **kwargs): + agent_id = kwargs.get('agent_id') + tenant_id = kwargs.get('tenant_id') + if not await thread_pool_exec(UserCanvasService.accessible, agent_id, tenant_id): + return get_json_result(data=False, message="Make sure you have permission to access the agent.", code=RetCode.OPERATING_ERROR) + return await func(*args, **kwargs) + return wrapper + + +def _require_canvas_owner_sync(func): + @wraps(func) + def wrapper(*args, **kwargs): + if not UserCanvasService.query(user_id=kwargs.get('tenant_id'), id=kwargs.get('agent_id')): + return get_json_result(data=False, message="Only the owner of the agent is authorized for this operation.", code=RetCode.OPERATING_ERROR) return func(*args, **kwargs) return wrapper @@ -172,6 +192,7 @@ def list_agent_sessions(agent_id, tenant_id): @manager.route("/agents//sessions", methods=["POST"]) # noqa: F821 @login_required @add_tenant_id_to_kwargs +@_require_canvas_access_async async def create_agent_session(agent_id, tenant_id): req = await get_request_json() user_id = req.get("user_id") or request.args.get("user_id", tenant_id) @@ -422,18 +443,12 @@ async def upload_agent_file(agent_id): @manager.route("/agents//components//input-form", methods=["GET"]) # noqa: F821 @login_required @add_tenant_id_to_kwargs +@_require_canvas_access_sync def get_agent_component_input_form(agent_id, component_id, tenant_id): try: exists, user_canvas = UserCanvasService.get_by_id(agent_id) if not exists: return get_data_error_result(message="canvas not found.") - if not UserCanvasService.query(user_id=tenant_id, id=agent_id): - return get_json_result( - data=False, - message="Only owner of canvas authorized for this operation.", - code=RetCode.OPERATING_ERROR, - ) - canvas = Canvas(json.dumps(user_canvas.dsl), tenant_id, canvas_id=user_canvas.id) return get_json_result(data=canvas.get_component_input_form(component_id)) except Exception as exc: @@ -444,14 +459,9 @@ def get_agent_component_input_form(agent_id, component_id, tenant_id): @validate_request("params") @login_required @add_tenant_id_to_kwargs +@_require_canvas_access_async async def debug_agent_component(agent_id, component_id, tenant_id): req = await get_request_json() - if not UserCanvasService.accessible(agent_id, tenant_id): - return get_json_result( - data=False, - message="Only owner of canvas authorized for this operation.", - code=RetCode.OPERATING_ERROR, - ) try: _, user_canvas = UserCanvasService.get_by_id(agent_id) canvas = Canvas(json.dumps(user_canvas.dsl), tenant_id, canvas_id=user_canvas.id) @@ -569,14 +579,8 @@ def get_agent_logs(agent_id, message_id, tenant_id): @manager.route("/agents/", methods=["DELETE"]) # noqa: F821 @login_required @add_tenant_id_to_kwargs +@_require_canvas_owner_sync def delete_agent(agent_id, tenant_id): - if not UserCanvasService.query(user_id=tenant_id, id=agent_id): - return get_json_result( - data=False, - message="Only owner of canvas authorized for this operation.", - code=RetCode.OPERATING_ERROR, - ) - UserCanvasService.delete_by_id(agent_id) return get_json_result(data=True) @@ -584,9 +588,9 @@ def delete_agent(agent_id, tenant_id): @manager.route("/agents/", methods=["PUT"]) # noqa: F821 @login_required @add_tenant_id_to_kwargs +@_require_canvas_access_async async def update_agent(agent_id, tenant_id): req = {k: v for k, v in (await get_request_json()).items() if v is not None} - req["user_id"] = tenant_id req["release"] = bool(req.get("release", "")) if req.get("dsl") is not None: @@ -602,13 +606,6 @@ async def update_agent(agent_id, tenant_id): if req.get("title") is not None: req["title"] = req["title"].strip() - if not UserCanvasService.query(user_id=tenant_id, id=agent_id): - return get_json_result( - data=False, - message="Only owner of canvas authorized for this operation.", - code=RetCode.OPERATING_ERROR, - ) - _, current_agent = UserCanvasService.get_by_id(agent_id) agent_title_for_version = req.get("title") or (current_agent.title if current_agent else "") canvas_category = ( @@ -642,14 +639,8 @@ async def update_agent(agent_id, tenant_id): @manager.route("/agents//reset", methods=["POST"]) # noqa: F821 @login_required @add_tenant_id_to_kwargs +@_require_canvas_access_async async def reset_agent(agent_id, tenant_id): - if not UserCanvasService.accessible(agent_id, tenant_id): - return get_json_result( - data=False, - message="Only owner of canvas authorized for this operation.", - code=RetCode.OPERATING_ERROR, - ) - try: exists, user_canvas = UserCanvasService.get_by_id(agent_id) if not exists: @@ -911,10 +902,11 @@ async def agent_chat_completion(tenant_id, agent_id=None): runtime_user_id = req.get("user_id") or tenant_id user_id = str(runtime_user_id) custom_header = req.get("custom_header", "") - if not await thread_pool_exec(UserCanvasService.accessible, agent_id, tenant_id): + + if not UserCanvasService.accessible(agent_id, tenant_id): return get_json_result( data=False, - message="Only owner of canvas authorized for this operation.", + message="Make sure you have permission to access the agent.", code=RetCode.OPERATING_ERROR, ) diff --git a/api/db/services/canvas_service.py b/api/db/services/canvas_service.py index ec79bf81881..4a5734e155d 100644 --- a/api/db/services/canvas_service.py +++ b/api/db/services/canvas_service.py @@ -221,8 +221,6 @@ def get_agent_dsl_with_release(cls, agent_id, release_mode=False, tenant_id=None e, cvs = cls.get_by_id(agent_id) if not e: raise LookupError("Agent not found.") - if tenant_id and cvs.user_id != tenant_id: - raise PermissionError("You do not own the agent.") if release_mode: released_version = UserCanvasVersionService.get_latest_released(agent_id) diff --git a/test/testcases/test_http_api/test_session_management/test_agent_sessions.py b/test/testcases/test_http_api/test_session_management/test_agent_sessions.py index 6672a04bd73..7d47954573f 100644 --- a/test/testcases/test_http_api/test_session_management/test_agent_sessions.py +++ b/test/testcases/test_http_api/test_session_management/test_agent_sessions.py @@ -108,8 +108,8 @@ def test_agent_crud_validation_contract(self, HttpApiAuth, agent_id): update_url = f"{HOST_ADDRESS}/api/{VERSION}/agents/invalid-agent-id" res = requests.put(update_url, auth=HttpApiAuth, json={"title": "updated", "dsl": MINIMAL_DSL}).json() assert res["code"] == 103, res - assert "Only owner of canvas authorized" in res["message"], res + assert "Make sure you have permission to access the agent." in res["message"], res res = delete_agent(HttpApiAuth, "invalid-agent-id") assert res["code"] == 103, res - assert "Only owner of canvas authorized" in res["message"], res + assert "Only the owner of the agent is authorized for this operation." in res["message"], res diff --git a/test/testcases/test_web_api/test_agent_app/test_agents_webhook_unit.py b/test/testcases/test_web_api/test_agent_app/test_agents_webhook_unit.py index b1f7b6c4a88..1022a9b45a0 100644 --- a/test/testcases/test_web_api/test_agent_app/test_agents_webhook_unit.py +++ b/test/testcases/test_web_api/test_agent_app/test_agents_webhook_unit.py @@ -568,12 +568,17 @@ async def req_update(): return {"dsl": {"nodes": []}, "title": " webhook-agent ", "unused": None} monkeypatch.setattr(module, "get_request_json", req_update) - monkeypatch.setattr(module.UserCanvasService, "query", lambda **_kwargs: False) - res = _run(module.update_agent.__wrapped__("agent-1", "tenant-1")) + monkeypatch.setattr(module.UserCanvasService, "accessible", lambda *_a, **_kw: False) + + @module._require_canvas_access_async + async def _dummy_update(agent_id, tenant_id): + return module.get_json_result(data=True) + + res = _run(_dummy_update(agent_id="agent-1", tenant_id="tenant-1")) assert res["code"] == module.RetCode.OPERATING_ERROR calls = {"update": 0, "save_or_replace_latest": 0, "replace_for_set": 0} - monkeypatch.setattr(module.UserCanvasService, "query", lambda **_kwargs: True) + monkeypatch.setattr(module.UserCanvasService, "accessible", lambda *_a, **_kw: True) monkeypatch.setattr( module.UserCanvasService, "get_by_id", @@ -599,7 +604,12 @@ async def req_update(): assert calls == {"update": 1, "save_or_replace_latest": 1, "replace_for_set": 1} monkeypatch.setattr(module.UserCanvasService, "query", lambda **_kwargs: False) - res = module.delete_agent.__wrapped__("agent-1", "tenant-1") + + @module._require_canvas_owner_sync + def _dummy_delete(agent_id, tenant_id): + return module.get_json_result(data=True) + + res = _dummy_delete(agent_id="agent-1", tenant_id="tenant-1") assert res["code"] == module.RetCode.OPERATING_ERROR