git.iko.soy/typeable/oauth2-proxy
on master help sign in
providers: iterate across all pages from /user/orgs github endpoint.

For some GHE instances where a user can have more than 100 organizations, traversing the other pages is important otherwise oauth2_proxy will consider the user unauthorized. This change traverses the list returned by the API to avoid that. Update github provider tests to include this case.

commit882fcf0a0108674002fdea2ba89c3d11598f8c2f authorMark Maglana <mmaglana@gmail.com> dateTue 4 Jul 2017, 14:36 (9 years ago) parentfaff555
2 files changed+80−37
compare with master
unifiedsplit
providers/github.go+43−27 providers/github_test.go+37−10
providers/github.go+43−27
@@ -8,6 +8,7 @@ import (
88 "net/http"
99 "net/url"
1010 "path"
11+ "strconv"
1112 "strings"
1213)
1314
@@ -61,36 +62,51 @@ func (p *GitHubProvider) hasOrg(accessToken string) (bool, error) {
6162 Login string `json:"login"`
6263 }
6364
64− params := url.Values{
65− "limit": {"100"},
65+ type orgsPage []struct {
66+ Login string `json:"login"`
6667 }
6768
68− endpoint := &url.URL{
69− Scheme: p.ValidateURL.Scheme,
70− Host: p.ValidateURL.Host,
71− Path: path.Join(p.ValidateURL.Path, "/user/orgs"),
72− RawQuery: params.Encode(),
73− }
74− req, _ := http.NewRequest("GET", endpoint.String(), nil)
75− req.Header.Set("Accept", "application/vnd.github.v3+json")
76− req.Header.Set("Authorization", fmt.Sprintf("token %s", accessToken))
77− resp, err := http.DefaultClient.Do(req)
78− if err != nil {
79− return false, err
80− }
69+ pn := 1
70+ for {
71+ params := url.Values{
72+ "limit": {"200"},
73+ "page": {strconv.Itoa(pn)},
74+ }
8175
82− body, err := ioutil.ReadAll(resp.Body)
83− resp.Body.Close()
84− if err != nil {
85− return false, err
86− }
87− if resp.StatusCode != 200 {
88− return false, fmt.Errorf(
89− "got %d from %q %s", resp.StatusCode, endpoint.String(), body)
90− }
76+ endpoint := &url.URL{
77+ Scheme: p.ValidateURL.Scheme,
78+ Host: p.ValidateURL.Host,
79+ Path: path.Join(p.ValidateURL.Path, "/user/orgs"),
80+ RawQuery: params.Encode(),
81+ }
82+ req, _ := http.NewRequest("GET", endpoint.String(), nil)
83+ req.Header.Set("Accept", "application/vnd.github.v3+json")
84+ req.Header.Set("Authorization", fmt.Sprintf("token %s", accessToken))
85+ resp, err := http.DefaultClient.Do(req)
86+ if err != nil {
87+ return false, err
88+ }
9189
92− if err := json.Unmarshal(body, &orgs); err != nil {
93− return false, err
90+ body, err := ioutil.ReadAll(resp.Body)
91+ resp.Body.Close()
92+ if err != nil {
93+ return false, err
94+ }
95+ if resp.StatusCode != 200 {
96+ return false, fmt.Errorf(
97+ "got %d from %q %s", resp.StatusCode, endpoint.String(), body)
98+ }
99+
100+ var op orgsPage
101+ if err := json.Unmarshal(body, &op); err != nil {
102+ return false, err
103+ }
104+ if len(op) == 0 {
105+ break
106+ }
107+
108+ orgs = append(orgs, op...)
109+ pn += 1
94110 }
95111
96112 var presentOrgs []string
@@ -118,7 +134,7 @@ func (p *GitHubProvider) hasOrgAndTeam(accessToken string) (bool, error) {
118134 }
119135
120136 params := url.Values{
121− "limit": {"100"},
137+ "limit": {"200"},
122138 }
123139
124140 endpoint := &url.URL{
providers/github_test.go+37−10
@@ -27,23 +27,32 @@ func testGitHubProvider(hostname string) *GitHubProvider {
2727 return p
2828}
2929
30−func testGitHubBackend(payload string) *httptest.Server {
31− pathToQueryMap := map[string]string{
32− "/user": "",
33− "/user/emails": "",
30+func testGitHubBackend(payload []string) *httptest.Server {
31+ pathToQueryMap := map[string][]string{
32+ "/user": []string{""},
33+ "/user/emails": []string{""},
34+ "/user/orgs": []string{"limit=200&page=1", "limit=200&page=2", "limit=200&page=3"},
3435 }
3536
3637 return httptest.NewServer(http.HandlerFunc(
3738 func(w http.ResponseWriter, r *http.Request) {
3839 url := r.URL
3940 query, ok := pathToQueryMap[url.Path]
41+ validQuery := false
42+ index := 0
43+ for i, q := range query {
44+ if q == url.RawQuery {
45+ validQuery = true
46+ index = i
47+ }
48+ }
4049 if !ok {
4150 w.WriteHeader(404)
42− } else if url.RawQuery != query {
51+ } else if !validQuery {
4352 w.WriteHeader(404)
4453 } else {
4554 w.WriteHeader(200)
46− w.Write([]byte(payload))
55+ w.Write([]byte(payload[index]))
4756 }
4857 }))
4958}
@@ -89,11 +98,29 @@ func TestGitHubProviderOverrides(t *testing.T) {
8998}
9099
91100func TestGitHubProviderGetEmailAddress(t *testing.T) {
92− b := testGitHubBackend(`[ {"email": "michael.bland@gsa.gov", "primary": true} ]`)
101+ b := testGitHubBackend([]string{`[ {"email": "michael.bland@gsa.gov", "primary": true} ]`})
102+ defer b.Close()
103+
104+ bURL, _ := url.Parse(b.URL)
105+ p := testGitHubProvider(bURL.Host)
106+
107+ session := &SessionState{AccessToken: "imaginary_access_token"}
108+ email, err := p.GetEmailAddress(session)
109+ assert.Equal(t, nil, err)
110+ assert.Equal(t, "michael.bland@gsa.gov", email)
111+}
112+
113+func TestGitHubProviderGetEmailAddressWithOrg(t *testing.T) {
114+ b := testGitHubBackend([]string{
115+ `[ {"email": "michael.bland@gsa.gov", "primary": true, "login":"testorg"} ]`,
116+ `[ {"email": "michael.bland1@gsa.gov", "primary": true, "login":"testorg1"} ]`,
117+ `[ ]`,
118+ })
93119 defer b.Close()
94120
95121 bURL, _ := url.Parse(b.URL)
96122 p := testGitHubProvider(bURL.Host)
123+ p.Org = "testorg1"
97124
98125 session := &SessionState{AccessToken: "imaginary_access_token"}
99126 email, err := p.GetEmailAddress(session)
@@ -104,7 +131,7 @@ func TestGitHubProviderGetEmailAddress(t *testing.T) {
104131// Note that trying to trigger the "failed building request" case is not
105132// practical, since the only way it can fail is if the URL fails to parse.
106133func TestGitHubProviderGetEmailAddressFailedRequest(t *testing.T) {
107− b := testGitHubBackend("unused payload")
134+ b := testGitHubBackend([]string{"unused payload"})
108135 defer b.Close()
109136
110137 bURL, _ := url.Parse(b.URL)
@@ -120,7 +147,7 @@ func TestGitHubProviderGetEmailAddressFailedRequest(t *testing.T) {
120147}
121148
122149func TestGitHubProviderGetEmailAddressEmailNotPresentInPayload(t *testing.T) {
123− b := testGitHubBackend("{\"foo\": \"bar\"}")
150+ b := testGitHubBackend([]string{"{\"foo\": \"bar\"}"})
124151 defer b.Close()
125152
126153 bURL, _ := url.Parse(b.URL)
@@ -133,7 +160,7 @@ func TestGitHubProviderGetEmailAddressEmailNotPresentInPayload(t *testing.T) {
133160}
134161
135162func TestGitHubProviderGetUserName(t *testing.T) {
136− b := testGitHubBackend(`{"email": "michael.bland@gsa.gov", "login": "mbland"}`)
163+ b := testGitHubBackend([]string{`{"email": "michael.bland@gsa.gov", "login": "mbland"}`})
137164 defer b.Close()
138165
139166 bURL, _ := url.Parse(b.URL)