fix(link): say "that address is already an account here", not "502"
Hit live today. An operator moved an email onto a different forge account; the next sign-in tried to create a user with that address, Gitea answered 422 "e-mail already in use", and the generic path turned it into a bare 502. The person approved a device code and got a number that reads like an outage, when the real answer was "that address already belongs to somebody here". create_user now distinguishes that case, and the binding rules translate it to 409 with a message naming the address, the login it tried to create, and the two ways out: have an operator bind your identity to the existing account, or use a different address. 211 tests (was 209): the stub now enforces unique emails like real Gitea, one test asserts the 409 names the address and login and never says 502, and one asserts an ordinary create is unaffected.
This commit is contained in:
@@ -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:
|
||||||
|
|||||||
@@ -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()
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user