diff --git a/shelfmark/core/oidc_routes.py b/shelfmark/core/oidc_routes.py index b6e95615..c0e589d0 100644 --- a/shelfmark/core/oidc_routes.py +++ b/shelfmark/core/oidc_routes.py @@ -73,6 +73,16 @@ def _has_username_or_email(claims: dict[str, Any]) -> bool: return False +def _is_email_verified(claims: dict[str, Any]) -> bool: + """Return True when claims explicitly mark the email address as verified.""" + email_verified = claims.get("email_verified") + if isinstance(email_verified, bool): + return email_verified + if isinstance(email_verified, str): + return email_verified.strip().lower() == "true" + return False + + def _login_error_url(message: str) -> str: """Build a login URL (with script_root) that includes an OIDC error message.""" script_root = request.script_root.rstrip("/") @@ -295,7 +305,7 @@ def register_oidc_routes(app: Flask, user_db: UserDB) -> None: if admin_group and use_admin_group: is_admin = admin_group in groups - allow_email_link = bool(user_info.get("email")) + allow_email_link = bool(user_info.get("email")) and _is_email_verified(claims) user = provision_oidc_user( user_db, user_info, diff --git a/tests/core/test_oidc_routes.py b/tests/core/test_oidc_routes.py index 36129606..48774f6c 100644 --- a/tests/core/test_oidc_routes.py +++ b/tests/core/test_oidc_routes.py @@ -445,6 +445,7 @@ class TestOIDCCallbackEndpoint: "userinfo": { "sub": "oidc-alice-sub", "email": "alice@example.com", + "email_verified": True, "preferred_username": "alice_oidc", "groups": [], } @@ -548,6 +549,7 @@ class TestOIDCCallbackEndpoint: "userinfo": { "sub": "oidc-new-sub", "email": "shared@example.com", + "email_verified": True, "preferred_username": "oidcuser", "groups": [], } @@ -574,6 +576,7 @@ class TestOIDCCallbackEndpoint: "userinfo": { "sub": "oidc-nomatch", "email": "different@example.com", + "email_verified": True, "preferred_username": "newuser", "groups": [], } @@ -589,6 +592,60 @@ class TestOIDCCallbackEndpoint: original = user_db.get_user(username="existing") assert original["oidc_subject"] is None + @patch("shelfmark.core.oidc_routes._get_oidc_client") + def test_callback_does_not_link_with_unverified_email(self, mock_get_client, client, user_db): + """OIDC login should not link by email when email_verified is false.""" + user_db.create_user(username="existing", email="shared@example.com", password_hash="hash") + + fake_client = Mock() + fake_client.authorize_access_token.return_value = { + "userinfo": { + "sub": "oidc-unverified", + "email": "shared@example.com", + "email_verified": False, + "preferred_username": "attackeruser", + "groups": [], + } + } + mock_get_client.return_value = (fake_client, MOCK_OIDC_CONFIG) + + resp = client.get("/api/auth/oidc/callback?code=abc123&state=test-state") + assert resp.status_code == 302 + + with client.session_transaction() as sess: + assert sess["user_id"] == "attackeruser" + + original = user_db.get_user(username="existing") + assert original["oidc_subject"] is None + + @patch("shelfmark.core.oidc_routes._get_oidc_client") + def test_callback_rejects_unverified_email_link_when_no_provision( + self, mock_get_client, client, user_db + ): + """OIDC login should not link by unverified email when creation is disabled.""" + config = {**MOCK_OIDC_CONFIG, "OIDC_AUTO_PROVISION": False} + user_db.create_user(username="existing", email="shared@example.com", password_hash="hash") + + fake_client = Mock() + fake_client.authorize_access_token.return_value = { + "userinfo": { + "sub": "oidc-unverified-no-provision", + "email": "shared@example.com", + "email_verified": False, + "preferred_username": "attackeruser", + "groups": [], + } + } + mock_get_client.return_value = (fake_client, config) + + resp = client.get("/api/auth/oidc/callback?code=abc123&state=test-state") + error = _get_oidc_error(resp) + assert error is not None + assert "Account not found" in error + + original = user_db.get_user(username="existing") + assert original["oidc_subject"] is None + @patch("shelfmark.core.oidc_routes._get_oidc_client") def test_callback_no_email_link_when_oidc_has_no_email(self, mock_get_client, client, user_db): """OIDC login without email in claims should not attempt email linking."""