Skip to content

Commit

Permalink
fix: use reader over readcloser, regen cookie-jar
Browse files Browse the repository at this point in the history
Similar to influxdata#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: influxdata#11478
  • Loading branch information
powersj committed Jul 15, 2022
1 parent 2ac311c commit 4b86537
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 12 deletions.
26 changes: 16 additions & 10 deletions plugins/common/cookie/cookie.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

Expand All @@ -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)
Expand All @@ -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),
)
}

Expand Down
4 changes: 2 additions & 2 deletions plugins/common/cookie/cookie_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 4b86537

Please sign in to comment.