diff --git a/.gitignore b/.gitignore index a5f59b4e..aff7b5b3 100644 --- a/.gitignore +++ b/.gitignore @@ -16,6 +16,7 @@ release # Folders _obj _test +.idea/ # Architecture specific extensions/prefixes *.[568vq] diff --git a/.travis.yml b/.travis.yml index 7fb4bce2..ef8aa3ec 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,6 +1,5 @@ language: go go: - - 1.12.x - 1.13.x install: # Fetch dependencies diff --git a/CHANGELOG.md b/CHANGELOG.md index 55548857..9a3281f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - [#275](https://github.com/pusher/oauth2_proxy/pull/275) docker: build from debian buster (@syscll) - [#258](https://github.com/pusher/oauth2_proxy/pull/258) Add IDToken for Azure provider - This PR adds the IDToken into the session for the Azure provider allowing requests to a backend to be identified as a specific user. As a consequence, if you are using a cookie to store the session the cookie will now exceed the 4kb size limit and be split into multiple cookies. This can cause problems when using nginx as a proxy, resulting in no cookie being passed at all. Either increase the proxy_buffer_size in nginx or implement the redis session storage (see https://pusher.github.io/oauth2_proxy/configuration#redis-storage) +- [#286](https://github.com/pusher/oauth2_proxy/pull/286) Requests.go updated with useful error messages (@biotom) # v4.0.0 diff --git a/pkg/requests/requests.go b/pkg/requests/requests.go index 82d1176a..9083b2d4 100644 --- a/pkg/requests/requests.go +++ b/pkg/requests/requests.go @@ -18,17 +18,23 @@ func Request(req *http.Request) (*simplejson.Json, error) { return nil, err } body, err := ioutil.ReadAll(resp.Body) - resp.Body.Close() - logger.Printf("%d %s %s %s", resp.StatusCode, req.Method, req.URL, body) - if err != nil { - return nil, err + if body != nil { + defer resp.Body.Close() } + + logger.Printf("%d %s %s %s", resp.StatusCode, req.Method, req.URL, body) + + if err != nil { + return nil, fmt.Errorf("problem reading http request body: %w", err) + } + if resp.StatusCode != 200 { return nil, fmt.Errorf("got %d %s", resp.StatusCode, body) } + data, err := simplejson.NewJson(body) if err != nil { - return nil, err + return nil, fmt.Errorf("error unmarshalling json: %w", err) } return data, nil } @@ -41,10 +47,13 @@ func RequestJSON(req *http.Request, v interface{}) error { return err } body, err := ioutil.ReadAll(resp.Body) - resp.Body.Close() + if body != nil { + defer resp.Body.Close() + } + logger.Printf("%d %s %s %s", resp.StatusCode, req.Method, req.URL, body) if err != nil { - return err + return fmt.Errorf("error reading body from http response: %w", err) } if resp.StatusCode != 200 { return fmt.Errorf("got %d %s", resp.StatusCode, body) @@ -56,7 +65,7 @@ func RequestJSON(req *http.Request, v interface{}) error { func RequestUnparsedResponse(url string, header http.Header) (resp *http.Response, err error) { req, err := http.NewRequest("GET", url, nil) if err != nil { - return nil, err + return nil, fmt.Errorf("error performing get request: %w", err) } req.Header = header diff --git a/pkg/requests/requests_test.go b/pkg/requests/requests_test.go index 99a4c3b6..c9ec4e88 100644 --- a/pkg/requests/requests_test.go +++ b/pkg/requests/requests_test.go @@ -8,20 +8,21 @@ import ( "testing" "github.com/bitly/go-simplejson" - "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) -func testBackend(responseCode int, payload string) *httptest.Server { +func testBackend(t *testing.T, responseCode int, payload string) *httptest.Server { return httptest.NewServer(http.HandlerFunc( func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(responseCode) - w.Write([]byte(payload)) + _, err := w.Write([]byte(payload)) + require.NoError(t, err) })) } func TestRequest(t *testing.T) { - backend := testBackend(200, "{\"foo\": \"bar\"}") + backend := testBackend(t, 200, "{\"foo\": \"bar\"}") defer backend.Close() req, _ := http.NewRequest("GET", backend.URL, nil) @@ -35,7 +36,7 @@ func TestRequest(t *testing.T) { func TestRequestFailure(t *testing.T) { // Create a backend to generate a test URL, then close it to cause a // connection error. - backend := testBackend(200, "{\"foo\": \"bar\"}") + backend := testBackend(t, 200, "{\"foo\": \"bar\"}") backend.Close() req, err := http.NewRequest("GET", backend.URL, nil) @@ -49,7 +50,7 @@ func TestRequestFailure(t *testing.T) { } func TestHttpErrorCode(t *testing.T) { - backend := testBackend(404, "{\"foo\": \"bar\"}") + backend := testBackend(t, 404, "{\"foo\": \"bar\"}") defer backend.Close() req, err := http.NewRequest("GET", backend.URL, nil) @@ -60,7 +61,7 @@ func TestHttpErrorCode(t *testing.T) { } func TestJsonParsingError(t *testing.T) { - backend := testBackend(200, "not well-formed JSON") + backend := testBackend(t, 200, "not well-formed JSON") defer backend.Close() req, err := http.NewRequest("GET", backend.URL, nil) @@ -77,7 +78,8 @@ func TestRequestUnparsedResponseUsingAccessTokenParameter(t *testing.T) { token := r.FormValue("access_token") if r.URL.Path == "/" && token == "my_token" { w.WriteHeader(200) - w.Write([]byte("some payload")) + _, err := w.Write([]byte("some payload")) + require.NoError(t, err) } else { w.WriteHeader(403) } @@ -86,16 +88,17 @@ func TestRequestUnparsedResponseUsingAccessTokenParameter(t *testing.T) { response, err := RequestUnparsedResponse( backend.URL+"?access_token=my_token", nil) + defer response.Body.Close() + assert.Equal(t, nil, err) assert.Equal(t, 200, response.StatusCode) body, err := ioutil.ReadAll(response.Body) assert.Equal(t, nil, err) - response.Body.Close() assert.Equal(t, "some payload", string(body)) } func TestRequestUnparsedResponseUsingAccessTokenParameterFailedResponse(t *testing.T) { - backend := testBackend(200, "some payload") + backend := testBackend(t, 200, "some payload") // Close the backend now to force a request failure. backend.Close() @@ -110,7 +113,8 @@ func TestRequestUnparsedResponseUsingHeaders(t *testing.T) { func(w http.ResponseWriter, r *http.Request) { if r.URL.Path == "/" && r.Header["Auth"][0] == "my_token" { w.WriteHeader(200) - w.Write([]byte("some payload")) + _, err := w.Write([]byte("some payload")) + require.NoError(t, err) } else { w.WriteHeader(403) } @@ -120,10 +124,12 @@ func TestRequestUnparsedResponseUsingHeaders(t *testing.T) { headers := make(http.Header) headers.Set("Auth", "my_token") response, err := RequestUnparsedResponse(backend.URL, headers) + defer response.Body.Close() + assert.Equal(t, nil, err) assert.Equal(t, 200, response.StatusCode) body, err := ioutil.ReadAll(response.Body) assert.Equal(t, nil, err) - response.Body.Close() + assert.Equal(t, "some payload", string(body)) }