From 4b865373c95e6a182d5dad4ec6dc480d6dc3fe40 Mon Sep 17 00:00:00 2001 From: Josh Powers <powersj@fastmail.com> Date: Fri, 8 Jul 2022 08:48:47 -0600 Subject: [PATCH] fix: use reader over readcloser, regen cookie-jar Similar to #11083, some HTTP servers seem to quietly requires the content-length header in messages sent. Use of Go's ReadCloser does not allow for the content-length to be sent. As this is for a simple login the body length will be very small and the more efficient processing of a closer is not required in this use case. Moves the creation of the cookie jar as well. On a re-auth, the cookie was included in the request. Not only does this not make sense it can cause issues on some APIs where this was not expected. Given the original request does not contain this it seems safe to remake the cookie jar, emptying it, for each auth request. This also adds the ability to see the response body in the event of an error to gain a bit more details. fixes: #11478 --- plugins/common/cookie/cookie.go | 26 ++++++++++++++++---------- plugins/common/cookie/cookie_test.go | 4 ++-- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/plugins/common/cookie/cookie.go b/plugins/common/cookie/cookie.go index e2e431aae31b9..b9e8934c41147 100644 --- a/plugins/common/cookie/cookie.go +++ b/plugins/common/cookie/cookie.go @@ -54,11 +54,6 @@ func (c *CookieAuthConfig) initializeClient(client *http.Client) (err error) { c.Method = http.MethodPost } - // add cookie jar to HTTP client - if c.client.Jar, err = cookiejar.New(nil); err != nil { - return err - } - return c.auth() } @@ -77,10 +72,19 @@ func (c *CookieAuthConfig) authRenewal(ctx context.Context, ticker *clockutil.Ti } func (c *CookieAuthConfig) auth() error { - var body io.ReadCloser + var err error + + // everytime we auth we clear out the cookie jar to ensure that the cookie + // is not used as a part of re-authing. The only way to empty or reset is + // to create a new cookie jar. + c.client.Jar, err = cookiejar.New(nil) + if err != nil { + return err + } + + var body io.Reader if c.Body != "" { - body = io.NopCloser(strings.NewReader(c.Body)) - defer body.Close() + body = strings.NewReader(c.Body) } req, err := http.NewRequest(c.Method, c.URL, body) @@ -106,15 +110,17 @@ func (c *CookieAuthConfig) auth() error { } defer resp.Body.Close() - if _, err = io.Copy(io.Discard, resp.Body); err != nil { + respBody, err := io.ReadAll(resp.Body) + if err != nil { return err } // check either 200 or 201 as some devices may return either if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated { - return fmt.Errorf("cookie auth renewal received status code: %v (%v)", + return fmt.Errorf("cookie auth renewal received status code: %v (%v) [%v]", resp.StatusCode, http.StatusText(resp.StatusCode), + string(respBody), ) } diff --git a/plugins/common/cookie/cookie_test.go b/plugins/common/cookie/cookie_test.go index c1c7ce294d0f5..24c527024fde7 100644 --- a/plugins/common/cookie/cookie_test.go +++ b/plugins/common/cookie/cookie_test.go @@ -189,7 +189,7 @@ func TestAuthConfig_Start(t *testing.T) { renewal: renewal, endpoint: authEndpointWithBasicAuth, }, - wantErr: fmt.Errorf("cookie auth renewal received status code: 401 (Unauthorized)"), + wantErr: fmt.Errorf("cookie auth renewal received status code: 401 (Unauthorized) []"), firstAuthCount: 0, lastAuthCount: 0, firstHTTPResponse: http.StatusForbidden, @@ -220,7 +220,7 @@ func TestAuthConfig_Start(t *testing.T) { renewal: renewal, endpoint: authEndpointWithBody, }, - wantErr: fmt.Errorf("cookie auth renewal received status code: 401 (Unauthorized)"), + wantErr: fmt.Errorf("cookie auth renewal received status code: 401 (Unauthorized) []"), firstAuthCount: 0, lastAuthCount: 0, firstHTTPResponse: http.StatusForbidden,