Merge pull request #1039 from estroz/move-group-scope-check
connector/github: fix groups scope check when 'orgs' is populated
This commit is contained in:
commit
763e174a7f
1 changed files with 43 additions and 48 deletions
|
@ -24,9 +24,12 @@ import (
|
||||||
)
|
)
|
||||||
|
|
||||||
const (
|
const (
|
||||||
apiURL = "https://api.github.com"
|
apiURL = "https://api.github.com"
|
||||||
|
// GitHub requires this scope to access '/user' and '/user/emails' API endpoints.
|
||||||
scopeEmail = "user:email"
|
scopeEmail = "user:email"
|
||||||
scopeOrgs = "read:org"
|
// GitHub requires this scope to access '/user/teams' and '/orgs' API endpoints
|
||||||
|
// which are used when a client includes the 'groups' scope.
|
||||||
|
scopeOrgs = "read:org"
|
||||||
)
|
)
|
||||||
|
|
||||||
// Pagination URL patterns
|
// Pagination URL patterns
|
||||||
|
@ -133,16 +136,22 @@ type githubConnector struct {
|
||||||
httpClient *http.Client
|
httpClient *http.Client
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// groupsRequired returns whether dex requires GitHub's 'read:org' scope. Dex
|
||||||
|
// needs 'read:org' if 'orgs' or 'org' fields are populated in a config file.
|
||||||
|
// Clients can require 'groups' scope without setting 'orgs'/'org'.
|
||||||
|
func (c *githubConnector) groupsRequired(groupScope bool) bool {
|
||||||
|
return len(c.orgs) > 0 || c.org != "" || groupScope
|
||||||
|
}
|
||||||
|
|
||||||
func (c *githubConnector) oauth2Config(scopes connector.Scopes) *oauth2.Config {
|
func (c *githubConnector) oauth2Config(scopes connector.Scopes) *oauth2.Config {
|
||||||
var githubScopes []string
|
// 'read:org' scope is required by the GitHub API, and thus for dex to ensure
|
||||||
if scopes.Groups {
|
// a user is a member of orgs and teams provided in configs.
|
||||||
githubScopes = []string{scopeEmail, scopeOrgs}
|
githubScopes := []string{scopeEmail}
|
||||||
} else {
|
if c.groupsRequired(scopes.Groups) {
|
||||||
githubScopes = []string{scopeEmail}
|
githubScopes = append(githubScopes, scopeOrgs)
|
||||||
}
|
}
|
||||||
|
|
||||||
endpoint := github.Endpoint
|
endpoint := github.Endpoint
|
||||||
|
|
||||||
// case when it is a GitHub Enterprise account.
|
// case when it is a GitHub Enterprise account.
|
||||||
if c.hostName != "" {
|
if c.hostName != "" {
|
||||||
endpoint = oauth2.Endpoint{
|
endpoint = oauth2.Endpoint{
|
||||||
|
@ -244,23 +253,11 @@ func (c *githubConnector) HandleCallback(s connector.Scopes, r *http.Request) (i
|
||||||
EmailVerified: true,
|
EmailVerified: true,
|
||||||
}
|
}
|
||||||
|
|
||||||
if s.Groups {
|
// Only set identity.Groups if 'orgs', 'org', or 'groups' scope are specified.
|
||||||
var groups []string
|
if c.groupsRequired(s.Groups) {
|
||||||
if len(c.orgs) > 0 {
|
groups, err := c.getGroups(ctx, client, s.Groups, user.Login)
|
||||||
if groups, err = c.listGroups(ctx, client, user.Login); err != nil {
|
if err != nil {
|
||||||
return identity, err
|
return identity, err
|
||||||
}
|
|
||||||
} else if c.org != "" {
|
|
||||||
inOrg, err := c.userInOrg(ctx, client, user.Login, c.org)
|
|
||||||
if err != nil {
|
|
||||||
return identity, err
|
|
||||||
}
|
|
||||||
if !inOrg {
|
|
||||||
return identity, fmt.Errorf("github: user %q not a member of org %q", user.Login, c.org)
|
|
||||||
}
|
|
||||||
if groups, err = c.teams(ctx, client, c.org); err != nil {
|
|
||||||
return identity, fmt.Errorf("github: get teams: %v", err)
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
identity.Groups = groups
|
identity.Groups = groups
|
||||||
}
|
}
|
||||||
|
@ -300,23 +297,11 @@ func (c *githubConnector) Refresh(ctx context.Context, s connector.Scopes, ident
|
||||||
identity.Username = username
|
identity.Username = username
|
||||||
identity.Email = user.Email
|
identity.Email = user.Email
|
||||||
|
|
||||||
if s.Groups {
|
// Only set identity.Groups if 'orgs', 'org', or 'groups' scope are specified.
|
||||||
var groups []string
|
if c.groupsRequired(s.Groups) {
|
||||||
if len(c.orgs) > 0 {
|
groups, err := c.getGroups(ctx, client, s.Groups, user.Login)
|
||||||
if groups, err = c.listGroups(ctx, client, user.Login); err != nil {
|
if err != nil {
|
||||||
return identity, err
|
return identity, err
|
||||||
}
|
|
||||||
} else if c.org != "" {
|
|
||||||
inOrg, err := c.userInOrg(ctx, client, user.Login, c.org)
|
|
||||||
if err != nil {
|
|
||||||
return identity, err
|
|
||||||
}
|
|
||||||
if !inOrg {
|
|
||||||
return identity, fmt.Errorf("github: user %q not a member of org %q", user.Login, c.org)
|
|
||||||
}
|
|
||||||
if groups, err = c.teams(ctx, client, c.org); err != nil {
|
|
||||||
return identity, fmt.Errorf("github: get teams: %v", err)
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
identity.Groups = groups
|
identity.Groups = groups
|
||||||
}
|
}
|
||||||
|
@ -324,13 +309,23 @@ func (c *githubConnector) Refresh(ctx context.Context, s connector.Scopes, ident
|
||||||
return identity, nil
|
return identity, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// listGroups enforces org and team constraints on user authorization
|
// getGroups retrieves GitHub orgs and teams a user is in, if any.
|
||||||
|
func (c *githubConnector) getGroups(ctx context.Context, client *http.Client, groupScope bool, userLogin string) ([]string, error) {
|
||||||
|
if len(c.orgs) > 0 {
|
||||||
|
return c.groupsForOrgs(ctx, client, userLogin)
|
||||||
|
} else if c.org != "" {
|
||||||
|
return c.teamsForOrg(ctx, client, c.org)
|
||||||
|
}
|
||||||
|
return nil, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// groupsForOrgs enforces org and team constraints on user authorization
|
||||||
// Cases in which user is authorized:
|
// Cases in which user is authorized:
|
||||||
// N orgs, no teams: user is member of at least 1 org
|
// N orgs, no teams: user is member of at least 1 org
|
||||||
// N orgs, M teams per org: user is member of any team from at least 1 org
|
// N orgs, M teams per org: user is member of any team from at least 1 org
|
||||||
// N-1 orgs, M teams per org, 1 org with no teams: user is member of any team
|
// N-1 orgs, M teams per org, 1 org with no teams: user is member of any team
|
||||||
// from at least 1 org, or member of org with no teams
|
// from at least 1 org, or member of org with no teams
|
||||||
func (c *githubConnector) listGroups(ctx context.Context, client *http.Client, userName string) (groups []string, err error) {
|
func (c *githubConnector) groupsForOrgs(ctx context.Context, client *http.Client, userName string) (groups []string, err error) {
|
||||||
var inOrgNoTeams bool
|
var inOrgNoTeams bool
|
||||||
for _, org := range c.orgs {
|
for _, org := range c.orgs {
|
||||||
inOrg, err := c.userInOrg(ctx, client, userName, org.Name)
|
inOrg, err := c.userInOrg(ctx, client, userName, org.Name)
|
||||||
|
@ -341,7 +336,7 @@ func (c *githubConnector) listGroups(ctx context.Context, client *http.Client, u
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
teams, err := c.teams(ctx, client, org.Name)
|
teams, err := c.teamsForOrg(ctx, client, org.Name)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return groups, err
|
return groups, err
|
||||||
}
|
}
|
||||||
|
@ -572,11 +567,11 @@ type team struct {
|
||||||
} `json:"organization"`
|
} `json:"organization"`
|
||||||
}
|
}
|
||||||
|
|
||||||
// teams queries the GitHub API for team membership within a specific organization.
|
// teamsForOrg queries the GitHub API for team membership within a specific organization.
|
||||||
//
|
//
|
||||||
// The HTTP passed client is expected to be constructed by the golang.org/x/oauth2 package,
|
// The HTTP passed client is expected to be constructed by the golang.org/x/oauth2 package,
|
||||||
// which inserts a bearer token as part of the request.
|
// which inserts a bearer token as part of the request.
|
||||||
func (c *githubConnector) teams(ctx context.Context, client *http.Client, orgName string) ([]string, error) {
|
func (c *githubConnector) teamsForOrg(ctx context.Context, client *http.Client, orgName string) ([]string, error) {
|
||||||
apiURL, groups := c.apiURL+"/user/teams", []string{}
|
apiURL, groups := c.apiURL+"/user/teams", []string{}
|
||||||
for {
|
for {
|
||||||
// https://developer.github.com/v3/orgs/teams/#list-user-teams
|
// https://developer.github.com/v3/orgs/teams/#list-user-teams
|
||||||
|
@ -585,7 +580,7 @@ func (c *githubConnector) teams(ctx context.Context, client *http.Client, orgNam
|
||||||
err error
|
err error
|
||||||
)
|
)
|
||||||
if apiURL, err = get(ctx, client, apiURL, &teams); err != nil {
|
if apiURL, err = get(ctx, client, apiURL, &teams); err != nil {
|
||||||
return nil, err
|
return nil, fmt.Errorf("github: get teams: %v", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
for _, team := range teams {
|
for _, team := range teams {
|
||||||
|
|
Reference in a new issue