mirror of
https://github.com/calibrain/shelfmark.git
synced 2026-10-05 16:31:14 +01:00
fix(download): default is_admin to False in the request policy guard (#1358)
`_resolve_policy_mode_for_current_user` reads `session.get("is_admin",
True)`, so a session carrying `user_id` but no `is_admin` key skips the
request policy entirely, while every other admin check in the codebase
defaults the key to `False`.
This uses the same default here. Every authenticated login path
(builtin, CWA, proxy, OIDC) writes `is_admin` into the session, and
`AUTH_METHOD=none` is already short-circuited one line earlier, so
sessions from those flows behave exactly as before.
## Verification
- `tests/core/test_request_routes_api.py::TestDownloadPolicyGuards`: a
session without `is_admin` now gets `policy_requires_request` and
nothing is queued. Fails on current main, passes here.
- Full suite (3147), ruff, ruff format, basedpyright, vulture green.
This commit is contained in:
+1
-1
@@ -398,7 +398,7 @@ def _resolve_policy_mode_for_current_user(*, source: Any, content_type: Any) ->
|
||||
auth_mode = get_auth_mode()
|
||||
if auth_mode == "none":
|
||||
return None
|
||||
if session.get("is_admin", True):
|
||||
if session.get("is_admin", False):
|
||||
return None
|
||||
if user_db is None:
|
||||
return None
|
||||
|
||||
@@ -98,6 +98,38 @@ class TestDownloadPolicyGuards:
|
||||
assert resp.json["required_mode"] == "request_release"
|
||||
mock_queue_release.assert_not_called()
|
||||
|
||||
def test_release_download_endpoint_applies_policy_when_session_lacks_is_admin(
|
||||
self, main_module, client
|
||||
):
|
||||
user = _create_user(main_module, prefix="reader")
|
||||
with client.session_transaction() as sess:
|
||||
sess["user_id"] = user["username"]
|
||||
sess["db_user_id"] = user["id"]
|
||||
sess.pop("is_admin", None)
|
||||
|
||||
with patch.object(main_module, "get_auth_mode", return_value="builtin"):
|
||||
with patch.object(
|
||||
main_module,
|
||||
"load_users_request_policy_settings",
|
||||
return_value=_policy(default_ebook="request_release"),
|
||||
):
|
||||
with patch.object(
|
||||
main_module.backend, "queue_release", return_value=(True, None)
|
||||
) as mock_queue_release:
|
||||
resp = client.post(
|
||||
"/api/releases/download",
|
||||
json={
|
||||
"source": "direct_download",
|
||||
"source_id": "book-123",
|
||||
"search_mode": "direct",
|
||||
},
|
||||
)
|
||||
|
||||
assert resp.status_code == 403
|
||||
assert resp.json["code"] == "policy_requires_request"
|
||||
assert resp.json["required_mode"] == "request_release"
|
||||
mock_queue_release.assert_not_called()
|
||||
|
||||
def test_release_download_endpoint_blocks_before_queue_when_policy_blocked(
|
||||
self, main_module, client
|
||||
):
|
||||
|
||||
Reference in New Issue
Block a user