SEP-1846: Resolve a request's Bearer credential once per request

[marcuscruz-percona]

Summary

• A mutating request authenticated the same credential twice: require_minimum_role_for_unsafe_methods resolves the caller in its body (a direct call FastAPI's per-request dependency cache can't see, since that cache keys on the declared callable), then the route's own authentication dependency resolves the same token again. Under Casdoor one resolution is two HTTP round-trips — introspect, then fetch the user — so every write paid four where two would do, and the two resolutions could disagree if the caller's role changed between them.
• Split the resolution into authenticate_bearer_token plus a memoizing get_current_user that keeps the public name, so all ~158 app/ references and every dependency_overrides[get_current_user] registration are untouched; the production change is two call sites. The cache is keyed on the credential, so a resolution is never served to a caller presenting a different one, and holds successes only, so a refusal is re-derived rather than remembered. The name states what the resolver does — authenticate, unconditionally, on every call — so nothing has to tell callers to declare IsAuthenticatedDep instead of reaching for it.
• Deviation from the ticket's approach, flagged deliberately. It specifies request.state; the cache lives under a namespaced key in the request scope instead. A request inherits scope["state"] as a shallow copy of the ASGI lifespan state (uvicorn/protocols/http/h11_impl.py), so a cache found there could be reached by every later request as soon as anything published a key of the same name. The invariant the ticket asks for — the cache never outlives its request — holds under the scope key and would not hold under request.state; a test pins that nothing is written to the inherited namespace.
• Entries key on a SHA-256 digest of the credential rather than on the credential itself. Neither harm this forecloses is reachable today — the raw Authorization header bytes already sit in request.scope["headers"] on every request, and a per-request dict only ever holds tokens its own caller presented — so this is hygiene that keeps a secret from being a dict key, not a fix for a live hole.
• No authorization outcome changes. The gate still resolves the caller imperatively rather than through a sub-dependency, which is what keeps the unauthenticated GET /health unauthenticated.

Coverage the round-trip claim previously lacked: provider round-trips are now counted per service, GET /health is asserted to resolve nothing on each of the three, and tests/app/api/test_role_gate.py pins that every unsafe inventory/tasks route authenticates itself as well — the second consumer the deduplication is measured against, without which the round-trip counts would read as green for the wrong reason. That last test is kept knowing it fail-locks a future deliberately-unauthenticated unsafe route; editing it with a stated reason is the right friction for that change.

Two further guards close the corners the acceptance criteria left untested: that the instance the gate and the route now share reaches the second consumer unrewritten, and that a safe method costs the one resolution its route declares rather than gaining the gate's (previously covered only by the gate's own unit tests, never end to end).

One correction to the ticket, recorded there as well: AC 5 cites tests/app/api/test_admin_gate.py and TestRequireAdminForUnsafeMethods. Both were renamed by the ordered-role-gate work, to tests/app/api/test_role_gate.py and TestRequireMinimumRoleForUnsafeMethods. Judged against the real equivalents the criterion holds — no assertion changed; the one altered line in that class is a setup call the resolver rename forced.

One weak assertion found while working here and fixed rather than left noted: tests/app/tasks/test_role_gate.py asserted != 403 across three routers, and PUT /periodic/1 was returning 500 under it — the route resolves its target through the celery-beat session, which nothing overrode, so it raised on a missing table before reaching the handler, and the assertion passed on that as it would on any other failure on the way in. The client fixture now overrides the beat session and the parametrized table names the concrete status each route answers with; removing the override fails with assert 500 == 404 instead of passing. Separate commit, no app code, and outside AC 5's cited gate suites.

Tested

• make test — 10234 passed, 424 skipped
• make lint — all checks passed
• make run-pre-commit — all hooks passed
• Verified the round-trip count moves: with the memoization stubbed to a pass-through, all three services fail assert 2 == 1; after the change each is 1
• Verified the scope-key choice is load-bearing: pre-seeding a cache under the old request.state name made get_current_user return the seeded object without validating it

Checklist

• New/modified functions have type hints and rST docstrings
• New tests added for new features or bug fixes
• All tests pass locally (make test)
• Pre-commit hooks pass (make run-pre-commit)
• Database migrations generated if models changed (make makemigrations)
• User-facing changes documented (README, inline help, UI text)
• Configuration changes documented with examples
• Changelog fragment added under changelog.d/ if the change is user-facing (make changelog-add), or confirmed N/A (internal-only change, or a same-release-cycle fix for an unreleased sibling ticket)

[Copilot]

Pull request overview

Adds request-scoped credential memoization to avoid duplicate authentication round-trips while preserving existing dependency interfaces.

Changes:

• Splits user resolution from the cached dependency.
• Propagates request context through SEP authentication.
• Adds cross-service cache and authentication coverage.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
app/api/deps.py	Adds request-scoped user caching.
app/sep/deps.py	Forwards requests to the cached resolver.
tests/app/api/test_deps.py	Tests cache isolation and failures.
tests/app/api/test_role_gate.py	Verifies unsafe routes authenticate independently.
tests/app/inventory/test_role_gate.py	Checks Inventory round-trip counts.
tests/app/tasks/test_role_gate.py	Checks Tasks round-trip counts.
tests/app/sep/api/test_router.py	Checks SEP round-trip counts.
tests/app/sep/test_deps.py	Verifies request forwarding.

---

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

[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/sep
deps.py
inventory.py
app/sep/sync/syncers
pmm.py
Project Total

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