Auth: Gitlab Improve group fetching when using read_api (#72277)

* improve group fetching when using read api

* add constant for access level
This commit is contained in:
Jo 2023-07-25 18:05:12 +02:00 committed by GitHub
parent ec5650cc82
commit 348233bddb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 153 additions and 35 deletions

View File

@ -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
}

View File

@ -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)
}