diff --git a/pkg/login/social/gitlab_oauth.go b/pkg/login/social/gitlab_oauth.go index b3e658799f7..eee04bb3e4b 100644 --- a/pkg/login/social/gitlab_oauth.go +++ b/pkg/login/social/gitlab_oauth.go @@ -5,12 +5,19 @@ import ( "encoding/json" "fmt" "net/http" - "regexp" + "net/url" + "strconv" "strings" "golang.org/x/oauth2" "github.com/grafana/grafana/pkg/models/roletype" + "github.com/grafana/grafana/pkg/setting" +) + +const ( + groupPerPage = 50 + accessLevelGuest = "10" ) type SocialGitlab struct { @@ -59,8 +66,11 @@ func (s *SocialGitlab) isGroupMember(groups []string) bool { func (s *SocialGitlab) getGroups(ctx context.Context, client *http.Client) []string { groups := make([]string, 0) + nextPage := new(int) - for page, url := s.getGroupsPage(ctx, client, s.apiUrl+"/groups"); page != nil; page, url = s.getGroupsPage(ctx, client, url) { + for *nextPage = 1; nextPage != nil; { + var page []string + page, nextPage = s.getGroupsPage(ctx, client, *nextPage) groups = append(groups, page...) } @@ -68,29 +78,50 @@ func (s *SocialGitlab) getGroups(ctx context.Context, client *http.Client) []str } // getGroupsPage returns groups and link to the next page if response is paginated -func (s *SocialGitlab) getGroupsPage(ctx context.Context, client *http.Client, url string) ([]string, string) { +func (s *SocialGitlab) getGroupsPage(ctx context.Context, client *http.Client, nextPage int) ([]string, *int) { type Group struct { FullPath string `json:"full_path"` } - var ( - groups []Group - next string - ) - - if url == "" { - return nil, next + groupURL, err := url.JoinPath(s.apiUrl, "/groups") + if err != nil { + s.log.Error("Error joining GitLab API URL", "err", err) + return nil, nil } - response, err := s.httpGet(ctx, client, url) + parsedUrl, err := url.Parse(groupURL) + if err != nil { + s.log.Error("Error parsing GitLab API URL", "err", err) + return nil, nil + } + + q := parsedUrl.Query() + q.Set("per_page", fmt.Sprintf("%d", groupPerPage)) + q.Set("min_access_level", accessLevelGuest) + q.Set("page", fmt.Sprintf("%d", nextPage)) + parsedUrl.RawQuery = q.Encode() + + response, err := s.httpGet(ctx, client, parsedUrl.String()) if err != nil { s.log.Error("Error getting groups from GitLab API", "err", err) - return nil, next + return nil, nil } + respSizeString := response.Headers.Get("X-Total") + respSize := groupPerPage + if respSizeString != "" { + foundSize, err := strconv.Atoi(respSizeString) + if err != nil { + s.log.Warn("Error parsing X-Total header from GitLab API", "err", err) + } else { + respSize = foundSize + } + } + + groups := make([]Group, 0, respSize) if err := json.Unmarshal(response.Body, &groups); err != nil { s.log.Error("Error parsing JSON from GitLab API", "err", err) - return nil, next + return nil, nil } fullPaths := make([]string, len(groups)) @@ -98,11 +129,14 @@ func (s *SocialGitlab) getGroupsPage(ctx context.Context, client *http.Client, u fullPaths[i] = group.FullPath } - // GitLab uses Link header with "rel" set to prev/next/first/last page. We need "next". - if link, ok := response.Headers["Link"]; ok { - pattern := regexp.MustCompile(`<([^>]+)>; rel="next"`) - if matches := pattern.FindStringSubmatch(link[0]); matches != nil { - next = matches[1] + var next *int = nil + nextString := response.Headers.Get("X-Next-Page") + if nextString != "" { + foundNext, err := strconv.Atoi(nextString) + if err != nil { + s.log.Warn("Error parsing X-Next-Page header from GitLab API", "err", err) + } else { + next = &foundNext } } @@ -187,6 +221,10 @@ func (s *SocialGitlab) extractFromAPI(ctx context.Context, client *http.Client, idData.Role = role } + if setting.Env == setting.Dev { + s.log.Debug("Resolved ID", "data", fmt.Sprintf("%+v", idData)) + } + return idData, nil } diff --git a/pkg/login/social/gitlab_oauth_test.go b/pkg/login/social/gitlab_oauth_test.go index c1276a91a34..a12c536d0cb 100644 --- a/pkg/login/social/gitlab_oauth_test.go +++ b/pkg/login/social/gitlab_oauth_test.go @@ -7,6 +7,7 @@ import ( "fmt" "net/http" "net/http/httptest" + "strconv" "strings" "testing" @@ -20,7 +21,7 @@ import ( const ( apiURI = "/api/v4" userURI = "/api/v4/user" - groupsURI = "/api/v4/groups" + groupsURI = "/api/v4/groups?min_access_level=10&page=1&per_page=50" gitlabAttrPath = `is_admin && 'GrafanaAdmin' || contains(groups[*], 'admins') && 'Admin' || contains(groups[*], 'editors') && 'Editor' || contains(groups[*], 'viewers') && 'Viewer'` @@ -54,6 +55,7 @@ func TestSocialGitlab_UserInfo(t *testing.T) { Cfg conf UserRespBody string GroupsRespBody string + GroupHeaders map[string]string RoleAttributePath string ExpectedLogin string ExpectedEmail string @@ -62,10 +64,16 @@ func TestSocialGitlab_UserInfo(t *testing.T) { ExpectedError error }{ { - Name: "Server Admin Allowed", - Cfg: conf{AllowAssignGrafanaAdmin: true}, - UserRespBody: rootUserRespBody, - GroupsRespBody: "[" + strings.Join([]string{adminGroup, editorGroup, viewerGroup}, ",") + "]", + Name: "Server Admin Allowed", + Cfg: conf{AllowAssignGrafanaAdmin: true}, + UserRespBody: rootUserRespBody, + GroupsRespBody: "[" + strings.Join([]string{adminGroup, editorGroup, viewerGroup}, ",") + "]", + GroupHeaders: map[string]string{ + "X-Total-Pages": "1", + "X-Total": "3", + "X-Page": "1", + "X-Next-Page": "", + }, RoleAttributePath: gitlabAttrPath, ExpectedLogin: "root", ExpectedEmail: "root@example.org", @@ -73,14 +81,21 @@ func TestSocialGitlab_UserInfo(t *testing.T) { ExpectedGrafanaAdmin: trueBoolPtr(), }, { // Edge case, user in Viewer Group, Server Admin disabled but attribute path contains a condition for Server Admin => User has the Admin role - Name: "Server Admin Disabled", - Cfg: conf{AllowAssignGrafanaAdmin: false}, - UserRespBody: rootUserRespBody, - GroupsRespBody: "[" + strings.Join([]string{viewerGroup}, ",") + "]", - RoleAttributePath: gitlabAttrPath, - ExpectedLogin: "root", - ExpectedEmail: "root@example.org", - ExpectedRole: "Admin", + Name: "Server Admin Disabled", + Cfg: conf{AllowAssignGrafanaAdmin: false}, + UserRespBody: rootUserRespBody, + GroupsRespBody: "[" + strings.Join([]string{adminGroup, editorGroup, viewerGroup}, ",") + "]", + GroupHeaders: map[string]string{ + "X-Total-Pages": "1", + "X-Total": "3", + "X-Page": "1", + // Next page omitted to test that the provider does not make a second request + }, + RoleAttributePath: gitlabAttrPath, + ExpectedLogin: "root", + ExpectedEmail: "root@example.org", + ExpectedRole: "Admin", + ExpectedGrafanaAdmin: nil, }, { Name: "Editor", @@ -92,6 +107,9 @@ func TestSocialGitlab_UserInfo(t *testing.T) { ExpectedEmail: "gitlab-editor@example.org", ExpectedRole: "Editor", ExpectedGrafanaAdmin: falseBoolPtr(), + GroupHeaders: map[string]string{ + // All headers omitted to test that the provider does not make a second request + }, }, { Name: "Should not sync role, return empty role and nil pointer for GrafanaAdmin for skip org role sync set to true", @@ -104,7 +122,7 @@ func TestSocialGitlab_UserInfo(t *testing.T) { ExpectedRole: "", ExpectedGrafanaAdmin: nilPointer, }, - { // Case that's going to change with Grafana 10 + { // Fallback to autoAssignOrgRole Name: "No fallback to default org role", Cfg: conf{AutoAssignOrgRole: org.RoleAdmin}, UserRespBody: editorUserRespBody, @@ -116,7 +134,7 @@ func TestSocialGitlab_UserInfo(t *testing.T) { }, { Name: "Strict mode prevents fallback to default", - Cfg: conf{RoleAttributeStrict: true, AutoAssignOrgRole: org.RoleViewer}, + Cfg: conf{RoleAttributeStrict: true, AutoAssignOrgRole: org.RoleAdmin}, UserRespBody: editorUserRespBody, GroupsRespBody: "[" + strings.Join([]string{}, ",") + "]", RoleAttributePath: gitlabAttrPath, @@ -126,7 +144,7 @@ func TestSocialGitlab_UserInfo(t *testing.T) { Name: "Fallback with no default will create a user with a default role", Cfg: conf{}, UserRespBody: editorUserRespBody, - GroupsRespBody: "[" + strings.Join([]string{}, ",") + "]", + GroupsRespBody: "[]", RoleAttributePath: gitlabAttrPath, ExpectedLogin: "gitlab-editor", ExpectedEmail: "gitlab-editor@example.org", @@ -155,13 +173,18 @@ func TestSocialGitlab_UserInfo(t *testing.T) { w.Header().Set("Content-Type", "application/json") switch r.RequestURI { case userURI: + w.WriteHeader(http.StatusOK) _, err := w.Write([]byte(test.UserRespBody)) require.NoError(t, err) case groupsURI: + w.WriteHeader(http.StatusOK) + for k, v := range test.GroupHeaders { + w.Header().Set(k, v) + } _, err := w.Write([]byte(test.GroupsRespBody)) require.NoError(t, err) default: - w.WriteHeader(http.StatusNotFound) + require.Fail(t, "unexpected request URI: "+r.RequestURI) } })) provider.apiUrl = ts.URL + apiURI @@ -383,3 +406,60 @@ func (t *tokenSource) Token() (*oauth2.Token, error) { TokenType: "Bearer", }, nil } + +func TestSocialGitlab_GetGroupsNextPage(t *testing.T) { + type Group struct { + FullPath string `json:"full_path"` + } + + groups := []Group{ + {FullPath: "admins"}, + {FullPath: "editors"}, + {FullPath: "viewers"}, + {FullPath: "serveradmins"}, + } + + calls := 0 + // Create a mock HTTP client and server + mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/groups" { + // Return a paginated response with 2 groups per page + page, err := strconv.Atoi(r.URL.Query().Get("page")) + require.NoError(t, err) + perPage := 2 + startIndex := (page - 1) * perPage + endIndex := startIndex + perPage + if endIndex > len(groups) { + endIndex = len(groups) + } + groupsPage := groups[startIndex:endIndex] + jsonBytes, err := json.Marshal(groupsPage) + require.NoError(t, err) + + w.Header().Set("X-Total", strconv.Itoa(len(groups))) + if endIndex < len(groups) { + w.Header().Set("X-Next-Page", strconv.Itoa(page+1)) + } + calls += 1 + _, err = w.Write(jsonBytes) + require.NoError(t, err) + } else { + w.WriteHeader(http.StatusNotFound) + } + })) + defer mockServer.Close() + + // Create a SocialGitlab instance with the mock server URL + s := &SocialGitlab{ + apiUrl: mockServer.URL, + SocialBase: &SocialBase{ + log: newLogger("test", "debug"), + }, + } + + // Call getGroups and verify that it returns all groups + expectedGroups := []string{"admins", "editors", "viewers", "serveradmins"} + actualGroups := s.getGroups(context.Background(), mockServer.Client()) + assert.Equal(t, expectedGroups, actualGroups) + assert.Equal(t, 2, calls) +}