diff --git a/Server/api/auth_characterization_test.go b/Server/api/auth_characterization_test.go index 859b6a62..cffb1259 100644 --- a/Server/api/auth_characterization_test.go +++ b/Server/api/auth_characterization_test.go @@ -449,23 +449,22 @@ func TestAuthCharacterization_RegisterPolicyAndFailurePaths(t *testing.T) { t.Error("user row exists after a failed insert") } }) - t.Run("session insert fails after the user is committed -> 500", func(t *testing.T) { + t.Run("session insert fails -> 500, nothing committed", func(t *testing.T) { database := newAuthTestDB(t) router := buildAuthRouter(database, auth.NewRateLimiter()) code := seedInvite(t, database) failWrite(t, database, "INSERT", "sessions") rr := send(t, router, http.MethodPost, "/api/v1/auth/register", "", "", "", body(code)) - wantErr(t, rr, http.StatusInternalServerError, "INTERNAL_ERROR", "failed to create session") - // known: the account and the invite use are already committed when the - // session insert fails, so the caller sees a 500 for a registration - // that succeeded — a retry gets "invalid invite or credentials" while - // a login with the same password works (ledger OC-0376). - if userByName(t, database, "fresh") == nil { - t.Error("user row missing: this row pins the partial-success behaviour, which has changed") + // OC-0376 (fixed in B3-9): the first session is inserted inside the + // registration transaction, so a store fault leaves no half-registered + // account and does not burn the invite — the caller simply retries. + if userByName(t, database, "fresh") != nil { + t.Error("user row exists after the session insert failed") } - if n := inviteUseCount(t, database, code); n != 1 { - t.Errorf("invite use_count = %d, want 1 (pinned partial success)", n) + if n := inviteUseCount(t, database, code); n != 0 { + t.Errorf("invite use_count = %d, want 0 (transaction rolled back)", n) } + wantErr(t, rr, http.StatusInternalServerError, "INTERNAL_ERROR", "registration failed — please try again") }) } diff --git a/Server/db/auth_queries.go b/Server/db/auth_queries.go index a06b9325..94fab803 100644 --- a/Server/db/auth_queries.go +++ b/Server/db/auth_queries.go @@ -71,9 +71,13 @@ func (d *DB) CreateOwnerIfEmpty(ctx context.Context, username, passwordHash stri return uid, nil } -// CreateUserWithInvite atomically consumes an invite and creates the user in -// the same transaction so a failed registration does not burn the invite. -func (d *DB) CreateUserWithInvite(ctx context.Context, username, passwordHash string, roleID int, inviteCode string) (int64, error) { +// CreateUserWithInvite atomically consumes an invite, creates the user and +// inserts the account's first session in one transaction, so a failure at any +// step — the session insert included (OC-0376) — leaves no half-registered +// account and does not burn the invite. sessionTokenHash must already be +// hashed. The H-6 session cap needs no eviction here: the user has no sessions +// yet. +func (d *DB) CreateUserWithInvite(ctx context.Context, username, passwordHash string, roleID int, inviteCode, sessionTokenHash, device, ip string) (int64, error) { tx, err := d.writer.BeginTx(ctx, nil) if err != nil { return 0, fmt.Errorf("CreateUserWithInvite begin: %w", err) @@ -114,6 +118,9 @@ func (d *DB) CreateUserWithInvite(ctx context.Context, username, passwordHash st if err != nil { return 0, fmt.Errorf("CreateUserWithInvite last insert id: %w", err) } + if _, err := insertSession(ctx, d.q.WithTx(tx), uid, sessionTokenHash, device, ip); err != nil { + return 0, fmt.Errorf("CreateUserWithInvite create session: %w", err) + } if err := tx.Commit(); err != nil { return 0, fmt.Errorf("CreateUserWithInvite commit: %w", err) } @@ -254,9 +261,15 @@ func (d *DB) CreateSession(ctx context.Context, userID int64, tokenHash, device, "user_id", userID, "err", err) } + return insertSession(ctx, d.q, userID, tokenHash, device, ip) +} + +// insertSession inserts one session row through q — d.q, or d.q.WithTx(tx) +// when the row must commit with other writes (CreateUserWithInvite). +func insertSession(ctx context.Context, q *dbgen.Queries, userID int64, tokenHash, device, ip string) (int64, error) { expiresAt := time.Now().Add(sessionTTL).UTC().Format(sessionTimeLayout) deviceCopy, ipCopy := device, ip - res, err := d.q.InsertSession(ctx, dbgen.InsertSessionParams{ + res, err := q.InsertSession(ctx, dbgen.InsertSessionParams{ UserID: userID, Token: tokenHash, Device: &deviceCopy, diff --git a/Server/db/coverage_boost_test.go b/Server/db/coverage_boost_test.go index 34ef4b17..6c1733ac 100644 --- a/Server/db/coverage_boost_test.go +++ b/Server/db/coverage_boost_test.go @@ -756,7 +756,7 @@ func TestCreateUserWithInvite_Success(t *testing.T) { t.Fatalf("CreateInvite: %v", err) } - uid, err := database.CreateUserWithInvite(context.Background(), "newuser", "hash", 4, code) + uid, err := database.CreateUserWithInvite(context.Background(), "newuser", "hash", 4, code, "sess-newuser", "test", "127.0.0.1") if err != nil { t.Fatalf("CreateUserWithInvite: %v", err) } @@ -769,12 +769,16 @@ func TestCreateUserWithInvite_Success(t *testing.T) { if inv == nil || inv.Uses != 1 { t.Errorf("invite uses = %v, want 1", inv) } + // The first session commits with the account (OC-0376). + if sess, err := database.GetSessionByTokenHash(context.Background(), "sess-newuser"); err != nil || sess == nil || sess.UserID != uid { + t.Errorf("session = %+v, %v; want a session for user %d", sess, err, uid) + } } func TestCreateUserWithInvite_InvalidCode(t *testing.T) { database := openMigratedMemory(t) - _, err := database.CreateUserWithInvite(context.Background(), "baduser", "hash", 4, "nonexistent-code") + _, err := database.CreateUserWithInvite(context.Background(), "baduser", "hash", 4, "nonexistent-code", "sess-bad", "test", "127.0.0.1") if err == nil { t.Error("expected error for invalid invite code") } @@ -787,7 +791,7 @@ func TestCreateUserWithInvite_RevokedInvite(t *testing.T) { code, _ := database.CreateInvite(context.Background(), creatorID, 0, nil) _ = database.RevokeInvite(context.Background(), code) - _, err := database.CreateUserWithInvite(context.Background(), "revokeduser", "hash", 4, code) + _, err := database.CreateUserWithInvite(context.Background(), "revokeduser", "hash", 4, code, "sess-revoked", "test", "127.0.0.1") if err == nil { t.Error("expected error for revoked invite") } @@ -801,7 +805,7 @@ func TestCreateUserWithInvite_ExpiredInvite(t *testing.T) { pastTime := time.Now().Add(-1 * time.Hour) code, _ := database.CreateInvite(context.Background(), creatorID, 0, &pastTime) - _, err := database.CreateUserWithInvite(context.Background(), "expireduser", "hash", 4, code) + _, err := database.CreateUserWithInvite(context.Background(), "expireduser", "hash", 4, code, "sess-expired", "test", "127.0.0.1") if err == nil { t.Error("expected error for expired invite") } diff --git a/Server/service/auth.go b/Server/service/auth.go index c64a275b..6ab5f41b 100644 --- a/Server/service/auth.go +++ b/Server/service/auth.go @@ -296,9 +296,16 @@ func (s *AuthService) Register(ctx context.Context, in RegisterInput) (*AuthResu return nil, ErrPasswordHash } - // Atomically consume the invite and create the user so failed - // registrations do not burn a valid invite code. - uid, err := s.st.CreateUserWithInvite(ctx, in.Username, hash, int(permissions.MemberRoleID), in.InviteCode) + // The session token exists before the transaction so the account, the + // invite use and the first session commit together: a fault at any step + // — the session insert included — rolls the whole registration back and + // burns nothing (OC-0376). + token, err := auth.GenerateToken() + if err != nil { + return nil, ErrSessionIssue + } + uid, err := s.st.CreateUserWithInvite(ctx, in.Username, hash, int(permissions.MemberRoleID), in.InviteCode, + auth.HashToken(token), in.Device, in.IP) if err != nil { // UNIQUE constraint violation → duplicate username → 400. // Any other DB error → 500. @@ -317,12 +324,6 @@ func (s *AuthService) Register(ctx context.Context, in RegisterInput) (*AuthResu db.WriteAudit(context.WithoutCancel(ctx), s.st, uid, "user_register", "user", uid, "new account created via invite") - // Issue session. - token, err := issueSession(ctx, s.st, uid, in.Device, in.IP) - if err != nil { - return nil, ErrSessionIssue - } - user, err := s.st.GetUserByID(ctx, uid) if err != nil || user == nil { slog.Error("failed to fetch user after registration", "user_id", uid, "error", err) diff --git a/Server/service/datastore.go b/Server/service/datastore.go index 5cb1ab52..e06057ed 100644 --- a/Server/service/datastore.go +++ b/Server/service/datastore.go @@ -81,7 +81,7 @@ type Store interface { GetUserByUsername(ctx context.Context, username string) (*db.User, error) CreateUser(ctx context.Context, username, passwordHash string, roleID int) (int64, error) CreateOwnerIfEmpty(ctx context.Context, username, passwordHash string, roleID int) (int64, error) - CreateUserWithInvite(ctx context.Context, username, passwordHash string, roleID int, inviteCode string) (int64, error) + CreateUserWithInvite(ctx context.Context, username, passwordHash string, roleID int, inviteCode, sessionTokenHash, device, ip string) (int64, error) UpdateUserProfile(ctx context.Context, userID int64, username string, avatar, displayName, about *string) error UpdateUserCustomStatus(ctx context.Context, userID int64, customStatus *string) error UpdateUserPassword(ctx context.Context, userID int64, newPasswordHash string) error