Repository: dexidp/dex
Pull request #2941 (2023-05-11 12:19:15)

GitLab connector: add GitLab additional group with role 
#### Overview

Issue - https://github.com/dexidp/dex/issues/2763
In this PR I add logic to add information about group Permission for user to GitLab connector.

##### Before MR:
Login with Gitlab Connector
```
time="2023-05-11T12:01:02Z" level=info msg="login successful: connector "gitlab", username="Vladimir Zemtsov", preferred_username="zvlb", email="***", groups=["group1", "group1/group1.1", "group1/group1.2", "group2", "group3"]
```

##### After MR (if set getGroupsPermission to `true` in connector config):
```
time="2023-05-11T12:05:22Z" level=info msg="login successful: connector "gitlab", username="Vladimir Zemtsov", preferred_username="zvlb", email="***", groups=["group1", "group1/group1.1", "group1/group1.2", "group2", "group3", "group1:owner", "group1/group1.1:owner", "group1/group1.2:owner", "group2:maintainer"]
```
If zvlb user has Owner-access to group `group1` and Maintainer-access to group `group2`. (And have Reporter or Guest access to group3)

In GitLab, If you have access to some group, you have the same access to nested groups (oк Projects), but API request [/oauth/userinfo](https://docs.gitlab.com/ee/api/oauth2.html) returns information only about "main" groups. I add logic to add permission-postfix for nested groups. 


#### What this PR does / why we need it

In my case, I want to use dex for authentification and use group-permission information to control users-access to my application.


#### Does this PR introduce a user-facing change?

If this PR will be applied I add information [to docs](https://github.com/dexidp/website/blob/main/content/docs/connectors/gitlab.md) 

```release-note
GitLab connector: add GitLab additional group with a role
```


sagikazarmark: Hey @zvlb !

Thanks for the contribution!

I wonder if we should make this feature more generic. For example: allow users to format group names somehow:

- %name%
- %name%:%role%
- %name%;%role%
- %name_full_path%

We can make that an incremental change probably.
sagikazarmark: @nabokihms what do you think?
zvlb: > I wonder if we should make this feature more generic. For example: allow users to format group names somehow:
> 
> * %name%
> * %name%:%role%
> * %name%;%role%
> * %name_full_path%

I think is not a good idea. `%name%` - it's GitLab group name? But if GitLab we can have groups with the same name, but different paths. For example:
- /group1/test-group
- /group2/test-group
In this case, we have 2 groups with the name `test-group`, but it's different groups with different permission
zvlb: I need some time to create unit tests
zvlb: Someone?

progmatv: Hello guys! We really need this PR to approve =)
svbrsn: can you merge this PR? really helpful

zvlb: Anyone?
@nabokihms 
@sagikazarmark 
eduherraiz: @zvlb thank you for the work!
This looks very promising for me to manage argo roles based on gitlab's groups permissions.
Is there any way to merge this and do some other future efforts to be more generic for use in other connectors?
cc @nabokihms @sagikazarmark
zvlb: someone?

zvlb: ????


Patch:
diff --git a/connector/gitlab/gitlab.go b/connector/gitlab/gitlab.go
index fdb2c482..7aa44398 100644
--- a/connector/gitlab/gitlab.go
+++ b/connector/gitlab/gitlab.go
@@ -28,12 +28,13 @@ 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"`
+	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"`
 }
 
 type gitlabUser struct {
@@ -51,13 +52,14 @@ func (c *Config) Open(id string, logger *slog.Logger) (connector.Connector, erro
 		c.BaseURL = "https://gitlab.com"
 	}
 	return &gitlabConnector{
-		baseURL:      c.BaseURL,
-		redirectURI:  c.RedirectURI,
-		clientID:     c.ClientID,
-		clientSecret: c.ClientSecret,
-		logger:       logger.With(slog.Group("connector", "type", "gitlab", "id", id)),
-		groups:       c.Groups,
-		useLoginAsID: c.UseLoginAsID,
+		baseURL:             c.BaseURL,
+		redirectURI:         c.RedirectURI,
+		clientID:            c.ClientID,
+		clientSecret:        c.ClientSecret,
+		logger:              logger.With(slog.Group("connector", "type", "gitlab", "id", id)),
+		groups:              c.Groups,
+		useLoginAsID:        c.UseLoginAsID,
+		getGroupsPermission: c.GetGroupsPermission,
 	}, nil
 }
 
@@ -82,6 +84,9 @@ type gitlabConnector struct {
 	httpClient   *http.Client
 	// if set to true will use the user's handle rather than their numeric id as the ID
 	useLoginAsID bool
+
+	// if set to true permissions will be added to list of groups
+	getGroupsPermission bool
 }
 
 func (c *gitlabConnector) oauth2Config(scopes connector.Scopes) *oauth2.Config {
@@ -256,7 +261,10 @@ func (c *gitlabConnector) user(ctx context.Context, client *http.Client) (gitlab
 }
 
 type userInfo struct {
-	Groups []string
+	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"`
 }
 
 // userGroups queries the GitLab API for group membership.
@@ -287,9 +295,62 @@ func (c *gitlabConnector) userGroups(ctx context.Context, client *http.Client) (
 		return nil, fmt.Errorf("failed to decode response: %v", err)
 	}
 
+	if c.getGroupsPermission {
+		groups := c.setGroupsPermission(u)
+		return groups, nil
+	}
+
 	return u.Groups, 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
+				}
+			}
+		}
+	}
+
+	return groups
+}
+
 func (c *gitlabConnector) getGroups(ctx context.Context, client *http.Client, groupScope bool, userLogin string) ([]string, error) {
 	gitlabGroups, err := c.userGroups(ctx, client)
 	if err != nil {


Test patch:
diff --git a/connector/gitlab/gitlab_test.go b/connector/gitlab/gitlab_test.go
index d828b8bd..b67b30c0 100644
--- a/connector/gitlab/gitlab_test.go
+++ b/connector/gitlab/gitlab_test.go
@@ -249,6 +249,47 @@ func TestRefreshWithEmptyConnectorData(t *testing.T) {
 	expectEquals(t, emptyIdentity, identity)
 }
 
+func TestGroupsWithPermission(t *testing.T) {
+	s := newTestServer(map[string]interface{}{
+		"/api/v4/user": gitlabUser{Email: "some@email.com", ID: 12345678, Name: "Joe Bloggs", Username: "joebloggs"},
+		"/oauth/token": map[string]interface{}{
+			"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"},
+		},
+	})
+	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}
+	identity, err := c.HandleCallback(connector.Scopes{Groups: true}, req)
+	expectNil(t, err)
+
+	expectEquals(t, identity.Groups, []string{
+		"ops",
+		"dev",
+		"ops-test",
+		"ops/project",
+		"dev/project1",
+		"dev/project2",
+		"ops:owner",
+		"dev:developer",
+		"ops/project:owner",
+		"dev/project1:maintainer",
+		"dev/project2:developer",
+	})
+}
+
 func newTestServer(responses map[string]interface{}) *httptest.Server {
 	return httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		response := responses[r.RequestURI]