diff --git a/CHANGELOG.md b/CHANGELOG.md index e841ecc14..e1ab0eea0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,20 @@ Canonical reference for changes, improvements, and bugfixes for Boundary. ## Next +### New and Improved + +* cli/api/sdk: New User's attributes for: + * Primary Account ID + * Login Name + * Full Name + * Email + + These new user attributes correspond to attributes + from the user's primary auth method account. These attributes will be empty + when the user has no account in the primary auth method for their scope, or + there is no designated primary auth method for their scope. + + ### Bug Fixes * cors: Fix allowing all origins by default diff --git a/api/users/user.gen.go b/api/users/user.gen.go index d9ba598aa..7473b817d 100644 --- a/api/users/user.gen.go +++ b/api/users/user.gen.go @@ -24,6 +24,10 @@ type User struct { AccountIds []string `json:"account_ids,omitempty"` Accounts []*Account `json:"accounts,omitempty"` AuthorizedActions []string `json:"authorized_actions,omitempty"` + LoginName string `json:"login_name,omitempty"` + FullName string `json:"full_name,omitempty"` + Email string `json:"email,omitempty"` + PrimaryAccountId string `json:"primary_account_id,omitempty"` response *api.Response } diff --git a/go.mod b/go.mod index 6431291ac..4ef4b3f18 100644 --- a/go.mod +++ b/go.mod @@ -4,8 +4,6 @@ go 1.16 replace github.com/hashicorp/boundary/api => ./api -replace github.com/hashicorp/boundary/sdk => ./sdk - require ( github.com/armon/go-metrics v0.3.6 github.com/bufbuild/buf v0.37.0 diff --git a/go.sum b/go.sum index 7ddf6854d..2d84224c3 100644 --- a/go.sum +++ b/go.sum @@ -436,6 +436,8 @@ github.com/grpc-ecosystem/grpc-gateway v1.9.5/go.mod h1:vNeuVxBJEsws4ogUvrchl83t github.com/grpc-ecosystem/grpc-gateway/v2 v2.3.0 h1:IvO4FbbQL6n3v3M1rQNobZ61SGL0gJLdvKA5KETM7Xs= github.com/grpc-ecosystem/grpc-gateway/v2 v2.3.0/go.mod h1:d2gYTOTUQklu06xp0AJYYmRdTVU1VKrqhkYfYag2L08= github.com/hailocab/go-hostpool v0.0.0-20160125115350-e80d13ce29ed/go.mod h1:tMWxXQ9wFIaZeTI9F+hmhFiGpFmhOHzyShyFUhRm0H4= +github.com/hashicorp/boundary/sdk v0.0.4 h1:l3uuXOL5ZQwYc63Bm8sLY36XGUlIN/4Q2r2SqkmGTyw= +github.com/hashicorp/boundary/sdk v0.0.4/go.mod h1:MQ8FJhseJrKpwvFDTn+AUAcJ1vdk1NKUxk7DxQgH7oQ= github.com/hashicorp/cap v0.0.0-20210408110729-0dd65efe5473 h1:qEikB8/ychrgsll9feuGjVLt0qo5oktfyTWi7BLsUrg= github.com/hashicorp/cap v0.0.0-20210408110729-0dd65efe5473/go.mod h1:hjMYKmFvk9AOL69NbqrahCHoMPZHRWqvRcdr7ciPU7g= github.com/hashicorp/consul/api v1.1.0/go.mod h1:VmuI/Lkw1nC05EYQWNKwWGbkg+FbDBtguAZLlVdkD9Q= diff --git a/internal/auth/password/argon2_test.go b/internal/auth/password/argon2_test.go index 6a316824f..a970e0345 100644 --- a/internal/auth/password/argon2_test.go +++ b/internal/auth/password/argon2_test.go @@ -323,7 +323,7 @@ func TestArgon2Credential_New(t *testing.T) { o, _ := iam.TestScopes(t, iam.TestRepo(t, conn, wrapper)) auts := TestAuthMethods(t, conn, o.GetPublicId(), 1) aut := auts[0] - accts := TestAccounts(t, conn, aut.PublicId, 5) + accts := TestMultipleAccounts(t, conn, aut.PublicId, 5) confs := testArgon2Confs(t, conn, accts[0].AuthMethodId, 1) type args struct { diff --git a/internal/auth/password/repository_account_test.go b/internal/auth/password/repository_account_test.go index 92803b54e..27eed3183 100644 --- a/internal/auth/password/repository_account_test.go +++ b/internal/auth/password/repository_account_test.go @@ -349,7 +349,7 @@ func TestRepository_LookupAccount(t *testing.T) { org, _ := iam.TestScopes(t, iamRepo) authMethod := TestAuthMethods(t, conn, org.GetPublicId(), 1)[0] - account := TestAccounts(t, conn, authMethod.GetPublicId(), 1)[0] + account := TestAccount(t, conn, authMethod.GetPublicId(), "name1") newAcctId, err := newAccountId() require.NoError(t, err) @@ -405,7 +405,7 @@ func TestRepository_DeleteAccount(t *testing.T) { org, _ := iam.TestScopes(t, iamRepo) authMethod := TestAuthMethods(t, conn, org.GetPublicId(), 1)[0] - account := TestAccounts(t, conn, authMethod.GetPublicId(), 1)[0] + account := TestAccount(t, conn, authMethod.GetPublicId(), "name1") newAcctId, err := newAccountId() require.NoError(t, err) @@ -462,8 +462,8 @@ func TestRepository_ListAccounts(t *testing.T) { org, _ := iam.TestScopes(t, iamRepo) authMethods := TestAuthMethods(t, conn, org.GetPublicId(), 3) - accounts1 := TestAccounts(t, conn, authMethods[0].GetPublicId(), 3) - accounts2 := TestAccounts(t, conn, authMethods[1].GetPublicId(), 4) + accounts1 := TestMultipleAccounts(t, conn, authMethods[0].GetPublicId(), 3) + accounts2 := TestMultipleAccounts(t, conn, authMethods[1].GetPublicId(), 4) _ = accounts2 tests := []struct { @@ -522,7 +522,7 @@ func TestRepository_ListAccounts_Limits(t *testing.T) { am := TestAuthMethods(t, conn, org.GetPublicId(), 1)[0] accountCount := 10 - _ = TestAccounts(t, conn, am.GetPublicId(), accountCount) + _ = TestMultipleAccounts(t, conn, am.GetPublicId(), accountCount) tests := []struct { name string @@ -982,7 +982,7 @@ func TestRepository_UpdateAccount_DupeNames(t *testing.T) { name := "test-dup-name" org, _ := iam.TestScopes(t, iamRepo) am := TestAuthMethods(t, conn, org.PublicId, 1)[0] - acts := TestAccounts(t, conn, am.PublicId, 2) + acts := TestMultipleAccounts(t, conn, am.PublicId, 2) aa := acts[0] ab := acts[1] @@ -1060,7 +1060,7 @@ func TestRepository_UpdateAccount_DupeNames(t *testing.T) { loginName := "kazmierczak12" org, _ := iam.TestScopes(t, iamRepo) am := TestAuthMethods(t, conn, org.PublicId, 1)[0] - acts := TestAccounts(t, conn, am.PublicId, 2) + acts := TestMultipleAccounts(t, conn, am.PublicId, 2) aa := acts[0] ab := acts[1] @@ -1137,8 +1137,8 @@ func TestRepository_UpdateAccount_DupeNames(t *testing.T) { ama := ams[0] amb := ams[1] - aa := TestAccounts(t, conn, ama.PublicId, 1)[0] - ab := TestAccounts(t, conn, amb.PublicId, 1)[0] + aa := TestMultipleAccounts(t, conn, ama.PublicId, 1)[0] + ab := TestMultipleAccounts(t, conn, amb.PublicId, 1)[0] assert.NotEqual(aa.AuthMethodId, ab.AuthMethodId) orig := aa.clone() diff --git a/internal/auth/password/testing.go b/internal/auth/password/testing.go index 7f533acdd..f81be1413 100644 --- a/internal/auth/password/testing.go +++ b/internal/auth/password/testing.go @@ -49,32 +49,42 @@ func TestAuthMethods(t *testing.T, conn *gorm.DB, scopeId string, count int) []* return auts } -// TestAccounts creates count number of password account to the provided DB +// TestMultipleAccounts creates count number of password account to the provided DB // with the provided auth method id. The auth method must have been created previously. // If any errors are encountered during the creation of the account, the test will fail. -func TestAccounts(t *testing.T, conn *gorm.DB, authMethodId string, count int) []*Account { +func TestMultipleAccounts(t *testing.T, conn *gorm.DB, authMethodId string, count int) []*Account { t.Helper() - assert, require := assert.New(t), require.New(t) - w := db.New(conn) var auts []*Account for i := 0; i < count; i++ { - cat, err := NewAccount(authMethodId, WithLoginName(fmt.Sprintf("name%d", i))) - assert.NoError(err) - require.NotNil(cat) - id, err := newAccountId() - assert.NoError(err) - require.NotEmpty(id) - cat.PublicId = id - - ctx := context.Background() - _, err2 := w.DoTx(ctx, db.StdRetryCnt, db.ExpBackoff{}, - func(_ db.Reader, iw db.Writer) error { - return iw.Create(ctx, cat) - }, - ) - - require.NoError(err2) - auts = append(auts, cat) + auts = append(auts, TestAccount(t, conn, authMethodId, fmt.Sprintf("name%d", i))) } return auts } + +// TestAccount creates a password account to the provided DB with the provided +// auth method id and loginName. The auth method must have been created +// previously. See password.NewAccount(...) for a list of supported options. +// If any errors are encountered during the creation of the account, the test will fail. +func TestAccount(t *testing.T, conn *gorm.DB, authMethodId, loginName string, opt ...Option) *Account { + t.Helper() + assert, require := assert.New(t), require.New(t) + require.NotEmpty(loginName) + w := db.New(conn) + opt = append(opt, WithLoginName(loginName)) + cat, err := NewAccount(authMethodId, opt...) + assert.NoError(err) + require.NotNil(cat) + id, err := newAccountId() + assert.NoError(err) + require.NotEmpty(id) + cat.PublicId = id + + ctx := context.Background() + _, err2 := w.DoTx(ctx, db.StdRetryCnt, db.ExpBackoff{}, + func(_ db.Reader, iw db.Writer) error { + return iw.Create(ctx, cat) + }, + ) + require.NoError(err2) + return cat +} diff --git a/internal/auth/password/testing_test.go b/internal/auth/password/testing_test.go index 02fa6d874..1acd2d8ce 100644 --- a/internal/auth/password/testing_test.go +++ b/internal/auth/password/testing_test.go @@ -25,7 +25,7 @@ func Test_TestAuthMethods(t *testing.T) { } } -func Test_TestAccounts(t *testing.T) { +func Test_TestMultipleAccounts(t *testing.T) { assert, require := assert.New(t), require.New(t) conn, _ := db.TestSetup(t, "postgres") wrapper := db.TestWrapper(t) @@ -37,7 +37,7 @@ func Test_TestAccounts(t *testing.T) { am := TestAuthMethods(t, conn, org.GetPublicId(), 1)[0] count := 4 - accounts := TestAccounts(t, conn, am.GetPublicId(), count) + accounts := TestMultipleAccounts(t, conn, am.GetPublicId(), count) assert.Len(accounts, count) for _, a := range accounts { assert.NotEmpty(a.GetPublicId()) diff --git a/internal/authtoken/authtoken_test.go b/internal/authtoken/authtoken_test.go index ab5c1ae5a..7f1df4279 100644 --- a/internal/authtoken/authtoken_test.go +++ b/internal/authtoken/authtoken_test.go @@ -29,7 +29,7 @@ func TestAuthToken_DbUpdate(t *testing.T) { org, _ := iam.TestScopes(t, iamRepo) am := password.TestAuthMethods(t, conn, org.GetPublicId(), 1)[0] - acct := password.TestAccounts(t, conn, am.GetPublicId(), 1)[0] + acct := password.TestAccount(t, conn, am.GetPublicId(), "name1") newAuthTokId, err := NewAuthTokenId() require.NoError(t, err) @@ -129,7 +129,7 @@ func TestAuthToken_DbCreate(t *testing.T) { kms := kms.TestKms(t, conn, wrapper) org, _ := iam.TestScopes(t, iam.TestRepo(t, conn, wrapper)) am := password.TestAuthMethods(t, conn, org.GetPublicId(), 1)[0] - acct := password.TestAccounts(t, conn, am.GetPublicId(), 1)[0] + acct := password.TestAccount(t, conn, am.GetPublicId(), "name1") createdAuthToken := TestAuthToken(t, conn, kms, org.GetPublicId()) testAuthTokenId := func() string { diff --git a/internal/authtoken/repository_test.go b/internal/authtoken/repository_test.go index 7afd53d10..3ad9f7ce5 100644 --- a/internal/authtoken/repository_test.go +++ b/internal/authtoken/repository_test.go @@ -188,7 +188,7 @@ func TestRepository_CreateAuthToken(t *testing.T) { org1, _ := iam.TestScopes(t, repo) am := password.TestAuthMethods(t, conn, org1.GetPublicId(), 1)[0] - aAcct := password.TestAccounts(t, conn, am.GetPublicId(), 1)[0] + aAcct := password.TestAccount(t, conn, am.GetPublicId(), "name1") iam.TestSetPrimaryAuthMethod(t, repo, org1, am.PublicId) u1 := iam.TestUser(t, repo, org1.PublicId, iam.WithAccountIds(aAcct.PublicId)) @@ -795,7 +795,7 @@ func Test_CloseExpiredPendingTokens(t *testing.T) { authMethods := password.TestAuthMethods(t, conn, org.PublicId, 1) authMethodId := authMethods[0].PublicId - accts := password.TestAccounts(t, conn, authMethodId, cnt) + accts := password.TestMultipleAccounts(t, conn, authMethodId, cnt) for i := 0; i < cnt; i++ { at := allocAuthToken() id, err := NewAuthTokenId() diff --git a/internal/authtoken/testing.go b/internal/authtoken/testing.go index f7df12caa..5f8f5fbb4 100644 --- a/internal/authtoken/testing.go +++ b/internal/authtoken/testing.go @@ -17,7 +17,7 @@ func TestAuthToken(t *testing.T, conn *gorm.DB, kms *kms.Kms, scopeId string, op authMethod := password.TestAuthMethods(t, conn, scopeId, 1)[0] // auth account is only used to join auth method to user. // We don't do anything else with the auth account in the test setup. - acct := password.TestAccounts(t, conn, authMethod.GetPublicId(), 1)[0] + acct := password.TestAccount(t, conn, authMethod.GetPublicId(), "name1") ctx := context.Background() rw := db.New(conn) diff --git a/internal/cmd/commands/userscmd/funcs.go b/internal/cmd/commands/userscmd/funcs.go index 3644f9040..fd1578811 100644 --- a/internal/cmd/commands/userscmd/funcs.go +++ b/internal/cmd/commands/userscmd/funcs.go @@ -179,6 +179,27 @@ func (c *Command) printListTable(items []*users.User) string { fmt.Sprintf(" Description: %s", u.Description), ) } + if u.PrimaryAccountId != "" { + output = append(output, + fmt.Sprintf(" Primary Account ID: %s", u.PrimaryAccountId), + ) + } + if u.LoginName != "" { + output = append(output, + fmt.Sprintf(" Login Name: %s", u.LoginName), + ) + } + if u.FullName != "" { + output = append(output, + fmt.Sprintf(" Full Name: %s", u.FullName), + ) + } + if u.Email != "" { + output = append(output, + fmt.Sprintf(" Email: %s", u.Email), + ) + } + if len(u.AuthorizedActions) > 0 { output = append(output, " Authorized Actions:", @@ -204,6 +225,18 @@ func printItemTable(in *users.User) string { if in.Description != "" { nonAttributeMap["Description"] = in.Description } + if in.PrimaryAccountId != "" { + nonAttributeMap["PrimaryAccountId"] = in.PrimaryAccountId + } + if in.LoginName != "" { + nonAttributeMap["LoginName"] = in.LoginName + } + if in.FullName != "" { + nonAttributeMap["FullName"] = in.FullName + } + if in.Email != "" { + nonAttributeMap["Email"] = in.Email + } maxLength := base.MaxAttributesLength(nonAttributeMap, nil, nil) diff --git a/internal/db/schema/migrations/postgres/3/01_auth.up.sql b/internal/db/schema/migrations/postgres/3/01_auth.up.sql new file mode 100644 index 000000000..2fd50e372 --- /dev/null +++ b/internal/db/schema/migrations/postgres/3/01_auth.up.sql @@ -0,0 +1,31 @@ +begin; + + + +-- this constraint is intended to ensure that a user cannot have more than one +-- account per auth_method. +-- +-- If this constraint causes the migrations to fail the operartor can run the +-- following query to get a list of user ids which have more that one account +-- within the same auth method. At that point, the operator will need to pick +-- which account to keep. +---------------------------------------------------------------------------- +-- with too_many_accounts(iam_user_id, acct_count) as ( +-- select +-- iam_user_id, +-- count(*) as acct_count +-- from +-- auth_account +-- group by auth_method_id, iam_user_id +-- ) +-- select +-- * +-- from +-- too_many_accounts +-- where +-- acct_count > 1; +alter table auth_account + add constraint auth_account_auth_method_id_public_id_uq + unique(auth_method_id, iam_user_id); + +commit; \ No newline at end of file diff --git a/internal/db/schema/migrations/postgres/3/02_iam.up.sql b/internal/db/schema/migrations/postgres/3/02_iam.up.sql new file mode 100644 index 000000000..8e1ee596c --- /dev/null +++ b/internal/db/schema/migrations/postgres/3/02_iam.up.sql @@ -0,0 +1,59 @@ +begin; + +drop view iam_user_acct_info; +drop view iam_acct_info; + +-- iam_user_acct_info provides account info for users by determining which +-- auth_method is designated as for "account info" in the user's scope via the +-- scope's primary_auth_method_id. Every sub-type of auth_account must be +-- added to this view's union. +create view iam_acct_info as +select + aa.iam_user_id, + oa.subject as login_name, + oa.public_id as primary_account_id, + oa.full_name as full_name, + oa.email as email +from + iam_scope s, + auth_account aa, + auth_oidc_account oa +where + aa.public_id = oa.public_id and + aa.auth_method_id = s.primary_auth_method_id +union +select + aa.iam_user_id, + pa.public_id as primary_account_id, + pa.login_name, + '' as full_name, + '' as email +from + iam_scope s, + auth_account aa, + auth_password_account pa +where + aa.public_id = pa.public_id and + aa.auth_method_id = s.primary_auth_method_id; + + +-- iam_user_acct_info provides a simple way to retrieve entries that include +-- both the iam_user fields with an outer join to the user's account info. +create view iam_user_acct_info as +select + u.public_id, + u.scope_id, + u.name, + u.description, + u.create_time, + u.update_time, + u.version, + i.primary_account_id, + i.login_name, + i.full_name, + i.email +from + iam_user u +left outer join iam_acct_info i on u.public_id = i.iam_user_id; + +commit; \ No newline at end of file diff --git a/internal/db/schema/postgres_migration.gen.go b/internal/db/schema/postgres_migration.gen.go index 8a86e7b35..106ac1d77 100644 --- a/internal/db/schema/postgres_migration.gen.go +++ b/internal/db/schema/postgres_migration.gen.go @@ -4,7 +4,7 @@ package schema func init() { migrationStates["postgres"] = migrationState{ - binarySchemaVersion: 2020, + binarySchemaVersion: 3002, upMigrations: map[int][]byte{ 1: []byte(` create domain wt_public_id as text @@ -5867,6 +5867,90 @@ from left outer join iam_scope s on am.public_id = s.primary_auth_method_id; comment on view auth_password_method_with_is_primary is 'password auth method with an is_primary_auth_method bool'; +`), + 3001: []byte(` +-- this constraint is intended to ensure that a user cannot have more than one +-- account per auth_method. +-- +-- If this constraint causes the migrations to fail the operartor can run the +-- following query to get a list of user ids which have more that one account +-- within the same auth method. At that point, the operator will need to pick +-- which account to keep. +---------------------------------------------------------------------------- +-- with too_many_accounts(iam_user_id, acct_count) as ( +-- select +-- iam_user_id, +-- count(*) as acct_count +-- from +-- auth_account +-- group by auth_method_id, iam_user_id +-- ) +-- select +-- * +-- from +-- too_many_accounts +-- where +-- acct_count > 1; +alter table auth_account + add constraint auth_account_auth_method_id_public_id_uq + unique(auth_method_id, iam_user_id); +`), + 3002: []byte(` +drop view iam_user_acct_info; +drop view iam_acct_info; + +-- iam_user_acct_info provides account info for users by determining which +-- auth_method is designated as for "account info" in the user's scope via the +-- scope's primary_auth_method_id. Every sub-type of auth_account must be +-- added to this view's union. +create view iam_acct_info as +select + aa.iam_user_id, + oa.subject as login_name, + oa.public_id as primary_account_id, + oa.full_name as full_name, + oa.email as email +from + iam_scope s, + auth_account aa, + auth_oidc_account oa +where + aa.public_id = oa.public_id and + aa.auth_method_id = s.primary_auth_method_id +union +select + aa.iam_user_id, + pa.public_id as primary_account_id, + pa.login_name, + '' as full_name, + '' as email +from + iam_scope s, + auth_account aa, + auth_password_account pa +where + aa.public_id = pa.public_id and + aa.auth_method_id = s.primary_auth_method_id; + + +-- iam_user_acct_info provides a simple way to retrieve entries that include +-- both the iam_user fields with an outer join to the user's account info. +create view iam_user_acct_info as +select + u.public_id, + u.scope_id, + u.name, + u.description, + u.create_time, + u.update_time, + u.version, + i.primary_account_id, + i.login_name, + i.full_name, + i.email +from + iam_user u +left outer join iam_acct_info i on u.public_id = i.iam_user_id; `), }, } diff --git a/internal/gen/controller.swagger.json b/internal/gen/controller.swagger.json index edd7c51f3..dc567010f 100644 --- a/internal/gen/controller.swagger.json +++ b/internal/gen/controller.swagger.json @@ -3643,6 +3643,26 @@ }, "description": "Output only. The available actions on this resource for this user.", "readOnly": true + }, + "login_name": { + "type": "string", + "title": "Output only. login_name is a string that maps to the user's account \"login\nname\" from the scope's primary auth method", + "readOnly": true + }, + "full_name": { + "type": "string", + "title": "Output only. full_name is a string that maps to the user's account name\nfrom the scope's primary auth method", + "readOnly": true + }, + "email": { + "type": "string", + "title": "Output only. email is a string that maps to the user's account email from\nthe scope's primary auth method", + "readOnly": true + }, + "primary_account_id": { + "type": "string", + "title": "Output only. primary_account_id is a string that maps to the user's account\npublic_id from the scope's primary auth method", + "readOnly": true } }, "title": "User contains all fields related to a User resource" diff --git a/internal/gen/controller/api/resources/users/user.pb.go b/internal/gen/controller/api/resources/users/user.pb.go index 1e4348214..cec01f031 100644 --- a/internal/gen/controller/api/resources/users/user.pb.go +++ b/internal/gen/controller/api/resources/users/user.pb.go @@ -110,6 +110,18 @@ type User struct { Accounts []*Account `protobuf:"bytes,100,rep,name=accounts,proto3" json:"accounts,omitempty"` // Output only. The available actions on this resource for this user. AuthorizedActions []string `protobuf:"bytes,300,rep,name=authorized_actions,proto3" json:"authorized_actions,omitempty"` + // Output only. login_name is a string that maps to the user's account "login + // name" from the scope's primary auth method + LoginName string `protobuf:"bytes,110,opt,name=login_name,proto3" json:"login_name,omitempty"` + // Output only. full_name is a string that maps to the user's account name + // from the scope's primary auth method + FullName string `protobuf:"bytes,120,opt,name=full_name,proto3" json:"full_name,omitempty"` + // Output only. email is a string that maps to the user's account email from + // the scope's primary auth method + Email string `protobuf:"bytes,130,opt,name=email,proto3" json:"email,omitempty"` + // Output only. primary_account_id is a string that maps to the user's account + // public_id from the scope's primary auth method + PrimaryAccountId string `protobuf:"bytes,140,opt,name=primary_account_id,proto3" json:"primary_account_id,omitempty"` } func (x *User) Reset() { @@ -221,6 +233,34 @@ func (x *User) GetAuthorizedActions() []string { return nil } +func (x *User) GetLoginName() string { + if x != nil { + return x.LoginName + } + return "" +} + +func (x *User) GetFullName() string { + if x != nil { + return x.FullName + } + return "" +} + +func (x *User) GetEmail() string { + if x != nil { + return x.Email + } + return "" +} + +func (x *User) GetPrimaryAccountId() string { + if x != nil { + return x.PrimaryAccountId + } + return "" +} + var File_controller_api_resources_users_v1_user_proto protoreflect.FileDescriptor var file_controller_api_resources_users_v1_user_proto_rawDesc = []byte{ @@ -242,7 +282,7 @@ var file_controller_api_resources_users_v1_user_proto_rawDesc = []byte{ 0x0a, 0x07, 0x41, 0x63, 0x63, 0x6f, 0x75, 0x6e, 0x74, 0x12, 0x0e, 0x0a, 0x02, 0x69, 0x64, 0x18, 0x0a, 0x20, 0x01, 0x28, 0x09, 0x52, 0x02, 0x69, 0x64, 0x12, 0x1a, 0x0a, 0x08, 0x73, 0x63, 0x6f, 0x70, 0x65, 0x5f, 0x69, 0x64, 0x18, 0x14, 0x20, 0x01, 0x28, 0x09, 0x52, 0x08, 0x73, 0x63, 0x6f, - 0x70, 0x65, 0x5f, 0x69, 0x64, 0x22, 0xd8, 0x04, 0x0a, 0x04, 0x55, 0x73, 0x65, 0x72, 0x12, 0x0e, + 0x70, 0x65, 0x5f, 0x69, 0x64, 0x22, 0xde, 0x05, 0x0a, 0x04, 0x55, 0x73, 0x65, 0x72, 0x12, 0x0e, 0x0a, 0x02, 0x69, 0x64, 0x18, 0x0a, 0x20, 0x01, 0x28, 0x09, 0x52, 0x02, 0x69, 0x64, 0x12, 0x1a, 0x0a, 0x08, 0x73, 0x63, 0x6f, 0x70, 0x65, 0x5f, 0x69, 0x64, 0x18, 0x14, 0x20, 0x01, 0x28, 0x09, 0x52, 0x08, 0x73, 0x63, 0x6f, 0x70, 0x65, 0x5f, 0x69, 0x64, 0x12, 0x43, 0x0a, 0x05, 0x73, 0x63, @@ -280,12 +320,21 @@ var file_controller_api_resources_users_v1_user_proto_rawDesc = []byte{ 0x2f, 0x0a, 0x12, 0x61, 0x75, 0x74, 0x68, 0x6f, 0x72, 0x69, 0x7a, 0x65, 0x64, 0x5f, 0x61, 0x63, 0x74, 0x69, 0x6f, 0x6e, 0x73, 0x18, 0xac, 0x02, 0x20, 0x03, 0x28, 0x09, 0x52, 0x12, 0x61, 0x75, 0x74, 0x68, 0x6f, 0x72, 0x69, 0x7a, 0x65, 0x64, 0x5f, 0x61, 0x63, 0x74, 0x69, 0x6f, 0x6e, 0x73, - 0x42, 0x51, 0x5a, 0x4f, 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, 0x67, 0x65, 0x6e, 0x2f, 0x63, - 0x6f, 0x6e, 0x74, 0x72, 0x6f, 0x6c, 0x6c, 0x65, 0x72, 0x2f, 0x61, 0x70, 0x69, 0x2f, 0x72, 0x65, - 0x73, 0x6f, 0x75, 0x72, 0x63, 0x65, 0x73, 0x2f, 0x75, 0x73, 0x65, 0x72, 0x73, 0x3b, 0x75, 0x73, - 0x65, 0x72, 0x73, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, + 0x12, 0x1e, 0x0a, 0x0a, 0x6c, 0x6f, 0x67, 0x69, 0x6e, 0x5f, 0x6e, 0x61, 0x6d, 0x65, 0x18, 0x6e, + 0x20, 0x01, 0x28, 0x09, 0x52, 0x0a, 0x6c, 0x6f, 0x67, 0x69, 0x6e, 0x5f, 0x6e, 0x61, 0x6d, 0x65, + 0x12, 0x1c, 0x0a, 0x09, 0x66, 0x75, 0x6c, 0x6c, 0x5f, 0x6e, 0x61, 0x6d, 0x65, 0x18, 0x78, 0x20, + 0x01, 0x28, 0x09, 0x52, 0x09, 0x66, 0x75, 0x6c, 0x6c, 0x5f, 0x6e, 0x61, 0x6d, 0x65, 0x12, 0x15, + 0x0a, 0x05, 0x65, 0x6d, 0x61, 0x69, 0x6c, 0x18, 0x82, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x05, + 0x65, 0x6d, 0x61, 0x69, 0x6c, 0x12, 0x2f, 0x0a, 0x12, 0x70, 0x72, 0x69, 0x6d, 0x61, 0x72, 0x79, + 0x5f, 0x61, 0x63, 0x63, 0x6f, 0x75, 0x6e, 0x74, 0x5f, 0x69, 0x64, 0x18, 0x8c, 0x01, 0x20, 0x01, + 0x28, 0x09, 0x52, 0x12, 0x70, 0x72, 0x69, 0x6d, 0x61, 0x72, 0x79, 0x5f, 0x61, 0x63, 0x63, 0x6f, + 0x75, 0x6e, 0x74, 0x5f, 0x69, 0x64, 0x42, 0x51, 0x5a, 0x4f, 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, 0x67, 0x65, 0x6e, 0x2f, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x6f, 0x6c, 0x6c, 0x65, 0x72, 0x2f, + 0x61, 0x70, 0x69, 0x2f, 0x72, 0x65, 0x73, 0x6f, 0x75, 0x72, 0x63, 0x65, 0x73, 0x2f, 0x75, 0x73, + 0x65, 0x72, 0x73, 0x3b, 0x75, 0x73, 0x65, 0x72, 0x73, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, + 0x33, } var ( diff --git a/internal/iam/query.go b/internal/iam/query.go index 56c0f92b4..665c9d907 100644 --- a/internal/iam/query.go +++ b/internal/iam/query.go @@ -6,12 +6,12 @@ package iam const ( // whereUserAccount - given an auth account id, return the associated user. whereUserAccount = ` - select iam_user.* - from iam_user + select iam_user_acct_info.* + from iam_user_acct_info inner join auth_account - on iam_user.public_id = auth_account.iam_user_id + on iam_user_acct_info.public_id = auth_account.iam_user_id where - iam_user.scope_id = auth_account.scope_id and + iam_user_acct_info.scope_id = auth_account.scope_id and auth_account.public_id = $1` // whereValidAuthMethod - determine if an auth method public_id within a scope_id diff --git a/internal/iam/repository_user.go b/internal/iam/repository_user.go index 392237bce..08562751b 100644 --- a/internal/iam/repository_user.go +++ b/internal/iam/repository_user.go @@ -39,6 +39,16 @@ func (r *Repository) CreateUser(ctx context.Context, user *User, opt ...Option) u.PublicId = id } + // There's no need to use r.lookupUser(...) here, because the new user cannot + // be associated with any accounts yet. Why would you typically want to + // call r.lookupUser(...) here vs returning the create resource? Well, the + // created resource doesn't include the user's primary account info (email, + // full name, etc), since you can't run DML against the view which does + // provide these output only attributes. But in this case, there's no way a + // newly created user could have any accounts, so we don't need to use + // r.lookupUser(...). I'm adding this comment so a future version of myself + // doesn't come along and decide to start using r.lookupUser(...) here which + // would just be an unnecessary database lookup. You're welcome future me. resource, err := r.create(ctx, u) if err != nil { if errors.IsUniqueError(err) { @@ -133,13 +143,15 @@ func (r *Repository) UpdateUser(ctx context.Context, user *User, version uint32, // return err, which will result in a rollback of the update return errors.New(errors.MultipleRecords, op, "more than 1 resource would have been updated") } - // we need a new repo, that's using the same reader/writer as this TxHandler txRepo := &Repository{ reader: reader, writer: w, kms: r.kms, - // intentionally not setting the defaultLimit, so we'll get all - // the account ids without a limit + // intentionally not setting the defaultLimit + } + returnedUser, err = txRepo.lookupUser(ctx, user.PublicId) + if err != nil { + return errors.Wrap(err, op, errors.WithMsg("unable to retrieve current user after update")) } currentAccountIds, err = txRepo.ListUserAccounts(ctx, user.PublicId) if err != nil { @@ -164,20 +176,15 @@ func (r *Repository) LookupUser(ctx context.Context, userId string, _ ...Option) if userId == "" { return nil, nil, errors.New(errors.InvalidParameter, op, "missing public id") } - - user := allocUser() - user.PublicId = userId - if err := r.reader.LookupByPublicId(ctx, &user); err != nil { - if errors.IsNotFoundError(err) { - return nil, nil, nil - } - return nil, nil, errors.Wrap(err, op, errors.WithMsg(fmt.Sprintf("for %s", userId))) + user, err := r.lookupUser(ctx, userId) + if err != nil { + return nil, nil, errors.Wrap(err, op) } currentAccountIds, err := r.ListUserAccounts(ctx, userId) if err != nil { return nil, nil, errors.Wrap(err, op, errors.WithMsg("unable to retrieve current account ids")) } - return &user, currentAccountIds, nil + return user, currentAccountIds, nil } // DeleteUser will delete a user from the repository @@ -186,7 +193,7 @@ func (r *Repository) DeleteUser(ctx context.Context, withPublicId string, _ ...O if withPublicId == "" { return db.NoRowsAffected, errors.New(errors.InvalidParameter, op, "missing public id") } - user := allocUser() + user := AllocUser() user.PublicId = withPublicId if err := r.reader.LookupByPublicId(ctx, &user); err != nil { return db.NoRowsAffected, errors.Wrap(err, op, errors.WithMsg(fmt.Sprintf("for %s", withPublicId))) @@ -204,8 +211,7 @@ func (r *Repository) ListUsers(ctx context.Context, withScopeIds []string, opt . if len(withScopeIds) == 0 { return nil, errors.New(errors.InvalidParameter, op, "missing scope id") } - var users []*User - err := r.list(ctx, &users, "scope_id in (?)", []interface{}{withScopeIds}, opt...) + users, err := r.getUsers(ctx, "", withScopeIds, opt...) if err != nil { return nil, errors.Wrap(err, op) } @@ -266,7 +272,7 @@ func (r *Repository) LookupUserWithLogin(ctx context.Context, accountId string, ctx, db.StdRetryCnt, db.ExpBackoff{}, - func(_ db.Reader, w db.Writer) error { + func(reader db.Reader, w db.Writer) error { msgs := make([]*oplog.Message, 0, 2) ticket, err := w.GetTicket(&acct) if err != nil { @@ -302,6 +308,16 @@ func (r *Repository) LookupUserWithLogin(ctx context.Context, accountId string, if err := w.WriteOplogEntryWith(ctx, oplogWrapper, ticket, metadata, msgs); err != nil { return errors.Wrap(err, op) } + txRepo := &Repository{ + reader: reader, + writer: w, + kms: r.kms, + // intentionally not setting the defaultLimit + } + obtainedUser, err = txRepo.lookupUser(ctx, obtainedUser.PublicId) + if err != nil { + return errors.Wrap(err, op, errors.WithMsg("unable to retrieve user")) + } return nil }, ) @@ -336,7 +352,7 @@ func (r *Repository) getUserWithAccount(ctx context.Context, withAccountId strin return nil, errors.Wrap(err, op, errors.WithMsg(fmt.Sprintf("unable to query account %s", withAccountId))) } defer rows.Close() - u := allocUser() + u := AllocUser() if rows.Next() { err = r.reader.ScanRows(rows, &u) if err != nil { @@ -388,10 +404,7 @@ func (r *Repository) AddUserAccounts(ctx context.Context, userId string, userVer return nil, errors.New(errors.InvalidParameter, op, "missing account ids") } - user := allocUser() - user.PublicId = userId - - err := r.reader.LookupById(ctx, &user) + user, err := r.lookupUser(ctx, userId) if err != nil { return nil, errors.Wrap(err, op, errors.WithMsg(fmt.Sprintf("unable to lookup user %s", userId))) } @@ -406,11 +419,11 @@ func (r *Repository) AddUserAccounts(ctx context.Context, userId string, userVer db.StdRetryCnt, db.ExpBackoff{}, func(reader db.Reader, w db.Writer) error { - userTicket, err := w.GetTicket(&user) + userTicket, err := w.GetTicket(user) if err != nil { return errors.Wrap(err, op, errors.WithMsg("unable to get ticket")) } - updatedUser := allocUser() + updatedUser := AllocUser() updatedUser.PublicId = userId updatedUser.Version = userVersion + 1 var userOplogMsg oplog.Message @@ -470,9 +483,7 @@ func (r *Repository) DeleteUserAccounts(ctx context.Context, userId string, user return nil, errors.New(errors.InvalidParameter, op, "missing account ids") } - user := allocUser() - user.PublicId = userId - err := r.reader.LookupById(ctx, &user) + user, err := r.lookupUser(ctx, userId) if err != nil { return nil, errors.Wrap(err, op, errors.WithMsg(fmt.Sprintf("unable to lookup user %s", userId))) } @@ -487,11 +498,11 @@ func (r *Repository) DeleteUserAccounts(ctx context.Context, userId string, user db.StdRetryCnt, db.ExpBackoff{}, func(reader db.Reader, w db.Writer) error { - userTicket, err := w.GetTicket(&user) + userTicket, err := w.GetTicket(user) if err != nil { return errors.Wrap(err, op, errors.WithMsg("unable to get ticket")) } - updatedUser := allocUser() + updatedUser := AllocUser() updatedUser.PublicId = userId updatedUser.Version = userVersion + 1 var userOplogMsg oplog.Message @@ -548,9 +559,7 @@ func (r *Repository) SetUserAccounts(ctx context.Context, userId string, userVer return nil, errors.New(errors.InvalidParameter, op, "missing version") } - user := allocUser() - user.PublicId = userId - err := r.reader.LookupById(ctx, &user) + user, err := r.lookupUser(ctx, userId) if err != nil { return nil, errors.Wrap(err, op, errors.WithMsg(fmt.Sprintf("unable to lookup user %s", userId))) } @@ -585,11 +594,11 @@ func (r *Repository) SetUserAccounts(ctx context.Context, userId string, userVer } return nil } - userTicket, err := w.GetTicket(&user) + userTicket, err := w.GetTicket(user) if err != nil { return errors.Wrap(err, op, errors.WithMsg("unable to get ticket")) } - updatedUser := allocUser() + updatedUser := AllocUser() updatedUser.PublicId = userId updatedUser.Version = userVersion + 1 var userOplogMsg oplog.Message @@ -666,7 +675,7 @@ func associateUserWithAccounts(ctx context.Context, repoKms *kms.Kms, reader db. for _, accountId := range accountIds { acct := allocAccount() acct.PublicId = accountId - err := reader.LookupByPublicId(context.Background(), &acct) + err := reader.LookupByPublicId(ctx, &acct) if err != nil { return errors.Wrap(err, op, errors.WithMsg(fmt.Sprintf("unable to lookup account %s", accountId))) } @@ -823,3 +832,67 @@ func associationChanges(ctx context.Context, reader db.Reader, userId string, ac } return associateIds, disassociateIds, nil } + +// lookupUser will lookup a single user and returns nil, nil when no user is found. +func (r *Repository) lookupUser(ctx context.Context, userId string, opt ...Option) (*User, error) { + const op = "iam.(Repository).lookupUser" + users, err := r.getUsers(ctx, userId, nil, opt...) + if err != nil { + return nil, errors.Wrap(err, op) + } + switch { + case len(users) == 0: + return nil, nil // not an error to return no rows for a lookup + case len(users) > 1: + return nil, errors.New(errors.NotSpecificIntegrity, op, fmt.Sprintf("%s matched more than 1 ", userId)) + default: + return users[0], nil + } +} + +// getUsers allows the caller to specify to either lookup a specific User via +// its ID or search for a set of Users within a set of scopes. Passing both +// scopeIds and a userId is an error. The WithLimit option is supported and all +// other options are ignored. +// +// When no record is found then it returns nil, nil +func (r *Repository) getUsers(ctx context.Context, userId string, scopeIds []string, opt ...Option) ([]*User, error) { + const op = "iam.(Repository).getUsers" + if userId == "" && len(scopeIds) == 0 { + return nil, errors.New(errors.InvalidParameter, op, "missing search criteria: both user id and scope ids are empty") + } + if userId != "" && len(scopeIds) > 0 { + return nil, errors.New(errors.InvalidParameter, op, "searching for both a specific user id and scope ids is not supported") + } + + opts := getOpts(opt...) + dbArgs := []db.Option{} + limit := r.defaultLimit + if opts.withLimit != 0 { + // non-zero signals an override of the default limit for the repo. + limit = opts.withLimit + } + dbArgs = append(dbArgs, db.WithLimit(limit)) + + var args []interface{} + var where []string + switch { + case userId != "": + where, args = append(where, "public_id = ?"), append(args, userId) + default: + where, args = append(where, "scope_id in(?)"), append(args, scopeIds) + } + var usersAcctInfo []*userAccountInfo + err := r.reader.SearchWhere(ctx, &usersAcctInfo, strings.Join(where, " and "), args, dbArgs...) + if err != nil { + return nil, errors.Wrap(err, op) + } + if len(usersAcctInfo) == 0 { // we're done if nothing is found. + return nil, nil + } + users := make([]*User, 0, len(usersAcctInfo)) + for _, u := range usersAcctInfo { + users = append(users, u.shallowConversion()) + } + return users, nil +} diff --git a/internal/iam/repository_user_iam_pkg_test.go b/internal/iam/repository_user_iam_pkg_test.go new file mode 100644 index 000000000..3abb3c562 --- /dev/null +++ b/internal/iam/repository_user_iam_pkg_test.go @@ -0,0 +1,328 @@ +package iam + +import ( + "context" + "testing" + + "github.com/hashicorp/boundary/internal/db" + dbassert "github.com/hashicorp/boundary/internal/db/assert" + "github.com/hashicorp/boundary/internal/errors" + "github.com/hashicorp/boundary/internal/kms" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestRepository_associateUserWithAccounts(t *testing.T) { + t.Parallel() + conn, _ := db.TestSetup(t, "postgres") + rw := db.New(conn) + wrapper := db.TestWrapper(t) + kms := kms.TestKms(t, conn, wrapper) + repo := TestRepo(t, conn, wrapper) + org, _ := TestScopes(t, repo) + authMethodId := testAuthMethod(t, conn, org.PublicId) + authMethodId2 := testAuthMethod(t, conn, org.PublicId) + + type Ids struct { + user string + accts []string + } + type args struct { + Ids Ids + opt []Option + } + tests := []struct { + name string + args args + want *User + want1 *authAccount + wantErr bool + wantErrCode errors.Code + wantAssoc bool + }{ + { + name: "simple", + args: args{ + Ids: func() Ids { + u := TestUser(t, repo, org.PublicId) + a := testAccount(t, conn, org.PublicId, authMethodId, "") + a2 := testAccount(t, conn, org.PublicId, authMethodId2, "") + return Ids{user: u.PublicId, accts: []string{a.PublicId, a2.PublicId}} + }(), + }, + }, + { + name: "two-accounts", + args: args{ + Ids: func() Ids { + u := TestUser(t, repo, org.PublicId) + a := testAccount(t, conn, org.PublicId, authMethodId, "") + a2 := testAccount(t, conn, org.PublicId, authMethodId, "") + return Ids{user: u.PublicId, accts: []string{a.PublicId, a2.PublicId}} + }(), + }, + wantErr: true, + wantErrCode: errors.NotUnique, + }, + { + name: "missing-acctId", + args: args{ + Ids: func() Ids { + id := testId(t) + + return Ids{user: id} + }(), + }, + wantErr: true, + wantErrCode: errors.InvalidParameter, + }, + { + name: "missing-userId", + args: args{ + Ids: func() Ids { + id := testId(t) + return Ids{accts: []string{id}} + }(), + }, + wantErr: true, + wantErrCode: errors.InvalidParameter, + }, + { + name: "already-properly-assoc", + args: args{ + Ids: func() Ids { + u := TestUser(t, repo, org.PublicId) + a := testAccount(t, conn, org.PublicId, authMethodId, u.PublicId) + return Ids{user: u.PublicId, accts: []string{a.PublicId}} + }(), + opt: []Option{WithDisassociate(true)}, + }, + }, + { + name: "assoc-with-diff-user", + args: args{ + Ids: func() Ids { + u := TestUser(t, repo, org.PublicId) + diffUser := TestUser(t, repo, org.PublicId) + a := testAccount(t, conn, org.PublicId, authMethodId, diffUser.PublicId) + return Ids{user: u.PublicId, accts: []string{a.PublicId}} + }(), + }, + wantErr: true, + wantErrCode: errors.AccountAlreadyAssociated, + }, + { + name: "assoc-with-diff-user-withDisassociateOption", + args: args{ + Ids: func() Ids { + u := TestUser(t, repo, org.PublicId) + diffUser := TestUser(t, repo, org.PublicId) + a := testAccount(t, conn, org.PublicId, authMethodId, diffUser.PublicId) + return Ids{user: u.PublicId, accts: []string{a.PublicId}} + }(), + }, + wantErr: true, + wantErrCode: errors.AccountAlreadyAssociated, + }, + { + name: "bad-acct-id", + args: args{ + Ids: func() Ids { + u := TestUser(t, repo, org.PublicId) + id := testId(t) + return Ids{user: u.PublicId, accts: []string{id}} + }(), + }, + wantErr: true, + wantErrCode: errors.RecordNotFound, + }, + { + name: "bad-user-id-not-associated-account", + args: args{ + Ids: func() Ids { + id := testId(t) + a := testAccount(t, conn, org.PublicId, authMethodId, "") + return Ids{user: id, accts: []string{a.PublicId}} + }(), + }, + wantErr: false, + }, + { + name: "bad-user-id", + args: args{ + Ids: func() Ids { + id := testId(t) + testUser := TestUser(t, repo, org.PublicId) + a := testAccount(t, conn, org.PublicId, authMethodId, testUser.PublicId) + return Ids{user: id, accts: []string{a.PublicId}} + }(), + }, + wantErr: true, + wantErrCode: errors.AccountAlreadyAssociated, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + err := associateUserWithAccounts(context.Background(), kms, rw, rw, tt.args.Ids.user, tt.args.Ids.accts, tt.args.opt...) + if tt.wantErr { + require.Error(err) + assert.Truef(errors.Match(errors.T(tt.wantErrCode), err), "unexpected error %s", err) + return + } + require.NoError(err) + for _, id := range tt.args.Ids.accts { + acct := allocAccount() + acct.PublicId = id + 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) + kms := kms.TestKms(t, conn, wrapper) + repo := TestRepo(t, conn, wrapper) + org, _ := TestScopes(t, repo) + authMethodId := testAuthMethod(t, conn, org.PublicId) + authMethodId2 := testAuthMethod(t, conn, org.PublicId) + + type Ids struct { + user string + accts []string + } + type args struct { + Ids Ids + opt []Option + } + tests := []struct { + name string + args args + want *User + want1 *authAccount + wantErr bool + wantErrCode errors.Code + }{ + { + name: "simple", + args: args{ + Ids: func() Ids { + u := TestUser(t, repo, org.PublicId) + a := testAccount(t, conn, org.PublicId, authMethodId, u.PublicId) + a2 := testAccount(t, conn, org.PublicId, authMethodId2, u.PublicId) + return Ids{user: u.PublicId, accts: []string{a.PublicId, a2.PublicId}} + }(), + }, + }, + { + name: "missing-acctId", + args: args{ + Ids: func() Ids { + id := testId(t) + + return Ids{user: id} + }(), + }, + wantErr: true, + wantErrCode: errors.InvalidParameter, + }, + { + name: "missing-userId", + args: args{ + Ids: func() Ids { + id := testId(t) + return Ids{accts: []string{id}} + }(), + }, + wantErr: true, + wantErrCode: errors.InvalidParameter, + }, + { + name: "already-properly-disassoc", + args: args{ + Ids: func() Ids { + u := TestUser(t, repo, org.PublicId) + a := testAccount(t, conn, org.PublicId, authMethodId, "") + return Ids{user: u.PublicId, accts: []string{a.PublicId}} + }(), + }, + wantErr: true, + wantErrCode: errors.AccountAlreadyAssociated, + }, + { + name: "assoc-with-diff-user", + args: args{ + Ids: func() Ids { + u := TestUser(t, repo, org.PublicId) + diffUser := TestUser(t, repo, org.PublicId) + a := testAccount(t, conn, org.PublicId, authMethodId, diffUser.PublicId) + return Ids{user: u.PublicId, accts: []string{a.PublicId}} + }(), + }, + wantErr: true, + wantErrCode: errors.AccountAlreadyAssociated, + }, + { + name: "bad-acct-id", + args: args{ + Ids: func() Ids { + u := TestUser(t, repo, org.PublicId) + id := testId(t) + return Ids{user: u.PublicId, accts: []string{id}} + }(), + }, + wantErr: true, + wantErrCode: errors.RecordNotFound, + }, + { + name: "bad-user-id-not-associated-account", + args: args{ + Ids: func() Ids { + id := testId(t) + a := testAccount(t, conn, org.PublicId, authMethodId, "") + return Ids{user: id, accts: []string{a.PublicId}} + }(), + }, + wantErr: true, + wantErrCode: errors.AccountAlreadyAssociated, + }, + { + name: "bad-user-id", + args: args{ + Ids: func() Ids { + id := testId(t) + testUser := TestUser(t, repo, org.PublicId) + a := testAccount(t, conn, org.PublicId, authMethodId, testUser.PublicId) + return Ids{user: id, accts: []string{a.PublicId}} + }(), + }, + wantErr: true, + wantErrCode: errors.AccountAlreadyAssociated, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + dbassert := dbassert.New(t, conn.DB()) + err := dissociateUserFromAccounts(context.Background(), kms, rw, rw, tt.args.Ids.user, tt.args.Ids.accts, tt.args.opt...) + if tt.wantErr { + require.Error(err) + assert.Truef(errors.Match(errors.T(tt.wantErrCode), err), "unexpected error %s", err) + return + } + require.NoError(err) + + for _, id := range tt.args.Ids.accts { + acct := allocAccount() + acct.PublicId = id + dbassert.IsNull(&acct, "IamUserId") + } + }) + } +} diff --git a/internal/iam/repository_user_test.go b/internal/iam/repository_user_test.go index 8d7b0ba44..0621b92d0 100644 --- a/internal/iam/repository_user_test.go +++ b/internal/iam/repository_user_test.go @@ -1,4 +1,4 @@ -package iam +package iam_test import ( "context" @@ -7,15 +7,22 @@ import ( "testing" "time" + "github.com/google/go-cmp/cmp" + "github.com/hashicorp/boundary/internal/auth/oidc" + "github.com/hashicorp/boundary/internal/auth/password" + "github.com/hashicorp/boundary/internal/auth/store" "github.com/hashicorp/boundary/internal/db" dbassert "github.com/hashicorp/boundary/internal/db/assert" "github.com/hashicorp/boundary/internal/errors" + "github.com/hashicorp/boundary/internal/iam" "github.com/hashicorp/boundary/internal/kms" "github.com/hashicorp/boundary/internal/oplog" "github.com/hashicorp/boundary/sdk/strutil" + "github.com/hashicorp/go-uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/testing/protocmp" ) func TestRepository_CreateUser(t *testing.T) { @@ -23,13 +30,13 @@ func TestRepository_CreateUser(t *testing.T) { conn, _ := db.TestSetup(t, "postgres") rw := db.New(conn) wrapper := db.TestWrapper(t) - repo := TestRepo(t, conn, wrapper) + repo := iam.TestRepo(t, conn, wrapper) id := testId(t) - org, _ := TestScopes(t, repo) + org, _ := iam.TestScopes(t, repo) type args struct { - user *User - opt []Option + user *iam.User + opt []iam.Option } tests := []struct { name string @@ -41,8 +48,8 @@ func TestRepository_CreateUser(t *testing.T) { { name: "valid", args: args{ - user: func() *User { - u, err := NewUser(org.PublicId, WithName("valid"+id), WithDescription(id)) + user: func() *iam.User { + u, err := iam.NewUser(org.PublicId, iam.WithName("valid"+id), iam.WithDescription(id)) assert.NoError(t, err) return u }(), @@ -52,8 +59,8 @@ func TestRepository_CreateUser(t *testing.T) { { name: "bad-scope-id", args: args{ - user: func() *User { - u, err := NewUser(id) + user: func() *iam.User { + u, err := iam.NewUser(id) assert.NoError(t, err) return u }(), @@ -64,8 +71,8 @@ func TestRepository_CreateUser(t *testing.T) { { name: "dup-name", args: args{ - user: func() *User { - u, err := NewUser(org.PublicId, WithName("dup-name"+id)) + user: func() *iam.User { + u, err := iam.NewUser(org.PublicId, iam.WithName("dup-name"+id)) assert.NoError(t, err) return u }(), @@ -114,30 +121,32 @@ func TestRepository_UpdateUser(t *testing.T) { conn, _ := db.TestSetup(t, "postgres") rw := db.New(conn) wrapper := db.TestWrapper(t) - repo := TestRepo(t, conn, wrapper) + kmsCache := kms.TestKms(t, conn, wrapper) + repo := iam.TestRepo(t, conn, wrapper) id := testId(t) - org, proj := TestScopes(t, repo) - authMethodId := testAuthMethod(t, conn, org.PublicId) + org, proj := iam.TestScopes(t, repo) + databaseWrapper, err := kmsCache.GetWrapper(context.Background(), org.PublicId, kms.KeyPurposeDatabase) + require.NoError(t, err) + pubId := func(s string) *string { return &s } type args struct { name string description string fieldMaskPaths []string - opt []Option + opt []iam.Option ScopeId string PublicId *string } tests := []struct { name string - newUserOpts []Option + newUserOpts []iam.Option args args wantRowsUpdate int wantErr bool wantErrMsg string wantIsErr errors.Code wantDup bool - directUpdate bool }{ { name: "valid", @@ -156,7 +165,7 @@ func TestRepository_UpdateUser(t *testing.T) { fieldMaskPaths: []string{"Name"}, ScopeId: org.PublicId, }, - newUserOpts: []Option{WithName("valid-no-op" + id)}, + newUserOpts: []iam.Option{iam.WithName("valid-no-op" + id)}, wantErr: false, wantRowsUpdate: 1, }, @@ -180,7 +189,7 @@ func TestRepository_UpdateUser(t *testing.T) { fieldMaskPaths: []string{"Name"}, ScopeId: org.PublicId, }, - newUserOpts: []Option{WithName("null-name" + id)}, + newUserOpts: []iam.Option{iam.WithName("null-name" + id)}, wantErr: false, wantRowsUpdate: 1, }, @@ -191,7 +200,7 @@ func TestRepository_UpdateUser(t *testing.T) { fieldMaskPaths: []string{"Description"}, ScopeId: org.PublicId, }, - newUserOpts: []Option{WithDescription("null-description" + id)}, + newUserOpts: []iam.Option{iam.WithDescription("null-description" + id)}, wantErr: false, wantRowsUpdate: 1, }, @@ -282,39 +291,66 @@ func TestRepository_UpdateUser(t *testing.T) { wantDup: true, wantErrMsg: `iam.(Repository).UpdateUser: user %s already exists in org %s: integrity violation: error #1002`, }, - { - name: "modified-scope", - args: args{ - name: "modified-scope" + id, - fieldMaskPaths: []string{"ScopeId"}, - ScopeId: "global", - opt: []Option{WithSkipVetForWrite(true)}, - }, - wantErr: true, - wantErrMsg: `iam.(Repository).update: db.DoTx: iam.(Repository).update: db.Update: immutable column: iam_user.scope_id: integrity violation: error #1003`, - directUpdate: true, - }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { assert, require := assert.New(t), require.New(t) + // we need to clean out any auth methods + _, err = rw.Exec(context.Background(), "delete from auth_method", nil) + require.NoError(err) + if tt.wantDup { - u := TestUser(t, repo, org.PublicId, tt.newUserOpts...) + u := iam.TestUser(t, repo, org.PublicId, tt.newUserOpts...) u.Name = tt.args.name _, _, _, err := repo.UpdateUser(context.Background(), u, 1, tt.args.fieldMaskPaths, tt.args.opt...) require.NoError(err) } - u := TestUser(t, repo, org.PublicId, tt.newUserOpts...) - acctCount := 3 + u := iam.TestUser(t, repo, org.PublicId, tt.newUserOpts...) + acctCount := 10 accountIds := make([]string, 0, acctCount) + var wantEmail, wantFullName, wantPrimaryAccountId string + var authMethod *oidc.AuthMethod for i := 0; i < acctCount; i++ { - aa := testAccount(t, conn, org.PublicId, authMethodId, u.PublicId) - accountIds = append(accountIds, aa.PublicId) + switch i % 2 { + case 0: + authMethod = oidc.TestAuthMethod(t, conn, databaseWrapper, org.PublicId, oidc.ActivePrivateState, fmt.Sprintf("alice-rp-%d", i), "fido", + oidc.WithIssuer(oidc.TestConvertToUrls(t, "https://alice.com")[0]), + oidc.WithSigningAlgs(oidc.RS256), + oidc.WithApiUrl(oidc.TestConvertToUrls(t, "http://localhost")[0])) + wantEmail, wantFullName = fmt.Sprintf("%s-%d@example.com", tt.name, i), fmt.Sprintf("%s-%d", tt.name, i) + aa := oidc.TestAccount(t, conn, authMethod, fmt.Sprintf(tt.name, i), oidc.WithFullName(wantFullName), oidc.WithEmail(wantEmail)) + wantPrimaryAccountId = aa.PublicId + accountIds = append(accountIds, aa.PublicId) + default: + pwAms := password.TestAuthMethods(t, conn, org.PublicId, 1) + require.Equal(1, len(pwAms)) + pwAcct := password.TestAccount(t, conn, pwAms[0].PublicId, "name1") + accountIds = append(accountIds, pwAcct.PublicId) + } } - sort.Strings(accountIds) + require.Equal(acctCount, len(accountIds)) + var s *iam.Scope + s, err = repo.LookupScope(context.Background(), org.PublicId) + require.NoError(err) + // we need a primary auth method, so let's just pick the last oidc + // one created. + iam.TestSetPrimaryAuthMethod(t, repo, s, authMethod.PublicId) - updateUser := allocUser() + sort.Strings(accountIds) + if len(accountIds) > 0 { + newAccts, err := repo.AddUserAccounts(context.Background(), u.PublicId, u.Version, accountIds) + require.NoError(err) + sort.Strings(newAccts) + require.Equal(accountIds, newAccts) + u.Version++ + } + // we need to clean out any oplog entries added because we + // associated accounts to the test user + _, err = rw.Exec(context.Background(), "delete from oplog_entry", nil) + require.NoError(err) + + updateUser := iam.AllocUser() updateUser.PublicId = u.PublicId if tt.args.PublicId != nil { updateUser.PublicId = *tt.args.PublicId @@ -323,20 +359,12 @@ func TestRepository_UpdateUser(t *testing.T) { updateUser.Name = tt.args.name updateUser.Description = tt.args.description - var userAfterUpdate *User + var userAfterUpdate *iam.User var acctIdsAfterUpdate []string var updatedRows int var err error - if tt.directUpdate { - u := updateUser.Clone() - var resource interface{} - resource, updatedRows, err = repo.update(context.Background(), u.(*User), 1, tt.args.fieldMaskPaths, nil, tt.args.opt...) - if err == nil { - userAfterUpdate = resource.(*User) - } - } else { - userAfterUpdate, acctIdsAfterUpdate, updatedRows, err = repo.UpdateUser(context.Background(), &updateUser, 1, tt.args.fieldMaskPaths, tt.args.opt...) - } + userAfterUpdate, acctIdsAfterUpdate, updatedRows, err = repo.UpdateUser(context.Background(), &updateUser, u.Version, tt.args.fieldMaskPaths, tt.args.opt...) + if tt.wantErr { require.Error(err) assert.True(errors.Match(errors.T(tt.wantIsErr), err)) @@ -348,7 +376,7 @@ func TestRepository_UpdateUser(t *testing.T) { default: assert.Containsf(err.Error(), tt.wantErrMsg, "unexpected error: %s", err.Error()) } - err = db.TestVerifyOplog(t, rw, u.PublicId, db.WithOperation(oplog.OpType_OP_TYPE_UPDATE), db.WithCreateNotBefore(10*time.Second)) + err = db.TestVerifyOplog(t, rw, u.PublicId, db.WithOperation(oplog.OpType_OP_TYPE_UPDATE), db.WithCreateNotBefore(1*time.Second)) require.Error(err) assert.Contains(err.Error(), "record not found") return @@ -358,6 +386,9 @@ func TestRepository_UpdateUser(t *testing.T) { assert.NotEqual(u.UpdateTime, userAfterUpdate.UpdateTime) sort.Strings(acctIdsAfterUpdate) assert.Equal(accountIds, acctIdsAfterUpdate) + assert.Equal(wantFullName, userAfterUpdate.FullName) + assert.Equal(wantEmail, userAfterUpdate.Email) + assert.Equal(wantPrimaryAccountId, userAfterUpdate.PrimaryAccountId) foundUser, foundAccountIds, err := repo.LookupUser(context.Background(), u.PublicId) require.NoError(err) @@ -384,12 +415,12 @@ func TestRepository_DeleteUser(t *testing.T) { conn, _ := db.TestSetup(t, "postgres") rw := db.New(conn) wrapper := db.TestWrapper(t) - repo := TestRepo(t, conn, wrapper) - org, _ := TestScopes(t, repo) + repo := iam.TestRepo(t, conn, wrapper) + org, _ := iam.TestScopes(t, repo) type args struct { - user *User - opt []Option + user *iam.User + opt []iam.Option } tests := []struct { name string @@ -401,7 +432,7 @@ func TestRepository_DeleteUser(t *testing.T) { { name: "valid", args: args{ - user: TestUser(t, repo, org.PublicId), + user: iam.TestUser(t, repo, org.PublicId), }, wantRowsDeleted: 1, wantErr: false, @@ -409,8 +440,8 @@ func TestRepository_DeleteUser(t *testing.T) { { name: "no-public-id", args: args{ - user: func() *User { - u := allocUser() + user: func() *iam.User { + u := iam.AllocUser() return &u }(), }, @@ -421,10 +452,10 @@ func TestRepository_DeleteUser(t *testing.T) { { name: "not-found", args: args{ - user: func() *User { - u, err := NewUser(org.PublicId) + user: func() *iam.User { + u, err := iam.NewUser(org.PublicId) require.NoError(t, err) - id, err := newUserId() + id, err := db.NewPublicId(iam.UserPrefix) require.NoError(t, err) u.PublicId = id return u @@ -465,12 +496,21 @@ func TestRepository_ListUsers(t *testing.T) { conn, _ := db.TestSetup(t, "postgres") const testLimit = 10 wrapper := db.TestWrapper(t) - repo := TestRepo(t, conn, wrapper, WithLimit(testLimit)) - org, _ := TestScopes(t, repo) + kmsCache := kms.TestKms(t, conn, wrapper) + repo := iam.TestRepo(t, conn, wrapper, iam.WithLimit(testLimit)) + org, _ := iam.TestScopes(t, repo) + databaseWrapper, err := kmsCache.GetWrapper(context.Background(), org.PublicId, kms.KeyPurposeDatabase) + require.NoError(t, err) + authMethod := oidc.TestAuthMethod(t, conn, databaseWrapper, org.PublicId, oidc.ActivePrivateState, "alice-rp", "fido", + oidc.WithIssuer(oidc.TestConvertToUrls(t, "https://alice.com")[0]), + oidc.WithSigningAlgs(oidc.RS256), + oidc.WithApiUrl(oidc.TestConvertToUrls(t, "http://localhost")[0])) + + iam.TestSetPrimaryAuthMethod(t, repo, org, authMethod.PublicId) type args struct { withOrgId string - opt []Option + opt []iam.Option } tests := []struct { name string @@ -481,29 +521,29 @@ func TestRepository_ListUsers(t *testing.T) { }{ { name: "no-limit", - createCnt: repo.defaultLimit + 1, + createCnt: testLimit + 1, args: args{ withOrgId: org.PublicId, - opt: []Option{WithLimit(-1)}, + opt: []iam.Option{iam.WithLimit(-1)}, }, - wantCnt: repo.defaultLimit + 1, + wantCnt: testLimit + 1, wantErr: false, }, { name: "default-limit", - createCnt: repo.defaultLimit + 1, + createCnt: testLimit + 1, args: args{ withOrgId: org.PublicId, }, - wantCnt: repo.defaultLimit, + wantCnt: testLimit, wantErr: false, }, { name: "custom-limit", - createCnt: repo.defaultLimit + 1, + createCnt: testLimit + 1, args: args{ withOrgId: org.PublicId, - opt: []Option{WithLimit(3)}, + opt: []iam.Option{iam.WithLimit(3)}, }, wantCnt: 3, wantErr: false, @@ -518,13 +558,32 @@ func TestRepository_ListUsers(t *testing.T) { wantErr: false, }, } + type userInfo struct { + email string + fullName string + primaryAcctId string + } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { assert, require := assert.New(t), require.New(t) - require.NoError(conn.Where("public_id != 'u_anon' and public_id != 'u_auth' and public_id != 'u_recovery'").Delete(allocUser()).Error) - testUsers := []*User{} + require.NoError(conn.Where("public_id != 'u_anon' and public_id != 'u_auth' and public_id != 'u_recovery'").Delete(iam.AllocUser()).Error) + testUsers := []*iam.User{} + wantUserInfo := map[string]userInfo{} for i := 0; i < tt.createCnt; i++ { - testUsers = append(testUsers, TestUser(t, repo, org.PublicId)) + u := iam.TestUser(t, repo, org.PublicId) + testUsers = append(testUsers, u) + wantEmail, wantFullName := fmt.Sprintf("%s-%d@example.com", tt.name, i), fmt.Sprintf("%s-%d", tt.name, i) + a := oidc.TestAccount(t, conn, authMethod, fmt.Sprintf(tt.name, i), oidc.WithFullName(wantFullName), oidc.WithEmail(wantEmail)) + wantUserInfo[u.PublicId] = userInfo{email: wantEmail, fullName: wantFullName, primaryAcctId: a.PublicId} + _, err := repo.AddUserAccounts(context.Background(), u.PublicId, u.Version, []string{a.PublicId}) + require.NoError(err) + + pwAms := password.TestAuthMethods(t, conn, org.PublicId, 1) + require.Equal(1, len(pwAms)) + pwAcct := password.TestAccount(t, conn, pwAms[0].PublicId, "name1") + _, err = repo.AddUserAccounts(context.Background(), u.PublicId, u.Version+1, []string{pwAcct.PublicId}) + require.NoError(err) + } assert.Equal(tt.createCnt, len(testUsers)) got, err := repo.ListUsers(context.Background(), []string{tt.args.withOrgId}, tt.args.opt...) @@ -534,6 +593,13 @@ func TestRepository_ListUsers(t *testing.T) { } require.NoError(err) assert.Equal(tt.wantCnt, len(got)) + for _, u := range got { + assert.Equal(wantUserInfo[u.PublicId], userInfo{ + email: u.Email, + fullName: u.FullName, + primaryAcctId: u.PrimaryAccountId, + }) + } }) } } @@ -542,17 +608,17 @@ func TestRepository_ListUsers_Multiple_Scopes(t *testing.T) { t.Parallel() conn, _ := db.TestSetup(t, "postgres") wrapper := db.TestWrapper(t) - repo := TestRepo(t, conn, wrapper) - org, _ := TestScopes(t, repo) + repo := iam.TestRepo(t, conn, wrapper) + org, _ := iam.TestScopes(t, repo) - require.NoError(t, conn.Where("public_id != 'u_anon' and public_id != 'u_auth' and public_id != 'u_recovery'").Delete(allocUser()).Error) + require.NoError(t, conn.Where("public_id != 'u_anon' and public_id != 'u_auth' and public_id != 'u_recovery'").Delete(iam.AllocUser()).Error) const numPerScope = 10 var total int = 3 // anon, auth, recovery for i := 0; i < numPerScope; i++ { - TestUser(t, repo, "global") + iam.TestUser(t, repo, "global") total++ - TestUser(t, repo, org.GetPublicId()) + iam.TestUser(t, repo, org.GetPublicId()) total++ } @@ -566,26 +632,61 @@ func TestRepository_LookupUserWithLogin(t *testing.T) { conn, _ := db.TestSetup(t, "postgres") rw := db.New(conn) wrapper := db.TestWrapper(t) - repo := TestRepo(t, conn, wrapper) + kmsCache := kms.TestKms(t, conn, wrapper) + repo := iam.TestRepo(t, conn, wrapper) id := testId(t) - org, _ := TestScopes(t, repo) - authMethodId := testAuthMethod(t, conn, org.PublicId) - TestSetPrimaryAuthMethod(t, repo, org, authMethodId) - newAuthAcct := testAccount(t, conn, org.PublicId, authMethodId, "") + org, _ := iam.TestScopes(t, repo) + databaseWrapper, err := kmsCache.GetWrapper(context.Background(), org.PublicId, kms.KeyPurposeDatabase) + require.NoError(t, err) + authMethod := oidc.TestAuthMethod(t, conn, databaseWrapper, org.PublicId, oidc.ActivePrivateState, "alice-rp", "fido", + oidc.WithIssuer(oidc.TestConvertToUrls(t, "https://alice.com")[0]), + oidc.WithSigningAlgs(oidc.RS256), + oidc.WithApiUrl(oidc.TestConvertToUrls(t, "http://localhost")[0])) - authMethodId2 := testAuthMethod(t, conn, org.PublicId) - newAuthAcctWithoutVivify := testAccount(t, conn, org.PublicId, authMethodId2, "") + iam.TestSetPrimaryAuthMethod(t, repo, org, authMethod.PublicId) - user := TestUser(t, repo, org.PublicId, WithName("existing-"+id)) - existingUserWithAcctWithVivify := testAccount(t, conn, org.PublicId, authMethodId, user.PublicId) - require.Equal(t, user.PublicId, existingUserWithAcctWithVivify.IamUserId) + // an account with no assoc user to test auto-vivify for the primary auth method. + newAuthAcct := oidc.TestAccount(t, conn, authMethod, "acct-1", oidc.WithFullName("acct-1"), oidc.WithEmail("acct-1@example.com")) - existingUserWithAcctNoVivify := testAccount(t, conn, org.PublicId, authMethodId2, user.PublicId) + // a 2nd auth method which will NOT be the primary auth method + authMethod2 := oidc.TestAuthMethod(t, conn, databaseWrapper, org.PublicId, oidc.ActivePrivateState, "alice-rp-2", "fido", + oidc.WithIssuer(oidc.TestConvertToUrls(t, "https://alice.com")[0]), + oidc.WithSigningAlgs(oidc.RS256), + oidc.WithApiUrl(oidc.TestConvertToUrls(t, "http://localhost")[0])) + + // an account with no assoc user to test non-primary auth methods do not auto-vivify + newAuthAcctWithoutVivify := oidc.TestAccount(t, conn, authMethod2, "acct-2", oidc.WithFullName("acct-2"), oidc.WithEmail("acct-2@example.com")) + + user := iam.TestUser(t, repo, org.PublicId, iam.WithName("existing-"+id)) + existingUserWithAcctWithVivify := oidc.TestAccount(t, conn, authMethod, "acct-3", oidc.WithFullName("acct-3"), oidc.WithEmail("acct-3@example.com")) + added, err := repo.AddUserAccounts(context.Background(), user.PublicId, user.Version, []string{existingUserWithAcctWithVivify.PublicId}) + require.NoError(t, err) + require.Equal(t, 1, len(added)) + foundAcct := allocAuthAccount() + foundAcct.PublicId = existingUserWithAcctWithVivify.PublicId + require.NoError(t, rw.LookupById(context.Background(), &foundAcct)) + require.Equal(t, user.PublicId, foundAcct.IamUserId) + + // we need to set these primary acct values on the user, so the test + // comparisons will work properly. + user.LoginName = "acct-3" + user.FullName = "acct-3" + user.Email = "acct-3@example.com" + + existingUserWithAcctNoVivify := oidc.TestAccount(t, conn, authMethod2, "acct-4", oidc.WithFullName("acct-4"), oidc.WithEmail("acct-4@example.com")) + user, _, err = repo.LookupUser(context.Background(), user.PublicId) + require.NoError(t, err) + added, err = repo.AddUserAccounts(context.Background(), user.PublicId, user.Version, []string{existingUserWithAcctNoVivify.PublicId}) + require.NoError(t, err) + require.Equal(t, 2, len(added)) + foundAcct.PublicId = existingUserWithAcctNoVivify.PublicId + require.NoError(t, rw.LookupById(context.Background(), &foundAcct, db.WithWhere("iam_user_id = ?", user.PublicId))) + require.Equal(t, user.PublicId, foundAcct.IamUserId) type args struct { withAccountId string - opt []Option + opt []iam.Option } tests := []struct { name string @@ -594,15 +695,15 @@ func TestRepository_LookupUserWithLogin(t *testing.T) { wantDescription string wantErr bool wantErrIs errors.Code - wantUser *User + wantUser *iam.User }{ { name: "valid", args: args{ withAccountId: newAuthAcct.PublicId, - opt: []Option{ - WithName("valid-" + id), - WithDescription("valid-" + id), + opt: []iam.Option{ + iam.WithName("valid-" + id), + iam.WithDescription("valid-" + id), }, }, wantName: "valid-" + id, @@ -638,7 +739,7 @@ func TestRepository_LookupUserWithLogin(t *testing.T) { name: "existing-user-with-account-no-vivify", args: args{ withAccountId: existingUserWithAcctNoVivify.PublicId, - opt: []Option{}, + opt: []iam.Option{}, }, wantErr: false, wantName: "existing-" + id, @@ -656,7 +757,7 @@ func TestRepository_LookupUserWithLogin(t *testing.T) { name: "bad-auth-account-id-with-vivify", args: args{ withAccountId: id, - opt: []Option{}, + opt: []iam.Option{}, }, wantErr: true, wantErrIs: errors.RecordNotFound, @@ -666,6 +767,7 @@ 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, conn.DB()) + got, err := repo.LookupUserWithLogin(context.Background(), tt.args.withAccountId, tt.args.opt...) if tt.wantErr { require.Error(err) @@ -673,7 +775,7 @@ func TestRepository_LookupUserWithLogin(t *testing.T) { assert.Truef(errors.Match(errors.T(tt.wantErrIs), err), "unexpected error %s", err.Error()) if tt.args.withAccountId != "" && tt.args.withAccountId != id { // need to assert that userid in auth_account is still null - acct := allocAccount() + acct := allocAuthAccount() acct.PublicId = tt.args.withAccountId dbassert.IsNull(&acct, "IamUserId") } @@ -688,9 +790,12 @@ func TestRepository_LookupUserWithLogin(t *testing.T) { } require.NotEmpty(got.PublicId) if tt.wantUser != nil { - assert.True(proto.Equal(tt.wantUser.User, got.User)) + tt.wantUser.Version = got.User.Version + tt.wantUser.CreateTime = got.User.CreateTime + tt.wantUser.UpdateTime = got.User.UpdateTime + assert.Empty(cmp.Diff(got.User, tt.wantUser.User, protocmp.Transform()), "got %q, wanted %q", got.User, tt.wantUser.User) } - acct := allocAccount() + acct := allocAuthAccount() acct.PublicId = tt.args.withAccountId err = rw.LookupByPublicId(context.Background(), &acct) require.NoError(err) @@ -699,321 +804,29 @@ func TestRepository_LookupUserWithLogin(t *testing.T) { } } -func TestRepository_associateUserWithAccounts(t *testing.T) { - t.Parallel() - conn, _ := db.TestSetup(t, "postgres") - rw := db.New(conn) - wrapper := db.TestWrapper(t) - kms := kms.TestKms(t, conn, wrapper) - repo := TestRepo(t, conn, wrapper) - org, _ := TestScopes(t, repo) - authMethodId := testAuthMethod(t, conn, org.PublicId) - - type Ids struct { - user string - accts []string - } - type args struct { - Ids Ids - opt []Option - } - tests := []struct { - name string - args args - want *User - want1 *authAccount - wantErr bool - wantErrCode errors.Code - wantAssoc bool - }{ - { - name: "simple", - args: args{ - Ids: func() Ids { - u := TestUser(t, repo, org.PublicId) - a := testAccount(t, conn, org.PublicId, authMethodId, "") - a2 := testAccount(t, conn, org.PublicId, authMethodId, "") - return Ids{user: u.PublicId, accts: []string{a.PublicId, a2.PublicId}} - }(), - }, - }, - { - name: "missing-acctId", - args: args{ - Ids: func() Ids { - id := testId(t) - - return Ids{user: id} - }(), - }, - wantErr: true, - wantErrCode: errors.InvalidParameter, - }, - { - name: "missing-userId", - args: args{ - Ids: func() Ids { - id := testId(t) - return Ids{accts: []string{id}} - }(), - }, - wantErr: true, - wantErrCode: errors.InvalidParameter, - }, - { - name: "already-properly-assoc", - args: args{ - Ids: func() Ids { - u := TestUser(t, repo, org.PublicId) - a := testAccount(t, conn, org.PublicId, authMethodId, u.PublicId) - return Ids{user: u.PublicId, accts: []string{a.PublicId}} - }(), - opt: []Option{WithDisassociate(true)}, - }, - }, - { - name: "assoc-with-diff-user", - args: args{ - Ids: func() Ids { - u := TestUser(t, repo, org.PublicId) - diffUser := TestUser(t, repo, org.PublicId) - a := testAccount(t, conn, org.PublicId, authMethodId, diffUser.PublicId) - return Ids{user: u.PublicId, accts: []string{a.PublicId}} - }(), - }, - wantErr: true, - wantErrCode: errors.AccountAlreadyAssociated, - }, - { - name: "assoc-with-diff-user-withDisassociateOption", - args: args{ - Ids: func() Ids { - u := TestUser(t, repo, org.PublicId) - diffUser := TestUser(t, repo, org.PublicId) - a := testAccount(t, conn, org.PublicId, authMethodId, diffUser.PublicId) - return Ids{user: u.PublicId, accts: []string{a.PublicId}} - }(), - }, - wantErr: true, - wantErrCode: errors.AccountAlreadyAssociated, - }, - { - name: "bad-acct-id", - args: args{ - Ids: func() Ids { - u := TestUser(t, repo, org.PublicId) - id := testId(t) - return Ids{user: u.PublicId, accts: []string{id}} - }(), - }, - wantErr: true, - wantErrCode: errors.RecordNotFound, - }, - { - name: "bad-user-id-not-associated-account", - args: args{ - Ids: func() Ids { - id := testId(t) - a := testAccount(t, conn, org.PublicId, authMethodId, "") - return Ids{user: id, accts: []string{a.PublicId}} - }(), - }, - wantErr: false, - }, - { - name: "bad-user-id", - args: args{ - Ids: func() Ids { - id := testId(t) - testUser := TestUser(t, repo, org.PublicId) - a := testAccount(t, conn, org.PublicId, authMethodId, testUser.PublicId) - return Ids{user: id, accts: []string{a.PublicId}} - }(), - }, - wantErr: true, - wantErrCode: errors.AccountAlreadyAssociated, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - assert, require := assert.New(t), require.New(t) - err := associateUserWithAccounts(context.Background(), kms, rw, rw, tt.args.Ids.user, tt.args.Ids.accts, tt.args.opt...) - if tt.wantErr { - require.Error(err) - assert.Truef(errors.Match(errors.T(tt.wantErrCode), err), "unexpected error %s", err) - return - } - require.NoError(err) - for _, id := range tt.args.Ids.accts { - acct := allocAccount() - acct.PublicId = id - 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) - kms := kms.TestKms(t, conn, wrapper) - repo := TestRepo(t, conn, wrapper) - org, _ := TestScopes(t, repo) - authMethodId := testAuthMethod(t, conn, org.PublicId) - - type Ids struct { - user string - accts []string - } - type args struct { - Ids Ids - opt []Option - } - tests := []struct { - name string - args args - want *User - want1 *authAccount - wantErr bool - wantErrCode errors.Code - }{ - { - name: "simple", - args: args{ - Ids: func() Ids { - u := TestUser(t, repo, org.PublicId) - a := testAccount(t, conn, org.PublicId, authMethodId, u.PublicId) - a2 := testAccount(t, conn, org.PublicId, authMethodId, u.PublicId) - return Ids{user: u.PublicId, accts: []string{a.PublicId, a2.PublicId}} - }(), - }, - }, - { - name: "missing-acctId", - args: args{ - Ids: func() Ids { - id := testId(t) - - return Ids{user: id} - }(), - }, - wantErr: true, - wantErrCode: errors.InvalidParameter, - }, - { - name: "missing-userId", - args: args{ - Ids: func() Ids { - id := testId(t) - return Ids{accts: []string{id}} - }(), - }, - wantErr: true, - wantErrCode: errors.InvalidParameter, - }, - { - name: "already-properly-disassoc", - args: args{ - Ids: func() Ids { - u := TestUser(t, repo, org.PublicId) - a := testAccount(t, conn, org.PublicId, authMethodId, "") - return Ids{user: u.PublicId, accts: []string{a.PublicId}} - }(), - }, - wantErr: true, - wantErrCode: errors.AccountAlreadyAssociated, - }, - { - name: "assoc-with-diff-user", - args: args{ - Ids: func() Ids { - u := TestUser(t, repo, org.PublicId) - diffUser := TestUser(t, repo, org.PublicId) - a := testAccount(t, conn, org.PublicId, authMethodId, diffUser.PublicId) - return Ids{user: u.PublicId, accts: []string{a.PublicId}} - }(), - }, - wantErr: true, - wantErrCode: errors.AccountAlreadyAssociated, - }, - { - name: "bad-acct-id", - args: args{ - Ids: func() Ids { - u := TestUser(t, repo, org.PublicId) - id := testId(t) - return Ids{user: u.PublicId, accts: []string{id}} - }(), - }, - wantErr: true, - wantErrCode: errors.RecordNotFound, - }, - { - name: "bad-user-id-not-associated-account", - args: args{ - Ids: func() Ids { - id := testId(t) - a := testAccount(t, conn, org.PublicId, authMethodId, "") - return Ids{user: id, accts: []string{a.PublicId}} - }(), - }, - wantErr: true, - wantErrCode: errors.AccountAlreadyAssociated, - }, - { - name: "bad-user-id", - args: args{ - Ids: func() Ids { - id := testId(t) - testUser := TestUser(t, repo, org.PublicId) - a := testAccount(t, conn, org.PublicId, authMethodId, testUser.PublicId) - return Ids{user: id, accts: []string{a.PublicId}} - }(), - }, - wantErr: true, - wantErrCode: errors.AccountAlreadyAssociated, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - assert, require := assert.New(t), require.New(t) - dbassert := dbassert.New(t, conn.DB()) - err := dissociateUserFromAccounts(context.Background(), kms, rw, rw, tt.args.Ids.user, tt.args.Ids.accts, tt.args.opt...) - if tt.wantErr { - require.Error(err) - assert.Truef(errors.Match(errors.T(tt.wantErrCode), err), "unexpected error %s", err) - return - } - require.NoError(err) - - for _, id := range tt.args.Ids.accts { - acct := allocAccount() - acct.PublicId = id - dbassert.IsNull(&acct, "IamUserId") - } - }) - } -} - func TestRepository_AssociateAccounts(t *testing.T) { t.Parallel() conn, _ := db.TestSetup(t, "postgres") rw := db.New(conn) wrapper := db.TestWrapper(t) - repo := TestRepo(t, conn, wrapper) - org, _ := TestScopes(t, repo) - authMethodId := testAuthMethod(t, conn, org.PublicId) - user := TestUser(t, repo, org.PublicId) + kmsCache := kms.TestKms(t, conn, wrapper) + repo := iam.TestRepo(t, conn, wrapper) + org, _ := iam.TestScopes(t, repo) + databaseWrapper, err := kmsCache.GetWrapper(context.Background(), org.PublicId, kms.KeyPurposeDatabase) + require.NoError(t, err) - createAccountsFn := func() []string { - require.NoError(t, conn.Where("iam_user_id = ?", user.PublicId).Delete(allocAccount()).Error) + user := iam.TestUser(t, repo, org.PublicId) + + createAccountsFn := func(prefix string) []string { + require.NoError(t, conn.Where("iam_user_id = ?", user.PublicId).Delete(allocAuthAccount()).Error) results := []string{} for i := 0; i < 5; i++ { - a := testAccount(t, conn, org.PublicId, authMethodId, "") + authMethod := oidc.TestAuthMethod(t, conn, databaseWrapper, org.PublicId, oidc.ActivePrivateState, fmt.Sprintf("%s-alice-rp-%d", prefix, i), "fido", + oidc.WithIssuer(oidc.TestConvertToUrls(t, "https://alice.com")[0]), + oidc.WithSigningAlgs(oidc.RS256), + oidc.WithApiUrl(oidc.TestConvertToUrls(t, "http://localhost")[0])) + + a := oidc.TestAccount(t, conn, authMethod, fmt.Sprintf("acct-%d", i)) results = append(results, a.PublicId) } return results @@ -1022,7 +835,7 @@ func TestRepository_AssociateAccounts(t *testing.T) { accountIdsFn func() []string userId string userVersionOverride *uint32 - opt []Option + opt []iam.Option } tests := []struct { name string @@ -1034,7 +847,7 @@ func TestRepository_AssociateAccounts(t *testing.T) { name: "valid", args: args{ userId: user.PublicId, - accountIdsFn: createAccountsFn, + accountIdsFn: func() []string { return createAccountsFn("valid") }, }, wantErr: false, }, @@ -1043,8 +856,17 @@ func TestRepository_AssociateAccounts(t *testing.T) { args: args{ userId: user.PublicId, accountIdsFn: func() []string { - ids := createAccountsFn() - a := testAccount(t, conn, org.PublicId, authMethodId, user.PublicId) + ids := createAccountsFn("already-associated") + authMethod := oidc.TestAuthMethod(t, conn, databaseWrapper, org.PublicId, oidc.ActivePrivateState, "already-associated-alice-rp", "fido", + oidc.WithIssuer(oidc.TestConvertToUrls(t, "https://alice.com")[0]), + oidc.WithSigningAlgs(oidc.RS256), + oidc.WithApiUrl(oidc.TestConvertToUrls(t, "http://localhost")[0])) + a := oidc.TestAccount(t, conn, authMethod, "already-associated") + user, _, err := repo.LookupUser(context.Background(), user.PublicId) + require.NoError(t, err) + added, err := repo.AddUserAccounts(context.Background(), user.PublicId, user.Version, []string{a.PublicId}) + require.NoError(t, err) + require.Contains(t, added, a.PublicId) ids = append(ids, a.PublicId) return ids }, @@ -1056,9 +878,16 @@ func TestRepository_AssociateAccounts(t *testing.T) { args: args{ userId: user.PublicId, accountIdsFn: func() []string { - ids := createAccountsFn() - u := TestUser(t, repo, org.PublicId) - a := testAccount(t, conn, org.PublicId, authMethodId, u.PublicId) + ids := createAccountsFn("associated-with-diff-user") + authMethod := oidc.TestAuthMethod(t, conn, databaseWrapper, org.PublicId, oidc.ActivePrivateState, "associated-with-diff-user-alice-rp", "fido", + oidc.WithIssuer(oidc.TestConvertToUrls(t, "https://alice.com")[0]), + oidc.WithSigningAlgs(oidc.RS256), + oidc.WithApiUrl(oidc.TestConvertToUrls(t, "http://localhost")[0])) + u := iam.TestUser(t, repo, org.PublicId) + a := oidc.TestAccount(t, conn, authMethod, "already-associated") + added, err := repo.AddUserAccounts(context.Background(), u.PublicId, u.Version, []string{a.PublicId}) + require.NoError(t, err) + require.Contains(t, added, a.PublicId) ids = append(ids, a.PublicId) return ids }, @@ -1074,7 +903,7 @@ func TestRepository_AssociateAccounts(t *testing.T) { return &i }(), userId: user.PublicId, - accountIdsFn: createAccountsFn, + accountIdsFn: func() []string { return createAccountsFn("bad-version") }, }, wantErr: true, wantErrCode: errors.MultipleRecords, @@ -1087,7 +916,7 @@ func TestRepository_AssociateAccounts(t *testing.T) { return &i }(), userId: user.PublicId, - accountIdsFn: createAccountsFn, + accountIdsFn: func() []string { return createAccountsFn("zero-version") }, }, wantErr: true, wantErrCode: errors.InvalidParameter, @@ -1150,16 +979,28 @@ func TestRepository_DisassociateAccounts(t *testing.T) { conn, _ := db.TestSetup(t, "postgres") rw := db.New(conn) wrapper := db.TestWrapper(t) - repo := TestRepo(t, conn, wrapper) - org, _ := TestScopes(t, repo) - authMethodId := testAuthMethod(t, conn, org.PublicId) - user := TestUser(t, repo, org.PublicId) + kmsCache := kms.TestKms(t, conn, wrapper) + repo := iam.TestRepo(t, conn, wrapper) + org, _ := iam.TestScopes(t, repo) + databaseWrapper, err := kmsCache.GetWrapper(context.Background(), org.PublicId, kms.KeyPurposeDatabase) + require.NoError(t, err) - createAccountsFn := func() []string { - require.NoError(t, conn.Where("iam_user_id = ?", user.PublicId).Delete(allocAccount()).Error) + user := iam.TestUser(t, repo, org.PublicId) + + createAccountsFn := func(prefix string) []string { + require.NoError(t, conn.Where("iam_user_id = ?", user.PublicId).Delete(allocAuthAccount()).Error) results := []string{} for i := 0; i < 1; i++ { - a := testAccount(t, conn, org.PublicId, authMethodId, user.PublicId) + authMethod := oidc.TestAuthMethod(t, conn, databaseWrapper, org.PublicId, oidc.ActivePrivateState, fmt.Sprintf("%s-alice-rp-%d", prefix, i), "fido", + oidc.WithIssuer(oidc.TestConvertToUrls(t, "https://alice.com")[0]), + oidc.WithSigningAlgs(oidc.RS256), + oidc.WithApiUrl(oidc.TestConvertToUrls(t, "http://localhost")[0])) + a := oidc.TestAccount(t, conn, authMethod, fmt.Sprintf("acct-%d", i)) + user, _, err := repo.LookupUser(context.Background(), user.PublicId) + require.NoError(t, err) + added, err := repo.AddUserAccounts(context.Background(), user.PublicId, user.Version, []string{a.PublicId}) + require.NoError(t, err) + require.Contains(t, added, a.PublicId) results = append(results, a.PublicId) } return results @@ -1168,7 +1009,7 @@ func TestRepository_DisassociateAccounts(t *testing.T) { accountIdsFn func() []string userId string userVersionOverride *uint32 - opt []Option + opt []iam.Option } tests := []struct { name string @@ -1180,7 +1021,7 @@ func TestRepository_DisassociateAccounts(t *testing.T) { name: "valid", args: args{ userId: user.PublicId, - accountIdsFn: createAccountsFn, + accountIdsFn: func() []string { return createAccountsFn("valid") }, }, wantErr: false, }, @@ -1189,9 +1030,16 @@ func TestRepository_DisassociateAccounts(t *testing.T) { args: args{ userId: user.PublicId, accountIdsFn: func() []string { - ids := createAccountsFn() - u := TestUser(t, repo, org.PublicId) - a := testAccount(t, conn, org.PublicId, authMethodId, u.PublicId) + ids := createAccountsFn("associated-with-diff-user") + authMethod := oidc.TestAuthMethod(t, conn, databaseWrapper, org.PublicId, oidc.ActivePrivateState, "associated-with-diff-user-alice-rp", "fido", + oidc.WithIssuer(oidc.TestConvertToUrls(t, "https://alice.com")[0]), + oidc.WithSigningAlgs(oidc.RS256), + oidc.WithApiUrl(oidc.TestConvertToUrls(t, "http://localhost")[0])) + u := iam.TestUser(t, repo, org.PublicId) + a := oidc.TestAccount(t, conn, authMethod, "already-associated") + added, err := repo.AddUserAccounts(context.Background(), u.PublicId, u.Version, []string{a.PublicId}) + require.NoError(t, err) + require.Contains(t, added, a.PublicId) ids = append(ids, a.PublicId) return ids }, @@ -1207,7 +1055,7 @@ func TestRepository_DisassociateAccounts(t *testing.T) { return &i }(), userId: user.PublicId, - accountIdsFn: createAccountsFn, + accountIdsFn: func() []string { return createAccountsFn("bad-version") }, }, wantErr: true, wantErrCode: errors.MultipleRecords, @@ -1220,7 +1068,7 @@ func TestRepository_DisassociateAccounts(t *testing.T) { return &i }(), userId: user.PublicId, - accountIdsFn: createAccountsFn, + accountIdsFn: func() []string { return createAccountsFn("zero-version") }, }, wantErr: true, wantErrCode: errors.InvalidParameter, @@ -1279,16 +1127,23 @@ func TestRepository_SetAssociatedAccounts(t *testing.T) { conn, _ := db.TestSetup(t, "postgres") rw := db.New(conn) wrapper := db.TestWrapper(t) - repo := TestRepo(t, conn, wrapper) - org, _ := TestScopes(t, repo) - authMethodId := testAuthMethod(t, conn, org.PublicId) - user := TestUser(t, repo, org.PublicId) + kmsCache := kms.TestKms(t, conn, wrapper) + repo := iam.TestRepo(t, conn, wrapper) + org, _ := iam.TestScopes(t, repo) + databaseWrapper, err := kmsCache.GetWrapper(context.Background(), org.PublicId, kms.KeyPurposeDatabase) + require.NoError(t, err) - createAccountsFn := func() []string { - require.NoError(t, conn.Where("iam_user_id = ?", user.PublicId).Delete(allocAccount()).Error) + user := iam.TestUser(t, repo, org.PublicId) + + createAccountsFn := func(prefix string) []string { + require.NoError(t, conn.Where("iam_user_id = ?", user.PublicId).Delete(allocAuthAccount()).Error) results := []string{} - for i := 0; i < 5; i++ { - a := testAccount(t, conn, org.PublicId, authMethodId, "") + for i := 0; i < 1; i++ { + authMethod := oidc.TestAuthMethod(t, conn, databaseWrapper, org.PublicId, oidc.ActivePrivateState, fmt.Sprintf("%s-alice-rp-%d", prefix, i), "fido", + oidc.WithIssuer(oidc.TestConvertToUrls(t, "https://alice.com")[0]), + oidc.WithSigningAlgs(oidc.RS256), + oidc.WithApiUrl(oidc.TestConvertToUrls(t, "http://localhost")[0])) + a := oidc.TestAccount(t, conn, authMethod, fmt.Sprintf("acct-%d", i)) results = append(results, a.PublicId) } return results @@ -1297,7 +1152,7 @@ func TestRepository_SetAssociatedAccounts(t *testing.T) { accountIdsFn func() ([]string, []string) userId string userVersionOverride *uint32 - opt []Option + opt []iam.Option } tests := []struct { name string @@ -1310,7 +1165,7 @@ func TestRepository_SetAssociatedAccounts(t *testing.T) { args: args{ userId: user.PublicId, accountIdsFn: func() ([]string, []string) { - ids := createAccountsFn() + ids := createAccountsFn("valid") return ids, ids }, }, @@ -1321,9 +1176,18 @@ func TestRepository_SetAssociatedAccounts(t *testing.T) { args: args{ userId: user.PublicId, accountIdsFn: func() ([]string, []string) { - ids := createAccountsFn() + ids := createAccountsFn("one-already-associated") changes := append([]string{}, ids...) - a := testAccount(t, conn, org.PublicId, authMethodId, user.PublicId) + authMethod := oidc.TestAuthMethod(t, conn, databaseWrapper, org.PublicId, oidc.ActivePrivateState, "one-already-associated-alice-rp", "fido", + oidc.WithIssuer(oidc.TestConvertToUrls(t, "https://alice.com")[0]), + oidc.WithSigningAlgs(oidc.RS256), + oidc.WithApiUrl(oidc.TestConvertToUrls(t, "http://localhost")[0])) + a := oidc.TestAccount(t, conn, authMethod, "acct-one-already-associated") + user, _, err := repo.LookupUser(context.Background(), user.PublicId) + require.NoError(t, err) + added, err := repo.AddUserAccounts(context.Background(), user.PublicId, user.Version, []string{a.PublicId}) + require.NoError(t, err) + require.Contains(t, added, a.PublicId) ids = append(ids, a.PublicId) return ids, changes }, @@ -1335,11 +1199,18 @@ func TestRepository_SetAssociatedAccounts(t *testing.T) { args: args{ userId: user.PublicId, accountIdsFn: func() ([]string, []string) { - ids := []string{} - for i := 0; i < 10; i++ { - a := testAccount(t, conn, org.PublicId, authMethodId, user.PublicId) - ids = append(ids, a.PublicId) - } + ids := createAccountsFn("no-change") + user, _, err := repo.LookupUser(context.Background(), user.PublicId) + require.NoError(t, err) + added, err := repo.AddUserAccounts(context.Background(), user.PublicId, user.Version, ids) + require.NoError(t, err) + require.Equal(t, len(ids), len(added)) + + // ids := []string{} + // for i := 0; i < 10; i++ { + // a := testAccount(t, conn, org.PublicId, authMethodId, user.PublicId) + // ids = append(ids, a.PublicId) + // } return ids, nil }, }, @@ -1350,11 +1221,12 @@ func TestRepository_SetAssociatedAccounts(t *testing.T) { args: args{ userId: user.PublicId, accountIdsFn: func() ([]string, []string) { - ids := []string{} - for i := 0; i < 10; i++ { - a := testAccount(t, conn, org.PublicId, authMethodId, user.PublicId) - ids = append(ids, a.PublicId) - } + ids := createAccountsFn("remove-all") + user, _, err := repo.LookupUser(context.Background(), user.PublicId) + require.NoError(t, err) + added, err := repo.AddUserAccounts(context.Background(), user.PublicId, user.Version, ids) + require.NoError(t, err) + require.Equal(t, len(ids), len(added)) return nil, ids }, }, @@ -1365,9 +1237,16 @@ func TestRepository_SetAssociatedAccounts(t *testing.T) { args: args{ userId: user.PublicId, accountIdsFn: func() ([]string, []string) { - ids := createAccountsFn() - u := TestUser(t, repo, org.PublicId) - a := testAccount(t, conn, org.PublicId, authMethodId, u.PublicId) + ids := createAccountsFn("associated-with-diff-user") + authMethod := oidc.TestAuthMethod(t, conn, databaseWrapper, org.PublicId, oidc.ActivePrivateState, "associated-with-diff-user-alice-rp", "fido", + oidc.WithIssuer(oidc.TestConvertToUrls(t, "https://alice.com")[0]), + oidc.WithSigningAlgs(oidc.RS256), + oidc.WithApiUrl(oidc.TestConvertToUrls(t, "http://localhost")[0])) + u := iam.TestUser(t, repo, org.PublicId) + a := oidc.TestAccount(t, conn, authMethod, "already-associated") + added, err := repo.AddUserAccounts(context.Background(), u.PublicId, u.Version, []string{a.PublicId}) + require.NoError(t, err) + require.Contains(t, added, a.PublicId) ids = append(ids, a.PublicId) return ids, ids }, @@ -1384,7 +1263,7 @@ func TestRepository_SetAssociatedAccounts(t *testing.T) { }(), userId: user.PublicId, accountIdsFn: func() ([]string, []string) { - ids := createAccountsFn() + ids := createAccountsFn("bad-version") return ids, ids }, }, @@ -1400,7 +1279,7 @@ func TestRepository_SetAssociatedAccounts(t *testing.T) { }(), userId: user.PublicId, accountIdsFn: func() ([]string, []string) { - ids := createAccountsFn() + ids := createAccountsFn("zero-version") return ids, ids }, }, @@ -1419,6 +1298,8 @@ func TestRepository_SetAssociatedAccounts(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { assert, require := assert.New(t), require.New(t) + require.NoError(conn.Where("iam_user_id = ?", user.PublicId).Delete(allocAuthAccount()).Error) + accountIds, changes := tt.args.accountIdsFn() sort.Strings(accountIds) @@ -1457,11 +1338,37 @@ func TestRepository_SetAssociatedAccounts(t *testing.T) { u, _, err := repo.LookupUser(context.Background(), tt.args.userId) require.NoError(err) switch tt.name { - case "no-accounts-no-changes": - assert.Equal(version, u.Version) + case "no-accounts-no-changes", "no-change": + assert.Equalf(version, u.Version, "expected version %d and got: %d", version, u.Version) default: - assert.Equal(version+1, u.Version) + assert.Equalf(version+1, u.Version, "expected version %d and got: %d", version+1, u.Version) } }) } } + +func testId(t *testing.T) string { + t.Helper() + id, err := uuid.GenerateUUID() + require.NoError(t, err) + return id +} + +type testAuthAccount struct { + *store.Account + tableName string `gorm:"-"` +} + +func allocAuthAccount() testAuthAccount { + return testAuthAccount{ + Account: &store.Account{}, + } +} + +// TableName returns the tablename to override the default gorm table name. +func (a *testAuthAccount) TableName() string { + if a.tableName != "" { + return a.tableName + } + return "auth_account" +} diff --git a/internal/iam/resource_test.go b/internal/iam/resource_test.go index 8bd9f69f8..4200ca3de 100644 --- a/internal/iam/resource_test.go +++ b/internal/iam/resource_test.go @@ -25,7 +25,7 @@ func Test_LookupScope(t *testing.T) { require.NoError(err) assert.Equal(foundScope.PublicId, user.ScopeId) - user2 := allocUser() + user2 := AllocUser() user2.PublicId = user.PublicId foundScope, err = LookupScope(context.Background(), w, user) require.NoError(err) @@ -46,7 +46,7 @@ func Test_LookupScope(t *testing.T) { assert.Nil(s) assert.Equal("iam.LookupScope: missing resource: parameter violation: error #100", err.Error()) - user2 := allocUser() + user2 := AllocUser() s, err = LookupScope(context.Background(), w, &user2) assert.Nil(s) assert.Contains(err.Error(), "iam.LookupScope: missing public id: parameter violation: error #100") diff --git a/internal/iam/store/user.pb.go b/internal/iam/store/user.pb.go index 81f9df51d..1c6ff43e3 100644 --- a/internal/iam/store/user.pb.go +++ b/internal/iam/store/user.pb.go @@ -49,6 +49,22 @@ type User struct { // version allows optimistic locking of the user // @inject_tag: `gorm:"default:null"` Version uint32 `protobuf:"varint,70,opt,name=version,proto3" json:"version,omitempty" gorm:"default:null"` + // Output only. login_name is a string that maps to the user's account "login + // name" from the scope's primary auth method + // @inject_tag: `gorm:"-"` + LoginName string `protobuf:"bytes,90,opt,name=login_name,proto3" json:"login_name,omitempty" gorm:"-"` + // Output only. full_name is a string that maps to the user's account name + // from the scope's primary auth method + // @inject_tag: `gorm:"-"` + FullName string `protobuf:"bytes,100,opt,name=full_name,proto3" json:"full_name,omitempty" gorm:"-"` + // Output only. email is a string that maps to the user's account email from + // the scope's primary auth method + // @inject_tag: `gorm:"-"` + Email string `protobuf:"bytes,110,opt,name=email,proto3" json:"email,omitempty" gorm:"-"` + // Output only. primary_account_id is a string that maps to the user's account + // public_id from the scope's primary auth method + // @inject_tag: `gorm:"-"` + PrimaryAccountId string `protobuf:"bytes,120,opt,name=primary_account_id,proto3" json:"primary_account_id,omitempty" gorm:"-"` } func (x *User) Reset() { @@ -132,6 +148,34 @@ func (x *User) GetVersion() uint32 { return 0 } +func (x *User) GetLoginName() string { + if x != nil { + return x.LoginName + } + return "" +} + +func (x *User) GetFullName() string { + if x != nil { + return x.FullName + } + return "" +} + +func (x *User) GetEmail() string { + if x != nil { + return x.Email + } + return "" +} + +func (x *User) GetPrimaryAccountId() string { + if x != nil { + return x.PrimaryAccountId + } + return "" +} + var File_controller_storage_iam_store_v1_user_proto protoreflect.FileDescriptor var file_controller_storage_iam_store_v1_user_proto_rawDesc = []byte{ @@ -145,7 +189,7 @@ var file_controller_storage_iam_store_v1_user_proto_rawDesc = []byte{ 0x69, 0x6d, 0x65, 0x73, 0x74, 0x61, 0x6d, 0x70, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x1a, 0x2a, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x6f, 0x6c, 0x6c, 0x65, 0x72, 0x2f, 0x63, 0x75, 0x73, 0x74, 0x6f, 0x6d, 0x5f, 0x6f, 0x70, 0x74, 0x69, 0x6f, 0x6e, 0x73, 0x2f, 0x76, 0x31, 0x2f, 0x6f, 0x70, 0x74, - 0x69, 0x6f, 0x6e, 0x73, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x22, 0xda, 0x02, 0x0a, 0x04, 0x55, + 0x69, 0x6f, 0x6e, 0x73, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x22, 0xde, 0x03, 0x0a, 0x04, 0x55, 0x73, 0x65, 0x72, 0x12, 0x1b, 0x0a, 0x09, 0x70, 0x75, 0x62, 0x6c, 0x69, 0x63, 0x5f, 0x69, 0x64, 0x18, 0x0a, 0x20, 0x01, 0x28, 0x09, 0x52, 0x08, 0x70, 0x75, 0x62, 0x6c, 0x69, 0x63, 0x49, 0x64, 0x12, 0x19, 0x0a, 0x08, 0x73, 0x63, 0x6f, 0x70, 0x65, 0x5f, 0x69, 0x64, 0x18, 0x14, 0x20, 0x01, @@ -167,11 +211,19 @@ var file_controller_storage_iam_store_v1_user_proto_rawDesc = []byte{ 0x74, 0x61, 0x6d, 0x70, 0x2e, 0x76, 0x31, 0x2e, 0x54, 0x69, 0x6d, 0x65, 0x73, 0x74, 0x61, 0x6d, 0x70, 0x52, 0x0a, 0x75, 0x70, 0x64, 0x61, 0x74, 0x65, 0x54, 0x69, 0x6d, 0x65, 0x12, 0x18, 0x0a, 0x07, 0x76, 0x65, 0x72, 0x73, 0x69, 0x6f, 0x6e, 0x18, 0x46, 0x20, 0x01, 0x28, 0x0d, 0x52, 0x07, - 0x76, 0x65, 0x72, 0x73, 0x69, 0x6f, 0x6e, 0x42, 0x38, 0x5a, 0x36, 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, 0x69, 0x61, 0x6d, 0x2f, 0x73, 0x74, 0x6f, 0x72, 0x65, 0x3b, 0x73, 0x74, 0x6f, 0x72, - 0x65, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, + 0x76, 0x65, 0x72, 0x73, 0x69, 0x6f, 0x6e, 0x12, 0x1e, 0x0a, 0x0a, 0x6c, 0x6f, 0x67, 0x69, 0x6e, + 0x5f, 0x6e, 0x61, 0x6d, 0x65, 0x18, 0x5a, 0x20, 0x01, 0x28, 0x09, 0x52, 0x0a, 0x6c, 0x6f, 0x67, + 0x69, 0x6e, 0x5f, 0x6e, 0x61, 0x6d, 0x65, 0x12, 0x1c, 0x0a, 0x09, 0x66, 0x75, 0x6c, 0x6c, 0x5f, + 0x6e, 0x61, 0x6d, 0x65, 0x18, 0x64, 0x20, 0x01, 0x28, 0x09, 0x52, 0x09, 0x66, 0x75, 0x6c, 0x6c, + 0x5f, 0x6e, 0x61, 0x6d, 0x65, 0x12, 0x14, 0x0a, 0x05, 0x65, 0x6d, 0x61, 0x69, 0x6c, 0x18, 0x6e, + 0x20, 0x01, 0x28, 0x09, 0x52, 0x05, 0x65, 0x6d, 0x61, 0x69, 0x6c, 0x12, 0x2e, 0x0a, 0x12, 0x70, + 0x72, 0x69, 0x6d, 0x61, 0x72, 0x79, 0x5f, 0x61, 0x63, 0x63, 0x6f, 0x75, 0x6e, 0x74, 0x5f, 0x69, + 0x64, 0x18, 0x78, 0x20, 0x01, 0x28, 0x09, 0x52, 0x12, 0x70, 0x72, 0x69, 0x6d, 0x61, 0x72, 0x79, + 0x5f, 0x61, 0x63, 0x63, 0x6f, 0x75, 0x6e, 0x74, 0x5f, 0x69, 0x64, 0x42, 0x38, 0x5a, 0x36, 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, 0x69, 0x61, 0x6d, 0x2f, 0x73, 0x74, 0x6f, 0x72, 0x65, 0x3b, + 0x73, 0x74, 0x6f, 0x72, 0x65, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, } var ( diff --git a/internal/iam/testing.go b/internal/iam/testing.go index 6dd2a0830..5741d2e39 100644 --- a/internal/iam/testing.go +++ b/internal/iam/testing.go @@ -48,6 +48,10 @@ func TestSetPrimaryAuthMethod(t *testing.T, repo *Repository, s *Scope, authMeth s.PrimaryAuthMethodId = authMethodId _, _, err := repo.UpdateScope(context.Background(), s, s.Version, []string{"PrimaryAuthMethodId"}) require.NoError(err) + + updated, err := repo.LookupScope(context.Background(), s.PublicId) + require.NoError(err) + require.Equalf(authMethodId, updated.PrimaryAuthMethodId, "expected %s to be the primary auth method for scope: %s", authMethodId, updated.PublicId) } // TestScopes creates an org and project suitable for testing. @@ -253,7 +257,7 @@ func testAccount(t *testing.T, conn *gorm.DB, scopeId, authMethodId, userId stri require.NotEmpty(authMethodId) if userId != "" { - foundUser := allocUser() + foundUser := AllocUser() foundUser.PublicId = userId err := rw.LookupByPublicId(context.Background(), &foundUser) require.NoError(err) diff --git a/internal/iam/user.go b/internal/iam/user.go index f414ba012..bbf9aa150 100644 --- a/internal/iam/user.go +++ b/internal/iam/user.go @@ -13,7 +13,8 @@ import ( ) const ( - defaultUserTableName = "iam_user" + defaultUserTableName = "iam_user" + defaultUserAccountInfoTableName = "iam_user_acct_info" ) // User defines boundary users which are scoped to an Org @@ -48,7 +49,8 @@ func NewUser(scopeId string, opt ...Option) (*User, error) { return u, nil } -func allocUser() User { +// AllocUser will allocate an empty user +func AllocUser() User { return User{ User: &store.User{}, } @@ -106,3 +108,35 @@ func (u *User) TableName() string { func (u *User) SetTableName(n string) { u.tableName = n } + +// userAccountInfo provides a way to represent a user along with the user's +// account info from the scope's primary auth method +type userAccountInfo struct { + *store.User + tableName string `gorm:"-"` +} + +func (u *userAccountInfo) shallowConversion() *User { + return &User{ + User: u.User, + } +} + +// TableName provides an overridden gorm table name.. +func (u *userAccountInfo) TableName() string { + if u.tableName != "" { + return u.tableName + } + return defaultUserAccountInfoTableName +} + +// SetTableName sets the table name for the resource. If the caller attempts to +// set the name to "" the name will be reset to the default name. +func (u *userAccountInfo) SetTableName(n string) { + switch n { + case "": + u.tableName = defaultUserAccountInfoTableName + default: + u.tableName = n + } +} diff --git a/internal/iam/user_test.go b/internal/iam/user_test.go index 43911e22f..8e00e4541 100644 --- a/internal/iam/user_test.go +++ b/internal/iam/user_test.go @@ -80,7 +80,7 @@ func Test_UserHardcoded(t *testing.T) { assert, require := assert.New(t), require.New(t) w := db.New(conn) for _, v := range []string{"u_anon", "u_auth"} { - foundUser := allocUser() + foundUser := AllocUser() foundUser.PublicId = v err := w.LookupByPublicId(context.Background(), &foundUser) require.NoError(err) @@ -107,7 +107,7 @@ func Test_UserCreate(t *testing.T) { require.NoError(err) require.NotEmpty(user.PublicId) - foundUser := allocUser() + foundUser := AllocUser() foundUser.PublicId = user.PublicId err = w.LookupByPublicId(context.Background(), &foundUser) require.NoError(err) @@ -135,12 +135,14 @@ func Test_UserUpdate(t *testing.T) { repo := TestRepo(t, conn, wrapper) id := testId(t) org, proj := TestScopes(t, repo) + org2, _ := TestScopes(t, repo) type args struct { name string description string fieldMaskPaths []string ScopeId string + dbOpts []db.Option } tests := []struct { name string @@ -201,6 +203,17 @@ func Test_UserUpdate(t *testing.T) { wantDup: true, wantErrMsg: `db.Update: duplicate key value violates unique constraint "iam_user_name_scope_id_key": unique constraint violation: integrity violation: error #1002`, }, + { + name: "modified-scope", + args: args{ + name: "modified-scope" + id, + fieldMaskPaths: []string{"ScopeId"}, + ScopeId: org2.PublicId, + dbOpts: []db.Option{db.WithSkipVetForWrite(true)}, + }, + wantErr: true, + wantErrMsg: `db.Update: immutable column: iam_user.scope_id: integrity violation: error #1003`, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -208,19 +221,19 @@ func Test_UserUpdate(t *testing.T) { if tt.wantDup { u := TestUser(t, repo, org.PublicId) u.Name = tt.args.name - _, err := rw.Update(context.Background(), u, tt.args.fieldMaskPaths, nil) + _, err := rw.Update(context.Background(), u, tt.args.fieldMaskPaths, nil, tt.args.dbOpts...) require.NoError(err) } u := TestUser(t, repo, org.PublicId) - updateUser := allocUser() + updateUser := AllocUser() updateUser.PublicId = u.PublicId updateUser.ScopeId = tt.args.ScopeId updateUser.Name = tt.args.name updateUser.Description = tt.args.description - updatedRows, err := rw.Update(context.Background(), &updateUser, tt.args.fieldMaskPaths, nil) + updatedRows, err := rw.Update(context.Background(), &updateUser, tt.args.fieldMaskPaths, nil, tt.args.dbOpts...) if tt.wantErr { require.Error(err) assert.Equal(0, updatedRows) @@ -262,19 +275,19 @@ func Test_UserDelete(t *testing.T) { }, { name: "bad-id", - user: func() *User { u := allocUser(); u.PublicId = id; return &u }(), + user: func() *User { u := AllocUser(); u.PublicId = id; return &u }(), wantErr: false, wantRowsDeleted: 0, }, { name: "anon-user", - user: func() *User { u := allocUser(); u.PublicId = "u_anon"; return &u }(), + user: func() *User { u := AllocUser(); u.PublicId = "u_anon"; return &u }(), wantErr: true, wantRowsDeleted: 0, }, { name: "auth-user", - user: func() *User { u := allocUser(); u.PublicId = "u_auth"; return &u }(), + user: func() *User { u := AllocUser(); u.PublicId = "u_auth"; return &u }(), wantErr: true, wantRowsDeleted: 0, }, @@ -282,7 +295,7 @@ func Test_UserDelete(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { assert, require := assert.New(t), require.New(t) - deleteUser := allocUser() + deleteUser := AllocUser() deleteUser.PublicId = tt.user.GetPublicId() deletedRows, err := rw.Delete(context.Background(), &deleteUser) if tt.wantRowsDeleted == 0 { @@ -368,7 +381,7 @@ func TestUser_Actions(t *testing.T) { func TestUser_ResourceType(t *testing.T) { t.Parallel() - u := allocUser() + u := AllocUser() assert.Equal(t, resource.User, u.ResourceType()) } @@ -396,7 +409,7 @@ func TestUser_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 := allocUser() + def := AllocUser() require.Equal(defaultTableName, def.TableName()) s := &User{ User: &store.User{}, diff --git a/internal/proto/local/controller/api/resources/users/v1/user.proto b/internal/proto/local/controller/api/resources/users/v1/user.proto index 68d324018..df0dcb0d8 100644 --- a/internal/proto/local/controller/api/resources/users/v1/user.proto +++ b/internal/proto/local/controller/api/resources/users/v1/user.proto @@ -10,46 +10,62 @@ import "controller/api/resources/scopes/v1/scope.proto"; import "controller/custom_options/v1/options.proto"; message Account { - // Output only. The ID of the Account. - string id = 10; + // Output only. The ID of the Account. + string id = 10; - // Output only. The Scope containing the Account. - string scope_id = 20 [json_name="scope_id"]; + // Output only. The Scope containing the Account. + string scope_id = 20 [json_name = "scope_id"]; } // User contains all fields related to a User resource message User { - // Output only. The ID of the User. - string id = 10; + // Output only. The ID of the User. + string id = 10; - // The ID of the Scope this resource is in. - string scope_id = 20 [json_name="scope_id"]; + // The ID of the Scope this resource is in. + string scope_id = 20 [json_name = "scope_id"]; - // Output only. Scope information for this resource. - resources.scopes.v1.ScopeInfo scope = 30; - - // Optional name for identification purposes. - google.protobuf.StringValue name = 40 [(custom_options.v1.generate_sdk_option) = true, (custom_options.v1.mask_mapping) = {this:"name" that: "name"}]; + // Output only. Scope information for this resource. + resources.scopes.v1.ScopeInfo scope = 30; - // Optional user-set description for identification purposes. - google.protobuf.StringValue description = 50 [(custom_options.v1.generate_sdk_option) = true, (custom_options.v1.mask_mapping) = {this:"description" that: "description"}]; + // Optional name for identification purposes. + google.protobuf.StringValue name = 40 [(custom_options.v1.generate_sdk_option) = true, (custom_options.v1.mask_mapping) = { this: "name" that: "name" }]; - // Output only. The time this resource was created. - google.protobuf.Timestamp created_time = 60 [json_name="created_time"]; + // Optional user-set description for identification purposes. + google.protobuf.StringValue description = 50 [(custom_options.v1.generate_sdk_option) = true, (custom_options.v1.mask_mapping) = { this: "description" that: "description" }]; - // Output only. The time this resource was last updated. - google.protobuf.Timestamp updated_time = 70 [json_name="updated_time"]; + // Output only. The time this resource was created. + google.protobuf.Timestamp created_time = 60 [json_name = "created_time"]; - // Version is used in mutation requests, after the initial creation, to ensure this resource has not changed. - // The mutation will fail if the version does not match the latest known good version. - uint32 version = 80; + // Output only. The time this resource was last updated. + google.protobuf.Timestamp updated_time = 70 [json_name = "updated_time"]; - // Output only. Contains the list of Account IDs linked to this User. - repeated string account_ids = 90 [json_name="account_ids"]; + // Version is used in mutation requests, after the initial creation, to ensure this resource has not changed. + // The mutation will fail if the version does not match the latest known good version. + uint32 version = 80; - // Output only. The Accounts linked to this User. - repeated Account accounts = 100; + // Output only. Contains the list of Account IDs linked to this User. + repeated string account_ids = 90 [json_name = "account_ids"]; - // Output only. The available actions on this resource for this user. - repeated string authorized_actions = 300 [json_name="authorized_actions"]; + // Output only. The Accounts linked to this User. + repeated Account accounts = 100; + + // Output only. The available actions on this resource for this user. + repeated string authorized_actions = 300 [json_name = "authorized_actions"]; + + // Output only. login_name is a string that maps to the user's account "login + // name" from the scope's primary auth method + string login_name = 110 [json_name = "login_name"]; + + // Output only. full_name is a string that maps to the user's account name + // from the scope's primary auth method + string full_name = 120 [json_name = "full_name"]; + + // Output only. email is a string that maps to the user's account email from + // the scope's primary auth method + string email = 130 [json_name = "email"]; + + // Output only. primary_account_id is a string that maps to the user's account + // public_id from the scope's primary auth method + string primary_account_id = 140 [json_name = "primary_account_id"]; } diff --git a/internal/proto/local/controller/storage/iam/store/v1/user.proto b/internal/proto/local/controller/storage/iam/store/v1/user.proto index 2796fff9e..8b454ad8b 100644 --- a/internal/proto/local/controller/storage/iam/store/v1/user.proto +++ b/internal/proto/local/controller/storage/iam/store/v1/user.proto @@ -18,11 +18,11 @@ message User { // name is the optional friendly name used to // access the User via an API // @inject_tag: `gorm:"default:null"` - string name = 30 [(custom_options.v1.mask_mapping) = {this:"name" that: "name"}]; + string name = 30 [(custom_options.v1.mask_mapping) = { this: "name" that: "name" }]; // description of the user // @inject_tag: `gorm:"default:null"` - string description = 40 [(custom_options.v1.mask_mapping) = {this:"description" that: "description"}]; + string description = 40 [(custom_options.v1.mask_mapping) = { this: "description" that: "description" }]; // create_time from the RDBMS // @inject_tag: `gorm:"default:current_timestamp"` @@ -35,4 +35,24 @@ message User { // version allows optimistic locking of the user // @inject_tag: `gorm:"default:null"` uint32 version = 70; + + // Output only. login_name is a string that maps to the user's account "login + // name" from the scope's primary auth method + // @inject_tag: `gorm:"-"` + string login_name = 90 [json_name = "login_name"]; + + // Output only. full_name is a string that maps to the user's account name + // from the scope's primary auth method + // @inject_tag: `gorm:"-"` + string full_name = 100 [json_name = "full_name"]; + + // Output only. email is a string that maps to the user's account email from + // the scope's primary auth method + // @inject_tag: `gorm:"-"` + string email = 110 [json_name = "email"]; + + // Output only. primary_account_id is a string that maps to the user's account + // public_id from the scope's primary auth method + // @inject_tag: `gorm:"-"` + string primary_account_id = 120 [json_name = "primary_account_id"]; } \ No newline at end of file diff --git a/internal/servers/controller/handlers/accounts/account_service_test.go b/internal/servers/controller/handlers/accounts/account_service_test.go index 4c048957c..12f87e811 100644 --- a/internal/servers/controller/handlers/accounts/account_service_test.go +++ b/internal/servers/controller/handlers/accounts/account_service_test.go @@ -121,7 +121,7 @@ func TestGet(t *testing.T) { org, _ := iam.TestScopes(t, iam.TestRepo(t, conn, wrap)) am := password.TestAuthMethods(t, conn, org.GetPublicId(), 1)[0] - pwA := password.TestAccounts(t, conn, am.GetPublicId(), 1)[0] + pwA := password.TestAccount(t, conn, am.GetPublicId(), "name1") pwWireAccount := pb.Account{ Id: pwA.GetPublicId(), @@ -236,7 +236,7 @@ func TestListPassword(t *testing.T) { amNoAccounts, amSomeAccounts, amOtherAccounts := ams[0], ams[1], ams[2] var wantSomeAccounts []*pb.Account - for _, aa := range password.TestAccounts(t, conn, amSomeAccounts.GetPublicId(), 3) { + for _, aa := range password.TestMultipleAccounts(t, conn, amSomeAccounts.GetPublicId(), 3) { wantSomeAccounts = append(wantSomeAccounts, &pb.Account{ Id: aa.GetPublicId(), AuthMethodId: aa.GetAuthMethodId(), @@ -251,7 +251,7 @@ func TestListPassword(t *testing.T) { } var wantOtherAccounts []*pb.Account - for _, aa := range password.TestAccounts(t, conn, amOtherAccounts.GetPublicId(), 3) { + for _, aa := range password.TestMultipleAccounts(t, conn, amOtherAccounts.GetPublicId(), 3) { wantOtherAccounts = append(wantOtherAccounts, &pb.Account{ Id: aa.GetPublicId(), AuthMethodId: aa.GetAuthMethodId(), @@ -487,7 +487,7 @@ func TestDelete(t *testing.T) { o, _ := iam.TestScopes(t, iam.TestRepo(t, conn, wrap)) am1 := password.TestAuthMethods(t, conn, o.GetPublicId(), 1)[0] - ac := password.TestAccounts(t, conn, am1.GetPublicId(), 1)[0] + ac := password.TestAccount(t, conn, am1.GetPublicId(), "name1") databaseWrapper, err := kmsCache.GetWrapper(ctx, o.PublicId, kms.KeyPurposeDatabase) require.NoError(t, err) @@ -577,7 +577,7 @@ func TestDelete_twice(t *testing.T) { o, _ := iam.TestScopes(t, iam.TestRepo(t, conn, wrap)) am := password.TestAuthMethods(t, conn, o.GetPublicId(), 1)[0] - ac := password.TestAccounts(t, conn, am.GetPublicId(), 1)[0] + ac := password.TestAccount(t, conn, am.GetPublicId(), "name1") s, err := accounts.NewService(pwRepoFn, oidcRepoFn) require.NoError(err, "Error when getting new user service") @@ -611,7 +611,7 @@ func TestCreatePassword(t *testing.T) { o, _ := iam.TestScopes(t, iam.TestRepo(t, conn, wrap)) am := password.TestAuthMethods(t, conn, o.GetPublicId(), 1)[0] - defaultAccount := password.TestAccounts(t, conn, am.GetPublicId(), 1)[0] + defaultAccount := password.TestAccount(t, conn, am.GetPublicId(), "name1") defaultCreated := defaultAccount.GetCreateTime().GetTimestamp() require.NoError(t, err, "Error converting proto to timestamp.") diff --git a/internal/servers/controller/handlers/users/user_service.go b/internal/servers/controller/handlers/users/user_service.go index 909f83afd..f6eef1c4c 100644 --- a/internal/servers/controller/handlers/users/user_service.go +++ b/internal/servers/controller/handlers/users/user_service.go @@ -473,6 +473,18 @@ func toProto(in *iam.User, accts []string) *pb.User { if in.GetName() != "" { out.Name = &wrapperspb.StringValue{Value: in.GetName()} } + if in.GetPrimaryAccountId() != "" { + out.PrimaryAccountId = in.GetPrimaryAccountId() + } + if in.GetLoginName() != "" { + out.LoginName = in.GetLoginName() + } + if in.GetFullName() != "" { + out.FullName = in.GetFullName() + } + if in.GetEmail() != "" { + out.Email = in.GetEmail() + } for _, a := range accts { out.Accounts = append(out.Accounts, &pb.Account{ Id: a, diff --git a/internal/servers/controller/handlers/users/user_service_test.go b/internal/servers/controller/handlers/users/user_service_test.go index 6d5ccb651..0ce970020 100644 --- a/internal/servers/controller/handlers/users/user_service_test.go +++ b/internal/servers/controller/handlers/users/user_service_test.go @@ -4,10 +4,10 @@ import ( "context" "errors" "fmt" + "sort" "strings" "testing" - "github.com/golang/protobuf/ptypes" "github.com/google/go-cmp/cmp" "github.com/hashicorp/boundary/internal/auth" "github.com/hashicorp/boundary/internal/auth/oidc" @@ -25,13 +25,14 @@ import ( "google.golang.org/grpc/codes" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/testing/protocmp" + "google.golang.org/protobuf/types/known/timestamppb" "google.golang.org/protobuf/types/known/wrapperspb" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -func createDefaultUserAndRepo(t *testing.T) (*iam.User, func() (*iam.Repository, error)) { +func createDefaultUserAndRepo(t *testing.T, withAccts bool) (*iam.User, []string, func() (*iam.Repository, error)) { t.Helper() conn, _ := db.TestSetup(t, "postgres") wrap := db.TestWrapper(t) @@ -41,11 +42,43 @@ func createDefaultUserAndRepo(t *testing.T) (*iam.User, func() (*iam.Repository, } o, _ := iam.TestScopes(t, repo) u := iam.TestUser(t, repo, o.GetPublicId(), iam.WithDescription("default"), iam.WithName("default")) - return u, repoFn + + switch withAccts { + case false: + return u, nil, repoFn + default: + require := require.New(t) + ctx := context.Background() + kmsCache := kms.TestKms(t, conn, wrap) + databaseWrap, err := kmsCache.GetWrapper(ctx, o.PublicId, kms.KeyPurposeDatabase) + require.NoError(err) + primaryAm := oidc.TestAuthMethod(t, conn, databaseWrap, o.PublicId, oidc.ActivePublicState, "alice-rp", "fido", + oidc.WithIssuer(oidc.TestConvertToUrls(t, "https://alice-eve-smith.com")[0]), + oidc.WithApiUrl(oidc.TestConvertToUrls(t, "http://localhost:9200")[0]), + oidc.WithSigningAlgs(oidc.RS256), + ) + iam.TestSetPrimaryAuthMethod(t, repo, o, primaryAm.PublicId) + + oidcAcct := oidc.TestAccount(t, conn, primaryAm, "alice", oidc.WithFullName("Alice Eve Smith"), oidc.WithEmail("alice@smith.com")) + + secondaryAm := password.TestAuthMethods(t, conn, o.PublicId, 1) + require.Len(secondaryAm, 1) + pwAcct := password.TestAccount(t, conn, secondaryAm[0].PublicId, "alice") + + added, err := repo.AddUserAccounts(ctx, u.PublicId, u.Version, []string{oidcAcct.PublicId, pwAcct.PublicId}) + require.NoError(err) + require.Len(added, 2) + + // reload the user with their accounts + u, accts, err := repo.LookupUser(ctx, u.PublicId) + require.NoError(err) + return u, accts, repoFn + } } func TestGet(t *testing.T) { - u, repoFn := createDefaultUserAndRepo(t) + u, uAccts, repoFn := createDefaultUserAndRepo(t, true) + toMerge := &pbs.GetUserRequest{ Id: u.GetPublicId(), } @@ -58,8 +91,14 @@ func TestGet(t *testing.T) { Description: &wrapperspb.StringValue{Value: u.GetDescription()}, CreatedTime: u.CreateTime.GetTimestamp(), UpdatedTime: u.UpdateTime.GetTimestamp(), - Version: 1, + Version: u.Version, AuthorizedActions: []string{"read", "update", "delete", "add-accounts", "set-accounts", "remove-accounts"}, + LoginName: u.LoginName, + FullName: u.GetFullName(), + Email: u.GetEmail(), + PrimaryAccountId: u.GetPrimaryAccountId(), + AccountIds: uAccts, + Accounts: []*pb.Account{{Id: uAccts[0], ScopeId: u.ScopeId}, {Id: uAccts[1], ScopeId: u.ScopeId}}, } cases := []struct { @@ -124,7 +163,21 @@ func TestList(t *testing.T) { oNoUsers, _ := iam.TestScopes(t, repo) oWithUsers, _ := iam.TestScopes(t, repo) + kmsCache := kms.TestKms(t, conn, wrap) + databaseWrap, err := kmsCache.GetWrapper(context.Background(), oWithUsers.PublicId, kms.KeyPurposeDatabase) + require.NoError(t, err) + primaryAm := oidc.TestAuthMethod(t, conn, databaseWrap, oWithUsers.PublicId, oidc.ActivePublicState, "alice-rp", "fido", + oidc.WithIssuer(oidc.TestConvertToUrls(t, "https://alice-eve-smith.com")[0]), + oidc.WithApiUrl(oidc.TestConvertToUrls(t, "http://localhost:9200")[0]), + oidc.WithSigningAlgs(oidc.RS256), + ) + iam.TestSetPrimaryAuthMethod(t, repo, oWithUsers, primaryAm.PublicId) + + secondaryAm := password.TestAuthMethods(t, conn, oWithUsers.PublicId, 1) + require.Len(t, secondaryAm, 1) + s, err := users.NewService(repoFn) + require.NoError(t, err) var wantUsers []*pb.User @@ -147,14 +200,27 @@ func TestList(t *testing.T) { require.NoError(t, err) u, err := repo.CreateUser(context.Background(), newU) require.NoError(t, err) + oidcAcct := oidc.TestAccount(t, conn, primaryAm, fmt.Sprintf("alice+%d", i), oidc.WithFullName("Alice Eve Smith"), oidc.WithEmail("alice@smith.com")) + pwAcct := password.TestAccount(t, conn, secondaryAm[0].PublicId, fmt.Sprintf("alice+%d", i)) + + added, err := repo.AddUserAccounts(ctx, u.PublicId, u.Version, []string{oidcAcct.PublicId, pwAcct.PublicId}) + require.NoError(t, err) + require.Len(t, added, 2) + + u, _, err = repo.LookupUser(ctx, u.PublicId) + require.NoError(t, err) wantUsers = append(wantUsers, &pb.User{ Id: u.GetPublicId(), ScopeId: u.GetScopeId(), Scope: &scopes.ScopeInfo{Id: u.GetScopeId(), Type: scope.Org.String(), ParentScopeId: scope.Global.String()}, CreatedTime: u.GetCreateTime().GetTimestamp(), UpdatedTime: u.GetUpdateTime().GetTimestamp(), - Version: 1, + Version: 2, AuthorizedActions: []string{"read", "update", "delete", "add-accounts", "set-accounts", "remove-accounts"}, + LoginName: oidcAcct.GetSubject(), + FullName: oidcAcct.GetFullName(), + Email: oidcAcct.GetEmail(), + PrimaryAccountId: oidcAcct.GetPublicId(), }) } @@ -215,13 +281,35 @@ func TestList(t *testing.T) { require.Error(gErr) assert.True(errors.Is(gErr, tc.err), "ListUsers(%+v) got error %v, wanted %v", tc.req, gErr, tc.err) } + gotUsers := sortableUsers{ + users: got.GetItems(), + } + resUsers := sortableUsers{ + users: tc.res.GetItems(), + } + if got != nil { + sort.Sort(gotUsers) + got.Items = gotUsers.users + } + if tc.res != nil { + sort.Sort(resUsers) + tc.res.Items = resUsers.users + } assert.Empty(cmp.Diff(got, tc.res, protocmp.Transform()), "ListUsers(%q) got response %q, wanted %q", tc.req, got, tc.res) }) } } +type sortableUsers struct { + users []*pb.User +} + +func (s sortableUsers) Len() int { return len(s.users) } +func (s sortableUsers) Less(i, j int) bool { return s.users[i].GetId() < s.users[j].GetId() } +func (s sortableUsers) Swap(i, j int) { s.users[i], s.users[j] = s.users[j], s.users[i] } + func TestDelete(t *testing.T) { - u, repoFn := createDefaultUserAndRepo(t) + u, _, repoFn := createDefaultUserAndRepo(t, false) s, err := users.NewService(repoFn) require.NoError(t, err, "Error when getting new user service.") @@ -268,7 +356,7 @@ func TestDelete(t *testing.T) { func TestDelete_twice(t *testing.T) { assert, require := assert.New(t), require.New(t) - u, repoFn := createDefaultUserAndRepo(t) + u, _, repoFn := createDefaultUserAndRepo(t, false) s, err := users.NewService(repoFn) require.NoError(err, "Error when getting new user service") @@ -284,9 +372,8 @@ func TestDelete_twice(t *testing.T) { } func TestCreate(t *testing.T) { - defaultUser, repoFn := createDefaultUserAndRepo(t) - defaultCreated, err := ptypes.Timestamp(defaultUser.GetCreateTime().GetTimestamp()) - require.NoError(t, err, "Error converting proto to timestamp.") + defaultUser, _, repoFn := createDefaultUserAndRepo(t, false) + defaultCreated := defaultUser.GetCreateTime().GetTimestamp().AsTime() cases := []struct { name string @@ -345,7 +432,7 @@ func TestCreate(t *testing.T) { name: "Can't specify Created Time", req: &pbs.CreateUserRequest{Item: &pb.User{ ScopeId: defaultUser.GetScopeId(), - CreatedTime: ptypes.TimestampNow(), + CreatedTime: timestamppb.Now(), }}, res: nil, err: handlers.ApiErrorWithCode(codes.InvalidArgument), @@ -354,7 +441,7 @@ func TestCreate(t *testing.T) { name: "Can't specify Update Time", req: &pbs.CreateUserRequest{Item: &pb.User{ ScopeId: defaultUser.GetScopeId(), - UpdatedTime: ptypes.TimestampNow(), + UpdatedTime: timestamppb.Now(), }}, res: nil, err: handlers.ApiErrorWithCode(codes.InvalidArgument), @@ -374,10 +461,8 @@ func TestCreate(t *testing.T) { if got != nil { assert.Contains(got.GetUri(), tc.res.Uri) assert.True(strings.HasPrefix(got.GetItem().GetId(), iam.UserPrefix+"_")) - gotCreateTime, err := ptypes.Timestamp(got.GetItem().GetCreatedTime()) - require.NoError(err, "Error converting proto to timestamp.") - gotUpdateTime, err := ptypes.Timestamp(got.GetItem().GetUpdatedTime()) - require.NoError(err, "Error converting proto to timestamp.") + gotCreateTime := got.GetItem().GetCreatedTime().AsTime() + gotUpdateTime := got.GetItem().GetUpdatedTime().AsTime() // Verify it is a user created after the test setup's default user assert.True(gotCreateTime.After(defaultCreated), "New user should have been created after default user. Was created %v, which is after %v", gotCreateTime, defaultCreated) assert.True(gotUpdateTime.After(defaultCreated), "New user should have been updated after default user. Was updated %v, which is after %v", gotUpdateTime, defaultCreated) @@ -393,12 +478,11 @@ func TestCreate(t *testing.T) { } func TestUpdate(t *testing.T) { - u, repoFn := createDefaultUserAndRepo(t) + u, _, repoFn := createDefaultUserAndRepo(t, false) tested, err := users.NewService(repoFn) require.NoError(t, err, "Error when getting new user service.") - created, err := ptypes.Timestamp(u.GetCreateTime().GetTimestamp()) - require.NoError(t, err, "Error converting proto to timestamp") + created := u.GetCreateTime().GetTimestamp().AsTime() toMerge := &pbs.UpdateUserRequest{ Id: u.GetPublicId(), } @@ -602,7 +686,7 @@ func TestUpdate(t *testing.T) { Paths: []string{"created_time"}, }, Item: &pb.User{ - CreatedTime: ptypes.TimestampNow(), + CreatedTime: timestamppb.Now(), }, }, res: nil, @@ -615,7 +699,7 @@ func TestUpdate(t *testing.T) { Paths: []string{"updated_time"}, }, Item: &pb.User{ - UpdatedTime: ptypes.TimestampNow(), + UpdatedTime: timestamppb.Now(), }, }, res: nil, @@ -649,8 +733,7 @@ func TestUpdate(t *testing.T) { if got != nil { assert.NotNilf(tc.res, "Expected UpdateUser response to be nil, but was %v", got) - gotUpdateTime, err := ptypes.Timestamp(got.GetItem().GetUpdatedTime()) - require.NoError(err, "Error converting proto to timestamp") + gotUpdateTime := got.GetItem().GetUpdatedTime().AsTime() // Verify it is a user updated after it was created assert.True(gotUpdateTime.After(created), "Updated user should have been updated after it's creation. Was updated %v, which is after %v", gotUpdateTime, created) @@ -676,10 +759,16 @@ func TestAddAccount(t *testing.T) { require.NoError(t, err, "Error when getting new user service.") o, _ := iam.TestScopes(t, iamRepo) - amId := password.TestAuthMethods(t, conn, o.GetPublicId(), 1)[0].GetPublicId() - accts := password.TestAccounts(t, conn, amId, 3) + acctCnt := 3 + accts := make([]*password.Account, 0, acctCnt) + for i := 0; i < acctCnt; i++ { + amId := password.TestAuthMethods(t, conn, o.GetPublicId(), 1)[0].GetPublicId() + newAcct := password.TestAccount(t, conn, amId, "name1") + accts = append(accts, newAcct) + } databaseWrapper, err := kmsCache.GetWrapper(context.Background(), o.PublicId, kms.KeyPurposeDatabase) + require.NoError(t, err) oidcAm := oidc.TestAuthMethod( t, conn, databaseWrapper, o.PublicId, oidc.ActivePrivateState, "alice-rp", "fido", @@ -733,7 +822,7 @@ func TestAddAccount(t *testing.T) { { name: "Add empty on populated user", setup: func(u *iam.User) { - iamRepo.SetUserAccounts(context.Background(), u.GetPublicId(), u.GetVersion(), + _, err := iamRepo.SetUserAccounts(context.Background(), u.GetPublicId(), u.GetVersion(), []string{accts[0].GetPublicId(), accts[1].GetPublicId()}) require.NoError(t, err) u.Version = u.Version + 1 @@ -815,10 +904,16 @@ func TestSetAccount(t *testing.T) { require.NoError(t, err, "Error when getting new user service.") o, _ := iam.TestScopes(t, iamRepo) - amId := password.TestAuthMethods(t, conn, o.GetPublicId(), 1)[0].GetPublicId() - accts := password.TestAccounts(t, conn, amId, 3) + acctCnt := 3 + accts := make([]*password.Account, 0, acctCnt) + for i := 0; i < acctCnt; i++ { + amId := password.TestAuthMethods(t, conn, o.GetPublicId(), 1)[0].GetPublicId() + newAcct := password.TestAccount(t, conn, amId, "name1") + accts = append(accts, newAcct) + } databaseWrapper, err := kmsCache.GetWrapper(context.Background(), o.PublicId, kms.KeyPurposeDatabase) + require.NoError(t, err) oidcAm := oidc.TestAuthMethod( t, conn, databaseWrapper, o.PublicId, oidc.ActivePrivateState, "alice-rp", "fido", @@ -850,7 +945,7 @@ func TestSetAccount(t *testing.T) { { name: "Set account on populated user", setup: func(u *iam.User) { - iamRepo.AddUserAccounts(context.Background(), u.GetPublicId(), u.GetVersion(), + _, err := iamRepo.AddUserAccounts(context.Background(), u.GetPublicId(), u.GetVersion(), []string{accts[0].GetPublicId()}) require.NoError(t, err) u.Version = u.Version + 1 @@ -861,7 +956,7 @@ func TestSetAccount(t *testing.T) { { name: "Set duplicate account on populated user", setup: func(u *iam.User) { - iamRepo.AddUserAccounts(context.Background(), u.GetPublicId(), u.GetVersion(), + _, err := iamRepo.AddUserAccounts(context.Background(), u.GetPublicId(), u.GetVersion(), []string{accts[0].GetPublicId()}) require.NoError(t, err) u.Version = u.Version + 1 @@ -872,7 +967,7 @@ func TestSetAccount(t *testing.T) { { name: "Set empty on populated user", setup: func(u *iam.User) { - iamRepo.AddUserAccounts(context.Background(), u.GetPublicId(), u.GetVersion(), + _, err := iamRepo.AddUserAccounts(context.Background(), u.GetPublicId(), u.GetVersion(), []string{accts[0].GetPublicId(), accts[1].GetPublicId()}) require.NoError(t, err) u.Version = u.Version + 1 @@ -886,7 +981,7 @@ func TestSetAccount(t *testing.T) { t.Run(tc.name, func(t *testing.T) { usr := iam.TestUser(t, iamRepo, o.GetPublicId()) defer func() { - iamRepo.DeleteUser(context.Background(), usr.GetPublicId()) + _, _ = iamRepo.DeleteUser(context.Background(), usr.GetPublicId()) }() tc.setup(usr) @@ -955,8 +1050,13 @@ func TestRemoveAccount(t *testing.T) { require.NoError(t, err, "Error when getting new user service.") o, _ := iam.TestScopes(t, iamRepo) - amId := password.TestAuthMethods(t, conn, o.GetPublicId(), 1)[0].GetPublicId() - accts := password.TestAccounts(t, conn, amId, 3) + acctCnt := 3 + accts := make([]*password.Account, 0, acctCnt) + for i := 0; i < acctCnt; i++ { + amId := password.TestAuthMethods(t, conn, o.GetPublicId(), 1)[0].GetPublicId() + newAcct := password.TestAccount(t, conn, amId, "name1") + accts = append(accts, newAcct) + } databaseWrapper, err := kmsCache.GetWrapper(context.Background(), o.PublicId, kms.KeyPurposeDatabase) require.NoError(t, err) @@ -1043,7 +1143,7 @@ func TestRemoveAccount(t *testing.T) { t.Run(tc.name, func(t *testing.T) { usr := iam.TestUser(t, iamRepo, o.GetPublicId()) defer func() { - iamRepo.DeleteUser(context.Background(), usr.GetPublicId()) + _, _ = iamRepo.DeleteUser(context.Background(), usr.GetPublicId()) }() tc.setup(usr) req := &pbs.RemoveUserAccountsRequest{ diff --git a/internal/session/testing.go b/internal/session/testing.go index 05d96e9a5..34d3fdffa 100644 --- a/internal/session/testing.go +++ b/internal/session/testing.go @@ -137,7 +137,7 @@ func TestSessionParams(t *testing.T, conn *gorm.DB, wrapper wrapping.Wrapper, ia require.NoError(err) authMethod := password.TestAuthMethods(t, conn, org.PublicId, 1)[0] - acct := password.TestAccounts(t, conn, authMethod.GetPublicId(), 1)[0] + acct := password.TestAccount(t, conn, authMethod.GetPublicId(), "name1") user := iam.TestUser(t, iamRepo, org.PublicId, iam.WithAccountIds(acct.PublicId)) authTokenRepo, err := authtoken.NewRepository(rw, rw, kms) diff --git a/internal/tests/api/users/user_test.go b/internal/tests/api/users/user_test.go index 9e2413e65..f72212787 100644 --- a/internal/tests/api/users/user_test.go +++ b/internal/tests/api/users/user_test.go @@ -7,6 +7,7 @@ import ( "github.com/hashicorp/boundary/api" "github.com/hashicorp/boundary/api/accounts" + "github.com/hashicorp/boundary/api/authmethods" "github.com/hashicorp/boundary/api/users" "github.com/hashicorp/boundary/internal/iam" "github.com/hashicorp/boundary/internal/servers/controller" @@ -14,6 +15,8 @@ import ( "github.com/stretchr/testify/require" ) +const global = "global" + func TestCustom(t *testing.T) { assert, require := assert.New(t), require.New(t) tc := controller.NewTestController(t, nil) @@ -24,13 +27,18 @@ func TestCustom(t *testing.T) { client.SetToken(token.Token) uClient := users.NewClient(client) aClient := accounts.NewClient(client) + amClient := authmethods.NewClient(client) - usr1, err := uClient.Create(tc.Context(), "global") + userAm, err := amClient.Create(tc.Context(), "password", global, + authmethods.WithName("bar")) + require.NoError(err) + + usr1, err := uClient.Create(tc.Context(), global) require.NoError(err) acct1, err := aClient.Create(tc.Context(), token.AuthMethodId, accounts.WithPasswordAccountLoginName("accountname")) require.NoError(err) - acct2, err := aClient.Create(tc.Context(), token.AuthMethodId, accounts.WithPasswordAccountLoginName("accountname2")) + acct2, err := aClient.Create(tc.Context(), userAm.Item.Id, accounts.WithPasswordAccountLoginName("accountname2")) require.NoError(err) addResult, err := uClient.AddAccounts(tc.Context(), usr1.Item.Id, 0, []string{acct1.Item.Id, acct2.Item.Id}, users.WithAutomaticVersioning(true))