CVE-2021-21411: Incorrect Authorization (CWE-863)
value: OAuth2-Proxy is an open source reverse proxy that provides authentication with Google, Github or other providers. The `--gitlab-group` flag for group-based authorization in the GitLab provider stopped working in the v7.0.0 release. Regardless of the flag settings, authorization wasn't restricted. Additionally, any authenticated users had whichever groups were set in `--gitlab-group` added to the new `X-Forwarded-Groups` header to the upstream application. While adding GitLab project based authorization support in #630, a bug was introduced where the user session's groups field was populated with the `--gitlab-group` config entries instead of pulling the individual user's group membership from the GitLab Userinfo endpoint. When the session groups where compared against the allowed groups for authorization, they matched improperly (since both lists were populated with the same data) so authorization was allowed. This impacts GitLab Provider users who relies on group membership for authorization restrictions. Any authenticated users in your GitLab environment can access your applications regardless of `--gitlab-group` membership restrictions. This is patched in v7.1.0. There is no workaround for the Group membership bug. But `--gitlab-project` can be set to use Project membership as the authorization checks instead of groups; it is not broken.
CWE description: The product performs an authorization check when an actor attempts to access a resource or perform an action, but it does not correctly perform the check. This allows attackers to bypass intended access restrictions.
CVSS v3 base score: 5.5
Published: 2021-03-26T21:15Z
Repository: https://github.com/oauth2-proxy/oauth2-proxy
Language: Go
Files changed: providers/gitlab.go, providers/gitlab_test.go
Commit date: 2021-03-25 17:20:45 +0000
Commit message: Merge pull request from GHSA-652x-m2gr-hppm

* Populate session Groups from userinfo response

* Fix: gitlab tests

Co-authored-by: Wilfried OLLIVIER <wollivier@bearstech.com>
Diff: diff --git a/providers/gitlab.go b/providers/gitlab.go
index eb7c2ffa..18f77fe7 100644
--- a/providers/gitlab.go
+++ b/providers/gitlab.go
@@ -293,25 +293,17 @@ func (p *GitLabProvider) EnrichSession(ctx context.Context, s *sessions.SessionS
 		return fmt.Errorf("user email is not verified")
 	}
 
 	s.User = userInfo.Username
 	s.Email = userInfo.Email
-
-	p.addGroupsToSession(ctx, s)
+	for _, group := range userInfo.Groups {
+		s.Groups = append(s.Groups, fmt.Sprintf("group:%s", group))
+	}
 
 	p.addProjectsToSession(ctx, s)
 
 	return nil
-
-}
-
-// addGroupsToSession projects into session.Groups
-func (p *GitLabProvider) addGroupsToSession(ctx context.Context, s *sessions.SessionState) {
-	// Iterate over projects, check if oauth2-proxy can get project information on behalf of the user
-	for _, group := range p.Groups {
-		s.Groups = append(s.Groups, fmt.Sprintf("group:%s", group))
-	}
 }
 
 // addProjectsToSession adds projects matching user access requirements into the session state groups list
 // This method prefix projects names with `project` to specify group kind
 func (p *GitLabProvider) addProjectsToSession(ctx context.Context, s *sessions.SessionState) {
@@ -339,26 +331,22 @@ func (p *GitLabProvider) addProjectsToSession(ctx context.Context, s *sessions.S
 			if perms != nil && perms.AccessLevel >= project.AccessLevel {
 				s.Groups = append(s.Groups, fmt.Sprintf("project:%s", project.Name))
 			} else {
 				logger.Errorf("Warning: user %q does not have the minimum required access level for project %q", s.Email, project.Name)
 			}
-		} else {
-			logger.Errorf("Warning: project %s is archived", project.Name)
+			continue
 		}
 
+		logger.Errorf("Warning: project %s is archived", project.Name)
 	}
-
 }
 
 // PrefixAllowedGroups returns a list of allowed groups, prefixed by their `kind` value
 func (p *GitLabProvider) PrefixAllowedGroups() (groups []string) {
-
 	for _, val := range p.Groups {
 		groups = append(groups, fmt.Sprintf("group:%s", val))
 	}
-
 	for _, val := range p.Projects {
 		groups = append(groups, fmt.Sprintf("project:%s", val.Name))
 	}
-
 	return groups
 }
diff --git a/providers/gitlab_test.go b/providers/gitlab_test.go
index 3a903d02..09f6d716 100644
--- a/providers/gitlab_test.go
+++ b/providers/gitlab_test.go
@@ -230,44 +230,44 @@ var _ = Describe("Gitlab Provider Tests", func() {
 
 				Expect(err).To(BeNil())
 				Expect(session.Groups).To(Equal(in.expectedValue))
 			},
 			Entry("project membership valid on group project", entitiesTableInput{
-				expectedValue: []string{"project:my_group/my_project"},
+				expectedValue: []string{"group:foo", "group:bar", "project:my_group/my_project"},
 				projects:      []string{"my_group/my_project"},
 			}),
 			Entry("project membership invalid on group project, insufficient access level level", entitiesTableInput{
-				expectedValue: nil,
+				expectedValue: []string{"group:foo", "group:bar"},
 				projects:      []string{"my_group/my_project=40"},
 			}),
 			Entry("project membership invalid on group project, no access at all", entitiesTableInput{
 				expectedValue: nil,
 				projects:      []string{"no_access_group/no_access_project=30"},
 			}),
 			Entry("project membership valid on personnal project", entitiesTableInput{
-				expectedValue: []string{"project:my_profile/my_personal_project"},
+				expectedValue: []string{"group:foo", "group:bar", "project:my_profile/my_personal_project"},
 				projects:      []string{"my_profile/my_personal_project"},
 			}),
 			Entry("project membership invalid on personnal project, insufficient access level", entitiesTableInput{
-				expectedValue: nil,
+				expectedValue: []string{"group:foo", "group:bar"},
 				projects:      []string{"my_profile/my_personal_project=40"},
 			}),
 			Entry("project membership invalid", entitiesTableInput{
-				expectedValue: nil,
+				expectedValue: []string{"group:foo", "group:bar"},
 				projects:      []string{"my_group/my_bad_project"},
 			}),
 			Entry("group membership valid", entitiesTableInput{
-				expectedValue: []string{"group:foo"},
+				expectedValue: []string{"group:foo", "group:bar"},
 				groups:        []string{"foo"},
 			}),
 			Entry("groups and projects", entitiesTableInput{
-				expectedValue: []string{"group:foo", "group:baz", "project:my_group/my_project", "project:my_profile/my_personal_project"},
+				expectedValue: []string{"group:foo", "group:bar", "project:my_group/my_project", "project:my_profile/my_personal_project"},
 				groups:        []string{"foo", "baz"},
 				projects:      []string{"my_group/my_project", "my_profile/my_personal_project"},
 			}),
 			Entry("archived projects", entitiesTableInput{
-				expectedValue: nil,
+				expectedValue: []string{"group:foo", "group:bar"},
 				groups:        []string{},
 				projects:      []string{"my_group/my_archived_project"},
 			}),
 		)
 

Vulnerable code: 
	p.addGroupsToSession(ctx, s)

}

// addGroupsToSession projects into session.Groups
func (p *GitLabProvider) addGroupsToSession(ctx context.Context, s *sessions.SessionState) {
	// Iterate over projects, check if oauth2-proxy can get project information on behalf of the user
	for _, group := range p.Groups {
		s.Groups = append(s.Groups, fmt.Sprintf("group:%s", group))
	}
		} else {
			logger.Errorf("Warning: project %s is archived", project.Name)




				expectedValue: []string{"project:my_group/my_project"},
				expectedValue: nil,
				expectedValue: []string{"project:my_profile/my_personal_project"},
				expectedValue: nil,
				expectedValue: nil,
				expectedValue: []string{"group:foo"},
				expectedValue: []string{"group:foo", "group:baz", "project:my_group/my_project", "project:my_profile/my_personal_project"},
				expectedValue: nil,
Fixed code: 	for _, group := range userInfo.Groups {
		s.Groups = append(s.Groups, fmt.Sprintf("group:%s", group))
	}
			continue
		logger.Errorf("Warning: project %s is archived", project.Name)
				expectedValue: []string{"group:foo", "group:bar", "project:my_group/my_project"},
				expectedValue: []string{"group:foo", "group:bar"},
				expectedValue: []string{"group:foo", "group:bar", "project:my_profile/my_personal_project"},
				expectedValue: []string{"group:foo", "group:bar"},
				expectedValue: []string{"group:foo", "group:bar"},
				expectedValue: []string{"group:foo", "group:bar"},
				expectedValue: []string{"group:foo", "group:bar", "project:my_group/my_project", "project:my_profile/my_personal_project"},
				expectedValue: []string{"group:foo", "group:bar"},