mirror of
https://github.com/calibrain/shelfmark.git
synced 2026-10-04 12:21:18 +01:00
Require verified OIDC email for account linking (#963)
Fixes a security issue relying on plain email fields for OIDC user linking. Requires verified email instead.
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -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."""
|
||||
|
||||
Reference in New Issue
Block a user