diff --git a/internal/auth/password/credential.go b/internal/auth/password/credential.go new file mode 100644 index 000000000..88b0c41ba --- /dev/null +++ b/internal/auth/password/credential.go @@ -0,0 +1,52 @@ +package password + +import ( + "github.com/hashicorp/boundary/internal/auth/password/store" + "github.com/hashicorp/boundary/internal/oplog" + "google.golang.org/protobuf/proto" +) + +// A Credential is a base type and contains the attributes common to all +// credentials. +type Credential struct { + *store.Credential + tableName string `gorm:"-"` +} + +func allocCredential() Credential { + return Credential{ + Credential: &store.Credential{}, + } +} + +func (c *Credential) clone() *Credential { + cp := proto.Clone(c.Credential) + return &Credential{ + Credential: cp.(*store.Credential), + } +} + +// TableName returns the table name. +func (c *Credential) TableName() string { + if c.tableName != "" { + return c.tableName + } + return "auth_password_credential" +} + +// SetTableName sets the table name. +func (c *Credential) SetTableName(n string) { + c.tableName = n +} + +func (c *Credential) oplog(op oplog.OpType) oplog.Metadata { + metadata := oplog.Metadata{ + "resource-public-id": []string{c.PrivateId}, + "resource-type": []string{"password-credential"}, + "op-type": []string{op.String()}, + } + if c.PasswordAccountId != "" { + metadata["account-id"] = []string{c.PasswordAccountId} + } + return metadata +} diff --git a/internal/auth/password/query.go b/internal/auth/password/query.go index f1fa90e85..b99788362 100644 --- a/internal/auth/password/query.go +++ b/internal/auth/password/query.go @@ -29,5 +29,15 @@ select acct.name, -- Account.Name and cred.password_conf_id = conf.private_id and cred.password_account_id = acct.public_id and acct.auth_method_id = meth.public_id ; +` + currentConfigForAccountQuery = ` +select * + from auth_password_current_conf + where password_method_id + in ( + select auth_method_id + from auth_password_account + where public_id = $1 + ); ` ) diff --git a/internal/auth/password/repository_account.go b/internal/auth/password/repository_account.go index ca0e5c356..f040de25f 100644 --- a/internal/auth/password/repository_account.go +++ b/internal/auth/password/repository_account.go @@ -5,16 +5,21 @@ import ( "errors" "fmt" "regexp" + "strings" "github.com/hashicorp/boundary/internal/db" + dbcommon "github.com/hashicorp/boundary/internal/db/common" "github.com/hashicorp/boundary/internal/oplog" ) -// CreateAccount inserts a into the repository and returns a new -// Account containing the account's PublicId. a is not changed. a must -// contain a valid AuthMethodId. a must not contain a PublicId. The PublicId is +// CreateAccount inserts a into the repository and returns a new Account +// containing the account's PublicId. a is not changed. a must contain a +// valid AuthMethodId. a must not contain a PublicId. The PublicId is // generated and assigned by this method. // +// a must contain a valid UserName. a.UserName must be unique within +// a.AuthMethodId. +// // WithPassword is the only valid option. All other options are ignored. // // Both a.Name and a.Description are optional. If a.Name is set, it must be @@ -172,3 +177,93 @@ func validUserName(u string) bool { } return !reInvalidUserName.Match([]byte(u)) } + +// UpdateAccount updates the repository entry for a.PublicId with the +// values in a for the fields listed in fieldMaskPaths. It returns a new +// Account containing the updated values and a count of the number of +// records updated. a is not changed. +// +// a must contain a valid PublicId. Only a.Name, a.Description and +// a.UserName can be updated. If a.Name is set to a non-empty string, it +// must be unique within a.AuthMethodId. If a.UserName is set to a +// non-empty string, it must be unique within a.AuthMethodId. +// +// An attribute of a will be set to NULL in the database if the attribute +// in a is the zero value and it is included in fieldMaskPaths. a.UserName +// cannot be set to NULL. +func (r *Repository) UpdateAccount(ctx context.Context, a *Account, fieldMaskPaths []string, opt ...Option) (*Account, int, error) { + if a == nil { + return nil, db.NoRowsAffected, fmt.Errorf("update: password account: %w", db.ErrNilParameter) + } + if a.Account == nil { + return nil, db.NoRowsAffected, fmt.Errorf("update: password account: embedded Account: %w", db.ErrNilParameter) + } + if a.PublicId == "" { + return nil, db.NoRowsAffected, fmt.Errorf("update: password account: missing public id: %w", db.ErrInvalidParameter) + } + + var changeUserName bool + for _, f := range fieldMaskPaths { + switch { + case strings.EqualFold("Name", f): + case strings.EqualFold("Description", f): + case strings.EqualFold("UserName", f): + if !validUserName(a.UserName) { + return nil, db.NoRowsAffected, fmt.Errorf("update: password account: invalid user name: %w", db.ErrInvalidParameter) + } + changeUserName = true + default: + return nil, db.NoRowsAffected, fmt.Errorf("update: password account: field: %s: %w", f, db.ErrInvalidFieldMask) + } + } + var dbMask, nullFields []string + dbMask, nullFields = dbcommon.BuildUpdatePaths( + map[string]interface{}{ + "Name": a.Name, + "Description": a.Description, + "UserName": a.UserName, + }, + fieldMaskPaths, + ) + if len(dbMask) == 0 && len(nullFields) == 0 { + return nil, db.NoRowsAffected, fmt.Errorf("update: password account: %w", db.ErrEmptyFieldMask) + } + + if changeUserName { + cc, err := r.currentConfigForAccount(ctx, a.PublicId) + if err != nil { + return nil, db.NoRowsAffected, fmt.Errorf("update: password account: retrieve current configuration: %w", err) + } + if cc.MinUserNameLength > len(a.UserName) { + return nil, db.NoRowsAffected, fmt.Errorf("update: password account: user name %q: %w", a.UserName, ErrTooShort) + } + } + + a = a.clone() + + metadata := a.oplog(oplog.OpType_OP_TYPE_UPDATE) + + var rowsUpdated int + var returnedAccount *Account + _, err := r.writer.DoTx(ctx, db.StdRetryCnt, db.ExpBackoff{}, + func(_ db.Reader, w db.Writer) error { + returnedAccount = a.clone() + var err error + rowsUpdated, err = w.Update(ctx, returnedAccount, dbMask, nullFields, db.WithOplog(r.wrapper, metadata)) + if err == nil && rowsUpdated > 1 { + return db.ErrMultipleRecords + } + return err + }, + ) + + if err != nil { + if db.IsUniqueError(err) { + return nil, db.NoRowsAffected, fmt.Errorf("update: password account: %s: name %s already exists: %w", + a.PublicId, a.Name, db.ErrNotUnique) + } + return nil, db.NoRowsAffected, fmt.Errorf("update: password account: %s: %w", a.PublicId, err) + } + + return returnedAccount, rowsUpdated, nil +} diff --git a/internal/auth/password/repository_account_test.go b/internal/auth/password/repository_account_test.go index 79730cc7f..181448eec 100644 --- a/internal/auth/password/repository_account_test.go +++ b/internal/auth/password/repository_account_test.go @@ -8,6 +8,7 @@ import ( "github.com/hashicorp/boundary/internal/auth/password/store" "github.com/hashicorp/boundary/internal/db" + dbassert "github.com/hashicorp/boundary/internal/db/assert" "github.com/hashicorp/boundary/internal/iam" "github.com/hashicorp/boundary/internal/oplog" "github.com/stretchr/testify/assert" @@ -536,3 +537,555 @@ func TestRepository_ListAccounts_Limits(t *testing.T) { }) } } + +func TestRepository_UpdateAccount(t *testing.T) { + conn, _ := db.TestSetup(t, "postgres") + rw := db.New(conn) + wrapper := db.TestWrapper(t) + + changeUserName := func(s string) func(*Account) *Account { + return func(a *Account) *Account { + a.UserName = s + return a + } + } + + changeName := func(s string) func(*Account) *Account { + return func(a *Account) *Account { + a.Name = s + return a + } + } + + changeDescription := func(s string) func(*Account) *Account { + return func(a *Account) *Account { + a.Description = s + return a + } + } + + makeNil := func() func(*Account) *Account { + return func(a *Account) *Account { + return nil + } + } + + makeEmbeddedNil := func() func(*Account) *Account { + return func(a *Account) *Account { + return &Account{} + } + } + + deletePublicId := func() func(*Account) *Account { + return func(a *Account) *Account { + a.PublicId = "" + return a + } + } + + nonExistentPublicId := func() func(*Account) *Account { + return func(a *Account) *Account { + a.PublicId = "abcd_OOOOOOOOOO" + return a + } + } + + combine := func(fns ...func(a *Account) *Account) func(*Account) *Account { + return func(a *Account) *Account { + for _, fn := range fns { + a = fn(a) + } + return a + } + } + + var tests = []struct { + name string + orig *Account + chgFn func(*Account) *Account + masks []string + want *Account + wantCount int + wantIsErr error + }{ + { + name: "nil-Account", + orig: &Account{ + Account: &store.Account{}, + }, + chgFn: makeNil(), + masks: []string{"Name", "Description"}, + wantIsErr: db.ErrNilParameter, + }, + { + name: "nil-embedded-Account", + orig: &Account{ + Account: &store.Account{}, + }, + chgFn: makeEmbeddedNil(), + masks: []string{"Name", "Description"}, + wantIsErr: db.ErrNilParameter, + }, + { + name: "no-public-id", + orig: &Account{ + Account: &store.Account{}, + }, + chgFn: deletePublicId(), + masks: []string{"Name", "Description"}, + wantIsErr: db.ErrInvalidParameter, + }, + { + name: "updating-non-existent-Account", + orig: &Account{ + Account: &store.Account{ + Name: "test-name-repo", + }, + }, + chgFn: combine(nonExistentPublicId(), changeName("test-update-name-repo")), + masks: []string{"Name"}, + wantIsErr: db.ErrRecordNotFound, + }, + { + name: "empty-field-mask", + orig: &Account{ + Account: &store.Account{ + Name: "test-name-repo", + }, + }, + chgFn: changeName("test-update-name-repo"), + wantIsErr: db.ErrEmptyFieldMask, + }, + { + name: "read-only-fields-in-field-mask", + orig: &Account{ + Account: &store.Account{ + Name: "test-name-repo", + }, + }, + chgFn: changeName("test-update-name-repo"), + masks: []string{"PublicId", "CreateTime", "UpdateTime", "AuthMethodId"}, + wantIsErr: db.ErrInvalidFieldMask, + }, + { + name: "unknown-field-in-field-mask", + orig: &Account{ + Account: &store.Account{ + Name: "test-name-repo", + }, + }, + chgFn: changeName("test-update-name-repo"), + masks: []string{"Bilbo"}, + wantIsErr: db.ErrInvalidFieldMask, + }, + { + name: "change-name", + orig: &Account{ + Account: &store.Account{ + Name: "test-name-repo", + }, + }, + chgFn: changeName("test-update-name-repo"), + masks: []string{"Name"}, + want: &Account{ + Account: &store.Account{ + Name: "test-update-name-repo", + }, + }, + wantCount: 1, + }, + { + name: "change-description", + orig: &Account{ + Account: &store.Account{ + Description: "test-description-repo", + }, + }, + chgFn: changeDescription("test-update-description-repo"), + masks: []string{"Description"}, + want: &Account{ + Account: &store.Account{ + Description: "test-update-description-repo", + }, + }, + wantCount: 1, + }, + { + name: "change-name-and-description", + orig: &Account{ + Account: &store.Account{ + Name: "test-name-repo", + Description: "test-description-repo", + }, + }, + chgFn: combine(changeDescription("test-update-description-repo"), changeName("test-update-name-repo")), + masks: []string{"Name", "Description"}, + want: &Account{ + Account: &store.Account{ + Name: "test-update-name-repo", + Description: "test-update-description-repo", + }, + }, + wantCount: 1, + }, + { + name: "delete-name", + orig: &Account{ + Account: &store.Account{ + Name: "test-name-repo", + Description: "test-description-repo", + }, + }, + masks: []string{"Name"}, + chgFn: combine(changeDescription("test-update-description-repo"), changeName("")), + want: &Account{ + Account: &store.Account{ + Description: "test-description-repo", + }, + }, + wantCount: 1, + }, + { + name: "delete-description", + orig: &Account{ + Account: &store.Account{ + Name: "test-name-repo", + Description: "test-description-repo", + }, + }, + masks: []string{"Description"}, + chgFn: combine(changeDescription(""), changeName("test-update-name-repo")), + want: &Account{ + Account: &store.Account{ + Name: "test-name-repo", + }, + }, + wantCount: 1, + }, + { + name: "do-not-delete-name", + orig: &Account{ + Account: &store.Account{ + Name: "test-name-repo", + Description: "test-description-repo", + }, + }, + masks: []string{"Description"}, + chgFn: combine(changeDescription("test-update-description-repo"), changeName("")), + want: &Account{ + Account: &store.Account{ + Name: "test-name-repo", + Description: "test-update-description-repo", + }, + }, + wantCount: 1, + }, + { + name: "do-not-delete-description", + orig: &Account{ + Account: &store.Account{ + Name: "test-name-repo", + Description: "test-description-repo", + }, + }, + masks: []string{"Name"}, + chgFn: combine(changeDescription(""), changeName("test-update-name-repo")), + want: &Account{ + Account: &store.Account{ + Name: "test-update-name-repo", + Description: "test-description-repo", + }, + }, + wantCount: 1, + }, + { + name: "change-user-name", + orig: &Account{ + Account: &store.Account{ + UserName: "kazmierczak", + }, + }, + chgFn: changeUserName("mothball"), + masks: []string{"UserName"}, + want: &Account{ + Account: &store.Account{ + UserName: "mothball", + }, + }, + wantCount: 1, + }, + { + name: "change-user-name-mixed-caps", + orig: &Account{ + Account: &store.Account{ + UserName: "kazmierczak", + }, + }, + chgFn: changeUserName("KaZmIeRcZaK"), + masks: []string{"UserName"}, + wantIsErr: db.ErrInvalidParameter, + }, + { + name: "change-user-name-to-short", + orig: &Account{ + Account: &store.Account{ + UserName: "kazmierczak", + }, + }, + chgFn: changeUserName("ka"), + masks: []string{"UserName"}, + wantIsErr: ErrTooShort, + }, + { + name: "delete-user-name", + orig: &Account{ + Account: &store.Account{ + UserName: "kazmierczak", + }, + }, + chgFn: changeUserName(""), + masks: []string{"UserName"}, + wantIsErr: db.ErrInvalidParameter, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + repo, err := NewRepository(rw, rw, wrapper) + assert.NoError(err) + require.NotNil(repo) + + org, _ := iam.TestScopes(t, conn) + am := TestAuthMethods(t, conn, org.PublicId, 1)[0] + + tt.orig.AuthMethodId = am.PublicId + if tt.orig.UserName == "" { + tt.orig.UserName = "kazmierczak" + } + orig, err := repo.CreateAccount(context.Background(), tt.orig) + assert.NoError(err) + require.NotNil(orig) + + if tt.chgFn != nil { + orig = tt.chgFn(orig) + } + got, gotCount, err := repo.UpdateAccount(context.Background(), orig, tt.masks) + if tt.wantIsErr != nil { + assert.Truef(errors.Is(err, tt.wantIsErr), "want err: %q got: %q", tt.wantIsErr, err) + assert.Equal(tt.wantCount, gotCount, "row count") + assert.Nil(got) + return + } + assert.NoError(err) + assert.Empty(tt.orig.PublicId) + if tt.wantCount == 0 { + assert.Equal(tt.wantCount, gotCount, "row count") + assert.Nil(got) + return + } + require.NotNil(got) + assertPublicId(t, AccountPrefix, got.PublicId) + assert.Equal(tt.wantCount, gotCount, "row count") + assert.NotSame(tt.orig, got) + assert.Equal(tt.orig.AuthMethodId, got.AuthMethodId) + dbassert := dbassert.New(t, rw) + if tt.want.Name == "" { + dbassert.IsNull(got, "name") + return + } + assert.Equal(tt.want.Name, got.Name) + if tt.want.Description == "" { + dbassert.IsNull(got, "description") + return + } + assert.Equal(tt.want.Description, got.Description) + if tt.wantCount > 0 { + assert.NoError(db.TestVerifyOplog(t, rw, got.PublicId, db.WithOperation(oplog.OpType_OP_TYPE_UPDATE), db.WithCreateNotBefore(10*time.Second))) + } + }) + } + + t.Run("invalid-duplicate-names", func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + repo, err := NewRepository(rw, rw, wrapper) + assert.NoError(err) + require.NotNil(repo) + + name := "test-dup-name" + org, _ := iam.TestScopes(t, conn) + am := TestAuthMethods(t, conn, org.PublicId, 1)[0] + acts := TestAccounts(t, conn, am.PublicId, 2) + + aa := acts[0] + ab := acts[1] + + aa.Name = name + got1, gotCount1, err := repo.UpdateAccount(context.Background(), aa, []string{"name"}) + assert.NoError(err) + require.NotNil(got1) + assert.Equal(name, got1.Name) + assert.Equal(1, gotCount1, "row count") + assert.NoError(db.TestVerifyOplog(t, rw, aa.PublicId, db.WithOperation(oplog.OpType_OP_TYPE_UPDATE), db.WithCreateNotBefore(10*time.Second))) + + ab.Name = name + got2, gotCount2, err := repo.UpdateAccount(context.Background(), ab, []string{"name"}) + assert.Truef(errors.Is(err, db.ErrNotUnique), "want err: %v got: %v", db.ErrNotUnique, err) + assert.Nil(got2) + assert.Equal(db.NoRowsAffected, gotCount2, "row count") + err = db.TestVerifyOplog(t, rw, ab.PublicId, db.WithOperation(oplog.OpType_OP_TYPE_UPDATE), db.WithCreateNotBefore(10*time.Second)) + assert.Error(err) + assert.True(errors.Is(db.ErrRecordNotFound, err)) + }) + + t.Run("valid-duplicate-names-diff-AuthMethods", func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + repo, err := NewRepository(rw, rw, wrapper) + assert.NoError(err) + require.NotNil(repo) + + org, _ := iam.TestScopes(t, conn) + ams := TestAuthMethods(t, conn, org.PublicId, 2) + + ama := ams[0] + amb := ams[1] + + in := &Account{ + Account: &store.Account{ + Name: "test-name-repo", + }, + } + in2 := in.clone() + + in.AuthMethodId = ama.PublicId + in.UserName = "kazmierczak" + got, err := repo.CreateAccount(context.Background(), in) + assert.NoError(err) + require.NotNil(got) + assertPublicId(t, AccountPrefix, got.PublicId) + assert.NotSame(in, got) + assert.Equal(in.Name, got.Name) + assert.Equal(in.Description, got.Description) + + in2.AuthMethodId = amb.PublicId + in2.UserName = "kazmierczak2" + in2.Name = "first-name" + got2, err := repo.CreateAccount(context.Background(), in2) + assert.NoError(err) + require.NotNil(got2) + got2.Name = got.Name + got3, gotCount3, err := repo.UpdateAccount(context.Background(), got2, []string{"name"}) + assert.NoError(err) + require.NotNil(got3) + assert.NotSame(got2, got3) + assert.Equal(got.Name, got3.Name) + assert.Equal(got2.Description, got3.Description) + assert.Equal(1, gotCount3, "row count") + assert.NoError(db.TestVerifyOplog(t, rw, got2.PublicId, db.WithOperation(oplog.OpType_OP_TYPE_UPDATE), db.WithCreateNotBefore(10*time.Second))) + }) + + t.Run("invalid-duplicate-usernames", func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + repo, err := NewRepository(rw, rw, wrapper) + assert.NoError(err) + require.NotNil(repo) + + userName := "kazmierczak12" + org, _ := iam.TestScopes(t, conn) + am := TestAuthMethods(t, conn, org.PublicId, 1)[0] + acts := TestAccounts(t, conn, am.PublicId, 2) + + aa := acts[0] + ab := acts[1] + + aa.UserName = userName + got1, gotCount1, err := repo.UpdateAccount(context.Background(), aa, []string{"UserName"}) + assert.NoError(err) + require.NotNil(got1) + assert.Equal(userName, got1.UserName) + assert.Equal(1, gotCount1, "row count") + assert.NoError(db.TestVerifyOplog(t, rw, aa.PublicId, db.WithOperation(oplog.OpType_OP_TYPE_UPDATE), db.WithCreateNotBefore(10*time.Second))) + + ab.UserName = userName + got2, gotCount2, err := repo.UpdateAccount(context.Background(), ab, []string{"UserName"}) + assert.Truef(errors.Is(err, db.ErrNotUnique), "want err: %v got: %v", db.ErrNotUnique, err) + assert.Nil(got2) + assert.Equal(db.NoRowsAffected, gotCount2, "row count") + err = db.TestVerifyOplog(t, rw, ab.PublicId, db.WithOperation(oplog.OpType_OP_TYPE_UPDATE), db.WithCreateNotBefore(10*time.Second)) + assert.Error(err) + assert.True(errors.Is(db.ErrRecordNotFound, err)) + }) + + t.Run("valid-duplicate-usernames-diff-AuthMethods", func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + repo, err := NewRepository(rw, rw, wrapper) + assert.NoError(err) + require.NotNil(repo) + + org, _ := iam.TestScopes(t, conn) + ams := TestAuthMethods(t, conn, org.PublicId, 2) + + ama := ams[0] + amb := ams[1] + + in := &Account{ + Account: &store.Account{ + UserName: "kazmierczak", + }, + } + in2 := in.clone() + + in.AuthMethodId = ama.PublicId + got, err := repo.CreateAccount(context.Background(), in) + assert.NoError(err) + require.NotNil(got) + assertPublicId(t, AccountPrefix, got.PublicId) + assert.NotSame(in, got) + assert.Equal(in.UserName, got.UserName) + + in2.AuthMethodId = amb.PublicId + in2.UserName = "kazmierczak2" + got2, err := repo.CreateAccount(context.Background(), in2) + assert.NoError(err) + require.NotNil(got2) + got2.UserName = got.UserName + got3, gotCount3, err := repo.UpdateAccount(context.Background(), got2, []string{"UserName"}) + assert.NoError(err) + require.NotNil(got3) + assert.NotSame(got2, got3) + assert.Equal(got.UserName, got3.UserName) + assert.Equal(1, gotCount3, "row count") + assert.NoError(db.TestVerifyOplog(t, rw, got2.PublicId, db.WithOperation(oplog.OpType_OP_TYPE_UPDATE), db.WithCreateNotBefore(10*time.Second))) + }) + + t.Run("change-authmethod-id", func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + repo, err := NewRepository(rw, rw, wrapper) + assert.NoError(err) + require.NotNil(repo) + + org, _ := iam.TestScopes(t, conn) + ams := TestAuthMethods(t, conn, org.PublicId, 2) + + ama := ams[0] + amb := ams[1] + + aa := TestAccounts(t, conn, ama.PublicId, 1)[0] + ab := TestAccounts(t, conn, amb.PublicId, 1)[0] + + assert.NotEqual(aa.AuthMethodId, ab.AuthMethodId) + orig := aa.clone() + + aa.AuthMethodId = ab.AuthMethodId + assert.Equal(aa.AuthMethodId, ab.AuthMethodId) + + got1, gotCount1, err := repo.UpdateAccount(context.Background(), aa, []string{"name"}) + + assert.NoError(err) + require.NotNil(got1) + assert.Equal(orig.AuthMethodId, got1.AuthMethodId) + assert.Equal(1, gotCount1, "row count") + assert.NoError(db.TestVerifyOplog(t, rw, aa.PublicId, db.WithOperation(oplog.OpType_OP_TYPE_UPDATE), db.WithCreateNotBefore(10*time.Second))) + }) +} diff --git a/internal/auth/password/repository_configuration.go b/internal/auth/password/repository_configuration.go index 37b63fdd7..19cab80e5 100644 --- a/internal/auth/password/repository_configuration.go +++ b/internal/auth/password/repository_configuration.go @@ -126,6 +126,38 @@ func (r *Repository) currentConfig(ctx context.Context, authMethodId string) (*c return &cc, nil } +func (r *Repository) currentConfigForAccount(ctx context.Context, accountId string) (*currentConfig, error) { + var confs []currentConfig + tx, err := r.reader.DB() + if err != nil { + return nil, err + } + rows, err := tx.Query(currentConfigForAccountQuery, accountId) + if err != nil { + return nil, err + } + defer rows.Close() + for rows.Next() { + var conf currentConfig + if err := r.reader.ScanRows(rows, &conf); err != nil { + return nil, err + } + confs = append(confs, conf) + } + + var cc currentConfig + switch { + case len(confs) == 0: + return nil, nil + case len(confs) > 1: + // this should never happen + return nil, fmt.Errorf("multiple current configs returned for account") + default: + cc = confs[0] + } + return &cc, nil +} + func (c *currentConfig) argon2() *Argon2Configuration { if c.ConfType != "argon2" { return nil diff --git a/internal/auth/password/repository_password.go b/internal/auth/password/repository_password.go index 84662b6ef..b17340605 100644 --- a/internal/auth/password/repository_password.go +++ b/internal/auth/password/repository_password.go @@ -3,6 +3,7 @@ package password import ( "context" "crypto/subtle" + "errors" "fmt" "github.com/hashicorp/boundary/internal/db" @@ -194,3 +195,58 @@ func (r *Repository) authenticate(ctx context.Context, authMethodId string, user } return &acct, nil } + +// SetPassword sets the password for accountId to password. If password +// contains an empty string, the password for accountId will be deleted. +func (r *Repository) SetPassword(ctx context.Context, accountId string, password string) error { + if accountId == "" { + return fmt.Errorf("set password: no accountId: %w", db.ErrInvalidParameter) + } + + var newCred *Argon2Credential + if password != "" { + cc, err := r.currentConfigForAccount(ctx, accountId) + if err != nil { + return fmt.Errorf("set password: retrieve current configuration: %w", err) + } + if cc.MinPasswordLength > len(password) { + return fmt.Errorf("set password: new password: %w", ErrTooShort) + } + newCred, err = newArgon2Credential(accountId, password, cc.argon2()) + if err != nil { + return fmt.Errorf("set password: %w", err) + } + if err := newCred.encrypt(ctx, r.wrapper); err != nil { + return fmt.Errorf("set password: encrypt: %w", err) + } + } + + _, err := r.writer.DoTx(ctx, db.StdRetryCnt, db.ExpBackoff{}, + func(rr db.Reader, w db.Writer) error { + oldCred := allocCredential() + if err := rr.LookupWhere(ctx, &oldCred, "password_account_id = ?", accountId); err != nil { + if !errors.Is(err, db.ErrRecordNotFound) { + return err + } + } + if oldCred.PrivateId != "" { + dCred := oldCred.clone() + rowsDeleted, err := w.Delete(ctx, dCred, db.WithOplog(r.wrapper, oldCred.oplog(oplog.OpType_OP_TYPE_DELETE))) + if err == nil && rowsDeleted > 1 { + return db.ErrMultipleRecords + } + if err != nil { + return err + } + } + if newCred != nil { + return w.Create(ctx, newCred, db.WithOplog(r.wrapper, newCred.oplog(oplog.OpType_OP_TYPE_CREATE))) + } + return nil + }, + ) + if err != nil { + return fmt.Errorf("set password: %w", err) + } + return nil +} diff --git a/internal/auth/password/repository_password_test.go b/internal/auth/password/repository_password_test.go index b23dfd7ff..e9b6b1711 100644 --- a/internal/auth/password/repository_password_test.go +++ b/internal/auth/password/repository_password_test.go @@ -411,3 +411,137 @@ func TestRepository_ChangePassword(t *testing.T) { }) } } + +func TestRepository_SetPassword(t *testing.T) { + conn, _ := db.TestSetup(t, "postgres") + rw := db.New(conn) + wrapper := db.TestWrapper(t) + + repo, err := NewRepository(rw, rw, wrapper) + require.NoError(t, err) + require.NotNil(t, repo) + + template := &Account{ + Account: &store.Account{ + UserName: "kazmierczak", + }, + } + origPasswd := "12345678" + + wantAuthenticate := func(t *testing.T, authMethodId string, pwd string, msg string) string { + acct, err := repo.Authenticate(context.Background(), authMethodId, template.UserName, pwd) + assert.NoErrorf(t, err, "%s: authenticate should not return an error", msg) + if assert.NotNilf(t, acct, "%s: authenticate should succeed", msg) { + return acct.CredentialId + } + return "" + } + wantNoAuthenticate := func(t *testing.T, authMethodId string, pwd string, msg string) string { + if authMethodId == "" || pwd == "" { + return "" + } + acct, err := repo.Authenticate(context.Background(), authMethodId, template.UserName, pwd) + assert.NoErrorf(t, err, "%s: authenticate should not return an error", msg) + assert.Nilf(t, acct, "%s: authenticate should not succeed", msg) + return "" + } + + type args struct { + accountId string + password string + } + + var tests = []struct { + name string + args args + createWithPassword bool + useArgsAccountId bool + wantIsErr error + }{ + { + name: "no-accountId", + args: args{ + accountId: "", + password: "abcdefghijk", + }, + createWithPassword: true, + useArgsAccountId: true, + wantIsErr: db.ErrInvalidParameter, + }, + { + name: "new-passsword-too-short", + args: args{ + password: "a", + }, + wantIsErr: ErrTooShort, + }, + { + name: "valid-new-password-no-old-password", + args: args{ + password: "abcdefghijk", + }, + }, + { + name: "valid-new-password-delete-old-password", + args: args{ + password: "abcdefghijk", + }, + createWithPassword: true, + }, + { + name: "valid-delete-password-no-new-password", + args: args{ + password: "", + }, + createWithPassword: true, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + + o, _ := iam.TestScopes(t, conn) + authMethod := TestAuthMethods(t, conn, o.GetPublicId(), 1)[0] + inAcct := template.clone() + inAcct.AuthMethodId = authMethod.PublicId + var opts []Option + if tt.createWithPassword { + opts = append(opts, WithPassword(origPasswd)) + } + outAcct, err := repo.CreateAccount(context.Background(), inAcct, opts...) + require.NoError(err) + require.NotNil(outAcct) + + nextAuth := wantNoAuthenticate + if tt.createWithPassword { + nextAuth = wantAuthenticate + } + oldCredId := nextAuth(t, authMethod.PublicId, origPasswd, "after create account") + + if !tt.useArgsAccountId { + tt.args.accountId = outAcct.PublicId + } + + err = repo.SetPassword(context.Background(), tt.args.accountId, tt.args.password) + if tt.wantIsErr != nil { + assert.Error(err) + assert.Truef(errors.Is(err, tt.wantIsErr), "want err: %q got: %q", tt.wantIsErr, err) + newCredId := nextAuth(t, authMethod.PublicId, origPasswd, "SetPassword failed") + assert.Equal(oldCredId, newCredId) + return + } + require.NoError(err) + + if oldCredId != "" { + wantNoAuthenticate(t, authMethod.PublicId, origPasswd, "old password") + assert.NoError(db.TestVerifyOplog(t, rw, oldCredId, db.WithOperation(oplog.OpType_OP_TYPE_DELETE), db.WithCreateNotBefore(10*time.Second))) + } + if tt.args.password != "" { + newCredId := wantAuthenticate(t, authMethod.PublicId, tt.args.password, "new password") + assert.NotEqual(oldCredId, newCredId) + assert.NoError(db.TestVerifyOplog(t, rw, newCredId, db.WithOperation(oplog.OpType_OP_TYPE_CREATE), db.WithCreateNotBefore(10*time.Second))) + } + }) + } +} diff --git a/internal/auth/password/store/password.pb.go b/internal/auth/password/store/password.pb.go index 73a5c86dd..f469f78b3 100644 --- a/internal/auth/password/store/password.pb.go +++ b/internal/auth/password/store/password.pb.go @@ -260,6 +260,81 @@ func (x *Account) GetUserName() string { return "" } +type Credential struct { + state protoimpl.MessageState + sizeCache protoimpl.SizeCache + unknownFields protoimpl.UnknownFields + + // @inject_tag: `gorm:"primary_key"` + PrivateId string `protobuf:"bytes,1,opt,name=private_id,json=privateId,proto3" json:"private_id,omitempty" gorm:"primary_key"` + // @inject_tag: `gorm:"not_null"` + PasswordAccountId string `protobuf:"bytes,2,opt,name=password_account_id,json=passwordAccountId,proto3" json:"password_account_id,omitempty" gorm:"not_null"` + // @inject_tag: `gorm:"not_null"` + PasswordConfId string `protobuf:"bytes,3,opt,name=password_conf_id,json=passwordConfId,proto3" json:"password_conf_id,omitempty" gorm:"not_null"` + // @inject_tag: `gorm:"not_null"` + PasswordMethodId string `protobuf:"bytes,4,opt,name=password_method_id,json=passwordMethodId,proto3" json:"password_method_id,omitempty" gorm:"not_null"` +} + +func (x *Credential) Reset() { + *x = Credential{} + if protoimpl.UnsafeEnabled { + mi := &file_controller_storage_auth_password_store_v1_password_proto_msgTypes[2] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) + } +} + +func (x *Credential) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*Credential) ProtoMessage() {} + +func (x *Credential) ProtoReflect() protoreflect.Message { + mi := &file_controller_storage_auth_password_store_v1_password_proto_msgTypes[2] + if protoimpl.UnsafeEnabled && x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use Credential.ProtoReflect.Descriptor instead. +func (*Credential) Descriptor() ([]byte, []int) { + return file_controller_storage_auth_password_store_v1_password_proto_rawDescGZIP(), []int{2} +} + +func (x *Credential) GetPrivateId() string { + if x != nil { + return x.PrivateId + } + return "" +} + +func (x *Credential) GetPasswordAccountId() string { + if x != nil { + return x.PasswordAccountId + } + return "" +} + +func (x *Credential) GetPasswordConfId() string { + if x != nil { + return x.PasswordConfId + } + return "" +} + +func (x *Credential) GetPasswordMethodId() string { + if x != nil { + return x.PasswordMethodId + } + return "" +} + var File_controller_storage_auth_password_store_v1_password_proto protoreflect.FileDescriptor var file_controller_storage_auth_password_store_v1_password_proto_rawDesc = []byte{ @@ -327,11 +402,23 @@ var file_controller_storage_auth_password_store_v1_password_proto_rawDesc = []by 0x5f, 0x69, 0x64, 0x18, 0x06, 0x20, 0x01, 0x28, 0x09, 0x52, 0x0c, 0x61, 0x75, 0x74, 0x68, 0x4d, 0x65, 0x74, 0x68, 0x6f, 0x64, 0x49, 0x64, 0x12, 0x1b, 0x0a, 0x09, 0x75, 0x73, 0x65, 0x72, 0x5f, 0x6e, 0x61, 0x6d, 0x65, 0x18, 0x07, 0x20, 0x01, 0x28, 0x09, 0x52, 0x08, 0x75, 0x73, 0x65, 0x72, - 0x4e, 0x61, 0x6d, 0x65, 0x42, 0x42, 0x5a, 0x40, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, - 0x6f, 0x6d, 0x2f, 0x68, 0x61, 0x73, 0x68, 0x69, 0x63, 0x6f, 0x72, 0x70, 0x2f, 0x62, 0x6f, 0x75, - 0x6e, 0x64, 0x61, 0x72, 0x79, 0x2f, 0x69, 0x6e, 0x74, 0x65, 0x72, 0x6e, 0x61, 0x6c, 0x2f, 0x61, - 0x75, 0x74, 0x68, 0x2f, 0x70, 0x61, 0x73, 0x73, 0x77, 0x6f, 0x72, 0x64, 0x2f, 0x73, 0x74, 0x6f, - 0x72, 0x65, 0x3b, 0x73, 0x74, 0x6f, 0x72, 0x65, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, + 0x4e, 0x61, 0x6d, 0x65, 0x22, 0xb3, 0x01, 0x0a, 0x0a, 0x43, 0x72, 0x65, 0x64, 0x65, 0x6e, 0x74, + 0x69, 0x61, 0x6c, 0x12, 0x1d, 0x0a, 0x0a, 0x70, 0x72, 0x69, 0x76, 0x61, 0x74, 0x65, 0x5f, 0x69, + 0x64, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x09, 0x70, 0x72, 0x69, 0x76, 0x61, 0x74, 0x65, + 0x49, 0x64, 0x12, 0x2e, 0x0a, 0x13, 0x70, 0x61, 0x73, 0x73, 0x77, 0x6f, 0x72, 0x64, 0x5f, 0x61, + 0x63, 0x63, 0x6f, 0x75, 0x6e, 0x74, 0x5f, 0x69, 0x64, 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x52, + 0x11, 0x70, 0x61, 0x73, 0x73, 0x77, 0x6f, 0x72, 0x64, 0x41, 0x63, 0x63, 0x6f, 0x75, 0x6e, 0x74, + 0x49, 0x64, 0x12, 0x28, 0x0a, 0x10, 0x70, 0x61, 0x73, 0x73, 0x77, 0x6f, 0x72, 0x64, 0x5f, 0x63, + 0x6f, 0x6e, 0x66, 0x5f, 0x69, 0x64, 0x18, 0x03, 0x20, 0x01, 0x28, 0x09, 0x52, 0x0e, 0x70, 0x61, + 0x73, 0x73, 0x77, 0x6f, 0x72, 0x64, 0x43, 0x6f, 0x6e, 0x66, 0x49, 0x64, 0x12, 0x2c, 0x0a, 0x12, + 0x70, 0x61, 0x73, 0x73, 0x77, 0x6f, 0x72, 0x64, 0x5f, 0x6d, 0x65, 0x74, 0x68, 0x6f, 0x64, 0x5f, + 0x69, 0x64, 0x18, 0x04, 0x20, 0x01, 0x28, 0x09, 0x52, 0x10, 0x70, 0x61, 0x73, 0x73, 0x77, 0x6f, + 0x72, 0x64, 0x4d, 0x65, 0x74, 0x68, 0x6f, 0x64, 0x49, 0x64, 0x42, 0x42, 0x5a, 0x40, 0x67, 0x69, + 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x68, 0x61, 0x73, 0x68, 0x69, 0x63, 0x6f, + 0x72, 0x70, 0x2f, 0x62, 0x6f, 0x75, 0x6e, 0x64, 0x61, 0x72, 0x79, 0x2f, 0x69, 0x6e, 0x74, 0x65, + 0x72, 0x6e, 0x61, 0x6c, 0x2f, 0x61, 0x75, 0x74, 0x68, 0x2f, 0x70, 0x61, 0x73, 0x73, 0x77, 0x6f, + 0x72, 0x64, 0x2f, 0x73, 0x74, 0x6f, 0x72, 0x65, 0x3b, 0x73, 0x74, 0x6f, 0x72, 0x65, 0x62, 0x06, + 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, } var ( @@ -346,17 +433,18 @@ func file_controller_storage_auth_password_store_v1_password_proto_rawDescGZIP() return file_controller_storage_auth_password_store_v1_password_proto_rawDescData } -var file_controller_storage_auth_password_store_v1_password_proto_msgTypes = make([]protoimpl.MessageInfo, 2) +var file_controller_storage_auth_password_store_v1_password_proto_msgTypes = make([]protoimpl.MessageInfo, 3) var file_controller_storage_auth_password_store_v1_password_proto_goTypes = []interface{}{ (*AuthMethod)(nil), // 0: controller.storage.auth.password.store.v1.AuthMethod (*Account)(nil), // 1: controller.storage.auth.password.store.v1.Account - (*timestamp.Timestamp)(nil), // 2: controller.storage.timestamp.v1.Timestamp + (*Credential)(nil), // 2: controller.storage.auth.password.store.v1.Credential + (*timestamp.Timestamp)(nil), // 3: controller.storage.timestamp.v1.Timestamp } var file_controller_storage_auth_password_store_v1_password_proto_depIdxs = []int32{ - 2, // 0: controller.storage.auth.password.store.v1.AuthMethod.create_time:type_name -> controller.storage.timestamp.v1.Timestamp - 2, // 1: controller.storage.auth.password.store.v1.AuthMethod.update_time:type_name -> controller.storage.timestamp.v1.Timestamp - 2, // 2: controller.storage.auth.password.store.v1.Account.create_time:type_name -> controller.storage.timestamp.v1.Timestamp - 2, // 3: controller.storage.auth.password.store.v1.Account.update_time:type_name -> controller.storage.timestamp.v1.Timestamp + 3, // 0: controller.storage.auth.password.store.v1.AuthMethod.create_time:type_name -> controller.storage.timestamp.v1.Timestamp + 3, // 1: controller.storage.auth.password.store.v1.AuthMethod.update_time:type_name -> controller.storage.timestamp.v1.Timestamp + 3, // 2: controller.storage.auth.password.store.v1.Account.create_time:type_name -> controller.storage.timestamp.v1.Timestamp + 3, // 3: controller.storage.auth.password.store.v1.Account.update_time:type_name -> controller.storage.timestamp.v1.Timestamp 4, // [4:4] is the sub-list for method output_type 4, // [4:4] is the sub-list for method input_type 4, // [4:4] is the sub-list for extension type_name @@ -394,6 +482,18 @@ func file_controller_storage_auth_password_store_v1_password_proto_init() { return nil } } + file_controller_storage_auth_password_store_v1_password_proto_msgTypes[2].Exporter = func(v interface{}, i int) interface{} { + switch v := v.(*Credential); i { + case 0: + return &v.state + case 1: + return &v.sizeCache + case 2: + return &v.unknownFields + default: + return nil + } + } } type x struct{} out := protoimpl.TypeBuilder{ @@ -401,7 +501,7 @@ func file_controller_storage_auth_password_store_v1_password_proto_init() { GoPackagePath: reflect.TypeOf(x{}).PkgPath(), RawDescriptor: file_controller_storage_auth_password_store_v1_password_proto_rawDesc, NumEnums: 0, - NumMessages: 2, + NumMessages: 3, NumExtensions: 0, NumServices: 0, }, diff --git a/internal/db/migrations/postgres.gen.go b/internal/db/migrations/postgres.gen.go index 4557aa2ba..7b41d06f3 100644 --- a/internal/db/migrations/postgres.gen.go +++ b/internal/db/migrations/postgres.gen.go @@ -1905,7 +1905,8 @@ begin; (name, version) values ('auth_password_method', 1), - ('auth_password_account', 1); + ('auth_password_account', 1), + ('auth_password_credential', 1); commit; diff --git a/internal/db/migrations/postgres/12_auth_password.up.sql b/internal/db/migrations/postgres/12_auth_password.up.sql index 44df77d70..9b1c1684d 100644 --- a/internal/db/migrations/postgres/12_auth_password.up.sql +++ b/internal/db/migrations/postgres/12_auth_password.up.sql @@ -264,6 +264,7 @@ begin; (name, version) values ('auth_password_method', 1), - ('auth_password_account', 1); + ('auth_password_account', 1), + ('auth_password_credential', 1); commit; diff --git a/internal/proto/local/controller/storage/auth/password/store/v1/password.proto b/internal/proto/local/controller/storage/auth/password/store/v1/password.proto index 3fe16eee7..93e741e2e 100644 --- a/internal/proto/local/controller/storage/auth/password/store/v1/password.proto +++ b/internal/proto/local/controller/storage/auth/password/store/v1/password.proto @@ -70,3 +70,14 @@ message Account { // the scope_id column is not included here as it is used only to ensure // data integrity in the database between iam users and auth methods. } + +message Credential { + // @inject_tag: `gorm:"primary_key"` + string private_id = 1; + // @inject_tag: `gorm:"not_null"` + string password_account_id = 2; + // @inject_tag: `gorm:"not_null"` + string password_conf_id = 3; + // @inject_tag: `gorm:"not_null"` + string password_method_id = 4; +}