mirror of
https://github.com/calibrain/shelfmark.git
synced 2026-10-05 21:31:15 +01:00
Patch: Certificate validation setting + Misc fixes (#642)
- Add certificate validation setting - Fix some OIDC providers not linking emails to local users - Reintroduce sort by peers option for prowlarr results - Fix "All languages" search query reverting to default language - Fix download/request dismissal with multiple admin users - Fix download / request behavior on details modal
This commit is contained in:
@@ -279,6 +279,40 @@ class TestActivityRoutes:
|
||||
assert "expired-task-1" in response.json["status"]["complete"]
|
||||
assert response.json["status"]["complete"]["expired-task-1"]["id"] == "expired-task-1"
|
||||
|
||||
def test_admin_snapshot_backfills_terminal_downloads_across_users(self, main_module, client):
|
||||
admin = _create_user(main_module, prefix="admin", role="admin")
|
||||
request_owner = _create_user(main_module, prefix="reader")
|
||||
_set_session(client, user_id=admin["username"], db_user_id=admin["id"], is_admin=True)
|
||||
|
||||
main_module.activity_service.record_terminal_snapshot(
|
||||
user_id=request_owner["id"],
|
||||
item_type="download",
|
||||
item_key="download:cross-user-expired-task",
|
||||
origin="requested",
|
||||
final_status="complete",
|
||||
source_id="cross-user-expired-task",
|
||||
snapshot={
|
||||
"kind": "download",
|
||||
"download": {
|
||||
"id": "cross-user-expired-task",
|
||||
"title": "Cross User Task",
|
||||
"author": "Another User",
|
||||
"added_time": 123,
|
||||
"status_message": "Finished",
|
||||
"source": "direct_download",
|
||||
"user_id": request_owner["id"],
|
||||
},
|
||||
},
|
||||
)
|
||||
|
||||
with patch.object(main_module, "get_auth_mode", return_value="builtin"):
|
||||
with patch.object(main_module.backend, "queue_status", return_value=_sample_status_payload()):
|
||||
response = client.get("/api/activity/snapshot")
|
||||
|
||||
assert response.status_code == 200
|
||||
assert "cross-user-expired-task" in response.json["status"]["complete"]
|
||||
assert response.json["status"]["complete"]["cross-user-expired-task"]["id"] == "cross-user-expired-task"
|
||||
|
||||
def test_snapshot_clears_stale_download_dismissal_when_same_task_is_active(self, main_module, client):
|
||||
user = _create_user(main_module, prefix="reader")
|
||||
_set_session(client, user_id=user["username"], db_user_id=user["id"], is_admin=False)
|
||||
@@ -332,6 +366,29 @@ class TestActivityRoutes:
|
||||
assert snapshot_two.status_code == 200
|
||||
assert {"item_type": "download", "item_key": "download:shared-task"} not in snapshot_two.json["dismissed"]
|
||||
|
||||
def test_admin_request_dismissal_is_shared_across_admin_users(self, main_module, client):
|
||||
admin_one = _create_user(main_module, prefix="admin-one", role="admin")
|
||||
admin_two = _create_user(main_module, prefix="admin-two", role="admin")
|
||||
|
||||
with patch.object(main_module, "get_auth_mode", return_value="builtin"):
|
||||
_set_session(client, user_id=admin_one["username"], db_user_id=admin_one["id"], is_admin=True)
|
||||
dismiss_response = client.post(
|
||||
"/api/activity/dismiss",
|
||||
json={"item_type": "request", "item_key": "request:999999"},
|
||||
)
|
||||
assert dismiss_response.status_code == 200
|
||||
|
||||
_set_session(client, user_id=admin_two["username"], db_user_id=admin_two["id"], is_admin=True)
|
||||
with patch.object(main_module.backend, "queue_status", return_value=_sample_status_payload()):
|
||||
snapshot_response = client.get("/api/activity/snapshot")
|
||||
history_response = client.get("/api/activity/history?limit=50&offset=0")
|
||||
|
||||
assert snapshot_response.status_code == 200
|
||||
assert {"item_type": "request", "item_key": "request:999999"} in snapshot_response.json["dismissed"]
|
||||
|
||||
assert history_response.status_code == 200
|
||||
assert any(row["item_key"] == "request:999999" for row in history_response.json)
|
||||
|
||||
def test_history_paging_is_stable_and_non_overlapping(self, main_module, client):
|
||||
user = _create_user(main_module, prefix="history-user")
|
||||
_set_session(client, user_id=user["username"], db_user_id=user["id"], is_admin=False)
|
||||
|
||||
@@ -235,7 +235,10 @@ class TestActivityService:
|
||||
item_key="download:task-2",
|
||||
)
|
||||
|
||||
rows = activity_service.get_undismissed_terminal_downloads(user["id"])
|
||||
rows = activity_service.get_undismissed_terminal_downloads(
|
||||
user["id"],
|
||||
owner_user_id=user["id"],
|
||||
)
|
||||
assert len(rows) == 1
|
||||
assert rows[0]["item_key"] == "download:task-1"
|
||||
assert rows[0]["final_status"] == "complete"
|
||||
@@ -243,3 +246,50 @@ class TestActivityService:
|
||||
"kind": "download",
|
||||
"download": {"id": "task-1", "status_message": "done"},
|
||||
}
|
||||
|
||||
def test_get_undismissed_terminal_downloads_can_span_owners_for_admin_viewer(
|
||||
self,
|
||||
user_db,
|
||||
activity_service,
|
||||
):
|
||||
viewer = user_db.create_user(username="admin-viewer", role="admin")
|
||||
owner_one = user_db.create_user(username="owner-one")
|
||||
owner_two = user_db.create_user(username="owner-two")
|
||||
|
||||
activity_service.record_terminal_snapshot(
|
||||
user_id=owner_one["id"],
|
||||
item_type="download",
|
||||
item_key="download:owner-one-task",
|
||||
origin="direct",
|
||||
final_status="complete",
|
||||
source_id="owner-one-task",
|
||||
terminal_at="2026-01-01T10:00:00+00:00",
|
||||
snapshot={"kind": "download", "download": {"id": "owner-one-task"}},
|
||||
)
|
||||
activity_service.record_terminal_snapshot(
|
||||
user_id=owner_two["id"],
|
||||
item_type="download",
|
||||
item_key="download:owner-two-task",
|
||||
origin="direct",
|
||||
final_status="complete",
|
||||
source_id="owner-two-task",
|
||||
terminal_at="2026-01-01T11:00:00+00:00",
|
||||
snapshot={"kind": "download", "download": {"id": "owner-two-task"}},
|
||||
)
|
||||
activity_service.dismiss_item(
|
||||
user_id=viewer["id"],
|
||||
item_type="download",
|
||||
item_key="download:owner-two-task",
|
||||
)
|
||||
|
||||
all_owner_rows = activity_service.get_undismissed_terminal_downloads(
|
||||
viewer["id"],
|
||||
owner_user_id=None,
|
||||
)
|
||||
assert [row["item_key"] for row in all_owner_rows] == ["download:owner-one-task"]
|
||||
|
||||
owner_one_rows = activity_service.get_undismissed_terminal_downloads(
|
||||
viewer["id"],
|
||||
owner_user_id=owner_one["id"],
|
||||
)
|
||||
assert [row["item_key"] for row in owner_one_rows] == ["download:owner-one-task"]
|
||||
|
||||
@@ -259,3 +259,51 @@ class TestProvisionOIDCUser:
|
||||
assert user["username"] != "john" # Should have a suffix
|
||||
assert user["oidc_subject"] == "sub-456"
|
||||
assert user["auth_source"] == "oidc"
|
||||
|
||||
def test_provision_links_to_existing_user_by_email(self, user_db):
|
||||
"""When allow_email_link=True and emails match, link to existing local user."""
|
||||
from shelfmark.core.oidc_auth import provision_oidc_user
|
||||
user_db.create_user(
|
||||
username="localuser",
|
||||
email="shared@example.com",
|
||||
password_hash="hash",
|
||||
)
|
||||
|
||||
user_info = {
|
||||
"oidc_subject": "oidc-sub-789",
|
||||
"username": "oidcuser",
|
||||
"email": "shared@example.com",
|
||||
"display_name": "OIDC User",
|
||||
}
|
||||
user = provision_oidc_user(
|
||||
user_db, user_info, is_admin=False, allow_email_link=True,
|
||||
)
|
||||
assert user["username"] == "localuser"
|
||||
assert user["oidc_subject"] == "oidc-sub-789"
|
||||
assert user["auth_source"] == "oidc"
|
||||
assert user["email"] == "shared@example.com"
|
||||
|
||||
def test_provision_does_not_link_by_email_when_disabled(self, user_db):
|
||||
"""When allow_email_link=False (default), don't link by email."""
|
||||
from shelfmark.core.oidc_auth import provision_oidc_user
|
||||
user_db.create_user(
|
||||
username="localuser",
|
||||
email="shared@example.com",
|
||||
password_hash="hash",
|
||||
)
|
||||
|
||||
user_info = {
|
||||
"oidc_subject": "oidc-sub-no-link",
|
||||
"username": "oidcuser",
|
||||
"email": "shared@example.com",
|
||||
"display_name": "OIDC User",
|
||||
}
|
||||
user = provision_oidc_user(
|
||||
user_db, user_info, is_admin=False, allow_email_link=False,
|
||||
)
|
||||
# Should create a new user, not link to existing
|
||||
assert user["username"] == "oidcuser"
|
||||
assert user["oidc_subject"] == "oidc-sub-no-link"
|
||||
|
||||
original = user_db.get_user(username="localuser")
|
||||
assert original["oidc_subject"] is None
|
||||
|
||||
@@ -253,13 +253,14 @@ class TestOIDCCallbackEndpoint:
|
||||
assert "issuer validation failed" in error
|
||||
|
||||
@patch("shelfmark.core.oidc_routes._get_oidc_client")
|
||||
def test_callback_redirects_when_auto_provision_disabled(self, mock_get_client, client):
|
||||
def test_callback_redirects_when_auto_provision_disabled_and_no_email_match(
|
||||
self, mock_get_client, client
|
||||
):
|
||||
config = {**MOCK_OIDC_CONFIG, "OIDC_AUTO_PROVISION": False}
|
||||
fake_client = Mock()
|
||||
fake_client.authorize_access_token.return_value = {
|
||||
"userinfo": {
|
||||
"sub": "unknown-user",
|
||||
"email": "unknown@example.com",
|
||||
"preferred_username": "unknown",
|
||||
"groups": [],
|
||||
}
|
||||
@@ -272,7 +273,7 @@ class TestOIDCCallbackEndpoint:
|
||||
assert "Account not found" in error
|
||||
|
||||
@patch("shelfmark.core.oidc_routes._get_oidc_client")
|
||||
def test_callback_allows_pre_created_user_by_verified_email_when_no_provision(
|
||||
def test_callback_links_pre_created_user_by_email_when_no_provision(
|
||||
self, mock_get_client, client, user_db
|
||||
):
|
||||
config = {**MOCK_OIDC_CONFIG, "OIDC_AUTO_PROVISION": False}
|
||||
@@ -283,7 +284,6 @@ class TestOIDCCallbackEndpoint:
|
||||
"userinfo": {
|
||||
"sub": "oidc-alice-sub",
|
||||
"email": "alice@example.com",
|
||||
"email_verified": True,
|
||||
"preferred_username": "alice_oidc",
|
||||
"groups": [],
|
||||
}
|
||||
@@ -298,18 +298,16 @@ class TestOIDCCallbackEndpoint:
|
||||
assert sess.get("db_user_id") is not None
|
||||
|
||||
@patch("shelfmark.core.oidc_routes._get_oidc_client")
|
||||
def test_callback_does_not_link_unverified_email_when_no_provision(
|
||||
def test_callback_does_not_link_when_no_email_and_no_provision(
|
||||
self, mock_get_client, client, user_db
|
||||
):
|
||||
config = {**MOCK_OIDC_CONFIG, "OIDC_AUTO_PROVISION": False}
|
||||
user = user_db.create_user(username="bob", email="bob@example.com", password_hash="hash")
|
||||
user_db.create_user(username="bob", email="bob@example.com", password_hash="hash")
|
||||
|
||||
fake_client = Mock()
|
||||
fake_client.authorize_access_token.return_value = {
|
||||
"userinfo": {
|
||||
"sub": "oidc-bob-sub",
|
||||
"email": "bob@example.com",
|
||||
"email_verified": False,
|
||||
"preferred_username": "bob_oidc",
|
||||
"groups": [],
|
||||
}
|
||||
@@ -321,7 +319,7 @@ class TestOIDCCallbackEndpoint:
|
||||
assert error is not None
|
||||
assert "Account not found" in error
|
||||
|
||||
updated_user = user_db.get_user(user_id=user["id"])
|
||||
updated_user = user_db.get_user(username="bob")
|
||||
assert updated_user["oidc_subject"] is None
|
||||
|
||||
@patch("shelfmark.core.oidc_routes._get_oidc_client")
|
||||
@@ -359,3 +357,84 @@ class TestOIDCCallbackEndpoint:
|
||||
error = _get_oidc_error(resp)
|
||||
assert error is not None
|
||||
assert "Authentication failed" in error
|
||||
|
||||
@patch("shelfmark.core.oidc_routes._get_oidc_client")
|
||||
def test_callback_links_to_existing_user_by_email(
|
||||
self, mock_get_client, client, user_db
|
||||
):
|
||||
"""OIDC login with matching email should link to existing local user."""
|
||||
user_db.create_user(username="localuser", email="shared@example.com", password_hash="hash")
|
||||
|
||||
fake_client = Mock()
|
||||
fake_client.authorize_access_token.return_value = {
|
||||
"userinfo": {
|
||||
"sub": "oidc-new-sub",
|
||||
"email": "shared@example.com",
|
||||
"preferred_username": "oidcuser",
|
||||
"groups": [],
|
||||
}
|
||||
}
|
||||
mock_get_client.return_value = (fake_client, MOCK_OIDC_CONFIG)
|
||||
|
||||
resp = client.get("/api/auth/oidc/callback?code=abc123&state=test-state")
|
||||
assert resp.status_code == 302
|
||||
|
||||
with client.session_transaction() as sess:
|
||||
assert sess["user_id"] == "localuser"
|
||||
|
||||
linked = user_db.get_user(username="localuser")
|
||||
assert linked["oidc_subject"] == "oidc-new-sub"
|
||||
assert linked["auth_source"] == "oidc"
|
||||
|
||||
@patch("shelfmark.core.oidc_routes._get_oidc_client")
|
||||
def test_callback_creates_new_user_when_no_email_match(
|
||||
self, mock_get_client, client, user_db
|
||||
):
|
||||
"""OIDC login without matching email creates a new user."""
|
||||
user_db.create_user(username="existing", email="other@example.com", password_hash="hash")
|
||||
|
||||
fake_client = Mock()
|
||||
fake_client.authorize_access_token.return_value = {
|
||||
"userinfo": {
|
||||
"sub": "oidc-nomatch",
|
||||
"email": "different@example.com",
|
||||
"preferred_username": "newuser",
|
||||
"groups": [],
|
||||
}
|
||||
}
|
||||
mock_get_client.return_value = (fake_client, MOCK_OIDC_CONFIG)
|
||||
|
||||
resp = client.get("/api/auth/oidc/callback?code=abc123&state=test-state")
|
||||
assert resp.status_code == 302
|
||||
|
||||
with client.session_transaction() as sess:
|
||||
assert sess["user_id"] == "newuser"
|
||||
|
||||
original = user_db.get_user(username="existing")
|
||||
assert original["oidc_subject"] is None
|
||||
|
||||
@patch("shelfmark.core.oidc_routes._get_oidc_client")
|
||||
def test_callback_no_email_link_when_oidc_has_no_email(
|
||||
self, mock_get_client, client, user_db
|
||||
):
|
||||
"""OIDC login without email in claims should not attempt email linking."""
|
||||
user_db.create_user(username="existing", email="existing@example.com", password_hash="hash")
|
||||
|
||||
fake_client = Mock()
|
||||
fake_client.authorize_access_token.return_value = {
|
||||
"userinfo": {
|
||||
"sub": "oidc-noemail",
|
||||
"preferred_username": "noemailuser",
|
||||
"groups": [],
|
||||
}
|
||||
}
|
||||
mock_get_client.return_value = (fake_client, MOCK_OIDC_CONFIG)
|
||||
|
||||
resp = client.get("/api/auth/oidc/callback?code=abc123&state=test-state")
|
||||
assert resp.status_code == 302
|
||||
|
||||
with client.session_transaction() as sess:
|
||||
assert sess["user_id"] == "noemailuser"
|
||||
|
||||
original = user_db.get_user(username="existing")
|
||||
assert original["oidc_subject"] is None
|
||||
|
||||
Reference in New Issue
Block a user