Feature: Add GitHub groups (orgs/teams) support (#2196)

* Add GitHub groups (orgs/teams) support

* align code of getTeams with getOrgs to support Github Enterprise Server instances with different domain

* add documentation

* add missing import after rebase

* add nightly build and push (#2297)

* add nightly build and push

* add date based nightly build tags

* only keep single multiarch image build and push

* add changelog

* add images to internal docs static files

* add docu for nightly builds

* remove unnecessary spaces

* update nightly repository

* Issue 978: Fix Custom cookie name breaks redis for session (#1949)

* Issue 978: Fix Custom cookie name breaks redis for session (see https://github.com/oauth2-proxy/oauth2-proxy/issues/978)

* Issue 978: Fix Custom cookie name breaks redis for session (see https://github.com/oauth2-proxy/oauth2-proxy/issues/978)

* Update CHANGELOG.md

* Issue 978: Fix Custom cookie name breaks redis for session

* Issue 978: Fix Custom cookie name breaks redis for session

* Issue 978: Fix Custom cookie name breaks redis for session

* Issue 978: Fix Custom cookie name breaks redis for session

* Issue 978: Fix Custom cookie name breaks redis for session

* Issue 978: Fix Custom cookie name breaks redis for session

* Update CHANGELOG.md

---------

Co-authored-by: Nuno Borges <Nuno.Borges@ctw.bmwgroup.com>
Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk>

* Support http.AllowQuerySemicolons (#2248)

* Support http.AllowQuerySemicolons

* Docs

* Make it clear we are overriding the handler

* Update documentation for allow-query-semicolons

* Fix changelog format

* Fix formatting

---------

Co-authored-by: MickMake <github@mickmake.com>

* Add GitHub groups (orgs/teams) support

* align code of getTeams with getOrgs to support Github Enterprise Server instances with different domain

* add documentation

* fix changelog & documentation

* fix missing import

---------

Co-authored-by: Tobias Mayer <github@tobiasm.de>
Co-authored-by: Nuno Miguel Micaelo Borges <miguelborges99@gmail.com>
Co-authored-by: Nuno Borges <Nuno.Borges@ctw.bmwgroup.com>
Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk>
Co-authored-by: Tim White <tim.white@su.org.au>
Co-authored-by: MickMake <github@mickmake.com>

CHANGELOG.md

@@ -19,7 +19,8 @@
 - [#1949](https://github.com/oauth2-proxy/oauth2-proxy/pull/1949) Allow cookie names with dots in redis sessions (@miguelborges99)
 - [#2297](https://github.com/oauth2-proxy/oauth2-proxy/pull/2297) Add nightly build and push (@tuunit)
 - [#2299](https://github.com/oauth2-proxy/oauth2-proxy/pull/2299) bugfix: OIDCConfig based providers are not respecting flags and configs (@tuunit)
-- [#2248](https://github.com/oauth2-proxy/oauth2-proxy/pull/2248) Added support for semicolons in query strings.
+- [#2248](https://github.com/oauth2-proxy/oauth2-proxy/pull/2248) Added support for semicolons in query strings. (@timwsuqld)
+- [#2196](https://github.com/oauth2-proxy/oauth2-proxy/pull/2196) Add GitHub groups (orgs/teams) support. Including `X-Forwarded-Groups` header (@tuunit)
 
 # V7.5.1
 

docs/docs/configuration/providers/github.md

@@ -7,7 +7,7 @@ title: GitHub
 2.  Under `Authorization callback URL` enter the correct url ie `https://internal.yourcompany.com/oauth2/callback`
 
 The GitHub auth provider supports two additional ways to restrict authentication to either organization and optional 
-team level access, or to collaborators of a repository. Restricting by these options is normally accompanied with `--email-domain=*`
+team level access, or to collaborators of a repository. Restricting by these options is normally accompanied with `--email-domain=*`. Additionally, all the organizations and teams a user belongs to are set as part of the `X-Forwarded-Groups` header. e.g. `org1:team1,org1:team2,org2:team1`
 
 NOTE: When `--github-user` is set, the specified users are allowed to log in even if they do not belong to the specified 
 org and team or collaborators.

docs/versioned_docs/version-7.5.x/configuration/auth.md

@@ -147,7 +147,7 @@ Note: When using the ADFS Auth provider with nginx and the cookie session store
 1.  Create a new project: https://github.com/settings/developers
 2.  Under `Authorization callback URL` enter the correct url ie `https://internal.yourcompany.com/oauth2/callback`
 
-The GitHub auth provider supports two additional ways to restrict authentication to either organization and optional team level access, or to collaborators of a repository. Restricting by these options is normally accompanied with `--email-domain=*`
+The GitHub auth provider supports two additional ways to restrict authentication to either organization and optional team level access, or to collaborators of a repository. Restricting by these options is normally accompanied with `--email-domain=*`. Additionally, all the organizations and teams a user belongs to are set as part of the `X-Forwarded-Groups` header. e.g. `org1:team1,org1:team2,org2:team1`
 
 NOTE: When `--github-user` is set, the specified users are allowed to login even if they do not belong to the specified org and team or collaborators.
 

providers/github.go

@@ -15,6 +15,7 @@ import (
 	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions"
 	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"
 	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/requests"
+	"golang.org/x/exp/maps"
 )
 
 // GitHubProvider represents an GitHub based Identity Provider
@@ -31,7 +32,8 @@ var _ Provider = (*GitHubProvider)(nil)
 
 const (
 	githubProviderName = "GitHub"
-	githubDefaultScope = "user:email"
+	githubDefaultScope = "user:email read:org"
+	orgTeamSeparator   = ":"
 )
 
 var (
@@ -114,9 +116,6 @@ func (p *GitHubProvider) makeGitHubAPIEndpoint(endpoint string, params *url.Valu
 func (p *GitHubProvider) setOrgTeam(org, team string) {
 	p.Org = org
 	p.Team = team
-	if org != "" || team != "" {
-		p.Scope += " read:org"
-	}
 }
 
 // setRepo configures the target repository and optional token to use
@@ -132,10 +131,19 @@ func (p *GitHubProvider) setUsers(users []string) {
 
 // EnrichSession updates the User & Email after the initial Redeem
 func (p *GitHubProvider) EnrichSession(ctx context.Context, s *sessions.SessionState) error {
-	err := p.getEmail(ctx, s)
+	// Construct user info JSON from multiple GitHub API endpoints to have a more detailed session state
-	if err != nil {
+	if err := p.getOrgAndTeam(ctx, s); err != nil {
 		return err
 	}
+
+	if err := p.checkRestrictions(ctx, s); err != nil {
+		return err
+	}
+
+	if err := p.getEmail(ctx, s); err != nil {
+		return err
+	}
+
 	return p.getUser(ctx, s)
 }
 
@@ -144,170 +152,75 @@ func (p *GitHubProvider) ValidateSession(ctx context.Context, s *sessions.Sessio
 	return validateToken(ctx, p, s.AccessToken, makeGitHubHeader(s.AccessToken))
 }
 
-func (p *GitHubProvider) hasOrg(ctx context.Context, accessToken string) (bool, error) {
+func (p *GitHubProvider) hasOrg(ctx context.Context, s *sessions.SessionState) error {
 	// https://developer.github.com/v3/orgs/#list-your-organizations
+	var orgs []string
 
-	var orgs []struct {
+	for _, group := range s.Groups {
-		Login string `json:"login"`
+		if !strings.Contains(group, ":") {
-	}
+			orgs = append(orgs, group)
-
-	type orgsPage []struct {
-		Login string `json:"login"`
-	}
-
-	pn := 1
-	for {
-		params := url.Values{
-			"per_page": {"100"},
-			"page":     {strconv.Itoa(pn)},
 		}
-
-		endpoint := p.makeGitHubAPIEndpoint("/user/orgs", &params)
-
-		var op orgsPage
-		err := requests.New(endpoint.String()).
-			WithContext(ctx).
-			WithHeaders(makeGitHubHeader(accessToken)).
-			Do().
-			UnmarshalInto(&op)
-		if err != nil {
-			return false, err
-		}
-
-		if len(op) == 0 {
-			break
-		}
-
-		orgs = append(orgs, op...)
-		pn++
 	}
 
 	presentOrgs := make([]string, 0, len(orgs))
 	for _, org := range orgs {
-		if p.Org == org.Login {
+		if p.Org == org {
-			logger.Printf("Found Github Organization: %q", org.Login)
+			logger.Printf("Found Github Organization:%q", org)
-			return true, nil
+			return nil
 		}
-		presentOrgs = append(presentOrgs, org.Login)
+		presentOrgs = append(presentOrgs, org)
 	}
 
 	logger.Printf("Missing Organization:%q in %v", p.Org, presentOrgs)
-	return false, nil
+	return errors.New("user is missing required organization")
 }
 
-func (p *GitHubProvider) hasOrgAndTeam(ctx context.Context, accessToken string) (bool, error) {
+func (p *GitHubProvider) hasOrgAndTeam(ctx context.Context, s *sessions.SessionState) error {
-	// https://developer.github.com/v3/orgs/teams/#list-user-teams
+	type orgTeam struct {
-
+		Org  string `json:"org"`
-	var teams []struct {
+		Team string `json:"team"`
-		Name string `json:"name"`
-		Slug string `json:"slug"`
-		Org  struct {
-			Login string `json:"login"`
-		} `json:"organization"`
 	}
 
-	type teamsPage []struct {
+	var presentOrgTeams []orgTeam
-		Name string `json:"name"`
-		Slug string `json:"slug"`
-		Org  struct {
-			Login string `json:"login"`
-		} `json:"organization"`
-	}
 
-	pn := 1
+	for _, group := range s.Groups {
-	last := 0
+		if strings.Contains(group, orgTeamSeparator) {
-	for {
+			ot := strings.Split(group, orgTeamSeparator)
-		params := url.Values{
+			presentOrgTeams = append(presentOrgTeams, orgTeam{ot[0], ot[1]})
-			"per_page": {"100"},
-			"page":     {strconv.Itoa(pn)},
 		}
-
-		endpoint := p.makeGitHubAPIEndpoint("/user/teams", &params)
-
-		// bodyclose cannot detect that the body is being closed later in requests.Into,
-		// so have to skip the linting for the next line.
-		// nolint:bodyclose
-		result := requests.New(endpoint.String()).
-			WithContext(ctx).
-			WithHeaders(makeGitHubHeader(accessToken)).
-			Do()
-		if result.Error() != nil {
-			return false, result.Error()
-		}
-
-		if last == 0 {
-			// link header may not be obtained
-			// When paging is not required and all data can be retrieved with a single call
-
-			// Conditions for obtaining the link header.
-			// 1. When paging is required (Example: When the data size is 100 and the page size is 99 or less)
-			// 2. When it exceeds the paging frame (Example: When there is only 10 records but the second page is called with a page size of 100)
-
-			// link header at not last page
-			// <https://api.github.com/user/teams?page=1&per_page=100>; rel="prev", <https://api.github.com/user/teams?page=1&per_page=100>; rel="last", <https://api.github.com/user/teams?page=1&per_page=100>; rel="first"
-			// link header at last page (doesn't exist last info)
-			// <https://api.github.com/user/teams?page=3&per_page=10>; rel="prev", <https://api.github.com/user/teams?page=1&per_page=10>; rel="first"
-
-			link := result.Headers().Get("Link")
-			rep1 := regexp.MustCompile(`(?s).*\<https://api.github.com/user/teams\?page=(.)&per_page=[0-9]+\>; rel="last".*`)
-			i, converr := strconv.Atoi(rep1.ReplaceAllString(link, "$1"))
-
-			// If the last page cannot be taken from the link in the http header, the last variable remains zero
-			if converr == nil {
-				last = i
-			}
-		}
-
-		var tp teamsPage
-		if err := result.UnmarshalInto(&tp); err != nil {
-			return false, err
-		}
-		if len(tp) == 0 {
-			break
-		}
-
-		teams = append(teams, tp...)
-
-		if pn == last {
-			break
-		}
-		if last == 0 {
-			break
-		}
-
-		pn++
 	}
 
 	var hasOrg bool
+
 	presentOrgs := make(map[string]bool)
 	var presentTeams []string
-	for _, team := range teams {
+
-		presentOrgs[team.Org.Login] = true
+	for _, ot := range presentOrgTeams {
-		if p.Org == team.Org.Login {
+		presentOrgs[ot.Org] = true
+
+		if strings.EqualFold(p.Org, ot.Org) {
 			hasOrg = true
-			ts := strings.Split(p.Team, ",")
+			teams := strings.Split(p.Team, ",")
-			for _, t := range ts {
+			for _, team := range teams {
-				if t == team.Slug {
+				if strings.EqualFold(strings.TrimSpace(team), ot.Team) {
-					logger.Printf("Found Github Organization:%q Team:%q (Name:%q)", team.Org.Login, team.Slug, team.Name)
+					logger.Printf("Found Github Organization/Team:%q/%q", ot.Org, ot.Team)
-					return true, nil
+					return nil
 				}
 			}
-			presentTeams = append(presentTeams, team.Slug)
+			presentTeams = append(presentTeams, ot.Team)
 		}
 	}
+
 	if hasOrg {
 		logger.Printf("Missing Team:%q from Org:%q in teams: %v", p.Team, p.Org, presentTeams)
-	} else {
+		return errors.New("user is missing required team")
-		var allOrgs []string
-		for org := range presentOrgs {
-			allOrgs = append(allOrgs, org)
-		}
-		logger.Printf("Missing Organization:%q in %#v", p.Org, allOrgs)
 	}
-	return false, nil
+
+	logger.Printf("Missing Organization:%q in %#v", p.Org, maps.Keys(presentOrgs))
+	return errors.New("user is missing required organization")
 }
 
-func (p *GitHubProvider) hasRepo(ctx context.Context, accessToken string) (bool, error) {
+func (p *GitHubProvider) hasRepoAccess(ctx context.Context, accessToken string) error {
 	// https://developer.github.com/v3/repos/#get-a-repository
 
 	type permissions struct {
@@ -328,13 +241,18 @@ func (p *GitHubProvider) hasRepo(ctx context.Context, accessToken string) (bool,
 		WithHeaders(makeGitHubHeader(accessToken)).
 		Do().
 		UnmarshalInto(&repo)
+
 	if err != nil {
-		return false, err
+		return err
 	}
 
 	// Every user can implicitly pull from a public repo, so only grant access
 	// if they have push access or the repo is private and they can pull
-	return repo.Permissions.Push || (repo.Private && repo.Permissions.Pull), nil
+	if repo.Permissions.Push || (repo.Private && repo.Permissions.Pull) {
+		return nil
+	}
+
+	return errors.New("user doesn't have repository access")
 }
 
 func (p *GitHubProvider) hasUser(ctx context.Context, accessToken string) (bool, error) {
@@ -393,39 +311,8 @@ func (p *GitHubProvider) getEmail(ctx context.Context, s *sessions.SessionState)
 		Verified bool   `json:"verified"`
 	}
 
-	// If usernames are set, check that first
-	verifiedUser := false
-	if len(p.Users) > 0 {
-		var err error
-		verifiedUser, err = p.hasUser(ctx, s.AccessToken)
-		if err != nil {
-			return err
-		}
-		// org and repository options are not configured
-		if !verifiedUser && p.Org == "" && p.Repo == "" {
-			return errors.New("missing github user")
-		}
-	}
-	// If a user is verified by username options, skip the following restrictions
-	if !verifiedUser {
-		if p.Org != "" {
-			if p.Team != "" {
-				if ok, err := p.hasOrgAndTeam(ctx, s.AccessToken); err != nil || !ok {
-					return err
-				}
-			} else {
-				if ok, err := p.hasOrg(ctx, s.AccessToken); err != nil || !ok {
-					return err
-				}
-			}
-		} else if p.Repo != "" && p.Token == "" { // If we have a token we'll do the collaborator check in GetUserName
-			if ok, err := p.hasRepo(ctx, s.AccessToken); err != nil || !ok {
-				return err
-			}
-		}
-	}
-
 	endpoint := p.makeGitHubAPIEndpoint("/user/emails", nil)
+
 	err := requests.New(endpoint.String()).
 		WithContext(ctx).
 		WithHeaders(makeGitHubHeader(s.AccessToken)).
@@ -476,7 +363,6 @@ func (p *GitHubProvider) getUser(ctx context.Context, s *sessions.SessionState)
 	return nil
 }
 
-// isVerifiedUser
 func (p *GitHubProvider) isVerifiedUser(username string) bool {
 	for _, u := range p.Users {
 		if username == u {
@@ -485,3 +371,144 @@ func (p *GitHubProvider) isVerifiedUser(username string) bool {
 	}
 	return false
 }
+
+func (p *GitHubProvider) checkRestrictions(ctx context.Context, s *sessions.SessionState) error {
+	// If a user is verified by username options, skip the following restrictions
+	if ok, err := p.checkUserRestriction(ctx, s); err != nil || ok {
+		return err
+	}
+
+	if err := p.hasOrgAndTeamAccess(ctx, s); err != nil {
+		return err
+	}
+
+	if p.Org == "" && p.Repo != "" && p.Token == "" {
+		// If we have a token we'll do the collaborator check in GetUserName
+		return p.hasRepoAccess(ctx, s.AccessToken)
+	}
+
+	return nil
+}
+
+func (p *GitHubProvider) checkUserRestriction(ctx context.Context, s *sessions.SessionState) (bool, error) {
+	if len(p.Users) == 0 {
+		return false, nil
+	}
+
+	verifiedUser, err := p.hasUser(ctx, s.AccessToken)
+	if err != nil {
+		return verifiedUser, err
+	}
+
+	// org and repository options are not configured
+	if !verifiedUser && p.Org == "" && p.Repo == "" {
+		return false, errors.New("missing github user")
+	}
+
+	return verifiedUser, nil
+}
+
+func (p *GitHubProvider) hasOrgAndTeamAccess(ctx context.Context, s *sessions.SessionState) error {
+	if p.Org != "" && p.Team != "" {
+		return p.hasOrgAndTeam(ctx, s)
+	}
+
+	if p.Org != "" {
+		return p.hasOrg(ctx, s)
+	}
+
+	return nil
+}
+
+func (p *GitHubProvider) getOrgAndTeam(ctx context.Context, s *sessions.SessionState) error {
+	err := p.getOrgs(ctx, s)
+	if err != nil {
+		return err
+	}
+
+	return p.getTeams(ctx, s)
+}
+
+func (p *GitHubProvider) getOrgs(ctx context.Context, s *sessions.SessionState) error {
+	// https://docs.github.com/en/rest/orgs/orgs#list-organizations-for-the-authenticated-user
+
+	type Organization struct {
+		Login string `json:"login"`
+	}
+
+	pn := 1
+	for {
+		params := url.Values{
+			"per_page": {"100"},
+			"page":     {strconv.Itoa(pn)},
+		}
+
+		endpoint := p.makeGitHubAPIEndpoint("/user/orgs", &params)
+
+		var orgs []Organization
+		err := requests.New(endpoint.String()).
+			WithContext(ctx).
+			WithHeaders(makeGitHubHeader(s.AccessToken)).
+			Do().
+			UnmarshalInto(&orgs)
+		if err != nil {
+			return err
+		}
+
+		if len(orgs) == 0 {
+			break
+		}
+
+		for _, org := range orgs {
+			logger.Printf("Member of Github Organization:%q", org.Login)
+			s.Groups = append(s.Groups, org.Login)
+		}
+		pn++
+	}
+
+	return nil
+}
+
+func (p *GitHubProvider) getTeams(ctx context.Context, s *sessions.SessionState) error {
+	// https://docs.github.com/en/rest/teams/teams?#list-user-teams
+	type Team struct {
+		Name string `json:"name"`
+		Slug string `json:"slug"`
+		Org  struct {
+			Login string `json:"login"`
+		} `json:"organization"`
+	}
+
+	pn := 1
+	for {
+		params := url.Values{
+			"per_page": {"100"},
+			"page":     {strconv.Itoa(pn)},
+		}
+
+		endpoint := p.makeGitHubAPIEndpoint("/user/teams", &params)
+
+		var teams []Team
+		err := requests.New(endpoint.String()).
+			WithContext(ctx).
+			WithHeaders(makeGitHubHeader(s.AccessToken)).
+			Do().
+			UnmarshalInto(&teams)
+		if err != nil {
+			return err
+		}
+
+		if len(teams) == 0 {
+			break
+		}
+
+		for _, team := range teams {
+			logger.Printf("Member of Github Organization/Team:%q/%q", team.Org.Login, team.Slug)
+			s.Groups = append(s.Groups, team.Org.Login+orgTeamSeparator+team.Slug)
+		}
+
+		pn++
+	}
+
+	return nil
+}

providers/github_test.go

@@ -82,8 +82,8 @@ func TestNewGitHubProvider(t *testing.T) {
 	g.Expect(providerData.LoginURL.String()).To(Equal(githubDefaultLoginURL.String()))
 	g.Expect(providerData.RedeemURL.String()).To(Equal(githubDefaultRedeemURL.String()))
 	g.Expect(providerData.ProfileURL.String()).To(Equal(""))
-	g.Expect(providerData.ValidateURL.String()).To(Equal(githubDefaultValidateURL.String()))
+	g.Expect(providerData.ValidateURL.String()).To(Equal("https://api.github.com/"))
-	g.Expect(providerData.Scope).To(Equal("user:email"))
+	g.Expect(providerData.Scope).To(Equal("user:email read:org"))
 }
 
 func TestGitHubProviderOverrides(t *testing.T) {
@@ -231,7 +231,7 @@ func TestGitHubProvider_getEmailWithWriteAccessToPrivateRepo(t *testing.T) {
 	assert.Equal(t, "michael.bland@gsa.gov", session.Email)
 }
 
-func TestGitHubProvider_getEmailWithNoAccessToPrivateRepo(t *testing.T) {
+func TestGitHubProvider_checkRestrictionsWithNoAccessToPrivateRepo(t *testing.T) {
 	b := testGitHubBackend(map[string][]string{
 		"/repos/oauth2-proxy/oauth2-proxy": {`{}`},
 	})
@@ -245,8 +245,8 @@ func TestGitHubProvider_getEmailWithNoAccessToPrivateRepo(t *testing.T) {
 	)
 
 	session := CreateAuthorizedSession()
-	err := p.getEmail(context.Background(), session)
+	err := p.checkRestrictions(context.Background(), session)
-	assert.NoError(t, err)
+	assert.Error(t, err)
 	assert.Empty(t, session.Email)
 }
 
@@ -377,7 +377,7 @@ func TestGitHubProvider_getEmailWithUsername(t *testing.T) {
 	assert.Equal(t, "michael.bland@gsa.gov", session.Email)
 }
 
-func TestGitHubProvider_getEmailWithNotAllowedUsername(t *testing.T) {
+func TestGitHubProvider_checkRestrictionsWithNotAllowedUsername(t *testing.T) {
 	b := testGitHubBackend(map[string][]string{
 		"/user":        {`{"email": "michael.bland@gsa.gov", "login": "mbland"}`},
 		"/user/emails": {`[ {"email": "michael.bland@gsa.gov", "verified": true, "primary": true} ]`},
@@ -392,7 +392,7 @@ func TestGitHubProvider_getEmailWithNotAllowedUsername(t *testing.T) {
 	)
 
 	session := CreateAuthorizedSession()
-	err := p.getEmail(context.Background(), session)
+	err := p.checkRestrictions(context.Background(), session)
 	assert.Error(t, err)
 	assert.Empty(t, session.Email)
 }

providers/providers_test.go

@@ -153,13 +153,13 @@ func TestScope(t *testing.T) {
 			name:            "github: with no scope provided",
 			configuredType:  "github",
 			configuredScope: "",
-			expectedScope:   "user:email",
+			expectedScope:   "user:email read:org",
 		},
 		{
 			name:            "github: with a configured scope provided",
 			configuredType:  "github",
-			configuredScope: "user:email org:read",
+			configuredScope: "read:user read:org",
-			expectedScope:   "user:email org:read",
+			expectedScope:   "read:user read:org",
 		},
 	}