diff --git a/docker/docker_client.go b/docker/docker_client.go index 94cbcb1d99..6b65c52edc 100644 --- a/docker/docker_client.go +++ b/docker/docker_client.go @@ -147,10 +147,22 @@ const ( noAuth ) -func newBearerTokenFromJSONBlob(blob []byte) (*bearerToken, error) { +// newBearerTokenFromHTTPResponseBody parses a http.Response to obtain a bearerToken. +// The caller is still responsible for ensuring res.Body is closed. +func newBearerTokenFromHTTPResponseBody(res *http.Response) (*bearerToken, error) { + blob, err := iolimits.ReadAtMost(res.Body, iolimits.MaxAuthTokenBodySize) + if err != nil { + return nil, err + } + token := new(bearerToken) if err := json.Unmarshal(blob, &token); err != nil { - return nil, err + const bodySampleLength = 50 + bodySample := blob + if len(bodySample) > bodySampleLength { + bodySample = bodySample[:bodySampleLength] + } + return nil, fmt.Errorf("decoding bearer token (last URL %q, body start %q): %w", res.Request.URL.Redacted(), string(bodySample), err) } if token.Token == "" { token.Token = token.AccessToken @@ -827,12 +839,7 @@ func (c *dockerClient) getBearerTokenOAuth2(ctx context.Context, challenge chall return nil, err } - tokenBlob, err := iolimits.ReadAtMost(res.Body, iolimits.MaxAuthTokenBodySize) - if err != nil { - return nil, err - } - - return newBearerTokenFromJSONBlob(tokenBlob) + return newBearerTokenFromHTTPResponseBody(res) } func (c *dockerClient) getBearerToken(ctx context.Context, challenge challenge, @@ -878,12 +885,8 @@ func (c *dockerClient) getBearerToken(ctx context.Context, challenge challenge, if err := httpResponseToError(res, "Requesting bearer token"); err != nil { return nil, err } - tokenBlob, err := iolimits.ReadAtMost(res.Body, iolimits.MaxAuthTokenBodySize) - if err != nil { - return nil, err - } - return newBearerTokenFromJSONBlob(tokenBlob) + return newBearerTokenFromHTTPResponseBody(res) } // detectPropertiesHelper performs the work of detectProperties which executes diff --git a/docker/docker_client_test.go b/docker/docker_client_test.go index a7d16ef7b5..563358bbdf 100644 --- a/docker/docker_client_test.go +++ b/docker/docker_client_test.go @@ -5,8 +5,10 @@ import ( "bytes" "context" "fmt" + "io" "net/http" "net/http/httptest" + "net/url" "path/filepath" "strings" "testing" @@ -91,72 +93,61 @@ func TestDockerCertDir(t *testing.T) { } } -func TestNewBearerTokenFromJsonBlob(t *testing.T) { - expected := &bearerToken{Token: "IAmAToken", ExpiresIn: 100, IssuedAt: time.Unix(1514800802, 0)} - tokenBlob := []byte(`{"token":"IAmAToken","expires_in":100,"issued_at":"2018-01-01T10:00:02+00:00"}`) - token, err := newBearerTokenFromJSONBlob(tokenBlob) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - assertBearerTokensEqual(t, expected, token) -} - -func TestNewBearerAccessTokenFromJsonBlob(t *testing.T) { - expected := &bearerToken{Token: "IAmAToken", ExpiresIn: 100, IssuedAt: time.Unix(1514800802, 0)} - tokenBlob := []byte(`{"access_token":"IAmAToken","expires_in":100,"issued_at":"2018-01-01T10:00:02+00:00"}`) - token, err := newBearerTokenFromJSONBlob(tokenBlob) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - assertBearerTokensEqual(t, expected, token) -} - -func TestNewBearerTokenFromInvalidJsonBlob(t *testing.T) { - tokenBlob := []byte("IAmNotJson") - _, err := newBearerTokenFromJSONBlob(tokenBlob) - if err == nil { - t.Fatalf("unexpected an error unmarshaling JSON") +// testTokenHTTPResponse creates just enough of a *http.Response to work with newBearerTokenFromHTTPResponseBody. +func testTokenHTTPResponse(t *testing.T, body string) *http.Response { + requestURL, err := url.Parse("https://example.com/token") + require.NoError(t, err) + return &http.Response{ + Body: io.NopCloser(bytes.NewReader([]byte(body))), + Request: &http.Request{ + Method: "", + URL: requestURL, + }, } } -func TestNewBearerTokenSmallExpiryFromJsonBlob(t *testing.T) { - expected := &bearerToken{Token: "IAmAToken", ExpiresIn: 60, IssuedAt: time.Unix(1514800802, 0)} - tokenBlob := []byte(`{"token":"IAmAToken","expires_in":1,"issued_at":"2018-01-01T10:00:02+00:00"}`) - token, err := newBearerTokenFromJSONBlob(tokenBlob) - if err != nil { - t.Fatalf("unexpected error: %v", err) +func TestNewBearerTokenFromHTTPResponseBody(t *testing.T) { + for _, c := range []struct { + input string + expected *bearerToken // or nil if a failure is expected + }{ + { // Invalid JSON + input: "IAmNotJson", + expected: nil, + }, + { // "token" + input: `{"token":"IAmAToken","expires_in":100,"issued_at":"2018-01-01T10:00:02+00:00"}`, + expected: &bearerToken{Token: "IAmAToken", ExpiresIn: 100, IssuedAt: time.Unix(1514800802, 0)}, + }, + { // "access_token" + input: `{"access_token":"IAmAToken","expires_in":100,"issued_at":"2018-01-01T10:00:02+00:00"}`, + expected: &bearerToken{Token: "IAmAToken", ExpiresIn: 100, IssuedAt: time.Unix(1514800802, 0)}, + }, + { // Small expiry + input: `{"token":"IAmAToken","expires_in":1,"issued_at":"2018-01-01T10:00:02+00:00"}`, + expected: &bearerToken{Token: "IAmAToken", ExpiresIn: 60, IssuedAt: time.Unix(1514800802, 0)}, + }, + } { + token, err := newBearerTokenFromHTTPResponseBody(testTokenHTTPResponse(t, c.input)) + if c.expected == nil { + assert.Error(t, err, c.input) + } else { + require.NoError(t, err, c.input) + assert.Equal(t, c.expected.Token, token.Token, c.input) + assert.Equal(t, c.expected.ExpiresIn, token.ExpiresIn, c.input) + assert.True(t, c.expected.IssuedAt.Equal(token.IssuedAt), + "expected [%s] to equal [%s], it did not", token.IssuedAt, c.expected.IssuedAt) + } } - - assertBearerTokensEqual(t, expected, token) } -func TestNewBearerTokenIssuedAtZeroFromJsonBlob(t *testing.T) { +func TestNewBearerTokenFromHTTPResponseBodyIssuedAtZero(t *testing.T) { zeroTime := time.Time{}.Format(time.RFC3339) now := time.Now() - tokenBlob := []byte(fmt.Sprintf(`{"token":"IAmAToken","expires_in":100,"issued_at":"%s"}`, zeroTime)) - token, err := newBearerTokenFromJSONBlob(tokenBlob) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - if token.IssuedAt.Before(now) { - t.Fatalf("expected [%s] not to be before [%s]", token.IssuedAt, now) - } - -} - -func assertBearerTokensEqual(t *testing.T, expected, subject *bearerToken) { - if expected.Token != subject.Token { - t.Fatalf("expected [%s] to equal [%s], it did not", subject.Token, expected.Token) - } - if expected.ExpiresIn != subject.ExpiresIn { - t.Fatalf("expected [%d] to equal [%d], it did not", subject.ExpiresIn, expected.ExpiresIn) - } - if !expected.IssuedAt.Equal(subject.IssuedAt) { - t.Fatalf("expected [%s] to equal [%s], it did not", subject.IssuedAt, expected.IssuedAt) - } + tokenBlob := fmt.Sprintf(`{"token":"IAmAToken","expires_in":100,"issued_at":"%s"}`, zeroTime) + token, err := newBearerTokenFromHTTPResponseBody(testTokenHTTPResponse(t, string(tokenBlob))) + require.NoError(t, err) + assert.False(t, token.IssuedAt.Before(now), "expected [%s] not to be before [%s]", token.IssuedAt, now) } func TestUserAgent(t *testing.T) {