Merge pull request 'fix(link): say "that address is already an account here", not "502"' (#10) from fix/link-duplicate-email-message into main

This commit is contained in:
Nirav Patel
2026-08-23 16:46:22 -04:00
2 changed files with 55 additions and 0 deletions
+23
View File
@@ -80,6 +80,9 @@ TEST_MODE_ENV = "GRANTHI_LINK_ALLOW_TEST_MODE"
# Sentinel: create_user hit a 409 (someone else created the login first). # Sentinel: create_user hit a 409 (someone else created the login first).
USER_CREATE_CONFLICT = object() USER_CREATE_CONFLICT = object()
# Sentinel: the address already belongs to another forge account, which is a
# 409 the caller can act on -- not a 502 that reads like the service is down.
EMAIL_IN_USE = object()
LOGIN_SAFE = re.compile(r"[^a-zA-Z0-9._-]+") LOGIN_SAFE = re.compile(r"[^a-zA-Z0-9._-]+")
@@ -292,6 +295,14 @@ def check_config_perms(path, euid=None):
# Identity map: zitadel sub -> gitea login (JSON, 0600, atomic writes) # Identity map: zitadel sub -> gitea login (JSON, 0600, atomic writes)
# -------------------------------------------------------------------------- # --------------------------------------------------------------------------
def _email_in_use_message(login, userinfo):
email = userinfo.get("email") or "your address"
return (f"cannot create the account '{login}': {email} already belongs to "
f"a different account on this forge. If that other account is "
f"yours, an operator must bind your identity to it; if it is not, "
f"use a different address.")
class IdentityStore: class IdentityStore:
"""Persistent map of Zitadel `sub` -> Gitea login binding records. """Persistent map of Zitadel `sub` -> Gitea login binding records.
@@ -677,6 +688,14 @@ class LinkService:
headers=self._admin_hdr(), body=body) headers=self._admin_hdr(), body=body)
if status == 409: if status == 409:
return USER_CREATE_CONFLICT return USER_CREATE_CONFLICT
if status == 422 and "e-mail already in use" in str(resp).lower():
# The address belongs to a DIFFERENT forge account. Reported as its
# own case because the generic path turns it into a bare 502, and
# "502" sends the person looking for an outage when the real answer
# is "that address is already somebody's account here". Hit live on
# 2026-08-23: an operator moved an email onto another account and
# the next sign-in failed with nothing but the number.
return EMAIL_IN_USE
if status != 201: if status != 201:
return f"gitea admin user create failed (HTTP {status}): {resp}" return f"gitea admin user create failed (HTTP {status}): {resp}"
return None return None
@@ -733,6 +752,8 @@ class LinkService:
"was not created by this service; refusing " "was not created by this service; refusing "
"to re-create"}, None "to re-create"}, None
err = self.create_user(login, userinfo) err = self.create_user(login, userinfo)
if err is EMAIL_IN_USE:
return 409, {"error": _email_in_use_message(login, userinfo)}, None
if err and err is not USER_CREATE_CONFLICT: if err and err is not USER_CREATE_CONFLICT:
return 502, {"error": err}, None return 502, {"error": err}, None
LOG.info("re-created service-managed gitea user %s", login) LOG.info("re-created service-managed gitea user %s", login)
@@ -758,6 +779,8 @@ class LinkService:
return 409, {"error": "login exists and is not linked " return 409, {"error": "login exists and is not linked "
"to this identity"}, None "to this identity"}, None
LOG.info("user %s created concurrently; continuing", login) LOG.info("user %s created concurrently; continuing", login)
elif err is EMAIL_IN_USE:
return 409, {"error": _email_in_use_message(login, userinfo)}, None
elif err: elif err:
return 502, {"error": err}, None return 502, {"error": err}, None
else: else:
+32
View File
@@ -77,6 +77,10 @@ class StubUpstream(BaseHTTPRequestHandler):
if self.path == "/api/v1/admin/users": if self.path == "/api/v1/admin/users":
if body["username"] in st["users"]: if body["username"] in st["users"]:
return self._json(409, {"message": "user already exists"}) return self._json(409, {"message": "user already exists"})
if body.get("email") in st["users"].values():
# real Gitea: emails are unique across accounts
return self._json(422, {"message":
f"e-mail already in use [email: {body['email']}]"})
st["users"][body["username"]] = body["email"] st["users"][body["username"]] = body["email"]
st["created"].append(body) st["created"].append(body)
return self._json(201, {"login": body["username"]}) return self._json(201, {"login": body["username"]})
@@ -1001,6 +1005,34 @@ class TestInviteSurvivesAFailedGrant(ServiceTestBase):
self.assertEqual([g["repo"] for g in self.svc.state.peek_invites("[email protected]")], self.assertEqual([g["repo"] for g in self.svc.state.peek_invites("[email protected]")],
["alice/reports"]) ["alice/reports"])
class TestDuplicateEmailIsExplained(ServiceTestBase):
"""A 502 sends someone looking for an outage. The real answer is that the
address already belongs to another account here -- say so. (Hit live on
2026-08-23 when an operator moved an email onto a different account.)"""
def setUp(self):
super().setUp()
self.enable_test_mode()
# an existing account already holds the address
StubUpstream.state["users"]["existing"] = "[email protected]"
def test_it_is_a_409_that_names_the_problem(self):
status, resp = self.stub_link("s-new", "brandnew", email="[email protected]",
verified=True, device_id="dev-x")
self.assertEqual(status, 409, resp)
msg = resp["error"]
self.assertIn("[email protected]", msg)
self.assertIn("already belongs to a different account", msg)
self.assertIn("brandnew", msg) # names the login it tried
self.assertNotIn("502", msg)
def test_a_normal_create_is_unaffected(self):
status, resp = self.stub_link("s-ok", "fresh", email="[email protected]",
verified=True, device_id="dev-y")
self.assertEqual(status, 200, resp)
self.assertIn("token", resp)
if __name__ == "__main__": if __name__ == "__main__":
unittest.main() unittest.main()