Issue #1184 - Github connector now returns a full group list when no org is specified
This commit is contained in:
Alexander Matyushentsev
parent
57b1031352
commit
51d9b3d3ca
@ -325,10 +325,17 @@ func (c *githubConnector) getGroups(ctx context.Context, client *http.Client, gr
|
||||
return c.groupsForOrgs(ctx, client, userLogin)
|
||||
} else if c.org != "" {
|
||||
return c.teamsForOrg(ctx, client, c.org)
|
||||
} else if groupScope {
|
||||
return c.userGroups(ctx, client)
|
||||
}
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
// formatTeamName return unique team name: prgs might have the same team names team name should be prefixed with org name to make team names unique across orgs.
|
||||
func formatTeamName(org string, team string) string {
|
||||
return fmt.Sprintf("%s:%s", org, team)
|
||||
}
|
||||
|
||||
// groupsForOrgs enforces org and team constraints on user authorization
|
||||
// Cases in which user is authorized:
|
||||
// N orgs, no teams: user is member of at least 1 org
|
||||
@ -359,11 +366,8 @@ func (c *githubConnector) groupsForOrgs(ctx context.Context, client *http.Client
|
||||
c.logger.Infof("github: user %q in org %q but no teams", userName, org.Name)
|
||||
}
|
||||
|
||||
// Orgs might have the same team names. We append orgPrefix to team name,
|
||||
// i.e. "org:team", to make team names unique across orgs.
|
||||
orgPrefix := org.Name + ":"
|
||||
for _, teamName := range teams {
|
||||
groups = append(groups, orgPrefix+teamName)
|
||||
groups = append(groups, formatTeamName(org.Name, teamName))
|
||||
}
|
||||
}
|
||||
if inOrgNoTeams || len(groups) > 0 {
|
||||
@ -372,6 +376,81 @@ func (c *githubConnector) groupsForOrgs(ctx context.Context, client *http.Client
|
||||
return groups, fmt.Errorf("github: user %q not in required orgs or teams", userName)
|
||||
}
|
||||
|
||||
func (c *githubConnector) userGroups(ctx context.Context, client *http.Client) (groups []string, err error) {
|
||||
orgs, err := c.userOrgs(ctx, client)
|
||||
if err != nil {
|
||||
return groups, err
|
||||
}
|
||||
|
||||
orgTeams, err := c.userOrgTeams(ctx, client)
|
||||
if err != nil {
|
||||
return groups, err
|
||||
}
|
||||
|
||||
for _, org := range orgs {
|
||||
if teams, ok := orgTeams[org]; !ok {
|
||||
groups = append(groups, org)
|
||||
} else {
|
||||
for _, team := range teams {
|
||||
groups = append(groups, formatTeamName(org, team))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return groups, err
|
||||
}
|
||||
|
||||
// userOrgs retrieves list of current user orgs
|
||||
func (c *githubConnector) userOrgs(ctx context.Context, client *http.Client) ([]string, error) {
|
||||
apiURL, groups := c.apiURL+"/user/orgs", []string{}
|
||||
for {
|
||||
// https://developer.github.com/v3/orgs/#list-your-organizations
|
||||
var (
|
||||
orgs []org
|
||||
err error
|
||||
)
|
||||
if apiURL, err = get(ctx, client, apiURL, &orgs); err != nil {
|
||||
return nil, fmt.Errorf("github: get orgs: %v", err)
|
||||
}
|
||||
|
||||
for _, org := range orgs {
|
||||
groups = append(groups, org.Login)
|
||||
}
|
||||
|
||||
if apiURL == "" {
|
||||
break
|
||||
}
|
||||
}
|
||||
|
||||
return groups, nil
|
||||
}
|
||||
|
||||
// userOrgTeams retrieves teams which current user belongs to.
|
||||
// Method returns a map where key is an org name and value list of teams under the org.
|
||||
func (c *githubConnector) userOrgTeams(ctx context.Context, client *http.Client) (map[string][]string, error) {
|
||||
apiURL, groups := c.apiURL+"/user/teams", map[string][]string{}
|
||||
for {
|
||||
// https://developer.github.com/v3/orgs/teams/#list-user-teams
|
||||
var (
|
||||
teams []team
|
||||
err error
|
||||
)
|
||||
if apiURL, err = get(ctx, client, apiURL, &teams); err != nil {
|
||||
return nil, fmt.Errorf("github: get teams: %v", err)
|
||||
}
|
||||
|
||||
for _, team := range teams {
|
||||
groups[team.Org.Login] = append(groups[team.Org.Login], team.Name)
|
||||
}
|
||||
|
||||
if apiURL == "" {
|
||||
break
|
||||
}
|
||||
}
|
||||
|
||||
return groups, nil
|
||||
}
|
||||
|
||||
// Filter the users' team memberships by 'teams' from config.
|
||||
func filterTeams(userTeams, configTeams []string) (teams []string) {
|
||||
teamFilter := make(map[string]struct{})
|
||||
@ -572,12 +651,14 @@ func (c *githubConnector) userInOrg(ctx context.Context, client *http.Client, us
|
||||
// https://developer.github.com/v3/orgs/teams/#response-12
|
||||
type team struct {
|
||||
Name string `json:"name"`
|
||||
Org struct {
|
||||
Login string `json:"login"`
|
||||
} `json:"organization"`
|
||||
Org org `json:"organization"`
|
||||
Slug string `json:"slug"`
|
||||
}
|
||||
|
||||
type org struct {
|
||||
Login string `json:"login"`
|
||||
}
|
||||
|
||||
// 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,
|
||||
|
||||
121
connector/github/github_test.go
Normal file
121
connector/github/github_test.go
Normal file
@ -0,0 +1,121 @@
|
||||
package github
|
||||
|
||||
import (
|
||||
"context"
|
||||
"crypto/tls"
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"net/url"
|
||||
"reflect"
|
||||
"testing"
|
||||
|
||||
"github.com/dexidp/dex/connector"
|
||||
)
|
||||
|
||||
func TestUserGroups(t *testing.T) {
|
||||
|
||||
orgs := []org{
|
||||
{Login: "org-1"},
|
||||
{Login: "org-2"},
|
||||
{Login: "org-3"},
|
||||
}
|
||||
|
||||
teams := []team{
|
||||
{Name: "team-1", Org: org{Login: "org-1"}},
|
||||
{Name: "team-2", Org: org{Login: "org-1"}},
|
||||
{Name: "team-3", Org: org{Login: "org-1"}},
|
||||
{Name: "team-4", Org: org{Login: "org-2"}},
|
||||
}
|
||||
|
||||
s := newTestServer(map[string]interface{}{
|
||||
"/user/orgs": orgs,
|
||||
"/user/teams": teams,
|
||||
})
|
||||
|
||||
connector := githubConnector{apiURL: s.URL}
|
||||
groups, err := connector.userGroups(context.Background(), newClient())
|
||||
|
||||
expectNil(t, err)
|
||||
expectEquals(t, groups, []string{
|
||||
"org-1:team-1",
|
||||
"org-1:team-2",
|
||||
"org-1:team-3",
|
||||
"org-2:team-4",
|
||||
"org-3",
|
||||
})
|
||||
|
||||
s.Close()
|
||||
}
|
||||
|
||||
func TestUserGroupsWithoutOrgs(t *testing.T) {
|
||||
|
||||
s := newTestServer(map[string]interface{}{
|
||||
"/user/orgs": []org{},
|
||||
"/user/teams": []team{},
|
||||
})
|
||||
|
||||
connector := githubConnector{apiURL: s.URL}
|
||||
groups, err := connector.userGroups(context.Background(), newClient())
|
||||
|
||||
expectNil(t, err)
|
||||
expectEquals(t, len(groups), 0)
|
||||
|
||||
s.Close()
|
||||
}
|
||||
|
||||
func TestUsernameIncludedInFederatedIdentity(t *testing.T) {
|
||||
|
||||
s := newTestServer(map[string]interface{}{
|
||||
"/user": user{Login: "some-login"},
|
||||
"/user/emails": []userEmail{{
|
||||
Email: "some@email.com",
|
||||
Verified: true,
|
||||
Primary: true,
|
||||
}},
|
||||
"/login/oauth/access_token": map[string]interface{}{
|
||||
"access_token": "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9",
|
||||
"expires_in": "30",
|
||||
},
|
||||
})
|
||||
|
||||
hostURL, err := url.Parse(s.URL)
|
||||
expectNil(t, err)
|
||||
|
||||
req, err := http.NewRequest("GET", hostURL.String(), nil)
|
||||
expectNil(t, err)
|
||||
|
||||
githubConnector := githubConnector{apiURL: s.URL, hostName: hostURL.Host, httpClient: newClient()}
|
||||
identity, err := githubConnector.HandleCallback(connector.Scopes{}, req)
|
||||
|
||||
expectNil(t, err)
|
||||
expectEquals(t, identity.Username, "some-login")
|
||||
|
||||
s.Close()
|
||||
}
|
||||
|
||||
func newTestServer(responses map[string]interface{}) *httptest.Server {
|
||||
return httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Add("Content-Type", "application/json")
|
||||
json.NewEncoder(w).Encode(responses[r.URL.Path])
|
||||
}))
|
||||
}
|
||||
|
||||
func newClient() *http.Client {
|
||||
tr := &http.Transport{
|
||||
TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
|
||||
}
|
||||
return &http.Client{Transport: tr}
|
||||
}
|
||||
|
||||
func expectNil(t *testing.T, a interface{}) {
|
||||
if a != nil {
|
||||
t.Errorf("Expected %+v to equal nil", a)
|
||||
}
|
||||
}
|
||||
|
||||
func expectEquals(t *testing.T, a interface{}, b interface{}) {
|
||||
if !reflect.DeepEqual(a, b) {
|
||||
t.Errorf("Expected %+v to equal %+v", a, b)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user