From 42d0f49d9dfb0e7c5b1d6129eaecef6e72b7c236 Mon Sep 17 00:00:00 2001 From: Anton Kulyashov Date: Tue, 19 May 2026 23:36:09 +0200 Subject: [PATCH 1/3] feat(gitlab): support inherited group claims Signed-off-by: Anton Kulyashov --- connector/gitlab/gitlab.go | 281 ++++++++++++++++++++++++++++++-- connector/gitlab/gitlab_test.go | 208 ++++++++++++++++++++++- 2 files changed, 470 insertions(+), 19 deletions(-) diff --git a/connector/gitlab/gitlab.go b/connector/gitlab/gitlab.go index b9fb3bec05..4822063fa7 100644 --- a/connector/gitlab/gitlab.go +++ b/connector/gitlab/gitlab.go @@ -23,9 +23,23 @@ import ( const ( // read operations of the /api/v4/user endpoint scopeUser = "read_user" + // read operations of the REST API, including /api/v4/groups + scopeReadAPI = "read_api" // used to retrieve groups from /oauth/userinfo // https://docs.gitlab.com/ee/integration/openid_connect_provider.html scopeOpenID = "openid" + + inheritedGroupsPerPage = 100 + + accessLevelMinimalAccess = 5 + accessLevelGuest = 10 + accessLevelPlanner = 15 + accessLevelReporter = 20 + accessLevelSecurityMgr = 25 + accessLevelDeveloper = 30 + accessLevelMaintainer = 40 + accessLevelOwner = 50 + accessLevelAdmin = 60 ) // Config holds configuration options for gitlab logins. @@ -37,7 +51,10 @@ type Config struct { Groups []string `json:"groups"` UseLoginAsID bool `json:"useLoginAsID"` GetGroupsPermission bool `json:"getGroupsPermission"` - RootCAData []byte `json:"rootCAData,omitempty"` + // When enabled, Dex uses /api/v4/groups as the source of truth for group names so + // inherited memberships are included as well. This requires GitLab's read_api scope. + InheritedGroups bool `json:"inheritedGroups"` + RootCAData []byte `json:"rootCAData,omitempty"` } type gitlabUser struct { @@ -76,6 +93,7 @@ func (c *Config) Open(id string, logger *slog.Logger) (connector.Connector, erro groups: c.Groups, useLoginAsID: c.UseLoginAsID, getGroupsPermission: c.GetGroupsPermission, + inheritedGroups: c.InheritedGroups, httpClient: httpClient, }, nil } @@ -105,12 +123,17 @@ type gitlabConnector struct { // if set to true permissions will be added to list of groups getGroupsPermission bool + + // if set to true inherited groups will be retrieved from /api/v4/groups + inheritedGroups bool } func (c *gitlabConnector) oauth2Config(scopes connector.Scopes) *oauth2.Config { - gitlabScopes := []string{scopeUser} + gitlabScopes := []string{scopeUser, scopeOpenID} if c.groupsRequired(scopes.Groups) { - gitlabScopes = []string{scopeUser, scopeOpenID} + if c.inheritedGroups { + gitlabScopes = append(gitlabScopes, scopeReadAPI) + } } gitlabEndpoint := oauth2.Endpoint{AuthURL: c.baseURL + "/oauth/authorize", TokenURL: c.baseURL + "/oauth/token"} @@ -189,7 +212,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) } @@ -313,42 +336,268 @@ type userInfo struct { DeveloperPermission []string `json:"https://gitlab.org/claims/groups/developer"` } -// userGroups queries the GitLab API for group membership. +type gitlabGroup struct { + ID int `json:"id"` + FullPath string `json:"full_path"` + Path string `json:"path"` +} + +type gitlabGroupMember struct { + AccessLevel int `json:"access_level"` +} + +// userInfo queries the GitLab OIDC userinfo endpoint for profile and direct 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) { +func (c *gitlabConnector) userInfo(ctx context.Context, client *http.Client) (userInfo, error) { + var u userInfo req, err := http.NewRequest("GET", c.baseURL+"/oauth/userinfo", nil) if err != nil { - return nil, fmt.Errorf("gitlab: new req: %v", err) + return u, fmt.Errorf("gitlab: new req: %v", err) } req = req.WithContext(ctx) resp, err := client.Do(req) if err != nil { - return nil, fmt.Errorf("gitlab: get URL %v", err) + return u, fmt.Errorf("gitlab: get URL %v", err) } defer resp.Body.Close() if resp.StatusCode != http.StatusOK { body, err := io.ReadAll(resp.Body) if err != nil { - return nil, fmt.Errorf("gitlab: read body: %v", err) + return u, fmt.Errorf("gitlab: read body: %v", err) } - return nil, fmt.Errorf("%s: %s", resp.Status, body) + return u, fmt.Errorf("%s: %s", resp.Status, body) } - var u userInfo if err := json.NewDecoder(resp.Body).Decode(&u); err != nil { - return nil, fmt.Errorf("failed to decode response: %v", err) + return u, fmt.Errorf("failed to decode response: %v", err) + } + return u, nil +} + +// apiUserGroups queries the GitLab groups API for all groups the current user is a member of. +// When inheritedGroups is enabled, this becomes the source of truth for group names. +func (c *gitlabConnector) apiUserGroups(ctx context.Context, client *http.Client) ([]gitlabGroup, error) { + apiGroupsAll := make([]gitlabGroup, 0) + for page := 1; ; page++ { + req, err := http.NewRequest("GET", c.baseURL+"/api/v4/groups", nil) + if err != nil { + return nil, fmt.Errorf("gitlab: new req: %v", err) + } + + q := req.URL.Query() + q.Set("all_available", "false") + q.Set("per_page", strconv.Itoa(inheritedGroupsPerPage)) + q.Set("page", strconv.Itoa(page)) + req.URL.RawQuery = q.Encode() + req = req.WithContext(ctx) + + resp, err := client.Do(req) + if err != nil { + return nil, fmt.Errorf("gitlab: get URL %v", err) + } + + if resp.StatusCode != http.StatusOK { + body, err := io.ReadAll(resp.Body) + _ = resp.Body.Close() + if err != nil { + return nil, fmt.Errorf("gitlab: read body: %v", err) + } + return nil, fmt.Errorf("%s: %s", resp.Status, body) + } + + var apiGroups []gitlabGroup + if err := json.NewDecoder(resp.Body).Decode(&apiGroups); err != nil { + _ = resp.Body.Close() + return nil, fmt.Errorf("failed to decode response: %v", err) + } + _ = resp.Body.Close() + + apiGroupsAll = append(apiGroupsAll, apiGroups...) + + if len(apiGroups) < inheritedGroupsPerPage { + break + } + } + + return apiGroupsAll, nil +} + +// userGroups queries the GitLab APIs 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, userID int) ([]string, error) { + if c.inheritedGroups { + apiGroups, err := c.apiUserGroups(ctx, client) + if err != nil { + return nil, err + } + + if !c.getGroupsPermission { + return groupNames(apiGroups), nil + } + + u, err := c.userInfo(ctx, client) + if err != nil { + return nil, err + } + + return c.apiGroupsWithPermission(ctx, client, apiGroups, userID, u) + } + + u, err := c.userInfo(ctx, client) + if err != nil { + return nil, err } if c.getGroupsPermission { - groups := c.setGroupsPermission(u) - return groups, nil + return c.setGroupsPermission(u), nil } return u.Groups, nil } +func (c *gitlabConnector) apiGroupsWithPermission(ctx context.Context, client *http.Client, apiGroups []gitlabGroup, userID int, u userInfo) ([]string, error) { + if userID == 0 { + return nil, errors.New("gitlab: user id is required to fetch effective group permissions") + } + + groups := groupNames(apiGroups) + for _, g := range apiGroups { + groupPath := groupName(g) + if groupPath == "" { + continue + } + + if permission, ok := userInfoPermission(groupPath, u); ok { + groups = append(groups, fmt.Sprintf("%s:%s", groupPath, permission)) + continue + } + + permission, ok, err := c.apiGroupPermission(ctx, client, g.ID, userID) + if err != nil { + return nil, err + } + if ok { + groups = append(groups, fmt.Sprintf("%s:%s", groupPath, permission)) + } + } + + return groups, nil +} + +func (c *gitlabConnector) apiGroupPermission(ctx context.Context, client *http.Client, groupID, userID int) (string, bool, error) { + if groupID == 0 { + return "", false, errors.New("gitlab: group id is required to fetch effective group permissions") + } + + req, err := http.NewRequest("GET", fmt.Sprintf("%s/api/v4/groups/%d/members/all/%d", c.baseURL, groupID, userID), nil) + if err != nil { + return "", false, fmt.Errorf("gitlab: new req: %v", err) + } + + req = req.WithContext(ctx) + resp, err := client.Do(req) + if err != nil { + return "", false, fmt.Errorf("gitlab: get URL %v", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + body, err := io.ReadAll(resp.Body) + if err != nil { + return "", false, fmt.Errorf("gitlab: read body: %v", err) + } + if resp.StatusCode == http.StatusForbidden || resp.StatusCode == http.StatusNotFound { + if c.logger != nil { + c.logger.Debug("gitlab: skipping effective group permission lookup", "groupID", groupID, "userID", userID, "status", resp.Status) + } + return "", false, nil + } + return "", false, fmt.Errorf("%s: %s", resp.Status, body) + } + + var member gitlabGroupMember + if err := json.NewDecoder(resp.Body).Decode(&member); err != nil { + return "", false, fmt.Errorf("failed to decode response: %v", err) + } + + permission, ok := accessLevelPermission(member.AccessLevel) + return permission, ok, nil +} + +func groupNames(apiGroups []gitlabGroup) []string { + groups := make([]string, 0, len(apiGroups)) + for _, g := range apiGroups { + if groupPath := groupName(g); groupPath != "" { + groups = append(groups, groupPath) + } + } + + return groups +} + +func groupName(g gitlabGroup) string { + if g.FullPath != "" { + return g.FullPath + } + return g.Path +} + +func userInfoPermission(groupPath string, u userInfo) (string, bool) { + if permissionMatches(groupPath, u.OwnerPermission) { + return "owner", true + } + if permissionMatches(groupPath, u.MaintainerPermission) { + return "maintainer", true + } + if permissionMatches(groupPath, u.DeveloperPermission) { + return "developer", true + } + return "", false +} + +func permissionMatches(groupPath string, permissions []string) bool { + for _, permissionPath := range permissions { + if groupPath == permissionPath { + return true + } + if len(groupPath) > len(permissionPath) && + groupPath[0:len(permissionPath)] == permissionPath && + string(groupPath[len(permissionPath)]) == "/" { + return true + } + } + return false +} + +func accessLevelPermission(accessLevel int) (string, bool) { + switch accessLevel { + case accessLevelMinimalAccess: + return "minimal_access", true + case accessLevelGuest: + return "guest", true + case accessLevelPlanner: + return "planner", true + case accessLevelReporter: + return "reporter", true + case accessLevelSecurityMgr: + return "security_manager", true + case accessLevelDeveloper: + return "developer", true + case accessLevelMaintainer: + return "maintainer", true + case accessLevelOwner: + return "owner", true + case accessLevelAdmin: + return "admin", true + default: + return "", false + } +} + func (c *gitlabConnector) setGroupsPermission(u userInfo) []string { groups := u.Groups @@ -397,8 +646,8 @@ L1: return groups } -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 } diff --git a/connector/gitlab/gitlab_test.go b/connector/gitlab/gitlab_test.go index 9261464329..630b1c5f58 100644 --- a/connector/gitlab/gitlab_test.go +++ b/connector/gitlab/gitlab_test.go @@ -176,6 +176,24 @@ func TestHandleCallbackWithoutRootCADataFailsTLS(t *testing.T) { } } +func TestOAuth2ConfigScopesForInheritedGroups(t *testing.T) { + c := gitlabConnector{inheritedGroups: true} + + cfg := c.oauth2Config(connector.Scopes{}) + expectEquals(t, cfg.Scopes, []string{scopeUser, scopeOpenID}) + + cfg = c.oauth2Config(connector.Scopes{Groups: true}) + expectEquals(t, cfg.Scopes, []string{scopeUser, scopeOpenID, scopeReadAPI}) + + c.groups = []string{"team-1"} + cfg = c.oauth2Config(connector.Scopes{}) + expectEquals(t, cfg.Scopes, []string{scopeUser, scopeOpenID, scopeReadAPI}) + + c.getGroupsPermission = true + cfg = c.oauth2Config(connector.Scopes{Groups: true}) + expectEquals(t, cfg.Scopes, []string{scopeUser, scopeOpenID, scopeReadAPI}) +} + func TestUserGroups(t *testing.T) { s := newTestServer(map[string]interface{}{ "/oauth/userinfo": userInfo{ @@ -185,7 +203,7 @@ func TestUserGroups(t *testing.T) { 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", 12345678) expectNil(t, err) expectEquals(t, groups, []string{ @@ -194,6 +212,56 @@ func TestUserGroups(t *testing.T) { }) } +func TestUserGroupsWithInheritedGroups(t *testing.T) { + s := newTestServer(map[string]interface{}{ + "/oauth/userinfo": userInfo{ + Groups: []string{"team-legacy"}, + }, + "/api/v4/groups?all_available=false&page=1&per_page=100": []gitlabGroup{ + {FullPath: "team-1"}, + {FullPath: "team-2/sub"}, + }, + }) + defer s.Close() + + c := gitlabConnector{baseURL: s.URL, inheritedGroups: true} + groups, err := c.getGroups(context.Background(), newClient(), true, "joebloggs", 12345678) + + expectNil(t, err) + expectEquals(t, groups, []string{ + "team-1", + "team-2/sub", + }) +} + +func TestUserGroupsWithInheritedGroupsPagination(t *testing.T) { + pageOneGroups := make([]gitlabGroup, 0, inheritedGroupsPerPage) + expectedGroups := make([]string, 0, inheritedGroupsPerPage+1) + for i := 0; i < inheritedGroupsPerPage; i++ { + group := fmt.Sprintf("team-%03d", i) + pageOneGroups = append(pageOneGroups, gitlabGroup{FullPath: group}) + expectedGroups = append(expectedGroups, group) + } + expectedGroups = append(expectedGroups, "team-100") + + s := newTestServer(map[string]interface{}{ + "/oauth/userinfo": userInfo{ + Groups: []string{}, + }, + "/api/v4/groups?all_available=false&page=1&per_page=100": pageOneGroups, + "/api/v4/groups?all_available=false&page=2&per_page=100": []gitlabGroup{ + {FullPath: "team-100"}, + }, + }) + defer s.Close() + + c := gitlabConnector{baseURL: s.URL, inheritedGroups: true} + groups, err := c.getGroups(context.Background(), newClient(), true, "joebloggs", 12345678) + + expectNil(t, err) + expectEquals(t, groups, expectedGroups) +} + func TestUserGroupsWithFiltering(t *testing.T) { s := newTestServer(map[string]interface{}{ "/oauth/userinfo": userInfo{ @@ -203,7 +271,7 @@ func TestUserGroupsWithFiltering(t *testing.T) { 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", 12345678) expectNil(t, err) expectEquals(t, groups, []string{ @@ -211,6 +279,31 @@ func TestUserGroupsWithFiltering(t *testing.T) { }) } +func TestUserGroupsWithInheritedGroupsFiltering(t *testing.T) { + s := newTestServer(map[string]interface{}{ + "/oauth/userinfo": userInfo{ + Groups: []string{"team-legacy"}, + }, + "/api/v4/groups?all_available=false&page=1&per_page=100": []gitlabGroup{ + {FullPath: "team-1"}, + {FullPath: "team-2/sub"}, + }, + }) + defer s.Close() + + c := gitlabConnector{ + baseURL: s.URL, + groups: []string{"team-2/sub"}, + inheritedGroups: true, + } + groups, err := c.getGroups(context.Background(), newClient(), true, "joebloggs", 12345678) + + expectNil(t, err) + expectEquals(t, groups, []string{ + "team-2/sub", + }) +} + func TestUserGroupsWithoutOrgs(t *testing.T) { s := newTestServer(map[string]interface{}{ "/oauth/userinfo": userInfo{ @@ -220,7 +313,7 @@ func TestUserGroupsWithoutOrgs(t *testing.T) { 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", 12345678) expectNil(t, err) expectEquals(t, len(groups), 0) @@ -453,6 +546,115 @@ func TestGroupsWithPermission(t *testing.T) { }) } +func TestGroupsWithPermissionAndInheritedGroups(t *testing.T) { + s := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Add("Content-Type", "application/json") + + switch r.RequestURI { + case "/api/v4/user": + json.NewEncoder(w).Encode(gitlabUser{Email: "some@email.com", ID: 12345678, Name: "Joe Bloggs", Username: "joebloggs"}) + case "/oauth/token": + json.NewEncoder(w).Encode(map[string]interface{}{ + "access_token": "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9", + "expires_in": "30", + }) + case "/oauth/userinfo": + json.NewEncoder(w).Encode(userInfo{ + Groups: []string{"ignored-direct-group"}, + OwnerPermission: []string{"ops"}, + }) + case "/api/v4/groups?all_available=false&page=1&per_page=100": + json.NewEncoder(w).Encode([]gitlabGroup{ + {ID: 1, FullPath: "ops"}, + {ID: 2, FullPath: "ops/project"}, + {ID: 3, FullPath: "analytics"}, + }) + case "/api/v4/groups/3/members/all/12345678": + json.NewEncoder(w).Encode(gitlabGroupMember{AccessLevel: accessLevelReporter}) + default: + http.NotFound(w, r) + } + })) + defer s.Close() + + hostURL, err := url.Parse(s.URL) + expectNil(t, err) + + req, err := http.NewRequest("GET", hostURL.String(), nil) + expectNil(t, err) + + c := gitlabConnector{ + baseURL: s.URL, + httpClient: newClient(), + getGroupsPermission: true, + inheritedGroups: true, + } + identity, err := c.HandleCallback(connector.Scopes{Groups: true}, nil, req) + expectNil(t, err) + + expectEquals(t, identity.Groups, []string{ + "ops", + "ops/project", + "analytics", + "ops:owner", + "ops/project:owner", + "analytics:reporter", + }) +} + +func TestGroupsWithPermissionAndInheritedGroupsSkipsForbiddenPermissionLookup(t *testing.T) { + s := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Add("Content-Type", "application/json") + + switch r.RequestURI { + case "/api/v4/user": + json.NewEncoder(w).Encode(gitlabUser{Email: "some@email.com", ID: 12345678, Name: "Joe Bloggs", Username: "joebloggs"}) + case "/oauth/token": + json.NewEncoder(w).Encode(map[string]interface{}{ + "access_token": "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9", + "expires_in": "30", + }) + case "/oauth/userinfo": + json.NewEncoder(w).Encode(userInfo{ + Groups: []string{"ignored-direct-group"}, + OwnerPermission: []string{"ops"}, + }) + case "/api/v4/groups?all_available=false&page=1&per_page=100": + json.NewEncoder(w).Encode([]gitlabGroup{ + {ID: 1, FullPath: "ops"}, + {ID: 2, FullPath: "private/analytics"}, + }) + case "/api/v4/groups/2/members/all/12345678": + w.WriteHeader(http.StatusForbidden) + _, _ = w.Write([]byte(`{"message":"403 Forbidden"}`)) + default: + http.NotFound(w, r) + } + })) + defer s.Close() + + hostURL, err := url.Parse(s.URL) + expectNil(t, err) + + req, err := http.NewRequest("GET", hostURL.String(), nil) + expectNil(t, err) + + c := gitlabConnector{ + baseURL: s.URL, + httpClient: newClient(), + getGroupsPermission: true, + inheritedGroups: true, + } + identity, err := c.HandleCallback(connector.Scopes{Groups: true}, nil, req) + expectNil(t, err) + + expectEquals(t, identity.Groups, []string{ + "ops", + "private/analytics", + "ops:owner", + }) +} + func newTestServer(responses map[string]interface{}) *httptest.Server { return httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { response := responses[r.RequestURI] From 777b1840102c843ec0c7744b836dc7d5eeea2642 Mon Sep 17 00:00:00 2001 From: Anton Kulyashov Date: Wed, 20 May 2026 21:54:53 +0200 Subject: [PATCH 2/3] feat(gitlab): support inherited group claims - post-review fixes Signed-off-by: Anton Kulyashov --- connector/gitlab/gitlab.go | 48 ++++++++++++++++++++++++++++---------- 1 file changed, 36 insertions(+), 12 deletions(-) diff --git a/connector/gitlab/gitlab.go b/connector/gitlab/gitlab.go index 4822063fa7..a7f0a101d8 100644 --- a/connector/gitlab/gitlab.go +++ b/connector/gitlab/gitlab.go @@ -28,9 +28,11 @@ const ( // used to retrieve groups from /oauth/userinfo // https://docs.gitlab.com/ee/integration/openid_connect_provider.html scopeOpenID = "openid" +) +const ( + //constants for inheritedGroups flag inheritedGroupsPerPage = 100 - accessLevelMinimalAccess = 5 accessLevelGuest = 10 accessLevelPlanner = 15 @@ -44,16 +46,24 @@ const ( // Config holds configuration options for gitlab logins. type Config struct { - BaseURL string `json:"baseURL"` - ClientID string `json:"clientID"` - ClientSecret string `json:"clientSecret"` - RedirectURI string `json:"redirectURI"` - Groups []string `json:"groups"` - UseLoginAsID bool `json:"useLoginAsID"` - GetGroupsPermission bool `json:"getGroupsPermission"` + // BaseURL is the root URL of the GitLab instance. Defaults to https://gitlab.com. + BaseURL string `json:"baseURL"` + // ClientID is the OAuth client ID registered in GitLab. + ClientID string `json:"clientID"` + // ClientSecret is the OAuth client secret registered in GitLab. + ClientSecret string `json:"clientSecret"` + // RedirectURI is the callback URL configured for the GitLab OAuth application. + RedirectURI string `json:"redirectURI"` + // Groups limits logins to users who belong to at least one of the configured GitLab groups. + Groups []string `json:"groups"` + // UseLoginAsID uses the GitLab username as the Dex user ID instead of the numeric GitLab user ID. + UseLoginAsID bool `json:"useLoginAsID"` + // GetGroupsPermission appends role-qualified entries, such as group:owner, to the groups claim. + GetGroupsPermission bool `json:"getGroupsPermission"` // When enabled, Dex uses /api/v4/groups as the source of truth for group names so // inherited memberships are included as well. This requires GitLab's read_api scope. InheritedGroups bool `json:"inheritedGroups"` + // RootCAData is a PEM-encoded CA bundle used to trust custom TLS certificates on the GitLab instance. RootCAData []byte `json:"rootCAData,omitempty"` } @@ -561,14 +571,28 @@ func userInfoPermission(groupPath string, u userInfo) (string, bool) { func permissionMatches(groupPath string, permissions []string) bool { for _, permissionPath := range permissions { + // Exact group match, for example "ops" matches "ops". if groupPath == permissionPath { return true } - if len(groupPath) > len(permissionPath) && - groupPath[0:len(permissionPath)] == permissionPath && - string(groupPath[len(permissionPath)]) == "/" { - return true + + // A parent-group permission cannot match a shorter or equally long path. + if len(groupPath) <= len(permissionPath) { + continue } + + // The permission path must be a prefix of the subgroup path. + if groupPath[0:len(permissionPath)] != permissionPath { + continue + } + + // Require a path separator so "dev" does not match "developer". + if string(groupPath[len(permissionPath)]) != "/" { + continue + } + + // Parent-group permissions apply to descendant subgroups. + return true } return false } From 086f500306932a385fbfb178a539f9e619b0644e Mon Sep 17 00:00:00 2001 From: Anton Kulyashov Date: Wed, 20 May 2026 22:57:07 +0200 Subject: [PATCH 3/3] feat(gitlab): support inherited group claims - post-review fixes and functions reimagining Signed-off-by: Anton Kulyashov --- connector/gitlab/gitlab.go | 313 +++++++++++++++----------------- connector/gitlab/gitlab_test.go | 12 +- 2 files changed, 153 insertions(+), 172 deletions(-) diff --git a/connector/gitlab/gitlab.go b/connector/gitlab/gitlab.go index a7f0a101d8..cc681a824a 100644 --- a/connector/gitlab/gitlab.go +++ b/connector/gitlab/gitlab.go @@ -31,8 +31,8 @@ const ( ) const ( - //constants for inheritedGroups flag - inheritedGroupsPerPage = 100 + // constants for inheritedGroups flag + inheritedGroupsPerPage = 100 accessLevelMinimalAccess = 5 accessLevelGuest = 10 accessLevelPlanner = 15 @@ -62,9 +62,9 @@ type Config struct { GetGroupsPermission bool `json:"getGroupsPermission"` // When enabled, Dex uses /api/v4/groups as the source of truth for group names so // inherited memberships are included as well. This requires GitLab's read_api scope. - InheritedGroups bool `json:"inheritedGroups"` - // RootCAData is a PEM-encoded CA bundle used to trust custom TLS certificates on the GitLab instance. - RootCAData []byte `json:"rootCAData,omitempty"` + InheritedGroups bool `json:"inheritedGroups"` + // RootCAData is a PEM-encoded CA bundle used to trust custom TLS certificates on the GitLab instance. + RootCAData []byte `json:"rootCAData,omitempty"` } type gitlabUser struct { @@ -138,6 +138,7 @@ type gitlabConnector struct { inheritedGroups bool } +// oauth2Config builds the OAuth2 client configuration and scopes for this connector. func (c *gitlabConnector) oauth2Config(scopes connector.Scopes) *oauth2.Config { gitlabScopes := []string{scopeUser, scopeOpenID} if c.groupsRequired(scopes.Groups) { @@ -156,6 +157,7 @@ func (c *gitlabConnector) oauth2Config(scopes connector.Scopes) *oauth2.Config { } } +// LoginURL returns the GitLab authorization URL for the requested scopes. func (c *gitlabConnector) LoginURL(scopes connector.Scopes, callbackURL, state string) (string, []byte, error) { if c.redirectURI != callbackURL { return "", nil, fmt.Errorf("expected callback URL %q did not match the URL in the config %q", c.redirectURI, callbackURL) @@ -168,6 +170,7 @@ type oauth2Error struct { errorDescription string } +// Error formats the OAuth error returned by GitLab during the callback flow. func (e *oauth2Error) Error() string { if e.errorDescription == "" { return e.error @@ -175,6 +178,7 @@ func (e *oauth2Error) Error() string { return e.error + ": " + e.errorDescription } +// HandleCallback exchanges the authorization code and resolves the authenticated identity. func (c *gitlabConnector) HandleCallback(s connector.Scopes, connData []byte, r *http.Request) (identity connector.Identity, err error) { q := r.URL.Query() if errType := q.Get("error"); errType != "" { @@ -196,6 +200,7 @@ func (c *gitlabConnector) HandleCallback(s connector.Scopes, connData []byte, r return c.identity(ctx, s, token) } +// identity resolves the Dex identity fields from a GitLab access token. func (c *gitlabConnector) identity(ctx context.Context, s connector.Scopes, token *oauth2.Token) (identity connector.Identity, err error) { oauth2Config := c.oauth2Config(s) client := oauth2Config.Client(ctx, token) @@ -222,7 +227,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, user.ID) + groups, err := c.resolveIdentityGroups(ctx, client, s.Groups, user.Username, user.ID) if err != nil { return identity, fmt.Errorf("gitlab: get groups: %v", err) } @@ -241,6 +246,7 @@ func (c *gitlabConnector) identity(ctx context.Context, s connector.Scopes, toke return identity, nil } +// Refresh rebuilds the identity using the stored refresh token or access token. func (c *gitlabConnector) Refresh(ctx context.Context, s connector.Scopes, ident connector.Identity) (connector.Identity, error) { var data connectorData if err := json.Unmarshal(ident.ConnectorData, &data); err != nil { @@ -305,6 +311,7 @@ func (c *gitlabConnector) TokenIdentity(ctx context.Context, _, subjectToken str return c.identity(ctx, scopes, token) } +// groupsRequired reports whether this request needs group resolution. func (c *gitlabConnector) groupsRequired(groupScope bool) bool { return len(c.groups) > 0 || groupScope } @@ -356,11 +363,98 @@ type gitlabGroupMember struct { AccessLevel int `json:"access_level"` } -// userInfo queries the GitLab OIDC userinfo endpoint for profile and direct group membership. +// resolveIdentityGroups resolves group claims and applies configured group filtering. +func (c *gitlabConnector) resolveIdentityGroups(ctx context.Context, client *http.Client, groupScope bool, userLogin string, userID int) ([]string, error) { + gitlabGroups, err := c.resolveGroupClaims(ctx, client, userID) + if err != nil { + return nil, err + } + + if len(c.groups) > 0 { + filteredGroups := groups.Filter(gitlabGroups, c.groups) + if len(filteredGroups) == 0 { + return nil, fmt.Errorf("gitlab: user %q is not in any of the required groups", userLogin) + } + return filteredGroups, nil + } else if groupScope { + return gitlabGroups, nil + } + + return nil, nil +} + +// resolveGroupClaims selects the direct or inherited group resolution path. +func (c *gitlabConnector) resolveGroupClaims(ctx context.Context, client *http.Client, userID int) ([]string, error) { + if c.inheritedGroups { + return c.resolveInheritedGroupClaims(ctx, client, userID) + } + + return c.resolveDirectGroupClaims(ctx, client) +} + +// resolveDirectGroupClaims returns group claims from the OIDC userinfo response. +func (c *gitlabConnector) resolveDirectGroupClaims(ctx context.Context, client *http.Client) ([]string, error) { + u, err := c.fetchUserInfo(ctx, client) + if err != nil { + return nil, err + } + + if !c.getGroupsPermission { + return u.Groups, nil + } + + return appendPermissionsFromUserInfo(u.Groups, u), nil +} + +// resolveInheritedGroupClaims returns group claims from the GitLab groups API. +func (c *gitlabConnector) resolveInheritedGroupClaims(ctx context.Context, client *http.Client, userID int) ([]string, error) { + groupRecords, err := c.fetchInheritedGroupRecords(ctx, client) + if err != nil { + return nil, err + } + + groupClaims := groupPathsFromRecords(groupRecords) + if !c.getGroupsPermission { + return groupClaims, nil + } + + u, err := c.fetchUserInfo(ctx, client) + if err != nil { + return nil, err + } + + groupClaims = appendPermissionsFromUserInfo(groupClaims, u) + if userID == 0 { + return nil, errors.New("gitlab: user id is required to fetch effective group permissions") + } + + for _, groupRecord := range groupRecords { + groupPath := groupPathFromRecord(groupRecord) + if groupPath == "" { + continue + } + + if _, ok := permissionFromUserInfo(groupPath, u); ok { + continue + } + + permission, ok, err := c.fetchEffectiveGroupPermission(ctx, client, groupRecord.ID, userID) + if err != nil { + return nil, err + } + if ok { + groupClaims = append(groupClaims, fmt.Sprintf("%s:%s", groupPath, permission)) + } + } + + return groupClaims, nil +} + +// fetchUserInfo queries the GitLab OIDC userinfo endpoint for profile and direct 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) userInfo(ctx context.Context, client *http.Client) (userInfo, error) { +func (c *gitlabConnector) fetchUserInfo(ctx context.Context, client *http.Client) (userInfo, error) { var u userInfo req, err := http.NewRequest("GET", c.baseURL+"/oauth/userinfo", nil) if err != nil { @@ -386,10 +480,10 @@ func (c *gitlabConnector) userInfo(ctx context.Context, client *http.Client) (us return u, nil } -// apiUserGroups queries the GitLab groups API for all groups the current user is a member of. +// fetchInheritedGroupRecords queries the GitLab groups API for all groups the current user is a member of. // When inheritedGroups is enabled, this becomes the source of truth for group names. -func (c *gitlabConnector) apiUserGroups(ctx context.Context, client *http.Client) ([]gitlabGroup, error) { - apiGroupsAll := make([]gitlabGroup, 0) +func (c *gitlabConnector) fetchInheritedGroupRecords(ctx context.Context, client *http.Client) ([]gitlabGroup, error) { + groupRecords := make([]gitlabGroup, 0) for page := 1; ; page++ { req, err := http.NewRequest("GET", c.baseURL+"/api/v4/groups", nil) if err != nil { @@ -417,88 +511,25 @@ func (c *gitlabConnector) apiUserGroups(ctx context.Context, client *http.Client return nil, fmt.Errorf("%s: %s", resp.Status, body) } - var apiGroups []gitlabGroup - if err := json.NewDecoder(resp.Body).Decode(&apiGroups); err != nil { + var pageGroupRecords []gitlabGroup + if err := json.NewDecoder(resp.Body).Decode(&pageGroupRecords); err != nil { _ = resp.Body.Close() return nil, fmt.Errorf("failed to decode response: %v", err) } _ = resp.Body.Close() - apiGroupsAll = append(apiGroupsAll, apiGroups...) + groupRecords = append(groupRecords, pageGroupRecords...) - if len(apiGroups) < inheritedGroupsPerPage { + if len(pageGroupRecords) < inheritedGroupsPerPage { break } } - return apiGroupsAll, nil -} - -// userGroups queries the GitLab APIs 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, userID int) ([]string, error) { - if c.inheritedGroups { - apiGroups, err := c.apiUserGroups(ctx, client) - if err != nil { - return nil, err - } - - if !c.getGroupsPermission { - return groupNames(apiGroups), nil - } - - u, err := c.userInfo(ctx, client) - if err != nil { - return nil, err - } - - return c.apiGroupsWithPermission(ctx, client, apiGroups, userID, u) - } - - u, err := c.userInfo(ctx, client) - if err != nil { - return nil, err - } - - if c.getGroupsPermission { - return c.setGroupsPermission(u), nil - } - - return u.Groups, nil -} - -func (c *gitlabConnector) apiGroupsWithPermission(ctx context.Context, client *http.Client, apiGroups []gitlabGroup, userID int, u userInfo) ([]string, error) { - if userID == 0 { - return nil, errors.New("gitlab: user id is required to fetch effective group permissions") - } - - groups := groupNames(apiGroups) - for _, g := range apiGroups { - groupPath := groupName(g) - if groupPath == "" { - continue - } - - if permission, ok := userInfoPermission(groupPath, u); ok { - groups = append(groups, fmt.Sprintf("%s:%s", groupPath, permission)) - continue - } - - permission, ok, err := c.apiGroupPermission(ctx, client, g.ID, userID) - if err != nil { - return nil, err - } - if ok { - groups = append(groups, fmt.Sprintf("%s:%s", groupPath, permission)) - } - } - - return groups, nil + return groupRecords, nil } -func (c *gitlabConnector) apiGroupPermission(ctx context.Context, client *http.Client, groupID, userID int) (string, bool, error) { +// fetchEffectiveGroupPermission returns the effective permission for a user within a GitLab group. +func (c *gitlabConnector) fetchEffectiveGroupPermission(ctx context.Context, client *http.Client, groupID, userID int) (string, bool, error) { if groupID == 0 { return "", false, errors.New("gitlab: group id is required to fetch effective group permissions") } @@ -534,43 +565,59 @@ func (c *gitlabConnector) apiGroupPermission(ctx context.Context, client *http.C return "", false, fmt.Errorf("failed to decode response: %v", err) } - permission, ok := accessLevelPermission(member.AccessLevel) + permission, ok := permissionFromAccessLevel(member.AccessLevel) return permission, ok, nil } -func groupNames(apiGroups []gitlabGroup) []string { - groups := make([]string, 0, len(apiGroups)) - for _, g := range apiGroups { - if groupPath := groupName(g); groupPath != "" { - groups = append(groups, groupPath) +// groupPathsFromRecords extracts claim-ready group paths from GitLab group records. +func groupPathsFromRecords(groupRecords []gitlabGroup) []string { + groupPaths := make([]string, 0, len(groupRecords)) + for _, groupRecord := range groupRecords { + if groupPath := groupPathFromRecord(groupRecord); groupPath != "" { + groupPaths = append(groupPaths, groupPath) } } - return groups + return groupPaths } -func groupName(g gitlabGroup) string { - if g.FullPath != "" { - return g.FullPath +// groupPathFromRecord returns the canonical path for a GitLab group record. +func groupPathFromRecord(groupRecord gitlabGroup) string { + if groupRecord.FullPath != "" { + return groupRecord.FullPath } - return g.Path + return groupRecord.Path } -func userInfoPermission(groupPath string, u userInfo) (string, bool) { - if permissionMatches(groupPath, u.OwnerPermission) { +// appendPermissionsFromUserInfo adds permission-qualified group claims derived from userinfo. +func appendPermissionsFromUserInfo(groupPaths []string, u userInfo) []string { + groupsWithPermissions := append([]string(nil), groupPaths...) + for _, groupPath := range groupPaths { + if permission, ok := permissionFromUserInfo(groupPath, u); ok { + groupsWithPermissions = append(groupsWithPermissions, fmt.Sprintf("%s:%s", groupPath, permission)) + } + } + + return groupsWithPermissions +} + +// permissionFromUserInfo resolves a permission suffix for a group path from userinfo claims. +func permissionFromUserInfo(groupPath string, u userInfo) (string, bool) { + if matchesGroupPathOrAncestor(groupPath, u.OwnerPermission) { return "owner", true } - if permissionMatches(groupPath, u.MaintainerPermission) { + if matchesGroupPathOrAncestor(groupPath, u.MaintainerPermission) { return "maintainer", true } - if permissionMatches(groupPath, u.DeveloperPermission) { + if matchesGroupPathOrAncestor(groupPath, u.DeveloperPermission) { return "developer", true } return "", false } -func permissionMatches(groupPath string, permissions []string) bool { - for _, permissionPath := range permissions { +// matchesGroupPathOrAncestor reports whether a permission path applies to the given group path. +func matchesGroupPathOrAncestor(groupPath string, permissionPaths []string) bool { + for _, permissionPath := range permissionPaths { // Exact group match, for example "ops" matches "ops". if groupPath == permissionPath { return true @@ -597,7 +644,8 @@ func permissionMatches(groupPath string, permissions []string) bool { return false } -func accessLevelPermission(accessLevel int) (string, bool) { +// permissionFromAccessLevel maps GitLab numeric access levels to permission suffix strings. +func permissionFromAccessLevel(accessLevel int) (string, bool) { switch accessLevel { case accessLevelMinimalAccess: return "minimal_access", true @@ -621,70 +669,3 @@ func accessLevelPermission(accessLevel int) (string, bool) { return "", false } } - -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 - } - } - } - } - - return groups -} - -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 - } - - if len(c.groups) > 0 { - filteredGroups := groups.Filter(gitlabGroups, c.groups) - if len(filteredGroups) == 0 { - return nil, fmt.Errorf("gitlab: user %q is not in any of the required groups", userLogin) - } - return filteredGroups, nil - } else if groupScope { - return gitlabGroups, nil - } - - return nil, nil -} diff --git a/connector/gitlab/gitlab_test.go b/connector/gitlab/gitlab_test.go index 630b1c5f58..74007063e6 100644 --- a/connector/gitlab/gitlab_test.go +++ b/connector/gitlab/gitlab_test.go @@ -203,7 +203,7 @@ func TestUserGroups(t *testing.T) { defer s.Close() c := gitlabConnector{baseURL: s.URL} - groups, err := c.getGroups(context.Background(), newClient(), true, "joebloggs", 12345678) + groups, err := c.resolveIdentityGroups(context.Background(), newClient(), true, "joebloggs", 12345678) expectNil(t, err) expectEquals(t, groups, []string{ @@ -225,7 +225,7 @@ func TestUserGroupsWithInheritedGroups(t *testing.T) { defer s.Close() c := gitlabConnector{baseURL: s.URL, inheritedGroups: true} - groups, err := c.getGroups(context.Background(), newClient(), true, "joebloggs", 12345678) + groups, err := c.resolveIdentityGroups(context.Background(), newClient(), true, "joebloggs", 12345678) expectNil(t, err) expectEquals(t, groups, []string{ @@ -256,7 +256,7 @@ func TestUserGroupsWithInheritedGroupsPagination(t *testing.T) { defer s.Close() c := gitlabConnector{baseURL: s.URL, inheritedGroups: true} - groups, err := c.getGroups(context.Background(), newClient(), true, "joebloggs", 12345678) + groups, err := c.resolveIdentityGroups(context.Background(), newClient(), true, "joebloggs", 12345678) expectNil(t, err) expectEquals(t, groups, expectedGroups) @@ -271,7 +271,7 @@ func TestUserGroupsWithFiltering(t *testing.T) { defer s.Close() c := gitlabConnector{baseURL: s.URL, groups: []string{"team-1"}} - groups, err := c.getGroups(context.Background(), newClient(), true, "joebloggs", 12345678) + groups, err := c.resolveIdentityGroups(context.Background(), newClient(), true, "joebloggs", 12345678) expectNil(t, err) expectEquals(t, groups, []string{ @@ -296,7 +296,7 @@ func TestUserGroupsWithInheritedGroupsFiltering(t *testing.T) { groups: []string{"team-2/sub"}, inheritedGroups: true, } - groups, err := c.getGroups(context.Background(), newClient(), true, "joebloggs", 12345678) + groups, err := c.resolveIdentityGroups(context.Background(), newClient(), true, "joebloggs", 12345678) expectNil(t, err) expectEquals(t, groups, []string{ @@ -313,7 +313,7 @@ func TestUserGroupsWithoutOrgs(t *testing.T) { defer s.Close() c := gitlabConnector{baseURL: s.URL} - groups, err := c.getGroups(context.Background(), newClient(), true, "joebloggs", 12345678) + groups, err := c.resolveIdentityGroups(context.Background(), newClient(), true, "joebloggs", 12345678) expectNil(t, err) expectEquals(t, len(groups), 0)