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.
This commit is contained in:
parent
faff555c55
commit
882fcf0a01
@ -8,6 +8,7 @@ import (
|
|||||||
"net/http"
|
"net/http"
|
||||||
"net/url"
|
"net/url"
|
||||||
"path"
|
"path"
|
||||||
|
"strconv"
|
||||||
"strings"
|
"strings"
|
||||||
)
|
)
|
||||||
|
|
||||||
@ -61,8 +62,15 @@ func (p *GitHubProvider) hasOrg(accessToken string) (bool, error) {
|
|||||||
Login string `json:"login"`
|
Login string `json:"login"`
|
||||||
}
|
}
|
||||||
|
|
||||||
|
type orgsPage []struct {
|
||||||
|
Login string `json:"login"`
|
||||||
|
}
|
||||||
|
|
||||||
|
pn := 1
|
||||||
|
for {
|
||||||
params := url.Values{
|
params := url.Values{
|
||||||
"limit": {"100"},
|
"limit": {"200"},
|
||||||
|
"page": {strconv.Itoa(pn)},
|
||||||
}
|
}
|
||||||
|
|
||||||
endpoint := &url.URL{
|
endpoint := &url.URL{
|
||||||
@ -89,9 +97,17 @@ func (p *GitHubProvider) hasOrg(accessToken string) (bool, error) {
|
|||||||
"got %d from %q %s", resp.StatusCode, endpoint.String(), body)
|
"got %d from %q %s", resp.StatusCode, endpoint.String(), body)
|
||||||
}
|
}
|
||||||
|
|
||||||
if err := json.Unmarshal(body, &orgs); err != nil {
|
var op orgsPage
|
||||||
|
if err := json.Unmarshal(body, &op); err != nil {
|
||||||
return false, err
|
return false, err
|
||||||
}
|
}
|
||||||
|
if len(op) == 0 {
|
||||||
|
break
|
||||||
|
}
|
||||||
|
|
||||||
|
orgs = append(orgs, op...)
|
||||||
|
pn += 1
|
||||||
|
}
|
||||||
|
|
||||||
var presentOrgs []string
|
var presentOrgs []string
|
||||||
for _, org := range orgs {
|
for _, org := range orgs {
|
||||||
@ -118,7 +134,7 @@ func (p *GitHubProvider) hasOrgAndTeam(accessToken string) (bool, error) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
params := url.Values{
|
params := url.Values{
|
||||||
"limit": {"100"},
|
"limit": {"200"},
|
||||||
}
|
}
|
||||||
|
|
||||||
endpoint := &url.URL{
|
endpoint := &url.URL{
|
||||||
|
@ -27,23 +27,32 @@ func testGitHubProvider(hostname string) *GitHubProvider {
|
|||||||
return p
|
return p
|
||||||
}
|
}
|
||||||
|
|
||||||
func testGitHubBackend(payload string) *httptest.Server {
|
func testGitHubBackend(payload []string) *httptest.Server {
|
||||||
pathToQueryMap := map[string]string{
|
pathToQueryMap := map[string][]string{
|
||||||
"/user": "",
|
"/user": []string{""},
|
||||||
"/user/emails": "",
|
"/user/emails": []string{""},
|
||||||
|
"/user/orgs": []string{"limit=200&page=1", "limit=200&page=2", "limit=200&page=3"},
|
||||||
}
|
}
|
||||||
|
|
||||||
return httptest.NewServer(http.HandlerFunc(
|
return httptest.NewServer(http.HandlerFunc(
|
||||||
func(w http.ResponseWriter, r *http.Request) {
|
func(w http.ResponseWriter, r *http.Request) {
|
||||||
url := r.URL
|
url := r.URL
|
||||||
query, ok := pathToQueryMap[url.Path]
|
query, ok := pathToQueryMap[url.Path]
|
||||||
|
validQuery := false
|
||||||
|
index := 0
|
||||||
|
for i, q := range query {
|
||||||
|
if q == url.RawQuery {
|
||||||
|
validQuery = true
|
||||||
|
index = i
|
||||||
|
}
|
||||||
|
}
|
||||||
if !ok {
|
if !ok {
|
||||||
w.WriteHeader(404)
|
w.WriteHeader(404)
|
||||||
} else if url.RawQuery != query {
|
} else if !validQuery {
|
||||||
w.WriteHeader(404)
|
w.WriteHeader(404)
|
||||||
} else {
|
} else {
|
||||||
w.WriteHeader(200)
|
w.WriteHeader(200)
|
||||||
w.Write([]byte(payload))
|
w.Write([]byte(payload[index]))
|
||||||
}
|
}
|
||||||
}))
|
}))
|
||||||
}
|
}
|
||||||
@ -89,7 +98,7 @@ func TestGitHubProviderOverrides(t *testing.T) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func TestGitHubProviderGetEmailAddress(t *testing.T) {
|
func TestGitHubProviderGetEmailAddress(t *testing.T) {
|
||||||
b := testGitHubBackend(`[ {"email": "michael.bland@gsa.gov", "primary": true} ]`)
|
b := testGitHubBackend([]string{`[ {"email": "michael.bland@gsa.gov", "primary": true} ]`})
|
||||||
defer b.Close()
|
defer b.Close()
|
||||||
|
|
||||||
bURL, _ := url.Parse(b.URL)
|
bURL, _ := url.Parse(b.URL)
|
||||||
@ -101,10 +110,28 @@ func TestGitHubProviderGetEmailAddress(t *testing.T) {
|
|||||||
assert.Equal(t, "michael.bland@gsa.gov", email)
|
assert.Equal(t, "michael.bland@gsa.gov", email)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestGitHubProviderGetEmailAddressWithOrg(t *testing.T) {
|
||||||
|
b := testGitHubBackend([]string{
|
||||||
|
`[ {"email": "michael.bland@gsa.gov", "primary": true, "login":"testorg"} ]`,
|
||||||
|
`[ {"email": "michael.bland1@gsa.gov", "primary": true, "login":"testorg1"} ]`,
|
||||||
|
`[ ]`,
|
||||||
|
})
|
||||||
|
defer b.Close()
|
||||||
|
|
||||||
|
bURL, _ := url.Parse(b.URL)
|
||||||
|
p := testGitHubProvider(bURL.Host)
|
||||||
|
p.Org = "testorg1"
|
||||||
|
|
||||||
|
session := &SessionState{AccessToken: "imaginary_access_token"}
|
||||||
|
email, err := p.GetEmailAddress(session)
|
||||||
|
assert.Equal(t, nil, err)
|
||||||
|
assert.Equal(t, "michael.bland@gsa.gov", email)
|
||||||
|
}
|
||||||
|
|
||||||
// Note that trying to trigger the "failed building request" case is not
|
// Note that trying to trigger the "failed building request" case is not
|
||||||
// practical, since the only way it can fail is if the URL fails to parse.
|
// practical, since the only way it can fail is if the URL fails to parse.
|
||||||
func TestGitHubProviderGetEmailAddressFailedRequest(t *testing.T) {
|
func TestGitHubProviderGetEmailAddressFailedRequest(t *testing.T) {
|
||||||
b := testGitHubBackend("unused payload")
|
b := testGitHubBackend([]string{"unused payload"})
|
||||||
defer b.Close()
|
defer b.Close()
|
||||||
|
|
||||||
bURL, _ := url.Parse(b.URL)
|
bURL, _ := url.Parse(b.URL)
|
||||||
@ -120,7 +147,7 @@ func TestGitHubProviderGetEmailAddressFailedRequest(t *testing.T) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func TestGitHubProviderGetEmailAddressEmailNotPresentInPayload(t *testing.T) {
|
func TestGitHubProviderGetEmailAddressEmailNotPresentInPayload(t *testing.T) {
|
||||||
b := testGitHubBackend("{\"foo\": \"bar\"}")
|
b := testGitHubBackend([]string{"{\"foo\": \"bar\"}"})
|
||||||
defer b.Close()
|
defer b.Close()
|
||||||
|
|
||||||
bURL, _ := url.Parse(b.URL)
|
bURL, _ := url.Parse(b.URL)
|
||||||
@ -133,7 +160,7 @@ func TestGitHubProviderGetEmailAddressEmailNotPresentInPayload(t *testing.T) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func TestGitHubProviderGetUserName(t *testing.T) {
|
func TestGitHubProviderGetUserName(t *testing.T) {
|
||||||
b := testGitHubBackend(`{"email": "michael.bland@gsa.gov", "login": "mbland"}`)
|
b := testGitHubBackend([]string{`{"email": "michael.bland@gsa.gov", "login": "mbland"}`})
|
||||||
defer b.Close()
|
defer b.Close()
|
||||||
|
|
||||||
bURL, _ := url.Parse(b.URL)
|
bURL, _ := url.Parse(b.URL)
|
||||||
|
Loading…
Reference in New Issue
Block a user