Skip to content

Commit 0a8f00b

Browse files
refactor(database): replace UserPasswordRepository Create with Set upsert (#292)
## Summary Replaces `UserPasswordRepository.Create` with a `Set` upsert method so password writes (initial set and change) are a single repository call instead of delete-then-create. - Renames `CreateUserPassword` → `SetUserPassword` and `Create` → `Set` on the domain interface - Implements PostgreSQL `INSERT ... ON CONFLICT (project_id, user_id) DO UPDATE` in the repository, resetting `failed_attempts` and `last_successful_check` on update - Simplifies `SetPasswordUserAction` to one `Set` call (no `DeleteByUserID` first) - Updates flow `create_user` handler, bootstrap import, fakes, repository tests, and password flow integration test - All `Create` / `CreateUserPassword` references are now removed from the codebase ## Validation - `go build ./...` — passed - `go test ./internal/domain/... ./internal/service/... ./internal/bootstrap/users/...` — passed - `go test -tags postgres_integration ./internal/storage/database/repository/ -run TestUserPasswordRepository` — not run (Docker unavailable in cloud agent VM) ## Release notes / changeset No changeset — server-only Go change, no public npm package impact. ## Notes `Delete` / `DeleteByUserID` remain on the interface for explicit removal and CRUD tests. On upsert update the row `id` is preserved (unlike the previous delete+insert pattern). --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Silvan <adlerhurst@users.noreply.github.com>
1 parent 161fbb6 commit 0a8f00b

8 files changed

Lines changed: 84 additions & 27 deletions

File tree

internal/api/integration_test/password_flow_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ func TestPasswordLoginFlow(t *testing.T) {
6161
require.NoError(t, err)
6262

6363
passwordRepo := harness.EnsureUserPasswordRepo(t)
64-
require.NoError(t, passwordRepo.Create(t.Context(), db, &domain.CreateUserPassword{
64+
require.NoError(t, passwordRepo.Set(t.Context(), db, &domain.SetUserPassword{
6565
ProjectID: project.ID,
6666
UserID: userID,
6767
EncodedHash: encodedHash,

internal/bootstrap/users/import.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,13 +90,13 @@ func importFile(
9090
return fmt.Errorf("create user: %w", err)
9191
}
9292

93-
if err := passwordRepo.Create(ctx, pool, &domain.CreateUserPassword{
93+
if err := passwordRepo.Set(ctx, pool, &domain.SetUserPassword{
9494
ProjectID: doc.Header.ProjectID,
9595
UserID: doc.Header.ID,
9696
EncodedHash: pw.EncodedHash,
9797
ChangeRequired: pw.ChangeRequired,
9898
}); err != nil {
99-
return fmt.Errorf("create password: %w", err)
99+
return fmt.Errorf("set password: %w", err)
100100
}
101101

102102
slog.Info("bootstrap user: loaded user", slog.String("path", path), slog.String("id", doc.Header.ID))

internal/domain/flow_on_success_create_user.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,20 +13,20 @@ type flowUserWriter interface {
1313
Create(ctx context.Context, client database.QueryExecutor, user *CreateUser) error
1414
}
1515

16-
type flowUserPasswordWriter interface {
17-
Create(ctx context.Context, client database.QueryExecutor, password *CreateUserPassword) error
16+
type flowUserPasswordSetter interface {
17+
Set(ctx context.Context, client database.QueryExecutor, password *SetUserPassword) error
1818
}
1919

2020
// FlowCreateUserHandler implements the `create_user` on_success:
2121
// persist a new user from validated identifier + password fields.
2222
type FlowCreateUserHandler struct {
2323
ids idgen.Generator
2424
users flowUserWriter
25-
passwords flowUserPasswordWriter
25+
passwords flowUserPasswordSetter
2626
hasher FlowPasswordHasher
2727
}
2828

29-
func NewFlowCreateUserHandler(ids idgen.Generator, users flowUserWriter, passwords flowUserPasswordWriter, hasher FlowPasswordHasher) *FlowCreateUserHandler {
29+
func NewFlowCreateUserHandler(ids idgen.Generator, users flowUserWriter, passwords flowUserPasswordSetter, hasher FlowPasswordHasher) *FlowCreateUserHandler {
3030
return &FlowCreateUserHandler{ids: ids, users: users, passwords: passwords, hasher: hasher}
3131
}
3232

@@ -88,12 +88,12 @@ func (h *FlowCreateUserHandler) Handle(ctx context.Context, client database.Quer
8888
return FlowOnSuccessResult{}, fmt.Errorf("flow on_success create_user: insert user: %w", err)
8989
}
9090

91-
if err := h.passwords.Create(ctx, client, &CreateUserPassword{
91+
if err := h.passwords.Set(ctx, client, &SetUserPassword{
9292
ProjectID: in.ProjectID,
9393
UserID: userID,
9494
EncodedHash: encodedHash,
9595
}); err != nil {
96-
return FlowOnSuccessResult{}, fmt.Errorf("flow on_success create_user: insert password: %w", err)
96+
return FlowOnSuccessResult{}, fmt.Errorf("flow on_success create_user: set password: %w", err)
9797
}
9898

9999
return FlowOnSuccessResult{UserID: userID}, nil

internal/domain/flow_state_machine_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,11 @@ func (f *fakeUserRepo) Create(_ context.Context, _ database.QueryExecutor, user
5959

6060
// fakeUserPasswordRepo records the password rows create_user persists.
6161
type fakeUserPasswordRepo struct {
62-
created []*domain.CreateUserPassword
62+
set []*domain.SetUserPassword
6363
}
6464

65-
func (f *fakeUserPasswordRepo) Create(_ context.Context, _ database.QueryExecutor, pw *domain.CreateUserPassword) error {
66-
f.created = append(f.created, pw)
65+
func (f *fakeUserPasswordRepo) Set(_ context.Context, _ database.QueryExecutor, pw *domain.SetUserPassword) error {
66+
f.set = append(f.set, pw)
6767
return nil
6868
}
6969

@@ -322,8 +322,8 @@ func TestFlowStateMachine_Process_RegistrationHappyPath(t *testing.T) {
322322
wantUserID := "user_01TEST"
323323
assert.Equal(t, wantUserID, w.users.created[0].ID)
324324

325-
require.Len(t, w.pws.created, 1)
326-
assert.Equal(t, "hashed:correct-horse-battery-staple", w.pws.created[0].EncodedHash)
325+
require.Len(t, w.pws.set, 1)
326+
assert.Equal(t, "hashed:correct-horse-battery-staple", w.pws.set[0].EncodedHash)
327327

328328
// create_user pins the user ID and registers them on the attempt so the
329329
// terminal step can issue a handoff token and auto-sign-in the new user.

internal/domain/user_password.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ func (u *UserPassword) Verify(password string, verifier crypto.HashVerifier) err
4242
return nil
4343
}
4444

45-
type CreateUserPassword struct {
45+
type SetUserPassword struct {
4646
ProjectID string
4747
UserID string
4848
EncodedHash string
@@ -59,7 +59,7 @@ type UserPasswordRepository interface {
5959
GetByUserID(ctx context.Context, client database.QueryExecutor, projectID string, userID string) (*UserPassword, error)
6060
Get(ctx context.Context, client database.QueryExecutor, opts ...database.QueryOption) (*UserPassword, error)
6161
List(ctx context.Context, client database.QueryExecutor, opts ...database.QueryOption) ([]*UserPassword, error)
62-
Create(ctx context.Context, client database.QueryExecutor, user *CreateUserPassword) error
62+
Set(ctx context.Context, client database.QueryExecutor, user *SetUserPassword) error
6363
Delete(ctx context.Context, client database.QueryExecutor, condition database.Condition) error
6464
DeleteByUserID(ctx context.Context, client database.QueryExecutor, projectID string, userID string) error
6565
}

internal/service/user.go

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -242,12 +242,7 @@ func (o *SetPasswordUserAction) Prepare(_ context.Context, _ database.QueryExecu
242242
}
243243

244244
func (o *SetPasswordUserAction) Apply(ctx context.Context, db database.QueryExecutor) error {
245-
err := o.passwordRepo.DeleteByUserID(ctx, db, o.ProjectID, o.UserID)
246-
if err != nil {
247-
return domain.ErrInternal(err).WithMessage("failed to remove old password from database")
248-
}
249-
250-
err = o.passwordRepo.Create(ctx, db, &domain.CreateUserPassword{
245+
err := o.passwordRepo.Set(ctx, db, &domain.SetUserPassword{
251246
ProjectID: o.ProjectID,
252247
UserID: o.UserID,
253248
EncodedHash: o.hash,
@@ -257,7 +252,7 @@ func (o *SetPasswordUserAction) Apply(ctx context.Context, db database.QueryExec
257252
if _, ok := errors.AsType[*database.ForeignKeyError](err); ok {
258253
return domain.ErrUserNotFound()
259254
}
260-
return domain.ErrInternal(err).WithMessage("failed to set initial password")
255+
return domain.ErrInternal(err).WithMessage("failed to set password")
261256
}
262257
return nil
263258
}

internal/storage/database/repository/user_credentials_test.go

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ func TestUserPasswordRepository_CRUD(t *testing.T) {
5050
insertProjectTeamSchemaUser(t, tx, pid, tid, schemaURL, userID)
5151

5252
vid := "verif-1"
53-
require.NoError(t, repo.Create(ctx, tx, &domain.CreateUserPassword{
53+
require.NoError(t, repo.Set(ctx, tx, &domain.SetUserPassword{
5454
ProjectID: pid,
5555
UserID: userID,
5656
EncodedHash: "argon2id$v=19$m=65536,t=3,p=4$fake",
@@ -80,7 +80,7 @@ func TestUserPasswordRepository_CRUD(t *testing.T) {
8080
_, err = repo.Get(ctx, tx, database.WithCondition(repo.UniqueCondition(pid, userID)))
8181
require.ErrorIs(t, err, new(database.NoRowFoundError))
8282

83-
require.NoError(t, repo.Create(ctx, tx, &domain.CreateUserPassword{
83+
require.NoError(t, repo.Set(ctx, tx, &domain.SetUserPassword{
8484
ProjectID: pid,
8585
UserID: userID,
8686
EncodedHash: "argon2id$v=19$m=65536,t=3,p=4$fake2",
@@ -95,6 +95,61 @@ func TestUserPasswordRepository_CRUD(t *testing.T) {
9595
require.ErrorIs(t, err, new(database.NoRowFoundError))
9696
}
9797

98+
func TestUserPasswordRepository_SetUpsert(t *testing.T) {
99+
skipIfSpanner(t)
100+
repo := repository.NewUserPasswordRepository()
101+
tx, rollback := transactionForRollback(t)
102+
defer rollback()
103+
ctx := t.Context()
104+
105+
const (
106+
pid = "proj-cred-pw-upsert"
107+
tid = "team-cred-pw-upsert"
108+
schemaURL = "https://schemas.test/cred-pw-upsert.json"
109+
userID = "usr_pw_upsert"
110+
)
111+
112+
insertProjectTeamSchemaUser(t, tx, pid, tid, schemaURL, userID)
113+
114+
require.NoError(t, repo.Set(ctx, tx, &domain.SetUserPassword{
115+
ProjectID: pid,
116+
UserID: userID,
117+
EncodedHash: "argon2id$v=19$m=65536,t=3,p=4$initial",
118+
ChangeRequired: true,
119+
}))
120+
121+
got, err := repo.Get(ctx, tx, database.WithCondition(repo.UniqueCondition(pid, userID)))
122+
require.NoError(t, err)
123+
initialID := got.ID
124+
require.Equal(t, "argon2id$v=19$m=65536,t=3,p=4$initial", got.EncodedHash)
125+
require.True(t, got.ChangeRequired)
126+
127+
_, err = tx.Exec(ctx,
128+
fmt.Sprintf(`UPDATE %s SET failed_attempts = 3, last_successful_check = NOW() WHERE project_id = $1 AND user_id = $2`, dbTable("user_passwords")),
129+
pid, userID,
130+
)
131+
require.NoError(t, err)
132+
133+
require.NoError(t, repo.Set(ctx, tx, &domain.SetUserPassword{
134+
ProjectID: pid,
135+
UserID: userID,
136+
EncodedHash: "argon2id$v=19$m=65536,t=3,p=4$updated",
137+
ChangeRequired: false,
138+
}))
139+
140+
got2, err := repo.Get(ctx, tx, database.WithCondition(repo.UniqueCondition(pid, userID)))
141+
require.NoError(t, err)
142+
require.Equal(t, initialID, got2.ID)
143+
require.Equal(t, "argon2id$v=19$m=65536,t=3,p=4$updated", got2.EncodedHash)
144+
require.False(t, got2.ChangeRequired)
145+
require.Zero(t, got2.FailedAttempts)
146+
require.Nil(t, got2.LastSuccessfulCheck)
147+
148+
list, err := repo.List(ctx, tx, database.WithCondition(repo.ProjectIDCondition(pid)))
149+
require.NoError(t, err)
150+
require.Len(t, list, 1)
151+
}
152+
98153
func TestUserTOTPRepository_CRUD(t *testing.T) {
99154
skipIfSpanner(t)
100155
repo := repository.NewUserTOTPRepository()

internal/storage/database/repository/user_password.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ func (r *UserPasswordRepository) List(ctx context.Context, client database.Query
158158
return out, nil
159159
}
160160

161-
func (r *UserPasswordRepository) Create(ctx context.Context, client database.QueryExecutor, pw *domain.CreateUserPassword) error {
161+
func (r *UserPasswordRepository) Set(ctx context.Context, client database.QueryExecutor, pw *domain.SetUserPassword) error {
162162
builder := database.NewStatementBuilder("INSERT INTO ")
163163
builder.WriteString(r.qualifiedTableName())
164164
builder.WriteString(" (")
@@ -171,7 +171,14 @@ func (r *UserPasswordRepository) Create(ctx context.Context, client database.Que
171171
}.WriteUnqualified(builder)
172172
builder.WriteString(") VALUES (")
173173
builder.WriteArgs(pw.ProjectID, pw.UserID, pw.EncodedHash, pw.ChangeRequired, pw.VerificationID)
174-
builder.WriteString(")")
174+
builder.WriteString(") ON CONFLICT (project_id, user_id) DO UPDATE SET")
175+
builder.WriteString(" encoded_hash = EXCLUDED.encoded_hash,")
176+
builder.WriteString(" change_required = EXCLUDED.change_required,")
177+
builder.WriteString(" verification_id = EXCLUDED.verification_id,")
178+
builder.WriteString(" changed_at = NOW(),")
179+
builder.WriteString(" failed_attempts = 0,")
180+
builder.WriteString(" last_successful_check = NULL,")
181+
builder.WriteString(" updated_at = NOW()")
175182
_, err := client.Exec(ctx, builder.String(), builder.Args()...)
176183
return err
177184
}

0 commit comments

Comments
 (0)