mirror of
https://github.com/calibrain/shelfmark.git
synced 2026-10-05 22:05:50 +01:00
Patch: Multi-user and OIDC polish (#612)
- Moved backend OIDC functionality to external library Authlib to help maintainability - Separated User settings UI into individual components, allowing for standard settings UI decorator components to be used. - Added full support for reverse proxy and CWA users alongside local and OIDC - Added mapping and syncing functionality for OIDC, CWA and reverse proxy users - Added per-user settings into the app-wide config system. Each config can be declared as user-overrideable, and app-wide functionality can now receive user-specific options via standard config calls. - Added per-user audiobook destination config - Updated login modal UI for simplified login, plus custom labels for OIDC login - Added user visibility in header dropdown - Unified "restrict settings to admin" to use app-wide user roles.
This commit is contained in:
@@ -5,6 +5,7 @@ Tests CRUD endpoints for managing users from the admin panel.
|
||||
"""
|
||||
|
||||
import os
|
||||
import sqlite3
|
||||
import tempfile
|
||||
|
||||
from unittest.mock import patch
|
||||
@@ -105,10 +106,61 @@ class TestAdminUsersListEndpoint:
|
||||
users = resp.json
|
||||
assert "password_hash" not in users[0]
|
||||
|
||||
def test_list_users_includes_auth_source_and_is_active(self, admin_client, user_db):
|
||||
user_db.create_user(username="local_user", auth_source="builtin")
|
||||
user_db.create_user(
|
||||
username="oidc_user",
|
||||
oidc_subject="oidc-sub-123",
|
||||
auth_source="oidc",
|
||||
)
|
||||
user_db.create_user(username="proxy_user", auth_source="proxy")
|
||||
|
||||
with patch("shelfmark.core.admin_routes._get_auth_mode", return_value="builtin"):
|
||||
resp = admin_client.get("/api/admin/users")
|
||||
|
||||
assert resp.status_code == 200
|
||||
by_username = {u["username"]: u for u in resp.json}
|
||||
|
||||
assert by_username["local_user"]["auth_source"] == "builtin"
|
||||
assert by_username["local_user"]["is_active"] is True
|
||||
assert by_username["local_user"]["edit_capabilities"]["canSetPassword"] is True
|
||||
assert by_username["local_user"]["edit_capabilities"]["canEditRole"] is True
|
||||
assert by_username["local_user"]["edit_capabilities"]["canEditEmail"] is True
|
||||
|
||||
assert by_username["oidc_user"]["auth_source"] == "oidc"
|
||||
assert by_username["oidc_user"]["is_active"] is False
|
||||
assert by_username["oidc_user"]["edit_capabilities"]["canSetPassword"] is False
|
||||
assert by_username["oidc_user"]["edit_capabilities"]["canEditRole"] is False
|
||||
assert by_username["oidc_user"]["edit_capabilities"]["canEditEmail"] is False
|
||||
assert by_username["oidc_user"]["edit_capabilities"]["canEditDisplayName"] is False
|
||||
|
||||
assert by_username["proxy_user"]["auth_source"] == "proxy"
|
||||
assert by_username["proxy_user"]["is_active"] is False
|
||||
assert by_username["proxy_user"]["edit_capabilities"]["canSetPassword"] is False
|
||||
assert by_username["proxy_user"]["edit_capabilities"]["canEditRole"] is False
|
||||
assert by_username["proxy_user"]["edit_capabilities"]["canEditEmail"] is True
|
||||
|
||||
def test_list_users_requires_admin(self, regular_client):
|
||||
resp = regular_client.get("/api/admin/users")
|
||||
assert resp.status_code == 403
|
||||
|
||||
def test_list_users_oidc_role_editable_when_group_auth_disabled(self, admin_client, user_db):
|
||||
user_db.create_user(
|
||||
username="oidc_user",
|
||||
oidc_subject="oidc-sub-123",
|
||||
auth_source="oidc",
|
||||
)
|
||||
|
||||
with patch(
|
||||
"shelfmark.core.admin_routes.load_config_file",
|
||||
return_value={"OIDC_USE_ADMIN_GROUP": False},
|
||||
):
|
||||
resp = admin_client.get("/api/admin/users")
|
||||
|
||||
assert resp.status_code == 200
|
||||
oidc_user = next(u for u in resp.json if u["username"] == "oidc_user")
|
||||
assert oidc_user["edit_capabilities"]["canEditRole"] is True
|
||||
|
||||
def test_list_users_no_session_allows_access_in_no_auth(self, no_session_client):
|
||||
"""No session + no-auth mode = admin access allowed."""
|
||||
resp = no_session_client.get("/api/admin/users")
|
||||
@@ -273,6 +325,36 @@ class TestAdminUserCreateEndpoint:
|
||||
assert resp.status_code == 201
|
||||
assert resp.json["role"] == "user"
|
||||
|
||||
def test_create_user_rejected_in_proxy_mode(self, admin_client):
|
||||
with patch("shelfmark.core.admin_routes._get_auth_mode", return_value="proxy"):
|
||||
resp = admin_client.post(
|
||||
"/api/admin/users",
|
||||
json={"username": "alice", "password": "pass1234"},
|
||||
)
|
||||
|
||||
assert resp.status_code == 400
|
||||
assert "Local user creation is disabled" in resp.json["error"]
|
||||
|
||||
def test_create_user_rejected_in_cwa_mode(self, admin_client):
|
||||
with patch("shelfmark.core.admin_routes._get_auth_mode", return_value="cwa"):
|
||||
resp = admin_client.post(
|
||||
"/api/admin/users",
|
||||
json={"username": "alice", "password": "pass1234"},
|
||||
)
|
||||
|
||||
assert resp.status_code == 400
|
||||
assert "Local user creation is disabled" in resp.json["error"]
|
||||
|
||||
def test_create_user_allowed_in_oidc_mode(self, admin_client):
|
||||
with patch("shelfmark.core.admin_routes._get_auth_mode", return_value="oidc"):
|
||||
resp = admin_client.post(
|
||||
"/api/admin/users",
|
||||
json={"username": "alice", "password": "pass1234"},
|
||||
)
|
||||
|
||||
assert resp.status_code == 201
|
||||
assert resp.json["username"] == "alice"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# GET /api/admin/users/<id>
|
||||
@@ -292,10 +374,10 @@ class TestAdminUserGetEndpoint:
|
||||
|
||||
def test_get_user_includes_settings(self, admin_client, user_db):
|
||||
user = user_db.create_user(username="alice")
|
||||
user_db.set_user_settings(user["id"], {"booklore_library_id": 5})
|
||||
user_db.set_user_settings(user["id"], {"BOOKLORE_LIBRARY_ID": 5})
|
||||
|
||||
resp = admin_client.get(f"/api/admin/users/{user['id']}")
|
||||
assert resp.json["settings"]["booklore_library_id"] == 5
|
||||
assert resp.json["settings"]["BOOKLORE_LIBRARY_ID"] == 5
|
||||
|
||||
def test_get_user_empty_settings(self, admin_client, user_db):
|
||||
user = user_db.create_user(username="alice")
|
||||
@@ -375,27 +457,38 @@ class TestAdminUserUpdateEndpoint:
|
||||
|
||||
resp = admin_client.put(
|
||||
f"/api/admin/users/{user['id']}",
|
||||
json={"settings": {"booklore_library_id": 3}},
|
||||
json={"settings": {"BOOKLORE_LIBRARY_ID": 3}},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
settings = user_db.get_user_settings(user["id"])
|
||||
assert settings["booklore_library_id"] == 3
|
||||
assert settings["BOOKLORE_LIBRARY_ID"] == 3
|
||||
|
||||
def test_update_settings_merges(self, admin_client, user_db):
|
||||
def test_update_user_settings_accepts_audiobook_destination(self, admin_client, user_db):
|
||||
user = user_db.create_user(username="alice")
|
||||
user_db.set_user_settings(user["id"], {"existing_key": "keep"})
|
||||
|
||||
resp = admin_client.put(
|
||||
f"/api/admin/users/{user['id']}",
|
||||
json={"settings": {"new_key": "added"}},
|
||||
json={"settings": {"DESTINATION_AUDIOBOOK": "/audiobooks/alice"}},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
assert resp.json["settings"]["existing_key"] == "keep"
|
||||
assert resp.json["settings"]["new_key"] == "added"
|
||||
settings = user_db.get_user_settings(user["id"])
|
||||
assert settings["DESTINATION_AUDIOBOOK"] == "/audiobooks/alice"
|
||||
|
||||
def test_update_settings_merges(self, admin_client, user_db):
|
||||
user = user_db.create_user(username="alice")
|
||||
user_db.set_user_settings(user["id"], {"DESTINATION": "/books/alice"})
|
||||
|
||||
resp = admin_client.put(
|
||||
f"/api/admin/users/{user['id']}",
|
||||
json={"settings": {"BOOKLORE_LIBRARY_ID": "2"}},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
assert resp.json["settings"]["DESTINATION"] == "/books/alice"
|
||||
assert resp.json["settings"]["BOOKLORE_LIBRARY_ID"] == "2"
|
||||
|
||||
def test_update_response_includes_settings(self, admin_client, user_db):
|
||||
user = user_db.create_user(username="alice")
|
||||
user_db.set_user_settings(user["id"], {"theme": "dark"})
|
||||
user_db.set_user_settings(user["id"], {"DESTINATION": "/books/alice"})
|
||||
|
||||
resp = admin_client.put(
|
||||
f"/api/admin/users/{user['id']}",
|
||||
@@ -403,7 +496,40 @@ class TestAdminUserUpdateEndpoint:
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
assert "settings" in resp.json
|
||||
assert resp.json["settings"]["theme"] == "dark"
|
||||
assert resp.json["settings"]["DESTINATION"] == "/books/alice"
|
||||
|
||||
def test_update_user_settings_rejects_unknown_key(self, admin_client, user_db):
|
||||
user = user_db.create_user(username="alice")
|
||||
|
||||
resp = admin_client.put(
|
||||
f"/api/admin/users/{user['id']}",
|
||||
json={"settings": {"UNKNOWN_SETTING": "value"}},
|
||||
)
|
||||
assert resp.status_code == 400
|
||||
assert resp.json["error"] == "Invalid settings payload"
|
||||
assert any("Unknown setting: UNKNOWN_SETTING" in msg for msg in resp.json["details"])
|
||||
|
||||
def test_update_user_settings_rejects_non_overridable_key(self, admin_client, user_db):
|
||||
user = user_db.create_user(username="alice")
|
||||
|
||||
resp = admin_client.put(
|
||||
f"/api/admin/users/{user['id']}",
|
||||
json={"settings": {"FILE_ORGANIZATION": "rename"}},
|
||||
)
|
||||
assert resp.status_code == 400
|
||||
assert resp.json["error"] == "Invalid settings payload"
|
||||
assert any("Setting not user-overridable: FILE_ORGANIZATION" in msg for msg in resp.json["details"])
|
||||
|
||||
def test_update_user_settings_rejects_lowercase_key(self, admin_client, user_db):
|
||||
user = user_db.create_user(username="alice")
|
||||
|
||||
resp = admin_client.put(
|
||||
f"/api/admin/users/{user['id']}",
|
||||
json={"settings": {"destination": "/books/alice"}},
|
||||
)
|
||||
assert resp.status_code == 400
|
||||
assert resp.json["error"] == "Invalid settings payload"
|
||||
assert any("Unknown setting: destination" in msg for msg in resp.json["details"])
|
||||
|
||||
def test_update_response_excludes_password_hash(self, admin_client, user_db):
|
||||
user = user_db.create_user(username="alice", password_hash="secret")
|
||||
@@ -429,6 +555,59 @@ class TestAdminUserUpdateEndpoint:
|
||||
)
|
||||
assert resp.status_code == 403
|
||||
|
||||
def test_update_proxy_role_rejected(self, admin_client, user_db):
|
||||
user = user_db.create_user(username="proxyuser", role="user", auth_source="proxy")
|
||||
|
||||
resp = admin_client.put(
|
||||
f"/api/admin/users/{user['id']}",
|
||||
json={"role": "admin"},
|
||||
)
|
||||
|
||||
assert resp.status_code == 400
|
||||
assert "Cannot change role for PROXY users" in resp.json["error"]
|
||||
|
||||
def test_update_proxy_role_noop_allowed(self, admin_client, user_db):
|
||||
user = user_db.create_user(username="proxyuser", role="user", auth_source="proxy")
|
||||
|
||||
resp = admin_client.put(
|
||||
f"/api/admin/users/{user['id']}",
|
||||
json={"role": "user", "display_name": "Proxy User"},
|
||||
)
|
||||
|
||||
assert resp.status_code == 200
|
||||
assert resp.json["display_name"] == "Proxy User"
|
||||
|
||||
def test_update_cwa_email_rejected(self, admin_client, user_db):
|
||||
user = user_db.create_user(
|
||||
username="cwauser",
|
||||
email="old@example.com",
|
||||
auth_source="cwa",
|
||||
)
|
||||
|
||||
resp = admin_client.put(
|
||||
f"/api/admin/users/{user['id']}",
|
||||
json={"email": "new@example.com"},
|
||||
)
|
||||
|
||||
assert resp.status_code == 400
|
||||
assert "Cannot change email for CWA users" in resp.json["error"]
|
||||
|
||||
def test_update_oidc_email_rejected(self, admin_client, user_db):
|
||||
user = user_db.create_user(
|
||||
username="oidcuser",
|
||||
email="old@example.com",
|
||||
oidc_subject="sub-oidc-1",
|
||||
auth_source="oidc",
|
||||
)
|
||||
|
||||
resp = admin_client.put(
|
||||
f"/api/admin/users/{user['id']}",
|
||||
json={"email": "new@example.com"},
|
||||
)
|
||||
|
||||
assert resp.status_code == 400
|
||||
assert "Cannot change email for OIDC users" in resp.json["error"]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# PUT /api/admin/users/<id> — password update
|
||||
@@ -502,6 +681,117 @@ class TestAdminUserPasswordUpdate:
|
||||
assert "password_hash" not in resp.json
|
||||
assert "password" not in resp.json
|
||||
|
||||
def test_update_password_rejected_for_proxy_user(self, admin_client, user_db):
|
||||
user = user_db.create_user(username="proxyuser", auth_source="proxy")
|
||||
|
||||
resp = admin_client.put(
|
||||
f"/api/admin/users/{user['id']}",
|
||||
json={"password": "newpass99"},
|
||||
)
|
||||
|
||||
assert resp.status_code == 400
|
||||
assert "Cannot set password for PROXY users" in resp.json["error"]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# POST /api/admin/users/sync-cwa
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestAdminSyncCwaUsersEndpoint:
|
||||
"""Tests for POST /api/admin/users/sync-cwa."""
|
||||
|
||||
def test_sync_cwa_users_links_by_email_and_avoids_username_overwrite(
|
||||
self,
|
||||
admin_client,
|
||||
user_db,
|
||||
tmp_path,
|
||||
):
|
||||
cwa_db_path = tmp_path / "app.db"
|
||||
conn = sqlite3.connect(cwa_db_path)
|
||||
try:
|
||||
conn.execute(
|
||||
"""
|
||||
CREATE TABLE user (
|
||||
id INTEGER PRIMARY KEY AUTOINCREMENT,
|
||||
name TEXT,
|
||||
role INTEGER,
|
||||
email TEXT
|
||||
)
|
||||
"""
|
||||
)
|
||||
conn.executemany(
|
||||
"INSERT INTO user (name, role, email) VALUES (?, ?, ?)",
|
||||
[
|
||||
("alice", 1, "alice@example.com"),
|
||||
("bob", 0, "bob@example.com"),
|
||||
(" ", 1, "skip@example.com"),
|
||||
],
|
||||
)
|
||||
conn.commit()
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
local_email_match = user_db.create_user(
|
||||
username="alice_local",
|
||||
email="alice@example.com",
|
||||
role="user",
|
||||
auth_source="builtin",
|
||||
)
|
||||
local_username_collision = user_db.create_user(
|
||||
username="bob",
|
||||
email="old@example.com",
|
||||
role="admin",
|
||||
auth_source="builtin",
|
||||
)
|
||||
|
||||
with patch("shelfmark.core.admin_routes._get_auth_mode", return_value="cwa"):
|
||||
with patch("shelfmark.core.admin_routes.CWA_DB_PATH", cwa_db_path):
|
||||
resp = admin_client.post("/api/admin/users/sync-cwa")
|
||||
|
||||
assert resp.status_code == 200
|
||||
assert resp.json["success"] is True
|
||||
assert resp.json["created"] == 1
|
||||
assert resp.json["updated"] == 1
|
||||
assert resp.json["total"] == 2
|
||||
|
||||
alice_linked = user_db.get_user(user_id=local_email_match["id"])
|
||||
assert alice_linked is not None
|
||||
assert alice_linked["username"] == "alice_local"
|
||||
assert alice_linked["auth_source"] == "cwa"
|
||||
assert alice_linked["role"] == "admin"
|
||||
assert alice_linked["email"] == "alice@example.com"
|
||||
|
||||
bob_original = user_db.get_user(user_id=local_username_collision["id"])
|
||||
assert bob_original is not None
|
||||
assert bob_original["username"] == "bob"
|
||||
assert bob_original["auth_source"] == "builtin"
|
||||
assert bob_original["role"] == "admin"
|
||||
assert bob_original["email"] == "old@example.com"
|
||||
|
||||
bob_cwa = next(
|
||||
user for user in user_db.list_users()
|
||||
if user.get("auth_source") == "cwa" and user.get("email") == "bob@example.com"
|
||||
)
|
||||
assert bob_cwa["username"].startswith("bob__cwa")
|
||||
assert bob_cwa["role"] == "user"
|
||||
|
||||
def test_sync_cwa_users_rejected_when_not_in_cwa_mode(self, admin_client):
|
||||
with patch("shelfmark.core.admin_routes._get_auth_mode", return_value="builtin"):
|
||||
resp = admin_client.post("/api/admin/users/sync-cwa")
|
||||
|
||||
assert resp.status_code == 400
|
||||
assert "only available" in resp.json["error"]
|
||||
|
||||
def test_sync_cwa_users_returns_503_when_db_unavailable(self, admin_client, tmp_path):
|
||||
missing_db_path = tmp_path / "missing.db"
|
||||
with patch("shelfmark.core.admin_routes._get_auth_mode", return_value="cwa"):
|
||||
with patch("shelfmark.core.admin_routes.CWA_DB_PATH", missing_db_path):
|
||||
resp = admin_client.post("/api/admin/users/sync-cwa")
|
||||
|
||||
assert resp.status_code == 503
|
||||
assert "not available" in resp.json["error"]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# GET /api/admin/download-defaults
|
||||
@@ -525,9 +815,10 @@ class TestAdminDownloadDefaults:
|
||||
config = {
|
||||
"BOOKS_OUTPUT_MODE": "folder",
|
||||
"DESTINATION": "/books",
|
||||
"DESTINATION_AUDIOBOOK": "/audiobooks",
|
||||
"BOOKLORE_LIBRARY_ID": "2",
|
||||
"BOOKLORE_PATH_ID": "5",
|
||||
"EMAIL_RECIPIENTS": [{"nickname": "kindle", "email": "me@kindle.com"}],
|
||||
"EMAIL_RECIPIENT": "reader@example.com",
|
||||
}
|
||||
(plugins_dir / "downloads.json").write_text(json.dumps(config))
|
||||
|
||||
@@ -537,9 +828,10 @@ class TestAdminDownloadDefaults:
|
||||
data = resp.json
|
||||
assert data["BOOKS_OUTPUT_MODE"] == "folder"
|
||||
assert data["DESTINATION"] == "/books"
|
||||
assert data["DESTINATION_AUDIOBOOK"] == "/audiobooks"
|
||||
assert data["BOOKLORE_LIBRARY_ID"] == "2"
|
||||
assert data["BOOKLORE_PATH_ID"] == "5"
|
||||
assert data["EMAIL_RECIPIENTS"] == [{"nickname": "kindle", "email": "me@kindle.com"}]
|
||||
assert data["EMAIL_RECIPIENT"] == "reader@example.com"
|
||||
|
||||
def test_returns_defaults_when_no_config(self, admin_client, tmp_path):
|
||||
"""If no downloads config file exists, return sensible defaults."""
|
||||
@@ -553,6 +845,7 @@ class TestAdminDownloadDefaults:
|
||||
data = resp.json
|
||||
assert "BOOKS_OUTPUT_MODE" in data
|
||||
assert "DESTINATION" in data
|
||||
assert "DESTINATION_AUDIOBOOK" in data
|
||||
|
||||
def test_requires_admin(self, regular_client):
|
||||
resp = regular_client.get("/api/admin/download-defaults")
|
||||
@@ -599,6 +892,208 @@ class TestAdminBookloreOptions:
|
||||
assert resp.status_code == 403
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# GET /api/admin/users/<id>/delivery-preferences
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestAdminDeliveryPreferences:
|
||||
"""Tests for GET /api/admin/users/<id>/delivery-preferences."""
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def setup_config(self, tmp_path, monkeypatch):
|
||||
import json
|
||||
from pathlib import Path
|
||||
|
||||
config_dir = str(tmp_path)
|
||||
monkeypatch.setenv("CONFIG_DIR", config_dir)
|
||||
monkeypatch.setattr("shelfmark.config.env.CONFIG_DIR", Path(config_dir))
|
||||
|
||||
plugins_dir = tmp_path / "plugins"
|
||||
plugins_dir.mkdir()
|
||||
downloads_config = {
|
||||
"BOOKS_OUTPUT_MODE": "folder",
|
||||
"DESTINATION": "/books",
|
||||
"DESTINATION_AUDIOBOOK": "/audiobooks",
|
||||
"BOOKLORE_LIBRARY_ID": "7",
|
||||
"BOOKLORE_PATH_ID": "21",
|
||||
"EMAIL_RECIPIENT": "global@example.com",
|
||||
}
|
||||
(plugins_dir / "downloads.json").write_text(json.dumps(downloads_config))
|
||||
|
||||
from shelfmark.core.config import config as app_config
|
||||
app_config.refresh()
|
||||
|
||||
def test_returns_curated_fields_and_effective_values(self, admin_client, user_db):
|
||||
user = user_db.create_user(username="alice")
|
||||
user_db.set_user_settings(
|
||||
user["id"],
|
||||
{
|
||||
"BOOKS_OUTPUT_MODE": "email",
|
||||
"EMAIL_RECIPIENT": "alice@example.com",
|
||||
"DESTINATION_AUDIOBOOK": "/audiobooks/alice",
|
||||
},
|
||||
)
|
||||
|
||||
resp = admin_client.get(f"/api/admin/users/{user['id']}/delivery-preferences")
|
||||
assert resp.status_code == 200
|
||||
|
||||
data = resp.json
|
||||
assert data["tab"] == "downloads"
|
||||
assert data["keys"] == [
|
||||
"BOOKS_OUTPUT_MODE",
|
||||
"DESTINATION",
|
||||
"BOOKLORE_LIBRARY_ID",
|
||||
"BOOKLORE_PATH_ID",
|
||||
"EMAIL_RECIPIENT",
|
||||
"DESTINATION_AUDIOBOOK",
|
||||
]
|
||||
|
||||
field_keys = [field["key"] for field in data["fields"]]
|
||||
assert set(field_keys) == set(data["keys"])
|
||||
|
||||
assert data["userOverrides"]["BOOKS_OUTPUT_MODE"] == "email"
|
||||
assert data["userOverrides"]["EMAIL_RECIPIENT"] == "alice@example.com"
|
||||
assert data["userOverrides"]["DESTINATION_AUDIOBOOK"] == "/audiobooks/alice"
|
||||
|
||||
assert data["effective"]["BOOKS_OUTPUT_MODE"]["source"] == "user_override"
|
||||
assert data["effective"]["BOOKS_OUTPUT_MODE"]["value"] == "email"
|
||||
assert data["effective"]["DESTINATION"]["source"] in {"global_config", "env_var"}
|
||||
assert data["effective"]["BOOKLORE_LIBRARY_ID"]["source"] == "global_config"
|
||||
assert data["effective"]["BOOKLORE_LIBRARY_ID"]["value"] == "7"
|
||||
assert data["effective"]["EMAIL_RECIPIENT"]["source"] == "user_override"
|
||||
assert data["effective"]["EMAIL_RECIPIENT"]["value"] == "alice@example.com"
|
||||
assert data["effective"]["DESTINATION_AUDIOBOOK"]["source"] == "user_override"
|
||||
assert data["effective"]["DESTINATION_AUDIOBOOK"]["value"] == "/audiobooks/alice"
|
||||
|
||||
def test_returns_404_for_unknown_user(self, admin_client):
|
||||
resp = admin_client.get("/api/admin/users/9999/delivery-preferences")
|
||||
assert resp.status_code == 404
|
||||
|
||||
def test_requires_admin(self, regular_client, user_db):
|
||||
user = user_db.create_user(username="alice")
|
||||
resp = regular_client.get(f"/api/admin/users/{user['id']}/delivery-preferences")
|
||||
assert resp.status_code == 403
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# GET /api/admin/settings/overrides-summary
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestAdminOverridesSummary:
|
||||
"""Tests for GET /api/admin/settings/overrides-summary."""
|
||||
|
||||
def test_returns_override_counts_for_downloads_tab(self, admin_client, user_db):
|
||||
alice = user_db.create_user(username="alice")
|
||||
bob = user_db.create_user(username="bob")
|
||||
|
||||
user_db.set_user_settings(
|
||||
alice["id"],
|
||||
{"BOOKS_OUTPUT_MODE": "folder", "DESTINATION": "/books/alice"},
|
||||
)
|
||||
user_db.set_user_settings(
|
||||
bob["id"],
|
||||
{
|
||||
"BOOKS_OUTPUT_MODE": "email",
|
||||
"DESTINATION": "/books/bob",
|
||||
"EMAIL_RECIPIENT": "bob@example.com",
|
||||
},
|
||||
)
|
||||
|
||||
resp = admin_client.get("/api/admin/settings/overrides-summary?tab=downloads")
|
||||
assert resp.status_code == 200
|
||||
|
||||
data = resp.json
|
||||
assert data["tab"] == "downloads"
|
||||
keys = data["keys"]
|
||||
|
||||
assert keys["BOOKS_OUTPUT_MODE"]["count"] == 2
|
||||
assert keys["DESTINATION"]["count"] == 2
|
||||
assert keys["EMAIL_RECIPIENT"]["count"] == 1
|
||||
assert "BOOKLORE_LIBRARY_ID" not in keys
|
||||
|
||||
destination_users = {u["username"] for u in keys["DESTINATION"]["users"]}
|
||||
assert destination_users == {"alice", "bob"}
|
||||
|
||||
email_users = keys["EMAIL_RECIPIENT"]["users"]
|
||||
assert len(email_users) == 1
|
||||
assert email_users[0]["username"] == "bob"
|
||||
assert email_users[0]["value"] == "bob@example.com"
|
||||
|
||||
def test_returns_404_for_unknown_tab(self, admin_client):
|
||||
resp = admin_client.get("/api/admin/settings/overrides-summary?tab=does-not-exist")
|
||||
assert resp.status_code == 404
|
||||
|
||||
def test_requires_admin(self, regular_client):
|
||||
resp = regular_client.get("/api/admin/settings/overrides-summary?tab=downloads")
|
||||
assert resp.status_code == 403
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# GET /api/admin/users/<id>/effective-settings
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestAdminEffectiveSettings:
|
||||
"""Tests for GET /api/admin/users/<id>/effective-settings."""
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def setup_config(self, tmp_path, monkeypatch):
|
||||
import json
|
||||
from pathlib import Path
|
||||
|
||||
config_dir = str(tmp_path)
|
||||
monkeypatch.setenv("CONFIG_DIR", config_dir)
|
||||
monkeypatch.setattr("shelfmark.config.env.CONFIG_DIR", Path(config_dir))
|
||||
|
||||
plugins_dir = tmp_path / "plugins"
|
||||
plugins_dir.mkdir()
|
||||
downloads_config = {
|
||||
"BOOKS_OUTPUT_MODE": "booklore",
|
||||
"BOOKLORE_LIBRARY_ID": "7",
|
||||
}
|
||||
(plugins_dir / "downloads.json").write_text(json.dumps(downloads_config))
|
||||
|
||||
monkeypatch.setenv("INGEST_DIR", "/env/books")
|
||||
|
||||
# Ensure config singleton sees the current test env/config dir.
|
||||
from shelfmark.core.config import config as app_config
|
||||
app_config.refresh()
|
||||
|
||||
def test_returns_effective_values_with_sources(self, admin_client, user_db):
|
||||
user = user_db.create_user(username="alice")
|
||||
user_db.set_user_settings(
|
||||
user["id"],
|
||||
{"EMAIL_RECIPIENT": "alice@kindle.com"},
|
||||
)
|
||||
|
||||
resp = admin_client.get(f"/api/admin/users/{user['id']}/effective-settings")
|
||||
assert resp.status_code == 200
|
||||
|
||||
data = resp.json
|
||||
assert data["DESTINATION"]["value"] == "/env/books"
|
||||
assert data["DESTINATION"]["source"] == "env_var"
|
||||
|
||||
assert data["BOOKLORE_LIBRARY_ID"]["value"] == "7"
|
||||
assert data["BOOKLORE_LIBRARY_ID"]["source"] == "global_config"
|
||||
|
||||
assert data["BOOKLORE_PATH_ID"]["value"] in ("", None)
|
||||
assert data["BOOKLORE_PATH_ID"]["source"] == "default"
|
||||
|
||||
assert data["EMAIL_RECIPIENT"]["value"] == "alice@kindle.com"
|
||||
assert data["EMAIL_RECIPIENT"]["source"] == "user_override"
|
||||
|
||||
def test_returns_404_for_unknown_user(self, admin_client):
|
||||
resp = admin_client.get("/api/admin/users/9999/effective-settings")
|
||||
assert resp.status_code == 404
|
||||
|
||||
def test_requires_admin(self, regular_client, user_db):
|
||||
user = user_db.create_user(username="alice")
|
||||
resp = regular_client.get(f"/api/admin/users/{user['id']}/effective-settings")
|
||||
assert resp.status_code == 403
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# DELETE /api/admin/users/<id>
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -634,6 +1129,37 @@ class TestAdminUserDeleteEndpoint:
|
||||
assert len(resp.json) == 1
|
||||
assert resp.json[0]["username"] == "bob"
|
||||
|
||||
def test_delete_active_proxy_user_rejected(self, admin_client, user_db):
|
||||
user = user_db.create_user(username="proxyuser", auth_source="proxy")
|
||||
|
||||
with patch("shelfmark.core.admin_routes._get_auth_mode", return_value="proxy"):
|
||||
resp = admin_client.delete(f"/api/admin/users/{user['id']}")
|
||||
|
||||
assert resp.status_code == 400
|
||||
assert "Cannot delete active PROXY users" in resp.json["error"]
|
||||
|
||||
def test_delete_inactive_proxy_user_allowed(self, admin_client, user_db):
|
||||
user = user_db.create_user(username="proxyuser", auth_source="proxy")
|
||||
|
||||
with patch("shelfmark.core.admin_routes._get_auth_mode", return_value="builtin"):
|
||||
resp = admin_client.delete(f"/api/admin/users/{user['id']}")
|
||||
|
||||
assert resp.status_code == 200
|
||||
assert resp.json["success"] is True
|
||||
|
||||
def test_delete_active_oidc_user_allowed_when_auto_provision_enabled(self, admin_client, user_db):
|
||||
user = user_db.create_user(
|
||||
username="oidcuser",
|
||||
oidc_subject="sub-123",
|
||||
auth_source="oidc",
|
||||
)
|
||||
|
||||
with patch("shelfmark.core.admin_routes._get_auth_mode", return_value="oidc"):
|
||||
resp = admin_client.delete(f"/api/admin/users/{user['id']}")
|
||||
|
||||
assert resp.status_code == 200
|
||||
assert resp.json["success"] is True
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# OIDC lockout prevention (security on_save handler)
|
||||
|
||||
@@ -4,7 +4,7 @@ from shelfmark.download.outputs.booklore import build_booklore_config
|
||||
|
||||
|
||||
class TestBuildBookloreConfigWithOverrides:
|
||||
"""build_booklore_config should accept per-user library/path overrides."""
|
||||
"""build_booklore_config should resolve per-user library/path via config."""
|
||||
|
||||
BASE_SETTINGS = {
|
||||
"BOOKLORE_HOST": "http://booklore:6060",
|
||||
@@ -14,77 +14,62 @@ class TestBuildBookloreConfigWithOverrides:
|
||||
"BOOKLORE_PATH_ID": 10,
|
||||
}
|
||||
|
||||
def test_global_config_no_overrides(self):
|
||||
def test_global_config_without_user_context(self):
|
||||
config = build_booklore_config(self.BASE_SETTINGS)
|
||||
assert config.library_id == 1
|
||||
assert config.path_id == 10
|
||||
|
||||
def test_override_library_and_path(self):
|
||||
overrides = {"booklore_library_id": 2, "booklore_path_id": 20}
|
||||
config = build_booklore_config(self.BASE_SETTINGS, user_overrides=overrides)
|
||||
def test_override_library_and_path_with_user_context(self, monkeypatch):
|
||||
def fake_get(key, default=None, user_id=None):
|
||||
if user_id == 7 and key == "BOOKLORE_LIBRARY_ID":
|
||||
return 2
|
||||
if user_id == 7 and key == "BOOKLORE_PATH_ID":
|
||||
return 20
|
||||
return default
|
||||
|
||||
monkeypatch.setattr("shelfmark.download.outputs.booklore.core_config.config.get", fake_get)
|
||||
config = build_booklore_config(self.BASE_SETTINGS, user_id=7)
|
||||
assert config.library_id == 2
|
||||
assert config.path_id == 20
|
||||
|
||||
def test_override_library_only(self):
|
||||
overrides = {"booklore_library_id": 3}
|
||||
config = build_booklore_config(self.BASE_SETTINGS, user_overrides=overrides)
|
||||
def test_override_library_only(self, monkeypatch):
|
||||
def fake_get(key, default=None, user_id=None):
|
||||
if user_id == 7 and key == "BOOKLORE_LIBRARY_ID":
|
||||
return 3
|
||||
return default
|
||||
|
||||
monkeypatch.setattr("shelfmark.download.outputs.booklore.core_config.config.get", fake_get)
|
||||
config = build_booklore_config(self.BASE_SETTINGS, user_id=7)
|
||||
assert config.library_id == 3
|
||||
assert config.path_id == 10 # falls back to global
|
||||
|
||||
def test_override_path_only(self):
|
||||
overrides = {"booklore_path_id": 30}
|
||||
config = build_booklore_config(self.BASE_SETTINGS, user_overrides=overrides)
|
||||
def test_override_path_only(self, monkeypatch):
|
||||
def fake_get(key, default=None, user_id=None):
|
||||
if user_id == 7 and key == "BOOKLORE_PATH_ID":
|
||||
return 30
|
||||
return default
|
||||
|
||||
monkeypatch.setattr("shelfmark.download.outputs.booklore.core_config.config.get", fake_get)
|
||||
config = build_booklore_config(self.BASE_SETTINGS, user_id=7)
|
||||
assert config.library_id == 1 # falls back to global
|
||||
assert config.path_id == 30
|
||||
|
||||
def test_empty_overrides_uses_global(self):
|
||||
config = build_booklore_config(self.BASE_SETTINGS, user_overrides={})
|
||||
def test_none_user_context_uses_global(self):
|
||||
config = build_booklore_config(self.BASE_SETTINGS, user_id=None)
|
||||
assert config.library_id == 1
|
||||
assert config.path_id == 10
|
||||
|
||||
def test_none_overrides_uses_global(self):
|
||||
config = build_booklore_config(self.BASE_SETTINGS, user_overrides=None)
|
||||
assert config.library_id == 1
|
||||
assert config.path_id == 10
|
||||
def test_auth_fields_remain_global(self, monkeypatch):
|
||||
"""Only Booklore library/path should be resolved with user context."""
|
||||
def fake_get(key, default=None, user_id=None):
|
||||
if user_id == 7 and key == "BOOKLORE_LIBRARY_ID":
|
||||
return 5
|
||||
if user_id == 7 and key == "BOOKLORE_PATH_ID":
|
||||
return 15
|
||||
return default
|
||||
|
||||
def test_auth_fields_not_overridable(self):
|
||||
"""Auth stays global - user overrides should not affect host/user/pass."""
|
||||
overrides = {
|
||||
"booklore_library_id": 5,
|
||||
"BOOKLORE_HOST": "http://evil:6060",
|
||||
"BOOKLORE_USERNAME": "hacker",
|
||||
}
|
||||
config = build_booklore_config(self.BASE_SETTINGS, user_overrides=overrides)
|
||||
monkeypatch.setattr("shelfmark.download.outputs.booklore.core_config.config.get", fake_get)
|
||||
config = build_booklore_config(self.BASE_SETTINGS, user_id=7)
|
||||
assert config.base_url == "http://booklore:6060"
|
||||
assert config.username == "admin"
|
||||
assert config.library_id == 5
|
||||
|
||||
|
||||
class TestOutputArgsForBooklore:
|
||||
"""Download tasks should carry per-user booklore settings in output_args."""
|
||||
|
||||
def test_output_args_with_booklore_settings(self):
|
||||
from shelfmark.core.models import DownloadTask
|
||||
|
||||
task = DownloadTask(
|
||||
task_id="test-1",
|
||||
source="direct_download",
|
||||
title="Book1",
|
||||
output_mode="booklore",
|
||||
output_args={"booklore_library_id": 2, "booklore_path_id": 20},
|
||||
user_id=1,
|
||||
)
|
||||
assert task.output_args["booklore_library_id"] == 2
|
||||
assert task.output_args["booklore_path_id"] == 20
|
||||
|
||||
def test_output_args_empty_for_global_booklore(self):
|
||||
from shelfmark.core.models import DownloadTask
|
||||
|
||||
task = DownloadTask(
|
||||
task_id="test-2",
|
||||
source="direct_download",
|
||||
title="Book1",
|
||||
output_mode="booklore",
|
||||
output_args={},
|
||||
)
|
||||
assert task.output_args == {}
|
||||
|
||||
@@ -0,0 +1,62 @@
|
||||
"""Tests for Config.get per-user override precedence."""
|
||||
|
||||
from types import SimpleNamespace
|
||||
|
||||
from shelfmark.core.config import config
|
||||
|
||||
|
||||
class _DummyField:
|
||||
def __init__(self, env_supported: bool, user_overridable: bool):
|
||||
self.env_supported = env_supported
|
||||
self.user_overridable = user_overridable
|
||||
|
||||
|
||||
def test_get_prefers_env_over_user_override(monkeypatch):
|
||||
monkeypatch.setattr(config, "_ensure_loaded", lambda: None)
|
||||
monkeypatch.setattr(config, "_cache", {"DESTINATION": "/env/books"})
|
||||
monkeypatch.setattr(
|
||||
config,
|
||||
"_field_map",
|
||||
{"DESTINATION": (_DummyField(env_supported=True, user_overridable=True), "downloads")},
|
||||
)
|
||||
monkeypatch.setattr(config, "_get_user_override", lambda user_id, key: "/user/books")
|
||||
monkeypatch.setattr(
|
||||
"shelfmark.core.config._get_registry",
|
||||
lambda: SimpleNamespace(is_value_from_env=lambda field: True),
|
||||
)
|
||||
|
||||
assert config.get("DESTINATION", "/default", user_id=10) == "/env/books"
|
||||
|
||||
|
||||
def test_get_uses_user_override_when_not_env(monkeypatch):
|
||||
monkeypatch.setattr(config, "_ensure_loaded", lambda: None)
|
||||
monkeypatch.setattr(config, "_cache", {"DESTINATION": "/global/books"})
|
||||
monkeypatch.setattr(
|
||||
config,
|
||||
"_field_map",
|
||||
{"DESTINATION": (_DummyField(env_supported=True, user_overridable=True), "downloads")},
|
||||
)
|
||||
monkeypatch.setattr(config, "_get_user_override", lambda user_id, key: "/user/books")
|
||||
monkeypatch.setattr(
|
||||
"shelfmark.core.config._get_registry",
|
||||
lambda: SimpleNamespace(is_value_from_env=lambda field: False),
|
||||
)
|
||||
|
||||
assert config.get("DESTINATION", "/default", user_id=10) == "/user/books"
|
||||
|
||||
|
||||
def test_get_ignores_user_override_for_non_overridable_field(monkeypatch):
|
||||
monkeypatch.setattr(config, "_ensure_loaded", lambda: None)
|
||||
monkeypatch.setattr(config, "_cache", {"FILE_ORGANIZATION": "rename"})
|
||||
monkeypatch.setattr(
|
||||
config,
|
||||
"_field_map",
|
||||
{"FILE_ORGANIZATION": (_DummyField(env_supported=True, user_overridable=False), "downloads")},
|
||||
)
|
||||
monkeypatch.setattr(config, "_get_user_override", lambda user_id, key: "organize")
|
||||
monkeypatch.setattr(
|
||||
"shelfmark.core.config._get_registry",
|
||||
lambda: SimpleNamespace(is_value_from_env=lambda field: False),
|
||||
)
|
||||
|
||||
assert config.get("FILE_ORGANIZATION", "rename", user_id=10) == "rename"
|
||||
@@ -0,0 +1,95 @@
|
||||
"""Tests for CWA user linking/provisioning helpers."""
|
||||
|
||||
import os
|
||||
import tempfile
|
||||
|
||||
import pytest
|
||||
|
||||
from shelfmark.core.cwa_user_sync import upsert_cwa_user
|
||||
from shelfmark.core.user_db import UserDB
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def user_db():
|
||||
with tempfile.TemporaryDirectory() as tmpdir:
|
||||
db = UserDB(os.path.join(tmpdir, "users.db"))
|
||||
db.initialize()
|
||||
yield db
|
||||
|
||||
|
||||
def test_upsert_links_existing_user_by_unique_email(user_db):
|
||||
existing = user_db.create_user(
|
||||
username="local_admin",
|
||||
email="admin@example.com",
|
||||
role="admin",
|
||||
auth_source="builtin",
|
||||
)
|
||||
|
||||
user, action = upsert_cwa_user(
|
||||
user_db,
|
||||
cwa_username="admin",
|
||||
cwa_email="admin@example.com",
|
||||
role="user",
|
||||
)
|
||||
|
||||
assert action == "updated"
|
||||
assert user["id"] == existing["id"]
|
||||
assert user["username"] == "local_admin"
|
||||
assert user["auth_source"] == "cwa"
|
||||
assert user["role"] == "user"
|
||||
|
||||
|
||||
def test_upsert_creates_alias_when_username_taken_by_non_cwa(user_db):
|
||||
local_user = user_db.create_user(
|
||||
username="admin",
|
||||
email="local@example.com",
|
||||
role="admin",
|
||||
auth_source="builtin",
|
||||
)
|
||||
|
||||
user, action = upsert_cwa_user(
|
||||
user_db,
|
||||
cwa_username="admin",
|
||||
cwa_email="cwa@example.com",
|
||||
role="admin",
|
||||
)
|
||||
|
||||
assert action == "created"
|
||||
assert user["username"].startswith("admin__cwa")
|
||||
assert user["auth_source"] == "cwa"
|
||||
assert user["email"] == "cwa@example.com"
|
||||
|
||||
local_after = user_db.get_user(user_id=local_user["id"])
|
||||
assert local_after is not None
|
||||
assert local_after["username"] == "admin"
|
||||
assert local_after["auth_source"] == "builtin"
|
||||
assert local_after["email"] == "local@example.com"
|
||||
|
||||
|
||||
def test_upsert_updates_existing_cwa_user_by_username_before_email(user_db):
|
||||
cwa_user = user_db.create_user(
|
||||
username="reader",
|
||||
email="old@example.com",
|
||||
role="user",
|
||||
auth_source="cwa",
|
||||
)
|
||||
user_db.create_user(
|
||||
username="other",
|
||||
email="new@example.com",
|
||||
role="user",
|
||||
auth_source="builtin",
|
||||
)
|
||||
|
||||
user, action = upsert_cwa_user(
|
||||
user_db,
|
||||
cwa_username="reader",
|
||||
cwa_email="new@example.com",
|
||||
role="admin",
|
||||
)
|
||||
|
||||
assert action == "updated"
|
||||
assert user["id"] == cwa_user["id"]
|
||||
assert user["username"] == "reader"
|
||||
assert user["auth_source"] == "cwa"
|
||||
assert user["email"] == "new@example.com"
|
||||
assert user["role"] == "admin"
|
||||
@@ -70,7 +70,7 @@ def _mock_destination_config(ingest_dir: Path, extra=None):
|
||||
}
|
||||
if extra:
|
||||
values.update(extra)
|
||||
return MagicMock(side_effect=lambda key, default=None: values.get(key, default))
|
||||
return MagicMock(side_effect=lambda key, default=None, **_kwargs: values.get(key, default))
|
||||
|
||||
|
||||
def _sync_core_config(mock_config, mock_core_config, mock_archive_config=None):
|
||||
@@ -238,7 +238,7 @@ class TestProcessDirectory:
|
||||
with patch('shelfmark.core.config.config') as mock_config, \
|
||||
patch('shelfmark.config.env.TMP_DIR', temp_dirs["staging"]):
|
||||
mock_config.USE_BOOK_TITLE = False
|
||||
mock_config.get = MagicMock(side_effect=lambda key, default=None: {
|
||||
mock_config.get = MagicMock(side_effect=lambda key, default=None, **_kwargs: {
|
||||
"SUPPORTED_FORMATS": ["epub"],
|
||||
"FILE_ORGANIZATION": "none",
|
||||
}.get(key, default))
|
||||
@@ -269,7 +269,7 @@ class TestProcessDirectory:
|
||||
with patch('shelfmark.core.config.config') as mock_config, \
|
||||
patch('shelfmark.config.env.TMP_DIR', temp_dirs["staging"]):
|
||||
mock_config.USE_BOOK_TITLE = False
|
||||
mock_config.get = MagicMock(side_effect=lambda key, default=None: {
|
||||
mock_config.get = MagicMock(side_effect=lambda key, default=None, **_kwargs: {
|
||||
"SUPPORTED_FORMATS": ["epub"],
|
||||
"FILE_ORGANIZATION": "none",
|
||||
}.get(key, default))
|
||||
@@ -295,7 +295,7 @@ class TestProcessDirectory:
|
||||
|
||||
with patch('shelfmark.core.config.config') as mock_config, \
|
||||
patch('shelfmark.config.env.TMP_DIR', temp_dirs["staging"]):
|
||||
mock_config.get = MagicMock(side_effect=lambda key, default=None: {
|
||||
mock_config.get = MagicMock(side_effect=lambda key, default=None, **_kwargs: {
|
||||
"SUPPORTED_FORMATS": ["epub"],
|
||||
"FILE_ORGANIZATION": "none",
|
||||
}.get(key, default))
|
||||
@@ -321,7 +321,7 @@ class TestProcessDirectory:
|
||||
|
||||
with patch('shelfmark.core.config.config') as mock_config, \
|
||||
patch('shelfmark.config.env.TMP_DIR', temp_dirs["staging"]):
|
||||
mock_config.get = MagicMock(side_effect=lambda key, default=None: {
|
||||
mock_config.get = MagicMock(side_effect=lambda key, default=None, **_kwargs: {
|
||||
"SUPPORTED_FORMATS": ["epub"], # PDF not supported
|
||||
"FILE_ORGANIZATION": "none",
|
||||
}.get(key, default))
|
||||
@@ -349,7 +349,7 @@ class TestProcessDirectory:
|
||||
with patch('shelfmark.core.config.config') as mock_config, \
|
||||
patch('shelfmark.config.env.TMP_DIR', temp_dirs["staging"]):
|
||||
mock_config.USE_BOOK_TITLE = True
|
||||
mock_config.get = MagicMock(side_effect=lambda key, default=None: {
|
||||
mock_config.get = MagicMock(side_effect=lambda key, default=None, **_kwargs: {
|
||||
"SUPPORTED_FORMATS": ["epub"],
|
||||
"FILE_ORGANIZATION": "rename",
|
||||
}.get(key, default))
|
||||
@@ -378,7 +378,7 @@ class TestProcessDirectory:
|
||||
with patch('shelfmark.core.config.config') as mock_config, \
|
||||
patch('shelfmark.config.env.TMP_DIR', temp_dirs["staging"]):
|
||||
mock_config.USE_BOOK_TITLE = True # Ignored for multi-file
|
||||
mock_config.get = MagicMock(side_effect=lambda key, default=None: {
|
||||
mock_config.get = MagicMock(side_effect=lambda key, default=None, **_kwargs: {
|
||||
"SUPPORTED_FORMATS": ["epub"],
|
||||
"FILE_ORGANIZATION": "none",
|
||||
}.get(key, default))
|
||||
@@ -407,7 +407,7 @@ class TestProcessDirectory:
|
||||
with patch('shelfmark.core.config.config') as mock_config, \
|
||||
patch('shelfmark.config.env.TMP_DIR', temp_dirs["staging"]):
|
||||
mock_config.USE_BOOK_TITLE = False
|
||||
mock_config.get = MagicMock(side_effect=lambda key, default=None: {
|
||||
mock_config.get = MagicMock(side_effect=lambda key, default=None, **_kwargs: {
|
||||
"SUPPORTED_FORMATS": ["epub"],
|
||||
"FILE_ORGANIZATION": "none",
|
||||
}.get(key, default))
|
||||
@@ -435,7 +435,7 @@ class TestProcessDirectory:
|
||||
patch('shelfmark.download.postprocess.transfer.atomic_move', side_effect=Exception("Move failed")):
|
||||
|
||||
mock_config.USE_BOOK_TITLE = False
|
||||
mock_config.get = MagicMock(side_effect=lambda key, default=None: {
|
||||
mock_config.get = MagicMock(side_effect=lambda key, default=None, **_kwargs: {
|
||||
"SUPPORTED_FORMATS": ["epub"],
|
||||
"FILE_ORGANIZATION": "none",
|
||||
}.get(key, default))
|
||||
@@ -542,7 +542,7 @@ class TestPostProcessDownload:
|
||||
mock_config.USE_BOOK_TITLE = True
|
||||
mock_config.CUSTOM_SCRIPT = None
|
||||
_sync_core_config(mock_config, mock_config)
|
||||
mock_config.get = MagicMock(side_effect=lambda key, default=None: {
|
||||
mock_config.get = MagicMock(side_effect=lambda key, default=None, **_kwargs: {
|
||||
"DESTINATION": str(library),
|
||||
"FILE_ORGANIZATION": "organize",
|
||||
"TEMPLATE_ORGANIZE": "{Author}/{Title}",
|
||||
@@ -579,7 +579,7 @@ class TestPostProcessDownload:
|
||||
mock_config.USE_BOOK_TITLE = False
|
||||
mock_config.CUSTOM_SCRIPT = None
|
||||
_sync_core_config(mock_config, mock_config)
|
||||
mock_config.get = MagicMock(side_effect=lambda key, default=None: {
|
||||
mock_config.get = MagicMock(side_effect=lambda key, default=None, **_kwargs: {
|
||||
"DESTINATION": str(temp_dirs["ingest"]),
|
||||
"FILE_ORGANIZATION": "none",
|
||||
}.get(key, default))
|
||||
@@ -651,7 +651,7 @@ class TestPostProcessDownload:
|
||||
mock_config.USE_BOOK_TITLE = False
|
||||
mock_config.CUSTOM_SCRIPT = None
|
||||
_sync_core_config(mock_config, mock_config)
|
||||
mock_config.get = MagicMock(side_effect=lambda key, default=None: {
|
||||
mock_config.get = MagicMock(side_effect=lambda key, default=None, **_kwargs: {
|
||||
"DESTINATION": str(temp_dirs["ingest"]),
|
||||
"INGEST_DIR": str(temp_dirs["ingest"]),
|
||||
"DESTINATION_AUDIOBOOK": str(audiobook_ingest),
|
||||
@@ -781,7 +781,7 @@ class TestCustomScriptExecution:
|
||||
mock_config.CUSTOM_SCRIPT = "/path/to/script.sh"
|
||||
_sync_core_config(mock_config, mock_config)
|
||||
|
||||
mock_config.get = MagicMock(side_effect=lambda key, default=None: {
|
||||
mock_config.get = MagicMock(side_effect=lambda key, default=None, **_kwargs: {
|
||||
"BOOKS_OUTPUT_MODE": "booklore",
|
||||
"BOOKLORE_HOST": "http://booklore:6060",
|
||||
"BOOKLORE_USERNAME": "user",
|
||||
|
||||
@@ -38,7 +38,7 @@ def _run_organize_post_process(
|
||||
patch('shelfmark.download.postprocess.transfer.same_filesystem', return_value=same_fs):
|
||||
|
||||
mock_config.CUSTOM_SCRIPT = None
|
||||
mock_config.get = MagicMock(side_effect=lambda key, default=None: {
|
||||
mock_config.get = MagicMock(side_effect=lambda key, default=None, **_kwargs: {
|
||||
"DESTINATION": str(library),
|
||||
"FILE_ORGANIZATION": "organize",
|
||||
"HARDLINK_TORRENTS": hardlink_enabled,
|
||||
@@ -229,10 +229,15 @@ class TestAtomicHardlink:
|
||||
source.write_text("content")
|
||||
dest = tmp_path / "dest.txt"
|
||||
|
||||
def _raise_perm(*_args, **_kwargs):
|
||||
raise PermissionError("hardlink not permitted")
|
||||
original_link = os.link
|
||||
|
||||
monkeypatch.setattr(os, "link", _raise_perm)
|
||||
def _raise_only_for_initial_link(src, dst, *_args, **_kwargs):
|
||||
# Force hardlink -> copy fallback while allowing atomic_copy publish step.
|
||||
if Path(src) == source:
|
||||
raise PermissionError("hardlink not permitted")
|
||||
return original_link(src, dst)
|
||||
|
||||
monkeypatch.setattr(os, "link", _raise_only_for_initial_link)
|
||||
|
||||
result = _atomic_hardlink(source, dest)
|
||||
|
||||
@@ -352,7 +357,7 @@ class TestHardlinkWithLibraryMode:
|
||||
def mock_config(self):
|
||||
"""Mock config for library mode."""
|
||||
with patch('shelfmark.core.config.config') as mock:
|
||||
mock.get = MagicMock(side_effect=lambda key, default=None: {
|
||||
mock.get = MagicMock(side_effect=lambda key, default=None, **_kwargs: {
|
||||
"LIBRARY_PATH": None,
|
||||
"LIBRARY_PATH_AUDIOBOOK": None,
|
||||
"LIBRARY_TEMPLATE": "{Author}/{Title}",
|
||||
@@ -874,7 +879,7 @@ class TestTorrentSourceCleanupProtection:
|
||||
|
||||
def _make_config_mock(self, library_path: str, hardlink: bool = True):
|
||||
"""Create config mock for library/organize mode with hardlinking."""
|
||||
return MagicMock(side_effect=lambda key, default=None: {
|
||||
return MagicMock(side_effect=lambda key, default=None, **_kwargs: {
|
||||
# Destination paths (what _get_final_destination uses)
|
||||
"DESTINATION": library_path,
|
||||
"DESTINATION_AUDIOBOOK": library_path,
|
||||
@@ -1349,7 +1354,7 @@ class TestEdgeCases:
|
||||
status_cb = MagicMock()
|
||||
|
||||
with patch('shelfmark.core.config.config') as mock_config:
|
||||
mock_config.get = MagicMock(side_effect=lambda key, default=None: {
|
||||
mock_config.get = MagicMock(side_effect=lambda key, default=None, **_kwargs: {
|
||||
"DESTINATION": str(library),
|
||||
"TEMPLATE_ORGANIZE": "{Title}",
|
||||
"FILE_ORGANIZATION": "organize",
|
||||
@@ -1384,7 +1389,7 @@ class TestEdgeCases:
|
||||
status_cb = MagicMock()
|
||||
|
||||
with patch('shelfmark.core.config.config') as mock_config:
|
||||
mock_config.get = MagicMock(side_effect=lambda key, default=None: {
|
||||
mock_config.get = MagicMock(side_effect=lambda key, default=None, **_kwargs: {
|
||||
"DESTINATION": "/nonexistent/protected/path",
|
||||
"TEMPLATE_ORGANIZE": "{Title}",
|
||||
"FILE_ORGANIZATION": "organize",
|
||||
|
||||
@@ -169,6 +169,7 @@ class TestProvisionOIDCUser:
|
||||
user = provision_oidc_user(user_db, user_info, is_admin=False)
|
||||
assert user["username"] == "john"
|
||||
assert user["oidc_subject"] == "sub-123"
|
||||
assert user["auth_source"] == "oidc"
|
||||
assert user["role"] == "user"
|
||||
|
||||
def test_provision_creates_admin_user(self, user_db):
|
||||
@@ -209,6 +210,7 @@ class TestProvisionOIDCUser:
|
||||
user = provision_oidc_user(user_db, user_info, is_admin=False)
|
||||
assert user["email"] == "newemail@example.com"
|
||||
assert user["display_name"] == "John D."
|
||||
assert user["auth_source"] == "oidc"
|
||||
|
||||
def test_provision_updates_admin_role(self, user_db):
|
||||
from shelfmark.core.oidc_auth import provision_oidc_user
|
||||
@@ -256,3 +258,4 @@ class TestProvisionOIDCUser:
|
||||
user = provision_oidc_user(user_db, user_info, is_admin=False)
|
||||
assert user["username"] != "john" # Should have a suffix
|
||||
assert user["oidc_subject"] == "sub-456"
|
||||
assert user["auth_source"] == "oidc"
|
||||
|
||||
@@ -1,206 +1,107 @@
|
||||
"""
|
||||
Tests for OIDC integration into existing auth system.
|
||||
"""Tests for auth mode and admin policy helpers used by OIDC integration."""
|
||||
|
||||
Tests get_auth_mode() logic with OIDC and login_required admin
|
||||
restriction logic. Since main.py has heavy dependencies, we test
|
||||
the logic directly rather than importing from main.
|
||||
"""
|
||||
from shelfmark.core.auth_modes import (
|
||||
determine_auth_mode,
|
||||
get_auth_check_admin_status,
|
||||
is_settings_or_onboarding_path,
|
||||
should_restrict_settings_to_admin,
|
||||
)
|
||||
|
||||
|
||||
class TestGetAuthModeOIDCLogic:
|
||||
"""Tests that get_auth_mode logic handles OIDC correctly.
|
||||
|
||||
Mirrors the logic in main.py:get_auth_mode() to verify OIDC
|
||||
support without importing the full app.
|
||||
"""
|
||||
|
||||
def _get_auth_mode(self, config):
|
||||
"""Replicate get_auth_mode logic with OIDC support."""
|
||||
auth_mode = config.get("AUTH_METHOD", "none")
|
||||
if auth_mode == "oidc":
|
||||
if config.get("OIDC_DISCOVERY_URL") and config.get("OIDC_CLIENT_ID"):
|
||||
return "oidc"
|
||||
return "none"
|
||||
if auth_mode == "builtin":
|
||||
if config.get("BUILTIN_USERNAME") and config.get("BUILTIN_PASSWORD_HASH"):
|
||||
return "builtin"
|
||||
return "none"
|
||||
if auth_mode == "proxy":
|
||||
if config.get("PROXY_AUTH_USER_HEADER"):
|
||||
return "proxy"
|
||||
return "none"
|
||||
return "none"
|
||||
|
||||
class TestDetermineAuthMode:
|
||||
def test_returns_oidc_when_fully_configured(self):
|
||||
config = {
|
||||
"AUTH_METHOD": "oidc",
|
||||
"OIDC_DISCOVERY_URL": "https://auth.example.com/.well-known/openid-configuration",
|
||||
"OIDC_CLIENT_ID": "shelfmark",
|
||||
}
|
||||
assert self._get_auth_mode(config) == "oidc"
|
||||
assert determine_auth_mode(config, cwa_db_path=None) == "oidc"
|
||||
|
||||
def test_returns_none_when_oidc_missing_client_id(self):
|
||||
config = {
|
||||
"AUTH_METHOD": "oidc",
|
||||
"OIDC_DISCOVERY_URL": "https://auth.example.com/.well-known/openid-configuration",
|
||||
}
|
||||
assert self._get_auth_mode(config) == "none"
|
||||
assert determine_auth_mode(config, cwa_db_path=None) == "none"
|
||||
|
||||
def test_returns_none_when_oidc_missing_discovery_url(self):
|
||||
config = {
|
||||
"AUTH_METHOD": "oidc",
|
||||
"OIDC_CLIENT_ID": "shelfmark",
|
||||
}
|
||||
assert self._get_auth_mode(config) == "none"
|
||||
|
||||
def test_returns_none_when_oidc_empty_strings(self):
|
||||
config = {
|
||||
"AUTH_METHOD": "oidc",
|
||||
"OIDC_DISCOVERY_URL": "",
|
||||
"OIDC_CLIENT_ID": "",
|
||||
}
|
||||
assert self._get_auth_mode(config) == "none"
|
||||
assert determine_auth_mode(config, cwa_db_path=None) == "none"
|
||||
|
||||
def test_builtin_still_works(self):
|
||||
config = {
|
||||
"AUTH_METHOD": "builtin",
|
||||
"BUILTIN_USERNAME": "admin",
|
||||
"BUILTIN_PASSWORD_HASH": "hash",
|
||||
}
|
||||
assert self._get_auth_mode(config) == "builtin"
|
||||
assert determine_auth_mode(config, cwa_db_path=None) == "builtin"
|
||||
|
||||
def test_builtin_requires_local_admin(self):
|
||||
config = {
|
||||
"AUTH_METHOD": "builtin",
|
||||
}
|
||||
assert determine_auth_mode(config, cwa_db_path=None, has_local_admin=False) == "none"
|
||||
|
||||
def test_proxy_still_works(self):
|
||||
config = {
|
||||
"AUTH_METHOD": "proxy",
|
||||
"PROXY_AUTH_USER_HEADER": "X-Auth-User",
|
||||
}
|
||||
assert self._get_auth_mode(config) == "proxy"
|
||||
assert determine_auth_mode(config, cwa_db_path=None) == "proxy"
|
||||
|
||||
|
||||
class TestLoginRequiredOIDCLogic:
|
||||
"""Tests the OIDC admin restriction logic.
|
||||
|
||||
Mirrors the admin check in main.py:login_required() to verify
|
||||
OIDC support without importing the full app.
|
||||
"""
|
||||
|
||||
def _check_admin_access(self, auth_mode, config, session, path):
|
||||
"""Replicate login_required admin check logic with OIDC."""
|
||||
if auth_mode == "none":
|
||||
return True # Allowed
|
||||
|
||||
if "user_id" not in session:
|
||||
return 401 # Unauthorized
|
||||
|
||||
settings_path = path.startswith("/api/settings") or path.startswith("/api/onboarding")
|
||||
|
||||
if auth_mode in ("proxy", "cwa", "oidc") and settings_path:
|
||||
if auth_mode == "proxy":
|
||||
restrict = config.get("PROXY_AUTH_RESTRICT_SETTINGS_TO_ADMIN", False)
|
||||
elif auth_mode == "cwa":
|
||||
restrict = config.get("CWA_RESTRICT_SETTINGS_TO_ADMIN", False)
|
||||
elif auth_mode == "oidc":
|
||||
restrict = config.get("OIDC_RESTRICT_SETTINGS_TO_ADMIN", False)
|
||||
else:
|
||||
restrict = False
|
||||
|
||||
if restrict and not session.get("is_admin", False):
|
||||
return 403 # Forbidden
|
||||
|
||||
return True # Allowed
|
||||
|
||||
def test_oidc_unauthenticated_returns_401(self):
|
||||
config = {"OIDC_RESTRICT_SETTINGS_TO_ADMIN": True}
|
||||
result = self._check_admin_access("oidc", config, {}, "/api/settings/test")
|
||||
assert result == 401
|
||||
|
||||
def test_oidc_non_admin_blocked_from_settings(self):
|
||||
config = {"OIDC_RESTRICT_SETTINGS_TO_ADMIN": True}
|
||||
session = {"user_id": "user", "is_admin": False}
|
||||
result = self._check_admin_access("oidc", config, session, "/api/settings/test")
|
||||
assert result == 403
|
||||
|
||||
def test_oidc_admin_can_access_settings(self):
|
||||
config = {"OIDC_RESTRICT_SETTINGS_TO_ADMIN": True}
|
||||
session = {"user_id": "admin", "is_admin": True}
|
||||
result = self._check_admin_access("oidc", config, session, "/api/settings/test")
|
||||
assert result is True
|
||||
|
||||
def test_oidc_non_admin_can_access_non_settings(self):
|
||||
config = {"OIDC_RESTRICT_SETTINGS_TO_ADMIN": True}
|
||||
session = {"user_id": "user", "is_admin": False}
|
||||
result = self._check_admin_access("oidc", config, session, "/api/search")
|
||||
assert result is True
|
||||
|
||||
def test_oidc_no_restrict_allows_non_admin_settings(self):
|
||||
config = {"OIDC_RESTRICT_SETTINGS_TO_ADMIN": False}
|
||||
session = {"user_id": "user", "is_admin": False}
|
||||
result = self._check_admin_access("oidc", config, session, "/api/settings/test")
|
||||
assert result is True
|
||||
|
||||
def test_oidc_non_admin_blocked_from_onboarding(self):
|
||||
config = {"OIDC_RESTRICT_SETTINGS_TO_ADMIN": True}
|
||||
session = {"user_id": "user", "is_admin": False}
|
||||
result = self._check_admin_access("oidc", config, session, "/api/onboarding")
|
||||
assert result == 403
|
||||
|
||||
|
||||
class TestAuthCheckOIDCLogic:
|
||||
"""Tests the /api/auth/check response logic for OIDC mode."""
|
||||
|
||||
def _build_auth_check_response(self, auth_mode, config, session):
|
||||
"""Replicate auth check logic with OIDC."""
|
||||
if auth_mode == "none":
|
||||
return {"authenticated": True, "auth_required": False, "auth_mode": "none", "is_admin": True}
|
||||
|
||||
is_authenticated = "user_id" in session
|
||||
|
||||
if auth_mode == "builtin":
|
||||
is_admin = True
|
||||
elif auth_mode == "cwa":
|
||||
restrict = config.get("CWA_RESTRICT_SETTINGS_TO_ADMIN", False)
|
||||
is_admin = session.get("is_admin", False) if restrict else True
|
||||
elif auth_mode == "proxy":
|
||||
restrict = config.get("PROXY_AUTH_RESTRICT_SETTINGS_TO_ADMIN", False)
|
||||
is_admin = session.get("is_admin", not restrict)
|
||||
elif auth_mode == "oidc":
|
||||
restrict = config.get("OIDC_RESTRICT_SETTINGS_TO_ADMIN", False)
|
||||
is_admin = session.get("is_admin", False) if restrict else True
|
||||
else:
|
||||
is_admin = False
|
||||
|
||||
return {
|
||||
"authenticated": is_authenticated,
|
||||
"auth_required": True,
|
||||
"auth_mode": auth_mode,
|
||||
"is_admin": is_admin if is_authenticated else False,
|
||||
"username": session.get("user_id") if is_authenticated else None,
|
||||
def test_oidc_requires_local_admin(self):
|
||||
config = {
|
||||
"AUTH_METHOD": "oidc",
|
||||
"OIDC_DISCOVERY_URL": "https://auth.example.com/.well-known/openid-configuration",
|
||||
"OIDC_CLIENT_ID": "shelfmark",
|
||||
}
|
||||
assert determine_auth_mode(config, cwa_db_path=None, has_local_admin=False) == "none"
|
||||
|
||||
def test_oidc_authenticated_admin(self):
|
||||
config = {"OIDC_RESTRICT_SETTINGS_TO_ADMIN": True}
|
||||
session = {"user_id": "admin", "is_admin": True}
|
||||
result = self._build_auth_check_response("oidc", config, session)
|
||||
assert result["authenticated"] is True
|
||||
assert result["auth_mode"] == "oidc"
|
||||
assert result["is_admin"] is True
|
||||
assert result["username"] == "admin"
|
||||
|
||||
def test_oidc_authenticated_non_admin(self):
|
||||
config = {"OIDC_RESTRICT_SETTINGS_TO_ADMIN": True}
|
||||
session = {"user_id": "user", "is_admin": False}
|
||||
result = self._build_auth_check_response("oidc", config, session)
|
||||
assert result["is_admin"] is False
|
||||
class TestSettingsRestrictionPolicy:
|
||||
def test_settings_path_detection(self):
|
||||
assert is_settings_or_onboarding_path("/api/settings/downloads")
|
||||
assert is_settings_or_onboarding_path("/api/onboarding")
|
||||
assert not is_settings_or_onboarding_path("/api/search")
|
||||
|
||||
def test_oidc_no_restrict_all_are_admin(self):
|
||||
config = {"OIDC_RESTRICT_SETTINGS_TO_ADMIN": False}
|
||||
session = {"user_id": "user", "is_admin": False}
|
||||
result = self._build_auth_check_response("oidc", config, session)
|
||||
assert result["is_admin"] is True
|
||||
def test_default_is_admin_restricted(self):
|
||||
assert should_restrict_settings_to_admin({}) is True
|
||||
|
||||
def test_oidc_unauthenticated(self):
|
||||
config = {"OIDC_RESTRICT_SETTINGS_TO_ADMIN": True}
|
||||
result = self._build_auth_check_response("oidc", config, {})
|
||||
assert result["authenticated"] is False
|
||||
assert result["is_admin"] is False
|
||||
assert result["auth_required"] is True
|
||||
def test_respects_global_users_toggle(self):
|
||||
assert should_restrict_settings_to_admin({"RESTRICT_SETTINGS_TO_ADMIN": True}) is True
|
||||
assert should_restrict_settings_to_admin({"RESTRICT_SETTINGS_TO_ADMIN": False}) is False
|
||||
|
||||
|
||||
class TestAuthCheckAdminStatus:
|
||||
def test_authenticated_admin_when_restricted(self):
|
||||
result = get_auth_check_admin_status(
|
||||
"oidc",
|
||||
{"RESTRICT_SETTINGS_TO_ADMIN": True},
|
||||
{"user_id": "admin", "is_admin": True},
|
||||
)
|
||||
assert result is True
|
||||
|
||||
def test_authenticated_non_admin_when_restricted(self):
|
||||
result = get_auth_check_admin_status(
|
||||
"oidc",
|
||||
{"RESTRICT_SETTINGS_TO_ADMIN": True},
|
||||
{"user_id": "user", "is_admin": False},
|
||||
)
|
||||
assert result is False
|
||||
|
||||
def test_authenticated_user_when_not_restricted(self):
|
||||
result = get_auth_check_admin_status(
|
||||
"proxy",
|
||||
{"RESTRICT_SETTINGS_TO_ADMIN": False},
|
||||
{"user_id": "user", "is_admin": False},
|
||||
)
|
||||
assert result is True
|
||||
|
||||
def test_unauthenticated_is_never_admin(self):
|
||||
result = get_auth_check_admin_status(
|
||||
"builtin",
|
||||
{"RESTRICT_SETTINGS_TO_ADMIN": False},
|
||||
{"is_admin": True},
|
||||
)
|
||||
assert result is False
|
||||
|
||||
+155
-209
@@ -1,17 +1,11 @@
|
||||
"""
|
||||
Tests for OIDC Flask route handlers.
|
||||
|
||||
Tests the /api/auth/oidc/login and /api/auth/oidc/callback endpoints
|
||||
using a minimal Flask test app (not the full shelfmark app).
|
||||
"""
|
||||
"""Tests for OIDC Flask route handlers using Authlib transport."""
|
||||
|
||||
import os
|
||||
import tempfile
|
||||
from unittest.mock import patch
|
||||
from urllib.parse import parse_qs, urlparse
|
||||
from unittest.mock import Mock, patch
|
||||
|
||||
import pytest
|
||||
from flask import Flask
|
||||
from flask import Flask, redirect
|
||||
|
||||
from shelfmark.core.user_db import UserDB
|
||||
|
||||
@@ -38,29 +32,18 @@ MOCK_OIDC_CONFIG = {
|
||||
"OIDC_GROUP_CLAIM": "groups",
|
||||
"OIDC_ADMIN_GROUP": "shelfmark-admins",
|
||||
"OIDC_AUTO_PROVISION": True,
|
||||
"OIDC_RESTRICT_SETTINGS_TO_ADMIN": True,
|
||||
}
|
||||
|
||||
MOCK_DISCOVERY = {
|
||||
"issuer": "https://auth.example.com",
|
||||
"authorization_endpoint": "https://auth.example.com/authorize",
|
||||
"token_endpoint": "https://auth.example.com/token",
|
||||
"userinfo_endpoint": "https://auth.example.com/userinfo",
|
||||
"jwks_uri": "https://auth.example.com/.well-known/jwks.json",
|
||||
"OIDC_USE_ADMIN_GROUP": True,
|
||||
}
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def app(user_db, db_path):
|
||||
"""Create a minimal Flask test app with OIDC routes."""
|
||||
def app(user_db):
|
||||
from shelfmark.core.oidc_routes import register_oidc_routes
|
||||
|
||||
test_app = Flask(__name__)
|
||||
test_app.config["SECRET_KEY"] = "test-secret"
|
||||
test_app.config["TESTING"] = True
|
||||
|
||||
register_oidc_routes(test_app, user_db)
|
||||
|
||||
return test_app
|
||||
|
||||
|
||||
@@ -69,245 +52,208 @@ def client(app):
|
||||
return app.test_client()
|
||||
|
||||
|
||||
class TestOIDCClientRegistration:
|
||||
@patch("shelfmark.core.oidc_routes.load_config_file", return_value=MOCK_OIDC_CONFIG)
|
||||
@patch("shelfmark.core.oidc_routes.oauth.create_client")
|
||||
@patch("shelfmark.core.oidc_routes.oauth.register")
|
||||
def test_registers_client_with_pkce_and_expected_scopes(
|
||||
self, mock_register, mock_create_client, _mock_config
|
||||
):
|
||||
from shelfmark.core.oidc_routes import _get_oidc_client
|
||||
|
||||
fake_client = Mock()
|
||||
mock_create_client.return_value = fake_client
|
||||
|
||||
client_obj, config = _get_oidc_client()
|
||||
|
||||
assert client_obj is fake_client
|
||||
assert config["OIDC_CLIENT_ID"] == "shelfmark"
|
||||
kwargs = mock_register.call_args.kwargs
|
||||
assert kwargs["name"] == "shelfmark_idp"
|
||||
assert kwargs["server_metadata_url"] == MOCK_OIDC_CONFIG["OIDC_DISCOVERY_URL"]
|
||||
assert kwargs["overwrite"] is True
|
||||
assert kwargs["client_kwargs"]["code_challenge_method"] == "S256"
|
||||
scope_str = kwargs["client_kwargs"]["scope"]
|
||||
assert "openid" in scope_str
|
||||
assert "email" in scope_str
|
||||
assert "profile" in scope_str
|
||||
assert "groups" in scope_str
|
||||
|
||||
@patch("shelfmark.core.oidc_routes.load_config_file")
|
||||
@patch("shelfmark.core.oidc_routes.oauth.create_client")
|
||||
@patch("shelfmark.core.oidc_routes.oauth.register")
|
||||
def test_does_not_append_group_claim_when_admin_group_auth_disabled(
|
||||
self, mock_register, mock_create_client, mock_config
|
||||
):
|
||||
from shelfmark.core.oidc_routes import _get_oidc_client
|
||||
|
||||
config = {
|
||||
**MOCK_OIDC_CONFIG,
|
||||
"OIDC_SCOPES": ["openid", "email", "profile"],
|
||||
"OIDC_USE_ADMIN_GROUP": False,
|
||||
"OIDC_GROUP_CLAIM": "groups",
|
||||
}
|
||||
mock_config.return_value = config
|
||||
mock_create_client.return_value = Mock()
|
||||
|
||||
_get_oidc_client()
|
||||
|
||||
scope_str = mock_register.call_args.kwargs["client_kwargs"]["scope"]
|
||||
assert "groups" not in scope_str
|
||||
|
||||
|
||||
class TestOIDCLoginEndpoint:
|
||||
"""Tests for GET /api/auth/oidc/login."""
|
||||
@patch("shelfmark.core.oidc_routes._get_oidc_client")
|
||||
def test_login_redirects_to_provider(self, mock_get_client, client):
|
||||
fake_client = Mock()
|
||||
fake_client.authorize_redirect.return_value = redirect("https://auth.example.com/authorize")
|
||||
mock_get_client.return_value = (fake_client, MOCK_OIDC_CONFIG)
|
||||
|
||||
@patch("shelfmark.core.oidc_routes.load_config_file", return_value=MOCK_OIDC_CONFIG)
|
||||
@patch("shelfmark.core.oidc_routes._fetch_discovery", return_value=MOCK_DISCOVERY)
|
||||
def test_login_redirects_to_idp(self, mock_discovery, mock_config, client):
|
||||
resp = client.get("/api/auth/oidc/login")
|
||||
|
||||
assert resp.status_code == 302
|
||||
location = resp.headers["Location"]
|
||||
assert location.startswith("https://auth.example.com/authorize")
|
||||
assert resp.headers["Location"].startswith("https://auth.example.com/authorize")
|
||||
fake_client.authorize_redirect.assert_called_once()
|
||||
redirect_uri = fake_client.authorize_redirect.call_args.args[0]
|
||||
assert redirect_uri.endswith("/api/auth/oidc/callback")
|
||||
|
||||
@patch("shelfmark.core.oidc_routes.load_config_file", return_value=MOCK_OIDC_CONFIG)
|
||||
@patch("shelfmark.core.oidc_routes._fetch_discovery", return_value=MOCK_DISCOVERY)
|
||||
def test_login_includes_required_params(self, mock_discovery, mock_config, client):
|
||||
@patch("shelfmark.core.oidc_routes._get_oidc_client", side_effect=ValueError("OIDC not configured"))
|
||||
def test_login_returns_500_when_not_configured(self, _mock_get_client, client):
|
||||
resp = client.get("/api/auth/oidc/login")
|
||||
location = resp.headers["Location"]
|
||||
parsed = urlparse(location)
|
||||
params = parse_qs(parsed.query)
|
||||
|
||||
assert params["client_id"] == ["shelfmark"]
|
||||
assert params["response_type"] == ["code"]
|
||||
assert "state" in params
|
||||
assert "code_challenge" in params
|
||||
assert params["code_challenge_method"] == ["S256"]
|
||||
|
||||
@patch("shelfmark.core.oidc_routes.load_config_file", return_value=MOCK_OIDC_CONFIG)
|
||||
@patch("shelfmark.core.oidc_routes._fetch_discovery", return_value=MOCK_DISCOVERY)
|
||||
def test_login_includes_scopes(self, mock_discovery, mock_config, client):
|
||||
resp = client.get("/api/auth/oidc/login")
|
||||
location = resp.headers["Location"]
|
||||
parsed = urlparse(location)
|
||||
params = parse_qs(parsed.query)
|
||||
|
||||
scope = params["scope"][0]
|
||||
assert "openid" in scope
|
||||
assert "email" in scope
|
||||
assert "profile" in scope
|
||||
assert "groups" in scope
|
||||
|
||||
@patch("shelfmark.core.oidc_routes.load_config_file", return_value=MOCK_OIDC_CONFIG)
|
||||
@patch("shelfmark.core.oidc_routes._fetch_discovery", return_value=MOCK_DISCOVERY)
|
||||
def test_login_stores_state_in_session(self, mock_discovery, mock_config, client):
|
||||
with client.session_transaction() as sess:
|
||||
assert "oidc_state" not in sess
|
||||
|
||||
client.get("/api/auth/oidc/login")
|
||||
|
||||
with client.session_transaction() as sess:
|
||||
assert "oidc_state" in sess
|
||||
assert "oidc_code_verifier" in sess
|
||||
assert resp.status_code == 500
|
||||
assert resp.get_json()["error"] == "OIDC not configured"
|
||||
|
||||
|
||||
class TestOIDCCallbackEndpoint:
|
||||
"""Tests for GET /api/auth/oidc/callback."""
|
||||
|
||||
@patch("shelfmark.core.oidc_routes.load_config_file", return_value=MOCK_OIDC_CONFIG)
|
||||
def test_callback_rejects_missing_state(self, mock_config, client):
|
||||
resp = client.get("/api/auth/oidc/callback?code=abc123")
|
||||
assert resp.status_code == 400
|
||||
|
||||
@patch("shelfmark.core.oidc_routes.load_config_file", return_value=MOCK_OIDC_CONFIG)
|
||||
def test_callback_rejects_mismatched_state(self, mock_config, client):
|
||||
with client.session_transaction() as sess:
|
||||
sess["oidc_state"] = "correct-state"
|
||||
sess["oidc_code_verifier"] = "verifier"
|
||||
|
||||
resp = client.get("/api/auth/oidc/callback?code=abc123&state=wrong-state")
|
||||
assert resp.status_code == 400
|
||||
|
||||
@patch("shelfmark.core.oidc_routes.load_config_file", return_value=MOCK_OIDC_CONFIG)
|
||||
@patch("shelfmark.core.oidc_routes._fetch_discovery", return_value=MOCK_DISCOVERY)
|
||||
@patch("shelfmark.core.oidc_routes._exchange_code")
|
||||
def test_callback_creates_session(self, mock_exchange, mock_discovery, mock_config, client, user_db):
|
||||
mock_exchange.return_value = {
|
||||
"sub": "user-123",
|
||||
"email": "john@example.com",
|
||||
"name": "John Doe",
|
||||
"preferred_username": "john",
|
||||
"groups": ["users"],
|
||||
@patch("shelfmark.core.oidc_routes._get_oidc_client")
|
||||
def test_callback_creates_session(self, mock_get_client, client):
|
||||
fake_client = Mock()
|
||||
fake_client.authorize_access_token.return_value = {
|
||||
"userinfo": {
|
||||
"sub": "user-123",
|
||||
"email": "john@example.com",
|
||||
"name": "John Doe",
|
||||
"preferred_username": "john",
|
||||
"groups": ["users"],
|
||||
}
|
||||
}
|
||||
|
||||
with client.session_transaction() as sess:
|
||||
sess["oidc_state"] = "test-state"
|
||||
sess["oidc_code_verifier"] = "test-verifier"
|
||||
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 # Redirect to frontend
|
||||
assert resp.status_code == 302
|
||||
|
||||
with client.session_transaction() as sess:
|
||||
assert sess["user_id"] == "john"
|
||||
assert "oidc_state" not in sess # Cleaned up
|
||||
assert sess["db_user_id"] is not None
|
||||
|
||||
@patch("shelfmark.core.oidc_routes.load_config_file", return_value=MOCK_OIDC_CONFIG)
|
||||
@patch("shelfmark.core.oidc_routes._fetch_discovery", return_value=MOCK_DISCOVERY)
|
||||
@patch("shelfmark.core.oidc_routes._exchange_code")
|
||||
def test_callback_sets_admin_from_groups(self, mock_exchange, mock_discovery, mock_config, client, user_db):
|
||||
mock_exchange.return_value = {
|
||||
"sub": "admin-123",
|
||||
"email": "admin@example.com",
|
||||
"preferred_username": "admin",
|
||||
"groups": ["users", "shelfmark-admins"],
|
||||
@patch("shelfmark.core.oidc_routes._get_oidc_client")
|
||||
def test_callback_sets_admin_from_groups(self, mock_get_client, client):
|
||||
fake_client = Mock()
|
||||
fake_client.authorize_access_token.return_value = {
|
||||
"userinfo": {
|
||||
"sub": "admin-123",
|
||||
"email": "admin@example.com",
|
||||
"preferred_username": "admin",
|
||||
"groups": ["users", "shelfmark-admins"],
|
||||
}
|
||||
}
|
||||
|
||||
with client.session_transaction() as sess:
|
||||
sess["oidc_state"] = "test-state"
|
||||
sess["oidc_code_verifier"] = "test-verifier"
|
||||
mock_get_client.return_value = (fake_client, MOCK_OIDC_CONFIG)
|
||||
|
||||
client.get("/api/auth/oidc/callback?code=abc123&state=test-state")
|
||||
|
||||
with client.session_transaction() as sess:
|
||||
assert sess["is_admin"] is True
|
||||
|
||||
@patch("shelfmark.core.oidc_routes.load_config_file", return_value=MOCK_OIDC_CONFIG)
|
||||
@patch("shelfmark.core.oidc_routes._fetch_discovery", return_value=MOCK_DISCOVERY)
|
||||
@patch("shelfmark.core.oidc_routes._exchange_code")
|
||||
def test_callback_provisions_user_in_db(self, mock_exchange, mock_discovery, mock_config, client, user_db):
|
||||
mock_exchange.return_value = {
|
||||
"sub": "user-789",
|
||||
"email": "new@example.com",
|
||||
"name": "New User",
|
||||
"preferred_username": "newuser",
|
||||
@patch("shelfmark.core.oidc_routes._get_oidc_client")
|
||||
def test_callback_falls_back_to_userinfo_endpoint(self, mock_get_client, client):
|
||||
fake_client = Mock()
|
||||
fake_client.authorize_access_token.return_value = {}
|
||||
fake_client.userinfo.return_value = {
|
||||
"sub": "fallback-123",
|
||||
"email": "fallback@example.com",
|
||||
"preferred_username": "fallback",
|
||||
"groups": [],
|
||||
}
|
||||
mock_get_client.return_value = (fake_client, MOCK_OIDC_CONFIG)
|
||||
|
||||
with client.session_transaction() as sess:
|
||||
sess["oidc_state"] = "test-state"
|
||||
sess["oidc_code_verifier"] = "test-verifier"
|
||||
resp = client.get("/api/auth/oidc/callback?code=abc123&state=test-state")
|
||||
assert resp.status_code == 302
|
||||
|
||||
client.get("/api/auth/oidc/callback?code=abc123&state=test-state")
|
||||
@patch("shelfmark.core.oidc_routes._get_oidc_client")
|
||||
def test_callback_returns_400_when_claims_missing(self, mock_get_client, client):
|
||||
fake_client = Mock()
|
||||
fake_client.authorize_access_token.return_value = {}
|
||||
fake_client.userinfo.side_effect = RuntimeError("userinfo failed")
|
||||
mock_get_client.return_value = (fake_client, MOCK_OIDC_CONFIG)
|
||||
|
||||
user = user_db.get_user(oidc_subject="user-789")
|
||||
assert user is not None
|
||||
assert user["username"] == "newuser"
|
||||
assert user["email"] == "new@example.com"
|
||||
resp = client.get("/api/auth/oidc/callback?code=abc123&state=test-state")
|
||||
assert resp.status_code == 400
|
||||
assert "missing user claims" in resp.get_json()["error"]
|
||||
|
||||
@patch("shelfmark.core.oidc_routes.load_config_file")
|
||||
@patch("shelfmark.core.oidc_routes._fetch_discovery", return_value=MOCK_DISCOVERY)
|
||||
@patch("shelfmark.core.oidc_routes._exchange_code")
|
||||
def test_callback_rejects_when_auto_provision_disabled(self, mock_exchange, mock_discovery, mock_config, client, user_db):
|
||||
@patch("shelfmark.core.oidc_routes._get_oidc_client")
|
||||
def test_callback_rejects_when_auto_provision_disabled(self, mock_get_client, client):
|
||||
config = {**MOCK_OIDC_CONFIG, "OIDC_AUTO_PROVISION": False}
|
||||
mock_config.return_value = config
|
||||
|
||||
mock_exchange.return_value = {
|
||||
"sub": "unknown-user",
|
||||
"email": "unknown@example.com",
|
||||
"preferred_username": "unknown",
|
||||
"groups": [],
|
||||
fake_client = Mock()
|
||||
fake_client.authorize_access_token.return_value = {
|
||||
"userinfo": {
|
||||
"sub": "unknown-user",
|
||||
"email": "unknown@example.com",
|
||||
"preferred_username": "unknown",
|
||||
"groups": [],
|
||||
}
|
||||
}
|
||||
|
||||
with client.session_transaction() as sess:
|
||||
sess["oidc_state"] = "test-state"
|
||||
sess["oidc_code_verifier"] = "test-verifier"
|
||||
mock_get_client.return_value = (fake_client, config)
|
||||
|
||||
resp = client.get("/api/auth/oidc/callback?code=abc123&state=test-state")
|
||||
assert resp.status_code == 403
|
||||
|
||||
@patch("shelfmark.core.oidc_routes.load_config_file")
|
||||
@patch("shelfmark.core.oidc_routes._fetch_discovery", return_value=MOCK_DISCOVERY)
|
||||
@patch("shelfmark.core.oidc_routes._exchange_code")
|
||||
def test_callback_allows_pre_created_user_by_email_when_no_provision(
|
||||
self, mock_exchange, mock_discovery, mock_config, client, user_db
|
||||
@patch("shelfmark.core.oidc_routes._get_oidc_client")
|
||||
def test_callback_allows_pre_created_user_by_verified_email_when_no_provision(
|
||||
self, mock_get_client, client, user_db
|
||||
):
|
||||
"""Pre-created user (by email) should log in even when auto-provision is off."""
|
||||
config = {**MOCK_OIDC_CONFIG, "OIDC_AUTO_PROVISION": False}
|
||||
mock_config.return_value = config
|
||||
|
||||
# Admin pre-creates a user with this email (no oidc_subject yet)
|
||||
user_db.create_user(username="alice", email="alice@example.com", password_hash="hash")
|
||||
|
||||
mock_exchange.return_value = {
|
||||
"sub": "oidc-alice-sub",
|
||||
"email": "alice@example.com",
|
||||
"preferred_username": "alice_oidc",
|
||||
"groups": [],
|
||||
fake_client = Mock()
|
||||
fake_client.authorize_access_token.return_value = {
|
||||
"userinfo": {
|
||||
"sub": "oidc-alice-sub",
|
||||
"email": "alice@example.com",
|
||||
"email_verified": True,
|
||||
"preferred_username": "alice_oidc",
|
||||
"groups": [],
|
||||
}
|
||||
}
|
||||
|
||||
with client.session_transaction() as sess:
|
||||
sess["oidc_state"] = "test-state"
|
||||
sess["oidc_code_verifier"] = "test-verifier"
|
||||
mock_get_client.return_value = (fake_client, config)
|
||||
|
||||
resp = client.get("/api/auth/oidc/callback?code=abc123&state=test-state")
|
||||
assert resp.status_code == 302 # Success, redirects to frontend
|
||||
assert resp.status_code == 302
|
||||
|
||||
with client.session_transaction() as sess:
|
||||
assert sess["user_id"] == "alice"
|
||||
assert sess.get("db_user_id") is not None
|
||||
|
||||
@patch("shelfmark.core.oidc_routes.load_config_file")
|
||||
@patch("shelfmark.core.oidc_routes._fetch_discovery", return_value=MOCK_DISCOVERY)
|
||||
@patch("shelfmark.core.oidc_routes._exchange_code")
|
||||
def test_callback_links_oidc_subject_to_pre_created_user(
|
||||
self, mock_exchange, mock_discovery, mock_config, client, user_db
|
||||
@patch("shelfmark.core.oidc_routes._get_oidc_client")
|
||||
def test_callback_does_not_link_unverified_email_when_no_provision(
|
||||
self, mock_get_client, client, user_db
|
||||
):
|
||||
"""When a pre-created user logs in via OIDC, their oidc_subject should be linked."""
|
||||
config = {**MOCK_OIDC_CONFIG, "OIDC_AUTO_PROVISION": False}
|
||||
mock_config.return_value = config
|
||||
|
||||
user = user_db.create_user(username="bob", email="bob@example.com", password_hash="hash")
|
||||
|
||||
mock_exchange.return_value = {
|
||||
"sub": "oidc-bob-sub",
|
||||
"email": "bob@example.com",
|
||||
"preferred_username": "bob_oidc",
|
||||
"groups": [],
|
||||
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": [],
|
||||
}
|
||||
}
|
||||
|
||||
with client.session_transaction() as sess:
|
||||
sess["oidc_state"] = "test-state"
|
||||
sess["oidc_code_verifier"] = "test-verifier"
|
||||
|
||||
client.get("/api/auth/oidc/callback?code=abc123&state=test-state")
|
||||
|
||||
# The OIDC subject should now be linked to the existing user
|
||||
updated_user = user_db.get_user(user_id=user["id"])
|
||||
assert updated_user["oidc_subject"] == "oidc-bob-sub"
|
||||
|
||||
@patch("shelfmark.core.oidc_routes.load_config_file")
|
||||
@patch("shelfmark.core.oidc_routes._fetch_discovery", return_value=MOCK_DISCOVERY)
|
||||
@patch("shelfmark.core.oidc_routes._exchange_code")
|
||||
def test_callback_rejects_unknown_email_when_no_provision(
|
||||
self, mock_exchange, mock_discovery, mock_config, client, user_db
|
||||
):
|
||||
"""When auto-provision is off and no user matches by email, reject login."""
|
||||
config = {**MOCK_OIDC_CONFIG, "OIDC_AUTO_PROVISION": False}
|
||||
mock_config.return_value = config
|
||||
|
||||
# Pre-create a user with a different email
|
||||
user_db.create_user(username="charlie", email="charlie@example.com", password_hash="hash")
|
||||
|
||||
mock_exchange.return_value = {
|
||||
"sub": "oidc-unknown-sub",
|
||||
"email": "stranger@example.com",
|
||||
"preferred_username": "stranger",
|
||||
"groups": [],
|
||||
}
|
||||
|
||||
with client.session_transaction() as sess:
|
||||
sess["oidc_state"] = "test-state"
|
||||
sess["oidc_code_verifier"] = "test-verifier"
|
||||
mock_get_client.return_value = (fake_client, config)
|
||||
|
||||
resp = client.get("/api/auth/oidc/callback?code=abc123&state=test-state")
|
||||
assert resp.status_code == 403
|
||||
|
||||
updated_user = user_db.get_user(user_id=user["id"])
|
||||
assert updated_user["oidc_subject"] is None
|
||||
|
||||
@@ -123,22 +123,30 @@ class TestQueueFilterByUser:
|
||||
|
||||
|
||||
class TestPerUserDestination:
|
||||
"""get_final_destination should respect per-user destination override in output_args."""
|
||||
"""get_final_destination should resolve destination via config user context."""
|
||||
|
||||
def test_uses_per_user_destination(self, monkeypatch):
|
||||
"""When output_args has a destination, it should be used instead of global."""
|
||||
def test_passes_user_id_to_get_destination(self, monkeypatch):
|
||||
"""When task has a user_id, destination resolution should receive it."""
|
||||
from pathlib import Path
|
||||
|
||||
captured: dict[str, object] = {"user_id": None, "username": None}
|
||||
|
||||
task = DownloadTask(
|
||||
task_id="book1",
|
||||
source="direct_download",
|
||||
title="Test Book",
|
||||
output_args={"destination": "/user-books/alice"},
|
||||
user_id=42,
|
||||
username="alice",
|
||||
)
|
||||
|
||||
def fake_get_destination(is_audiobook: bool = False, user_id=None, username=None):
|
||||
captured["user_id"] = user_id
|
||||
captured["username"] = username
|
||||
return Path("/user-books/alice")
|
||||
|
||||
monkeypatch.setattr(
|
||||
"shelfmark.download.postprocess.destination.get_destination",
|
||||
lambda is_audiobook=False: Path("/global/books"),
|
||||
fake_get_destination,
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"shelfmark.download.postprocess.destination.get_aa_content_type_dir",
|
||||
@@ -149,21 +157,29 @@ class TestPerUserDestination:
|
||||
|
||||
result = get_final_destination(task)
|
||||
assert result == Path("/user-books/alice")
|
||||
assert captured["user_id"] == 42
|
||||
assert captured["username"] == "alice"
|
||||
|
||||
def test_falls_back_to_global_without_override(self, monkeypatch):
|
||||
"""When no per-user destination, should use global destination."""
|
||||
def test_without_user_id_uses_global_context(self, monkeypatch):
|
||||
"""When task has no user_id, destination resolution should use global context."""
|
||||
from pathlib import Path
|
||||
|
||||
captured: dict[str, object] = {"user_id": 99, "username": "someone"}
|
||||
|
||||
task = DownloadTask(
|
||||
task_id="book1",
|
||||
source="direct_download",
|
||||
title="Test Book",
|
||||
output_args={},
|
||||
)
|
||||
|
||||
def fake_get_destination(is_audiobook: bool = False, user_id=None, username=None):
|
||||
captured["user_id"] = user_id
|
||||
captured["username"] = username
|
||||
return Path("/global/books")
|
||||
|
||||
monkeypatch.setattr(
|
||||
"shelfmark.download.postprocess.destination.get_destination",
|
||||
lambda is_audiobook=False: Path("/global/books"),
|
||||
fake_get_destination,
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"shelfmark.download.postprocess.destination.get_aa_content_type_dir",
|
||||
@@ -174,31 +190,74 @@ class TestPerUserDestination:
|
||||
|
||||
result = get_final_destination(task)
|
||||
assert result == Path("/global/books")
|
||||
assert captured["user_id"] is None
|
||||
assert captured["username"] is None
|
||||
|
||||
def test_per_user_destination_empty_string_falls_back_to_global(self, monkeypatch):
|
||||
"""Empty string destination should fall back to global."""
|
||||
def test_content_type_routing_still_wins(self, monkeypatch):
|
||||
"""Direct mode content-type routing should take priority over destination lookup."""
|
||||
from pathlib import Path
|
||||
|
||||
task = DownloadTask(
|
||||
task_id="book1",
|
||||
source="direct_download",
|
||||
title="Test Book",
|
||||
output_args={"destination": ""},
|
||||
content_type="book (fiction)",
|
||||
user_id=42,
|
||||
)
|
||||
|
||||
monkeypatch.setattr(
|
||||
"shelfmark.download.postprocess.destination.get_destination",
|
||||
lambda is_audiobook=False: Path("/global/books"),
|
||||
lambda is_audiobook=False, user_id=None, username=None: Path("/global/books"),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"shelfmark.download.postprocess.destination.get_aa_content_type_dir",
|
||||
lambda ct: None,
|
||||
lambda ct: Path("/routed/books"),
|
||||
)
|
||||
|
||||
from shelfmark.download.postprocess.destination import get_final_destination
|
||||
|
||||
result = get_final_destination(task)
|
||||
assert result == Path("/global/books")
|
||||
assert result == Path("/routed/books")
|
||||
|
||||
|
||||
class TestUserDestinationTemplate:
|
||||
"""Destination settings should support {User} placeholder expansion."""
|
||||
|
||||
def test_get_destination_expands_user_for_books(self, monkeypatch):
|
||||
from pathlib import Path
|
||||
|
||||
from shelfmark.core.config import config
|
||||
from shelfmark.core.utils import get_destination
|
||||
|
||||
def fake_config_get(key, default=None, user_id=None):
|
||||
if key == "DESTINATION":
|
||||
return "/books/{User}"
|
||||
if key == "INGEST_DIR":
|
||||
return "/books"
|
||||
return default
|
||||
|
||||
monkeypatch.setattr(config, "get", fake_config_get)
|
||||
result = get_destination(is_audiobook=False, user_id=42, username="alice")
|
||||
assert result == Path("/books/alice")
|
||||
|
||||
def test_get_destination_expands_user_for_audiobooks(self, monkeypatch):
|
||||
from pathlib import Path
|
||||
|
||||
from shelfmark.core.config import config
|
||||
from shelfmark.core.utils import get_destination
|
||||
|
||||
def fake_config_get(key, default=None, user_id=None):
|
||||
if key == "DESTINATION_AUDIOBOOK":
|
||||
return "/audiobooks/{User}"
|
||||
if key == "DESTINATION":
|
||||
return "/books/{User}"
|
||||
if key == "INGEST_DIR":
|
||||
return "/books"
|
||||
return default
|
||||
|
||||
monkeypatch.setattr(config, "get", fake_config_get)
|
||||
result = get_destination(is_audiobook=True, user_id=42, username="alice")
|
||||
assert result == Path("/audiobooks/alice")
|
||||
|
||||
|
||||
class TestTaskToDictUsername:
|
||||
|
||||
@@ -34,7 +34,7 @@ def _build_config(
|
||||
"HARDLINK_TORRENTS": hardlink,
|
||||
"HARDLINK_TORRENTS_AUDIOBOOK": hardlink,
|
||||
}
|
||||
return MagicMock(side_effect=lambda key, default=None: values.get(key, default))
|
||||
return MagicMock(side_effect=lambda key, default=None, **_kwargs: values.get(key, default))
|
||||
|
||||
|
||||
def _sync_config(mock_config, mock_core):
|
||||
@@ -475,7 +475,7 @@ def test_booklore_mode_uploads_and_cleans_staging(tmp_path):
|
||||
patch("shelfmark.download.outputs.booklore.booklore_login", return_value="token"), \
|
||||
patch("shelfmark.download.outputs.booklore.booklore_upload_file", side_effect=_upload_stub), \
|
||||
patch("shelfmark.config.env.TMP_DIR", staging):
|
||||
mock_config.get = MagicMock(side_effect=lambda key, default=None: booklore_values.get(key, default))
|
||||
mock_config.get = MagicMock(side_effect=lambda key, default=None, **_kwargs: booklore_values.get(key, default))
|
||||
|
||||
result = _post_process_download(temp_file, task, Event(), status_cb)
|
||||
|
||||
@@ -519,7 +519,7 @@ def test_booklore_mode_rejects_unsupported_files(tmp_path):
|
||||
patch("shelfmark.download.outputs.booklore.booklore_login") as mock_login, \
|
||||
patch("shelfmark.download.outputs.booklore.booklore_upload_file") as mock_upload, \
|
||||
patch("shelfmark.config.env.TMP_DIR", staging):
|
||||
mock_config.get = MagicMock(side_effect=lambda key, default=None: booklore_values.get(key, default))
|
||||
mock_config.get = MagicMock(side_effect=lambda key, default=None, **_kwargs: booklore_values.get(key, default))
|
||||
|
||||
result = _post_process_download(temp_file, task, Event(), status_cb)
|
||||
|
||||
|
||||
@@ -69,6 +69,56 @@ class TestUserDBInitialization:
|
||||
db.initialize() # Should not raise
|
||||
assert os.path.exists(db_path)
|
||||
|
||||
def test_initialize_migrates_auth_source_column_and_backfills(self, db_path):
|
||||
"""Existing DBs without auth_source should be migrated in place."""
|
||||
conn = sqlite3.connect(db_path)
|
||||
conn.executescript(
|
||||
"""
|
||||
CREATE TABLE users (
|
||||
id INTEGER PRIMARY KEY AUTOINCREMENT,
|
||||
username TEXT UNIQUE NOT NULL,
|
||||
email TEXT,
|
||||
display_name TEXT,
|
||||
password_hash TEXT,
|
||||
oidc_subject TEXT UNIQUE,
|
||||
role TEXT NOT NULL DEFAULT 'user',
|
||||
created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP
|
||||
);
|
||||
CREATE TABLE user_settings (
|
||||
user_id INTEGER PRIMARY KEY REFERENCES users(id) ON DELETE CASCADE,
|
||||
settings_json TEXT NOT NULL DEFAULT '{}'
|
||||
);
|
||||
"""
|
||||
)
|
||||
conn.execute(
|
||||
"INSERT INTO users (username, password_hash, oidc_subject, role) VALUES (?, ?, ?, ?)",
|
||||
("local_admin", "hash", None, "admin"),
|
||||
)
|
||||
conn.execute(
|
||||
"INSERT INTO users (username, oidc_subject, role) VALUES (?, ?, ?)",
|
||||
("oidc_user", "sub-123", "user"),
|
||||
)
|
||||
conn.commit()
|
||||
conn.close()
|
||||
|
||||
from shelfmark.core.user_db import UserDB
|
||||
|
||||
db = UserDB(db_path)
|
||||
db.initialize()
|
||||
|
||||
conn = sqlite3.connect(db_path)
|
||||
conn.row_factory = sqlite3.Row
|
||||
columns = conn.execute("PRAGMA table_info(users)").fetchall()
|
||||
assert "auth_source" in {str(c["name"]) for c in columns}
|
||||
|
||||
rows = conn.execute(
|
||||
"SELECT username, auth_source FROM users ORDER BY username"
|
||||
).fetchall()
|
||||
by_username = {r["username"]: r["auth_source"] for r in rows}
|
||||
assert by_username["local_admin"] == "builtin"
|
||||
assert by_username["oidc_user"] == "oidc"
|
||||
conn.close()
|
||||
|
||||
|
||||
class TestUserCRUD:
|
||||
"""Tests for user create, read, update, delete operations."""
|
||||
@@ -83,6 +133,7 @@ class TestUserCRUD:
|
||||
assert user["username"] == "john"
|
||||
assert user["email"] == "john@example.com"
|
||||
assert user["display_name"] == "John Doe"
|
||||
assert user["auth_source"] == "builtin"
|
||||
assert user["role"] == "user"
|
||||
|
||||
def test_create_user_with_password(self, user_db):
|
||||
@@ -99,8 +150,14 @@ class TestUserCRUD:
|
||||
username="oidcuser",
|
||||
oidc_subject="sub-12345",
|
||||
email="oidc@example.com",
|
||||
auth_source="oidc",
|
||||
)
|
||||
assert user["oidc_subject"] == "sub-12345"
|
||||
assert user["auth_source"] == "oidc"
|
||||
|
||||
def test_create_user_with_invalid_auth_source_fails(self, user_db):
|
||||
with pytest.raises(ValueError, match="Invalid auth_source"):
|
||||
user_db.create_user(username="john", auth_source="not-real")
|
||||
|
||||
def test_create_duplicate_username_fails(self, user_db):
|
||||
user_db.create_user(username="john")
|
||||
@@ -132,10 +189,21 @@ class TestUserCRUD:
|
||||
|
||||
def test_update_user(self, user_db):
|
||||
user = user_db.create_user(username="john", role="user")
|
||||
user_db.update_user(user["id"], role="admin", email="new@example.com")
|
||||
user_db.update_user(
|
||||
user["id"],
|
||||
role="admin",
|
||||
email="new@example.com",
|
||||
auth_source="proxy",
|
||||
)
|
||||
updated = user_db.get_user(user_id=user["id"])
|
||||
assert updated["role"] == "admin"
|
||||
assert updated["email"] == "new@example.com"
|
||||
assert updated["auth_source"] == "proxy"
|
||||
|
||||
def test_update_user_rejects_invalid_auth_source(self, user_db):
|
||||
user = user_db.create_user(username="john")
|
||||
with pytest.raises(ValueError, match="Invalid auth_source"):
|
||||
user_db.update_user(user["id"], auth_source="bad")
|
||||
|
||||
def test_update_nonexistent_user_raises(self, user_db):
|
||||
with pytest.raises(ValueError, match="not found"):
|
||||
|
||||
Reference in New Issue
Block a user