diff --git a/server/src/agent_control_server/endpoints/control_bindings.py b/server/src/agent_control_server/endpoints/control_bindings.py index 18cb75b4..92798ae1 100644 --- a/server/src/agent_control_server/endpoints/control_bindings.py +++ b/server/src/agent_control_server/endpoints/control_bindings.py @@ -36,13 +36,11 @@ async def _binding_body_context(request: Request) -> dict[str, Any]: - """Surface ``(target_type, target_id)`` to the authorizer's context. + """Surface ``(target_type, target_id)`` to the authorization context. The body-bearing binding endpoints carry the target identifiers in - the request payload. Upstream authorizers that resolve the target's - owning project (e.g., Galileo's ``check_management_access``) need - those identifiers to make a project-level decision; without them the - upstream returns 400. + the request payload. Authorization providers can use those + identifiers when a request needs target-scoped access checks. FastAPI caches the parsed body, so the endpoint's own Pydantic request model still binds normally. @@ -60,13 +58,12 @@ async def _binding_body_context(request: Request) -> dict[str, Any]: async def _binding_list_context(request: Request) -> dict[str, Any]: - """Surface optional target query parameters to the authorizer. + """Surface optional target query parameters to authorization context. When the GET list endpoint is called with ``target_type`` and ``target_id`` query params, the request is target-scoped and the - upstream needs the identifiers to make a project-level decision. - When neither is present the request is namespace-wide and forwards - no target context (upstream may then reject if it requires one). + request context includes those identifiers. When neither is present + the request is namespace-wide and forwards no target context. """ target_type = request.query_params.get("target_type") target_id = request.query_params.get("target_id") diff --git a/server/src/agent_control_server/endpoints/controls.py b/server/src/agent_control_server/endpoints/controls.py index 6208652b..fcb7cb18 100644 --- a/server/src/agent_control_server/endpoints/controls.py +++ b/server/src/agent_control_server/endpoints/controls.py @@ -33,7 +33,7 @@ from sqlalchemy.exc import IntegrityError from sqlalchemy.ext.asyncio import AsyncSession -from ..auth import require_admin_key +from ..auth_framework import Operation, Principal, require_operation from ..db import get_async_db from ..errors import ( APIValidationError, @@ -443,9 +443,11 @@ async def _validate_control_definition( summary="Render a control template preview", response_description="Rendered control preview", ) +# Rendering is part of the authoring flow, so require create access. async def render_control_template( request: RenderControlTemplateRequest, db: AsyncSession = Depends(get_async_db), + _principal: Principal = Depends(require_operation(Operation.CONTROLS_CREATE)), ) -> RenderControlTemplateResponse: """Render a template-backed control without persisting it.""" control_def = await _render_and_validate_template_input( @@ -461,13 +463,14 @@ async def render_control_template( @router.put( "", - dependencies=[Depends(require_admin_key)], response_model=CreateControlResponse, summary="Create a new control", response_description="Created control ID", ) async def create_control( - request: CreateControlRequest, db: AsyncSession = Depends(get_async_db) + request: CreateControlRequest, + db: AsyncSession = Depends(get_async_db), + _principal: Principal = Depends(require_operation(Operation.CONTROLS_CREATE)), ) -> CreateControlResponse: """ Create a new control with a unique name. @@ -549,6 +552,7 @@ async def create_control( summary="Get control definition JSON schema", response_description="JSON schema for ControlDefinition", ) +# Public schema metadata: no tenant state, no auth operation. async def get_control_schema() -> GetControlSchemaResponse: """Return the canonical JSON schema for ControlDefinition.""" return GetControlSchemaResponse( @@ -563,7 +567,9 @@ async def get_control_schema() -> GetControlSchemaResponse: response_description="Control metadata and configuration", ) async def get_control( - control_id: int, db: AsyncSession = Depends(get_async_db) + control_id: int, + db: AsyncSession = Depends(get_async_db), + _principal: Principal = Depends(require_operation(Operation.CONTROLS_READ)), ) -> GetControlResponse: """ Retrieve a control by ID including its name and configuration data. @@ -600,7 +606,9 @@ async def get_control( response_description="Control data payload", ) async def get_control_data( - control_id: int, db: AsyncSession = Depends(get_async_db) + control_id: int, + db: AsyncSession = Depends(get_async_db), + _principal: Principal = Depends(require_operation(Operation.CONTROLS_READ)), ) -> GetControlDataResponse: """ Retrieve the configuration data for a control. @@ -640,6 +648,7 @@ async def list_control_versions( ), limit: int = Query(_DEFAULT_PAGINATION_LIMIT, ge=1, le=_MAX_PAGINATION_LIMIT), db: AsyncSession = Depends(get_async_db), + _principal: Principal = Depends(require_operation(Operation.CONTROLS_READ)), ) -> ListControlVersionsResponse: """List control versions ordered newest-first using cursor-based pagination.""" page = await ControlService(db).list_versions(control_id, cursor=cursor, limit=limit) @@ -673,6 +682,7 @@ async def get_control_version( control_id: int, version_num: int, db: AsyncSession = Depends(get_async_db), + _principal: Principal = Depends(require_operation(Operation.CONTROLS_READ)), ) -> GetControlVersionResponse: """Return a specific control version, including its raw persisted snapshot.""" version = await ControlService(db).get_version_or_404(control_id, version_num) @@ -687,7 +697,6 @@ async def get_control_version( @router.put( "/{control_id}/data", - dependencies=[Depends(require_admin_key)], response_model=SetControlDataResponse, summary="Update control configuration data", response_description="Success confirmation", @@ -696,6 +705,7 @@ async def set_control_data( control_id: int, request: SetControlDataRequest, db: AsyncSession = Depends(get_async_db), + _principal: Principal = Depends(require_operation(Operation.CONTROLS_UPDATE)), ) -> SetControlDataResponse: """ Update the configuration data for a control. @@ -757,8 +767,11 @@ async def set_control_data( summary="Validate control configuration", response_description="Validation result", ) +# Validation uses the authoring path, so require create access. async def validate_control_data( - request: ValidateControlDataRequest, db: AsyncSession = Depends(get_async_db) + request: ValidateControlDataRequest, + db: AsyncSession = Depends(get_async_db), + _principal: Principal = Depends(require_operation(Operation.CONTROLS_CREATE)), ) -> ValidateControlDataResponse: """ Validate control configuration data without saving it. @@ -798,6 +811,7 @@ async def list_controls( execution: str | None = Query(None, description="Filter by execution ('server' or 'sdk')"), tag: str | None = Query(None, description="Filter by tag"), db: AsyncSession = Depends(get_async_db), + _principal: Principal = Depends(require_operation(Operation.CONTROLS_READ)), ) -> ListControlsResponse: """ List all controls with optional filtering and cursor-based pagination. @@ -884,7 +898,6 @@ async def list_controls( @router.delete( "/{control_id}", - dependencies=[Depends(require_admin_key)], response_model=DeleteControlResponse, summary="Delete a control", response_description="Deletion confirmation with dissociation info", @@ -897,6 +910,7 @@ async def delete_control( "If false, fail if control is associated with any policy or agent.", ), db: AsyncSession = Depends(get_async_db), + _principal: Principal = Depends(require_operation(Operation.CONTROLS_DELETE)), ) -> DeleteControlResponse: """ Delete a control by ID. @@ -1035,7 +1049,6 @@ async def delete_control( @router.patch( "/{control_id}", - dependencies=[Depends(require_admin_key)], response_model=PatchControlResponse, summary="Update control metadata", response_description="Updated control information", @@ -1044,6 +1057,7 @@ async def patch_control( control_id: int, request: PatchControlRequest, db: AsyncSession = Depends(get_async_db), + _principal: Principal = Depends(require_operation(Operation.CONTROLS_UPDATE)), ) -> PatchControlResponse: """ Update control metadata (name and/or enabled status). diff --git a/server/src/agent_control_server/main.py b/server/src/agent_control_server/main.py index 76416e04..bc1bf04b 100644 --- a/server/src/agent_control_server/main.py +++ b/server/src/agent_control_server/main.py @@ -273,9 +273,10 @@ async def attach_version_header(request, call_next): # type: ignore[no-untyped- dependencies=[Depends(require_api_key)], ) app.include_router( + # Endpoint dependencies handle auth; this advertises X-API-Key. control_router, prefix=api_v1_prefix, - dependencies=[Depends(require_api_key)], + dependencies=[Depends(get_api_key_from_header)], ) app.include_router( # The auth framework on each endpoint owns authentication and @@ -300,9 +301,10 @@ async def attach_version_header(request, call_next): # type: ignore[no-untyped- dependencies=[Depends(get_api_key_from_header)], ) app.include_router( + # Endpoint dependencies handle auth; this advertises X-API-Key. control_template_router, prefix=api_v1_prefix, - dependencies=[Depends(require_api_key)], + dependencies=[Depends(get_api_key_from_header)], ) app.include_router( evaluation_router, diff --git a/server/tests/test_controls_auth.py b/server/tests/test_controls_auth.py new file mode 100644 index 00000000..1a2af21f --- /dev/null +++ b/server/tests/test_controls_auth.py @@ -0,0 +1,314 @@ +"""HTTP-level auth coverage for ``/controls`` and ``/control-templates``.""" + +from __future__ import annotations + +import uuid + +import pytest +from fastapi.testclient import TestClient + +from agent_control_server.config import auth_settings + +from .utils import VALID_CONTROL_PAYLOAD + + +_CONTROLS_URL = "/api/v1/controls" +_TEMPLATES_URL = "/api/v1/control-templates" + + +def _create_control(client: TestClient, name: str | None = None) -> int: + payload = { + "name": name or f"control-{uuid.uuid4().hex[:12]}", + "data": VALID_CONTROL_PAYLOAD, + } + resp = client.put(_CONTROLS_URL, json=payload) + assert resp.status_code == 200, resp.text + return int(resp.json()["control_id"]) + + +# --------------------------------------------------------------------------- +# /controls/schema is intentionally public metadata. +# --------------------------------------------------------------------------- + + +def test_schema_endpoint_reachable_without_credentials( + unauthenticated_client: TestClient, +) -> None: + # Given: a client that never sends an API key + # When: the schema endpoint is fetched + resp = unauthenticated_client.get(f"{_CONTROLS_URL}/schema") + + # Then: the canonical ControlDefinition schema is returned + assert resp.status_code == 200, resp.text + body = resp.json() + assert "schema" in body + assert isinstance(body["schema"], dict) + + +def test_schema_endpoint_reachable_with_admin_key(client: TestClient) -> None: + # Given: an admin client + # When: the schema endpoint is fetched + resp = client.get(f"{_CONTROLS_URL}/schema") + + # Then: the schema is returned (header is ignored, route is public) + assert resp.status_code == 200, resp.text + + +def test_schema_endpoint_reachable_with_non_admin_key( + non_admin_client: TestClient, +) -> None: + # Given: a non-admin client + # When: the schema endpoint is fetched + resp = non_admin_client.get(f"{_CONTROLS_URL}/schema") + + # Then: the schema is returned + assert resp.status_code == 200, resp.text + + +# --------------------------------------------------------------------------- +# CONTROLS_READ operations: AUTHENTICATED suffices. +# --------------------------------------------------------------------------- + + +def test_non_admin_can_list_controls( + non_admin_client: TestClient, client: TestClient +) -> None: + # Given: an existing control + _create_control(client) + + # When: a non-admin lists controls + resp = non_admin_client.get(_CONTROLS_URL) + + # Then: the list is returned + assert resp.status_code == 200, resp.text + + +def test_non_admin_can_get_control( + non_admin_client: TestClient, client: TestClient +) -> None: + # Given: an existing control + control_id = _create_control(client) + + # When: a non-admin reads it + resp = non_admin_client.get(f"{_CONTROLS_URL}/{control_id}") + + # Then: the control is returned + assert resp.status_code == 200, resp.text + + +def test_non_admin_can_get_control_data( + non_admin_client: TestClient, client: TestClient +) -> None: + # Given: an existing control + control_id = _create_control(client) + + # When: a non-admin reads its data + resp = non_admin_client.get(f"{_CONTROLS_URL}/{control_id}/data") + + # Then: the data is returned + assert resp.status_code == 200, resp.text + + +def test_non_admin_can_list_versions( + non_admin_client: TestClient, client: TestClient +) -> None: + # Given: an existing control with at least one version (creation) + control_id = _create_control(client) + + # When: a non-admin lists versions + resp = non_admin_client.get(f"{_CONTROLS_URL}/{control_id}/versions") + + # Then: the version list is returned + assert resp.status_code == 200, resp.text + + +def test_non_admin_can_get_specific_version( + non_admin_client: TestClient, client: TestClient +) -> None: + # Given: an existing control (version 1 = "created") + control_id = _create_control(client) + + # When: a non-admin reads version 1 + resp = non_admin_client.get(f"{_CONTROLS_URL}/{control_id}/versions/1") + + # Then: the version snapshot is returned + assert resp.status_code == 200, resp.text + + +# --------------------------------------------------------------------------- +# CONTROLS_CREATE / UPDATE / DELETE: ADMIN required. +# --------------------------------------------------------------------------- + + +def test_non_admin_cannot_create_control(non_admin_client: TestClient) -> None: + # When: a non-admin attempts to create + resp = non_admin_client.put( + _CONTROLS_URL, + json={ + "name": f"control-{uuid.uuid4().hex[:12]}", + "data": VALID_CONTROL_PAYLOAD, + }, + ) + + # Then: the request is forbidden + assert resp.status_code == 403, resp.text + + +def test_non_admin_cannot_set_control_data( + non_admin_client: TestClient, client: TestClient +) -> None: + # Given: an existing control + control_id = _create_control(client) + + # When: a non-admin attempts to replace its data + resp = non_admin_client.put( + f"{_CONTROLS_URL}/{control_id}/data", + json={"data": VALID_CONTROL_PAYLOAD}, + ) + + # Then: the request is forbidden + assert resp.status_code == 403, resp.text + + +def test_non_admin_cannot_patch_control( + non_admin_client: TestClient, client: TestClient +) -> None: + # Given: an existing control + control_id = _create_control(client) + + # When: a non-admin attempts to rename it + resp = non_admin_client.patch( + f"{_CONTROLS_URL}/{control_id}", + json={"name": "renamed"}, + ) + + # Then: the request is forbidden + assert resp.status_code == 403, resp.text + + +def test_non_admin_cannot_delete_control( + non_admin_client: TestClient, client: TestClient +) -> None: + # Given: an existing control + control_id = _create_control(client) + + # When: a non-admin attempts to delete it + resp = non_admin_client.delete(f"{_CONTROLS_URL}/{control_id}") + + # Then: the request is forbidden + assert resp.status_code == 403, resp.text + + +def test_non_admin_cannot_validate_control_data( + non_admin_client: TestClient, +) -> None: + """``/controls/validate`` requires ``CONTROLS_CREATE``.""" + # When: a non-admin attempts to validate a draft payload + resp = non_admin_client.post( + f"{_CONTROLS_URL}/validate", + json={"data": VALID_CONTROL_PAYLOAD}, + ) + + # Then: validation is admin-only + assert resp.status_code == 403, resp.text + + +def test_non_admin_cannot_render_template(non_admin_client: TestClient) -> None: + """``/control-templates/render`` requires ``CONTROLS_CREATE``.""" + # When: a non-admin attempts to render a template + resp = non_admin_client.post( + f"{_TEMPLATES_URL}/render", + json={"template": {}, "template_values": {}}, + ) + + # Then: rendering is admin-only + assert resp.status_code == 403, resp.text + + +# --------------------------------------------------------------------------- +# Unauthenticated requests are rejected on every framework-protected route. +# --------------------------------------------------------------------------- + + +def test_unauthenticated_cannot_list_controls( + unauthenticated_client: TestClient, +) -> None: + # When: a client without credentials lists controls + resp = unauthenticated_client.get(_CONTROLS_URL) + + # Then: the request is rejected + assert resp.status_code == 401, resp.text + + +def test_unauthenticated_cannot_create_control( + unauthenticated_client: TestClient, +) -> None: + # When: a client without credentials attempts to create + resp = unauthenticated_client.put( + _CONTROLS_URL, + json={ + "name": f"control-{uuid.uuid4().hex[:12]}", + "data": VALID_CONTROL_PAYLOAD, + }, + ) + + # Then: the request is rejected + assert resp.status_code == 401, resp.text + + +def test_unauthenticated_cannot_validate( + unauthenticated_client: TestClient, +) -> None: + # When: a client without credentials attempts to validate + resp = unauthenticated_client.post( + f"{_CONTROLS_URL}/validate", + json={"data": VALID_CONTROL_PAYLOAD}, + ) + + # Then: the request is rejected + assert resp.status_code == 401, resp.text + + +def test_unauthenticated_cannot_render_template( + unauthenticated_client: TestClient, +) -> None: + # When: a client without credentials attempts to render + resp = unauthenticated_client.post( + f"{_TEMPLATES_URL}/render", + json={"template": {}, "template_values": {}}, + ) + + # Then: the request is rejected + assert resp.status_code == 401, resp.text + + +# --------------------------------------------------------------------------- +# No-auth deployment mode: api_key_enabled=False bypasses every gate. +# --------------------------------------------------------------------------- + + +def test_no_auth_mode_allows_writes_without_credentials( + unauthenticated_client: TestClient, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """When ``api_key_enabled`` is False, the ``HeaderAuthProvider`` + short-circuits to a non-admin ``Principal`` for every operation, + including admin-level writes. This pins the "no auth" deployment + path so a future refactor can't silently start enforcing. + """ + # Given: api_key_enabled is False (single-tenant OSS dev mode) + monkeypatch.setattr(auth_settings, "api_key_enabled", False) + + # When: an unauthenticated client creates a control + resp = unauthenticated_client.put( + _CONTROLS_URL, + json={ + "name": f"control-{uuid.uuid4().hex[:12]}", + "data": VALID_CONTROL_PAYLOAD, + }, + ) + + # Then: the create succeeds because auth is disabled at the provider + assert resp.status_code == 200, resp.text + assert "control_id" in resp.json() +