From 922f8761dc01c492be8afc8f4e3279b6d8e62af3 Mon Sep 17 00:00:00 2001 From: CorentinPtrl Date: Fri, 7 Nov 2025 23:19:33 +0100 Subject: [PATCH] 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 fe9c0ea9..3bc1af5a 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 b67b30c0..28377291 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", }) }