Change how gitlab-group is parsed on options (#639)

* Changed how gitlab-group is parsed, from string to []string

See #637

* Point out that gitlab-group can be a list

See #637

* Reflect to the []string change on pkg/apis/options/options.go

See #637

* Move cfg option gitlab_group to gitlab_groups

See #637

* Renamed Group to Groups

See #637

* Reflect the change on gitlab.go as well

See #637

* Added #639

* Added the author of #639 to the CHANGELOG

* Add the gitlab_groups env change to CHANGELOG.md

See #639

Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk>

Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk>

CHANGELOG.md

@@ -52,9 +52,15 @@
   - Fixes an inconsistency in the `--exclude-logging-paths` option by renaming it to `--exclude-logging-option`.
   - This flag may now be given multiple times as with other list options
   - This flag also accepts comma separated values
+- [#639](https://github.com/oauth2-proxy/oauth2-proxy/pull/639) Change how gitlab-group is parsed on options
+  - Previously, the flag gitlab-group used comma seperated values, while the config option used space seperated values.
+  - This fixes the config value to use slices internally.
+  - The config option `gitlab_group` is now `gitlab_groups`
+  - The environment variable `OAUTH2_PROXY_GITLAB_GROUP` is now `OAUTH2_PROXY_GITLAB_GROUPS`
 
 ## Changes since v5.1.1
 
+- [#639](https://github.com/oauth2-proxy/oauth2-proxy/pull/639) Change how gitlab-group is parsed on options (@linuxgemini)
 - [#615](https://github.com/oauth2-proxy/oauth2-proxy/pull/615) Kubernetes example based on Kind cluster and Nginx ingress (@EvgeniGordeev)
 - [#596](https://github.com/oauth2-proxy/oauth2-proxy/pull/596) Validate Bearer IDTokens in headers with correct provider/extra JWT Verifier (@NickMeves)
 - [#620](https://github.com/oauth2-proxy/oauth2-proxy/pull/620) Add HealthCheck middleware (@JoelSpeed)

docs/configuration/configuration.md

@@ -57,7 +57,7 @@ An example [oauth2-proxy.cfg]({{ site.gitweb }}/contrib/oauth2-proxy.cfg.example
 | `--github-repo` | string | restrict logins to collaborators of this repository formatted as `orgname/repo` | |
 | `--github-token` | string | the token to use when verifying repository collaborators (must have push access to the repository) | |
 | `--github-user` | string \| list | To allow users to login by username even if they do not belong to the specified org and team or collaborators | |
-| `--gitlab-group` | string | restrict logins to members of any of these groups (slug), separated by a comma | |
+| `--gitlab-group` | string \| list | restrict logins to members of any of these groups (slug), separated by a comma | |
 | `--google-admin-email` | string | the google admin to impersonate for api calls | |
 | `--google-group` | string | restrict logins to members of this google group (may be given multiple times). | |
 | `--google-service-account-json` | string | the path to the service account json credentials | |

pkg/apis/options/options.go

@@ -50,7 +50,7 @@ type Options struct {
 	GitHubRepo               string   `flag:"github-repo" cfg:"github_repo"`
 	GitHubToken              string   `flag:"github-token" cfg:"github_token"`
 	GitHubUsers              []string `flag:"github-user" cfg:"github_users"`
-	GitLabGroup              string   `flag:"gitlab-group" cfg:"gitlab_group"`
+	GitLabGroup              []string `flag:"gitlab-group" cfg:"gitlab_groups"`
 	GoogleGroups             []string `flag:"google-group" cfg:"google_group"`
 	GoogleAdminEmail         string   `flag:"google-admin-email" cfg:"google_admin_email"`
 	GoogleServiceAccountJSON string   `flag:"google-service-account-json" cfg:"google_service_account_json"`
@@ -231,7 +231,7 @@ func NewFlagSet() *pflag.FlagSet {
 	flagSet.String("github-repo", "", "restrict logins to collaborators of this repository")
 	flagSet.String("github-token", "", "the token to use when verifying repository collaborators (must have push access to the repository)")
 	flagSet.StringSlice("github-user", []string{}, "allow users with these usernames to login even if they do not belong to the specified org and team or collaborators (may be given multiple times)")
-	flagSet.String("gitlab-group", "", "restrict logins to members of this group")
+	flagSet.StringSlice("gitlab-group", []string{}, "restrict logins to members of this group (may be given multiple times)")
 	flagSet.StringSlice("google-group", []string{}, "restrict logins to members of this google group (may be given multiple times).")
 	flagSet.String("google-admin-email", "", "the google admin to impersonate for api calls")
 	flagSet.String("google-service-account-json", "", "the path to the service account json credentials")

pkg/validation/options.go

@@ -330,7 +330,7 @@ func parseProviderInfo(o *options.Options, msgs []string) []string {
 		}
 	case *providers.GitLabProvider:
 		p.AllowUnverifiedEmail = o.InsecureOIDCAllowUnverifiedEmail
-		p.Group = o.GitLabGroup
+		p.Groups = o.GitLabGroup
 		p.EmailDomains = o.EmailDomains
 
 		if o.GetOIDCVerifier() != nil {

providers/gitlab.go

@@ -18,7 +18,7 @@ import (
 type GitLabProvider struct {
 	*ProviderData
 
-	Group        string
+	Groups       []string
 	EmailDomains []string
 
 	Verifier             *oidc.IDTokenVerifier
@@ -162,7 +162,7 @@ func (p *GitLabProvider) getUserInfo(ctx context.Context, s *sessions.SessionSta
 }
 
 func (p *GitLabProvider) verifyGroupMembership(userInfo *gitlabUserInfo) error {
-	if p.Group == "" {
+	if len(p.Groups) == 0 {
 		return nil
 	}
 
@@ -173,14 +173,13 @@ func (p *GitLabProvider) verifyGroupMembership(userInfo *gitlabUserInfo) error {
 	}
 
 	// Find a valid group that they are a member of
-	validGroups := strings.Split(p.Group, " ")
+	for _, validGroup := range p.Groups {
-	for _, validGroup := range validGroups {
 		if _, ok := membershipSet[validGroup]; ok {
 			return nil
 		}
 	}
 
-	return fmt.Errorf("user is not a member of '%s'", p.Group)
+	return fmt.Errorf("user is not a member of '%s'", p.Groups)
 }
 
 func (p *GitLabProvider) verifyEmailDomain(userInfo *gitlabUserInfo) error {

providers/gitlab_test.go

@@ -115,7 +115,7 @@ func TestGitLabProviderGroupMembershipValid(t *testing.T) {
 	bURL, _ := url.Parse(b.URL)
 	p := testGitLabProvider(bURL.Host)
 	p.AllowUnverifiedEmail = true
-	p.Group = "foo"
+	p.Groups = []string{"foo"}
 
 	session := &sessions.SessionState{AccessToken: "gitlab_access_token"}
 	email, err := p.GetEmailAddress(context.Background(), session)
@@ -130,7 +130,7 @@ func TestGitLabProviderGroupMembershipMissing(t *testing.T) {
 	bURL, _ := url.Parse(b.URL)
 	p := testGitLabProvider(bURL.Host)
 	p.AllowUnverifiedEmail = true
-	p.Group = "baz"
+	p.Groups = []string{"baz"}
 
 	session := &sessions.SessionState{AccessToken: "gitlab_access_token"}
 	_, err := p.GetEmailAddress(context.Background(), session)