Try and fix OIDC (#1127)

docs/oidc.md

@@ -30,7 +30,19 @@ Configure in **Settings → Security → Authentication Method → OIDC**.
 | Auto-Provision Users | Create accounts on first login | `true` |
 | Login Button Label | Custom text for the sign-in button | — |
 
-Use **Test Connection** to verify discovery and client configuration before attempting login.
+Use **Test Connection** to verify discovery, client configuration, and the provider's token signing keys (JWKS) before attempting login.
+
+> **Authentik users:** make sure your provider has a **Signing Key** selected (e.g. the default self-signed certificate). Without one, Authentik serves an empty JWKS document and every login fails with an OIDC callback error, even though the discovery document looks healthy.
+
+## Account Linking
+
+On login, Shelfmark matches the OIDC identity to a user account in this order:
+
+1. **OIDC subject** — a user who has logged in through this provider before.
+2. **Email** — a local account with the same (unique) email address. This only happens when the provider also asserts `email_verified: true` for the address; an unverified email would let anyone claim a local account by registering its address at the IdP.
+3. Otherwise, a new account is created when **Auto-Provision Users** is enabled (username conflicts get a numeric suffix), or the login is rejected with "Account not found" when it is disabled.
+
+If the `email_verified` claim is missing or `false`, email linking is silently skipped — a common surprise when the address was never verified at the identity provider (e.g. Keycloak's **Email verified** toggle on the user, or Authentik accounts created without email verification). Make sure the `email` scope is requested and the address is marked verified in your IdP.
 
 ## Environment Variables
 
@@ -46,6 +58,8 @@ If `DISABLE_LOCAL_AUTH` and `OIDC_AUTO_REDIRECT` are both enabled, users are red
 
 ## Troubleshooting
 
+- **No token signing keys (empty JWKS)** — The provider's JWKS endpoint returned no keys, so ID tokens can't be verified. In Authentik this happens when the provider has no **Signing Key** selected; pick one (e.g. the default self-signed certificate) and try again.
 - **Issuer validation failed** — The issuer in the token doesn't match the discovery document. Check your provider's external URL / issuer configuration.
 - **Callback URL mismatch** — Reverse proxy isn't forwarding `X-Forwarded-Proto` or `X-Forwarded-Host`, so the constructed callback URL doesn't match what's registered in the provider.
-- **Account not found** — Auto-provision is disabled and the user hasn't been pre-created by an admin.
+- **Account not found** — Auto-provision is disabled and the user hasn't been pre-created by an admin. If you pre-created the account with a matching email, see [Account Linking](#account-linking): the provider must send `email_verified: true` for linking to happen.
+- **Login created a duplicate account instead of linking to my local one** — Email linking requires a verified email; see [Account Linking](#account-linking). With `DEBUG=true`, the log notes when linking is skipped because the address isn't verified.

shelfmark/config/security_handlers.py

@@ -115,7 +115,7 @@ def check_oidc_connection(
         response.raise_for_status()
         document = response.json()
 
-        required_fields = ["issuer", "authorization_endpoint", "token_endpoint"]
+        required_fields = ["issuer", "authorization_endpoint", "token_endpoint", "jwks_uri"]
         missing_fields = [field for field in required_fields if field not in document]
         if missing_fields:
             return {
@@ -123,6 +123,24 @@ def check_oidc_connection(
                 "message": f"Discovery document missing fields: {', '.join(missing_fields)}",
             }
 
+        # Logins verify the ID token against the provider's JWKS, so an empty key
+        # set (e.g. an Authentik provider with no Signing Key selected) means every
+        # login will fail even though discovery looks healthy.
+        jwks_uri = str(document["jwks_uri"])
+        jwks_response = requests.get(jwks_uri, timeout=10, verify=get_ssl_verify(jwks_uri))
+        jwks_response.raise_for_status()
+        jwks_document = jwks_response.json()
+        jwks_keys = jwks_document.get("keys") if isinstance(jwks_document, dict) else None
+        if not jwks_keys:
+            return {
+                "success": False,
+                "message": (
+                    "Discovery document is valid, but the provider returned no token "
+                    "signing keys (empty JWKS), so logins will fail. If you use "
+                    "Authentik, select a Signing Key in the provider settings."
+                ),
+            }
+
         return {"success": True, "message": f"Connected to {document['issuer']}"}
     except Exception as exc:
         logger.exception("OIDC connection test failed")

shelfmark/core/oidc_routes.py

@@ -33,6 +33,11 @@ logger = setup_logger(__name__)
 oauth = OAuth()
 _RETURN_TO_SESSION_KEY = "oidc_return_to"
 _OIDC_CLIENT_ERRORS = (OAuthError, OSError, RuntimeError, TypeError, ValueError)
+_EMPTY_JWKS_MESSAGE = (
+    "Authentication failed: the identity provider returned no token signing keys "
+    "(empty JWKS). If you use Authentik, select a Signing Key in the provider "
+    "settings and try again."
+)
 
 
 class _ClaimsMappingLike(Protocol):
@@ -121,6 +126,17 @@ def _normalize_return_to(raw_return_to: object) -> str | None:
     return urlunsplit(("", "", path, parsed.query, parsed.fragment))
 
 
+def _idp_jwks_has_no_keys(client: Any) -> bool:
+    """Return True when the IdP's JWKS document verifiably contains no signing keys."""
+    try:
+        jwk_set = client.fetch_jwk_set(force=True)
+    except (*_OIDC_CLIENT_ERRORS, KeyError):
+        return False
+    if not isinstance(jwk_set, Mapping):
+        return False
+    return not jwk_set.get("keys")
+
+
 def _get_pending_return_to(*, clear: bool = False) -> str | None:
     """Read the pending post-login target from the session."""
     raw_return_to = (
@@ -274,6 +290,17 @@ def register_oidc_routes(app: Flask, user_db: UserDB) -> None:
                 return redirect(
                     _login_error_url(f"OIDC token claim validation failed: {claim_name}")
                 )
+            except KeyError, ValueError:
+                # An IdP serving an empty JWKS document (e.g. an Authentik provider
+                # with no Signing Key selected) surfaces as KeyError('keys') while
+                # importing the key set. Test Connection only validates discovery,
+                # so this is the first place the misconfiguration becomes visible.
+                if _idp_jwks_has_no_keys(client):
+                    logger.exception(
+                        "OIDC callback failed: the IdP JWKS document contains no signing keys"
+                    )
+                    return redirect(_login_error_url(_EMPTY_JWKS_MESSAGE))
+                raise
             claims = _normalize_claims(token.get("userinfo"))
 
             # If userinfo is missing or claims are too sparse, request it explicitly.
@@ -306,6 +333,12 @@ def register_oidc_routes(app: Flask, user_db: UserDB) -> None:
                 is_admin = admin_group in groups
 
             allow_email_link = bool(user_info.get("email")) and _is_email_verified(claims)
+            if user_info.get("email") and not allow_email_link:
+                logger.debug(
+                    "OIDC email %s is not marked verified by the IdP; skipping "
+                    "email-based account linking",
+                    user_info["email"],
+                )
             user = provision_oidc_user(
                 user_db,
                 user_info,

tests/config/test_security_handlers.py

@@ -0,0 +1,82 @@
+"""Tests for the OIDC Test Connection handler."""
+
+from unittest.mock import MagicMock, patch
+
+from shelfmark.config.security_handlers import check_oidc_connection
+
+DISCOVERY_URL = "https://auth.example.com/.well-known/openid-configuration"
+
+DISCOVERY_DOCUMENT = {
+    "issuer": "https://auth.example.com",
+    "authorization_endpoint": "https://auth.example.com/authorize",
+    "token_endpoint": "https://auth.example.com/token",
+    "jwks_uri": "https://auth.example.com/jwks",
+}
+
+
+def _mock_response(payload):
+    response = MagicMock()
+    response.json.return_value = payload
+    response.raise_for_status.return_value = None
+    return response
+
+
+def _run_check(responses):
+    """Run check_oidc_connection with requests.get returning the given responses."""
+    with (
+        patch("requests.get", side_effect=responses) as mock_get,
+        patch("shelfmark.config.security_handlers.get_ssl_verify", return_value=True),
+    ):
+        result = check_oidc_connection(
+            load_security_config=lambda: {"OIDC_DISCOVERY_URL": DISCOVERY_URL},
+            current_values={},
+            logger=MagicMock(),
+        )
+    return result, mock_get
+
+
+class TestCheckOIDCConnection:
+    def test_succeeds_when_discovery_and_jwks_are_valid(self):
+        responses = [
+            _mock_response(DISCOVERY_DOCUMENT),
+            _mock_response({"keys": [{"kty": "RSA", "kid": "abc"}]}),
+        ]
+        result, mock_get = _run_check(responses)
+        assert result["success"] is True
+        assert "Connected to" in result["message"]
+        jwks_call = mock_get.call_args_list[1]
+        assert jwks_call.args[0] == DISCOVERY_DOCUMENT["jwks_uri"]
+
+    def test_fails_with_signing_key_guidance_when_jwks_is_empty(self):
+        responses = [
+            _mock_response(DISCOVERY_DOCUMENT),
+            _mock_response({}),
+        ]
+        result, _ = _run_check(responses)
+        assert result["success"] is False
+        assert "no token signing keys" in result["message"]
+        assert "Signing Key" in result["message"]
+
+    def test_fails_with_signing_key_guidance_when_jwks_keys_list_is_empty(self):
+        responses = [
+            _mock_response(DISCOVERY_DOCUMENT),
+            _mock_response({"keys": []}),
+        ]
+        result, _ = _run_check(responses)
+        assert result["success"] is False
+        assert "no token signing keys" in result["message"]
+
+    def test_fails_when_discovery_document_missing_jwks_uri(self):
+        document = {k: v for k, v in DISCOVERY_DOCUMENT.items() if k != "jwks_uri"}
+        responses = [_mock_response(document)]
+        result, _ = _run_check(responses)
+        assert result["success"] is False
+        assert "jwks_uri" in result["message"]
+
+    def test_fails_when_jwks_request_errors(self):
+        jwks_response = MagicMock()
+        jwks_response.raise_for_status.side_effect = RuntimeError("boom")
+        responses = [_mock_response(DISCOVERY_DOCUMENT), jwks_response]
+        result, _ = _run_check(responses)
+        assert result["success"] is False
+        assert "Connection failed" in result["message"]

tests/core/test_oidc_routes.py

@@ -413,6 +413,43 @@ class TestOIDCCallbackEndpoint:
         assert error is not None
         assert "issuer validation failed" in error
 
+    @patch("shelfmark.core.oidc_routes._get_oidc_client")
+    def test_callback_redirects_with_signing_key_guidance_on_empty_jwks(
+        self, mock_get_client, client
+    ):
+        fake_client = Mock()
+        fake_client.authorize_access_token.side_effect = KeyError("keys")
+        fake_client.fetch_jwk_set.return_value = {}
+        mock_get_client.return_value = (fake_client, MOCK_OIDC_CONFIG)
+
+        resp = client.get("/api/auth/oidc/callback?code=abc123&state=test-state")
+        error = _get_oidc_error(resp)
+        assert error is not None
+        assert "no token signing keys" in error
+        assert "Signing Key" in error
+
+    @patch("shelfmark.core.oidc_routes._get_oidc_client")
+    def test_callback_uses_generic_error_when_jwks_has_keys(self, mock_get_client, client):
+        fake_client = Mock()
+        fake_client.authorize_access_token.side_effect = KeyError("keys")
+        fake_client.fetch_jwk_set.return_value = {"keys": [{"kty": "RSA", "kid": "abc"}]}
+        mock_get_client.return_value = (fake_client, MOCK_OIDC_CONFIG)
+
+        resp = client.get("/api/auth/oidc/callback?code=abc123&state=test-state")
+        error = _get_oidc_error(resp)
+        assert error == "Authentication failed"
+
+    @patch("shelfmark.core.oidc_routes._get_oidc_client")
+    def test_callback_uses_generic_error_when_jwks_diagnosis_fails(self, mock_get_client, client):
+        fake_client = Mock()
+        fake_client.authorize_access_token.side_effect = KeyError("keys")
+        fake_client.fetch_jwk_set.side_effect = RuntimeError("jwks fetch failed")
+        mock_get_client.return_value = (fake_client, MOCK_OIDC_CONFIG)
+
+        resp = client.get("/api/auth/oidc/callback?code=abc123&state=test-state")
+        error = _get_oidc_error(resp)
+        assert error == "Authentication failed"
+
     @patch("shelfmark.core.oidc_routes._get_oidc_client")
     def test_callback_redirects_when_auto_provision_disabled_and_no_email_match(
         self, mock_get_client, client