SEP-1850: Require a minimum role per route

[yyyyyyyan]

Summary

The unsafe-method gate in app/api/deps.py decided authorization on one is_admin
boolean, with an endpoint-keyed exemption set (_NON_ADMIN_MUTATIONS /
allow_non_admin_mutation) carved out for the single route that had to stay open to
non-admins. It now resolves a minimum UserRole per route and compares the
caller's rank against it.

The gate (require_minimum_role_for_unsafe_methods, renamed from
require_admin_for_unsafe_methods):

1. safe methods (GET/HEAD/OPTIONS) return before anything is resolved;
2. minimum_role_for(request.scope.get("route")) reads the matched route's
   endpoint out of the registry — no request path or prefix is compared anywhere
   in the gate;
3. a UserRole.NONE minimum returns before the Bearer check, so the gate
   imposes no authentication of its own on a route it waives;
4. otherwise a Bearer credential is required and resolved;
5. the SEP_INTERNAL_TOKEN service principal is admitted by identity, ahead of the
   rank comparison — it holds UserRole.VIEWER and would fail any comparison;
6. user.role < minimum refuses with 403.

The registry. require_minimum_role(role) is a decorator factory registering an
endpoint object; minimum_role_for is the single home of the default,
DEFAULT_MINIMUM_ROLE = UserRole.ADMIN. An unregistered endpoint, a request that
matched no route, and a matched object without an .endpoint attribute all resolve
to ADMIN, so a route added without an authorization decision ships admin-only.

Authorization changes, explicitly. Of the 97 gated unsafe route-methods across
the three sub-apps, 94 keep an administrator minimum. Three do not:

Route	Minimum	Change
POST /api/apps/alerts/restore	EDITOR	newly reachable by an Editor
POST /api/apps/alerts/push	EDITOR	newly reachable by an Editor
POST /api/tasks/history/latest	NONE	unchanged — the previous exemption, re-expressed

POST /api/apps/alerts/pagerduty and /pagerduty/delete sit on the same router and
deliberately take the ADMIN default: they persist third-party routing and an
integration key, not alert-template content. Reads are untouched — no read-side
gating is introduced. Behaviour at an ADMIN minimum is identical to before, because
BaseUser.is_admin is itself self.role >= UserRole.ADMIN.

Removed, not deprecated. allow_non_admin_mutation and _NON_ADMIN_MUTATIONS
are deleted, and require_admin_for_unsafe_methods /
RequireAdminForUnsafeMethods are renamed with no compatibility re-export —
neither old name resolves anywhere in the repo after this change (the only surviving
mention is the changelog fragment, which documents the rename for anyone carrying an
out-of-tree app package). 27 of the 33 changed files reference the renamed gate symbol; in most of them the rename is the only change.

Tests. The three test_admin_gate.py files become test_role_gate.py. The
cross-app oracle changes from "exactly one route is exempt" to an equality against a
named map of every non-ADMIN route and its minimum, so opening or closing a surface
fails the test until the map names it. A new TestRoleGate class in the alerts suite
drives the real gate over HTTP at Editor, Viewer and admin ranks — its fixture pops
the gate and user dependency overrides so the live gate runs, and the Viewer-403 rows
are the canary that the pop landed.

Tested

All Editor and Viewer scenarios need an auth provider that reports those ranks, so
run them against a SEP deployment using the Grafana auth provider with PMM
configured. Casdoor deployments cannot produce an Editor at all and are expected to
behave exactly as before.

• Signed in as an Editor, pushed alert templates from Alert Templates and confirmed the push lands (the templates appear in PMM), then restored a backup and confirmed the restore reports success.
• Signed in as a Viewer, attempted the same push and restore, and confirmed both are refused with 403 rather than succeeding or erroring.
• As an Editor, attempted to save and to delete the PagerDuty contact point and confirmed both are refused with 403.
• As an Editor, attempted a mutation outside Alert Templates (for example enabling a periodic task) and confirmed it is refused with 403.
• As an administrator, repeated the push, the restore and the PagerDuty save, and confirmed all still succeed.
• Repeated the Editor push and the Viewer refusal with SEP served under a URL prefix, and confirmed the verdicts are identical to the unprefixed case.
• On a Casdoor deployment, confirmed an ordinary (non-admin) user is still refused on every unsafe route and an administrator still passes, unchanged from before.
• Let a scheduled inventory sync run and confirmed it still writes nodes and services (the service principal is admitted by identity, and holds viewer).
• Opened a task list page as a non-admin and confirmed per-task statuses still resolve rather than blanking out (the batch lookup stays reachable without the gate authenticating it).
• Confirmed reads are unaffected: listed inventory nodes and alert backups as a non-admin.

Known limitations

• The Editor tier is not exercisable in the PMM-embedded side-car as it ships today: that image carries only inventory, atw and mysql_backups, all of which remain administrator-only under this classification, so Alert Templates is absent there. The value of the tier in that deployment is that the vocabulary is correct in advance — an app joining the embedded set carries a considered minimum instead of inheriting a blanket admin gate.
• Framework-derived routes (TaskExecutionApp's generated create/update/delete and /execute) are closures local to their builders, so there is no function object to register and they cannot express a minimum below ADMIN. Every one of them is classified ADMIN, so nothing is blocked today; giving the framework a declarative knob is separate work with no consumer yet.
• The two read-only diagnostic runs (dipper and topology) stay ADMIN even though a rank-based reading would put them lower: both forward the caller's own bearer to the tasks service and dispatch POST /execute/{name}, which stays ADMIN, so an Editor tier on the outer route would clear this gate and then be refused downstream. The ticket records the decision under Out of Scope.
• Two routes sharing one endpoint object would share one registered minimum. Nothing in the tree does this today (verified: no endpoint object backs more than one route across the three sub-apps), so it is recorded as a hazard for a future add_api_route that reuses a handler rather than a defect here.

Checklist

• New/modified functions have type hints and rST docstrings
• New tests added for new features or bug fixes
• Database migrations generated if models changed (make makemigrations) (N/A for this change — no models touched)
• User-facing changes documented (README, inline help, UI text) (N/A for this change — no UI or README surface changes; the behaviour change is recorded in the changelog fragment)
• Configuration changes documented with examples (N/A for this change — no new configuration)

[Copilot]

Pull request overview

Introduces route-specific minimum-role authorization while retaining admin-only defaults.

Changes:

• Adds the minimum-role registry and unsafe-method gate.
• Grants Editors access to alert restore/push routes.
• Updates gate wiring, tests, fixtures, and changelog.

Reviewed changes

Copilot reviewed 33 out of 33 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
app/api/deps.py	Implements minimum-role authorization.
app/inventory/main.py	Wires the new gate into Inventory.
app/sep/api/router.py	Wires the new SEP API gate.
app/sep/apps/alerts/api_routes.py	Grants Editors restore/push access.
app/tasks/main.py	Wires the new gate into Tasks.
app/tasks/routes.py	Registers the history lookup at NONE.
changelog.d/SEP-1850.changed.md	Documents authorization changes.
tests/app/api/test_deps.py	Covers role resolution and enforcement.
tests/app/api/test_role_gate.py	Classifies gated routes by minimum role.
tests/app/conftest.py	Updates shared gate overrides.
tests/app/inventory/conftest.py	Updates Inventory fixtures.
tests/app/inventory/test_role_gate.py	Renames Inventory gate documentation.
tests/app/inventory/test_settings_routes.py	Updates Inventory settings fixtures.
tests/app/sep/api/routes/test_app_state.py	Updates app-state overrides.
tests/app/sep/api/routes/test_connectivity_check.py	Updates connectivity overrides.
tests/app/sep/api/routes/test_settings.py	Updates settings overrides.
tests/app/sep/api/routes/test_settings_export.py	Updates export test overrides.
tests/app/sep/api/routes/test_settings_proxy.py	Updates proxy test overrides.
tests/app/sep/api/test_router.py	Tests router-level role enforcement.
tests/app/sep/apps/alerts/test_api_routes.py	Adds live Editor/Viewer route tests.
tests/app/sep/apps/atw/conftest.py	Updates ATW fixtures.
tests/app/sep/apps/atw/test_batch_api.py	Updates ATW gate overrides.
tests/app/sep/apps/mysql_backups/conftest.py	Updates backup fixtures.
tests/app/sep/apps/snippets/conftest.py	Updates snippet fixtures.
tests/app/sep/apps/snippets/test_api_routes.py	Updates snippet route tests.
tests/app/sep/routes/test_stream_logs.py	Updates renamed test reference.
tests/app/sep/test_proxy_routes_with_override.py	Updates proxy gate override.
tests/app/tasks/conftest.py	Updates shared Tasks fixtures.
tests/app/tasks/connectivity/test_routes.py	Updates connectivity fixtures.
tests/app/tasks/periodic/conftest.py	Updates periodic-task fixtures.
tests/app/tasks/settings/test_routes.py	Updates Tasks settings fixtures.
tests/app/tasks/test_role_gate.py	Renames Tasks gate documentation.
tests/app/tasks/test_routes.py	Updates Tasks route overrides.

Suppressed comments (2)

tests/app/sep/api/test_router.py:855

• This touched docstring still calls the renamed dependency the “admin gate.” Update it to match the role-gate behavior and the symbol asserted below.

        mutation must still carry ``BEARER_REQUIRED_DETAIL``; declaring the admin
        gate first would answer a bare 401 with a different detail.

tests/app/sep/api/test_router.py:866

• The enclosing test name still refers to the removed admin gate, while this changed assertion targets require_minimum_role_for_unsafe_methods. Rename it so test reports identify the current behavior.

        assert api_deps.index(require_minimum_role_for_unsafe_methods) > api_deps.index(

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[yyyyyyyan]

Automated QA — PASS

Verified all ten Tested items against fresh instances built from the PR head, using a Grafana-backed deployment with real Editor, Viewer and administrator accounts:

• An Editor's push genuinely writes to PMM, and an Editor's restore succeeds. The push endpoint reports skipped / Already present in PMM whenever the name is in its cached listing, so a bare 200 does not prove a write. Instead: deleted the MySQL Slow Queries rule straight from PMM (3 rules left), pushed it back as an Editor, and PMM listed 4 again — the only mutation in between was the Editor's push. Restore as the same account returned success with templates.skipped=43, rules_skipped=4, contact_points.updated=1, notification_policies: restored.

  

• A Viewer is refused where an Editor just passed. The rank comparison rejects viewer against an editor minimum on exactly the surface the Editor cleared moments earlier.

  • Push returns 403.

    

  • Restore returns 403.

    

• The PagerDuty routes hold the ADMIN default even for an Editor. Save and delete both 403 for an Editor. These sit on the same router as push and restore, so this is the row that shows the minimum is resolved per route rather than per router.

  

• An Editor is refused on mutations outside Alert Templates. Exercised exactly as worded — PUT /api/tasks/periodic/{id} as an Editor returns 403, while the same call as an administrator reaches 404. That contrast is the useful part: the administrator cleared the gate and fell through to a genuine routing miss, so the Editor's 403 is provably the gate and not a bad path.

  

• An administrator is unaffected. Behaviour at an ADMIN minimum is unchanged, as expected given is_admin is itself role >= ADMIN.

  • Push returns 200.

    

  • Restore returns 200 with status: success.

    

  • PagerDuty save returns 200 with status: created.

    

• A URL prefix changes nothing. Ran the full matrix — 11 gated routes × 4 principals — against a second instance with SEP.ROOT_PATH set, and every authorization outcome was identical to the unprefixed run. Both the bare and prefixed request shapes reach the gate and are decided the same way, which is what you would expect given nothing in the gate compares a path.

  

• Casdoor deployments behave exactly as before. On an instance with the Casdoor provider selected, the admin account resolves to role=admin and the ordinary account to role=viewer — there is no Editor to mint. The non-admin is refused 403 on every unsafe route probed, including the two editor-minimum ones, the admin passes everywhere, and history/latest stays open to both.

  

• The service principal writes despite holding viewer. It reports role=viewer, isAdmin=false, and still creates an inventory node (201, persisted) where a real Viewer making the identical call gets 403; inventory sync returns 202. Admission is by identity, ahead of a comparison it would otherwise fail.

  

• A non-admin's task list still resolves per-task status. This is the item guarding the UserRole.NONE registration, and its failure mode is silent — resolve_latest_history (app/sep/apps/framework/task_status.py:188) wraps the batch call in a bare except Exception and degrades a failed chunk to None, so a 403 there would blank the column with nothing raised anywhere. Seeded and executed a task so the column had something to resolve, then signed into the shell as a Viewer: the row renders a real status badge, identical to the administrator's view of the same list, with no 403 in any request.

  

• Reads are untouched. As a Viewer: inventory nodes 200, alert backups 200, tasks index 200. Safe methods return before a minimum is resolved.

  

Separately, I enumerated the registry independently of the suite — walking every APIRoute across the three sub-apps and resolving each through minimum_role_for — to check the description's numbers rather than take them on trust. They reproduce exactly: 97 gated unsafe route-methods, 94 at ADMIN, 3 below (alerts/push and alerts/restore at editor, tasks/history/latest at none), no endpoint object backing more than one route, and the only unsafe routes outside the gate are the six /api/oauth/* ones. minimum_role_for returns ADMIN for None, for an object with no .endpoint, and for an unregistered endpoint.

Two notes on the classification that might be worth a line in the description rather than any code change:

• The unsafe-method matrix above spans all three sub-apps, not just the one the diff registered routes in — worth recording, since a single gate wired into three mounts is the kind of thing that can regress in only one of them. All three refuse an Editor on their ADMIN defaults.
• "A request that matches no route requires ADMIN" holds of minimum_role_for, but is not observable over HTTP — routing returns 404 before app-level dependencies run, so no principal ever reaches the resolver on that path. Nothing to fix (the 404 mutates nothing and leaks nothing); the claim is just about the resolver rather than about a response code.

Observations — pre-existing, out of scope

• After the rule was deleted from PMM out of band, GET /api/apps/alerts/ still reported it as in_pmm: true, and the subsequent push reported skipped / Already present in PMM while in fact recreating it (the handler calls create_rule on that branch anyway, so the rule came back correctly). The outcome is right and the message is not — the presence set the response is built from appears to be resolved from a listing that had not caught up. Untouched by this PR, and only cosmetic, but the reported status does not match what the endpoint did.

[github-actions]

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
app/api
deps.py
app/inventory
main.py
app/sep
inventory.py
app/sep/api
router.py
app/sep/apps/alerts
api_routes.py
app/sep/sync/syncers
pmm.py
app/tasks
main.py
routes.py
Project Total

This report was generated by python-coverage-comment-action