Backend auth config check (#911)

Enforces auth config required fields, providing a warning if fields are
left empty upon save

Fixes #907

shelfmark/config/security_handlers.py

@@ -12,6 +12,11 @@ if TYPE_CHECKING:
     from collections.abc import Callable
 
 _OIDC_LOCKOUT_MESSAGE = "A local admin account with a password is required before enabling OIDC. Use the 'Go to Users' button above to create one. This ensures you can still sign in if your identity provider is unavailable."
+_OIDC_REQUIRED_FIELDS = (
+    ("OIDC_DISCOVERY_URL", "Discovery URL"),
+    ("OIDC_CLIENT_ID", "Client ID"),
+    ("OIDC_CLIENT_SECRET", "Client Secret"),
+)
 
 
 def _has_local_password_admin() -> bool:
@@ -23,6 +28,30 @@ def _has_local_password_admin() -> bool:
     )
 
 
+def _load_effective_security_values(values: dict[str, Any]) -> dict[str, Any]:
+    """Merge the current save payload onto the persisted security config."""
+    from shelfmark.core.settings_registry import load_config_file
+
+    effective_values = load_config_file("security")
+    effective_values.update(values)
+    return effective_values
+
+
+def _get_missing_oidc_required_fields(effective_values: dict[str, Any]) -> list[str]:
+    """Return missing required OIDC field labels from the effective config."""
+    missing_fields: list[str] = []
+
+    for key, label in _OIDC_REQUIRED_FIELDS:
+        value = effective_values.get(key)
+        if value is None:
+            missing_fields.append(label)
+            continue
+        if isinstance(value, str) and not value.strip():
+            missing_fields.append(label)
+
+    return missing_fields
+
+
 def on_save_security(
     values: dict[str, Any],
 ) -> dict[str, Any]:
@@ -45,8 +74,21 @@ def on_save_security(
             strip_trailing_slash=False,
         )
 
-    if normalized_values.get("AUTH_METHOD") == "oidc" and not _has_local_password_admin():
-        return {"error": True, "message": _OIDC_LOCKOUT_MESSAGE, "values": normalized_values}
+    effective_values = _load_effective_security_values(normalized_values)
+    auth_method = str(effective_values.get("AUTH_METHOD", "") or "").strip().lower()
+
+    if auth_method == "oidc":
+        if not _has_local_password_admin():
+            return {"error": True, "message": _OIDC_LOCKOUT_MESSAGE, "values": normalized_values}
+
+        missing_fields = _get_missing_oidc_required_fields(effective_values)
+        if missing_fields:
+            missing_fields_text = ", ".join(missing_fields)
+            return {
+                "error": True,
+                "message": f"OIDC configuration is incomplete: missing {missing_fields_text}.",
+                "values": normalized_values,
+            }
 
     return {"error": False, "values": normalized_values}
 

tests/config/test_security.py

@@ -15,6 +15,12 @@ import pytest
 from shelfmark.core.user_db import UserDB
 
 
+def _set_config_dir(monkeypatch, config_dir: Path) -> None:
+    """Point config helpers at a test-local config directory."""
+    monkeypatch.setenv("CONFIG_DIR", str(config_dir))
+    monkeypatch.setattr("shelfmark.config.env.CONFIG_DIR", config_dir)
+
+
 @pytest.fixture
 def temp_config_dir():
     """Create a temporary config directory for tests."""
@@ -404,7 +410,7 @@ class TestSecurityOnSave:
     def test_on_save_passthrough_for_non_oidc(self, tmp_path, monkeypatch):
         from shelfmark.config.security import _on_save_security
 
-        monkeypatch.setenv("CONFIG_DIR", str(tmp_path))
+        _set_config_dir(monkeypatch, tmp_path)
         values = {"AUTH_METHOD": "builtin", "PROXY_AUTH_USER_HEADER": "X-Auth-User"}
 
         result = _on_save_security(values.copy())
@@ -415,7 +421,7 @@ class TestSecurityOnSave:
     def test_on_save_blocks_oidc_without_local_admin(self, tmp_path, monkeypatch):
         from shelfmark.config.security import _on_save_security
 
-        monkeypatch.setenv("CONFIG_DIR", str(tmp_path))
+        _set_config_dir(monkeypatch, tmp_path)
         UserDB(str(tmp_path / "users.db")).initialize()
 
         result = _on_save_security({"AUTH_METHOD": "oidc"})
@@ -423,24 +429,92 @@ class TestSecurityOnSave:
         assert result["error"] is True
         assert "local admin" in result["message"].lower()
 
-    def test_on_save_allows_oidc_with_local_password_admin(self, tmp_path, monkeypatch):
+    def test_on_save_blocks_oidc_when_client_id_is_missing(self, tmp_path, monkeypatch):
         from shelfmark.config.security import _on_save_security
 
-        monkeypatch.setenv("CONFIG_DIR", str(tmp_path))
+        _set_config_dir(monkeypatch, tmp_path)
         user_db = UserDB(str(tmp_path / "users.db"))
         user_db.initialize()
         user_db.create_user(username="admin", password_hash="hash", role="admin")
 
-        result = _on_save_security({"AUTH_METHOD": "oidc"})
+        result = _on_save_security(
+            {
+                "AUTH_METHOD": "oidc",
+                "OIDC_DISCOVERY_URL": "https://auth.example.com/.well-known/openid-configuration",
+                "OIDC_CLIENT_SECRET": "secret123",
+            }
+        )
+
+        assert result["error"] is True
+        assert "client id" in result["message"].lower()
+
+    def test_on_save_blocks_oidc_when_discovery_url_is_missing(self, tmp_path, monkeypatch):
+        from shelfmark.config.security import _on_save_security
+
+        _set_config_dir(monkeypatch, tmp_path)
+        user_db = UserDB(str(tmp_path / "users.db"))
+        user_db.initialize()
+        user_db.create_user(username="admin", password_hash="hash", role="admin")
+
+        result = _on_save_security(
+            {
+                "AUTH_METHOD": "oidc",
+                "OIDC_CLIENT_ID": "shelfmark",
+                "OIDC_CLIENT_SECRET": "secret123",
+            }
+        )
+
+        assert result["error"] is True
+        assert "discovery url" in result["message"].lower()
+
+    def test_on_save_blocks_oidc_when_secret_is_missing(self, tmp_path, monkeypatch):
+        from shelfmark.config.security import _on_save_security
+
+        _set_config_dir(monkeypatch, tmp_path)
+        user_db = UserDB(str(tmp_path / "users.db"))
+        user_db.initialize()
+        user_db.create_user(username="admin", password_hash="hash", role="admin")
+
+        result = _on_save_security(
+            {
+                "AUTH_METHOD": "oidc",
+                "OIDC_DISCOVERY_URL": "https://auth.example.com/.well-known/openid-configuration",
+                "OIDC_CLIENT_ID": "shelfmark",
+            }
+        )
+
+        assert result["error"] is True
+        assert "client secret" in result["message"].lower()
+
+    def test_on_save_allows_oidc_with_existing_saved_secret(self, tmp_path, monkeypatch):
+        from shelfmark.config.security import _on_save_security
+        from shelfmark.core.settings_registry import save_config_file
+
+        _set_config_dir(monkeypatch, tmp_path)
+        user_db = UserDB(str(tmp_path / "users.db"))
+        user_db.initialize()
+        user_db.create_user(username="admin", password_hash="hash", role="admin")
+        save_config_file(
+            "security",
+            {
+                "AUTH_METHOD": "oidc",
+                "OIDC_DISCOVERY_URL": "https://auth.example.com/.well-known/openid-configuration",
+                "OIDC_CLIENT_ID": "existing-client",
+                "OIDC_CLIENT_SECRET": "saved-secret",
+            },
+        )
+
+        result = _on_save_security({"AUTH_METHOD": "oidc", "OIDC_CLIENT_ID": "updated-client"})
 
         assert result["error"] is False
+        assert result["values"]["OIDC_CLIENT_ID"] == "updated-client"
 
     def test_on_save_normalizes_oidc_discovery_url_without_stripping_trailing_slash(
         self, tmp_path, monkeypatch
     ):
         from shelfmark.config.security import _on_save_security
 
-        monkeypatch.setenv("CONFIG_DIR", str(tmp_path))
+        _set_config_dir(monkeypatch, tmp_path)
         values = {"OIDC_DISCOVERY_URL": " 'auth.example.com/.well-known/openid-configuration/' "}
 
         result = _on_save_security(values)
@@ -454,7 +528,7 @@ class TestSecurityOnSave:
     def test_on_save_normalizes_proxy_logout_url(self, tmp_path, monkeypatch):
         from shelfmark.config.security import _on_save_security
 
-        monkeypatch.setenv("CONFIG_DIR", str(tmp_path))
+        _set_config_dir(monkeypatch, tmp_path)
         values = {"PROXY_AUTH_LOGOUT_URL": "auth.example.com/logout"}
 
         result = _on_save_security(values)

tests/core/test_admin_users_api.py

@@ -1726,6 +1726,7 @@ class TestOIDCLockoutPrevention:
         """Point CONFIG_DIR to a temp dir so _on_save_security can find users.db."""
         config_dir = str(tmp_path)
         monkeypatch.setenv("CONFIG_DIR", config_dir)
+        monkeypatch.setattr("shelfmark.config.env.CONFIG_DIR", tmp_path)
         # Create user_db at the path _on_save_security will look for
         self._user_db = UserDB(os.path.join(config_dir, "users.db"))
         self._user_db.initialize()
@@ -1768,7 +1769,14 @@ class TestOIDCLockoutPrevention:
             password_hash="hashed_pw",
             role="admin",
         )
-        result = self._call_on_save({"AUTH_METHOD": "oidc"})
+        result = self._call_on_save(
+            {
+                "AUTH_METHOD": "oidc",
+                "OIDC_DISCOVERY_URL": "https://auth.example.com/.well-known/openid-configuration",
+                "OIDC_CLIENT_ID": "shelfmark",
+                "OIDC_CLIENT_SECRET": "secret123",
+            }
+        )
         assert result["error"] is False
 
     def test_non_oidc_methods_not_blocked(self):