Skip to content

Commit

Permalink
Fix GitHub teams for large orgs
Browse files Browse the repository at this point in the history
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
  • Loading branch information
skwashd authored and ploxiln committed Nov 27, 2018
1 parent eebef9f commit 1cfae43
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 34 deletions.
80 changes: 47 additions & 33 deletions providers/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"net/http"
"net/url"
"path"
"regexp"
"strconv"
"strings"
)
Expand Down Expand Up @@ -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)},
}

Expand Down Expand Up @@ -134,7 +135,7 @@ func (p *GitHubProvider) hasOrgAndTeam(accessToken string) (bool, error) {
}

params := url.Values{
"limit": {"200"},
"limit": {"100"},
}

endpoint := &url.URL{
Expand All @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion providers/github_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down

0 comments on commit 1cfae43

Please sign in to comment.