Merge pull request #2480 from mtrmac/token-cleanup

Clean up obtaining bearer tokens for registries

Author: rhatdan

docker/docker_client.go

@@ -86,11 +86,9 @@ type extensionSignatureList struct {
 	Signatures []extensionSignature `json:"signatures"`
 }
 
+// bearerToken records a cached token we can use to authenticate.
 type bearerToken struct {
-	Token          string    `json:"token"`
-	AccessToken    string    `json:"access_token"`
-	ExpiresIn      int       `json:"expires_in"`
-	IssuedAt       time.Time `json:"issued_at"`
+	token          string
 	expirationTime time.Time
 }
 
@@ -147,37 +145,6 @@ const (
 	noAuth
 )
 
-// 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 {
-		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
-	}
-	if token.ExpiresIn < minimumTokenLifetimeSeconds {
-		token.ExpiresIn = minimumTokenLifetimeSeconds
-		logrus.Debugf("Increasing token expiration to: %d seconds", token.ExpiresIn)
-	}
-	if token.IssuedAt.IsZero() {
-		token.IssuedAt = time.Now().UTC()
-	}
-	token.expirationTime = token.IssuedAt.Add(time.Duration(token.ExpiresIn) * time.Second)
-	return token, nil
-}
-
 // dockerCertDir returns a path to a directory to be consumed by tlsclientconfig.SetupCertificates() depending on ctx and hostPort.
 func dockerCertDir(sys *types.SystemContext, hostPort string) (string, error) {
 	if sys != nil && sys.DockerCertPath != "" {
@@ -786,7 +753,7 @@ func (c *dockerClient) setupRequestAuth(req *http.Request, extraScope *authScope
 					token = *t
 					c.tokenCache.Store(cacheKey, token)
 				}
-				registryToken = token.Token
+				registryToken = token.token
 			}
 			req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", registryToken))
 			return nil
@@ -889,6 +856,48 @@ func (c *dockerClient) getBearerToken(ctx context.Context, challenge challenge,
 	return newBearerTokenFromHTTPResponseBody(res)
 }
 
+// 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
+	}
+
+	var token struct {
+		Token          string    `json:"token"`
+		AccessToken    string    `json:"access_token"`
+		ExpiresIn      int       `json:"expires_in"`
+		IssuedAt       time.Time `json:"issued_at"`
+		expirationTime time.Time
+	}
+	if err := json.Unmarshal(blob, &token); err != nil {
+		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)
+	}
+
+	bt := &bearerToken{
+		token: token.Token,
+	}
+	if bt.token == "" {
+		bt.token = token.AccessToken
+	}
+
+	if token.ExpiresIn < minimumTokenLifetimeSeconds {
+		token.ExpiresIn = minimumTokenLifetimeSeconds
+		logrus.Debugf("Increasing token expiration to: %d seconds", token.ExpiresIn)
+	}
+	if token.IssuedAt.IsZero() {
+		token.IssuedAt = time.Now().UTC()
+	}
+	bt.expirationTime = token.IssuedAt.Add(time.Duration(token.ExpiresIn) * time.Second)
+	return bt, nil
+}
+
 // detectPropertiesHelper performs the work of detectProperties which executes
 // it at most once.
 func (c *dockerClient) detectPropertiesHelper(ctx context.Context) error {

docker/docker_client_test.go

@@ -117,26 +117,25 @@ func TestNewBearerTokenFromHTTPResponseBody(t *testing.T) {
 		},
 		{ // "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)},
+			expected: &bearerToken{token: "IAmAToken", expirationTime: time.Unix(1514800802+100, 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)},
+			expected: &bearerToken{token: "IAmAToken", expirationTime: time.Unix(1514800802+100, 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)},
+			expected: &bearerToken{token: "IAmAToken", expirationTime: time.Unix(1514800802+60, 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)
+			assert.Equal(t, c.expected.token, token.token, c.input)
+			assert.True(t, c.expected.expirationTime.Equal(token.expirationTime),
+				"expected [%s] to equal [%s], it did not", token.expirationTime, c.expected.expirationTime)
 		}
 	}
 }
@@ -145,9 +144,11 @@ func TestNewBearerTokenFromHTTPResponseBodyIssuedAtZero(t *testing.T) {
 	zeroTime := time.Time{}.Format(time.RFC3339)
 	now := time.Now()
 	tokenBlob := fmt.Sprintf(`{"token":"IAmAToken","expires_in":100,"issued_at":"%s"}`, zeroTime)
-	token, err := newBearerTokenFromHTTPResponseBody(testTokenHTTPResponse(t, string(tokenBlob)))
+	token, err := newBearerTokenFromHTTPResponseBody(testTokenHTTPResponse(t, tokenBlob))
 	require.NoError(t, err)
-	assert.False(t, token.IssuedAt.Before(now), "expected [%s] not to be before [%s]", token.IssuedAt, now)
+	expectedExpiration := now.Add(time.Duration(100) * time.Second)
+	require.False(t, token.expirationTime.Before(expectedExpiration),
+		"expected [%s] not to be before [%s]", token.expirationTime, expectedExpiration)
 }
 
 func TestUserAgent(t *testing.T) {