From 384258231c7d0a59e8b9de44add92f7bd0cb79e7 Mon Sep 17 00:00:00 2001 From: Dave Hall Date: Fri, 1 Jun 2018 21:42:24 +1000 Subject: [PATCH 1/2] Fix GitHub teams for large orgs Some large organisations have more than 100 teams. The current implementation only checks the first 100 teams returned by the API. This checks all teams the user is a member of. Added support for GitHub's new nested team feature which means inheritence works as designed. Again useful for large orgs. Updated the limit from 200 to the documented limit of 100: https://developer.github.com/v3/#pagination --- providers/github.go | 80 +++++++++++++++++++++++----------------- providers/github_test.go | 2 +- 2 files changed, 48 insertions(+), 34 deletions(-) diff --git a/providers/github.go b/providers/github.go index 26526ce76..22cea6664 100644 --- a/providers/github.go +++ b/providers/github.go @@ -8,6 +8,7 @@ import ( "net/http" "net/url" "path" + "regexp" "strconv" "strings" ) @@ -69,7 +70,7 @@ func (p *GitHubProvider) hasOrg(accessToken string) (bool, error) { pn := 1 for { params := url.Values{ - "limit": {"200"}, + "limit": {"100"}, "page": {strconv.Itoa(pn)}, } @@ -134,7 +135,7 @@ func (p *GitHubProvider) hasOrgAndTeam(accessToken string) (bool, error) { } params := url.Values{ - "limit": {"200"}, + "limit": {"100"}, } endpoint := &url.URL{ @@ -143,45 +144,58 @@ func (p *GitHubProvider) hasOrgAndTeam(accessToken string) (bool, error) { Path: path.Join(p.ValidateURL.Path, "/user/teams"), RawQuery: params.Encode(), } - req, _ := http.NewRequest("GET", endpoint.String(), nil) - req.Header.Set("Accept", "application/vnd.github.v3+json") - req.Header.Set("Authorization", fmt.Sprintf("token %s", accessToken)) - resp, err := http.DefaultClient.Do(req) - if err != nil { - return false, err - } - - body, err := ioutil.ReadAll(resp.Body) - resp.Body.Close() - if err != nil { - return false, err - } - if resp.StatusCode != 200 { - return false, fmt.Errorf( - "got %d from %q %s", resp.StatusCode, endpoint.String(), body) - } - - if err := json.Unmarshal(body, &teams); err != nil { - return false, fmt.Errorf("%s unmarshaling %s", err, body) - } + team_url := endpoint.String() + pattern := regexp.MustCompile(`<([^>]+)>; rel="next"`) var hasOrg bool presentOrgs := make(map[string]bool) var presentTeams []string - for _, team := range teams { - presentOrgs[team.Org.Login] = true - if p.Org == team.Org.Login { - hasOrg = true - ts := strings.Split(p.Team, ",") - for _, t := range ts { - if t == team.Slug { - log.Printf("Found Github Organization:%q Team:%q (Name:%q)", team.Org.Login, team.Slug, team.Name) - return true, nil + for { + req, _ := http.NewRequest("GET", team_url, nil) + req.Header.Set("Accept", "application/vnd.github.hellcat-preview+json") + req.Header.Set("Authorization", fmt.Sprintf("token %s", accessToken)) + resp, err := http.DefaultClient.Do(req) + if err != nil { + return false, err + } + + body, err := ioutil.ReadAll(resp.Body) + resp.Body.Close() + if err != nil { + return false, err + } + if resp.StatusCode != 200 { + return false, fmt.Errorf( + "got %d from %q %s", resp.StatusCode, endpoint.String(), body) + } + + if err := json.Unmarshal(body, &teams); err != nil { + return false, fmt.Errorf("%s unmarshaling %s", err, body) + } + + for _, team := range teams { + presentOrgs[team.Org.Login] = true + if p.Org == team.Org.Login { + hasOrg = true + ts := strings.Split(p.Team, ",") + for _, t := range ts { + if t == team.Slug { + log.Printf("Found Github Organization:%q Team:%q (Name:%q)", + team.Org.Login, team.Slug, team.Name) + return true, nil + } } + presentTeams = append(presentTeams, team.Slug) } - presentTeams = append(presentTeams, team.Slug) } + + matches := pattern.FindStringSubmatch(resp.Header["Link"][0]) + if len(matches) == 0 { + break + } + team_url = matches[1] } + if hasOrg { log.Printf("Missing Team:%q from Org:%q in teams: %v", p.Team, p.Org, presentTeams) } else { diff --git a/providers/github_test.go b/providers/github_test.go index 481018258..d1e4477fd 100644 --- a/providers/github_test.go +++ b/providers/github_test.go @@ -31,7 +31,7 @@ func testGitHubBackend(payload []string) *httptest.Server { pathToQueryMap := map[string][]string{ "/user": []string{""}, "/user/emails": []string{""}, - "/user/orgs": []string{"limit=200&page=1", "limit=200&page=2", "limit=200&page=3"}, + "/user/orgs": []string{"limit=100&page=1", "limit=100&page=2", "limit=100&page=3"}, } return httptest.NewServer(http.HandlerFunc( From 166c1840bcdedf831316ff31f97b3c81e4d5dffb Mon Sep 17 00:00:00 2001 From: Pierce Lopez Date: Sun, 25 Nov 2018 23:03:50 -0500 Subject: [PATCH 2/2] GitHub provider: limit to 10 pages of organizations/teams --- providers/github.go | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/providers/github.go b/providers/github.go index 22cea6664..4f3452216 100644 --- a/providers/github.go +++ b/providers/github.go @@ -58,22 +58,18 @@ func (p *GitHubProvider) SetOrgTeam(org, team string) { func (p *GitHubProvider) hasOrg(accessToken string) (bool, error) { // https://developer.github.com/v3/orgs/#list-your-organizations - var orgs []struct { Login string `json:"login"` } - type orgsPage []struct { Login string `json:"login"` } - pn := 1 - for { + for pn := 1; pn <= 10; pn++ { params := url.Values{ "limit": {"100"}, "page": {strconv.Itoa(pn)}, } - endpoint := &url.URL{ Scheme: p.ValidateURL.Scheme, Host: p.ValidateURL.Host, @@ -107,7 +103,6 @@ func (p *GitHubProvider) hasOrg(accessToken string) (bool, error) { } orgs = append(orgs, op...) - pn += 1 } var presentOrgs []string @@ -137,7 +132,6 @@ func (p *GitHubProvider) hasOrgAndTeam(accessToken string) (bool, error) { params := url.Values{ "limit": {"100"}, } - endpoint := &url.URL{ Scheme: p.ValidateURL.Scheme, Host: p.ValidateURL.Host, @@ -150,7 +144,8 @@ func (p *GitHubProvider) hasOrgAndTeam(accessToken string) (bool, error) { var hasOrg bool presentOrgs := make(map[string]bool) var presentTeams []string - for { + + for i := 0; i < 10; i++ { req, _ := http.NewRequest("GET", team_url, nil) req.Header.Set("Accept", "application/vnd.github.hellcat-preview+json") req.Header.Set("Authorization", fmt.Sprintf("token %s", accessToken))