From fad9f6d52580cbb9b291f8ec5124ae366ad6d47c Mon Sep 17 00:00:00 2001 From: Todd Knight Date: Wed, 14 Apr 2021 11:34:17 -0700 Subject: [PATCH] Allow oidc accounts to be add|set|removed to users. (#1117) --- .../controller/handlers/users/user_service.go | 7 ++- .../handlers/users/user_service_test.go | 58 +++++++++++++++++++ 2 files changed, 62 insertions(+), 3 deletions(-) diff --git a/internal/servers/controller/handlers/users/user_service.go b/internal/servers/controller/handlers/users/user_service.go index f760b3eff4..db213c97c1 100644 --- a/internal/servers/controller/handlers/users/user_service.go +++ b/internal/servers/controller/handlers/users/user_service.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/hashicorp/boundary/internal/auth" + "github.com/hashicorp/boundary/internal/auth/oidc" "github.com/hashicorp/boundary/internal/auth/password" "github.com/hashicorp/boundary/internal/errors" pb "github.com/hashicorp/boundary/internal/gen/controller/api/resources/users" @@ -531,7 +532,7 @@ func validateAddUserAccountsRequest(req *pbs.AddUserAccountsRequest) error { } for _, a := range req.GetAccountIds() { // TODO: Increase the type of auth accounts that can be added to a user. - if !handlers.ValidId(handlers.Id(a), password.AccountPrefix) { + if !handlers.ValidId(handlers.Id(a), password.AccountPrefix, oidc.AccountPrefix) { badFields["account_ids"] = "Values must be valid account ids." break } @@ -552,7 +553,7 @@ func validateSetUserAccountsRequest(req *pbs.SetUserAccountsRequest) error { } for _, a := range req.GetAccountIds() { // TODO: Increase the type of auth accounts that can be added to a user. - if !handlers.ValidId(handlers.Id(a), password.AccountPrefix) { + if !handlers.ValidId(handlers.Id(a), password.AccountPrefix, oidc.AccountPrefix) { badFields["account_ids"] = "Values must be valid account ids." break } @@ -576,7 +577,7 @@ func validateRemoveUserAccountsRequest(req *pbs.RemoveUserAccountsRequest) error } for _, a := range req.GetAccountIds() { // TODO: Increase the type of auth accounts that can be added to a user. - if !handlers.ValidId(handlers.Id(a), password.AccountPrefix) { + if !handlers.ValidId(handlers.Id(a), password.AccountPrefix, oidc.AccountPrefix) { badFields["account_ids"] = "Values must be valid account ids." break } diff --git a/internal/servers/controller/handlers/users/user_service_test.go b/internal/servers/controller/handlers/users/user_service_test.go index 2f67b7c86a..38156d8b3a 100644 --- a/internal/servers/controller/handlers/users/user_service_test.go +++ b/internal/servers/controller/handlers/users/user_service_test.go @@ -10,12 +10,14 @@ import ( "github.com/golang/protobuf/ptypes" "github.com/google/go-cmp/cmp" "github.com/hashicorp/boundary/internal/auth" + "github.com/hashicorp/boundary/internal/auth/oidc" "github.com/hashicorp/boundary/internal/auth/password" "github.com/hashicorp/boundary/internal/db" "github.com/hashicorp/boundary/internal/gen/controller/api/resources/scopes" pb "github.com/hashicorp/boundary/internal/gen/controller/api/resources/users" pbs "github.com/hashicorp/boundary/internal/gen/controller/api/services" "github.com/hashicorp/boundary/internal/iam" + "github.com/hashicorp/boundary/internal/kms" "github.com/hashicorp/boundary/internal/servers/controller/handlers" "github.com/hashicorp/boundary/internal/servers/controller/handlers/users" "github.com/hashicorp/boundary/internal/types/scope" @@ -665,6 +667,7 @@ func TestUpdate(t *testing.T) { func TestAddAccount(t *testing.T) { conn, _ := db.TestSetup(t, "postgres") wrap := db.TestWrapper(t) + kmsCache := kms.TestKms(t, conn, wrap) iamRepo := iam.TestRepo(t, conn, wrap) repoFn := func() (*iam.Repository, error) { return iamRepo, nil @@ -676,6 +679,16 @@ func TestAddAccount(t *testing.T) { amId := password.TestAuthMethods(t, conn, o.GetPublicId(), 1)[0].GetPublicId() accts := password.TestAccounts(t, conn, amId, 3) + databaseWrapper, err := kmsCache.GetWrapper(context.Background(), o.PublicId, kms.KeyPurposeDatabase) + oidcAm := oidc.TestAuthMethod( + t, conn, databaseWrapper, o.PublicId, oidc.ActivePrivateState, + "alice-rp", "fido", + oidc.WithIssuer(oidc.TestConvertToUrls(t, "https://www.alice.com")[0]), + oidc.WithSigningAlgs(oidc.RS256), + oidc.WithApiUrl(oidc.TestConvertToUrls(t, "https://www.alice.com/callback")[0]), + ) + oidcAcct := oidc.TestAccount(t, conn, oidcAm, "test-subject") + addCases := []struct { name string setup func(*iam.User) @@ -689,6 +702,12 @@ func TestAddAccount(t *testing.T) { addAccounts: []string{accts[1].GetPublicId()}, resultAccounts: []string{accts[1].GetPublicId()}, }, + { + name: "Add oidc account on empty user", + setup: func(u *iam.User) {}, + addAccounts: []string{oidcAcct.GetPublicId()}, + resultAccounts: []string{oidcAcct.GetPublicId()}, + }, { name: "Add account on populated user", setup: func(u *iam.User) { @@ -787,6 +806,7 @@ func TestAddAccount(t *testing.T) { func TestSetAccount(t *testing.T) { conn, _ := db.TestSetup(t, "postgres") wrap := db.TestWrapper(t) + kmsCache := kms.TestKms(t, conn, wrap) iamRepo := iam.TestRepo(t, conn, wrap) repoFn := func() (*iam.Repository, error) { return iamRepo, nil @@ -798,6 +818,16 @@ func TestSetAccount(t *testing.T) { amId := password.TestAuthMethods(t, conn, o.GetPublicId(), 1)[0].GetPublicId() accts := password.TestAccounts(t, conn, amId, 3) + databaseWrapper, err := kmsCache.GetWrapper(context.Background(), o.PublicId, kms.KeyPurposeDatabase) + oidcAm := oidc.TestAuthMethod( + t, conn, databaseWrapper, o.PublicId, oidc.ActivePrivateState, + "alice-rp", "fido", + oidc.WithIssuer(oidc.TestConvertToUrls(t, "https://www.alice.com")[0]), + oidc.WithSigningAlgs(oidc.RS256), + oidc.WithApiUrl(oidc.TestConvertToUrls(t, "https://www.alice.com/callback")[0]), + ) + oidcAcct := oidc.TestAccount(t, conn, oidcAm, "test-subject") + setCases := []struct { name string setup func(*iam.User) @@ -811,6 +841,12 @@ func TestSetAccount(t *testing.T) { setAccounts: []string{accts[1].GetPublicId()}, resultAccounts: []string{accts[1].GetPublicId()}, }, + { + name: "Set oidc account on empty user", + setup: func(u *iam.User) {}, + setAccounts: []string{oidcAcct.GetPublicId()}, + resultAccounts: []string{oidcAcct.GetPublicId()}, + }, { name: "Set account on populated user", setup: func(u *iam.User) { @@ -910,6 +946,7 @@ func TestSetAccount(t *testing.T) { func TestRemoveAccount(t *testing.T) { conn, _ := db.TestSetup(t, "postgres") wrap := db.TestWrapper(t) + kmsCache := kms.TestKms(t, conn, wrap) iamRepo := iam.TestRepo(t, conn, wrap) repoFn := func() (*iam.Repository, error) { return iamRepo, nil @@ -921,6 +958,16 @@ func TestRemoveAccount(t *testing.T) { amId := password.TestAuthMethods(t, conn, o.GetPublicId(), 1)[0].GetPublicId() accts := password.TestAccounts(t, conn, amId, 3) + databaseWrapper, err := kmsCache.GetWrapper(context.Background(), o.PublicId, kms.KeyPurposeDatabase) + oidcAm := oidc.TestAuthMethod( + t, conn, databaseWrapper, o.PublicId, oidc.ActivePrivateState, + "alice-rp", "fido", + oidc.WithIssuer(oidc.TestConvertToUrls(t, "https://www.alice.com")[0]), + oidc.WithSigningAlgs(oidc.RS256), + oidc.WithApiUrl(oidc.TestConvertToUrls(t, "https://www.alice.com/callback")[0]), + ) + oidcAcct := oidc.TestAccount(t, conn, oidcAm, "test-subject") + addCases := []struct { name string setup func(*iam.User) @@ -945,6 +992,17 @@ func TestRemoveAccount(t *testing.T) { removeAccounts: []string{accts[1].GetPublicId()}, resultAccounts: []string{accts[0].GetPublicId()}, }, + { + name: "Remove 1 oidc account of 2 accounts from user", + setup: func(u *iam.User) { + _, err := iamRepo.SetUserAccounts(context.Background(), u.GetPublicId(), u.GetVersion(), + []string{accts[0].GetPublicId(), oidcAcct.GetPublicId()}) + require.NoError(t, err) + u.Version = u.Version + 1 + }, + removeAccounts: []string{oidcAcct.GetPublicId()}, + resultAccounts: []string{accts[0].GetPublicId()}, + }, { name: "Remove 1 duplicate accounts of 2 accounts from user", setup: func(u *iam.User) {