You've already forked oauth2-proxy
mirror of
https://github.com/oauth2-proxy/oauth2-proxy.git
synced 2025-07-13 01:40:48 +02:00
feat: support for multiple github orgs (#3072)
* fix for github teams * Update github.go * added errorhandling * Update github.md * refactored GitHub provider refactored hasOrg, hasOrgAndTeams and hasTeam into hasAccess to stay within function limit * reverted Refactoring * refactored github.go - joined hasOrgAndTeamAccess into checkRestrictions * refactored github.go - reduced number of returns of function checkRestrictions to 4 * updated GitHub provider to accept legacy team ids * GoFmt and golangci-lint Formatted with GoFmt and followed recommendations of GoLint * added Tests added Tests for checkRestrictions. * refactored in maintainer feedback * Removed code, documentation and tests for legacy ids * add changelog and update docs --------- Signed-off-by: Jan Larwig <jan@larwig.com> Co-authored-by: Jan Larwig <jan@larwig.com>
This commit is contained in:
Daniel Mersch
GitHub
@ -200,6 +200,7 @@ func (p *GitHubProvider) hasOrgAndTeam(s *sessions.SessionState) error {
|
||||
|
||||
if strings.EqualFold(p.Org, ot.Org) {
|
||||
hasOrg = true
|
||||
|
||||
teams := strings.Split(p.Team, ",")
|
||||
for _, team := range teams {
|
||||
if strings.EqualFold(strings.TrimSpace(team), ot.Team) {
|
||||
@ -220,6 +221,37 @@ func (p *GitHubProvider) hasOrgAndTeam(s *sessions.SessionState) error {
|
||||
return errors.New("user is missing required organization")
|
||||
}
|
||||
|
||||
func (p *GitHubProvider) hasTeam(s *sessions.SessionState) error {
|
||||
var teams []string
|
||||
|
||||
for _, group := range s.Groups {
|
||||
if strings.Contains(group, orgTeamSeparator) {
|
||||
teams = append(teams, strings.TrimSpace(group))
|
||||
}
|
||||
}
|
||||
|
||||
var presentTeams = make([]string, 0, len(teams))
|
||||
|
||||
for _, ot := range teams {
|
||||
allowedTeams := strings.Split(p.Team, ",")
|
||||
for _, team := range allowedTeams {
|
||||
if !strings.Contains(team, orgTeamSeparator) {
|
||||
logger.Printf("Please use fully qualified team names (org:team-slug) if you omit the organisation. Current Team name: %s", team)
|
||||
return errors.New("team name is invalid")
|
||||
}
|
||||
|
||||
if strings.EqualFold(strings.TrimSpace(team), ot) {
|
||||
logger.Printf("Found Github Organization/Team:%s", ot)
|
||||
return nil
|
||||
}
|
||||
}
|
||||
presentTeams = append(presentTeams, ot)
|
||||
}
|
||||
|
||||
logger.Printf("Missing Team:%q in teams: %v", p.Team, presentTeams)
|
||||
return errors.New("user is missing required team")
|
||||
}
|
||||
|
||||
func (p *GitHubProvider) hasRepoAccess(ctx context.Context, accessToken string) error {
|
||||
// https://developer.github.com/v3/repos/#get-a-repository
|
||||
|
||||
@ -378,12 +410,22 @@ func (p *GitHubProvider) checkRestrictions(ctx context.Context, s *sessions.Sess
|
||||
return err
|
||||
}
|
||||
|
||||
if err := p.hasOrgAndTeamAccess(s); err != nil {
|
||||
var err error
|
||||
switch {
|
||||
case p.Org != "" && p.Team != "":
|
||||
err = p.hasOrgAndTeam(s)
|
||||
case p.Org != "":
|
||||
err = p.hasOrg(s)
|
||||
case p.Team != "":
|
||||
err = p.hasTeam(s)
|
||||
}
|
||||
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
if p.Org == "" && p.Repo != "" && p.Token == "" {
|
||||
// If we have a token we'll do the collaborator check in GetUserName
|
||||
// If we have a token we'll do the collaborator check
|
||||
return p.hasRepoAccess(ctx, s.AccessToken)
|
||||
}
|
||||
|
||||
@ -408,18 +450,6 @@ func (p *GitHubProvider) checkUserRestriction(ctx context.Context, s *sessions.S
|
||||
return verifiedUser, nil
|
||||
}
|
||||
|
||||
func (p *GitHubProvider) hasOrgAndTeamAccess(s *sessions.SessionState) error {
|
||||
if p.Org != "" && p.Team != "" {
|
||||
return p.hasOrgAndTeam(s)
|
||||
}
|
||||
|
||||
if p.Org != "" {
|
||||
return p.hasOrg(s)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
func (p *GitHubProvider) getOrgAndTeam(ctx context.Context, s *sessions.SessionState) error {
|
||||
err := p.getOrgs(ctx, s)
|
||||
if err != nil {
|
||||
@ -503,8 +533,8 @@ func (p *GitHubProvider) getTeams(ctx context.Context, s *sessions.SessionState)
|
||||
}
|
||||
|
||||
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)
|
||||
logger.Printf("Member of Github Organization/Team: %q/%q", team.Org.Login, team.Slug)
|
||||
s.Groups = append(s.Groups, fmt.Sprintf("%s%s%s", team.Org.Login, orgTeamSeparator, team.Slug))
|
||||
}
|
||||
|
||||
pn++
|
||||
|
||||
@ -39,6 +39,7 @@ func testGitHubBackend(payloads map[string][]string) *httptest.Server {
|
||||
"/repos/oauth2-proxy/oauth2-proxy/collaborators/mbland": {""},
|
||||
"/user": {""},
|
||||
"/user/emails": {""},
|
||||
"/user/teams": {"page=1&per_page=100", "page=2&per_page=100", "page=3&per_page=100"},
|
||||
"/user/orgs": {"page=1&per_page=100", "page=2&per_page=100", "page=3&per_page=100"},
|
||||
// GitHub Enterprise Server API
|
||||
"/api/v3": {""},
|
||||
@ -168,6 +169,134 @@ func TestGitHubProvider_getEmailWithOrg(t *testing.T) {
|
||||
assert.Equal(t, "michael.bland@gsa.gov", session.Email)
|
||||
}
|
||||
|
||||
func TestGitHubProvider_checkRestrictionsOrg(t *testing.T) {
|
||||
b := testGitHubBackend(map[string][]string{
|
||||
"/user/emails": {`[ {"email": "john.doe@example", "verified": true, "primary": true} ]`},
|
||||
"/user/orgs": {
|
||||
`[ { "login": "test-org-1" } ]`,
|
||||
`[ { "login": "test-org-2" } ]`,
|
||||
`[ ]`,
|
||||
},
|
||||
"/user/teams": {
|
||||
`[ { "name":"test-team-1", "slug":"test-team-1", "organization": { "login": "test-org-1" } } ]`,
|
||||
`[ { "name":"test-team-2", "slug":"test-team-2", "organization": { "login": "test-org-2" } } ]`,
|
||||
`[ ]`,
|
||||
},
|
||||
})
|
||||
defer b.Close()
|
||||
|
||||
// This test should succeed, because of the valid organization.
|
||||
bURL, _ := url.Parse(b.URL)
|
||||
p := testGitHubProvider(bURL.Host,
|
||||
options.GitHubOptions{
|
||||
Org: "test-org-1",
|
||||
},
|
||||
)
|
||||
|
||||
var err error
|
||||
session := CreateAuthorizedSession()
|
||||
err = p.getOrgAndTeam(context.Background(), session)
|
||||
assert.NoError(t, err)
|
||||
err = p.checkRestrictions(context.Background(), session)
|
||||
assert.NoError(t, err)
|
||||
|
||||
// This part should fail, because user is not part of the organization
|
||||
p = testGitHubProvider(bURL.Host,
|
||||
options.GitHubOptions{
|
||||
Org: "test-org-1-fail",
|
||||
},
|
||||
)
|
||||
|
||||
err = p.checkRestrictions(context.Background(), session)
|
||||
assert.Error(t, err)
|
||||
}
|
||||
|
||||
func TestGitHubProvider_checkRestrictionsOrgTeam(t *testing.T) {
|
||||
b := testGitHubBackend(map[string][]string{
|
||||
"/user/emails": {`[ {"email": "john.doe@example", "verified": true, "primary": true} ]`},
|
||||
"/user/orgs": {
|
||||
`[ { "login": "test-org-1" } ]`,
|
||||
`[ { "login": "test-org-2" } ]`,
|
||||
`[ ]`,
|
||||
},
|
||||
"/user/teams": {
|
||||
`[ { "name":"test-team-1", "slug":"test-team-1", "organization": { "login": "test-org-1" } } ]`,
|
||||
`[ { "name":"test-team-2", "slug":"test-team-2", "organization": { "login": "test-org-2" } } ]`,
|
||||
`[ ]`,
|
||||
},
|
||||
})
|
||||
defer b.Close()
|
||||
|
||||
// This test should succeed, because of the valid Org and Team combination.
|
||||
bURL, _ := url.Parse(b.URL)
|
||||
p := testGitHubProvider(bURL.Host,
|
||||
options.GitHubOptions{
|
||||
Org: "test-org-1",
|
||||
Team: "test-team-1",
|
||||
},
|
||||
)
|
||||
|
||||
var err error
|
||||
session := CreateAuthorizedSession()
|
||||
err = p.getOrgAndTeam(context.Background(), session)
|
||||
assert.NoError(t, err)
|
||||
err = p.checkRestrictions(context.Background(), session)
|
||||
assert.NoError(t, err)
|
||||
|
||||
// This part should fail, because user is not part of the organization and the team
|
||||
p = testGitHubProvider(bURL.Host,
|
||||
options.GitHubOptions{
|
||||
Org: "test-org-1-fail",
|
||||
Team: "test-team-1-fail",
|
||||
},
|
||||
)
|
||||
|
||||
err = p.checkRestrictions(context.Background(), session)
|
||||
assert.Error(t, err)
|
||||
}
|
||||
|
||||
func TestGitHubProvider_checkRestrictionsTeam(t *testing.T) {
|
||||
b := testGitHubBackend(map[string][]string{
|
||||
"/user/emails": {`[ {"email": "john.doe@example", "verified": true, "primary": true} ]`},
|
||||
"/user/orgs": {
|
||||
`[ { "login": "test-org-1" } ]`,
|
||||
`[ { "login": "test-org-2" } ]`,
|
||||
`[ ]`,
|
||||
},
|
||||
"/user/teams": {
|
||||
`[ { "name":"test-team-1", "slug":"test-team-1", "organization": { "login": "test-org-1" } } ]`,
|
||||
`[ { "name":"test-team-2", "slug":"test-team-2", "organization": { "login": "test-org-2" } } ]`,
|
||||
`[ ]`,
|
||||
},
|
||||
})
|
||||
defer b.Close()
|
||||
|
||||
// This test should succeed, because of the valid org:slug combination.
|
||||
bURL, _ := url.Parse(b.URL)
|
||||
p := testGitHubProvider(bURL.Host,
|
||||
options.GitHubOptions{
|
||||
Team: "test-org-1:test-team-1, test-org-2:test-team-2-fail",
|
||||
},
|
||||
)
|
||||
|
||||
var err error
|
||||
session := CreateAuthorizedSession()
|
||||
err = p.getOrgAndTeam(context.Background(), session)
|
||||
assert.NoError(t, err)
|
||||
err = p.checkRestrictions(context.Background(), session)
|
||||
assert.NoError(t, err)
|
||||
|
||||
// This part should fail, because user is not part of the organization:team combination
|
||||
p = testGitHubProvider(bURL.Host,
|
||||
options.GitHubOptions{
|
||||
Team: "test-org-1-fail:test-team-1-fail, test-org-2-fail:test-team-2-fail",
|
||||
},
|
||||
)
|
||||
|
||||
err = p.checkRestrictions(context.Background(), session)
|
||||
assert.Error(t, err)
|
||||
}
|
||||
|
||||
func TestGitHubProvider_getEmailWithWriteAccessToPublicRepo(t *testing.T) {
|
||||
b := testGitHubBackend(map[string][]string{
|
||||
"/repo/oauth2-proxy/oauth2-proxy": {`{"permissions": {"pull": true, "push": true}, "private": false}`},
|
||||
|
||||
Reference in New Issue
Block a user