diff --git a/server/events/vcs/gitlab_client.go b/server/events/vcs/gitlab_client.go index 2ac61a403f..68f78a3829 100644 --- a/server/events/vcs/gitlab_client.go +++ b/server/events/vcs/gitlab_client.go @@ -637,10 +637,10 @@ func (g *GitlabClient) GetTeamNamesForUser(logger logging.SimpleLogging, _ model if err != nil { return nil, errors.Wrapf(err, "GET /users returned: %d", resp.StatusCode) } else if len(users) == 0 { - return nil, errors.Wrap(err, "GET /users returned no user") + return nil, errors.New("GET /users returned no user") } else if len(users) > 1 { // Theoretically impossible, just being extra safe - return nil, errors.Wrap(err, "GET /users returned more than 1 user") + return nil, errors.New("GET /users returned more than 1 user") } userID := users[0].ID for _, groupName := range g.ConfiguredGroups { diff --git a/server/events/vcs/gitlab_client_test.go b/server/events/vcs/gitlab_client_test.go index 49c9a0f8f0..aaa85b0e5b 100644 --- a/server/events/vcs/gitlab_client_test.go +++ b/server/events/vcs/gitlab_client_test.go @@ -1121,40 +1121,82 @@ func TestGitlabClient_GetTeamNamesForUser(t *testing.T) { userSuccess, err := os.ReadFile("testdata/gitlab-user-success.json") Ok(t, err) - configuredGroups := []string{"someorg/group1", "someorg/group2", "someorg/group3", "someorg/group4"} - testServer := httptest.NewServer( - http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch r.RequestURI { - case "/api/v4/users?username=testuser": - w.WriteHeader(http.StatusOK) - w.Write(userSuccess) // nolint: errcheck - case "/api/v4/groups/someorg%2Fgroup1/members/123", "/api/v4/groups/someorg%2Fgroup2/members/123": - w.WriteHeader(http.StatusOK) - w.Write(groupMembershipSuccess) // nolint: errcheck - case "/api/v4/groups/someorg%2Fgroup3/members/123": - http.Error(w, "forbidden", http.StatusForbidden) - case "/api/v4/groups/someorg%2Fgroup4/members/123": - http.Error(w, "not found", http.StatusNotFound) - default: - t.Errorf("got unexpected request at %q", r.RequestURI) - http.Error(w, "not found", http.StatusNotFound) - } - })) - internalClient, err := gitlab.NewClient("token", gitlab.WithBaseURL(testServer.URL)) + userEmpty, err := os.ReadFile("testdata/gitlab-user-none.json") Ok(t, err) - client := &GitlabClient{ - Client: internalClient, - Version: nil, - ConfiguredGroups: configuredGroups, + + multipleUsers, err := os.ReadFile("testdata/gitlab-user-multiple.json") + Ok(t, err) + + configuredGroups := []string{"someorg/group1", "someorg/group2", "someorg/group3", "someorg/group4"} + + cases := []struct { + userName string + expErr string + expTeams []string + }{ + { + userName: "testuser", + expTeams: []string{"someorg/group1", "someorg/group2"}, + }, + { + userName: "none", + expErr: "GET /users returned no user", + }, + { + userName: "multiuser", + expErr: "GET /users returned more than 1 user", + }, } + for _, c := range cases { + t.Run(c.userName, func(t *testing.T) { + + testServer := httptest.NewServer( + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.RequestURI { + case "/api/v4/users?username=testuser": + w.WriteHeader(http.StatusOK) + w.Write(userSuccess) // nolint: errcheck + case "/api/v4/users?username=none": + w.WriteHeader(http.StatusOK) + w.Write(userEmpty) // nolint: errcheck + case "/api/v4/users?username=multiuser": + w.WriteHeader(http.StatusOK) + w.Write(multipleUsers) // nolint: errcheck + case "/api/v4/groups/someorg%2Fgroup1/members/123", "/api/v4/groups/someorg%2Fgroup2/members/123": + w.WriteHeader(http.StatusOK) + w.Write(groupMembershipSuccess) // nolint: errcheck + case "/api/v4/groups/someorg%2Fgroup3/members/123": + http.Error(w, "forbidden", http.StatusForbidden) + case "/api/v4/groups/someorg%2Fgroup4/members/123": + http.Error(w, "not found", http.StatusNotFound) + default: + t.Errorf("got unexpected request at %q", r.RequestURI) + http.Error(w, "not found", http.StatusNotFound) + } + })) + internalClient, err := gitlab.NewClient("token", gitlab.WithBaseURL(testServer.URL)) + Ok(t, err) + client := &GitlabClient{ + Client: internalClient, + Version: nil, + ConfiguredGroups: configuredGroups, + } + + teams, err := client.GetTeamNamesForUser( + logger, + models.Repo{ + Owner: "someorg", + }, models.User{ + Username: c.userName, + }) + if c.expErr == "" { + Ok(t, err) + Equals(t, c.expTeams, teams) + } else { + ErrContains(t, c.expErr, err) + + } - teams, err := client.GetTeamNamesForUser( - logger, - models.Repo{ - Owner: "someorg", - }, models.User{ - Username: "testuser", }) - Ok(t, err) - Equals(t, []string{"someorg/group1", "someorg/group2"}, teams) + } } diff --git a/server/events/vcs/testdata/gitlab-user-multiple.json b/server/events/vcs/testdata/gitlab-user-multiple.json new file mode 100644 index 0000000000..dfd5373067 --- /dev/null +++ b/server/events/vcs/testdata/gitlab-user-multiple.json @@ -0,0 +1,20 @@ +[ + { + "id": 123, + "username": "multiuser", + "name": "Multiple User 1", + "state": "active", + "locked": false, + "avatar_url": "https://gitlab.com/uploads/-/system/user/avatar/123/avatar.png", + "web_url": "https://gitlab.com/multiuser" + }, + { + "id": 124, + "username": "multiuser", + "name": "Multiple User 2", + "state": "active", + "locked": false, + "avatar_url": "https://gitlab.com/uploads/-/system/user/avatar/124/avatar.png", + "web_url": "https://gitlab.com/multiuser" + } +] diff --git a/server/events/vcs/testdata/gitlab-user-none.json b/server/events/vcs/testdata/gitlab-user-none.json new file mode 100644 index 0000000000..fe51488c70 --- /dev/null +++ b/server/events/vcs/testdata/gitlab-user-none.json @@ -0,0 +1 @@ +[]