mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-10-25 14:00:40 +00:00
fix(server): register-with-code claims the invitation atomically; a gone invitation refuses and rolls the account back (BUG-3438) (#1834)
* test(server): register-with-code through an invitation gone before its claim joins nothing (BUG-3438) * fix(server): register-with-code claims the invitation in the membership's transaction; a gone invitation refuses and rolls the account back (BUG-3438) * test(server): a cloud signup refused for a gone invitation is rolled back; a failed claim beside a membership is not a join (BUG-3438, codex r1) * fix(server): the signup rollback removes the account's verification tokens with it; a membership the claim did not write is not a join (BUG-3438, codex r1) * test(server): a session minted in the window does not block the signup rollback; a refused signup keeps the emailed proof for the retry (BUG-3438, codex r2) * fix(server): the signup rollback removes the account's sessions under its row lock and restores a spent invitation proof (BUG-3438, codex r2) * test(server): a reset token minted in the window does not block the signup rollback (BUG-3438, codex r3) * fix(server): the signup rollback is the full account deletion; a proof whose consume reported an error counts as possibly spent (BUG-3438, codex r3) * fix(server): an invitation proof whose consume errored is never restored from what the caller sent (BUG-3438, codex r4) * test(server): an invited signup reports the role the account holds (BUG-3438, codex r5) * fix(server): an invited signup reports the role the claim kept (BUG-3438, codex r5) * fix(server): the signup rollback kicks the account's connections, as every account deletion does (BUG-3438, codex r6) * fix(server): the signup rollback kicks even when the deletion reports an error (BUG-3438, codex r7) * test(server): on open registration a refused invited signup keeps the account, joins nothing and says why (BUG-3438, lead ruling) * test(web): a registration kept without its invitation says so on the join page and lands nowhere (BUG-3438) * fix(server,web): a refused invited signup is rolled back only when the invitation was its admission; on open registration the account is kept and told (BUG-3438, lead ruling) * test(server): a claim refused before commit needs no reconcile read; a failed rollback is not reported as one (BUG-3438, codex r9) * fix(server): a claim refused before commit skips the reconcile read; a failed rollback answers 500; docs name the admin case (BUG-3438, codex r9)
This commit is contained in:
@@ -128,7 +128,7 @@ One browser tab's content writes to an item are ordered by the server, also **pe
|
||||
- `GET /api/v1/me/invitations`, `POST /api/v1/me/invitations/{id}/accept` — the caller's pending, unexpired invitations to live workspaces they are not already in (PLAN-3002 U4b / TASK-3277), backing the Invitations section of the TopBar "+". Matched on the caller's email ONLY when it is VERIFIED, since an unverified address is just a string typed at signup; every self-hosted account is verified (migration 070 + `CreateUser`'s default), so this bites only on Pad Cloud self-serve accounts that have not confirmed, which get an empty list with `email_verified: false`. Entries carry no code (only its hash is stored), hence the by-id accept, which shares `acceptInvitationCore` with `/invitations/{code}/accept` (membership, role, `accepted_at`, the `workspace_access_changed` gain). It differs in admission only: the caller must already be verified, and it never verifies them. Neither does the code route, since BUG-3348: the inviter also sees the code. What verifies is the mailbox-only proof the invitation EMAIL's link carries in its fragment (`/join/<code>#proof=…`, TASK-3352), sent as `proof` on `POST /invitations/{code}/accept` or `invitation_proof` on register. It is stored hashed, single-use and bound to the invitation, and never appears in the invite response, the members or admin invitation lists, or copy-link; a wrong or spent one is ignored, not refused. An id that is not a pending invitation to the caller answers 404, byte-identical to an unknown id. Both accept routes' success bodies now also carry `workspace_slug` and `owner_username` (additive). An accept by a caller who is ALREADY a member, and the loser of two concurrent accepts, answers 200 and marks the invitation accepted without changing the member's role in either direction; the body's `role` is the role held, and nothing is published (BUG-3281; it used to be a 500). The web client (the TopBar "+" and its count badge) and the CLI (`pad workspace invitations` / `accept <id>` / `decline <id>`, BUG-2136) use them; not on MCP
|
||||
- `GET /api/v1/auth/session` — auth status (`setup_required`, `setup_method`, `auth_method`, `authenticated`, `email_configured`, `user`)
|
||||
- `POST /api/v1/auth/bootstrap` — create the first admin account from localhost on a fresh instance
|
||||
- `POST /api/v1/auth/register` — create account (admin-created or invitation-based after setup)
|
||||
- `POST /api/v1/auth/register` — create account (admin-created or invitation-based after setup). An `invitation_code` signup claims the invitation in the same transaction as the membership (BUG-3438). If the invitation was declined, cancelled or replaced since the code was read: when the code was the signup's ADMISSION (registration is otherwise closed) the account is rolled back and the answer is `404 not_found` (`500` if the rollback itself fails, since the account then still holds the email); where the signup would have been admitted anyway (Pad Cloud self-serve, or an admin creating the account) the account is KEPT, joins nothing, and the 201 carries `invitation_not_joined: {code, message}` (`invitation_gone`, or `invitation_not_applied` for another refused claim) instead of `accepted_invitation`. Additive, omitempty
|
||||
- `POST /api/v1/auth/login` — email/password login (returns session token)
|
||||
- `POST /api/v1/auth/logout` — destroy session
|
||||
- `GET/PATCH /api/v1/auth/me` — current user profile (GET) and update name/password (PATCH)
|
||||
|
||||
@@ -0,0 +1,368 @@
|
||||
package server
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/PerpetualSoftware/pad/internal/models"
|
||||
"github.com/PerpetualSoftware/pad/internal/store"
|
||||
)
|
||||
|
||||
// BUG-3438: register-with-code read the invitation, created the account,
|
||||
// added the membership and then marked the invitation accepted without
|
||||
// checking that the row was still there. An invitation declined, cancelled
|
||||
// or replaced inside that window still let the new account join. The signup
|
||||
// must claim the invitation in the transaction that adds the membership, and
|
||||
// a claim that finds no row refuses the signup and rolls the account back,
|
||||
// as the two accept doors do since BUG-2136.
|
||||
func TestRegisterWithCode_InvitationGoneBeforeClaimJoinsNothing(t *testing.T) {
|
||||
forEachDialect(t, func(t *testing.T, d store.DriverType) {
|
||||
f := newAccessFixture(t, d)
|
||||
register := func(email, code string) *httptest.ResponseRecorder {
|
||||
return doRequest(f.srv, "POST", "/api/v1/auth/register", map[string]string{
|
||||
"email": email,
|
||||
"name": "Invitee",
|
||||
"password": "correct-horse-battery-staple",
|
||||
"invitation_code": code,
|
||||
})
|
||||
}
|
||||
joined := func(email string) bool {
|
||||
t.Helper()
|
||||
u, err := f.srv.store.GetUserByEmail(email)
|
||||
if err != nil {
|
||||
t.Fatalf("GetUserByEmail: %v", err)
|
||||
}
|
||||
if u == nil {
|
||||
return false
|
||||
}
|
||||
m, err := f.srv.store.GetWorkspaceMember(f.wsID, u.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("GetWorkspaceMember: %v", err)
|
||||
}
|
||||
return m != nil
|
||||
}
|
||||
|
||||
// CONTROL: nothing happens in the window; the signup joins.
|
||||
ctl := f.invite("control@example.com", "editor")
|
||||
if rr := register("control@example.com", ctl.Code); rr.Code != http.StatusCreated {
|
||||
t.Fatalf("control signup: %d %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
if !joined("control@example.com") {
|
||||
t.Fatal("control signup did not join")
|
||||
}
|
||||
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
email string
|
||||
gone func(invID string)
|
||||
}{
|
||||
{"declined or cancelled", "declined@example.com", func(invID string) {
|
||||
if err := f.srv.store.DeleteInvitation(f.wsID, invID); err != nil {
|
||||
t.Fatalf("DeleteInvitation: %v", err)
|
||||
}
|
||||
}},
|
||||
{"replaced by a re-invite", "replaced@example.com", func(string) {
|
||||
f.invite("replaced@example.com", "viewer")
|
||||
}},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
inv := f.invite(tc.email, "editor")
|
||||
f.srv.registerInvitationPreClaimHook = tc.gone
|
||||
defer func() { f.srv.registerInvitationPreClaimHook = nil }()
|
||||
|
||||
rr := register(tc.email, inv.Code)
|
||||
if rr.Code != http.StatusNotFound {
|
||||
t.Errorf("signup with an invitation gone before its claim: %d %s, want 404", rr.Code, rr.Body.String())
|
||||
}
|
||||
if joined(tc.email) {
|
||||
t.Error("the signup joined the workspace through an invitation that was gone")
|
||||
}
|
||||
// Rolled back, so the invitee can sign up again with a live code.
|
||||
if u, _ := f.srv.store.GetUserByEmail(tc.email); u != nil {
|
||||
t.Error("the refused signup left its account behind, holding the email")
|
||||
}
|
||||
})
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// notJoinedBody is the 201 of a signup kept without its invitation.
|
||||
type notJoinedBody struct {
|
||||
Token string `json:"token"`
|
||||
Accepted map[string]any `json:"accepted_invitation"`
|
||||
NotJoined *struct {
|
||||
Code string `json:"code"`
|
||||
Message string `json:"message"`
|
||||
} `json:"invitation_not_joined"`
|
||||
}
|
||||
|
||||
// Lead ruling on #1834: the account is rolled back only when the invitation
|
||||
// was the signup's ADMISSION. Where registration is open anyway (Pad Cloud
|
||||
// with a deliverable verification email, the self-serve path), the account is
|
||||
// KEPT, nothing is joined, and the response says the invitation is no longer
|
||||
// valid. The self-hosted legs above are the admission case and roll back.
|
||||
func TestRegisterWithCode_OpenRegistrationKeepsTheAccountWhenTheInvitationIsGone(t *testing.T) {
|
||||
srv, mails := newCloudEmailServer(t)
|
||||
admin, err := srv.store.GetUserByEmail("admin@pad.test")
|
||||
if err != nil || admin == nil {
|
||||
t.Fatalf("admin lookup: %v", err)
|
||||
}
|
||||
ws, err := srv.store.CreateWorkspace(models.WorkspaceCreate{Name: "Cloud Invite WS", OwnerID: admin.ID})
|
||||
if err != nil {
|
||||
t.Fatalf("CreateWorkspace: %v", err)
|
||||
}
|
||||
inv, err := srv.store.CreateInvitation(ws.ID, "late@example.com", "editor", admin.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("CreateInvitation: %v", err)
|
||||
}
|
||||
srv.registerInvitationPreClaimHook = func(invID string) {
|
||||
if err := srv.store.DeleteInvitation(ws.ID, invID); err != nil {
|
||||
t.Fatalf("DeleteInvitation: %v", err)
|
||||
}
|
||||
}
|
||||
rr := doRequest(srv, "POST", "/api/v1/auth/register", map[string]string{
|
||||
"email": "late@example.com", "name": "Late",
|
||||
"password": "correct-horse-battery-staple", "invitation_code": inv.Code,
|
||||
})
|
||||
if rr.Code != http.StatusCreated {
|
||||
t.Fatalf("open-registration signup with a gone invitation: %d %s, want 201 (account kept)", rr.Code, rr.Body.String())
|
||||
}
|
||||
var body notJoinedBody
|
||||
parseJSON(t, rr, &body)
|
||||
if body.Token == "" {
|
||||
t.Error("the kept account was not signed in")
|
||||
}
|
||||
if body.Accepted != nil {
|
||||
t.Errorf("a signup that joined nothing reported accepted_invitation %v", body.Accepted)
|
||||
}
|
||||
if body.NotJoined == nil || body.NotJoined.Code != "invitation_gone" || body.NotJoined.Message == "" {
|
||||
t.Errorf("invitation_not_joined = %+v, want code invitation_gone with a message", body.NotJoined)
|
||||
}
|
||||
u, _ := srv.store.GetUserByEmail("late@example.com")
|
||||
if u == nil {
|
||||
t.Fatal("open registration: the account was rolled back")
|
||||
}
|
||||
if member, _ := srv.store.IsWorkspaceMember(ws.ID, u.ID); member {
|
||||
t.Error("the kept account joined through an invitation that was gone")
|
||||
}
|
||||
// Kept like any self-serve signup: unverified, with its link sent.
|
||||
if u.IsEmailVerified() {
|
||||
t.Error("the kept account is verified, though nothing proved the address")
|
||||
}
|
||||
select {
|
||||
case m := <-mails:
|
||||
if m.to != "late@example.com" {
|
||||
t.Errorf("verification mail went to %q", m.to)
|
||||
}
|
||||
case <-time.After(5 * time.Second):
|
||||
t.Error("the kept account was sent no verification link")
|
||||
}
|
||||
}
|
||||
|
||||
// Codex r1: the claim now commits with the membership, so a membership alone no
|
||||
// longer proves the claim landed. A failed claim beside a membership that came
|
||||
// from elsewhere must not be reported as a joined signup.
|
||||
func TestRegisterWithCode_FailedClaimBesideAMembershipIsNotAJoin(t *testing.T) {
|
||||
e := newAcceptLimitEnv(t)
|
||||
inv := e.invite(t, "beside@example.com")
|
||||
// The claim fails (the workspace reaches its cap in the window), and the
|
||||
// membership read finds a row, as an admin's concurrent add would leave.
|
||||
e.srv.registerInvitationPreClaimHook = func(string) { e.setMemberCap(t, e.members(t)) }
|
||||
e.srv.membershipCheck = func(workspaceID, userID string) (*models.WorkspaceMember, error) {
|
||||
return &models.WorkspaceMember{WorkspaceID: workspaceID, UserID: userID, Role: "editor"}, nil
|
||||
}
|
||||
rr := e.register("beside@example.com", inv)
|
||||
if rr.Code == http.StatusCreated {
|
||||
t.Fatalf("a signup whose claim failed was reported joined: %s", rr.Body.String())
|
||||
}
|
||||
if !e.stillPending(t, inv) {
|
||||
t.Error("the invitation was consumed by a claim that failed")
|
||||
}
|
||||
}
|
||||
|
||||
// Codex r2/r3: the account exists from CreateUser on, so a sign-in can mint it
|
||||
// a session, and /forgot-password a reset token, before the claim. The
|
||||
// rollback must still remove the account.
|
||||
func TestRegisterWithCode_CredentialsMintedBeforeTheClaimDoNotBlockTheRollback(t *testing.T) {
|
||||
forEachDialect(t, func(t *testing.T, d store.DriverType) {
|
||||
f := newAccessFixture(t, d)
|
||||
inv := f.invite("signedin@example.com", "editor")
|
||||
f.srv.registerInvitationPreClaimHook = func(invID string) {
|
||||
u, err := f.srv.store.GetUserByEmail("signedin@example.com")
|
||||
if err != nil || u == nil {
|
||||
t.Fatalf("lookup the new account: %v", err)
|
||||
}
|
||||
if _, err := f.srv.store.CreateSession(u.ID, "go-test", "192.0.2.1", "", time.Hour); err != nil {
|
||||
t.Fatalf("CreateSession: %v", err)
|
||||
}
|
||||
// And a public /forgot-password, which mints a reset token (codex r3).
|
||||
if _, err := f.srv.store.CreatePasswordReset(u.ID); err != nil {
|
||||
t.Fatalf("CreatePasswordReset: %v", err)
|
||||
}
|
||||
if err := f.srv.store.DeleteInvitation(f.wsID, invID); err != nil {
|
||||
t.Fatalf("DeleteInvitation: %v", err)
|
||||
}
|
||||
}
|
||||
rr := doRequest(f.srv, "POST", "/api/v1/auth/register", map[string]string{
|
||||
"email": "signedin@example.com", "name": "Invitee",
|
||||
"password": "correct-horse-battery-staple", "invitation_code": inv.Code,
|
||||
})
|
||||
if rr.Code != http.StatusNotFound {
|
||||
t.Fatalf("signup: %d %s, want 404", rr.Code, rr.Body.String())
|
||||
}
|
||||
if u, _ := f.srv.store.GetUserByEmail("signedin@example.com"); u != nil {
|
||||
t.Fatal("a session minted in the window kept the refused signup's account")
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// Open registration, a claim refused for another reason (the workspace filled
|
||||
// in the window): the account is kept and verified by its emailed proof,
|
||||
// nothing is joined, the invitation stays pending, and the kept account
|
||||
// accepts it from its own list once there is room.
|
||||
func TestRegisterWithCode_OpenRegistrationKeepsTheAccountWhenTheClaimIsRefused(t *testing.T) {
|
||||
srv, mails, adminTok, ws := newProofFixture(t)
|
||||
_, code, proof := inviteByEmail(t, srv, mails, adminTok, ws.Slug, "proved@example.com")
|
||||
admin, err := srv.store.GetUserByEmail("admin@pad.test")
|
||||
if err != nil || admin == nil {
|
||||
t.Fatalf("admin lookup: %v", err)
|
||||
}
|
||||
count := func() int {
|
||||
members, err := srv.store.ListWorkspaceMembers(ws.ID)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return len(members)
|
||||
}
|
||||
setCap := func(n int) {
|
||||
if err := srv.store.SetUserPlanOverrides(admin.ID, fmt.Sprintf(`{"members_per_workspace":%d}`, n)); err != nil {
|
||||
t.Fatalf("SetUserPlanOverrides: %v", err)
|
||||
}
|
||||
}
|
||||
srv.registerInvitationPreClaimHook = func(string) { setCap(count()) }
|
||||
rr := doRequest(srv, "POST", "/api/v1/auth/register", map[string]string{
|
||||
"email": "proved@example.com", "name": "Invitee", "password": "correct-horse-battery-staple",
|
||||
"invitation_code": code, "invitation_proof": proof,
|
||||
})
|
||||
srv.registerInvitationPreClaimHook = nil
|
||||
if rr.Code != http.StatusCreated {
|
||||
t.Fatalf("open-registration signup refused at the cap: %d %s, want 201 (account kept)", rr.Code, rr.Body.String())
|
||||
}
|
||||
var body notJoinedBody
|
||||
parseJSON(t, rr, &body)
|
||||
if body.Accepted != nil || body.NotJoined == nil || body.NotJoined.Code != "invitation_not_applied" {
|
||||
t.Errorf("accepted_invitation = %v, invitation_not_joined = %+v; want none and code invitation_not_applied", body.Accepted, body.NotJoined)
|
||||
}
|
||||
if !verified(t, srv, "proved@example.com") {
|
||||
t.Error("the kept account is not verified, though its emailed proof was spent on it")
|
||||
}
|
||||
pending, err := srv.store.GetInvitationByCode(code)
|
||||
if err != nil || pending == nil {
|
||||
t.Fatalf("the invitation should stay pending: %v %v", pending, err)
|
||||
}
|
||||
// With room, the kept account accepts it from its own list.
|
||||
setCap(count() + 10)
|
||||
rr = doRequestWithCookie(srv, "POST", "/api/v1/me/invitations/"+pending.ID+"/accept", nil, body.Token)
|
||||
if rr.Code != http.StatusOK {
|
||||
t.Fatalf("accepting from the list afterwards: %d %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
// Codex r5: the claim keeps a membership that already exists (an admin's add
|
||||
// in the window) at ITS role, and the signup must report that role, as the
|
||||
// accept doors do, not the invitation's.
|
||||
func TestRegisterWithCode_ReportsTheRoleTheAccountHolds(t *testing.T) {
|
||||
forEachDialect(t, func(t *testing.T, d store.DriverType) {
|
||||
f := newAccessFixture(t, d)
|
||||
inv := f.invite("added@example.com", "editor")
|
||||
f.srv.registerInvitationPreClaimHook = func(string) {
|
||||
u, err := f.srv.store.GetUserByEmail("added@example.com")
|
||||
if err != nil || u == nil {
|
||||
t.Fatalf("lookup the new account: %v", err)
|
||||
}
|
||||
if err := f.srv.store.AddWorkspaceMember(f.wsID, u.ID, "viewer"); err != nil {
|
||||
t.Fatalf("AddWorkspaceMember: %v", err)
|
||||
}
|
||||
}
|
||||
rr := doRequest(f.srv, "POST", "/api/v1/auth/register", map[string]string{
|
||||
"email": "added@example.com", "name": "Invitee",
|
||||
"password": "correct-horse-battery-staple", "invitation_code": inv.Code,
|
||||
})
|
||||
if rr.Code != http.StatusCreated {
|
||||
t.Fatalf("signup: %d %s", rr.Code, rr.Body.String())
|
||||
}
|
||||
var body struct {
|
||||
Accepted struct {
|
||||
Role string `json:"role"`
|
||||
} `json:"accepted_invitation"`
|
||||
}
|
||||
parseJSON(t, rr, &body)
|
||||
if body.Accepted.Role != "viewer" {
|
||||
t.Errorf("accepted_invitation.role = %q, want the role held (viewer)", body.Accepted.Role)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// Codex r9: a claim the store refused BEFORE its commit is definitely not
|
||||
// landed, so a failing membership read must not turn the open-registration
|
||||
// outcome (account kept, told) into a 500 that leaves the account behind with
|
||||
// no session and a retry that meets the duplicate-email 409.
|
||||
func TestRegisterWithCode_OpenRegistrationRefusedClaimNeedsNoReconcileRead(t *testing.T) {
|
||||
srv, mails, adminTok, ws := newProofFixture(t)
|
||||
_, code, _ := inviteByEmail(t, srv, mails, adminTok, ws.Slug, "unread@example.com")
|
||||
admin, err := srv.store.GetUserByEmail("admin@pad.test")
|
||||
if err != nil || admin == nil {
|
||||
t.Fatalf("admin lookup: %v", err)
|
||||
}
|
||||
srv.registerInvitationPreClaimHook = func(string) {
|
||||
members, err := srv.store.ListWorkspaceMembers(ws.ID)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := srv.store.SetUserPlanOverrides(admin.ID, fmt.Sprintf(`{"members_per_workspace":%d}`, len(members))); err != nil {
|
||||
t.Fatalf("SetUserPlanOverrides: %v", err)
|
||||
}
|
||||
}
|
||||
srv.membershipCheck = func(string, string) (*models.WorkspaceMember, error) {
|
||||
return nil, fmt.Errorf("injected: membership read failed")
|
||||
}
|
||||
rr := doRequest(srv, "POST", "/api/v1/auth/register", map[string]string{
|
||||
"email": "unread@example.com", "name": "Invitee",
|
||||
"password": "correct-horse-battery-staple", "invitation_code": code,
|
||||
})
|
||||
if rr.Code != http.StatusCreated {
|
||||
t.Fatalf("open-registration signup refused before commit, with an unreadable membership: %d %s, want 201", rr.Code, rr.Body.String())
|
||||
}
|
||||
var body notJoinedBody
|
||||
parseJSON(t, rr, &body)
|
||||
if body.NotJoined == nil || body.NotJoined.Code != "invitation_not_applied" || body.Token == "" {
|
||||
t.Errorf("invitation_not_joined = %+v, token set = %v; want invitation_not_applied and a session", body.NotJoined, body.Token != "")
|
||||
}
|
||||
}
|
||||
|
||||
// Codex r9: when the invitation was the admission and the account rollback
|
||||
// FAILS, the signup must not answer as if it had been rolled back: the
|
||||
// account still holds the email.
|
||||
func TestRegisterWithCode_FailedRollbackIsNotReportedAsARollback(t *testing.T) {
|
||||
f := newAccessFixture(t, store.DriverSQLite)
|
||||
inv := f.invite("stuck@example.com", "editor")
|
||||
f.srv.registerInvitationPreClaimHook = func(invID string) {
|
||||
if err := f.srv.store.DeleteInvitation(f.wsID, invID); err != nil {
|
||||
t.Fatalf("DeleteInvitation: %v", err)
|
||||
}
|
||||
}
|
||||
f.srv.signupRollbackDelete = func(string) error { return fmt.Errorf("injected: rollback failed") }
|
||||
rr := doRequest(f.srv, "POST", "/api/v1/auth/register", map[string]string{
|
||||
"email": "stuck@example.com", "name": "Invitee",
|
||||
"password": "correct-horse-battery-staple", "invitation_code": inv.Code,
|
||||
})
|
||||
if rr.Code == http.StatusNotFound || rr.Code == http.StatusCreated {
|
||||
t.Fatalf("a failed rollback answered %d %s; want a server error, not the rolled-back 404 nor a success", rr.Code, rr.Body.String())
|
||||
}
|
||||
if rr.Code != http.StatusInternalServerError {
|
||||
t.Errorf("status %d, want 500", rr.Code)
|
||||
}
|
||||
}
|
||||
@@ -598,6 +598,37 @@ func (s *Server) handleBootstrap(w http.ResponseWriter, r *http.Request) {
|
||||
}
|
||||
|
||||
// handleRegister creates a new user account.
|
||||
// invitationNotJoined is the additive register response member for a signup
|
||||
// that was kept without its invitation (BUG-3438): open registration, where
|
||||
// the invitation was not the admission and failed at its claim.
|
||||
type invitationNotJoined struct {
|
||||
Code string `json:"code"`
|
||||
Message string `json:"message"`
|
||||
}
|
||||
|
||||
// rollbackSignup removes an account this signup created moments ago and is
|
||||
// now refusing (BUG-3438). It is the full account deletion, not a bare
|
||||
// DELETE FROM users: the account exists from CreateUser on, so a sign-in, a
|
||||
// /forgot-password or the signup's own verification mint can add rows that
|
||||
// reference it before the refusal, and DeleteAccountAtomic removes or
|
||||
// detaches every such row and retries one that lands late.
|
||||
//
|
||||
// Like every account deletion it then kicks the account's connections
|
||||
// (TASK-3365): a sign-in in the same window can have opened a stream, which
|
||||
// would otherwise run until its next revalidation tick (codex r6). The kick
|
||||
// runs on an error too: a lost commit acknowledgement can land the deletion,
|
||||
// and a kick for an account that survived only makes its connections
|
||||
// re-check and keep going (codex r7).
|
||||
func (s *Server) rollbackSignup(userID string) error {
|
||||
del := s.store.DeleteAccountAtomic
|
||||
if s.signupRollbackDelete != nil {
|
||||
del = s.signupRollbackDelete
|
||||
}
|
||||
err := del(userID)
|
||||
s.invalidateUserAccess(userID)
|
||||
return err
|
||||
}
|
||||
|
||||
// Registration is restricted to admins or users with a valid invitation code
|
||||
// so invitees can create an account via the /join/[code] flow.
|
||||
func (s *Server) handleRegister(w http.ResponseWriter, r *http.Request) {
|
||||
@@ -695,9 +726,17 @@ func (s *Server) handleRegister(w http.ResponseWriter, r *http.Request) {
|
||||
// fails SAFE (verified), never write-locked.
|
||||
selfServe := false
|
||||
adminCreated := false
|
||||
reqUser := currentUser(r)
|
||||
isAdmin := reqUser != nil && reqUser.Role == "admin"
|
||||
// inviteIsAdmission: the invitation is what let this signup in, i.e. it
|
||||
// would have been refused without one. Only then does an invitation that
|
||||
// fails at its claim take the account with it; where registration is open
|
||||
// anyway, the account is kept and the invitation reported not applied
|
||||
// (BUG-3438, lead ruling).
|
||||
inviteIsAdmission := invitation != nil &&
|
||||
!(isAdmin && (!isAPITokenAuth(r) || s.adminAcceptsAPITokens())) &&
|
||||
!(s.cloudMode && s.emailConfigured())
|
||||
if invitation == nil {
|
||||
reqUser := currentUser(r)
|
||||
isAdmin := reqUser != nil && reqUser.Role == "admin"
|
||||
switch {
|
||||
case isAdmin && isAPITokenAuth(r) && !s.adminAcceptsAPITokens():
|
||||
// Creating an account is platform administration (BUG-3361):
|
||||
@@ -794,6 +833,12 @@ func (s *Server) handleRegister(w http.ResponseWriter, r *http.Request) {
|
||||
if needsVerification && invitation != nil && input.InvitationProof != "" {
|
||||
ok, perr := s.store.ConsumeInvitationProof(invitation.ID, user.ID, strings.TrimSpace(input.InvitationProof))
|
||||
if perr != nil {
|
||||
// NOT counted as spent, even though a lost commit acknowledgement
|
||||
// could have spent it. Restoring a proof the caller merely SENT
|
||||
// would install an unproven secret on any pending invitation whose
|
||||
// proof was already empty (codex r4). The residual is the safe
|
||||
// direction: a refused signup after such an error leaves the proof
|
||||
// spent, and the retry is verified by the email link instead.
|
||||
slog.Error("invitation proof: consume failed; continuing unverified", "error", perr, "user_id", user.ID)
|
||||
} else if ok {
|
||||
needsVerification = false
|
||||
@@ -824,7 +869,7 @@ func (s *Server) handleRegister(w http.ResponseWriter, r *http.Request) {
|
||||
vtoken, verr := s.store.CreateEmailVerification(user.ID)
|
||||
if verr != nil {
|
||||
slog.Error("failed to create email verification token; rolling back signup", "error", verr, "user_id", user.ID)
|
||||
if derr := s.store.DeleteUser(user.ID); derr != nil {
|
||||
if derr := s.rollbackSignup(user.ID); derr != nil {
|
||||
slog.Error("failed to roll back user after verification-token error", "error", derr, "user_id", user.ID)
|
||||
}
|
||||
writeError(w, http.StatusInternalServerError, "internal_error", "Failed to start email verification")
|
||||
@@ -841,34 +886,66 @@ func (s *Server) handleRegister(w http.ResponseWriter, r *http.Request) {
|
||||
|
||||
// If registering via invitation, automatically add the user to the
|
||||
// workspace and mark the invitation as accepted.
|
||||
// The role the account holds in the invited workspace, as the claim
|
||||
// reports it: an existing membership keeps its own (codex r5).
|
||||
joinedRole := ""
|
||||
// notJoined is set when the signup is kept but its invitation was not
|
||||
// applied: open registration only (inviteIsAdmission false).
|
||||
var notJoined *invitationNotJoined
|
||||
if invitation != nil {
|
||||
// THE MEMBERSHIP WRITE IS FATAL, AND THE ORDER IS THE FIX (BUG-2715).
|
||||
//
|
||||
// (The ACCEPT below is deliberately not fatal; the reason is with it.
|
||||
// An earlier revision of this line said "both writes are fatal", which
|
||||
// its own neighbour contradicted.)
|
||||
//
|
||||
// The membership write's error was discarded and the invitation was
|
||||
// consumed on the next line regardless, so a failure burned the code
|
||||
// AND left the user with no access — unrecoverable without an admin,
|
||||
// because the invitation cannot be redeemed twice.
|
||||
if s.registerInvitationPreClaimHook != nil {
|
||||
s.registerInvitationPreClaimHook(invitation.ID)
|
||||
}
|
||||
// THE MEMBERSHIP WRITE IS FATAL (BUG-2715), AND IT CLAIMS THE
|
||||
// INVITATION IN THE SAME TRANSACTION (BUG-3438).
|
||||
//
|
||||
// Refusing the registration leaves the invitation OPEN, which is the
|
||||
// recoverable state: the user retries with the same code. That is the
|
||||
// whole reason the membership goes first and the accept second.
|
||||
// recoverable state: the user retries with the same code. A failure
|
||||
// that burned the code while leaving the user with no access needed an
|
||||
// admin to undo.
|
||||
//
|
||||
// This is not a new posture — it is making two doors agree.
|
||||
// handleAcceptInvitation (handlers_members.go) is the authenticated
|
||||
// counterpart and refuses the same way: a membership that did not land
|
||||
// leaves the invitation pending. There the two writes commit in one
|
||||
// transaction (BUG-3281); this door was the one that diverged.
|
||||
if addErr := s.store.AddWorkspaceMember(invitation.WorkspaceID, user.ID, invitation.Role, s.workspaceLimitMintOpts()...); addErr != nil {
|
||||
// The invitation was read before the account was created, and it can
|
||||
// be declined, cancelled or replaced since. The claim (accepted_at on a
|
||||
// row still pending) commits with the membership, and a claim that
|
||||
// finds no row rolls the membership back and answers ErrInvitationGone:
|
||||
// the account is removed too, so the retry this refusal permits exists
|
||||
// — when the invitation was the admission. Where registration is open
|
||||
// anyway the account is kept and the invitation reported not applied.
|
||||
// This is the store call the two accept doors make
|
||||
// (acceptInvitationCore), so the three doors agree.
|
||||
_, heldRole, addErr := s.store.AcceptWorkspaceInvitation(invitation.ID, invitation.WorkspaceID, user.ID, invitation.Role, s.workspaceLimitMintOpts()...)
|
||||
if errors.Is(addErr, store.ErrInvitationGone) && !inviteIsAdmission {
|
||||
slog.Info("invitation signup: the invitation was gone at its claim; registration is open, so the "+
|
||||
"account is kept and joins nothing",
|
||||
"workspace_id", invitation.WorkspaceID, "user_id", user.ID, "invitation_id", invitation.ID)
|
||||
notJoined = &invitationNotJoined{Code: "invitation_gone",
|
||||
Message: "This invitation is no longer valid. Your account was created, but you were not added to the workspace."}
|
||||
addErr = nil
|
||||
} else if errors.Is(addErr, store.ErrInvitationGone) {
|
||||
slog.Info("invitation signup: the invitation was gone at its claim; rolling back the account",
|
||||
"workspace_id", invitation.WorkspaceID, "user_id", user.ID, "invitation_id", invitation.ID)
|
||||
if derr := s.rollbackSignup(user.ID); derr != nil {
|
||||
// Not rolled back: the account still holds the email, so the
|
||||
// rolled-back 404 would be false (codex r9).
|
||||
slog.Error("invitation signup: failed to roll back the account; the email is held by an "+
|
||||
"account with no workspace access",
|
||||
"user_id", user.ID, "error", derr)
|
||||
writeInternalError(w, derr)
|
||||
return
|
||||
}
|
||||
writeError(w, http.StatusNotFound, "not_found", "Invitation not found or already accepted")
|
||||
return
|
||||
}
|
||||
if notJoined == nil {
|
||||
joinedRole = heldRole
|
||||
}
|
||||
if addErr != nil {
|
||||
// RECONCILE BEFORE DESTROYING — here too (codex round 2).
|
||||
//
|
||||
// Round 1 added a DeleteUser rollback here and got the ORDER of
|
||||
// reasoning wrong: AddWorkspaceMember returns the raw tx.Commit()
|
||||
// error, so a lost acknowledgement LANDS THE MEMBERSHIP and reports
|
||||
// failure. Deleting the account on that error destroys a signup that
|
||||
// reasoning wrong: the store returns the raw tx.Commit() error, so a
|
||||
// lost acknowledgement LANDS THE MEMBERSHIP (and, since BUG-3438, the
|
||||
// claim in the same transaction) and reports failure. Deleting the account on that error destroys a signup that
|
||||
// actually succeeded — the BUG-3026 shape, reintroduced forty lines
|
||||
// from the helper written to prevent it, by a fix for a different
|
||||
// finding in the same unit.
|
||||
@@ -882,7 +959,16 @@ func (s *Server) handleRegister(w http.ResponseWriter, r *http.Request) {
|
||||
if s.membershipCheck != nil {
|
||||
membershipCheck = s.membershipCheck
|
||||
}
|
||||
member, cerr := membershipCheck(invitation.WorkspaceID, user.ID)
|
||||
// The store returns the held role only with a COMMIT error; a claim
|
||||
// it refused before committing (heldRole empty) is definitely not
|
||||
// landed, so it needs no reconcile read, and a failing read must
|
||||
// not turn it into a 500 (codex r9).
|
||||
var member *models.WorkspaceMember
|
||||
var cerr error
|
||||
refusedBeforeCommit := heldRole == ""
|
||||
if !refusedBeforeCommit {
|
||||
member, cerr = membershipCheck(invitation.WorkspaceID, user.ID)
|
||||
}
|
||||
switch {
|
||||
case cerr != nil:
|
||||
// UNREADABLE — cannot tell an ack-lost success from a genuine
|
||||
@@ -897,13 +983,42 @@ func (s *Server) handleRegister(w http.ResponseWriter, r *http.Request) {
|
||||
writeInternalError(w, addErr)
|
||||
return
|
||||
case member != nil:
|
||||
// LANDED despite the reported error. The user HAS access, so the
|
||||
// signup succeeded and the only thing left is the bookkeeping
|
||||
// below. Falling through is the honest outcome; refusing here
|
||||
// would delete a working account.
|
||||
slog.Warn("invitation signup: the membership write reported an error but the row is present; "+
|
||||
"reconciled to success",
|
||||
// The membership alone does not prove THIS transaction landed:
|
||||
// it also claimed the invitation, and a membership can come from
|
||||
// elsewhere (an admin's add in the window). So the claim decides,
|
||||
// as it does at the accept doors (codex r1).
|
||||
stored, ierr := s.store.GetInvitation(invitation.ID)
|
||||
if ierr == nil && stored != nil && stored.AcceptedAt != nil {
|
||||
joinedRole = member.Role
|
||||
// LANDED despite the reported error: the user HAS access and
|
||||
// the signup succeeded; refusing would delete a working account.
|
||||
slog.Warn("invitation signup: the claim reported an error but the membership and the "+
|
||||
"claim are present; reconciled to success",
|
||||
"workspace_id", invitation.WorkspaceID, "user_id", user.ID, "error", addErr)
|
||||
break
|
||||
}
|
||||
// Not this signup's join. A membership exists, so the account is
|
||||
// KEPT (deleting it cannot be undone, and it has access through
|
||||
// whatever added it), and the signup is refused.
|
||||
slog.Error("invitation signup: the claim failed beside a membership it did not write; "+
|
||||
"KEEPING the account and refusing the signup",
|
||||
"workspace_id", invitation.WorkspaceID, "user_id", user.ID,
|
||||
"error", addErr, "check_error", ierr)
|
||||
if s.writeStoreMemberLimitError(w, r, invitation.WorkspaceID, addErr) {
|
||||
return
|
||||
}
|
||||
writeInternalError(w, addErr)
|
||||
return
|
||||
case !inviteIsAdmission:
|
||||
// ABSENT, on open registration: the account did not need the
|
||||
// invitation, so it is KEPT and joins nothing; the invitation
|
||||
// stays pending, and the account can accept it from its own
|
||||
// list once whatever refused it clears (lead ruling).
|
||||
slog.Warn("invitation signup: the claim was refused; registration is open, so the account is "+
|
||||
"kept and joins nothing",
|
||||
"workspace_id", invitation.WorkspaceID, "user_id", user.ID, "error", addErr)
|
||||
notJoined = &invitationNotJoined{Code: "invitation_not_applied",
|
||||
Message: "Your account was created, but the invitation could not be applied, so you were not added to the workspace."}
|
||||
default:
|
||||
// ABSENT — no membership. Refuse, and roll the account back so
|
||||
// the retry this refusal exists to permit actually exists:
|
||||
@@ -914,10 +1029,12 @@ func (s *Server) handleRegister(w http.ResponseWriter, r *http.Request) {
|
||||
slog.Error("invitation signup: member was not added; rolling back the account and leaving the "+
|
||||
"invitation redeemable",
|
||||
"workspace_id", invitation.WorkspaceID, "user_id", user.ID, "error", addErr)
|
||||
if derr := s.store.DeleteUser(user.ID); derr != nil {
|
||||
if derr := s.rollbackSignup(user.ID); derr != nil {
|
||||
slog.Error("invitation signup: failed to roll back the account; the email is held by an "+
|
||||
"account with no workspace access",
|
||||
"user_id", user.ID, "error", derr)
|
||||
writeInternalError(w, derr)
|
||||
return
|
||||
}
|
||||
// BUG-3098: a member cap reached after the pre-check is a
|
||||
// refusal the invitee can act on, not a server fault.
|
||||
@@ -928,28 +1045,6 @@ func (s *Server) handleRegister(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
}
|
||||
if err := s.store.AcceptInvitation(invitation.ID); err != nil {
|
||||
// The membership LANDED, so the user has access and the only
|
||||
// casualty is the invitation row still reading unaccepted. Refusing
|
||||
// here would report failure for a registration that succeeded, and
|
||||
// a retry would then hit the duplicate-email path. An invitation
|
||||
// that stays open alongside a membership that exists is the benign
|
||||
// direction of this pair — the accept is bookkeeping, the
|
||||
// membership is the access.
|
||||
//
|
||||
// DELIBERATE DIVERGENCE from handleAcceptInvitation, which treats
|
||||
// this same error as fatal. There the caller is an EXISTING,
|
||||
// authenticated user, so a 500 is cleanly retryable. Here the
|
||||
// account was created moments ago by this very request: refusing
|
||||
// now would report failure for a registration that succeeded, and
|
||||
// the retry would land on the duplicate-email path with no way
|
||||
// forward. The ORDER is shared with that door, which is what the
|
||||
// ruling specified; the second write's disposition differs because
|
||||
// the rollback cost does.
|
||||
slog.Error("invitation signup: member added but the invitation was not marked accepted",
|
||||
"workspace_id", invitation.WorkspaceID, "user_id", user.ID,
|
||||
"invitation_id", invitation.ID, "error", err)
|
||||
}
|
||||
}
|
||||
|
||||
// An admin creating someone else's account is not signing in as them, so
|
||||
@@ -984,8 +1079,13 @@ func (s *Server) handleRegister(w http.ResponseWriter, r *http.Request) {
|
||||
// A signup with an invitation code has already joined the workspace, so
|
||||
// the client can land IN it, as an accept does (BUG-3284, PLAN-3002 Q5).
|
||||
// Additive; absent without an invitation.
|
||||
if invitation != nil {
|
||||
resp["accepted_invitation"] = s.invitationAcceptedFields(invitation, invitation.Role)
|
||||
if notJoined != nil {
|
||||
resp["invitation_not_joined"] = notJoined
|
||||
} else if invitation != nil {
|
||||
if joinedRole == "" {
|
||||
joinedRole = invitation.Role
|
||||
}
|
||||
resp["accepted_invitation"] = s.invitationAcceptedFields(invitation, joinedRole)
|
||||
}
|
||||
writeJSON(w, http.StatusCreated, resp)
|
||||
}
|
||||
|
||||
@@ -458,6 +458,18 @@ type Server struct {
|
||||
// read-then-insert would race (BUG-2792).
|
||||
autoAddPreInsertHook func(requestID, workspaceID string)
|
||||
|
||||
// registerInvitationPreClaimHook, when non-nil, runs in handleRegister
|
||||
// after the invitation was read and validated and before the signup
|
||||
// claims it (BUG-3438). Test seam only (rowlessPreDeleteHook precedent):
|
||||
// it lets a test decline, cancel or replace the invitation inside that
|
||||
// window.
|
||||
registerInvitationPreClaimHook func(invitationID string)
|
||||
|
||||
// signupRollbackDelete, when non-nil, replaces DeleteAccountAtomic in
|
||||
// rollbackSignup (BUG-3438). Test seam only: it lets a test make the
|
||||
// rollback fail.
|
||||
signupRollbackDelete func(userID string) error
|
||||
|
||||
// bg tracks fire-and-forget goroutines spawned by request handlers
|
||||
// (TouchUserActivity in middleware_auth, async email sends, etc.) so
|
||||
// the server can drain them before shutdown / test cleanup. Without
|
||||
|
||||
@@ -2714,7 +2714,7 @@ export const api = {
|
||||
invitation_code?: string,
|
||||
invitation_proof?: string
|
||||
) =>
|
||||
request<{ user: { id: string; email: string; username: string; name: string; role: string; email_verified?: boolean }; token: string; accepted_invitation?: InvitationAcceptedFields }>('/auth/register', {
|
||||
request<{ user: { id: string; email: string; username: string; name: string; role: string; email_verified?: boolean }; token: string; accepted_invitation?: InvitationAcceptedFields; invitation_not_joined?: { code: string; message: string } }>('/auth/register', {
|
||||
method: 'POST',
|
||||
body: JSON.stringify({
|
||||
email,
|
||||
|
||||
@@ -28,7 +28,7 @@
|
||||
// same-origin relative path — no open redirect.
|
||||
let oauthRedirectTarget = $derived(validateRedirect(`/join/${code}`));
|
||||
let status = $state<
|
||||
'loading' | 'login' | 'register' | 'confirm' | 'accepting' | 'declining' | 'declined' | 'error' | 'setup' | '2fa'
|
||||
'loading' | 'login' | 'register' | 'confirm' | 'accepting' | 'declining' | 'declined' | 'not-joined' | 'error' | 'setup' | '2fa'
|
||||
>('loading');
|
||||
// The invited workspace's name, from the preview, for the accept/decline
|
||||
// card (BUG-2136 U2).
|
||||
@@ -357,6 +357,17 @@
|
||||
if (restartIfMovedOn(current)) return;
|
||||
// Registration with invitation_code already accepted the invite,
|
||||
// so land directly instead of calling acceptInvitation().
|
||||
// Kept without its invitation (BUG-3438): registration was open
|
||||
// anyway, so the account exists, but the invitation was gone or
|
||||
// could not be applied. Say so, and land nowhere.
|
||||
if (registered.invitation_not_joined) {
|
||||
await authStore.load().catch(() => {});
|
||||
if (!current()) return;
|
||||
submitting = false;
|
||||
errorMsg = registered.invitation_not_joined.message;
|
||||
status = 'not-joined';
|
||||
return;
|
||||
}
|
||||
await landInJoinedWorkspace(registered.accepted_invitation, current);
|
||||
return;
|
||||
} else {
|
||||
@@ -476,6 +487,9 @@
|
||||
actionHref="/login"
|
||||
actionLabel="Go to login"
|
||||
/>
|
||||
{:else if status === 'not-joined'}
|
||||
<p class="subtitle error-text" role="alert">{errorMsg}</p>
|
||||
<a href="/console" class="link">Go to Pad</a>
|
||||
{:else if status === 'error'}
|
||||
<p class="subtitle error-text" role="alert">{errorMsg}</p>
|
||||
<a href="/login" class="link">Go to login</a>
|
||||
|
||||
@@ -181,6 +181,31 @@ describe('/join/[code] asks before joining (BUG-2136)', () => {
|
||||
expect(mocks.accept).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('a registration kept without its invitation says so and lands nowhere (BUG-3438)', async () => {
|
||||
mocks.session.mockResolvedValue({ authenticated: false });
|
||||
mocks.preview.mockResolvedValue({ found: true, email: 'new@example.com', has_account: false, workspace_name: 'Acme' });
|
||||
mocks.register.mockResolvedValue({
|
||||
user: { id: 'u', email: 'new@example.com' },
|
||||
token: 't',
|
||||
invitation_not_joined: {
|
||||
code: 'invitation_gone',
|
||||
message: 'This invitation is no longer valid. Your account was created, but you were not added to the workspace.',
|
||||
},
|
||||
});
|
||||
render(JoinPage);
|
||||
await settle();
|
||||
await fireEvent.input(document.querySelector<HTMLInputElement>('input[placeholder="Name"]')!, { target: { value: 'New' } });
|
||||
const pws = document.querySelectorAll<HTMLInputElement>('input[type="password"]');
|
||||
await fireEvent.input(pws[0], { target: { value: 'password123' } });
|
||||
await fireEvent.input(pws[1], { target: { value: 'password123' } });
|
||||
await fireEvent.click(submitBtn());
|
||||
await settle();
|
||||
expect(mocks.goto).not.toHaveBeenCalled();
|
||||
expect(document.querySelector('[role="alert"]')?.textContent).toContain('This invitation is no longer valid');
|
||||
const toPad = Array.from(document.querySelectorAll<HTMLAnchorElement>('a')).find((a) => a.getAttribute('href') === '/console');
|
||||
expect(toPad).toBeTruthy();
|
||||
});
|
||||
|
||||
it('registering a new account through the code still joins in one step', async () => {
|
||||
mocks.session.mockResolvedValue({ authenticated: false });
|
||||
mocks.preview.mockResolvedValue({ found: true, email: 'new@example.com', has_account: false, workspace_name: 'Acme' });
|
||||
|
||||
Reference in New Issue
Block a user