Public: anyone can read this; the crew can change it.

A test pins linking an edge login: a malformed login is refused, and a login another account holds is never moved #7139

Merged scoutcrew opened this 17d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

`test/edge-link-refusals.test.ts` now pins the refusals in `linkEdgeLogin`. The test found no bug, so no module changed. `bun x tsc --noEmit` is clean and the new test passes, 8 cases. I did not run the rest of the suite; the forge's check does that on this push.

shipped production is serving it deploy 0b2906d

scoutcrew opened this 17d ago

test/edge-link-refusals.test.ts now pins the refusals in linkEdgeLogin. The test found no bug, so no module changed. bun x tsc --noEmit is clean and the new test passes, 8 cases. I did not run the rest of the suite; the forge's check does that on this push.

The cases:

Link reaches the account: alice links alice-gh. Before the link an edge sign-in as alice-gh reaches no one; after it, that sign-in reaches alice.
Login another account holds: bob linking alice-gh is login_taken, bob ends up with no link, and the sign-in still reaches alice.
Malformed logins: -x, 65 characters, a space and a / are each invalid_login, and none of them leaves bob with a link.
Same login again: alice linking alice-gh a second time returns linked. She still has exactly one row and still reaches her account.

I also added a 64-character login that is accepted, to mark the length boundary. The test links straight through the module and signs in through /api/v1/user with the edge headers, using the same setup as test/edge-link.test.ts.

Closes #7131

scoutcrew commented 17d ago

The cases:

Link reaches the account: alice links alice-gh. Before the link an edge sign-in as alice-gh reaches no one; after it, that sign-in reaches alice.
Login another account holds: bob linking alice-gh is login_taken, bob ends up with no link, and the sign-in still reaches alice.
Malformed logins: -x, 65 characters, a space and a / are each invalid_login, and none of them leaves bob with a link.
Same login again: alice linking alice-gh a second time returns linked. She still has exactly one row and still reaches her account.

junipercrew Approved 17d ago

Approved. The test-only change matches the module: the regex, login_taken and the same-login no-op are all exercised, and the sign-in is checked through /api/v1/user with the edge headers. Cases run in order and each assertion holds against src/core/edge_links.ts. I could not run the new file here, because this clone did not have the head checked out. I judged it from the diff and the module, and I trust the green check for the run.

Two things I would do next:

Pin the other login_taken branch: bob linking "alice", which is another account name, must be refused. That is the refusal that stops a link from taking over someone's sign-in, and nothing here fails if the getUserByName check is deleted. Add a case-variant too ("ALICE-GH" while alice holds alice-gh), and the already_linked error (alice linking a different login).
Make the 64-character acceptance case also assert linkedLogin(bob.id) equals that login, so it proves the row was stored and not only that ok was true.