From 67b7c3832327370b27cf0a46aa68aedd7c058c7d Mon Sep 17 00:00:00 2001 From: CorentinPtrl Date: Thu, 6 Nov 2025 18:06:51 +0100 Subject: [PATCH 1/2] feat(gitlab): handle all groups Signed-off-by: CorentinPtrl --- connector/gitlab/gitlab.go | 142 +++++++++++++++++++++---------------- 1 file changed, 81 insertions(+), 61 deletions(-) diff --git a/connector/gitlab/gitlab.go b/connector/gitlab/gitlab.go index 7aa4439842..fe9c0ea94e 100644 --- a/connector/gitlab/gitlab.go +++ b/connector/gitlab/gitlab.go @@ -12,10 +12,9 @@ import ( "strconv" "time" - "golang.org/x/oauth2" - "github.com/dexidp/dex/connector" "github.com/dexidp/dex/pkg/groups" + "golang.org/x/oauth2" ) const ( @@ -24,6 +23,8 @@ const ( // used to retrieve groups from /oauth/userinfo // https://docs.gitlab.com/ee/integration/openid_connect_provider.html scopeOpenID = "openid" + // read operations of the /api/v4/groups endpoint + scopeReadApi = "read_api" ) // Config holds configuration options for gitlab logins. @@ -92,7 +93,7 @@ type gitlabConnector struct { func (c *gitlabConnector) oauth2Config(scopes connector.Scopes) *oauth2.Config { gitlabScopes := []string{scopeUser} if c.groupsRequired(scopes.Groups) { - gitlabScopes = []string{scopeUser, scopeOpenID} + gitlabScopes = []string{scopeUser, scopeOpenID, scopeReadApi} } gitlabEndpoint := oauth2.Endpoint{AuthURL: c.baseURL + "/oauth/authorize", TokenURL: c.baseURL + "/oauth/token"} @@ -171,7 +172,7 @@ func (c *gitlabConnector) identity(ctx context.Context, s connector.Scopes, toke } if c.groupsRequired(s.Groups) { - groups, err := c.getGroups(ctx, client, s.Groups, user.Username) + groups, err := c.getGroups(ctx, client, s.Groups, user.Username, user.ID) if err != nil { return identity, fmt.Errorf("gitlab: get groups: %v", err) } @@ -267,12 +268,64 @@ type userInfo struct { DeveloperPermission []string `json:"https://gitlab.org/claims/groups/developer"` } +type group struct { + ID int `json:"id"` + Name string `json:"name"` + Path string `json:"path"` + FullName string `json:"full_name"` + FullPath string `json:"full_path"` +} + // userGroups queries the GitLab API for group membership. // // The HTTP passed client is expected to be constructed by the golang.org/x/oauth2 package, // which inserts a bearer token as part of the request. -func (c *gitlabConnector) userGroups(ctx context.Context, client *http.Client) ([]string, error) { - req, err := http.NewRequest("GET", c.baseURL+"/oauth/userinfo", nil) +func (c *gitlabConnector) userGroups(ctx context.Context, client *http.Client, userId int) ([]string, error) { + groupsRaw, err := c.getGitlabGroups(ctx, client) + if err != nil { + return []string{}, err + } + var groups []string + for _, group := range groupsRaw { + groupMembership, notMember, err := c.getGroupsMembership(ctx, client, group.ID, userId) + if err != nil { + return []string{}, fmt.Errorf("gitlab: get group membership: %v", err) + } + if notMember { + continue + } + if !c.getGroupsPermission { + groups = append(groups, group.FullPath) + continue + } + switch groupMembership["access_level"].(float64) { + case 10: + groups = append(groups, fmt.Sprintf("%s:guest", group.FullPath)) + break + case 20: + groups = append(groups, fmt.Sprintf("%s:reporter", group.FullPath)) + break + case 30: + groups = append(groups, fmt.Sprintf("%s:developer", group.FullPath)) + break + case 40: + groups = append(groups, fmt.Sprintf("%s:maintainer", group.FullPath)) + break + case 50: + groups = append(groups, fmt.Sprintf("%s:owner", group.FullPath)) + break + case 60: + groups = append(groups, fmt.Sprintf("%s:admin", group.FullPath)) + break + } + } + + return groups, nil +} + +func (c *gitlabConnector) getGitlabGroups(ctx context.Context, client *http.Client) ([]group, error) { + var group []group + req, err := http.NewRequest("GET", c.baseURL+"/api/v4/groups", nil) if err != nil { return nil, fmt.Errorf("gitlab: new req: %v", err) } @@ -290,69 +343,36 @@ func (c *gitlabConnector) userGroups(ctx context.Context, client *http.Client) ( } return nil, fmt.Errorf("%s: %s", resp.Status, body) } - var u userInfo - if err := json.NewDecoder(resp.Body).Decode(&u); err != nil { + if err = json.NewDecoder(resp.Body).Decode(&group); err != nil { return nil, fmt.Errorf("failed to decode response: %v", err) } - - if c.getGroupsPermission { - groups := c.setGroupsPermission(u) - return groups, nil - } - - return u.Groups, nil + return group, nil } -func (c *gitlabConnector) setGroupsPermission(u userInfo) []string { - groups := u.Groups - -L1: - for _, g := range groups { - for _, op := range u.OwnerPermission { - if g == op { - groups = append(groups, fmt.Sprintf("%s:owner", g)) - continue L1 - } - if len(g) > len(op) { - if g[0:len(op)] == op && string(g[len(op)]) == "/" { - groups = append(groups, fmt.Sprintf("%s:owner", g)) - continue L1 - } - } - } - - for _, mp := range u.MaintainerPermission { - if g == mp { - groups = append(groups, fmt.Sprintf("%s:maintainer", g)) - continue L1 - } - if len(g) > len(mp) { - if g[0:len(mp)] == mp && string(g[len(mp)]) == "/" { - groups = append(groups, fmt.Sprintf("%s:maintainer", g)) - continue L1 - } - } - } - - for _, dp := range u.DeveloperPermission { - if g == dp { - groups = append(groups, fmt.Sprintf("%s:developer", g)) - continue L1 - } - if len(g) > len(dp) { - if g[0:len(dp)] == dp && string(g[len(dp)]) == "/" { - groups = append(groups, fmt.Sprintf("%s:developer", g)) - continue L1 - } - } - } +func (c *gitlabConnector) getGroupsMembership(ctx context.Context, client *http.Client, groupId int, userId int) (map[string]interface{}, bool, error) { + var groupMembership map[string]interface{} + req, err := http.NewRequest("GET", c.baseURL+fmt.Sprintf("/api/v4/groups/%d/members/all/%d", groupId, userId), nil) + if err != nil { + return map[string]interface{}{}, false, fmt.Errorf("gitlab: new req: %v", err) + } + req = req.WithContext(ctx) + resp, err := client.Do(req) + if err != nil { + return map[string]interface{}{}, false, fmt.Errorf("gitlab: get URL %v", err) } + defer resp.Body.Close() - return groups + if resp.StatusCode != http.StatusOK { + return map[string]interface{}{}, true, nil + } + if err = json.NewDecoder(resp.Body).Decode(&groupMembership); err != nil { + return map[string]interface{}{}, false, fmt.Errorf("failed to decode response: %v", err) + } + return groupMembership, false, nil } -func (c *gitlabConnector) getGroups(ctx context.Context, client *http.Client, groupScope bool, userLogin string) ([]string, error) { - gitlabGroups, err := c.userGroups(ctx, client) +func (c *gitlabConnector) getGroups(ctx context.Context, client *http.Client, groupScope bool, userLogin string, userId int) ([]string, error) { + gitlabGroups, err := c.userGroups(ctx, client, userId) if err != nil { return nil, err } From 922f8761dc01c492be8afc8f4e3279b6d8e62af3 Mon Sep 17 00:00:00 2001 From: CorentinPtrl Date: Fri, 7 Nov 2025 23:19:33 +0100 Subject: [PATCH 2/2] fix(gitlab): tests, linting Signed-off-by: CorentinPtrl --- connector/gitlab/gitlab.go | 20 ++-- connector/gitlab/gitlab_test.go | 199 +++++++++++++++++++++++++++----- 2 files changed, 176 insertions(+), 43 deletions(-) diff --git a/connector/gitlab/gitlab.go b/connector/gitlab/gitlab.go index fe9c0ea94e..3bc1af5aea 100644 --- a/connector/gitlab/gitlab.go +++ b/connector/gitlab/gitlab.go @@ -12,9 +12,10 @@ import ( "strconv" "time" + "golang.org/x/oauth2" + "github.com/dexidp/dex/connector" "github.com/dexidp/dex/pkg/groups" - "golang.org/x/oauth2" ) const ( @@ -261,13 +262,6 @@ func (c *gitlabConnector) user(ctx context.Context, client *http.Client) (gitlab return u, nil } -type userInfo struct { - Groups []string `json:"groups"` - OwnerPermission []string `json:"https://gitlab.org/claims/groups/owner"` - MaintainerPermission []string `json:"https://gitlab.org/claims/groups/maintainer"` - DeveloperPermission []string `json:"https://gitlab.org/claims/groups/developer"` -} - type group struct { ID int `json:"id"` Name string `json:"name"` @@ -285,19 +279,23 @@ func (c *gitlabConnector) userGroups(ctx context.Context, client *http.Client, u if err != nil { return []string{}, err } - var groups []string + groups := []string{} for _, group := range groupsRaw { groupMembership, notMember, err := c.getGroupsMembership(ctx, client, group.ID, userId) if err != nil { return []string{}, fmt.Errorf("gitlab: get group membership: %v", err) } - if notMember { + + if _, ok := groupMembership["access_level"]; notMember || !ok { continue } + + groups = append(groups, group.FullPath) + if !c.getGroupsPermission { - groups = append(groups, group.FullPath) continue } + switch groupMembership["access_level"].(float64) { case 10: groups = append(groups, fmt.Sprintf("%s:guest", group.FullPath)) diff --git a/connector/gitlab/gitlab_test.go b/connector/gitlab/gitlab_test.go index b67b30c045..2837729116 100644 --- a/connector/gitlab/gitlab_test.go +++ b/connector/gitlab/gitlab_test.go @@ -15,14 +15,33 @@ import ( func TestUserGroups(t *testing.T) { s := newTestServer(map[string]interface{}{ - "/oauth/userinfo": userInfo{ - Groups: []string{"team-1", "team-2"}, + "/api/v4/groups": []group{ + { + ID: 1, + Name: "team-1", + FullName: "team-1", + Path: "team-1", + FullPath: "team-1", + }, + { + ID: 2, + Name: "team-2", + FullName: "team-2", + Path: "team-2", + FullPath: "team-2", + }, + }, + "/api/v4/groups/1/members/all/1": map[string]interface{}{ + "access_level": 50, + }, + "/api/v4/groups/2/members/all/1": map[string]interface{}{ + "access_level": 50, }, }) defer s.Close() c := gitlabConnector{baseURL: s.URL} - groups, err := c.getGroups(context.Background(), newClient(), true, "joebloggs") + groups, err := c.getGroups(context.Background(), newClient(), true, "joebloggs", 1) expectNil(t, err) expectEquals(t, groups, []string{ @@ -33,14 +52,33 @@ func TestUserGroups(t *testing.T) { func TestUserGroupsWithFiltering(t *testing.T) { s := newTestServer(map[string]interface{}{ - "/oauth/userinfo": userInfo{ - Groups: []string{"team-1", "team-2"}, + "/api/v4/groups": []group{ + { + ID: 1, + Name: "team-1", + FullName: "team-1", + Path: "team-1", + FullPath: "team-1", + }, + { + ID: 2, + Name: "team-2", + FullName: "team-2", + Path: "team-2", + FullPath: "team-2", + }, + }, + "/api/v4/groups/1/members/all/1": map[string]interface{}{ + "access_level": 50, + }, + "/api/v4/groups/2/members/all/1": map[string]interface{}{ + "access_level": 50, }, }) defer s.Close() c := gitlabConnector{baseURL: s.URL, groups: []string{"team-1"}} - groups, err := c.getGroups(context.Background(), newClient(), true, "joebloggs") + groups, err := c.getGroups(context.Background(), newClient(), true, "joebloggs", 1) expectNil(t, err) expectEquals(t, groups, []string{ @@ -50,14 +88,12 @@ func TestUserGroupsWithFiltering(t *testing.T) { func TestUserGroupsWithoutOrgs(t *testing.T) { s := newTestServer(map[string]interface{}{ - "/oauth/userinfo": userInfo{ - Groups: []string{}, - }, + "/api/v4/groups": []group{}, }) defer s.Close() c := gitlabConnector{baseURL: s.URL} - groups, err := c.getGroups(context.Background(), newClient(), true, "joebloggs") + groups, err := c.getGroups(context.Background(), newClient(), true, "joebloggs", 1) expectNil(t, err) expectEquals(t, len(groups), 0) @@ -71,8 +107,17 @@ func TestUsernameIncludedInFederatedIdentity(t *testing.T) { "access_token": "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9", "expires_in": "30", }, - "/oauth/userinfo": userInfo{ - Groups: []string{"team-1"}, + "/api/v4/groups": []group{ + { + ID: 1, + Name: "team-1", + FullName: "team-1", + Path: "team-1", + FullPath: "team-1", + }, + }, + "/api/v4/groups/1/members/all/12345678": map[string]interface{}{ + "access_level": 50, }, }) defer s.Close() @@ -107,8 +152,17 @@ func TestLoginUsedAsIDWhenConfigured(t *testing.T) { "access_token": "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9", "expires_in": "30", }, - "/oauth/userinfo": userInfo{ - Groups: []string{"team-1"}, + "/api/v4/groups": []group{ + { + ID: 1, + Name: "team-1", + FullName: "team-1", + Path: "team-1", + FullPath: "team-1", + }, + }, + "/api/v4/groups/1/members/all/1": map[string]interface{}{ + "access_level": 50, }, }) defer s.Close() @@ -134,8 +188,17 @@ func TestLoginWithTeamWhitelisted(t *testing.T) { "access_token": "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9", "expires_in": "30", }, - "/oauth/userinfo": userInfo{ - Groups: []string{"team-1"}, + "/api/v4/groups": []group{ + { + ID: 1, + Name: "team-1", + FullName: "team-1", + Path: "team-1", + FullPath: "team-1", + }, + }, + "/api/v4/groups/1/members/all/12345678": map[string]interface{}{ + "access_level": 50, }, }) defer s.Close() @@ -161,8 +224,17 @@ func TestLoginWithTeamNonWhitelisted(t *testing.T) { "access_token": "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9", "expires_in": "30", }, - "/oauth/userinfo": userInfo{ - Groups: []string{"team-1"}, + "/api/v4/groups": []group{ + { + ID: 1, + Name: "team-1", + FullName: "team-1", + Path: "team-1", + FullPath: "team-1", + }, + }, + "/api/v4/groups/1/members/all/12345678": map[string]interface{}{ + "access_level": 50, }, }) defer s.Close() @@ -188,8 +260,17 @@ func TestRefresh(t *testing.T) { "refresh_token": "oRzxVjCnohYRHEYEhZshkmakKmoyVoTjfUGC", "expires_in": "30", }, - "/oauth/userinfo": userInfo{ - Groups: []string{"team-1"}, + "/api/v4/groups": []group{ + { + ID: 1, + Name: "team-1", + FullName: "team-1", + Path: "team-1", + FullPath: "team-1", + }, + }, + "/api/v4/groups/1/members/all/12345678": map[string]interface{}{ + "access_level": 50, }, }) defer s.Close() @@ -229,8 +310,17 @@ func TestRefreshWithEmptyConnectorData(t *testing.T) { "refresh_token": "oRzxVjCnohYRHEYEhZshkmakKmoyVoTjfUGC", "expires_in": "30", }, - "/oauth/userinfo": userInfo{ - Groups: []string{"team-1"}, + "/api/v4/groups": []group{ + { + ID: 1, + Name: "team-1", + FullName: "team-1", + Path: "team-1", + FullPath: "team-1", + }, + }, + "/api/v4/groups/1/members/all/12345678": map[string]interface{}{ + "access_level": 50, }, }) defer s.Close() @@ -256,11 +346,57 @@ func TestGroupsWithPermission(t *testing.T) { "access_token": "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9", "expires_in": "30", }, - "/oauth/userinfo": userInfo{ - Groups: []string{"ops", "dev", "ops-test", "ops/project", "dev/project1", "dev/project2"}, - OwnerPermission: []string{"ops"}, - DeveloperPermission: []string{"dev"}, - MaintainerPermission: []string{"dev/project1"}, + "/api/v4/groups": []group{ + { + ID: 1, + Name: "ops", + FullName: "ops", + Path: "ops", + FullPath: "ops", + }, + { + ID: 2, + Name: "dev", + FullName: "dev", + Path: "dev", + FullPath: "dev", + }, + { + ID: 3, + Name: "ops/project", + FullName: "ops/project", + Path: "ops/project", + FullPath: "ops/project", + }, + { + ID: 4, + Name: "dev/project1", + FullName: "dev/project1", + Path: "dev/project1", + FullPath: "dev/project1", + }, + { + ID: 5, + Name: "dev/project2", + FullName: "dev/project2", + Path: "dev/project2", + FullPath: "dev/project2", + }, + }, + "/api/v4/groups/1/members/all/12345678": map[string]interface{}{ + "access_level": 50, + }, + "/api/v4/groups/2/members/all/12345678": map[string]interface{}{ + "access_level": 30, + }, + "/api/v4/groups/3/members/all/12345678": map[string]interface{}{ + "access_level": 50, + }, + "/api/v4/groups/4/members/all/12345678": map[string]interface{}{ + "access_level": 40, + }, + "/api/v4/groups/5/members/all/12345678": map[string]interface{}{ + "access_level": 30, }, }) defer s.Close() @@ -277,15 +413,14 @@ func TestGroupsWithPermission(t *testing.T) { expectEquals(t, identity.Groups, []string{ "ops", - "dev", - "ops-test", - "ops/project", - "dev/project1", - "dev/project2", "ops:owner", + "dev", "dev:developer", + "ops/project", "ops/project:owner", + "dev/project1", "dev/project1:maintainer", + "dev/project2", "dev/project2:developer", }) }