From 51d9b3d3ca654954b268d13ab65fa59e4014584a Mon Sep 17 00:00:00 2001 From: Alexander Matyushentsev Date: Wed, 14 Nov 2018 15:31:31 -0800 Subject: [PATCH] Issue #1184 - Github connector now returns a full group list when no org is specified --- connector/github/github.go | 95 +++++++++++++++++++++++-- connector/github/github_test.go | 121 ++++++++++++++++++++++++++++++++ 2 files changed, 209 insertions(+), 7 deletions(-) create mode 100644 connector/github/github_test.go diff --git a/connector/github/github.go b/connector/github/github.go index 831a8f13..edb821ea 100644 --- a/connector/github/github.go +++ b/connector/github/github.go @@ -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, diff --git a/connector/github/github_test.go b/connector/github/github_test.go new file mode 100644 index 00000000..57aef554 --- /dev/null +++ b/connector/github/github_test.go @@ -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) + } +}