Security: fix the admin authentication bypass in AdminAuthMiddleware

[igorbenav]

Security: fix the admin authentication bypass in AdminAuthMiddleware

AdminAuthMiddleware exempted the login page and static assets from the session check, but it matched both by path shape rather than by path identity — a suffix test for /login and a substring test for /static/. Because the admin's routes take path parameters, the value of a parameter could push a request into either exemption, and the admin's read endpoints had no authentication dependency of their own to fall back on. This branch anchors both exemptions to the configured admin prefix, and — while verifying that the middleware was no longer a single point of failure — found that four route registrations passed the auth dependency factory uncalled, so the dependencies that were supposed to be the backstop enforced nothing. Both defects are fixed, with a regression suite that fails against v0.5.0, the security policy published on the docs site, and a bump to v0.5.1. No public API changes.

Reported privately by Ubaid Ur Rehman, found using shadowaudit.

---

The bypass

The two exemptions in dispatch were:

is_login_path = request.url.path.endswith("/login")   # any path ENDING in /login
is_static_path = "/static/" in request.url.path       # any path CONTAINING /static/

Both are satisfiable by choosing a path parameter, so an unauthenticated request could be steered past the session check:

Crafted URL	Route matched	Result on main
/admin/Article/update/login	/update/{id} with id="login"	200 — update form rendered, no session
/admin/Product/update/login	/update/{id} with id="login"	Unauthenticated 500 (id not coercible to an integer PK)
/admin/Author/related/static/books	/related/{id}/{relationship_name}	Unauthenticated 500

Each row was reproduced against a real app before the fix and is now covered by a test.

On the actual impact. Rendering real record data required a model with a string primary key holding a row keyed literally "login"; on integer-PK models the same URL produced an unauthenticated 500 instead. List views, the dashboard, health, and the event log were never reachable this way — their URLs neither end in /login nor contain a /static/ segment — so the original report's "all READ endpoints are exposed" overstates it. It remains a real broken-access-control defect: unauthenticated requests reached endpoint code and issued database queries. The release notes state the narrower scope explicitly rather than repeating the report's framing.

---

Exact path matching

Both exemptions are now anchored to the admin's configured URL prefix, so no path parameter can reach them:

url_prefix = self.admin_instance.get_url_prefix()
is_login_path = request.url.path.rstrip("/") == f"{url_prefix}/login"
is_static_path = request.url.path.startswith(f"{url_prefix}/static/")

url_prefix is hoisted out of the four get_url_prefix() calls that built the redirect targets further down dispatch, which is why the diff on that file is larger than the two changed predicates.

Why: the exemptions describe two specific resources — one page and one mount point — so they should be expressed as identity and prefix, not as a suffix and a substring that any parameterized route can satisfy. rstrip("/") keeps /admin/login/ working; it previously fell through to the session check and redirected to the login page carrying an error= parameter.

---

Defense in depth on the read routes

get_model_admin_page, get_model_create_page, get_model_update_page, get_related_data_endpoint, and get_relationship_options_endpoint carried no Depends(get_current_user). The write handlers (form_create, form_update, bulk-delete) always have, which is why write operations were never exposed by this bug.

setup_routes now builds an auth_dependencies list once and attaches it to all six previously-unprotected GET routes. It resolves to [] when admin_site is None, so a ModelView constructed standalone still sets up.

Why: the middleware was the only thing standing in front of every read path in the admin, so a single flawed predicate exposed all of them at once. Under normal use nothing changes — the middleware still redirects anonymous visitors before the dependency runs — but the dependency is what keeps a future middleware mistake from being a bypass again. Measured: with the old middleware restored and only these dependencies in place, all six bypass tests still pass.

---

Four auth dependencies that enforced nothing

AdminAuthentication.get_current_user is a factory: it returns the dependency, it is not the dependency. Four registration sites passed it uncalled.

dependencies=[Depends(self.admin_authentication.get_current_user)]     # inert
dependencies=[Depends(self.admin_authentication.get_current_user())]   # enforces

Depends(factory) hands FastAPI a zero-parameter sync callable that returns a function and never raises — the route reads as protected and enforces nothing. The affected registrations:

Site	Routes
admin_site.py:204,212	/ (dashboard), /dashboard-content
crud_admin.py:849	/management/health, /management/health/content, and the event-log routes registered in that block
crud_admin.py:1163	the router-level dependency on include_router for every model view

The last one matters most: that router-level dependency was written to protect all model-view routes, and the missing () is why the read endpoints had nothing behind the middleware when the bypass reached them. Two cast(...) wrappers around those references are why mypy never flagged the type mismatch; both casts are removed rather than corrected, since the call now returns the right type on its own.

How it surfaced. Mounting admin.app at a path that differs from the configured mount_path makes the middleware's outer prefix check miss, and it returns call_next — it fails open. That configuration therefore isolates the dependencies. Before this change, /backoffice/ and /backoffice/management/health returned 200 to an anonymous request; they now return 401, as do the model-view routes.

---

One session validation per request instead of two

get_current_user_inner called session_manager.validate_session twice with identical arguments, discarded the first result after a truthiness check, and used the second. Both calls default to update_activity=True, so every authenticated request also wrote the activity timestamp twice.

That was invisible while the dependencies were inert. Making them enforce would have put the duplicate on every admin request, so the redundant call is removed as part of the same change. get_current_user now also memoizes the dependency it builds, so the router-level and route-level declarations share one callable and FastAPI's per-request dependency cache resolves it once.

Measured on an authenticated page load: 3 validate_session calls before this branch's dependency work, 2 after (one in the middleware, one in the dependency). Without the memoization it would have been 5.

---

Regression tests

tests/auth/test_middleware_path_bypass.py (8 tests) builds a real admin app under TestClient — a string-PK model holding a row keyed "login" whose body is a sentinel string, plus a model with relationships — and asserts that each crafted URL is redirected or rejected and that the sentinel never appears in a response body. Two further tests assert the genuine login page stays open to anonymous visitors, including with a trailing slash.

The fixture is module-scoped because CRUDAdmin registers its admin models on a shared declarative base, so only one instance can be constructed per process.

The app is mounted twice — at /admin and at /backoffice — so one fixture covers both the middleware path and the middleware-skipped path. test_route_dependencies_hold_when_the_middleware_is_skipped drives the second mount and asserts 401. test_protected_routes_declare_a_real_auth_dependency inspects route.dependant and asserts each protected route declares an awaitable dependency, which is what catches the Depends(factory) mistake structurally rather than by symptom.

Verified as regressions rather than assumed: 7 of the 8 original tests fail with the middleware change stashed, and 3 of the 5 new tests fail with the dependency fixes stashed.

---

The security policy is now published

docs/community/ mirrors the root governance files through pymdownx snippet includes, but SECURITY.md was missing from that set — the private-reporting process lived only on GitHub, not on the docs site. This adds docs/community/SECURITY.md (a one-line --8<-- "SECURITY.md", matching the three existing mirrors), a Security Policy section in docs/community/overview.md, and the nav entry in zensical.toml.

SECURITY.md itself gains an Acknowledgments section — a table of reporters of fixed issues, opened by this report. The ModelView.setup_routes docstring, which feeds the published API reference, now states that routes declare an authentication dependency alongside the middleware.

---

Version bump

pyproject.toml and uv.lock move to 0.5.1. Nothing else hardcodes the version: crudadmin.__version__ reads it from package metadata, and tests/test_version.py compares against installed metadata rather than a literal.

---

Test Plan

Automated

• uv run pytest — 361 passed (13 new), SQLite; Postgres/MySQL container paths run in CI
• uv run ruff check crudadmin tests / ruff format --check — clean
• uv run mypy crudadmin — clean (43 source files)
• uv run zensical build — no issues; the rendered site/community/SECURITY/index.html carries the Acknowledgments section and the nav entry

The bypass is closed

• /admin/Article/update/login (string PK, row keyed "login") — 303 to the login page; the sentinel body is absent from the response
• /admin/Author/update/login (integer PK) — 303, no unauthenticated 500
• /admin/Author/related/1/login and /admin/Author/related/static/books — 303
• /admin/Author/relationship-options/login — 303

Normal use is unaffected

• GET /admin/login still returns 200 with the login form to an anonymous visitor
• GET /admin/login/ no longer redirects to an error-carrying login URL
• Authenticated browsing, static assets, and every pre-existing auth/session/cache-header test unchanged

The dependencies actually enforce

• With the app mounted off its mount_path (middleware prefix check misses), /backoffice/, /backoffice/Article/, /backoffice/Article/update/login, and /backoffice/management/health all return 401 — they returned 200 before
• Every protected route declares an awaitable auth dependency, asserted structurally against route.dependant
• Authenticated flow re-verified end to end: login 303 + cookie, dashboard/list/update/health/dashboard-content all 200, logout, then dashboard redirects again
• validate_session calls per authenticated request: 2 (was 3; would have been 5 without memoizing the dependency)

Regression value confirmed

• 7 of the 8 original tests fail with the middleware change stashed
• 3 of the 5 new tests fail with the dependency fixes stashed
• With the old middleware restored and only the route dependencies in place, all 6 bypass tests still pass

---

Dependencies

None added, removed, or re-pinned. The only uv.lock change is the editable self-reference moving 0.5.0 → 0.5.1.

Breaking Changes

None to the public API. Three behavior changes worth knowing about, all documented in the release notes:

• /admin/login/ (trailing slash) previously redirected to the login page with an error= query parameter; it now resolves to the login page normally.
• The dashboard, health, and event-log routes now genuinely require a session. They were always intended to, and the middleware already enforced it for normally-mounted apps, so no supported configuration changes behavior. An app that mounted admin.app off its mount_path was serving those pages unauthenticated and will now get 401s — that was the bug, not the fix.
• Static assets are matched against the admin prefix. If admin.app is mounted at a path that differs from the configured mount_path, static files are no longer served unauthenticated. That configuration was already broken — the middleware's outer prefix check skipped authentication entirely for it — so the fix is to set mount_path to match the actual mount point.