diff --git a/CHANGELOG.md b/CHANGELOG.md index 06f5732e..6d64545b 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/docs/docs/configuration/providers/github.md b/docs/docs/configuration/providers/github.md index c16e9605..6de91d82 100644 --- a/docs/docs/configuration/providers/github.md +++ b/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. diff --git a/docs/versioned_docs/version-7.5.x/configuration/auth.md b/docs/versioned_docs/version-7.5.x/configuration/auth.md index cdf4e501..10803b12 100644 --- a/docs/versioned_docs/version-7.5.x/configuration/auth.md +++ b/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. diff --git a/providers/github.go b/providers/github.go index 6c6fc3db..27adfd75 100644 --- a/providers/github.go +++ b/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) - if err != nil { + // Construct user info JSON from multiple GitHub API endpoints to have a more detailed session state + 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 { - Login string `json:"login"` - } - - type orgsPage []struct { - Login string `json:"login"` - } - - pn := 1 - for { - params := url.Values{ - "per_page": {"100"}, - "page": {strconv.Itoa(pn)}, + for _, group := range s.Groups { + if !strings.Contains(group, ":") { + orgs = append(orgs, group) } - - endpoint := p.makeGitHubAPIEndpoint("/user/orgs", ¶ms) - - 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 { - logger.Printf("Found Github Organization: %q", org.Login) - return true, nil + if p.Org == org { + logger.Printf("Found Github Organization:%q", org) + 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) { - // https://developer.github.com/v3/orgs/teams/#list-user-teams - - var teams []struct { - Name string `json:"name"` - Slug string `json:"slug"` - Org struct { - Login string `json:"login"` - } `json:"organization"` +func (p *GitHubProvider) hasOrgAndTeam(ctx context.Context, s *sessions.SessionState) error { + type orgTeam struct { + Org string `json:"org"` + Team string `json:"team"` } - type teamsPage []struct { - Name string `json:"name"` - Slug string `json:"slug"` - Org struct { - Login string `json:"login"` - } `json:"organization"` - } + var presentOrgTeams []orgTeam - pn := 1 - last := 0 - for { - params := url.Values{ - "per_page": {"100"}, - "page": {strconv.Itoa(pn)}, + for _, group := range s.Groups { + if strings.Contains(group, orgTeamSeparator) { + ot := strings.Split(group, orgTeamSeparator) + presentOrgTeams = append(presentOrgTeams, orgTeam{ot[0], ot[1]}) } - - endpoint := p.makeGitHubAPIEndpoint("/user/teams", ¶ms) - - // 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 - // ; rel="prev", ; rel="last", ; rel="first" - // link header at last page (doesn't exist last info) - // ; rel="prev", ; rel="first" - - link := result.Headers().Get("Link") - rep1 := regexp.MustCompile(`(?s).*\; 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 - if p.Org == team.Org.Login { + + for _, ot := range presentOrgTeams { + presentOrgs[ot.Org] = true + + if strings.EqualFold(p.Org, ot.Org) { hasOrg = true - ts := strings.Split(p.Team, ",") - for _, t := range ts { - if t == team.Slug { - logger.Printf("Found Github Organization:%q Team:%q (Name:%q)", team.Org.Login, team.Slug, team.Name) - return true, nil + teams := strings.Split(p.Team, ",") + for _, team := range teams { + if strings.EqualFold(strings.TrimSpace(team), ot.Team) { + logger.Printf("Found Github Organization/Team:%q/%q", ot.Org, ot.Team) + 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 { - var allOrgs []string - for org := range presentOrgs { - allOrgs = append(allOrgs, org) - } - logger.Printf("Missing Organization:%q in %#v", p.Org, allOrgs) + return errors.New("user is missing required team") } - 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", ¶ms) + + 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", ¶ms) + + 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 +} diff --git a/providers/github_test.go b/providers/github_test.go index deb35aee..8dfb943f 100644 --- a/providers/github_test.go +++ b/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.Scope).To(Equal("user:email")) + g.Expect(providerData.ValidateURL.String()).To(Equal("https://api.github.com/")) + 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) - assert.NoError(t, err) + err := p.checkRestrictions(context.Background(), session) + 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) } diff --git a/providers/providers_test.go b/providers/providers_test.go index 5476cce7..5c5df8a8 100644 --- a/providers/providers_test.go +++ b/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", - expectedScope: "user:email org:read", + configuredScope: "read:user read:org", + expectedScope: "read:user read:org", }, }