k-space/dex
Archived
10
0
Fork 0
Code Issues Pull Requests Packages Projects Activity

connector/github: error if no groups scope without orgs

Browse Source 
We should always check if a user is in any orgs or teams specified
in config, and whether the groups scope is also included in client
requests. If not, return an error, because dex wouldn't have required
permissions to do the request anyway (need read:org).
This commit is contained in:
Eric Stroczynski 2017-08-17 10:13:00 -07:00
parent 20fd3163d9
commit e92f38f38f
1 changed files with 35 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

77
connector/github/github.go
View File

@ -134,15 +134,14 @@ type githubConnector struct {
} }
func (c *githubConnector) oauth2Config(scopes connector.Scopes) *oauth2.Config { func (c *githubConnector) oauth2Config(scopes connector.Scopes) *oauth2.Config {
var githubScopes []string // The GitHub API requires requires read:org scope to ensure users are members
if scopes.Groups { // of orgs and teams provided in configs.
githubScopes = []string{scopeEmail, scopeOrgs} githubScopes := []string{scopeEmail}
} else { if len(c.orgs) > 0 || c.org != "" {
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{
@ -207,6 +206,22 @@ func newHTTPClient(rootCA string) (*http.Client, error) {
}, nil }, nil
} }
// 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) (groups []string, err error) {
// Org and team membership should always be checked if specified by config.
if len(c.orgs) > 0 {
if groups, err = c.listGroups(ctx, client, userLogin); err != nil {
return groups, err
}
} else if groupScope && c.org != "" {
// Do not check org membership here to preserve legacy behavior.
if groups, err = c.teams(ctx, client, c.org); err != nil {
return groups, fmt.Errorf("github: get teams: %v", err)
}
}
return groups, nil
}
func (c *githubConnector) HandleCallback(s connector.Scopes, r *http.Request) (identity connector.Identity, err error) { func (c *githubConnector) HandleCallback(s connector.Scopes, r *http.Request) (identity connector.Identity, err error) {
q := r.URL.Query() q := r.URL.Query()
if errType := q.Get("error"); errType != "" { if errType := q.Get("error"); errType != "" {
@ -244,24 +259,13 @@ func (c *githubConnector) HandleCallback(s connector.Scopes, r *http.Request) (i
EmailVerified: true, EmailVerified: true,
} }
if s.Groups { groups, err := c.getGroups(ctx, client, s.Groups, user.Login)
var groups []string if err != nil {
if len(c.orgs) > 0 { return identity, err
if groups, err = c.listGroups(ctx, client, user.Login); err != nil { }
return identity, err // Preserve behavior of only setting identity.Groups if 'orgs' or groups scope
} // and 'org' are specified.
} else if c.org != "" { if len(c.orgs) > 0 || (s.Groups && 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,24 +304,13 @@ 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 { groups, err := c.getGroups(ctx, client, s.Groups, user.Login)
var groups []string if err != nil {
if len(c.orgs) > 0 { return identity, err
if groups, err = c.listGroups(ctx, client, user.Login); err != nil { }
return identity, err // Preserve behavior of only setting identity.Groups if 'orgs' or groups scope
} // and 'org' are specified.
} else if c.org != "" { if len(c.orgs) > 0 || (s.Groups && 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
} }