diff --git a/Makefile b/Makefile index dd08121ef..06726072a 100644 --- a/Makefile +++ b/Makefile @@ -77,7 +77,7 @@ protobuild: @protoc-go-inject-tag -input=./internal/db/db_test/db_test.pb.go @protoc-go-inject-tag -input=./internal/host/static/store/static.pb.go @protoc-go-inject-tag -input=./internal/authtoken/store/authtoken.pb.go - @protoc-go-inject-tag -input=./internal/iam/store/auth_account.pb.go + @protoc-go-inject-tag -input=./internal/auth/store/account.pb.go @protoc-go-inject-tag -input=./internal/auth/password/store/password.pb.go @protoc-go-inject-tag -input=./internal/auth/password/store/argon2.pb.go @rm -R ${TMP_DIR} diff --git a/internal/auth/store/account.pb.go b/internal/auth/store/account.pb.go new file mode 100644 index 000000000..58c20490c --- /dev/null +++ b/internal/auth/store/account.pb.go @@ -0,0 +1,200 @@ +// Code generated by protoc-gen-go. DO NOT EDIT. +// versions: +// protoc-gen-go v1.25.0 +// protoc v3.12.3 +// source: controller/storage/auth/store/v1/account.proto + +package store + +import ( + proto "github.com/golang/protobuf/proto" + _ "github.com/hashicorp/watchtower/internal/db/timestamp" + protoreflect "google.golang.org/protobuf/reflect/protoreflect" + protoimpl "google.golang.org/protobuf/runtime/protoimpl" + reflect "reflect" + sync "sync" +) + +const ( + // Verify that this generated code is sufficiently up-to-date. + _ = protoimpl.EnforceVersion(20 - protoimpl.MinVersion) + // Verify that runtime/protoimpl is sufficiently up-to-date. + _ = protoimpl.EnforceVersion(protoimpl.MaxVersion - 20) +) + +// This is a compile-time assertion that a sufficiently up-to-date version +// of the legacy proto package is being used. +const _ = proto.ProtoPackageIsVersion4 + +// Account is a "shared table" between the iam, authtoken, and auth +// subsystems. The auth system is responsible for creating entries. The iam +// system is only responsible/allowed to update the iam_user_id. The authtoken +// system is a reader of this data. +type Account struct { + state protoimpl.MessageState + sizeCache protoimpl.SizeCache + unknownFields protoimpl.UnknownFields + + // auth_account_id is primary key for the user account + // @inject_tag: gorm:"primary_key" + PublicId string `protobuf:"bytes,1,opt,name=public_id,json=publicId,proto3" json:"public_id,omitempty" gorm:"primary_key"` + // auth_method_id is the auth method associated with this user account. + // @inject_tag: `gorm:"default:null"` + AuthMethodId string `protobuf:"bytes,2,opt,name=auth_method_id,json=authMethodId,proto3" json:"auth_method_id,omitempty" gorm:"default:null"` + // scope_id for the user account + // @inject_tag: `gorm:"default:null"` + ScopeId string `protobuf:"bytes,4,opt,name=scope_id,json=scopeId,proto3" json:"scope_id,omitempty" gorm:"default:null"` + // user_id is the iam.user associated with this account + // @inject_tag: `gorm:"default:null"` + IamUserId string `protobuf:"bytes,5,opt,name=iam_user_id,json=iamUserId,proto3" json:"iam_user_id,omitempty" gorm:"default:null"` +} + +func (x *Account) Reset() { + *x = Account{} + if protoimpl.UnsafeEnabled { + mi := &file_controller_storage_auth_store_v1_account_proto_msgTypes[0] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) + } +} + +func (x *Account) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*Account) ProtoMessage() {} + +func (x *Account) ProtoReflect() protoreflect.Message { + mi := &file_controller_storage_auth_store_v1_account_proto_msgTypes[0] + 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 Account.ProtoReflect.Descriptor instead. +func (*Account) Descriptor() ([]byte, []int) { + return file_controller_storage_auth_store_v1_account_proto_rawDescGZIP(), []int{0} +} + +func (x *Account) GetPublicId() string { + if x != nil { + return x.PublicId + } + return "" +} + +func (x *Account) GetAuthMethodId() string { + if x != nil { + return x.AuthMethodId + } + return "" +} + +func (x *Account) GetScopeId() string { + if x != nil { + return x.ScopeId + } + return "" +} + +func (x *Account) GetIamUserId() string { + if x != nil { + return x.IamUserId + } + return "" +} + +var File_controller_storage_auth_store_v1_account_proto protoreflect.FileDescriptor + +var file_controller_storage_auth_store_v1_account_proto_rawDesc = []byte{ + 0x0a, 0x2e, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x6f, 0x6c, 0x6c, 0x65, 0x72, 0x2f, 0x73, 0x74, 0x6f, + 0x72, 0x61, 0x67, 0x65, 0x2f, 0x61, 0x75, 0x74, 0x68, 0x2f, 0x73, 0x74, 0x6f, 0x72, 0x65, 0x2f, + 0x76, 0x31, 0x2f, 0x61, 0x63, 0x63, 0x6f, 0x75, 0x6e, 0x74, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, + 0x12, 0x20, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x6f, 0x6c, 0x6c, 0x65, 0x72, 0x2e, 0x73, 0x74, 0x6f, + 0x72, 0x61, 0x67, 0x65, 0x2e, 0x61, 0x75, 0x74, 0x68, 0x2e, 0x73, 0x74, 0x6f, 0x72, 0x65, 0x2e, + 0x76, 0x31, 0x1a, 0x2f, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x6f, 0x6c, 0x6c, 0x65, 0x72, 0x2f, 0x73, + 0x74, 0x6f, 0x72, 0x61, 0x67, 0x65, 0x2f, 0x74, 0x69, 0x6d, 0x65, 0x73, 0x74, 0x61, 0x6d, 0x70, + 0x2f, 0x76, 0x31, 0x2f, 0x74, 0x69, 0x6d, 0x65, 0x73, 0x74, 0x61, 0x6d, 0x70, 0x2e, 0x70, 0x72, + 0x6f, 0x74, 0x6f, 0x22, 0x87, 0x01, 0x0a, 0x07, 0x41, 0x63, 0x63, 0x6f, 0x75, 0x6e, 0x74, 0x12, + 0x1b, 0x0a, 0x09, 0x70, 0x75, 0x62, 0x6c, 0x69, 0x63, 0x5f, 0x69, 0x64, 0x18, 0x01, 0x20, 0x01, + 0x28, 0x09, 0x52, 0x08, 0x70, 0x75, 0x62, 0x6c, 0x69, 0x63, 0x49, 0x64, 0x12, 0x24, 0x0a, 0x0e, + 0x61, 0x75, 0x74, 0x68, 0x5f, 0x6d, 0x65, 0x74, 0x68, 0x6f, 0x64, 0x5f, 0x69, 0x64, 0x18, 0x02, + 0x20, 0x01, 0x28, 0x09, 0x52, 0x0c, 0x61, 0x75, 0x74, 0x68, 0x4d, 0x65, 0x74, 0x68, 0x6f, 0x64, + 0x49, 0x64, 0x12, 0x19, 0x0a, 0x08, 0x73, 0x63, 0x6f, 0x70, 0x65, 0x5f, 0x69, 0x64, 0x18, 0x04, + 0x20, 0x01, 0x28, 0x09, 0x52, 0x07, 0x73, 0x63, 0x6f, 0x70, 0x65, 0x49, 0x64, 0x12, 0x1e, 0x0a, + 0x0b, 0x69, 0x61, 0x6d, 0x5f, 0x75, 0x73, 0x65, 0x72, 0x5f, 0x69, 0x64, 0x18, 0x05, 0x20, 0x01, + 0x28, 0x09, 0x52, 0x09, 0x69, 0x61, 0x6d, 0x55, 0x73, 0x65, 0x72, 0x49, 0x64, 0x42, 0x3b, 0x5a, + 0x39, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x68, 0x61, 0x73, 0x68, + 0x69, 0x63, 0x6f, 0x72, 0x70, 0x2f, 0x77, 0x61, 0x74, 0x63, 0x68, 0x74, 0x6f, 0x77, 0x65, 0x72, + 0x2f, 0x69, 0x6e, 0x74, 0x65, 0x72, 0x6e, 0x61, 0x6c, 0x2f, 0x61, 0x75, 0x74, 0x68, 0x2f, 0x73, + 0x74, 0x6f, 0x72, 0x65, 0x3b, 0x73, 0x74, 0x6f, 0x72, 0x65, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, + 0x6f, 0x33, +} + +var ( + file_controller_storage_auth_store_v1_account_proto_rawDescOnce sync.Once + file_controller_storage_auth_store_v1_account_proto_rawDescData = file_controller_storage_auth_store_v1_account_proto_rawDesc +) + +func file_controller_storage_auth_store_v1_account_proto_rawDescGZIP() []byte { + file_controller_storage_auth_store_v1_account_proto_rawDescOnce.Do(func() { + file_controller_storage_auth_store_v1_account_proto_rawDescData = protoimpl.X.CompressGZIP(file_controller_storage_auth_store_v1_account_proto_rawDescData) + }) + return file_controller_storage_auth_store_v1_account_proto_rawDescData +} + +var file_controller_storage_auth_store_v1_account_proto_msgTypes = make([]protoimpl.MessageInfo, 1) +var file_controller_storage_auth_store_v1_account_proto_goTypes = []interface{}{ + (*Account)(nil), // 0: controller.storage.auth.store.v1.Account +} +var file_controller_storage_auth_store_v1_account_proto_depIdxs = []int32{ + 0, // [0:0] is the sub-list for method output_type + 0, // [0:0] is the sub-list for method input_type + 0, // [0:0] is the sub-list for extension type_name + 0, // [0:0] is the sub-list for extension extendee + 0, // [0:0] is the sub-list for field type_name +} + +func init() { file_controller_storage_auth_store_v1_account_proto_init() } +func file_controller_storage_auth_store_v1_account_proto_init() { + if File_controller_storage_auth_store_v1_account_proto != nil { + return + } + if !protoimpl.UnsafeEnabled { + file_controller_storage_auth_store_v1_account_proto_msgTypes[0].Exporter = func(v interface{}, i int) interface{} { + switch v := v.(*Account); 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{ + File: protoimpl.DescBuilder{ + GoPackagePath: reflect.TypeOf(x{}).PkgPath(), + RawDescriptor: file_controller_storage_auth_store_v1_account_proto_rawDesc, + NumEnums: 0, + NumMessages: 1, + NumExtensions: 0, + NumServices: 0, + }, + GoTypes: file_controller_storage_auth_store_v1_account_proto_goTypes, + DependencyIndexes: file_controller_storage_auth_store_v1_account_proto_depIdxs, + MessageInfos: file_controller_storage_auth_store_v1_account_proto_msgTypes, + }.Build() + File_controller_storage_auth_store_v1_account_proto = out.File + file_controller_storage_auth_store_v1_account_proto_rawDesc = nil + file_controller_storage_auth_store_v1_account_proto_goTypes = nil + file_controller_storage_auth_store_v1_account_proto_depIdxs = nil +} diff --git a/internal/authtoken/account.go b/internal/authtoken/account.go new file mode 100644 index 000000000..59b638118 --- /dev/null +++ b/internal/authtoken/account.go @@ -0,0 +1,46 @@ +package authtoken + +import ( + authStore "github.com/hashicorp/watchtower/internal/auth/store" + "google.golang.org/protobuf/proto" +) + +const ( + defaultAuthAccountTableName = "auth_account" +) + +// AuthAccount is from the auth subsystem and iam is only allowed to: lookup and +// update auth accounts. That's why there is no "new" factory for AuthAccounts. +type authAccount struct { + *authStore.Account + tableName string `gorm:"-"` +} + +func allocAuthAccount() *authAccount { + return &authAccount{ + Account: &authStore.Account{}, + } +} + +// Clone creates a clone of the auth account. +func (a *authAccount) clone() *authAccount { + cp := proto.Clone(a.Account) + return &authAccount{ + Account: cp.(*authStore.Account), + } +} + +// TableName returns the tablename to override the default gorm table name. +func (a *authAccount) TableName() string { + if a.tableName != "" { + return a.tableName + } + return defaultAuthAccountTableName +} + +// SetTableName sets the tablename and satisfies the ReplayableMessage +// interface. If the caller attempts to set the name to "" the name will be +// reset to the default name. +func (a *authAccount) SetTableName(n string) { + a.tableName = n +} diff --git a/internal/authtoken/repository.go b/internal/authtoken/repository.go index bf1e4e214..3d3a6e3f3 100644 --- a/internal/authtoken/repository.go +++ b/internal/authtoken/repository.go @@ -8,12 +8,9 @@ import ( "github.com/golang/protobuf/ptypes" wrapping "github.com/hashicorp/go-kms-wrapping" - "github.com/hashicorp/watchtower/internal/db/timestamp" - "github.com/hashicorp/watchtower/internal/iam" - iamStore "github.com/hashicorp/watchtower/internal/iam/store" - "github.com/hashicorp/watchtower/internal/authtoken/store" "github.com/hashicorp/watchtower/internal/db" + "github.com/hashicorp/watchtower/internal/db/timestamp" "github.com/hashicorp/watchtower/internal/oplog" ) @@ -100,8 +97,8 @@ func (r *Repository) CreateAuthToken(ctx context.Context, withIamUserId, withAut db.StdRetryCnt, db.ExpBackoff{}, func(read db.Reader, w db.Writer) error { - // TODO: Remove this and either rely on either Alloc or a method exposed by the auth repo. - acct := &iam.AuthAccount{AuthAccount: &iamStore.AuthAccount{PublicId: withAuthAccountId}} + acct := allocAuthAccount() + acct.PublicId = withAuthAccountId if err := read.LookupByPublicId(ctx, acct); err != nil { return fmt.Errorf("create: auth token: auth account lookup: %w", err) } diff --git a/internal/authtoken/repository_test.go b/internal/authtoken/repository_test.go index af0d297cf..e8001e83c 100644 --- a/internal/authtoken/repository_test.go +++ b/internal/authtoken/repository_test.go @@ -11,7 +11,6 @@ import ( "github.com/google/go-cmp/cmp" wrapping "github.com/hashicorp/go-kms-wrapping" "github.com/hashicorp/watchtower/internal/auth/password" - iamStore "github.com/hashicorp/watchtower/internal/iam/store" "github.com/hashicorp/watchtower/internal/oplog" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -410,7 +409,8 @@ func TestRepository_ValidateToken_expired(t *testing.T) { org, _ := iam.TestScopes(t, conn) baseAT := TestAuthToken(t, conn, wrapper, org.GetPublicId()) baseAT.GetAuthAccountId() - aAcct := &iam.AuthAccount{AuthAccount: &iamStore.AuthAccount{PublicId: baseAT.GetAuthAccountId()}} + aAcct := allocAuthAccount() + aAcct.PublicId = baseAT.GetAuthAccountId() require.NoError(t, rw.LookupByPublicId(context.Background(), aAcct)) iamUserId := aAcct.GetIamUserId() diff --git a/internal/db/option.go b/internal/db/option.go index 896b0c996..5985b7581 100644 --- a/internal/db/option.go +++ b/internal/db/option.go @@ -36,6 +36,9 @@ type Options struct { WithVersion *uint32 withSkipVetForWrite bool + + withWhereClause string + withWhereClauseArgs []interface{} } type oplogOpts struct { @@ -134,3 +137,12 @@ func WithSkipVetForWrite(enable bool) Option { o.withSkipVetForWrite = enable } } + +// WithWhere provides an option to provide a where clause with arguments for an +// operation. +func WithWhere(whereClause string, args ...interface{}) Option { + return func(o *Options) { + o.withWhereClause = whereClause + o.withWhereClauseArgs = append(o.withWhereClauseArgs, args...) + } +} diff --git a/internal/db/option_test.go b/internal/db/option_test.go index b7c9f3e08..4e32494ff 100644 --- a/internal/db/option_test.go +++ b/internal/db/option_test.go @@ -147,4 +147,19 @@ func Test_getOpts(t *testing.T) { testOpts.withSkipVetForWrite = true assert.Equal(opts, testOpts) }) + t.Run("WithWhere", func(t *testing.T) { + assert := assert.New(t) + // test default of false + opts := GetOpts() + testOpts := getDefaultOptions() + testOpts.withWhereClause = "" + testOpts.withWhereClauseArgs = nil + assert.Equal(opts, testOpts) + + // try setting to false + opts = GetOpts(WithWhere("id = ? and foo = ?", 1234, "bar")) + testOpts.withWhereClause = "id = ? and foo = ?" + testOpts.withWhereClauseArgs = []interface{}{1234, "bar"} + assert.Equal(opts, testOpts) + }) } diff --git a/internal/db/read_writer.go b/internal/db/read_writer.go index 53e08abf0..330fe49e4 100644 --- a/internal/db/read_writer.go +++ b/internal/db/read_writer.go @@ -424,14 +424,22 @@ func (rw *Db) Update(ctx context.Context, i interface{}, fieldMaskPaths []string } var underlying *gorm.DB switch { - case opts.WithVersion != nil: - if *opts.WithVersion == 0 { - return NoRowsAffected, fmt.Errorf("update: with version option is zero: %w", ErrInvalidParameter) + case opts.WithVersion != nil || opts.withWhereClause != "": + var where []string + var args []interface{} + if opts.WithVersion != nil { + if *opts.WithVersion == 0 { + return NoRowsAffected, fmt.Errorf("update: with version option is zero: %w", ErrInvalidParameter) + } + if _, ok := scope.FieldByName("version"); !ok { + return NoRowsAffected, fmt.Errorf("update: %s does not have a version field", scope.TableName()) + } + where, args = append(where, "version = ?"), append(args, opts.WithVersion) } - if _, ok := scope.FieldByName("version"); !ok { - return NoRowsAffected, fmt.Errorf("update: %s does not have a version field", scope.TableName()) + if opts.withWhereClause != "" { + where, args = append(where, opts.withWhereClause), append(args, opts.withWhereClauseArgs...) } - underlying = rw.underlying.Model(i).Where("version = ?", opts.WithVersion).Updates(updateFields) + underlying = rw.underlying.Model(i).Where(strings.Join(where, " and "), args...).Updates(updateFields) default: underlying = rw.underlying.Model(i).Updates(updateFields) } diff --git a/internal/db/read_writer_test.go b/internal/db/read_writer_test.go index db775114a..9097970e7 100644 --- a/internal/db/read_writer_test.go +++ b/internal/db/read_writer_test.go @@ -151,6 +151,68 @@ func TestDb_Update(t *testing.T) { wantPhoneNumber: "updated" + id, wantVersion: 2, }, + { + name: "simple-with-where", + args: args{ + i: &db_test.TestUser{ + StoreTestUser: &db_test.StoreTestUser{ + Name: "simple-with-where" + id, + Email: "updated" + id, + PhoneNumber: "updated" + id, + }, + }, + fieldMaskPaths: []string{"Name", "PhoneNumber"}, + setToNullPaths: []string{"Email"}, + opt: []Option{WithWhere("email = ? and phone_number = ?", id, id)}, + }, + want: 1, + wantErr: false, + wantErrMsg: "", + wantName: "simple-with-where" + id, + wantEmail: "", + wantPhoneNumber: "updated" + id, + wantVersion: 2, + }, + { + name: "simple-with-where-and-version", + args: args{ + i: &db_test.TestUser{ + StoreTestUser: &db_test.StoreTestUser{ + Name: "simple-with-where-and-version" + id, + Email: "updated" + id, + PhoneNumber: "updated" + id, + }, + }, + fieldMaskPaths: []string{"Name", "PhoneNumber"}, + setToNullPaths: []string{"Email"}, + opt: []Option{WithWhere("email = ? and phone_number = ?", id, id), WithVersion(&versionOne)}, + }, + want: 1, + wantErr: false, + wantErrMsg: "", + wantName: "simple-with-where-and-version" + id, + wantEmail: "", + wantPhoneNumber: "updated" + id, + wantVersion: 2, + }, + { + name: "bad-with-where", + args: args{ + i: &db_test.TestUser{ + StoreTestUser: &db_test.StoreTestUser{ + Name: "bad-with-where" + id, + Email: "updated" + id, + PhoneNumber: "updated" + id, + }, + }, + fieldMaskPaths: []string{"Name", "PhoneNumber"}, + setToNullPaths: []string{"Email"}, + opt: []Option{WithWhere("foo = ? and phone_number = ?", id, id)}, + }, + want: 0, + wantErr: true, + wantErrMsg: `update: failed: pq: column "foo" does not exist`, + }, { name: "multiple-null", args: args{ diff --git a/internal/iam/auth_account.go b/internal/iam/account.go similarity index 57% rename from internal/iam/auth_account.go rename to internal/iam/account.go index 3ab8133be..66f668205 100644 --- a/internal/iam/auth_account.go +++ b/internal/iam/account.go @@ -4,44 +4,44 @@ import ( "context" "fmt" + authStore "github.com/hashicorp/watchtower/internal/auth/store" "github.com/hashicorp/watchtower/internal/db" - "github.com/hashicorp/watchtower/internal/iam/store" "github.com/hashicorp/watchtower/internal/oplog" "github.com/hashicorp/watchtower/internal/types/scope" "google.golang.org/protobuf/proto" ) const ( - defaultAuthAccountTableName = "auth_account" + defaultAccountTableName = "auth_account" ) -// AuthAccount is from the auth subsystem and iam is only allowed to: lookup and -// update auth accounts. That's why there is no "new" factory for AuthAccounts. -type AuthAccount struct { - *store.AuthAccount +// authAccount is from the auth subsystem and iam is only allowed to: lookup and +// update auth accounts. That's why there is no "new" factory for Accounts. +type authAccount struct { + *authStore.Account tableName string `gorm:"-"` } -var _ Cloneable = (*AuthAccount)(nil) -var _ db.VetForWriter = (*AuthAccount)(nil) -var _ oplog.ReplayableMessage = (*AuthAccount)(nil) +var _ Cloneable = (*authAccount)(nil) +var _ db.VetForWriter = (*authAccount)(nil) +var _ oplog.ReplayableMessage = (*authAccount)(nil) -func allocAuthAccount() AuthAccount { - return AuthAccount{ - AuthAccount: &store.AuthAccount{}, +func allocAccount() authAccount { + return authAccount{ + Account: &authStore.Account{}, } } // Clone creates a clone of the auth account. -func (a *AuthAccount) Clone() interface{} { - cp := proto.Clone(a.AuthAccount) - return &AuthAccount{ - AuthAccount: cp.(*store.AuthAccount), +func (a *authAccount) Clone() interface{} { + cp := proto.Clone(a.Account) + return &authAccount{ + Account: cp.(*authStore.Account), } } // VetForWrite implements db.VetForWrite() interface. -func (a *AuthAccount) VetForWrite(ctx context.Context, r db.Reader, opType db.OpType, opt ...db.Option) error { +func (a *authAccount) VetForWrite(ctx context.Context, r db.Reader, opType db.OpType, opt ...db.Option) error { if a.PublicId == "" { return fmt.Errorf("error public id is empty string for auth account write: %w", db.ErrInvalidParameter) } @@ -51,26 +51,26 @@ func (a *AuthAccount) VetForWrite(ctx context.Context, r db.Reader, opType db.Op return nil } -func (a *AuthAccount) validScopeTypes() []scope.Type { +func (a *authAccount) validScopeTypes() []scope.Type { return []scope.Type{scope.Org} } // GetScope returns the scope for the auth account. -func (a *AuthAccount) GetScope(ctx context.Context, r db.Reader) (*Scope, error) { +func (a *authAccount) GetScope(ctx context.Context, r db.Reader) (*Scope, error) { return LookupScope(ctx, r, a) } // TableName returns the tablename to override the default gorm table name. -func (a *AuthAccount) TableName() string { +func (a *authAccount) TableName() string { if a.tableName != "" { return a.tableName } - return defaultAuthAccountTableName + return defaultAccountTableName } // SetTableName sets the tablename and satisfies the ReplayableMessage // interface. If the caller attempts to set the name to "" the name will be // reset to the default name. -func (a *AuthAccount) SetTableName(n string) { +func (a *authAccount) SetTableName(n string) { a.tableName = n } diff --git a/internal/iam/auth_account_test.go b/internal/iam/account_test.go similarity index 72% rename from internal/iam/auth_account_test.go rename to internal/iam/account_test.go index 213647638..24be1f699 100644 --- a/internal/iam/auth_account_test.go +++ b/internal/iam/account_test.go @@ -4,9 +4,9 @@ import ( "context" "testing" + authStore "github.com/hashicorp/watchtower/internal/auth/store" "github.com/hashicorp/watchtower/internal/db" dbassert "github.com/hashicorp/watchtower/internal/db/assert" - "github.com/hashicorp/watchtower/internal/iam/store" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "google.golang.org/protobuf/proto" @@ -17,7 +17,7 @@ import ( // where they are implemented). we just want to make sure that the iam subsystem // can update the IamUserId field successfully since that's what the iam system // relies on. -func Test_AuthAccountUpdate(t *testing.T) { +func Test_AccountUpdate(t *testing.T) { t.Parallel() conn, _ := db.TestSetup(t, "postgres") org, _ := TestScopes(t, conn) @@ -26,15 +26,15 @@ func Test_AuthAccountUpdate(t *testing.T) { assert, require := assert.New(t), require.New(t) u := TestUser(t, conn, org.PublicId) authMethodPublicId := testAuthMethod(t, conn, org.PublicId) - acct := testAuthAccount(t, conn, org.PublicId, authMethodPublicId, "") + acct := testAccount(t, conn, org.PublicId, authMethodPublicId, "") - updateAcct := acct.Clone().(*AuthAccount) + updateAcct := acct.Clone().(*authAccount) updateAcct.IamUserId = u.PublicId updatedRows, err := rw.Update(context.Background(), updateAcct, []string{"IamUserId"}, nil) require.NoError(err) assert.Equal(1, updatedRows) - foundAcct := allocAuthAccount() + foundAcct := allocAccount() foundAcct.PublicId = acct.PublicId err = rw.LookupByPublicId(context.Background(), &foundAcct) require.NoError(err) @@ -42,7 +42,7 @@ func Test_AuthAccountUpdate(t *testing.T) { }) } -func TestAuthAccount_GetScope(t *testing.T) { +func TestAccount_GetScope(t *testing.T) { t.Parallel() conn, _ := db.TestSetup(t, "postgres") org, _ := TestScopes(t, conn) @@ -52,14 +52,14 @@ func TestAuthAccount_GetScope(t *testing.T) { w := db.New(conn) u := TestUser(t, conn, org.PublicId) authMethodPublicId := testAuthMethod(t, conn, org.PublicId) - acct := testAuthAccount(t, conn, org.PublicId, authMethodPublicId, u.PublicId) + acct := testAccount(t, conn, org.PublicId, authMethodPublicId, u.PublicId) scope, err := acct.GetScope(context.Background(), w) require.NoError(err) assert.True(proto.Equal(org, scope)) }) } -func TestAuthAccount_Clone(t *testing.T) { +func TestAccount_Clone(t *testing.T) { t.Parallel() conn, _ := db.TestSetup(t, "postgres") org, _ := TestScopes(t, conn) @@ -67,26 +67,26 @@ func TestAuthAccount_Clone(t *testing.T) { assert := assert.New(t) u := TestUser(t, conn, org.PublicId) authMethodPublicId := testAuthMethod(t, conn, org.PublicId) - acct := testAuthAccount(t, conn, org.PublicId, authMethodPublicId, u.PublicId) + acct := testAccount(t, conn, org.PublicId, authMethodPublicId, u.PublicId) cp := acct.Clone() - assert.True(proto.Equal(cp.(*AuthAccount).AuthAccount, acct.AuthAccount)) + assert.True(proto.Equal(cp.(*authAccount).Account, acct.Account)) }) t.Run("not-equal", func(t *testing.T) { assert := assert.New(t) rw := db.New(conn) u := TestUser(t, conn, org.PublicId) authMethodPublicId := testAuthMethod(t, conn, org.PublicId) - acct := testAuthAccount(t, conn, org.PublicId, authMethodPublicId, u.PublicId) - acct2 := testAuthAccount(t, conn, org.PublicId, authMethodPublicId, "") + acct := testAccount(t, conn, org.PublicId, authMethodPublicId, u.PublicId) + acct2 := testAccount(t, conn, org.PublicId, authMethodPublicId, "") dbassert := dbassert.New(t, rw) dbassert.IsNull(acct2, "IamUserId") cp := acct.Clone() - assert.True(!proto.Equal(cp.(*AuthAccount).AuthAccount, acct2.AuthAccount)) + assert.True(!proto.Equal(cp.(*authAccount).Account, acct2.Account)) }) } -func TestAuthAccount_SetTableName(t *testing.T) { - defaultTableName := defaultAuthAccountTableName +func TestAccount_SetTableName(t *testing.T) { + defaultTableName := defaultAccountTableName tests := []struct { name string initialName string @@ -109,13 +109,13 @@ func TestAuthAccount_SetTableName(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { assert, require := assert.New(t), require.New(t) - def := &AuthAccount{ - AuthAccount: &store.AuthAccount{}, + def := &authAccount{ + Account: &authStore.Account{}, } require.Equal(defaultTableName, def.TableName()) - s := &AuthAccount{ - AuthAccount: &store.AuthAccount{}, - tableName: tt.initialName, + s := &authAccount{ + Account: &authStore.Account{}, + tableName: tt.initialName, } s.SetTableName(tt.setNameTo) assert.Equal(tt.want, s.TableName()) diff --git a/internal/iam/options.go b/internal/iam/options.go index 791cbb513..d6f254345 100644 --- a/internal/iam/options.go +++ b/internal/iam/options.go @@ -22,6 +22,7 @@ type options struct { withAutoVivify bool withGrantScopeId string withSkipVetForWrite bool + withDisassociate bool } func getDefaultOptions() options { @@ -97,3 +98,11 @@ func WithSkipVetForWrite(enable bool) Option { o.withSkipVetForWrite = enable } } + +// WithDisassociate provides an option to allow the combining of disassociating +// and associating a user in one operation. +func WithDisassociate(enable bool) Option { + return func(o *options) { + o.withDisassociate = enable + } +} diff --git a/internal/iam/options_test.go b/internal/iam/options_test.go index 0e37c6d6f..18824ba1a 100644 --- a/internal/iam/options_test.go +++ b/internal/iam/options_test.go @@ -61,4 +61,17 @@ func Test_GetOpts(t *testing.T) { testOpts.withGrantScopeId = "o_1234" assert.Equal(opts, testOpts) }) + t.Run("WithDisassociate", func(t *testing.T) { + assert := assert.New(t) + // test default of false + opts := getOpts() + testOpts := getDefaultOptions() + testOpts.withDisassociate = false + assert.Equal(opts, testOpts) + + opts = getOpts(WithDisassociate(true)) + testOpts = getDefaultOptions() + testOpts.withDisassociate = true + assert.Equal(opts, testOpts) + }) } diff --git a/internal/iam/query.go b/internal/iam/query.go index f133caba5..b0481ad62 100644 --- a/internal/iam/query.go +++ b/internal/iam/query.go @@ -4,8 +4,8 @@ package iam // the db via sql.DB vs the standard pattern of using the internal/db package to // interact with the db. const ( - // whereUserAuthAccount - given an auth account id, return the associated user. - whereUserAuthAccount = ` + // whereUserAccount - given an auth account id, return the associated user. + whereUserAccount = ` select iam_user.* from iam_user inner join auth_account diff --git a/internal/iam/repository_user.go b/internal/iam/repository_user.go index 3959aca9b..7ee2f112f 100644 --- a/internal/iam/repository_user.go +++ b/internal/iam/repository_user.go @@ -2,7 +2,6 @@ package iam import ( "context" - "errors" "fmt" "strings" @@ -124,18 +123,18 @@ func (r *Repository) ListUsers(ctx context.Context, withOrgId string, opt ...Opt return users, nil } -// LookupUserWithLogin will attempt to lookup the user with a matching auth +// LookupUserWithLogin will attempt to lookup the user with a matching // account id and return the user if found. If a user is not found and the // WithAutoVivify() option is true, then a new iam User will be -// created in the scope of the auth account, and associated with the auth +// created in the scope of the account, and associated with the // account. If a new user is auto vivified, then the WithName and // WithDescription options are supported as well. -func (r *Repository) LookupUserWithLogin(ctx context.Context, withAuthAccountId string, opt ...Option) (*User, error) { +func (r *Repository) LookupUserWithLogin(ctx context.Context, accountId string, opt ...Option) (*User, error) { opts := getOpts(opt...) - if withAuthAccountId == "" { - return nil, fmt.Errorf("lookup user with login: missing auth account id %w", db.ErrInvalidParameter) + if accountId == "" { + return nil, fmt.Errorf("lookup user with login: missing account id %w", db.ErrInvalidParameter) } - u, err := r.getUserWithAuthAccount(ctx, withAuthAccountId) + u, err := r.getUserWithAccount(ctx, accountId) if err != nil { return nil, fmt.Errorf("lookup user with login: %w", err) } @@ -143,24 +142,24 @@ func (r *Repository) LookupUserWithLogin(ctx context.Context, withAuthAccountId return u, nil } if !opts.withAutoVivify { - return nil, fmt.Errorf("lookup user with login: user not found for auth account %s: %w", withAuthAccountId, db.ErrRecordNotFound) + return nil, fmt.Errorf("lookup user with login: user not found for account %s: %w", accountId, db.ErrRecordNotFound) } - acct := allocAuthAccount() - acct.PublicId = withAuthAccountId + acct := allocAccount() + acct.PublicId = accountId err = r.reader.LookupByPublicId(context.Background(), &acct) if err != nil { - return nil, fmt.Errorf("lookup user with login: unable to lookup auth account %s: %w", withAuthAccountId, err) + return nil, fmt.Errorf("lookup user with login: unable to lookup account %s: %w", accountId, err) } metadata := oplog.Metadata{ - "resource-public-id": []string{withAuthAccountId}, + "resource-public-id": []string{accountId}, "scope-id": []string{acct.ScopeId}, "scope-type": []string{scope.Org.String()}, "resource-type": []string{"auth-account"}, } - // We will create a new user and associate the user with the auth account + // We will create a new user and associate the user with the account // within one retryable transaction using writer.DoTx var obtainedUser *User _, err = r.writer.DoTx( @@ -190,14 +189,14 @@ func (r *Repository) LookupUserWithLogin(ctx context.Context, withAuthAccountId msgs = append(msgs, &createMsg) var updateMsg oplog.Message - updateAcct := acct.Clone().(*AuthAccount) + updateAcct := acct.Clone().(*authAccount) updateAcct.IamUserId = id updatedRows, err := w.Update(ctx, updateAcct, []string{"IamUserId"}, nil, db.NewOplogMsg(&updateMsg)) if err != nil { return err } if updatedRows != 1 { - return fmt.Errorf("auth account update affected %d rows", updatedRows) + return fmt.Errorf("account update affected %d rows", updatedRows) } msgs = append(msgs, &updateMsg) if err := w.WriteOplogEntryWith(ctx, r.wrapper, ticket, metadata, msgs); err != nil { @@ -212,85 +211,180 @@ func (r *Repository) LookupUserWithLogin(ctx context.Context, withAuthAccountId return obtainedUser, nil } -func (r *Repository) getUserWithAuthAccount(ctx context.Context, withAuthAccountId string, opt ...Option) (*User, error) { - if withAuthAccountId == "" { - return nil, fmt.Errorf("missing auth account id %w", db.ErrInvalidParameter) +func (r *Repository) getUserWithAccount(ctx context.Context, withAccountId string, opt ...Option) (*User, error) { + if withAccountId == "" { + return nil, fmt.Errorf("missing account id %w", db.ErrInvalidParameter) } underlying, err := r.reader.DB() if err != nil { - return nil, fmt.Errorf("unable to get underlying db for auth account search: %w", err) + return nil, fmt.Errorf("unable to get underlying db for account search: %w", err) } - rows, err := underlying.Query(whereUserAuthAccount, withAuthAccountId) + rows, err := underlying.Query(whereUserAccount, withAccountId) if err != nil { - return nil, fmt.Errorf("unable to query auth account %s", withAuthAccountId) + return nil, fmt.Errorf("unable to query account %s", withAccountId) } defer rows.Close() u := allocUser() if rows.Next() { err = r.reader.ScanRows(rows, &u) if err != nil { - return nil, fmt.Errorf("unable to scan rows for auth account %s: %w", withAuthAccountId, err) + return nil, fmt.Errorf("unable to scan rows for account %s: %w", withAccountId, err) } } else { if err := rows.Err(); err != nil { - return nil, fmt.Errorf("unable to get next auth account: %w", err) + return nil, fmt.Errorf("unable to get next account: %w", err) } return nil, nil } if err != nil { - return nil, fmt.Errorf("unable to get next row for auth accounts %s: %w", withAuthAccountId, err) + return nil, fmt.Errorf("unable to get next row for accounts %s: %w", withAccountId, err) } return &u, nil } -func (r *Repository) validateLoginParameters(ctx context.Context, withScope, withAuthMethodId, withAuthAccountId string) (*AuthAccount, error) { - if withScope == "" { - return nil, fmt.Errorf("missing scope id %w", db.ErrInvalidParameter) +// AssociateUserWithAccount will associate a user with an existing account. +// The account must not already be associated with a different user. No +// options are currently supported. +func (r *Repository) AssociateUserWithAccount(ctx context.Context, userPublicId, accountId string, opt ...Option) (*User, error) { + opts := getOpts(opt...) + if userPublicId == "" { + return nil, fmt.Errorf("associate user with account: missing user public id %w", db.ErrInvalidParameter) } - if withAuthMethodId == "" { - return nil, fmt.Errorf("missing auth method id %w", db.ErrInvalidParameter) + if accountId == "" { + return nil, fmt.Errorf("associate user with account: missing account id %w", db.ErrInvalidParameter) } - if withAuthAccountId == "" { - return nil, fmt.Errorf("missing auth account id %w", db.ErrInvalidParameter) - } - ok, err := r.validateAuthMethodId(ctx, withScope, withAuthMethodId) + user := allocUser() + user.PublicId = userPublicId + + err := r.reader.LookupById(ctx, &user) if err != nil { - return nil, fmt.Errorf("unable to validate auth method %s in scope %s: %w", withAuthMethodId, withScope, err) + return nil, fmt.Errorf("associate user with account: unable to lookup user %s: %w", userPublicId, err) } - if !ok { - return nil, fmt.Errorf("auth method id %s in scope %s is not valid: %w", withAuthMethodId, withScope, db.ErrInvalidParameter) - } - acct := allocAuthAccount() - err = r.reader.LookupWhere(context.Background(), &acct, "public_id = ? and auth_method_id = ?", withAuthAccountId, withAuthMethodId) + + acct := allocAccount() + acct.PublicId = accountId + err = r.reader.LookupByPublicId(context.Background(), &acct) if err != nil { - if errors.Is(err, db.ErrRecordNotFound) { - return nil, fmt.Errorf("unable to search for auth account %s in auth method %s: %w", withAuthAccountId, withAuthMethodId, db.ErrInvalidParameter) + return nil, fmt.Errorf("associate user with account: unable to lookup account %s: %w", accountId, err) + } + // first, let's handle the case where the account is already + // associated with the user, so we're done! + if acct.IamUserId == userPublicId { + return &user, nil + } + + if !opts.withDisassociate { + if acct.IamUserId != "" && acct.IamUserId != userPublicId { + return nil, fmt.Errorf("associate user with account: %s account is already associated with a user: %w", accountId, db.ErrInvalidParameter) } - return nil, fmt.Errorf("unable to search for auth account %s in auth method %s: %w", withAuthAccountId, withAuthMethodId, err) } - return &acct, nil + + // validate, associated the user with the account, and then read the + // user back in the same tx for consistency. + _, err = r.writer.DoTx( + ctx, + db.StdRetryCnt, + db.ExpBackoff{}, + func(txReader db.Reader, w db.Writer) error { + + metadata := oplog.Metadata{ + "resource-public-id": []string{accountId}, + "scope-id": []string{acct.ScopeId}, + "scope-type": []string{scope.Org.String()}, + "resource-type": []string{"auth-account"}, + } + var updatedRows int + updatedAcct := acct.Clone().(*authAccount) + updatedAcct.IamUserId = userPublicId + // we are using WithWhere to make sure the account is not + // associated with a user (handling race conditions with concurrent + // transactions) + switch { + case opts.withDisassociate: + updatedRows, err = w.Update(ctx, updatedAcct, []string{"IamUserId"}, nil, db.WithOplog(r.wrapper, metadata), db.WithWhere("iam_user_id = ?", acct.IamUserId)) + default: + updatedRows, err = w.Update(ctx, updatedAcct, []string{"IamUserId"}, nil, db.WithOplog(r.wrapper, metadata), db.WithWhere("iam_user_id is NULL")) + } + if err != nil { + return err + } + if updatedRows != 1 { + return fmt.Errorf("account update affected %d rows", updatedRows) + } + return nil + }, + ) + if err != nil { + return nil, fmt.Errorf("associate user with account: %w", err) + } + return &user, nil } -// TODO (jimlambrt 6/2020) replace the raw query with a Lookup using the auth -// method repo. -func (r *Repository) validateAuthMethodId(ctx context.Context, scopeId, authMethodId string) (bool, error) { - if scopeId == "" { - return false, fmt.Errorf("scope id is unset: %w", db.ErrInvalidParameter) +// DissociateUserWithAccount will dissociate a user with its existing account. +// An error is returned if account is associated with a different user. No +// options are currently supported. +func (r *Repository) DissociateUserWithAccount(ctx context.Context, userPublicId, accountId string, opt ...Option) (*User, error) { + if userPublicId == "" { + return nil, fmt.Errorf("dissociate user with account: missing user public id %w", db.ErrInvalidParameter) } - if authMethodId == "" { - return false, fmt.Errorf("auth method id is unset: %w", db.ErrInvalidParameter) + if accountId == "" { + return nil, fmt.Errorf("dissociate user with account: missing account id %w", db.ErrInvalidParameter) } - db, err := r.reader.DB() + + user := allocUser() + user.PublicId = userPublicId + err := r.reader.LookupById(ctx, &user) if err != nil { - return false, err + return nil, fmt.Errorf("dissociate user with account: unable to lookup user %s: %w", userPublicId, err) } - var cnt int - if err := db.QueryRow(whereValidAuthMethod, authMethodId, scopeId).Scan(&cnt); err != nil { - return false, err + + acct := allocAccount() + acct.PublicId = accountId + err = r.reader.LookupByPublicId(ctx, &acct) + if err != nil { + return nil, fmt.Errorf("dissociate user with account: unable to lookup account %s: %w", accountId, err) } - if cnt == 0 { - return false, nil + // first, let's handle the case where the account is not associated + // with any user, so we're done! + if acct.IamUserId == "" { + return &user, nil } - return true, nil + // before proceeding with an update, is the account associated with the different user? + if acct.IamUserId != userPublicId { + return nil, fmt.Errorf("dissociate user with account: %s account is not associated with a user: %w", accountId, db.ErrInvalidParameter) + } + + // validate, dissociate the user with the account and then read the user back in + // the same tx for consistency. + _, err = r.writer.DoTx( + ctx, + db.StdRetryCnt, + db.ExpBackoff{}, + func(txReader db.Reader, w db.Writer) error { + metadata := oplog.Metadata{ + "resource-public-id": []string{accountId}, + "scope-id": []string{acct.ScopeId}, + "scope-type": []string{scope.Org.String()}, + "resource-type": []string{"auth-account"}, + } + updatedAcct := acct.Clone().(*authAccount) + updatedAcct.IamUserId = "" + // set the user id to null and use WithWhere to ensure that the auth + // account is associated with the user (handling race conditions + // with other concurrent transactions) + updatedRows, err := w.Update(ctx, updatedAcct, nil, []string{"IamUserId"}, db.WithOplog(r.wrapper, metadata), db.WithWhere("iam_user_id = ?", userPublicId)) + if err != nil { + return err + } + if updatedRows != 1 { + return fmt.Errorf("account update affected %d rows", updatedRows) + } + return nil + }, + ) + if err != nil { + return nil, fmt.Errorf("dissociate user with account: %w", err) + } + return &user, nil } diff --git a/internal/iam/repository_user_test.go b/internal/iam/repository_user_test.go index 9533e8c90..de2ccbd20 100644 --- a/internal/iam/repository_user_test.go +++ b/internal/iam/repository_user_test.go @@ -541,17 +541,16 @@ func TestRepository_LookupUserWithLogin(t *testing.T) { id := testId(t) org, _ := TestScopes(t, conn) authMethodId := testAuthMethod(t, conn, org.PublicId) - newAuthAcct := testAuthAccount(t, conn, org.PublicId, authMethodId, "") - newAuthAcctWithoutVivify := testAuthAccount(t, conn, org.PublicId, authMethodId, "") - // negativeTestAuthAcct := testAuthAccount(t, conn, org.PublicId, authMethodId, "") + newAuthAcct := testAccount(t, conn, org.PublicId, authMethodId, "") + newAuthAcctWithoutVivify := testAccount(t, conn, org.PublicId, authMethodId, "") user := TestUser(t, conn, org.PublicId, WithName("existing-"+id)) - existingAuthAcct := testAuthAccount(t, conn, org.PublicId, authMethodId, user.PublicId) + existingAuthAcct := testAccount(t, conn, org.PublicId, authMethodId, user.PublicId) require.Equal(t, user.PublicId, existingAuthAcct.IamUserId) type args struct { - withAuthAccountId string - opt []Option + withAccountId string + opt []Option } tests := []struct { name string @@ -565,7 +564,7 @@ func TestRepository_LookupUserWithLogin(t *testing.T) { { name: "valid", args: args{ - withAuthAccountId: newAuthAcct.PublicId, + withAccountId: newAuthAcct.PublicId, opt: []Option{ WithAutoVivify(true), WithName("valid-" + id), @@ -579,7 +578,7 @@ func TestRepository_LookupUserWithLogin(t *testing.T) { { name: "new-acct-without-vivify", args: args{ - withAuthAccountId: newAuthAcctWithoutVivify.PublicId, + withAccountId: newAuthAcctWithoutVivify.PublicId, }, wantErr: true, wantErrIs: db.ErrRecordNotFound, @@ -587,7 +586,7 @@ func TestRepository_LookupUserWithLogin(t *testing.T) { { name: "missing auth acct id", args: args{ - withAuthAccountId: "", + withAccountId: "", }, wantErr: true, wantErrIs: db.ErrInvalidParameter, @@ -595,7 +594,7 @@ func TestRepository_LookupUserWithLogin(t *testing.T) { { name: "existing-auth-account", args: args{ - withAuthAccountId: existingAuthAcct.PublicId, + withAccountId: existingAuthAcct.PublicId, }, wantErr: false, wantName: "existing-" + id, @@ -604,7 +603,7 @@ func TestRepository_LookupUserWithLogin(t *testing.T) { { name: "existing-auth-account-with-vivify", args: args{ - withAuthAccountId: existingAuthAcct.PublicId, + withAccountId: existingAuthAcct.PublicId, opt: []Option{ WithAutoVivify(true), }, @@ -616,7 +615,7 @@ func TestRepository_LookupUserWithLogin(t *testing.T) { { name: "bad-auth-account-id", args: args{ - withAuthAccountId: id, + withAccountId: id, }, wantErr: true, wantErrIs: db.ErrRecordNotFound, @@ -624,7 +623,7 @@ func TestRepository_LookupUserWithLogin(t *testing.T) { { name: "bad-auth-account-id-with-vivify", args: args{ - withAuthAccountId: id, + withAccountId: id, opt: []Option{ WithAutoVivify(true), }, @@ -637,17 +636,17 @@ func TestRepository_LookupUserWithLogin(t *testing.T) { t.Run(tt.name, func(t *testing.T) { assert, require := assert.New(t), require.New(t) dbassert := dbassert.New(t, rw) - got, err := repo.LookupUserWithLogin(context.Background(), tt.args.withAuthAccountId, tt.args.opt...) + got, err := repo.LookupUserWithLogin(context.Background(), tt.args.withAccountId, tt.args.opt...) if tt.wantErr { require.Error(err) assert.Nil(got) if tt.wantErrIs != nil { assert.Truef(errors.Is(err, tt.wantErrIs), "unexpected error %s", err.Error()) } - if tt.args.withAuthAccountId != "" && tt.args.withAuthAccountId != id { + if tt.args.withAccountId != "" && tt.args.withAccountId != id { // need to assert that userid in auth_account is still null - acct := allocAuthAccount() - acct.PublicId = tt.args.withAuthAccountId + acct := allocAccount() + acct.PublicId = tt.args.withAccountId dbassert.IsNull(&acct, "IamUserId") } return @@ -663,11 +662,287 @@ func TestRepository_LookupUserWithLogin(t *testing.T) { if tt.wantUser != nil { assert.True(proto.Equal(tt.wantUser.User, got.User)) } - acct := allocAuthAccount() - acct.PublicId = tt.args.withAuthAccountId + acct := allocAccount() + acct.PublicId = tt.args.withAccountId err = rw.LookupByPublicId(context.Background(), &acct) require.NoError(err) assert.Equal(got.PublicId, acct.IamUserId) }) } } + +func TestRepository_AssociateUserWithAccount(t *testing.T) { + t.Parallel() + conn, _ := db.TestSetup(t, "postgres") + rw := db.New(conn) + wrapper := db.TestWrapper(t) + repo, err := NewRepository(rw, rw, wrapper) + require.NoError(t, err) + org, _ := TestScopes(t, conn) + authMethodId := testAuthMethod(t, conn, org.PublicId) + + type Ids struct { + user string + acct string + } + type args struct { + Ids Ids + opt []Option + } + tests := []struct { + name string + args args + want *User + want1 *authAccount + wantErr bool + wantErrIs error + wantAssoc bool + }{ + { + name: "simple", + args: args{ + Ids: func() Ids { + u := TestUser(t, conn, org.PublicId) + a := testAccount(t, conn, org.PublicId, authMethodId, "") + return Ids{user: u.PublicId, acct: a.PublicId} + }(), + }, + }, + { + name: "missing-acctId", + args: args{ + Ids: func() Ids { + id := testId(t) + + return Ids{user: id} + }(), + }, + wantErr: true, + wantErrIs: db.ErrInvalidParameter, + }, + { + name: "missing-userId", + args: args{ + Ids: func() Ids { + id := testId(t) + return Ids{acct: id} + }(), + }, + wantErr: true, + wantErrIs: db.ErrInvalidParameter, + }, + { + name: "already-properly-assoc", + args: args{ + Ids: func() Ids { + u := TestUser(t, conn, org.PublicId) + a := testAccount(t, conn, org.PublicId, authMethodId, u.PublicId) + return Ids{user: u.PublicId, acct: a.PublicId} + }(), + opt: []Option{WithDisassociate(true)}, + }, + }, + { + name: "assoc-with-diff-user", + args: args{ + Ids: func() Ids { + u := TestUser(t, conn, org.PublicId) + diffUser := TestUser(t, conn, org.PublicId) + a := testAccount(t, conn, org.PublicId, authMethodId, diffUser.PublicId) + return Ids{user: u.PublicId, acct: a.PublicId} + }(), + }, + wantErr: true, + wantErrIs: db.ErrInvalidParameter, + }, + { + name: "assoc-with-diff-user-withDisassociateOption", + args: args{ + Ids: func() Ids { + u := TestUser(t, conn, org.PublicId) + diffUser := TestUser(t, conn, org.PublicId) + a := testAccount(t, conn, org.PublicId, authMethodId, diffUser.PublicId) + return Ids{user: u.PublicId, acct: a.PublicId} + }(), + }, + wantErr: true, + wantErrIs: db.ErrInvalidParameter, + }, + { + name: "bad-acct-id", + args: args{ + Ids: func() Ids { + u := TestUser(t, conn, org.PublicId) + id := testId(t) + return Ids{user: u.PublicId, acct: id} + }(), + }, + wantErr: true, + wantErrIs: db.ErrRecordNotFound, + }, + { + name: "bad-user-id", + args: args{ + Ids: func() Ids { + id := testId(t) + a := testAccount(t, conn, org.PublicId, authMethodId, "") + return Ids{user: id, acct: a.PublicId} + }(), + }, + wantErr: true, + wantErrIs: db.ErrRecordNotFound, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + u, err := repo.AssociateUserWithAccount(context.Background(), tt.args.Ids.user, tt.args.Ids.acct, tt.args.opt...) + if tt.wantErr { + require.Error(err) + assert.Empty(u) + if tt.wantErrIs != nil { + assert.Truef(errors.Is(err, tt.wantErrIs), "unexpected error %s", err.Error()) + } + return + } + require.NoError(err) + assert.Equal(tt.args.Ids.user, u.PublicId) + acct := allocAccount() + acct.PublicId = tt.args.Ids.acct + err = rw.LookupByPublicId(context.Background(), &acct) + require.NoError(err) + assert.Equal(tt.args.Ids.user, acct.IamUserId) + }) + } +} + +func TestRepository_DissociateUserWithAccount(t *testing.T) { + t.Parallel() + conn, _ := db.TestSetup(t, "postgres") + rw := db.New(conn) + wrapper := db.TestWrapper(t) + repo, err := NewRepository(rw, rw, wrapper) + require.NoError(t, err) + org, _ := TestScopes(t, conn) + authMethodId := testAuthMethod(t, conn, org.PublicId) + + type Ids struct { + user string + acct string + } + type args struct { + Ids Ids + opt []Option + } + tests := []struct { + name string + args args + want *User + want1 *authAccount + wantErr bool + wantErrIs error + }{ + { + name: "simple", + args: args{ + Ids: func() Ids { + u := TestUser(t, conn, org.PublicId) + a := testAccount(t, conn, org.PublicId, authMethodId, u.PublicId) + return Ids{user: u.PublicId, acct: a.PublicId} + }(), + }, + }, + { + name: "missing-acctId", + args: args{ + Ids: func() Ids { + id := testId(t) + + return Ids{user: id} + }(), + }, + wantErr: true, + wantErrIs: db.ErrInvalidParameter, + }, + { + name: "missing-userId", + args: args{ + Ids: func() Ids { + id := testId(t) + return Ids{acct: id} + }(), + }, + wantErr: true, + wantErrIs: db.ErrInvalidParameter, + }, + { + name: "already-properly-disassoc", + args: args{ + Ids: func() Ids { + u := TestUser(t, conn, org.PublicId) + a := testAccount(t, conn, org.PublicId, authMethodId, "") + return Ids{user: u.PublicId, acct: a.PublicId} + }(), + }, + }, + { + name: "assoc-with-diff-user", + args: args{ + Ids: func() Ids { + u := TestUser(t, conn, org.PublicId) + diffUser := TestUser(t, conn, org.PublicId) + a := testAccount(t, conn, org.PublicId, authMethodId, diffUser.PublicId) + return Ids{user: u.PublicId, acct: a.PublicId} + }(), + }, + wantErr: true, + wantErrIs: db.ErrInvalidParameter, + }, + { + name: "bad-acct-id", + args: args{ + Ids: func() Ids { + u := TestUser(t, conn, org.PublicId) + id := testId(t) + return Ids{user: u.PublicId, acct: id} + }(), + }, + wantErr: true, + wantErrIs: db.ErrRecordNotFound, + }, + { + name: "bad-user-id", + args: args{ + Ids: func() Ids { + id := testId(t) + a := testAccount(t, conn, org.PublicId, authMethodId, "") + return Ids{user: id, acct: a.PublicId} + }(), + }, + wantErr: true, + wantErrIs: db.ErrRecordNotFound, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + dbassert := dbassert.New(t, rw) + u, err := repo.DissociateUserWithAccount(context.Background(), tt.args.Ids.user, tt.args.Ids.acct, tt.args.opt...) + if tt.wantErr { + require.Error(err) + assert.Empty(u) + if tt.wantErrIs != nil { + assert.Truef(errors.Is(err, tt.wantErrIs), "unexpected error %s", err.Error()) + } + return + } + require.NoError(err) + assert.Equal(tt.args.Ids.user, u.PublicId) + + acct := allocAccount() + acct.PublicId = tt.args.Ids.acct + dbassert.IsNull(&acct, "IamUserId") + + }) + } +} diff --git a/internal/iam/store/auth_account.pb.go b/internal/iam/store/auth_account.pb.go deleted file mode 100644 index 1b8bd28ac..000000000 --- a/internal/iam/store/auth_account.pb.go +++ /dev/null @@ -1,199 +0,0 @@ -// Code generated by protoc-gen-go. DO NOT EDIT. -// versions: -// protoc-gen-go v1.25.0 -// protoc v3.12.4 -// source: controller/storage/iam/store/v1/auth_account.proto - -package store - -import ( - proto "github.com/golang/protobuf/proto" - _ "github.com/hashicorp/watchtower/internal/db/timestamp" - protoreflect "google.golang.org/protobuf/reflect/protoreflect" - protoimpl "google.golang.org/protobuf/runtime/protoimpl" - reflect "reflect" - sync "sync" -) - -const ( - // Verify that this generated code is sufficiently up-to-date. - _ = protoimpl.EnforceVersion(20 - protoimpl.MinVersion) - // Verify that runtime/protoimpl is sufficiently up-to-date. - _ = protoimpl.EnforceVersion(protoimpl.MaxVersion - 20) -) - -// This is a compile-time assertion that a sufficiently up-to-date version -// of the legacy proto package is being used. -const _ = proto.ProtoPackageIsVersion4 - -// AuthAccount is a "shared table" between the iam and auth subsystems. The -// auth system is responsible for creating entries. The iam system is only -// responsible/allowed to update the iam_user_id. -type AuthAccount struct { - state protoimpl.MessageState - sizeCache protoimpl.SizeCache - unknownFields protoimpl.UnknownFields - - // auth_account_id is primary key for the user account - // @inject_tag: gorm:"primary_key" - PublicId string `protobuf:"bytes,1,opt,name=public_id,json=publicId,proto3" json:"public_id,omitempty" gorm:"primary_key"` - // auth_method_id is the auth method associated with this user account. - // @inject_tag: `gorm:"default:null"` - AuthMethodId string `protobuf:"bytes,2,opt,name=auth_method_id,json=authMethodId,proto3" json:"auth_method_id,omitempty" gorm:"default:null"` - // scope_id for the user account - // @inject_tag: `gorm:"default:null"` - ScopeId string `protobuf:"bytes,4,opt,name=scope_id,json=scopeId,proto3" json:"scope_id,omitempty" gorm:"default:null"` - // user_id is the iam.user associated with this account - // @inject_tag: `gorm:"default:null"` - IamUserId string `protobuf:"bytes,5,opt,name=iam_user_id,json=iamUserId,proto3" json:"iam_user_id,omitempty" gorm:"default:null"` -} - -func (x *AuthAccount) Reset() { - *x = AuthAccount{} - if protoimpl.UnsafeEnabled { - mi := &file_controller_storage_iam_store_v1_auth_account_proto_msgTypes[0] - ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) - ms.StoreMessageInfo(mi) - } -} - -func (x *AuthAccount) String() string { - return protoimpl.X.MessageStringOf(x) -} - -func (*AuthAccount) ProtoMessage() {} - -func (x *AuthAccount) ProtoReflect() protoreflect.Message { - mi := &file_controller_storage_iam_store_v1_auth_account_proto_msgTypes[0] - 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 AuthAccount.ProtoReflect.Descriptor instead. -func (*AuthAccount) Descriptor() ([]byte, []int) { - return file_controller_storage_iam_store_v1_auth_account_proto_rawDescGZIP(), []int{0} -} - -func (x *AuthAccount) GetPublicId() string { - if x != nil { - return x.PublicId - } - return "" -} - -func (x *AuthAccount) GetAuthMethodId() string { - if x != nil { - return x.AuthMethodId - } - return "" -} - -func (x *AuthAccount) GetScopeId() string { - if x != nil { - return x.ScopeId - } - return "" -} - -func (x *AuthAccount) GetIamUserId() string { - if x != nil { - return x.IamUserId - } - return "" -} - -var File_controller_storage_iam_store_v1_auth_account_proto protoreflect.FileDescriptor - -var file_controller_storage_iam_store_v1_auth_account_proto_rawDesc = []byte{ - 0x0a, 0x32, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x6f, 0x6c, 0x6c, 0x65, 0x72, 0x2f, 0x73, 0x74, 0x6f, - 0x72, 0x61, 0x67, 0x65, 0x2f, 0x69, 0x61, 0x6d, 0x2f, 0x73, 0x74, 0x6f, 0x72, 0x65, 0x2f, 0x76, - 0x31, 0x2f, 0x61, 0x75, 0x74, 0x68, 0x5f, 0x61, 0x63, 0x63, 0x6f, 0x75, 0x6e, 0x74, 0x2e, 0x70, - 0x72, 0x6f, 0x74, 0x6f, 0x12, 0x1f, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x6f, 0x6c, 0x6c, 0x65, 0x72, - 0x2e, 0x73, 0x74, 0x6f, 0x72, 0x61, 0x67, 0x65, 0x2e, 0x69, 0x61, 0x6d, 0x2e, 0x73, 0x74, 0x6f, - 0x72, 0x65, 0x2e, 0x76, 0x31, 0x1a, 0x2f, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x6f, 0x6c, 0x6c, 0x65, - 0x72, 0x2f, 0x73, 0x74, 0x6f, 0x72, 0x61, 0x67, 0x65, 0x2f, 0x74, 0x69, 0x6d, 0x65, 0x73, 0x74, - 0x61, 0x6d, 0x70, 0x2f, 0x76, 0x31, 0x2f, 0x74, 0x69, 0x6d, 0x65, 0x73, 0x74, 0x61, 0x6d, 0x70, - 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x22, 0x8b, 0x01, 0x0a, 0x0b, 0x41, 0x75, 0x74, 0x68, 0x41, - 0x63, 0x63, 0x6f, 0x75, 0x6e, 0x74, 0x12, 0x1b, 0x0a, 0x09, 0x70, 0x75, 0x62, 0x6c, 0x69, 0x63, - 0x5f, 0x69, 0x64, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x08, 0x70, 0x75, 0x62, 0x6c, 0x69, - 0x63, 0x49, 0x64, 0x12, 0x24, 0x0a, 0x0e, 0x61, 0x75, 0x74, 0x68, 0x5f, 0x6d, 0x65, 0x74, 0x68, - 0x6f, 0x64, 0x5f, 0x69, 0x64, 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x52, 0x0c, 0x61, 0x75, 0x74, - 0x68, 0x4d, 0x65, 0x74, 0x68, 0x6f, 0x64, 0x49, 0x64, 0x12, 0x19, 0x0a, 0x08, 0x73, 0x63, 0x6f, - 0x70, 0x65, 0x5f, 0x69, 0x64, 0x18, 0x04, 0x20, 0x01, 0x28, 0x09, 0x52, 0x07, 0x73, 0x63, 0x6f, - 0x70, 0x65, 0x49, 0x64, 0x12, 0x1e, 0x0a, 0x0b, 0x69, 0x61, 0x6d, 0x5f, 0x75, 0x73, 0x65, 0x72, - 0x5f, 0x69, 0x64, 0x18, 0x05, 0x20, 0x01, 0x28, 0x09, 0x52, 0x09, 0x69, 0x61, 0x6d, 0x55, 0x73, - 0x65, 0x72, 0x49, 0x64, 0x42, 0x3a, 0x5a, 0x38, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, - 0x6f, 0x6d, 0x2f, 0x68, 0x61, 0x73, 0x68, 0x69, 0x63, 0x6f, 0x72, 0x70, 0x2f, 0x77, 0x61, 0x74, - 0x63, 0x68, 0x74, 0x6f, 0x77, 0x65, 0x72, 0x2f, 0x69, 0x6e, 0x74, 0x65, 0x72, 0x6e, 0x61, 0x6c, - 0x2f, 0x69, 0x61, 0x6d, 0x2f, 0x73, 0x74, 0x6f, 0x72, 0x65, 0x3b, 0x73, 0x74, 0x6f, 0x72, 0x65, - 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, -} - -var ( - file_controller_storage_iam_store_v1_auth_account_proto_rawDescOnce sync.Once - file_controller_storage_iam_store_v1_auth_account_proto_rawDescData = file_controller_storage_iam_store_v1_auth_account_proto_rawDesc -) - -func file_controller_storage_iam_store_v1_auth_account_proto_rawDescGZIP() []byte { - file_controller_storage_iam_store_v1_auth_account_proto_rawDescOnce.Do(func() { - file_controller_storage_iam_store_v1_auth_account_proto_rawDescData = protoimpl.X.CompressGZIP(file_controller_storage_iam_store_v1_auth_account_proto_rawDescData) - }) - return file_controller_storage_iam_store_v1_auth_account_proto_rawDescData -} - -var file_controller_storage_iam_store_v1_auth_account_proto_msgTypes = make([]protoimpl.MessageInfo, 1) -var file_controller_storage_iam_store_v1_auth_account_proto_goTypes = []interface{}{ - (*AuthAccount)(nil), // 0: controller.storage.iam.store.v1.AuthAccount -} -var file_controller_storage_iam_store_v1_auth_account_proto_depIdxs = []int32{ - 0, // [0:0] is the sub-list for method output_type - 0, // [0:0] is the sub-list for method input_type - 0, // [0:0] is the sub-list for extension type_name - 0, // [0:0] is the sub-list for extension extendee - 0, // [0:0] is the sub-list for field type_name -} - -func init() { file_controller_storage_iam_store_v1_auth_account_proto_init() } -func file_controller_storage_iam_store_v1_auth_account_proto_init() { - if File_controller_storage_iam_store_v1_auth_account_proto != nil { - return - } - if !protoimpl.UnsafeEnabled { - file_controller_storage_iam_store_v1_auth_account_proto_msgTypes[0].Exporter = func(v interface{}, i int) interface{} { - switch v := v.(*AuthAccount); 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{ - File: protoimpl.DescBuilder{ - GoPackagePath: reflect.TypeOf(x{}).PkgPath(), - RawDescriptor: file_controller_storage_iam_store_v1_auth_account_proto_rawDesc, - NumEnums: 0, - NumMessages: 1, - NumExtensions: 0, - NumServices: 0, - }, - GoTypes: file_controller_storage_iam_store_v1_auth_account_proto_goTypes, - DependencyIndexes: file_controller_storage_iam_store_v1_auth_account_proto_depIdxs, - MessageInfos: file_controller_storage_iam_store_v1_auth_account_proto_msgTypes, - }.Build() - File_controller_storage_iam_store_v1_auth_account_proto = out.File - file_controller_storage_iam_store_v1_auth_account_proto_rawDesc = nil - file_controller_storage_iam_store_v1_auth_account_proto_goTypes = nil - file_controller_storage_iam_store_v1_auth_account_proto_depIdxs = nil -} diff --git a/internal/iam/testing.go b/internal/iam/testing.go index da5ea50ef..7e635a288 100644 --- a/internal/iam/testing.go +++ b/internal/iam/testing.go @@ -5,9 +5,9 @@ import ( "testing" "github.com/hashicorp/go-uuid" + "github.com/hashicorp/watchtower/internal/auth/store" "github.com/hashicorp/watchtower/internal/db" dbassert "github.com/hashicorp/watchtower/internal/db/assert" - "github.com/hashicorp/watchtower/internal/iam/store" "github.com/jinzhu/gorm" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -191,12 +191,12 @@ func TestGroupRole(t *testing.T, conn *gorm.DB, roleId, grpId string, opt ...Opt return r } -// testAuthAccount is a temporary test function. TODO - replace with an auth -// subsystem testAuthAccount function. If userId is zero value, then an auth +// testAccount is a temporary test function. TODO - replace with an auth +// subsystem testAccount function. If userId is zero value, then an auth // account will be created with a null IamUserId -func testAuthAccount(t *testing.T, conn *gorm.DB, scopeId, authMethodId, userId string) *AuthAccount { +func testAccount(t *testing.T, conn *gorm.DB, scopeId, authMethodId, userId string) *authAccount { const ( - authAccountPrefix = "aa_" + accountPrefix = "aa_" ) t.Helper() rw := db.New(conn) @@ -218,11 +218,11 @@ func testAuthAccount(t *testing.T, conn *gorm.DB, scopeId, authMethodId, userId require.NoError(err) require.Equal(1, count) - id, err := db.NewPublicId(authAccountPrefix) + id, err := db.NewPublicId(accountPrefix) require.NoError(err) - acct := &AuthAccount{ - AuthAccount: &store.AuthAccount{ + acct := &authAccount{ + Account: &store.Account{ PublicId: id, ScopeId: scopeId, AuthMethodId: authMethodId, diff --git a/internal/proto/local/controller/storage/iam/store/v1/auth_account.proto b/internal/proto/local/controller/storage/auth/store/v1/account.proto similarity index 58% rename from internal/proto/local/controller/storage/iam/store/v1/auth_account.proto rename to internal/proto/local/controller/storage/auth/store/v1/account.proto index d203299d2..84b7a6444 100644 --- a/internal/proto/local/controller/storage/iam/store/v1/auth_account.proto +++ b/internal/proto/local/controller/storage/auth/store/v1/account.proto @@ -1,14 +1,15 @@ syntax = "proto3"; -package controller.storage.iam.store.v1; -option go_package = "github.com/hashicorp/watchtower/internal/iam/store;store"; +package controller.storage.auth.store.v1; +option go_package = "github.com/hashicorp/watchtower/internal/auth/store;store"; import "controller/storage/timestamp/v1/timestamp.proto"; -// AuthAccount is a "shared table" between the iam and auth subsystems. The -// auth system is responsible for creating entries. The iam system is only -// responsible/allowed to update the iam_user_id. -message AuthAccount { +// Account is a "shared table" between the iam, authtoken, and auth +// subsystems. The auth system is responsible for creating entries. The iam +// system is only responsible/allowed to update the iam_user_id. The authtoken +// system is a reader of this data. +message Account { // auth_account_id is primary key for the user account // @inject_tag: gorm:"primary_key" string public_id = 1;