fix(auth): deny Operator statistics and enforce default authentication for every route #197

New issue

Open

opened 2026-09-30 18:45:09 +00:00 by gabogg · 1 comment

gabogg commented

2026-09-30 18:45:09 +00:00

Owner

Production reproduction (2026-09-30)

An authenticated Operator can press F5 and open the Statistics deck; the deck loads data. A read-only production check confirmed that an Operator session receives HTTP 200 with records from GET /api/statistics/periods?granularity=day&limit=1. The same endpoint returns 401 without a session. The immediate defect is missing role authorization, not anonymous access to this endpoint.

Separate read-only requests without a session returned 200 from /openapi.json, /docs, /api-docs, and /health. POST /api/event/webhook has no authentication dependency in the current code; it was not called in production because that would mutate door state.

Code path

app/static/js/app.js: KEY_DECK_MAP.F5 = 'statistics', ROLE_ALLOWED_DECKS.operator includes statistics, and the Operator UI configuration shows that deck. Shortcut checks and switchTab() therefore allow F5 intentionally under the current client policy.
app/controllers/statistics_controller.py: statistics endpoints depend on require_auth, which accepts any authenticated role.
app/dependencies.py: the app-wide enforce_viewer_role_boundary restricts only Viewer sessions; it does not reject an anonymous request or restrict Operators. Per-route dependencies are therefore easy to omit on newly added routes.
app/main.py: documentation routes and /health are currently public; app/controllers/webhook_controller.py accepts a callback without a visible credential check.

Required solution

Define a server-side role/capability matrix for every operational HTTP and WebSocket route. Remove Statistics access from the Operator role, deny its API calls with 403, and remove its F5/other shortcut, button, deep-link, and direct switchTab('statistics') paths. UI hiding is only presentation; the server must decide.
Enforce authentication by default at the application boundary for current and future routes, including mounted applications and WebSockets. New routes must be protected automatically unless they are deliberately declared public. An absent/invalid session must produce 401 for HTTP and an authentication failure for WebSockets before operational data is sent.
Keep the public surface as a small explicit allowlist limited to login and the shell/assets strictly needed to display it. Require authentication for /api/auth/me, logout, docs/OpenAPI, health, and all other routes unless a documented integration contract requires a different authenticated mechanism. The HikCentral webhook must use an independently verified machine credential/signature or equivalent trusted transport; do not rely on a browser login cookie for it.
Add an automated route-policy audit in CI that enumerates registered HTTP routes, WebSocket routes, and mounts, fails on any route without an explicit policy, and checks that public exceptions are limited to the reviewed allowlist. Add integration tests that exercise representative routes as anonymous, Operator, Viewer, Admin, and invalid/expired-session clients; include F5/deep-link behavior and direct API calls. This must enforce the rule without relying on future agents remembering it.
Apply the same policy to any future Supervisor role from #189. Keep the statistics capability decision explicit for Supervisor/Admin/Viewer; do not grant it through a broad authenticated-user fallback.

Acceptance criteria

An Operator cannot load Statistics by F5, F8, a button, a URL hash, JavaScript navigation, or direct /api/statistics/* requests; direct data requests return 403.
An anonymous client cannot read operational API, WebSocket, docs, health, or other non-login routes. Public bootstrap resources return only static login UI/assets and no operational data.
HikCentral callbacks still work through an explicit machine-authenticated exception, with unauthenticated callback attempts rejected before processing.
CI fails when a new route/mount lacks a reviewed policy or an unapproved anonymous exception is introduced.

Related: #188 Operator workspace scope, #189 Supervisor access, #72 door-command RBAC. Keep this cross-cutting access-control issue separate from those UI/features unless triage makes it a milestone prerequisite.

## Production reproduction (2026-09-30) An authenticated Operator can press **F5** and open the Statistics deck; the deck loads data. A read-only production check confirmed that an Operator session receives HTTP **200** with records from `GET /api/statistics/periods?granularity=day&limit=1`. The same endpoint returns **401** without a session. The immediate defect is missing **role authorization**, not anonymous access to this endpoint. Separate read-only requests without a session returned **200** from `/openapi.json`, `/docs`, `/api-docs`, and `/health`. `POST /api/event/webhook` has no authentication dependency in the current code; it was **not** called in production because that would mutate door state. ## Code path - `app/static/js/app.js`: `KEY_DECK_MAP.F5 = 'statistics'`, `ROLE_ALLOWED_DECKS.operator` includes `statistics`, and the Operator UI configuration shows that deck. Shortcut checks and `switchTab()` therefore allow F5 intentionally under the current client policy. - `app/controllers/statistics_controller.py`: statistics endpoints depend on `require_auth`, which accepts any authenticated role. - `app/dependencies.py`: the app-wide `enforce_viewer_role_boundary` restricts only Viewer sessions; it does not reject an anonymous request or restrict Operators. Per-route dependencies are therefore easy to omit on newly added routes. - `app/main.py`: documentation routes and `/health` are currently public; `app/controllers/webhook_controller.py` accepts a callback without a visible credential check. ## Required solution 1. Define a server-side role/capability matrix for **every operational HTTP and WebSocket route**. Remove Statistics access from the Operator role, deny its API calls with 403, and remove its F5/other shortcut, button, deep-link, and direct `switchTab('statistics')` paths. UI hiding is only presentation; the server must decide. 2. Enforce **authentication by default at the application boundary** for current and future routes, including mounted applications and WebSockets. New routes must be protected automatically unless they are deliberately declared public. An absent/invalid session must produce 401 for HTTP and an authentication failure for WebSockets before operational data is sent. 3. Keep the public surface as a small explicit allowlist limited to login and the shell/assets strictly needed to display it. Require authentication for `/api/auth/me`, logout, docs/OpenAPI, health, and all other routes unless a documented integration contract requires a different authenticated mechanism. The HikCentral webhook must use an independently verified machine credential/signature or equivalent trusted transport; do not rely on a browser login cookie for it. 4. Add an **automated route-policy audit in CI** that enumerates registered HTTP routes, WebSocket routes, and mounts, fails on any route without an explicit policy, and checks that public exceptions are limited to the reviewed allowlist. Add integration tests that exercise representative routes as anonymous, Operator, Viewer, Admin, and invalid/expired-session clients; include F5/deep-link behavior and direct API calls. This must enforce the rule without relying on future agents remembering it. 5. Apply the same policy to any future Supervisor role from #189. Keep the statistics capability decision explicit for Supervisor/Admin/Viewer; do not grant it through a broad authenticated-user fallback. ## Acceptance criteria - An Operator cannot load Statistics by F5, F8, a button, a URL hash, JavaScript navigation, or direct `/api/statistics/*` requests; direct data requests return 403. - An anonymous client cannot read operational API, WebSocket, docs, health, or other non-login routes. Public bootstrap resources return only static login UI/assets and no operational data. - HikCentral callbacks still work through an explicit machine-authenticated exception, with unauthenticated callback attempts rejected before processing. - CI fails when a new route/mount lacks a reviewed policy or an unapproved anonymous exception is introduced. Related: #188 Operator workspace scope, #189 Supervisor access, #72 door-command RBAC. Keep this cross-cutting access-control issue separate from those UI/features unless triage makes it a milestone prerequisite.

gabogg added the

high-priority

needs-triage

labels

2026-09-30 18:45:09 +00:00

gabogg added this to the Focused operator and supervisor door operations milestone

2026-10-03 12:16:49 +00:00

gabogg commented

2026-10-03 12:16:50 +00:00

Author

Owner

Joined milestone Focused operator and supervisor door operations as a prerequisite (maintainer decision, 2026-10-03). The Operator and Supervisor workspaces (#187, #188, #189) depend on correct per-role route authorization. It is still needs-triage and high-priority, and since it is a live authorization gap in production it can land before the rest of the milestone.

Joined milestone **Focused operator and supervisor door operations** as a prerequisite (maintainer decision, 2026-10-03). The Operator and Supervisor workspaces (#187, #188, #189) depend on correct per-role route authorization. It is still `needs-triage` and `high-priority`, and since it is a live authorization gap in production it can land before the rest of the milestone.

gabogg referenced this issue

2026-10-03 12:21:13 +00:00

docs(agents): milestones hold only what their delivery needs #266