From 7c6979df28f4a4f240dbc2309de43e0449225813 Mon Sep 17 00:00:00 2001 From: Caleb Washburn Date: Tue, 30 Jan 2024 10:04:59 -0800 Subject: [PATCH 1/9] adding additional logging to help triage uaa errors --- uaa/uaa.go | 1 + 1 file changed, 1 insertion(+) diff --git a/uaa/uaa.go b/uaa/uaa.go index ee900eb7..aa26a9f0 100644 --- a/uaa/uaa.go +++ b/uaa/uaa.go @@ -45,6 +45,7 @@ func NewDefaultUAAManager(sysDomain, clientID, clientSecret, userAgent string, p uaaclient.WithClientCredentials(clientID, clientSecret, uaaclient.OpaqueToken), uaaclient.WithUserAgent(userAgent), uaaclient.WithSkipSSLValidation(true), + uaaclient.WithVerbosity(true), ) if err != nil { return nil, err From 81e8a63cbe357da2d085618a249591c07851ccf1 Mon Sep 17 00:00:00 2001 From: Caleb Washburn Date: Wed, 7 Feb 2024 12:17:31 -0700 Subject: [PATCH 2/9] update to extract raw error message when uaa api call errors --- .gitignore | 1 + uaa/uaa.go | 10 +++++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 63c244c1..fff99f0e 100644 --- a/.gitignore +++ b/.gitignore @@ -19,3 +19,4 @@ publish-develop.sh make-binary.sh .idea test-for-pagination.sh +export-cli.sh diff --git a/uaa/uaa.go b/uaa/uaa.go index aa26a9f0..e47749b0 100644 --- a/uaa/uaa.go +++ b/uaa/uaa.go @@ -1,6 +1,7 @@ package uaa import ( + "errors" "fmt" "strings" @@ -45,7 +46,6 @@ func NewDefaultUAAManager(sysDomain, clientID, clientSecret, userAgent string, p uaaclient.WithClientCredentials(clientID, clientSecret, uaaclient.OpaqueToken), uaaclient.WithUserAgent(userAgent), uaaclient.WithSkipSSLValidation(true), - uaaclient.WithVerbosity(true), ) if err != nil { return nil, err @@ -78,6 +78,10 @@ func (m *DefaultUAAManager) CreateExternalUser(userName, userEmail, externalID, }, }) if err != nil { + var requestError uaaclient.RequestError + if errors.As(err, &requestError) { + return "", fmt.Errorf("got an error calling %s with response %s", requestError.Url, requestError.ErrorResponse) + } return "", err } lo.G.Infof("successfully added user [%s]", userName) @@ -90,6 +94,10 @@ func (m *DefaultUAAManager) ListUsers() (*Users, error) { lo.G.Debug("Getting users from Cloud Foundry") userList, err := m.Client.ListAllUsers("", "", "userName,id,externalId,emails,origin", "") if err != nil { + var requestError uaaclient.RequestError + if errors.As(err, &requestError) { + return nil, fmt.Errorf("got an error calling %s with response %s", requestError.Url, requestError.ErrorResponse) + } return nil, err } From 4b715b310df2e9f0f5e05e2c6a427b4824c1e1cf Mon Sep 17 00:00:00 2001 From: Caleb Washburn Date: Thu, 8 Feb 2024 13:47:37 -0700 Subject: [PATCH 3/9] update to add additional logging --- role/role_users.go | 2 ++ user/users.go | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/role/role_users.go b/role/role_users.go index 81aafa3c..4ed96a88 100644 --- a/role/role_users.go +++ b/role/role_users.go @@ -55,6 +55,7 @@ func (r *RoleUsers) OrphanedUsers() []string { func (r *RoleUsers) HasUserForGUID(userName, userGUID string) bool { userList := r.users[strings.ToLower(userName)] + lo.G.Debugf("Users %v found for userName [%s] and guid [%s]", userList, userName, userGUID) for _, user := range userList { if strings.EqualFold(user.GUID, userGUID) { return true @@ -65,6 +66,7 @@ func (r *RoleUsers) HasUserForGUID(userName, userGUID string) bool { func (r *RoleUsers) HasUserForOrigin(userName, origin string) bool { userList := r.users[strings.ToLower(userName)] + lo.G.Debugf("Users %v found for userName [%s] and origin [%s]", userList, userName, origin) for _, user := range userList { if strings.EqualFold(user.Origin, origin) { return true diff --git a/user/users.go b/user/users.go index 37d556cd..843dacf3 100644 --- a/user/users.go +++ b/user/users.go @@ -100,7 +100,7 @@ func (m *DefaultManager) updateSpaceUsers(input *config.SpaceConfig) error { } lo.G.Debug("") lo.G.Debug("") - lo.G.Debugf("Processing Org(%s)/Space(%s)", input.Org, input.Space) + lo.G.Debugf("Processing Org(%s/%s)/Space(%s/%s)", input.Org, space.Relationships.Organization.Data.GUID, input.Space, space.GUID) lo.G.Debug("") lo.G.Debug("") From edb29a57f4db0159bd53216a75f3771efc8f8bb2 Mon Sep 17 00:00:00 2001 From: Caleb Washburn Date: Thu, 29 Feb 2024 14:50:14 -0700 Subject: [PATCH 4/9] adding cf and uaa trace support --- commands/initialize.go | 13 +++++++++++++ uaa/uaa.go | 4 ++++ util/logging_transport.go | 39 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 56 insertions(+) create mode 100644 util/logging_transport.go diff --git a/commands/initialize.go b/commands/initialize.go index b3b93bab..b17ac8e2 100644 --- a/commands/initialize.go +++ b/commands/initialize.go @@ -2,6 +2,8 @@ package commands import ( "fmt" + "net/http" + "os" "strings" routing_api "code.cloudfoundry.org/routing-api" @@ -24,6 +26,7 @@ import ( "github.com/vmwarepivotallabs/cf-mgmt/space" "github.com/vmwarepivotallabs/cf-mgmt/uaa" "github.com/vmwarepivotallabs/cf-mgmt/user" + "github.com/vmwarepivotallabs/cf-mgmt/util" "github.com/xchapter7x/lo" ) @@ -92,6 +95,9 @@ func InitializePeekManagers(baseCommand BaseCFConfigCommand, peek bool, ldapMgr } cfMgmt.UAAManager = uaaMgr + httpClient := &http.Client{} + httpClient.Transport = util.NewLoggingTransport(http.DefaultTransport) + var c *cfclient.Config var cv3 *v3config.Config if baseCommand.Password != "" { @@ -103,6 +109,10 @@ func InitializePeekManagers(baseCommand BaseCFConfigCommand, peek bool, ldapMgr Password: baseCommand.Password, UserAgent: userAgent, } + if strings.EqualFold(os.Getenv("LOG_LEVEL"), "trace") { + c.HttpClient = httpClient + } + cv3, err = v3config.NewUserPassword(fmt.Sprintf("https://api.%s", cfMgmt.SystemDomain), baseCommand.UserID, baseCommand.Password) @@ -129,6 +139,9 @@ func InitializePeekManagers(baseCommand BaseCFConfigCommand, peek bool, ldapMgr return nil, err } cv3.WithSkipTLSValidation(true) + if strings.EqualFold(os.Getenv("LOG_LEVEL"), "trace") { + cv3.WithHTTPClient(httpClient) + } v3client, err := v3cfclient.New(cv3) if err != nil { return nil, err diff --git a/uaa/uaa.go b/uaa/uaa.go index e47749b0..2b7d5e8b 100644 --- a/uaa/uaa.go +++ b/uaa/uaa.go @@ -3,6 +3,7 @@ package uaa import ( "errors" "fmt" + "os" "strings" "github.com/xchapter7x/lo" @@ -47,6 +48,9 @@ func NewDefaultUAAManager(sysDomain, clientID, clientSecret, userAgent string, p uaaclient.WithUserAgent(userAgent), uaaclient.WithSkipSSLValidation(true), ) + if strings.EqualFold(os.Getenv("LOG_LEVEL"), "trace") { + uaaclient.WithVerbosity(true) + } if err != nil { return nil, err } diff --git a/util/logging_transport.go b/util/logging_transport.go new file mode 100644 index 00000000..6d805eb6 --- /dev/null +++ b/util/logging_transport.go @@ -0,0 +1,39 @@ +package util + +import ( + "net/http" + "net/http/httputil" + + "github.com/xchapter7x/lo" +) + +type LoggingTransport struct { + base http.RoundTripper +} + +func NewLoggingTransport(roundTripper http.RoundTripper) *LoggingTransport { + return &LoggingTransport{base: roundTripper} +} + +func (t *LoggingTransport) RoundTrip(req *http.Request) (*http.Response, error) { + t.logRequest(req) + + resp, err := t.base.RoundTrip(req) + if err != nil { + return resp, err + } + + t.logResponse(resp) + + return resp, err +} + +func (t *LoggingTransport) logRequest(req *http.Request) { + bytes, _ := httputil.DumpRequest(req, false) + lo.G.Infof("Request: [%s]", string(bytes)) +} + +func (t *LoggingTransport) logResponse(resp *http.Response) { + bytes, _ := httputil.DumpResponse(resp, true) + lo.G.Infof("Response: [%s]", string(bytes)) +} From 2f18c0bb95c8ef8660ad96e083f164176e673431 Mon Sep 17 00:00:00 2001 From: Caleb Washburn Date: Fri, 1 Mar 2024 08:18:51 -0700 Subject: [PATCH 5/9] fix logic where trace was not applied to uaa logging --- .gitignore | 1 + uaa/uaa.go | 14 +++++++++----- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/.gitignore b/.gitignore index fff99f0e..a574398b 100644 --- a/.gitignore +++ b/.gitignore @@ -20,3 +20,4 @@ make-binary.sh .idea test-for-pagination.sh export-cli.sh +docker.env diff --git a/uaa/uaa.go b/uaa/uaa.go index 2b7d5e8b..8d59e8b4 100644 --- a/uaa/uaa.go +++ b/uaa/uaa.go @@ -42,15 +42,19 @@ type User struct { // NewDefaultUAAManager - func NewDefaultUAAManager(sysDomain, clientID, clientSecret, userAgent string, peek bool) (Manager, error) { target := fmt.Sprintf("https://uaa.%s", sysDomain) + + opts := []uaaclient.Option{} + opts = append(opts, uaaclient.WithUserAgent(userAgent)) + opts = append(opts, uaaclient.WithSkipSSLValidation(true)) + if strings.EqualFold(os.Getenv("LOG_LEVEL"), "trace") { + opts = append(opts, uaaclient.WithVerbosity(true)) + } client, err := uaaclient.New( target, uaaclient.WithClientCredentials(clientID, clientSecret, uaaclient.OpaqueToken), - uaaclient.WithUserAgent(userAgent), - uaaclient.WithSkipSSLValidation(true), + opts..., ) - if strings.EqualFold(os.Getenv("LOG_LEVEL"), "trace") { - uaaclient.WithVerbosity(true) - } + if err != nil { return nil, err } From 404617432fcf938fdf034ce733e899174380adaa Mon Sep 17 00:00:00 2001 From: Caleb Washburn Date: Sun, 3 Mar 2024 13:45:39 -0700 Subject: [PATCH 6/9] refactor uaa / user logic to have a single cache to avoid issues --- commands/cleanup_org_users.go | 13 +-- commands/initialize.go | 51 +++++---- commands/update_space_users.go | 14 +-- role/manager.go | 10 +- role/manager_org_test.go | 10 +- role/manager_space_test.go | 10 +- space/spaces_metadata_test.go | 7 +- space/spaces_test.go | 7 +- uaa/definition.go | 2 +- uaa/fakes/fake_mgr.go | 192 --------------------------------- uaa/fakes/uaa_client.go | 94 ++++++++++++++++ uaa/uaa.go | 60 +++++++---- uaa/uaa_test.go | 14 ++- user/ldap_users.go | 13 +-- user/ldap_users_test.go | 62 ++++++----- user/saml_users.go | 11 +- user/saml_users_test.go | 78 ++++++++------ user/users.go | 14 +-- user/users_test.go | 46 ++++---- 19 files changed, 319 insertions(+), 389 deletions(-) delete mode 100644 uaa/fakes/fake_mgr.go diff --git a/commands/cleanup_org_users.go b/commands/cleanup_org_users.go index ff9dec09..c7841ce8 100644 --- a/commands/cleanup_org_users.go +++ b/commands/cleanup_org_users.go @@ -9,12 +9,13 @@ type CleanupOrgUsersCommand struct { // Execute - removes org users func (c *CleanupOrgUsersCommand) Execute([]string) error { - if cfMgmt, err := InitializePeekManagers(c.BaseCFConfigCommand, c.Peek, nil); err == nil { - errs := cfMgmt.UserManager.CleanupOrgUsers() - if len(errs) > 0 { - return fmt.Errorf("got errors processing cleanup users %v", errs) - } - return nil + cfMgmt, err := InitializePeekManagers(c.BaseCFConfigCommand, c.Peek, nil) + if err != nil { + return err + } + errs := cfMgmt.UserManager.CleanupOrgUsers() + if len(errs) > 0 { + return fmt.Errorf("got errors processing cleanup users %v", errs) } return nil } diff --git a/commands/initialize.go b/commands/initialize.go index b17ac8e2..cf74aa5c 100644 --- a/commands/initialize.go +++ b/commands/initialize.go @@ -1,6 +1,7 @@ package commands import ( + "crypto/tls" "fmt" "net/http" "os" @@ -88,31 +89,37 @@ func InitializePeekManagers(baseCommand BaseCFConfigCommand, peek bool, ldapMgr cfMgmt.SystemDomain = baseCommand.SystemDomain cfMgmt.ConfigManager = config.NewManager(cfMgmt.ConfigDirectory) + httpClient := &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{ + InsecureSkipVerify: true, + }, + }, + } + if strings.EqualFold(os.Getenv("LOG_LEVEL"), "trace") { + loggingTranport := util.NewLoggingTransport(httpClient.Transport) + httpClient = &http.Client{ + Transport: loggingTranport, + } + } + userAgent := fmt.Sprintf("cf-mgmt/%s", configcommands.VERSION) - uaaMgr, err := uaa.NewDefaultUAAManager(cfMgmt.SystemDomain, baseCommand.UserID, baseCommand.ClientSecret, userAgent, peek) + uaaMgr, err := uaa.NewDefaultUAAManager(cfMgmt.SystemDomain, baseCommand.UserID, baseCommand.ClientSecret, userAgent, httpClient, peek) if err != nil { return nil, err } cfMgmt.UAAManager = uaaMgr - httpClient := &http.Client{} - httpClient.Transport = util.NewLoggingTransport(http.DefaultTransport) - var c *cfclient.Config var cv3 *v3config.Config if baseCommand.Password != "" { lo.G.Warning("Password parameter is deprecated, create uaa client and client-secret instead") c = &cfclient.Config{ - ApiAddress: fmt.Sprintf("https://api.%s", cfMgmt.SystemDomain), - SkipSslValidation: true, - Username: baseCommand.UserID, - Password: baseCommand.Password, - UserAgent: userAgent, + ApiAddress: fmt.Sprintf("https://api.%s", cfMgmt.SystemDomain), + Username: baseCommand.UserID, + Password: baseCommand.Password, + UserAgent: userAgent, } - if strings.EqualFold(os.Getenv("LOG_LEVEL"), "trace") { - c.HttpClient = httpClient - } - cv3, err = v3config.NewUserPassword(fmt.Sprintf("https://api.%s", cfMgmt.SystemDomain), baseCommand.UserID, baseCommand.Password) @@ -121,11 +128,10 @@ func InitializePeekManagers(baseCommand BaseCFConfigCommand, peek bool, ldapMgr } } else { c = &cfclient.Config{ - ApiAddress: fmt.Sprintf("https://api.%s", cfMgmt.SystemDomain), - SkipSslValidation: true, - ClientID: baseCommand.UserID, - ClientSecret: baseCommand.ClientSecret, - UserAgent: userAgent, + ApiAddress: fmt.Sprintf("https://api.%s", cfMgmt.SystemDomain), + ClientID: baseCommand.UserID, + ClientSecret: baseCommand.ClientSecret, + UserAgent: userAgent, } cv3, err = v3config.NewClientSecret(fmt.Sprintf("https://api.%s", cfMgmt.SystemDomain), baseCommand.UserID, @@ -134,15 +140,16 @@ func InitializePeekManagers(baseCommand BaseCFConfigCommand, peek bool, ldapMgr return nil, err } } + c.HttpClient = httpClient + cv3.UserAgent = userAgent + cv3.WithHTTPClient(httpClient) + client, err := cfclient.NewClient(c) if err != nil { return nil, err } - cv3.WithSkipTLSValidation(true) - if strings.EqualFold(os.Getenv("LOG_LEVEL"), "trace") { - cv3.WithHTTPClient(httpClient) - } v3client, err := v3cfclient.New(cv3) + if err != nil { return nil, err } diff --git a/commands/update_space_users.go b/commands/update_space_users.go index 986f1378..f02b87b2 100644 --- a/commands/update_space_users.go +++ b/commands/update_space_users.go @@ -17,12 +17,14 @@ func (c *UpdateSpaceUsersCommand) Execute([]string) error { if ldapMgr != nil { defer ldapMgr.Close() } - if cfMgmt, err := InitializePeekManagers(c.BaseCFConfigCommand, c.Peek, ldapMgr); err == nil { - errs := cfMgmt.UserManager.UpdateSpaceUsers() - if len(errs) > 0 { - return fmt.Errorf("got errors processing update space users %v", errs) - } - return nil + cfMgmt, err := InitializePeekManagers(c.BaseCFConfigCommand, c.Peek, ldapMgr) + if err != nil { + return err + } + errs := cfMgmt.UserManager.UpdateSpaceUsers() + if len(errs) > 0 { + return fmt.Errorf("got errors processing update space users %v", errs) } return nil + } diff --git a/role/manager.go b/role/manager.go index a719da12..5f572155 100644 --- a/role/manager.go +++ b/role/manager.go @@ -26,7 +26,6 @@ type DefaultManager struct { OrgRoles map[string]map[string]*RoleUsers SpaceRoles map[string]map[string]*RoleUsers CFUsers map[string]*resource.User - UAAUsers *uaa.Users UAAMgr uaa.Manager Peek bool OrgRolesUsers map[string]map[string]map[string]string @@ -276,14 +275,7 @@ func (m *DefaultManager) GetCFUsers() (map[string]*resource.User, error) { } func (m *DefaultManager) GetUAAUsers() (*uaa.Users, error) { - if m.UAAUsers == nil { - uaaUsers, err := m.UAAMgr.ListUsers() - if err != nil { - return nil, err - } - m.UAAUsers = uaaUsers - } - return m.UAAUsers, nil + return m.UAAMgr.ListUsers() } func (m *DefaultManager) UpdateOrgRoleUsers(orgGUID string, roleUser *RoleUsers) { diff --git a/role/manager_org_test.go b/role/manager_org_test.go index b5cc6983..d778761c 100644 --- a/role/manager_org_test.go +++ b/role/manager_org_test.go @@ -4,6 +4,7 @@ import ( "errors" "github.com/cloudfoundry-community/go-cfclient/v3/resource" + uaaclient "github.com/cloudfoundry-community/go-uaa" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" . "github.com/vmwarepivotallabs/cf-mgmt/role" @@ -18,13 +19,13 @@ var _ = Describe("given RoleManager", func() { roleClient *fakes.FakeCFRoleClient userClient *fakes.FakeCFUserClient jobClient *fakes.FakeCFJobClient - uaaFake *uaafakes.FakeManager + uaaFake *uaafakes.FakeUaa ) BeforeEach(func() { roleClient = new(fakes.FakeCFRoleClient) userClient = new(fakes.FakeCFUserClient) jobClient = new(fakes.FakeCFJobClient) - uaaFake = new(uaafakes.FakeManager) + uaaFake = new(uaafakes.FakeUaa) }) Context("Role Manager", func() { BeforeEach(func() { @@ -32,7 +33,7 @@ var _ = Describe("given RoleManager", func() { RoleClient: roleClient, UserClient: userClient, JobClient: jobClient, - UAAMgr: uaaFake, + UAAMgr: &uaa.DefaultUAAManager{Client: uaaFake}, } }) @@ -181,8 +182,7 @@ var _ = Describe("given RoleManager", func() { }) Context("Remove", func() { BeforeEach(func() { - uaaUsers := &uaa.Users{} - uaaFake.ListUsersReturns(uaaUsers, nil) + uaaFake.ListAllUsersReturns([]uaaclient.User{}, nil) userClient.ListAllReturns([]*resource.User{ {GUID: "test-user-guid"}, }, nil) diff --git a/role/manager_space_test.go b/role/manager_space_test.go index 8db0430f..a5129cc0 100644 --- a/role/manager_space_test.go +++ b/role/manager_space_test.go @@ -4,6 +4,7 @@ import ( "errors" "github.com/cloudfoundry-community/go-cfclient/v3/resource" + uaaclient "github.com/cloudfoundry-community/go-uaa" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" . "github.com/vmwarepivotallabs/cf-mgmt/role" @@ -18,13 +19,13 @@ var _ = Describe("given RoleManager", func() { roleClient *fakes.FakeCFRoleClient userClient *fakes.FakeCFUserClient jobClient *fakes.FakeCFJobClient - uaaFake *uaafakes.FakeManager + uaaFake *uaafakes.FakeUaa ) BeforeEach(func() { roleClient = new(fakes.FakeCFRoleClient) userClient = new(fakes.FakeCFUserClient) jobClient = new(fakes.FakeCFJobClient) - uaaFake = new(uaafakes.FakeManager) + uaaFake = new(uaafakes.FakeUaa) }) Context("Role Manager", func() { BeforeEach(func() { @@ -32,14 +33,13 @@ var _ = Describe("given RoleManager", func() { RoleClient: roleClient, UserClient: userClient, JobClient: jobClient, - UAAMgr: uaaFake, + UAAMgr: &uaa.DefaultUAAManager{Client: uaaFake}, } }) Context("Remove", func() { BeforeEach(func() { - uaaUsers := &uaa.Users{} - uaaFake.ListUsersReturns(uaaUsers, nil) + uaaFake.ListAllUsersReturns([]uaaclient.User{}, nil) userClient.ListAllReturns([]*resource.User{ {GUID: "test-user-guid"}, }, nil) diff --git a/space/spaces_metadata_test.go b/space/spaces_metadata_test.go index 1f9120d6..81bc8a26 100644 --- a/space/spaces_metadata_test.go +++ b/space/spaces_metadata_test.go @@ -6,6 +6,7 @@ import ( . "github.com/onsi/gomega" "github.com/vmwarepivotallabs/cf-mgmt/config" configfakes "github.com/vmwarepivotallabs/cf-mgmt/config/fakes" + "github.com/vmwarepivotallabs/cf-mgmt/uaa" orgfakes "github.com/vmwarepivotallabs/cf-mgmt/organizationreader/fakes" "github.com/vmwarepivotallabs/cf-mgmt/space" @@ -15,7 +16,7 @@ import ( var _ = Describe("given SpaceManager", func() { var ( - fakeUaa *uaafakes.FakeManager + fakeUaa *uaafakes.FakeUaa fakeOrgMgr *orgfakes.FakeReader spaceManager space.DefaultManager fakeReader *configfakes.FakeReader @@ -24,14 +25,14 @@ var _ = Describe("given SpaceManager", func() { ) BeforeEach(func() { - fakeUaa = new(uaafakes.FakeManager) + fakeUaa = new(uaafakes.FakeUaa) fakeOrgMgr = new(orgfakes.FakeReader) fakeReader = new(configfakes.FakeReader) fakeSpaceClient = new(spacefakes.FakeCFSpaceClient) fakeSpaceFeatureClient = new(spacefakes.FakeCFSpaceFeatureClient) spaceManager = space.DefaultManager{ Cfg: fakeReader, - UAAMgr: fakeUaa, + UAAMgr: &uaa.DefaultUAAManager{Client: fakeUaa}, OrgReader: fakeOrgMgr, Peek: false, SpaceClient: fakeSpaceClient, diff --git a/space/spaces_test.go b/space/spaces_test.go index e9866068..c0cfad6f 100644 --- a/space/spaces_test.go +++ b/space/spaces_test.go @@ -9,6 +9,7 @@ import ( . "github.com/onsi/gomega" "github.com/vmwarepivotallabs/cf-mgmt/config" configfakes "github.com/vmwarepivotallabs/cf-mgmt/config/fakes" + "github.com/vmwarepivotallabs/cf-mgmt/uaa" "time" @@ -20,7 +21,7 @@ import ( var _ = Describe("given SpaceManager", func() { var ( - fakeUaa *uaafakes.FakeManager + fakeUaa *uaafakes.FakeUaa fakeOrgMgr *orgfakes.FakeReader spaceManager space.DefaultManager fakeReader *configfakes.FakeReader @@ -29,14 +30,14 @@ var _ = Describe("given SpaceManager", func() { ) BeforeEach(func() { - fakeUaa = new(uaafakes.FakeManager) + fakeUaa = new(uaafakes.FakeUaa) fakeOrgMgr = new(orgfakes.FakeReader) fakeReader = new(configfakes.FakeReader) fakeSpaceClient = new(spacefakes.FakeCFSpaceClient) fakeSpaceFeatureClient = new(spacefakes.FakeCFSpaceFeatureClient) spaceManager = space.DefaultManager{ Cfg: fakeReader, - UAAMgr: fakeUaa, + UAAMgr: &uaa.DefaultUAAManager{Client: fakeUaa}, OrgReader: fakeOrgMgr, Peek: false, SpaceClient: fakeSpaceClient, diff --git a/uaa/definition.go b/uaa/definition.go index 96867357..b8ac50c0 100644 --- a/uaa/definition.go +++ b/uaa/definition.go @@ -1,4 +1,4 @@ package uaa //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 -generate -//counterfeiter:generate -o fakes/fake_mgr.go uaa.go Manager +//counterfeiter:generate -o fakes/uaa_client.go uaa.go uaa diff --git a/uaa/fakes/fake_mgr.go b/uaa/fakes/fake_mgr.go deleted file mode 100644 index 0f462012..00000000 --- a/uaa/fakes/fake_mgr.go +++ /dev/null @@ -1,192 +0,0 @@ -// Code generated by counterfeiter. DO NOT EDIT. -package fakes - -import ( - "sync" - - "github.com/vmwarepivotallabs/cf-mgmt/uaa" -) - -type FakeManager struct { - CreateExternalUserStub func(string, string, string, string) (string, error) - createExternalUserMutex sync.RWMutex - createExternalUserArgsForCall []struct { - arg1 string - arg2 string - arg3 string - arg4 string - } - createExternalUserReturns struct { - result1 string - result2 error - } - createExternalUserReturnsOnCall map[int]struct { - result1 string - result2 error - } - ListUsersStub func() (*uaa.Users, error) - listUsersMutex sync.RWMutex - listUsersArgsForCall []struct { - } - listUsersReturns struct { - result1 *uaa.Users - result2 error - } - listUsersReturnsOnCall map[int]struct { - result1 *uaa.Users - result2 error - } - invocations map[string][][]interface{} - invocationsMutex sync.RWMutex -} - -func (fake *FakeManager) CreateExternalUser(arg1 string, arg2 string, arg3 string, arg4 string) (string, error) { - fake.createExternalUserMutex.Lock() - ret, specificReturn := fake.createExternalUserReturnsOnCall[len(fake.createExternalUserArgsForCall)] - fake.createExternalUserArgsForCall = append(fake.createExternalUserArgsForCall, struct { - arg1 string - arg2 string - arg3 string - arg4 string - }{arg1, arg2, arg3, arg4}) - stub := fake.CreateExternalUserStub - fakeReturns := fake.createExternalUserReturns - fake.recordInvocation("CreateExternalUser", []interface{}{arg1, arg2, arg3, arg4}) - fake.createExternalUserMutex.Unlock() - if stub != nil { - return stub(arg1, arg2, arg3, arg4) - } - if specificReturn { - return ret.result1, ret.result2 - } - return fakeReturns.result1, fakeReturns.result2 -} - -func (fake *FakeManager) CreateExternalUserCallCount() int { - fake.createExternalUserMutex.RLock() - defer fake.createExternalUserMutex.RUnlock() - return len(fake.createExternalUserArgsForCall) -} - -func (fake *FakeManager) CreateExternalUserCalls(stub func(string, string, string, string) (string, error)) { - fake.createExternalUserMutex.Lock() - defer fake.createExternalUserMutex.Unlock() - fake.CreateExternalUserStub = stub -} - -func (fake *FakeManager) CreateExternalUserArgsForCall(i int) (string, string, string, string) { - fake.createExternalUserMutex.RLock() - defer fake.createExternalUserMutex.RUnlock() - argsForCall := fake.createExternalUserArgsForCall[i] - return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3, argsForCall.arg4 -} - -func (fake *FakeManager) CreateExternalUserReturns(result1 string, result2 error) { - fake.createExternalUserMutex.Lock() - defer fake.createExternalUserMutex.Unlock() - fake.CreateExternalUserStub = nil - fake.createExternalUserReturns = struct { - result1 string - result2 error - }{result1, result2} -} - -func (fake *FakeManager) CreateExternalUserReturnsOnCall(i int, result1 string, result2 error) { - fake.createExternalUserMutex.Lock() - defer fake.createExternalUserMutex.Unlock() - fake.CreateExternalUserStub = nil - if fake.createExternalUserReturnsOnCall == nil { - fake.createExternalUserReturnsOnCall = make(map[int]struct { - result1 string - result2 error - }) - } - fake.createExternalUserReturnsOnCall[i] = struct { - result1 string - result2 error - }{result1, result2} -} - -func (fake *FakeManager) ListUsers() (*uaa.Users, error) { - fake.listUsersMutex.Lock() - ret, specificReturn := fake.listUsersReturnsOnCall[len(fake.listUsersArgsForCall)] - fake.listUsersArgsForCall = append(fake.listUsersArgsForCall, struct { - }{}) - stub := fake.ListUsersStub - fakeReturns := fake.listUsersReturns - fake.recordInvocation("ListUsers", []interface{}{}) - fake.listUsersMutex.Unlock() - if stub != nil { - return stub() - } - if specificReturn { - return ret.result1, ret.result2 - } - return fakeReturns.result1, fakeReturns.result2 -} - -func (fake *FakeManager) ListUsersCallCount() int { - fake.listUsersMutex.RLock() - defer fake.listUsersMutex.RUnlock() - return len(fake.listUsersArgsForCall) -} - -func (fake *FakeManager) ListUsersCalls(stub func() (*uaa.Users, error)) { - fake.listUsersMutex.Lock() - defer fake.listUsersMutex.Unlock() - fake.ListUsersStub = stub -} - -func (fake *FakeManager) ListUsersReturns(result1 *uaa.Users, result2 error) { - fake.listUsersMutex.Lock() - defer fake.listUsersMutex.Unlock() - fake.ListUsersStub = nil - fake.listUsersReturns = struct { - result1 *uaa.Users - result2 error - }{result1, result2} -} - -func (fake *FakeManager) ListUsersReturnsOnCall(i int, result1 *uaa.Users, result2 error) { - fake.listUsersMutex.Lock() - defer fake.listUsersMutex.Unlock() - fake.ListUsersStub = nil - if fake.listUsersReturnsOnCall == nil { - fake.listUsersReturnsOnCall = make(map[int]struct { - result1 *uaa.Users - result2 error - }) - } - fake.listUsersReturnsOnCall[i] = struct { - result1 *uaa.Users - result2 error - }{result1, result2} -} - -func (fake *FakeManager) Invocations() map[string][][]interface{} { - fake.invocationsMutex.RLock() - defer fake.invocationsMutex.RUnlock() - fake.createExternalUserMutex.RLock() - defer fake.createExternalUserMutex.RUnlock() - fake.listUsersMutex.RLock() - defer fake.listUsersMutex.RUnlock() - copiedInvocations := map[string][][]interface{}{} - for key, value := range fake.invocations { - copiedInvocations[key] = value - } - return copiedInvocations -} - -func (fake *FakeManager) recordInvocation(key string, args []interface{}) { - fake.invocationsMutex.Lock() - defer fake.invocationsMutex.Unlock() - if fake.invocations == nil { - fake.invocations = map[string][][]interface{}{} - } - if fake.invocations[key] == nil { - fake.invocations[key] = [][]interface{}{} - } - fake.invocations[key] = append(fake.invocations[key], args) -} - -var _ uaa.Manager = new(FakeManager) diff --git a/uaa/fakes/uaa_client.go b/uaa/fakes/uaa_client.go index 9b10cc8c..d7a7e564 100644 --- a/uaa/fakes/uaa_client.go +++ b/uaa/fakes/uaa_client.go @@ -37,6 +37,26 @@ type FakeUaa struct { result1 []uaaa.User result2 error } + ListUsersStub func(string, string, string, uaaa.SortOrder, int, int) ([]uaaa.User, uaaa.Page, error) + listUsersMutex sync.RWMutex + listUsersArgsForCall []struct { + arg1 string + arg2 string + arg3 string + arg4 uaaa.SortOrder + arg5 int + arg6 int + } + listUsersReturns struct { + result1 []uaaa.User + result2 uaaa.Page + result3 error + } + listUsersReturnsOnCall map[int]struct { + result1 []uaaa.User + result2 uaaa.Page + result3 error + } invocations map[string][][]interface{} invocationsMutex sync.RWMutex } @@ -172,6 +192,78 @@ func (fake *FakeUaa) ListAllUsersReturnsOnCall(i int, result1 []uaaa.User, resul }{result1, result2} } +func (fake *FakeUaa) ListUsers(arg1 string, arg2 string, arg3 string, arg4 uaaa.SortOrder, arg5 int, arg6 int) ([]uaaa.User, uaaa.Page, error) { + fake.listUsersMutex.Lock() + ret, specificReturn := fake.listUsersReturnsOnCall[len(fake.listUsersArgsForCall)] + fake.listUsersArgsForCall = append(fake.listUsersArgsForCall, struct { + arg1 string + arg2 string + arg3 string + arg4 uaaa.SortOrder + arg5 int + arg6 int + }{arg1, arg2, arg3, arg4, arg5, arg6}) + stub := fake.ListUsersStub + fakeReturns := fake.listUsersReturns + fake.recordInvocation("ListUsers", []interface{}{arg1, arg2, arg3, arg4, arg5, arg6}) + fake.listUsersMutex.Unlock() + if stub != nil { + return stub(arg1, arg2, arg3, arg4, arg5, arg6) + } + if specificReturn { + return ret.result1, ret.result2, ret.result3 + } + return fakeReturns.result1, fakeReturns.result2, fakeReturns.result3 +} + +func (fake *FakeUaa) ListUsersCallCount() int { + fake.listUsersMutex.RLock() + defer fake.listUsersMutex.RUnlock() + return len(fake.listUsersArgsForCall) +} + +func (fake *FakeUaa) ListUsersCalls(stub func(string, string, string, uaaa.SortOrder, int, int) ([]uaaa.User, uaaa.Page, error)) { + fake.listUsersMutex.Lock() + defer fake.listUsersMutex.Unlock() + fake.ListUsersStub = stub +} + +func (fake *FakeUaa) ListUsersArgsForCall(i int) (string, string, string, uaaa.SortOrder, int, int) { + fake.listUsersMutex.RLock() + defer fake.listUsersMutex.RUnlock() + argsForCall := fake.listUsersArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3, argsForCall.arg4, argsForCall.arg5, argsForCall.arg6 +} + +func (fake *FakeUaa) ListUsersReturns(result1 []uaaa.User, result2 uaaa.Page, result3 error) { + fake.listUsersMutex.Lock() + defer fake.listUsersMutex.Unlock() + fake.ListUsersStub = nil + fake.listUsersReturns = struct { + result1 []uaaa.User + result2 uaaa.Page + result3 error + }{result1, result2, result3} +} + +func (fake *FakeUaa) ListUsersReturnsOnCall(i int, result1 []uaaa.User, result2 uaaa.Page, result3 error) { + fake.listUsersMutex.Lock() + defer fake.listUsersMutex.Unlock() + fake.ListUsersStub = nil + if fake.listUsersReturnsOnCall == nil { + fake.listUsersReturnsOnCall = make(map[int]struct { + result1 []uaaa.User + result2 uaaa.Page + result3 error + }) + } + fake.listUsersReturnsOnCall[i] = struct { + result1 []uaaa.User + result2 uaaa.Page + result3 error + }{result1, result2, result3} +} + func (fake *FakeUaa) Invocations() map[string][][]interface{} { fake.invocationsMutex.RLock() defer fake.invocationsMutex.RUnlock() @@ -179,6 +271,8 @@ func (fake *FakeUaa) Invocations() map[string][][]interface{} { defer fake.createUserMutex.RUnlock() fake.listAllUsersMutex.RLock() defer fake.listAllUsersMutex.RUnlock() + fake.listUsersMutex.RLock() + defer fake.listUsersMutex.RUnlock() copiedInvocations := map[string][][]interface{}{} for key, value := range fake.invocations { copiedInvocations[key] = value diff --git a/uaa/uaa.go b/uaa/uaa.go index 8d59e8b4..2d7ce163 100644 --- a/uaa/uaa.go +++ b/uaa/uaa.go @@ -3,7 +3,7 @@ package uaa import ( "errors" "fmt" - "os" + "net/http" "strings" "github.com/xchapter7x/lo" @@ -11,24 +11,24 @@ import ( uaaclient "github.com/cloudfoundry-community/go-uaa" ) -//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 -generate -//counterfeiter:generate -o fakes/uaa_client.go uaa.go uaa type uaa interface { CreateUser(user uaaclient.User) (*uaaclient.User, error) ListAllUsers(filter string, sortBy string, attributes string, sortOrder uaaclient.SortOrder) ([]uaaclient.User, error) + ListUsers(filter string, sortBy string, attributes string, sortOrder uaaclient.SortOrder, startIndex int, itemsPerPage int) ([]uaaclient.User, uaaclient.Page, error) } // Manager - type Manager interface { //Returns a map keyed and valued by user id. User id is converted to lowercase ListUsers() (*Users, error) - CreateExternalUser(userName, userEmail, externalID, origin string) (GUID string, err error) + CreateExternalUser(userName, userEmail, externalID, origin string) (err error) } // DefaultUAAManager - type DefaultUAAManager struct { Peek bool Client uaa + Users *Users } type User struct { @@ -40,19 +40,15 @@ type User struct { } // NewDefaultUAAManager - -func NewDefaultUAAManager(sysDomain, clientID, clientSecret, userAgent string, peek bool) (Manager, error) { +func NewDefaultUAAManager(sysDomain, clientID, clientSecret, userAgent string, httpClient *http.Client, peek bool) (Manager, error) { target := fmt.Sprintf("https://uaa.%s", sysDomain) - opts := []uaaclient.Option{} - opts = append(opts, uaaclient.WithUserAgent(userAgent)) - opts = append(opts, uaaclient.WithSkipSSLValidation(true)) - if strings.EqualFold(os.Getenv("LOG_LEVEL"), "trace") { - opts = append(opts, uaaclient.WithVerbosity(true)) - } client, err := uaaclient.New( target, uaaclient.WithClientCredentials(clientID, clientSecret, uaaclient.OpaqueToken), - opts..., + uaaclient.WithUserAgent(userAgent), + uaaclient.WithClient(httpClient), + uaaclient.WithSkipSSLValidation(true), ) if err != nil { @@ -65,14 +61,28 @@ func NewDefaultUAAManager(sysDomain, clientID, clientSecret, userAgent string, p }, nil } +func (m *DefaultUAAManager) addUser(user User) { + if m.Users == nil { + m.Users = &Users{} + } + m.Users.Add(user) +} + // CreateExternalUser - -func (m *DefaultUAAManager) CreateExternalUser(userName, userEmail, externalID, origin string) (string, error) { +func (m *DefaultUAAManager) CreateExternalUser(userName, userEmail, externalID, origin string) error { if userName == "" || userEmail == "" || externalID == "" { - return "", fmt.Errorf("skipping user as missing name[%s], email[%s] or externalID[%s]", userName, userEmail, externalID) + return fmt.Errorf("skipping user as missing name[%s], email[%s] or externalID[%s]", userName, userEmail, externalID) } if m.Peek { lo.G.Infof("[dry-run]: successfully added user [%s]", userName) - return fmt.Sprintf("dry-run-%s-%s-guid", userName, origin), nil + m.addUser(User{ + Username: userName, + Email: userEmail, + ExternalID: externalID, + Origin: origin, + GUID: fmt.Sprintf("dry-run-%s-%s-guid", userName, origin), + }) + return nil } createdUser, err := m.Client.CreateUser(uaaclient.User{ @@ -88,16 +98,27 @@ func (m *DefaultUAAManager) CreateExternalUser(userName, userEmail, externalID, if err != nil { var requestError uaaclient.RequestError if errors.As(err, &requestError) { - return "", fmt.Errorf("got an error calling %s with response %s", requestError.Url, requestError.ErrorResponse) + return fmt.Errorf("got an error calling %s with response %s", requestError.Url, requestError.ErrorResponse) } - return "", err + return err } + m.addUser(User{ + Username: userName, + Email: userEmail, + ExternalID: externalID, + Origin: origin, + GUID: createdUser.ID, + }) lo.G.Infof("successfully added user [%s]", userName) - return createdUser.ID, nil + return nil } // ListUsers - returns uaa.Users func (m *DefaultUAAManager) ListUsers() (*Users, error) { + if m.Users != nil { + return m.Users, nil + } + users := &Users{} lo.G.Debug("Getting users from Cloud Foundry") userList, err := m.Client.ListAllUsers("", "", "userName,id,externalId,emails,origin", "") @@ -108,8 +129,8 @@ func (m *DefaultUAAManager) ListUsers() (*Users, error) { } return nil, err } - lo.G.Debugf("Found %d users in the CF instance", len(userList)) + for _, user := range userList { userName := strings.Trim(user.Username, " ") externalID := strings.Trim(user.ExternalID, " ") @@ -122,6 +143,7 @@ func (m *DefaultUAAManager) ListUsers() (*Users, error) { GUID: user.ID, }) } + m.Users = users return users, nil } diff --git a/uaa/uaa_test.go b/uaa/uaa_test.go index de6639bb..61d3e7ef 100644 --- a/uaa/uaa_test.go +++ b/uaa/uaa_test.go @@ -12,6 +12,10 @@ import ( "github.com/vmwarepivotallabs/cf-mgmt/uaa/fakes" ) +type UaaResponse struct { + Response []uaaclient.User `json:"resources"` +} + var _ = Describe("given uaa manager", func() { var ( fakeuaa *fakes.FakeUaa @@ -69,7 +73,7 @@ var _ = Describe("given uaa manager", func() { }}, nil, ) - _, err := manager.CreateExternalUser(userName, userEmail, externalID, "ldap") + err := manager.CreateExternalUser(userName, userEmail, externalID, "ldap") Ω(err).ShouldNot(HaveOccurred()) }) It("should successfully create user with complex dn", func() { @@ -86,7 +90,7 @@ var _ = Describe("given uaa manager", func() { }}, nil, ) - _, err := manager.CreateExternalUser(userName, userEmail, externalID, "ldap") + err := manager.CreateExternalUser(userName, userEmail, externalID, "ldap") Ω(err).ShouldNot(HaveOccurred()) }) @@ -95,12 +99,12 @@ var _ = Describe("given uaa manager", func() { userEmail := "email" externalID := "userDN" manager.Peek = true - _, err := manager.CreateExternalUser(userName, userEmail, externalID, "ldap") + err := manager.CreateExternalUser(userName, userEmail, externalID, "ldap") Ω(err).ShouldNot(HaveOccurred()) Ω(fakeuaa.CreateUserCallCount()).Should(Equal(0)) }) It("should not invoke post", func() { - _, err := manager.CreateExternalUser("", "", "", "ldap") + err := manager.CreateExternalUser("", "", "", "ldap") Ω(err).Should(HaveOccurred()) Ω(fakeuaa.CreateUserCallCount()).Should(Equal(0)) }) @@ -122,7 +126,7 @@ var _ = Describe("given uaa manager", func() { }}, nil, ) - _, err := manager.CreateExternalUser(userName, userEmail, externalID, origin) + err := manager.CreateExternalUser(userName, userEmail, externalID, origin) Ω(err).ShouldNot(HaveOccurred()) }) }) diff --git a/user/ldap_users.go b/user/ldap_users.go index 1e5187d1..899a7c2f 100644 --- a/user/ldap_users.go +++ b/user/ldap_users.go @@ -8,7 +8,6 @@ import ( "github.com/pkg/errors" "github.com/vmwarepivotallabs/cf-mgmt/ldap" "github.com/vmwarepivotallabs/cf-mgmt/role" - "github.com/vmwarepivotallabs/cf-mgmt/uaa" "github.com/xchapter7x/lo" ) @@ -29,17 +28,9 @@ func (m *DefaultManager) SyncLdapUsers(roleUsers *role.RoleUsers, usersInput Use userID := userToUse.UserID uaaUser := uaaUsers.GetByNameAndOrigin(userID, origin) if uaaUser == nil { - lo.G.Debugf("User %s doesn't exist in cloud foundry with origin, so creating user", userID, origin) - if userGUID, err := m.UAAMgr.CreateExternalUser(userID, userToUse.Email, userToUse.UserDN, m.LdapConfig.Origin); err != nil { + lo.G.Debugf("User %s doesn't exist in cloud foundry with origin %s, so creating user", userID, origin) + if err := m.UAAMgr.CreateExternalUser(userID, userToUse.Email, userToUse.UserDN, m.LdapConfig.Origin); err != nil { return err - } else { - m.AddUAAUser(uaa.User{ - Username: userID, - ExternalID: userToUse.UserDN, - Origin: origin, - Email: userToUse.Email, - GUID: userGUID, - }) } } if !roleUsers.HasUserForOrigin(userID, origin) { diff --git a/user/ldap_users_test.go b/user/ldap_users_test.go index 71a98325..b9cca87d 100644 --- a/user/ldap_users_test.go +++ b/user/ldap_users_test.go @@ -4,6 +4,7 @@ import ( "errors" "github.com/cloudfoundry-community/go-cfclient/v3/resource" + uaaclient "github.com/cloudfoundry-community/go-uaa" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "github.com/vmwarepivotallabs/cf-mgmt/config" @@ -23,7 +24,7 @@ var _ = Describe("given UserSpaces", func() { var ( userManager *DefaultManager ldapFake *fakes.FakeLdapManager - uaaFake *uaafakes.FakeManager + uaaFake *uaafakes.FakeUaa fakeReader *configfakes.FakeReader spaceFake *spacefakes.FakeManager orgFake *orgfakes.FakeReader @@ -31,7 +32,7 @@ var _ = Describe("given UserSpaces", func() { ) BeforeEach(func() { ldapFake = new(fakes.FakeLdapManager) - uaaFake = new(uaafakes.FakeManager) + uaaFake = new(uaafakes.FakeUaa) fakeReader = new(configfakes.FakeReader) spaceFake = new(spacefakes.FakeManager) orgFake = new(orgfakes.FakeReader) @@ -41,7 +42,7 @@ var _ = Describe("given UserSpaces", func() { BeforeEach(func() { userManager = &DefaultManager{ Cfg: fakeReader, - UAAMgr: uaaFake, + UAAMgr: &uaa.DefaultUAAManager{Client: uaaFake}, LdapMgr: ldapFake, SpaceMgr: spaceFake, OrgReader: orgFake, @@ -59,13 +60,17 @@ var _ = Describe("given UserSpaces", func() { Origin: "ldap", Enabled: true, } - uaaUsers := &uaa.Users{} - uaaUsers.Add(uaa.User{Username: "test_ldap", Origin: "ldap", ExternalID: "cn=test_ldap", GUID: "test_ldap-id"}) - uaaUsers.Add(uaa.User{Username: "test_ldap2", Origin: "ldap", ExternalID: "cn=test_ldap2", GUID: "test_ldap2-id"}) + uaaUsers := []uaaclient.User{} + uaaUsers = append(uaaUsers, uaaclient.User{Username: "test_ldap", Origin: "ldap", ExternalID: "cn=test_ldap", ID: "test_ldap-id"}) + uaaUsers = append(uaaUsers, uaaclient.User{Username: "test_ldap2", Origin: "ldap", ExternalID: "cn=test_ldap2", ID: "test_ldap2-id"}) + uaaFake.ListAllUsersReturns(uaaUsers, nil) + + users, err := userManager.UAAMgr.ListUsers() + Expect(err).ShouldNot(HaveOccurred()) roleUsers, _ = role.NewRoleUsers([]*resource.User{ {Username: "test_ldap", GUID: "test_ldap-id"}, - }, uaaUsers) - userManager.UAAUsers = uaaUsers + }, users) + }) It("Should add ldap user to role", func() { updateUsersInput := UsersInput{ @@ -105,14 +110,18 @@ var _ = Describe("given UserSpaces", func() { Origin: "ldap", Enabled: true, } - uaaUsers := &uaa.Users{} - uaaUsers.Add(uaa.User{Username: "test_ldap", Origin: "ldap", ExternalID: "cn=test_ldap", GUID: "test_ldap-id"}) - uaaUsers.Add(uaa.User{Username: "test_ldap2", Origin: "ldap", ExternalID: "cn=test_ldap2", GUID: "test_ldap2-id"}) + + uaaUsers := []uaaclient.User{} + uaaUsers = append(uaaUsers, uaaclient.User{Username: "test_ldap", Origin: "ldap", ExternalID: "cn=test_ldap", ID: "test_ldap-id"}) + uaaUsers = append(uaaUsers, uaaclient.User{Username: "test_ldap2", Origin: "ldap", ExternalID: "cn=test_ldap2", ID: "test_ldap2-id"}) + uaaFake.ListAllUsersReturns(uaaUsers, nil) + + users, err := userManager.UAAMgr.ListUsers() + Expect(err).ShouldNot(HaveOccurred()) roleUsers, _ = role.NewRoleUsers([]*resource.User{ {Username: "test_ldap", GUID: "test_ldap-id"}, - }, uaaUsers) + }, users) - userManager.UAAUsers = uaaUsers updateUsersInput := UsersInput{ LdapUsers: []string{"test_ldap2"}, LdapGroupNames: []string{}, @@ -132,7 +141,7 @@ var _ = Describe("given UserSpaces", func() { }, nil) - err := userManager.SyncLdapUsers(roleUsers, updateUsersInput) + err = userManager.SyncLdapUsers(roleUsers, updateUsersInput) Expect(err).ShouldNot(HaveOccurred()) Expect(roleMgrFake.AssociateSpaceAuditorCallCount()).Should(Equal(1)) orgGUID, spaceName, spaceGUID, userName, userGUID := roleMgrFake.AssociateSpaceAuditorArgsForCall(0) @@ -209,14 +218,15 @@ var _ = Describe("given UserSpaces", func() { Email: "test@test.com", }, nil) + uaaFake.CreateUserReturns(&uaaclient.User{ID: "user-guid"}, nil) err := userManager.SyncLdapUsers(roleUsers, updateUsersInput) Expect(err).ShouldNot(HaveOccurred()) - Expect(uaaFake.CreateExternalUserCallCount()).Should(Equal(1)) - arg1, arg2, arg3, origin := uaaFake.CreateExternalUserArgsForCall(0) - Expect(arg1).Should(Equal("test_ldap_new")) - Expect(arg2).Should(Equal("test@test.com")) - Expect(arg3).Should(Equal("ldap_test_dn")) - Expect(origin).Should(Equal("ldap")) + Expect(uaaFake.CreateUserCallCount()).Should(Equal(1)) + user := uaaFake.CreateUserArgsForCall(0) + Expect(user.Username).Should(Equal("test_ldap_new")) + Expect(user.Emails[0].Value).Should(Equal("test@test.com")) + Expect(user.ExternalID).Should(Equal("ldap_test_dn")) + Expect(user.Origin).Should(Equal("ldap")) }) It("Should not error when create external user errors", func() { @@ -234,16 +244,18 @@ var _ = Describe("given UserSpaces", func() { Email: "test@test.com", }, nil) - uaaFake.CreateExternalUserReturns("guid", errors.New("error")) + uaaFake.CreateUserReturns(nil, errors.New("error")) err := userManager.SyncLdapUsers(roleUsers, updateUsersInput) Expect(err).Should(HaveOccurred()) - Expect(userManager.UAAUsers.GetByNameAndOrigin("test_ldap3", "ldap")).Should(BeNil()) - Expect(uaaFake.CreateExternalUserCallCount()).Should(Equal(1)) + uaaUsers, err := userManager.UAAMgr.ListUsers() + Expect(err).ShouldNot(HaveOccurred()) + Expect(uaaUsers.GetByNameAndOrigin("test_ldap3", "ldap")).Should(BeNil()) + Expect(uaaFake.CreateUserCallCount()).Should(Equal(1)) }) It("Should return error", func() { updateUsersInput := UsersInput{ - LdapUsers: []string{"test_ldap3"}, + LdapUsers: []string{"test_ldap2"}, SpaceGUID: "space_guid", OrgGUID: "org_guid", AddUser: roleMgrFake.AssociateSpaceAuditor, @@ -259,7 +271,7 @@ var _ = Describe("given UserSpaces", func() { roleMgrFake.AssociateSpaceAuditorReturns(errors.New("error")) err := userManager.SyncLdapUsers(roleUsers, updateUsersInput) Expect(err).Should(HaveOccurred()) - Expect(err.Error()).Should(Equal("User test_ldap3 with origin ldap: error")) + Expect(err.Error()).Should(Equal("User test_ldap2 with origin ldap: error")) Expect(roleMgrFake.AssociateSpaceAuditorCallCount()).Should(Equal(1)) }) diff --git a/user/saml_users.go b/user/saml_users.go index 340c9725..c33d7499 100644 --- a/user/saml_users.go +++ b/user/saml_users.go @@ -5,7 +5,6 @@ import ( "github.com/pkg/errors" "github.com/vmwarepivotallabs/cf-mgmt/role" - "github.com/vmwarepivotallabs/cf-mgmt/uaa" "github.com/xchapter7x/lo" ) @@ -19,16 +18,8 @@ func (m *DefaultManager) SyncSamlUsers(roleUsers *role.RoleUsers, usersInput Use uaaUser := uaaUsers.GetByNameAndOrigin(userEmail, origin) if uaaUser == nil { lo.G.Debugf("user %s doesn't exist in cloud foundry with origin %s, so creating user", userEmail, origin) - if userGUID, err := m.UAAMgr.CreateExternalUser(userEmail, userEmail, userEmail, origin); err != nil { + if err := m.UAAMgr.CreateExternalUser(userEmail, userEmail, userEmail, origin); err != nil { return err - } else { - m.AddUAAUser(uaa.User{ - Username: userEmail, - Email: userEmail, - ExternalID: userEmail, - Origin: origin, - GUID: userGUID, - }) } } user := uaaUsers.GetByNameAndOrigin(userEmail, origin) diff --git a/user/saml_users_test.go b/user/saml_users_test.go index 2bad384e..8a8fa981 100644 --- a/user/saml_users_test.go +++ b/user/saml_users_test.go @@ -4,6 +4,7 @@ import ( "errors" "github.com/cloudfoundry-community/go-cfclient/v3/resource" + uaaclient "github.com/cloudfoundry-community/go-uaa" "github.com/vmwarepivotallabs/cf-mgmt/config" configfakes "github.com/vmwarepivotallabs/cf-mgmt/config/fakes" orgfakes "github.com/vmwarepivotallabs/cf-mgmt/organizationreader/fakes" @@ -23,7 +24,7 @@ var _ = Describe("SamlUsers", func() { var ( userManager *DefaultManager ldapFake *fakes.FakeLdapManager - uaaFake *uaafakes.FakeManager + uaaFake *uaafakes.FakeUaa fakeReader *configfakes.FakeReader spaceFake *spacefakes.FakeManager orgFake *orgfakes.FakeReader @@ -31,14 +32,14 @@ var _ = Describe("SamlUsers", func() { ) BeforeEach(func() { ldapFake = new(fakes.FakeLdapManager) - uaaFake = new(uaafakes.FakeManager) + uaaFake = new(uaafakes.FakeUaa) fakeReader = new(configfakes.FakeReader) spaceFake = new(spacefakes.FakeManager) orgFake = new(orgfakes.FakeReader) roleMgrFake = new(rolefakes.FakeManager) userManager = &DefaultManager{ Cfg: fakeReader, - UAAMgr: uaaFake, + UAAMgr: &uaa.DefaultUAAManager{Client: uaaFake}, LdapMgr: ldapFake, SpaceMgr: spaceFake, OrgReader: orgFake, @@ -53,17 +54,21 @@ var _ = Describe("SamlUsers", func() { var roleUsers *role.RoleUsers BeforeEach(func() { userManager.LdapConfig = &config.LdapConfig{Origin: "saml_origin"} - uaaUsers := &uaa.Users{} - uaaUsers.Add(uaa.User{Username: "Test.Test@test.com", Email: "test.test@test.com", ExternalID: "Test.Test@test.com", Origin: "saml_origin", GUID: "test-id"}) - uaaUsers.Add(uaa.User{Username: "test2.test2@test.com", Email: "test2.test2@test.com", ExternalID: "test2.test2@test.com", Origin: "saml_origin", GUID: "test2-id"}) + uaaUsers := []uaaclient.User{} + uaaUsers = append(uaaUsers, uaaclient.User{Username: "Test.Test@test.com", Emails: []uaaclient.Email{{Value: "test.test@test.com"}}, ExternalID: "Test.Test@test.com", Origin: "saml_origin", ID: "test-id"}) + uaaUsers = append(uaaUsers, uaaclient.User{Username: "test2.test2@test.com", Emails: []uaaclient.Email{{Value: "test2.test2@test.com"}}, ExternalID: "test2.test2@test.com", Origin: "saml_origin", ID: "test2-id"}) + uaaFake.ListAllUsersReturns(uaaUsers, nil) + + users, err := userManager.UAAMgr.ListUsers() + Expect(err).ShouldNot(HaveOccurred()) roleUsers, _ = role.NewRoleUsers( []*resource.User{ {Username: "Test.Test@test.com", GUID: "test-id"}, }, - uaaUsers, + users, ) - userManager.UAAUsers = uaaUsers + }) It("Should add saml user to role", func() { updateUsersInput := UsersInput{ @@ -98,7 +103,7 @@ var _ = Describe("SamlUsers", func() { err := userManager.SyncSamlUsers(roleUsers, updateUsersInput) Expect(err).ShouldNot(HaveOccurred()) Expect(roleUsers.HasUserForOrigin("test.test@test.com", "saml_origin")).Should(BeFalse()) - Expect(uaaFake.CreateExternalUserCallCount()).Should(Equal(0)) + Expect(uaaFake.CreateUserCallCount()).Should(Equal(0)) Expect(roleMgrFake.AssociateSpaceAuditorCallCount()).Should(Equal(0)) }) @@ -114,7 +119,7 @@ var _ = Describe("SamlUsers", func() { err := userManager.SyncSamlUsers(roleUsers, updateUsersInput) Expect(err).ShouldNot(HaveOccurred()) Expect(roleUsers.HasUserForOrigin("Test.Test@test.com", "saml_origin")).Should(BeFalse()) - Expect(uaaFake.CreateExternalUserCallCount()).Should(Equal(0)) + Expect(uaaFake.CreateUserCallCount()).Should(Equal(0)) Expect(roleMgrFake.AssociateSpaceAuditorCallCount()).Should(Equal(0)) }) It("Should create external user when user doesn't exist in uaa", func() { @@ -125,29 +130,30 @@ var _ = Describe("SamlUsers", func() { AddUser: roleMgrFake.AssociateSpaceAuditor, RoleUsers: role.InitRoleUsers(), } + uaaFake.CreateUserReturns(&uaaclient.User{ID: "user-guid"}, nil) err := userManager.SyncSamlUsers(roleUsers, updateUsersInput) Expect(err).ShouldNot(HaveOccurred()) - Expect(uaaFake.CreateExternalUserCallCount()).Should(Equal(1)) - arg1, arg2, arg3, origin := uaaFake.CreateExternalUserArgsForCall(0) - Expect(arg1).Should(Equal("test3.test3@test.com")) - Expect(arg2).Should(Equal("test3.test3@test.com")) - Expect(arg3).Should(Equal("test3.test3@test.com")) - Expect(origin).Should(Equal("saml_origin")) + Expect(uaaFake.CreateUserCallCount()).Should(Equal(1)) + user := uaaFake.CreateUserArgsForCall(0) + Expect(user.Username).Should(Equal("test3.test3@test.com")) + Expect(user.Emails[0].Value).Should(Equal("test3.test3@test.com")) + Expect(user.ExternalID).Should(Equal("test3.test3@test.com")) + Expect(user.Origin).Should(Equal("saml_origin")) }) It("Should not error when create external user errors", func() { updateUsersInput := UsersInput{ - SamlUsers: []string{"test.test@test.com"}, + SamlUsers: []string{"test.test@foo.com"}, SpaceGUID: "space_guid", OrgGUID: "org_guid", AddUser: roleMgrFake.AssociateSpaceAuditor, RoleUsers: role.InitRoleUsers(), } - userManager.UAAUsers = &uaa.Users{} - uaaFake.CreateExternalUserReturns("guid", errors.New("error")) + uaaFake.ListAllUsersReturns([]uaaclient.User{}, nil) + uaaFake.CreateUserReturns(nil, errors.New("error")) err := userManager.SyncSamlUsers(roleUsers, updateUsersInput) Expect(err).Should(HaveOccurred()) - Expect(uaaFake.CreateExternalUserCallCount()).Should(Equal(1)) + Expect(uaaFake.CreateUserCallCount()).Should(Equal(1)) }) It("Should return error", func() { @@ -155,8 +161,9 @@ var _ = Describe("SamlUsers", func() { roleUsers.AddUsers([]role.RoleUser{ {UserName: "test"}, }) - uaaUsers := &uaa.Users{} - uaaUsers.Add(uaa.User{Username: "test.test@test.com"}) + + uaaUsers := []uaaclient.User{} + uaaUsers = append(uaaUsers, uaaclient.User{Username: "test.test@test.com", Origin: "saml_origin"}) updateUsersInput := UsersInput{ SamlUsers: []string{"test.test@test.com"}, SpaceGUID: "space_guid", @@ -164,7 +171,7 @@ var _ = Describe("SamlUsers", func() { AddUser: roleMgrFake.AssociateSpaceAuditor, RoleUsers: role.InitRoleUsers(), } - userManager.UAAUsers = uaaUsers + uaaFake.ListAllUsersReturns(uaaUsers, nil) roleMgrFake.AssociateSpaceAuditorReturns(errors.New("Got an error")) err := userManager.SyncSamlUsers(roleUsers, updateUsersInput) Expect(err).Should(HaveOccurred()) @@ -175,15 +182,18 @@ var _ = Describe("SamlUsers", func() { var roleUsers *role.RoleUsers BeforeEach(func() { userManager.LdapConfig = &config.LdapConfig{Origin: "saml_origin"} - uaaUsers := &uaa.Users{} - uaaUsers.Add(uaa.User{Username: "test.test@test.com", Email: "test.test@test.com", ExternalID: "test.test@test.com", Origin: "saml_original_origin", GUID: "test-id"}) + uaaUsers := []uaaclient.User{} + uaaUsers = append(uaaUsers, uaaclient.User{Username: "test.test@test.com", Emails: []uaaclient.Email{{Value: "test.test@test.com"}}, ExternalID: "test.test@test.com", Origin: "saml_original_origin", ID: "test-id"}) + uaaFake.ListAllUsersReturns(uaaUsers, nil) + + users, err := userManager.UAAMgr.ListUsers() + Expect(err).ShouldNot(HaveOccurred()) roleUsers, _ = role.NewRoleUsers( []*resource.User{ {Username: "test.test@test.com", GUID: "test-id", Origin: "saml_original_origin"}, }, - uaaUsers, + users, ) - userManager.UAAUsers = uaaUsers }) It("Should add saml new user to role with new origin and remove old user from role", func() { @@ -198,15 +208,15 @@ var _ = Describe("SamlUsers", func() { RemoveUser: roleMgrFake.RemoveSpaceAuditor, RemoveUsers: true, } - uaaFake.CreateExternalUserReturns("new-user-guid", nil) + uaaFake.CreateUserReturns(&uaaclient.User{ID: "new-user-guid"}, nil) err := userManager.SyncUsers(updateUsersInput) Expect(err).ShouldNot(HaveOccurred()) - Expect(uaaFake.CreateExternalUserCallCount()).Should(Equal(1)) - createExternalUserArg1, createExternalUserArg2, createExternalUserArg3, createExternalUserArg4 := uaaFake.CreateExternalUserArgsForCall(0) - Expect(createExternalUserArg1).Should(Equal("test.test@test.com")) - Expect(createExternalUserArg2).Should(Equal("test.test@test.com")) - Expect(createExternalUserArg3).Should(Equal("test.test@test.com")) - Expect(createExternalUserArg4).Should(Equal("saml_origin")) + Expect(uaaFake.CreateUserCallCount()).Should(Equal(1)) + user := uaaFake.CreateUserArgsForCall(0) + Expect(user.Username).Should(Equal("test.test@test.com")) + Expect(user.Emails[0].Value).Should(Equal("test.test@test.com")) + Expect(user.ExternalID).Should(Equal("test.test@test.com")) + Expect(user.Origin).Should(Equal("saml_origin")) Expect(roleMgrFake.AssociateSpaceAuditorCallCount()).Should(Equal(1)) orgGUID, spaceName, spaceGUID, userName, userGUID := roleMgrFake.AssociateSpaceAuditorArgsForCall(0) diff --git a/user/users.go b/user/users.go index 843dacf3..2401eb56 100644 --- a/user/users.go +++ b/user/users.go @@ -58,22 +58,10 @@ type DefaultManager struct { Peek bool LdapMgr LdapManager LdapConfig *config.LdapConfig - UAAUsers *uaa.Users } func (m *DefaultManager) GetUAAUsers() (*uaa.Users, error) { - if m.UAAUsers == nil { - uaaUsers, err := m.UAAMgr.ListUsers() - if err != nil { - return nil, err - } - m.UAAUsers = uaaUsers - } - return m.UAAUsers, nil -} - -func (m *DefaultManager) AddUAAUser(user uaa.User) { - m.UAAUsers.Add(user) + return m.UAAMgr.ListUsers() } // UpdateSpaceUsers - diff --git a/user/users_test.go b/user/users_test.go index 791b3959..80c87ba8 100644 --- a/user/users_test.go +++ b/user/users_test.go @@ -4,6 +4,7 @@ import ( "errors" "github.com/cloudfoundry-community/go-cfclient/v3/resource" + uaaclient "github.com/cloudfoundry-community/go-uaa" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "github.com/vmwarepivotallabs/cf-mgmt/config" @@ -22,7 +23,7 @@ var _ = Describe("given UserSpaces", func() { var ( userManager *DefaultManager ldapFake *fakes.FakeLdapManager - uaaFake *uaafakes.FakeManager + uaaFake *uaafakes.FakeUaa fakeReader *configfakes.FakeReader spaceFake *spacefakes.FakeManager orgFake *orgfakes.FakeReader @@ -30,7 +31,7 @@ var _ = Describe("given UserSpaces", func() { ) BeforeEach(func() { ldapFake = new(fakes.FakeLdapManager) - uaaFake = new(uaafakes.FakeManager) + uaaFake = new(uaafakes.FakeUaa) fakeReader = new(configfakes.FakeReader) spaceFake = new(spacefakes.FakeManager) orgFake = new(orgfakes.FakeReader) @@ -40,7 +41,7 @@ var _ = Describe("given UserSpaces", func() { BeforeEach(func() { userManager = &DefaultManager{ Cfg: fakeReader, - UAAMgr: uaaFake, + UAAMgr: &uaa.DefaultUAAManager{Client: uaaFake}, LdapMgr: ldapFake, SpaceMgr: spaceFake, OrgReader: orgFake, @@ -57,14 +58,16 @@ var _ = Describe("given UserSpaces", func() { Context("SyncInternalUsers", func() { var roleUsers *role.RoleUsers BeforeEach(func() { - uaaUsers := &uaa.Users{} - uaaUsers.Add(uaa.User{Username: "test", Origin: "uaa", GUID: "test-user-guid"}) - uaaUsers.Add(uaa.User{Username: "test-existing", Origin: "uaa", GUID: "test-existing-id"}) + uaaUsers := []uaaclient.User{} + uaaUsers = append(uaaUsers, uaaclient.User{Username: "test", Origin: "uaa", ID: "test-user-guid"}) + uaaUsers = append(uaaUsers, uaaclient.User{Username: "test-existing", Origin: "uaa", ID: "test-existing-id"}) + uaaFake.ListAllUsersReturns(uaaUsers, nil) + + users, err := userManager.UAAMgr.ListUsers() + Expect(err).ShouldNot(HaveOccurred()) roleUsers, _ = role.NewRoleUsers([]*resource.User{ {Username: "test-existing", GUID: "test-existing-id"}, - }, uaaUsers) - - userManager.UAAUsers = uaaUsers + }, users) }) It("Should add internal user to role", func() { updateUsersInput := UsersInput{ @@ -233,9 +236,9 @@ var _ = Describe("given UserSpaces", func() { Context("UpdateSpaceUsers", func() { It("Should succeed", func() { - userMap := &uaa.Users{} - userMap.Add(uaa.User{Username: "test-user-guid", GUID: "test-user"}) - uaaFake.ListUsersReturns(userMap, nil) + uaaUsers := []uaaclient.User{} + uaaUsers = append(uaaUsers, uaaclient.User{Username: "test-user-guid", ID: "test-user"}) + uaaFake.ListAllUsersReturns(uaaUsers, nil) fakeReader.GetSpaceConfigsReturns([]config.SpaceConfig{ { Space: "test-space", @@ -261,9 +264,9 @@ var _ = Describe("given UserSpaces", func() { Context("UpdateSpaceUsers", func() { It("Should succeed", func() { - userMap := &uaa.Users{} - userMap.Add(uaa.User{Username: "test-user-guid", GUID: "test-user"}) - uaaFake.ListUsersReturns(userMap, nil) + uaaUsers := []uaaclient.User{} + uaaUsers = append(uaaUsers, uaaclient.User{Username: "test-user-guid", ID: "test-user"}) + uaaFake.ListAllUsersReturns(uaaUsers, nil) fakeReader.GetOrgConfigsReturns([]config.OrgConfig{ { Org: "test-org", @@ -281,12 +284,15 @@ var _ = Describe("given UserSpaces", func() { Context("Sync Users", func() { var roleUsers *role.RoleUsers BeforeEach(func() { - uaaUsers := &uaa.Users{} - uaaUsers.Add(uaa.User{Username: "test", Origin: "uaa", GUID: "test-user-guid"}) - uaaUsers.Add(uaa.User{Username: "test", Origin: "ldap", GUID: "test-ldap-user-guid", ExternalID: "cn=test"}) - roleUsers, _ = role.NewRoleUsers([]*resource.User{}, uaaUsers) + uaaUsers := []uaaclient.User{} + uaaUsers = append(uaaUsers, uaaclient.User{Username: "test", Origin: "uaa", ID: "test-user-guid"}) + uaaUsers = append(uaaUsers, uaaclient.User{Username: "test", Origin: "ldap", ID: "test-ldap-user-guid", ExternalID: "cn=test"}) + uaaFake.ListAllUsersReturns(uaaUsers, nil) + + users, err := userManager.UAAMgr.ListUsers() + Expect(err).ShouldNot(HaveOccurred()) + roleUsers, _ = role.NewRoleUsers([]*resource.User{}, users) - userManager.UAAUsers = uaaUsers }) It("Should add internal user to role and ldap user with same name to role", func() { updateUsersInput := UsersInput{ From 585e9f9a31a7dcd8168f986d2fa65897f6ce7d7a Mon Sep 17 00:00:00 2001 From: Caleb Washburn Date: Fri, 8 Mar 2024 09:01:53 -0700 Subject: [PATCH 7/9] refactor to do uaa pagination vs leverage client library --- role/manager_org_test.go | 2 +- role/manager_space_test.go | 2 +- uaa/fakes/uaa_client.go | 85 -------------------------------------- uaa/uaa.go | 54 +++++++++++++++++++++++- uaa/uaa_test.go | 31 +++++++------- user/ldap_users_test.go | 4 +- user/saml_users_test.go | 8 ++-- user/users_test.go | 8 ++-- 8 files changed, 80 insertions(+), 114 deletions(-) diff --git a/role/manager_org_test.go b/role/manager_org_test.go index d778761c..efa210db 100644 --- a/role/manager_org_test.go +++ b/role/manager_org_test.go @@ -182,7 +182,7 @@ var _ = Describe("given RoleManager", func() { }) Context("Remove", func() { BeforeEach(func() { - uaaFake.ListAllUsersReturns([]uaaclient.User{}, nil) + uaaFake.ListUsersReturns([]uaaclient.User{}, uaaclient.Page{StartIndex: 1, TotalResults: 0, ItemsPerPage: 500}, nil) userClient.ListAllReturns([]*resource.User{ {GUID: "test-user-guid"}, }, nil) diff --git a/role/manager_space_test.go b/role/manager_space_test.go index a5129cc0..93d35232 100644 --- a/role/manager_space_test.go +++ b/role/manager_space_test.go @@ -39,7 +39,7 @@ var _ = Describe("given RoleManager", func() { Context("Remove", func() { BeforeEach(func() { - uaaFake.ListAllUsersReturns([]uaaclient.User{}, nil) + uaaFake.ListUsersReturns([]uaaclient.User{}, uaaclient.Page{StartIndex: 1, TotalResults: 0, ItemsPerPage: 500}, nil) userClient.ListAllReturns([]*resource.User{ {GUID: "test-user-guid"}, }, nil) diff --git a/uaa/fakes/uaa_client.go b/uaa/fakes/uaa_client.go index d7a7e564..b8279ac6 100644 --- a/uaa/fakes/uaa_client.go +++ b/uaa/fakes/uaa_client.go @@ -21,22 +21,6 @@ type FakeUaa struct { result1 *uaaa.User result2 error } - ListAllUsersStub func(string, string, string, uaaa.SortOrder) ([]uaaa.User, error) - listAllUsersMutex sync.RWMutex - listAllUsersArgsForCall []struct { - arg1 string - arg2 string - arg3 string - arg4 uaaa.SortOrder - } - listAllUsersReturns struct { - result1 []uaaa.User - result2 error - } - listAllUsersReturnsOnCall map[int]struct { - result1 []uaaa.User - result2 error - } ListUsersStub func(string, string, string, uaaa.SortOrder, int, int) ([]uaaa.User, uaaa.Page, error) listUsersMutex sync.RWMutex listUsersArgsForCall []struct { @@ -125,73 +109,6 @@ func (fake *FakeUaa) CreateUserReturnsOnCall(i int, result1 *uaaa.User, result2 }{result1, result2} } -func (fake *FakeUaa) ListAllUsers(arg1 string, arg2 string, arg3 string, arg4 uaaa.SortOrder) ([]uaaa.User, error) { - fake.listAllUsersMutex.Lock() - ret, specificReturn := fake.listAllUsersReturnsOnCall[len(fake.listAllUsersArgsForCall)] - fake.listAllUsersArgsForCall = append(fake.listAllUsersArgsForCall, struct { - arg1 string - arg2 string - arg3 string - arg4 uaaa.SortOrder - }{arg1, arg2, arg3, arg4}) - stub := fake.ListAllUsersStub - fakeReturns := fake.listAllUsersReturns - fake.recordInvocation("ListAllUsers", []interface{}{arg1, arg2, arg3, arg4}) - fake.listAllUsersMutex.Unlock() - if stub != nil { - return stub(arg1, arg2, arg3, arg4) - } - if specificReturn { - return ret.result1, ret.result2 - } - return fakeReturns.result1, fakeReturns.result2 -} - -func (fake *FakeUaa) ListAllUsersCallCount() int { - fake.listAllUsersMutex.RLock() - defer fake.listAllUsersMutex.RUnlock() - return len(fake.listAllUsersArgsForCall) -} - -func (fake *FakeUaa) ListAllUsersCalls(stub func(string, string, string, uaaa.SortOrder) ([]uaaa.User, error)) { - fake.listAllUsersMutex.Lock() - defer fake.listAllUsersMutex.Unlock() - fake.ListAllUsersStub = stub -} - -func (fake *FakeUaa) ListAllUsersArgsForCall(i int) (string, string, string, uaaa.SortOrder) { - fake.listAllUsersMutex.RLock() - defer fake.listAllUsersMutex.RUnlock() - argsForCall := fake.listAllUsersArgsForCall[i] - return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3, argsForCall.arg4 -} - -func (fake *FakeUaa) ListAllUsersReturns(result1 []uaaa.User, result2 error) { - fake.listAllUsersMutex.Lock() - defer fake.listAllUsersMutex.Unlock() - fake.ListAllUsersStub = nil - fake.listAllUsersReturns = struct { - result1 []uaaa.User - result2 error - }{result1, result2} -} - -func (fake *FakeUaa) ListAllUsersReturnsOnCall(i int, result1 []uaaa.User, result2 error) { - fake.listAllUsersMutex.Lock() - defer fake.listAllUsersMutex.Unlock() - fake.ListAllUsersStub = nil - if fake.listAllUsersReturnsOnCall == nil { - fake.listAllUsersReturnsOnCall = make(map[int]struct { - result1 []uaaa.User - result2 error - }) - } - fake.listAllUsersReturnsOnCall[i] = struct { - result1 []uaaa.User - result2 error - }{result1, result2} -} - func (fake *FakeUaa) ListUsers(arg1 string, arg2 string, arg3 string, arg4 uaaa.SortOrder, arg5 int, arg6 int) ([]uaaa.User, uaaa.Page, error) { fake.listUsersMutex.Lock() ret, specificReturn := fake.listUsersReturnsOnCall[len(fake.listUsersArgsForCall)] @@ -269,8 +186,6 @@ func (fake *FakeUaa) Invocations() map[string][][]interface{} { defer fake.invocationsMutex.RUnlock() fake.createUserMutex.RLock() defer fake.createUserMutex.RUnlock() - fake.listAllUsersMutex.RLock() - defer fake.listAllUsersMutex.RUnlock() fake.listUsersMutex.RLock() defer fake.listUsersMutex.RUnlock() copiedInvocations := map[string][][]interface{}{} diff --git a/uaa/uaa.go b/uaa/uaa.go index 2d7ce163..52bb2cb0 100644 --- a/uaa/uaa.go +++ b/uaa/uaa.go @@ -13,7 +13,6 @@ import ( type uaa interface { CreateUser(user uaaclient.User) (*uaaclient.User, error) - ListAllUsers(filter string, sortBy string, attributes string, sortOrder uaaclient.SortOrder) ([]uaaclient.User, error) ListUsers(filter string, sortBy string, attributes string, sortOrder uaaclient.SortOrder, startIndex int, itemsPerPage int) ([]uaaclient.User, uaaclient.Page, error) } @@ -121,7 +120,7 @@ func (m *DefaultUAAManager) ListUsers() (*Users, error) { users := &Users{} lo.G.Debug("Getting users from Cloud Foundry") - userList, err := m.Client.ListAllUsers("", "", "userName,id,externalId,emails,origin", "") + userList, err := m.ListAllUsers() if err != nil { var requestError uaaclient.RequestError if errors.As(err, &requestError) { @@ -147,6 +146,57 @@ func (m *DefaultUAAManager) ListUsers() (*Users, error) { return users, nil } +func (m *DefaultUAAManager) ListAllUsers() ([]uaaclient.User, error) { + page := uaaclient.Page{ + StartIndex: 1, + ItemsPerPage: 500, + } + var ( + results []uaaclient.User + currentPage []uaaclient.User + err error + totalResults int + ) + currentPage, page, err = m.Client.ListUsers("", "", "userName,id,externalId,emails,origin", "", page.StartIndex, page.ItemsPerPage) + totalResults = page.TotalResults + if err != nil { + return nil, err + } + results = append(results, currentPage...) + if (page.StartIndex + page.ItemsPerPage) <= page.TotalResults { + page.StartIndex = page.StartIndex + page.ItemsPerPage + for { + currentPage, page, err = m.Client.ListUsers("", "", "userName,id,externalId,emails,origin", "", page.StartIndex, page.ItemsPerPage) + if err != nil { + return nil, err + } + if totalResults != page.TotalResults { + lo.G.Infof("Result size changed during pagination from %d to %d", totalResults, page.TotalResults) + totalResults = page.TotalResults + } + results = append(results, currentPage...) + + if (page.StartIndex + page.ItemsPerPage) > page.TotalResults { + break + } + page.StartIndex = page.StartIndex + page.ItemsPerPage + } + } + if len(results) != totalResults { + return nil, fmt.Errorf("results %d is not equal to expected results %d", len(results), totalResults) + } + uniqueMap := make(map[string]string) + for _, user := range results { + userKey := fmt.Sprintf("%s-%s", user.Username, user.Origin) + if userId, ok := uniqueMap[userKey]; ok { + return nil, fmt.Errorf("user with userName [%s], origin [%s], id [%s] already returned with id [%s]", user.Username, user.Origin, userId, user.ID) + } else { + uniqueMap[userKey] = user.ID + } + } + return results, nil +} + func Email(u uaaclient.User) string { for _, email := range u.Emails { if email.Primary == nil { diff --git a/uaa/uaa_test.go b/uaa/uaa_test.go index 61d3e7ef..9ceda9df 100644 --- a/uaa/uaa_test.go +++ b/uaa/uaa_test.go @@ -31,7 +31,7 @@ var _ = Describe("given uaa manager", func() { Context("ListUsers()", func() { It("should return list of users", func() { - fakeuaa.ListAllUsersReturns([]uaaclient.User{ + fakeuaa.ListUsersReturns([]uaaclient.User{ {Username: "foo4", ID: "foo4-id"}, {Username: "admin", ID: "admin-id"}, {Username: "user", ID: "user-id"}, @@ -41,24 +41,25 @@ var _ = Describe("given uaa manager", func() { {Username: "foo2", ID: "foo2-id"}, {Username: "foo3", ID: "foo3-id"}, {Username: "cn=admin", ID: "cn=admin-id"}, - }, nil) + }, uaaclient.Page{ItemsPerPage: 500, StartIndex: 1, TotalResults: 9}, nil) users, err := manager.ListUsers() - Ω(err).ShouldNot(HaveOccurred()) + Expect(fakeuaa.ListUsersCallCount()).Should(Equal(1)) + Expect(err).ShouldNot(HaveOccurred()) keys := make([]string, 0, len(users.List())) for _, k := range users.List() { keys = append(keys, k.Username) } - Ω(len(users.List())).Should(Equal(9)) - Ω(keys).Should(ConsistOf("foo4", "admin", "user", "cwashburn", "foo", "foo1", "foo2", "foo3", "cn=admin")) + Expect(len(users.List())).Should(Equal(9)) + Expect(keys).Should(ConsistOf("foo4", "admin", "user", "cwashburn", "foo", "foo1", "foo2", "foo3", "cn=admin")) }) It("should return an error", func() { - fakeuaa.ListAllUsersReturns(nil, errors.New("Got an error")) + fakeuaa.ListUsersReturns(nil, uaaclient.Page{ItemsPerPage: 500, StartIndex: 1, TotalResults: 10}, errors.New("Got an error")) _, err := manager.ListUsers() - Ω(err).Should(HaveOccurred()) + Expect(err).Should(HaveOccurred()) + Expect(fakeuaa.ListUsersCallCount()).Should(Equal(1)) }) }) Context("CreateLdapUser()", func() { - It("should successfully create user", func() { userName := "user" userEmail := "email" @@ -74,7 +75,7 @@ var _ = Describe("given uaa manager", func() { nil, ) err := manager.CreateExternalUser(userName, userEmail, externalID, "ldap") - Ω(err).ShouldNot(HaveOccurred()) + Expect(err).ShouldNot(HaveOccurred()) }) It("should successfully create user with complex dn", func() { userName := "asdfasdfsadf" @@ -91,7 +92,7 @@ var _ = Describe("given uaa manager", func() { nil, ) err := manager.CreateExternalUser(userName, userEmail, externalID, "ldap") - Ω(err).ShouldNot(HaveOccurred()) + Expect(err).ShouldNot(HaveOccurred()) }) It("should peek", func() { @@ -100,13 +101,13 @@ var _ = Describe("given uaa manager", func() { externalID := "userDN" manager.Peek = true err := manager.CreateExternalUser(userName, userEmail, externalID, "ldap") - Ω(err).ShouldNot(HaveOccurred()) - Ω(fakeuaa.CreateUserCallCount()).Should(Equal(0)) + Expect(err).ShouldNot(HaveOccurred()) + Expect(fakeuaa.CreateUserCallCount()).Should(Equal(0)) }) It("should not invoke post", func() { err := manager.CreateExternalUser("", "", "", "ldap") - Ω(err).Should(HaveOccurred()) - Ω(fakeuaa.CreateUserCallCount()).Should(Equal(0)) + Expect(err).Should(HaveOccurred()) + Expect(fakeuaa.CreateUserCallCount()).Should(Equal(0)) }) }) Context("CreateSamlUser()", func() { @@ -127,7 +128,7 @@ var _ = Describe("given uaa manager", func() { nil, ) err := manager.CreateExternalUser(userName, userEmail, externalID, origin) - Ω(err).ShouldNot(HaveOccurred()) + Expect(err).ShouldNot(HaveOccurred()) }) }) }) diff --git a/user/ldap_users_test.go b/user/ldap_users_test.go index b9cca87d..f7107615 100644 --- a/user/ldap_users_test.go +++ b/user/ldap_users_test.go @@ -63,7 +63,7 @@ var _ = Describe("given UserSpaces", func() { uaaUsers := []uaaclient.User{} uaaUsers = append(uaaUsers, uaaclient.User{Username: "test_ldap", Origin: "ldap", ExternalID: "cn=test_ldap", ID: "test_ldap-id"}) uaaUsers = append(uaaUsers, uaaclient.User{Username: "test_ldap2", Origin: "ldap", ExternalID: "cn=test_ldap2", ID: "test_ldap2-id"}) - uaaFake.ListAllUsersReturns(uaaUsers, nil) + uaaFake.ListUsersReturns(uaaUsers, uaaclient.Page{StartIndex: 1, TotalResults: 2, ItemsPerPage: 500}, nil) users, err := userManager.UAAMgr.ListUsers() Expect(err).ShouldNot(HaveOccurred()) @@ -114,7 +114,7 @@ var _ = Describe("given UserSpaces", func() { uaaUsers := []uaaclient.User{} uaaUsers = append(uaaUsers, uaaclient.User{Username: "test_ldap", Origin: "ldap", ExternalID: "cn=test_ldap", ID: "test_ldap-id"}) uaaUsers = append(uaaUsers, uaaclient.User{Username: "test_ldap2", Origin: "ldap", ExternalID: "cn=test_ldap2", ID: "test_ldap2-id"}) - uaaFake.ListAllUsersReturns(uaaUsers, nil) + uaaFake.ListUsersReturns(uaaUsers, uaaclient.Page{StartIndex: 1, TotalResults: 10, ItemsPerPage: 500}, nil) users, err := userManager.UAAMgr.ListUsers() Expect(err).ShouldNot(HaveOccurred()) diff --git a/user/saml_users_test.go b/user/saml_users_test.go index 8a8fa981..fd0bf6ca 100644 --- a/user/saml_users_test.go +++ b/user/saml_users_test.go @@ -58,7 +58,7 @@ var _ = Describe("SamlUsers", func() { uaaUsers := []uaaclient.User{} uaaUsers = append(uaaUsers, uaaclient.User{Username: "Test.Test@test.com", Emails: []uaaclient.Email{{Value: "test.test@test.com"}}, ExternalID: "Test.Test@test.com", Origin: "saml_origin", ID: "test-id"}) uaaUsers = append(uaaUsers, uaaclient.User{Username: "test2.test2@test.com", Emails: []uaaclient.Email{{Value: "test2.test2@test.com"}}, ExternalID: "test2.test2@test.com", Origin: "saml_origin", ID: "test2-id"}) - uaaFake.ListAllUsersReturns(uaaUsers, nil) + uaaFake.ListUsersReturns(uaaUsers, uaaclient.Page{StartIndex: 1, TotalResults: 2, ItemsPerPage: 500}, nil) users, err := userManager.UAAMgr.ListUsers() Expect(err).ShouldNot(HaveOccurred()) @@ -149,7 +149,7 @@ var _ = Describe("SamlUsers", func() { AddUser: roleMgrFake.AssociateSpaceAuditor, RoleUsers: role.InitRoleUsers(), } - uaaFake.ListAllUsersReturns([]uaaclient.User{}, nil) + uaaFake.ListUsersReturns([]uaaclient.User{}, uaaclient.Page{StartIndex: 1, TotalResults: 0, ItemsPerPage: 500}, nil) uaaFake.CreateUserReturns(nil, errors.New("error")) err := userManager.SyncSamlUsers(roleUsers, updateUsersInput) Expect(err).Should(HaveOccurred()) @@ -171,7 +171,7 @@ var _ = Describe("SamlUsers", func() { AddUser: roleMgrFake.AssociateSpaceAuditor, RoleUsers: role.InitRoleUsers(), } - uaaFake.ListAllUsersReturns(uaaUsers, nil) + uaaFake.ListUsersReturns(uaaUsers, uaaclient.Page{StartIndex: 1, TotalResults: 0, ItemsPerPage: 500}, nil) roleMgrFake.AssociateSpaceAuditorReturns(errors.New("Got an error")) err := userManager.SyncSamlUsers(roleUsers, updateUsersInput) Expect(err).Should(HaveOccurred()) @@ -184,7 +184,7 @@ var _ = Describe("SamlUsers", func() { userManager.LdapConfig = &config.LdapConfig{Origin: "saml_origin"} uaaUsers := []uaaclient.User{} uaaUsers = append(uaaUsers, uaaclient.User{Username: "test.test@test.com", Emails: []uaaclient.Email{{Value: "test.test@test.com"}}, ExternalID: "test.test@test.com", Origin: "saml_original_origin", ID: "test-id"}) - uaaFake.ListAllUsersReturns(uaaUsers, nil) + uaaFake.ListUsersReturns(uaaUsers, uaaclient.Page{StartIndex: 1, TotalResults: 1, ItemsPerPage: 500}, nil) users, err := userManager.UAAMgr.ListUsers() Expect(err).ShouldNot(HaveOccurred()) diff --git a/user/users_test.go b/user/users_test.go index 80c87ba8..ebf551ea 100644 --- a/user/users_test.go +++ b/user/users_test.go @@ -61,7 +61,7 @@ var _ = Describe("given UserSpaces", func() { uaaUsers := []uaaclient.User{} uaaUsers = append(uaaUsers, uaaclient.User{Username: "test", Origin: "uaa", ID: "test-user-guid"}) uaaUsers = append(uaaUsers, uaaclient.User{Username: "test-existing", Origin: "uaa", ID: "test-existing-id"}) - uaaFake.ListAllUsersReturns(uaaUsers, nil) + uaaFake.ListUsersReturns(uaaUsers, uaaclient.Page{StartIndex: 1, TotalResults: len(uaaUsers), ItemsPerPage: 500}, nil) users, err := userManager.UAAMgr.ListUsers() Expect(err).ShouldNot(HaveOccurred()) @@ -238,7 +238,7 @@ var _ = Describe("given UserSpaces", func() { It("Should succeed", func() { uaaUsers := []uaaclient.User{} uaaUsers = append(uaaUsers, uaaclient.User{Username: "test-user-guid", ID: "test-user"}) - uaaFake.ListAllUsersReturns(uaaUsers, nil) + uaaFake.ListUsersReturns(uaaUsers, uaaclient.Page{StartIndex: 1, TotalResults: 1, ItemsPerPage: 500}, nil) fakeReader.GetSpaceConfigsReturns([]config.SpaceConfig{ { Space: "test-space", @@ -266,7 +266,7 @@ var _ = Describe("given UserSpaces", func() { It("Should succeed", func() { uaaUsers := []uaaclient.User{} uaaUsers = append(uaaUsers, uaaclient.User{Username: "test-user-guid", ID: "test-user"}) - uaaFake.ListAllUsersReturns(uaaUsers, nil) + uaaFake.ListUsersReturns(uaaUsers, uaaclient.Page{StartIndex: 1, TotalResults: 1, ItemsPerPage: 500}, nil) fakeReader.GetOrgConfigsReturns([]config.OrgConfig{ { Org: "test-org", @@ -287,7 +287,7 @@ var _ = Describe("given UserSpaces", func() { uaaUsers := []uaaclient.User{} uaaUsers = append(uaaUsers, uaaclient.User{Username: "test", Origin: "uaa", ID: "test-user-guid"}) uaaUsers = append(uaaUsers, uaaclient.User{Username: "test", Origin: "ldap", ID: "test-ldap-user-guid", ExternalID: "cn=test"}) - uaaFake.ListAllUsersReturns(uaaUsers, nil) + uaaFake.ListUsersReturns(uaaUsers, uaaclient.Page{StartIndex: 1, TotalResults: 2, ItemsPerPage: 500}, nil) users, err := userManager.UAAMgr.ListUsers() Expect(err).ShouldNot(HaveOccurred()) From 67f17ceb365a994971bf6959905ca06393eff951 Mon Sep 17 00:00:00 2001 From: Caleb Washburn Date: Wed, 27 Mar 2024 10:38:02 -0700 Subject: [PATCH 8/9] changing sort by for uaa to avoid pagination issues when multiple users are created with same created time --- uaa/uaa.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/uaa/uaa.go b/uaa/uaa.go index 52bb2cb0..a246ee91 100644 --- a/uaa/uaa.go +++ b/uaa/uaa.go @@ -157,7 +157,7 @@ func (m *DefaultUAAManager) ListAllUsers() ([]uaaclient.User, error) { err error totalResults int ) - currentPage, page, err = m.Client.ListUsers("", "", "userName,id,externalId,emails,origin", "", page.StartIndex, page.ItemsPerPage) + currentPage, page, err = m.Client.ListUsers("", "id", "userName,id,externalId,emails,origin", "", page.StartIndex, page.ItemsPerPage) totalResults = page.TotalResults if err != nil { return nil, err From c48bfd810d64e93653a8ef8887c1d4932bec92fa Mon Sep 17 00:00:00 2001 From: Caleb Washburn Date: Wed, 27 Mar 2024 10:48:08 -0700 Subject: [PATCH 9/9] changing sort by for uaa to avoid pagination issues when multiple users are created with same created time --- uaa/uaa.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/uaa/uaa.go b/uaa/uaa.go index a246ee91..9af89ecf 100644 --- a/uaa/uaa.go +++ b/uaa/uaa.go @@ -166,7 +166,7 @@ func (m *DefaultUAAManager) ListAllUsers() ([]uaaclient.User, error) { if (page.StartIndex + page.ItemsPerPage) <= page.TotalResults { page.StartIndex = page.StartIndex + page.ItemsPerPage for { - currentPage, page, err = m.Client.ListUsers("", "", "userName,id,externalId,emails,origin", "", page.StartIndex, page.ItemsPerPage) + currentPage, page, err = m.Client.ListUsers("", "id", "userName,id,externalId,emails,origin", "", page.StartIndex, page.ItemsPerPage) if err != nil { return nil, err }