feat(server): migrate controls routes to auth framework#212
feat(server): migrate controls routes to auth framework#212abhinav-galileo wants to merge 3 commits intomainfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Mirrors #204's bindings migration: replaces require_admin_key and router-level require_api_key with require_operation(CONTROLS_*) on every protected route on /controls and on /control-templates/render. Both routers now mount with the non-validating get_api_key_from_header so the framework owns authentication and authorization, with the extractor attached purely so the generated OpenAPI advertises X-API-Key. GET /controls/schema is intentionally left without a require_operation dependency: it returns a static model schema with no tenant state and routing it through the framework would force the upstream provider to handle a meta-only operation that has no permission semantics. POST /controls/validate and POST /control-templates/render are wired to CONTROLS_CREATE rather than CONTROLS_READ. Both exercise the authoring materialization path and exist to support the create / set- data flow; a caller who cannot create controls has no use for the result. Backwards-incompatible for OSS deployments that previously called these routes with non-admin keys; deployments that want the old behavior can override with HeaderAuthProvider(operation_access={...}). Storage namespace continues to come from get_namespace_key, matching the bindings migration in #204. The unified principal-derived cutover across /controls, /policies, /agents, and /evaluation is a follow-up.
…utes Move auth-framework rationale on /controls/schema, /controls/validate, and /control-templates/render from route docstrings into normal code comments. The docstrings flow into the generated TypeScript SDK as public API documentation, so internal terminology like ``require_operation`` and "upstream authorizer" should not appear there. Function-level comments preserve the rationale for readers of the source. Also remove the skipped placeholder test for the project-scoped credential deny scenario; that scenario depends on a deployment-side provider configuration that is not part of the OSS server, so tracking it as a permanent skipped test in this repo was the wrong home for it. Regenerate the TypeScript SDK to drop the leaked rationale lines.
abhinav-galileo
changed the title
abhinav-galileo
force-pushed
the
abhi/controls-auth-framework
branch
from
ad586bb to
3a5b7e4
Compare
| summary="Get control definition JSON schema", | ||
| response_description="JSON schema for ControlDefinition", | ||
| ) | ||
| # Public schema metadata: no tenant state, no auth operation. |
There was a problem hiding this comment.
P1: GET /controls/schema is now anonymous. Previously gated by require_api_key at the router. The diff intentionally drops auth and test_schema_endpoint_reachable_without_credentials pins it. Schema content is ControlDefinition.model_json_schema() (no tenant data), so likely fine, but worth explicit security sign-off because it changes the "every /api/v1/controls/* route requires creds" invariant operators may have depended on.
| 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)), |
There was a problem hiding this comment.
The endpoint binds _principal for the auth side-effect but never threads _principal.namespace_key into the ControlService calls. list_controls_page doesn't even accept a namespace; get_active_control_or_404 accepts one but isn't called with it. Today this is fine because HeaderAuthProvider always returns DEFAULT_NAMESPACE_KEY, but the moment any other provider lands (and the framework is explicitly built for that), controls become cross-tenant readable/writable here.
Recommended fix in this PR (cheap, defensive): add a small _require_default_namespace(principal) helper in this module and call it from every endpoint that captures _principal. That converts a silent forward-compat hazard into a loud deploy-time failure the day a non-default namespace shows up. Threading it properly through the service layer can be a follow-up PR.
2 participants
Summary
/controlsand/control-templates/renderonto operation-based auth.GET /controls/schemapublic because it returns static metadata.CONTROLS_CREATEfor validate and render because both use the authoring path.Behavior Change
POST /controls/validateandPOST /control-templates/rendernow require create access under the default header provider.Testing
make prepushon the stacked branch in feat(sdk): add runtime token auth #215.