From 87179b61ea69b3e4e8b58609002a843f3b7cbd57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Mon, 29 May 2023 22:41:25 +0200 Subject: [PATCH] Move creation of bearerToken to obtainBearerToken MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of having getBearerToken* construct a new bearerToken object, have the caller provide one. This will allow us to record that a token is being obtained, so that others can wait for it. Should not change behavior. Signed-off-by: Miloslav Trmač --- docker/docker_client.go | 67 +++++++++++++++++++----------------- docker/docker_client_test.go | 10 +++--- 2 files changed, 41 insertions(+), 36 deletions(-) diff --git a/docker/docker_client.go b/docker/docker_client.go index 2437163dcb..42c56ed58a 100644 --- a/docker/docker_client.go +++ b/docker/docker_client.go @@ -141,7 +141,8 @@ const ( noAuth ) -func newBearerTokenFromJSONBlob(blob []byte) (*bearerToken, error) { +// readFromJSONBlob sets token data in dest from the provided JSON. +func (bt *bearerToken) readFromJSONBlob(blob []byte) error { var token struct { Token string `json:"token"` AccessToken string `json:"access_token"` @@ -150,14 +151,12 @@ func newBearerTokenFromJSONBlob(blob []byte) (*bearerToken, error) { expirationTime time.Time } if err := json.Unmarshal(blob, &token); err != nil { - return nil, err + return err } - res := &bearerToken{ - token: token.Token, - } - if res.token == "" { - res.token = token.AccessToken + bt.token = token.Token + if bt.token == "" { + bt.token = token.AccessToken } if token.ExpiresIn < minimumTokenLifetimeSeconds { @@ -167,8 +166,8 @@ func newBearerTokenFromJSONBlob(blob []byte) (*bearerToken, error) { if token.IssuedAt.IsZero() { token.IssuedAt = time.Now().UTC() } - res.expirationTime = token.IssuedAt.Add(time.Duration(token.ExpiresIn) * time.Second) - return res, nil + bt.expirationTime = token.IssuedAt.Add(time.Duration(token.ExpiresIn) * time.Second) + return nil } // this is cloned from docker/go-connections because upstream docker has changed @@ -712,20 +711,18 @@ func (c *dockerClient) obtainBearerToken(ctx context.Context, challenge challeng token, inCache = c.tokenCache[cacheKey] }() if !inCache || time.Now().After(token.expirationTime) { - var ( - t *bearerToken - err error - ) + token = &bearerToken{} + + var err error if c.auth.IdentityToken != "" { - t, err = c.getBearerTokenOAuth2(ctx, challenge, scopes) + err = c.getBearerTokenOAuth2(ctx, token, challenge, scopes) } else { - t, err = c.getBearerToken(ctx, challenge, scopes) + err = c.getBearerToken(ctx, token, challenge, scopes) } if err != nil { return "", err } - token = t func() { // A scope for defer c.tokenCacheLock.Lock() defer c.tokenCacheLock.Unlock() @@ -735,16 +732,19 @@ func (c *dockerClient) obtainBearerToken(ctx context.Context, challenge challeng return token.token, nil } -func (c *dockerClient) getBearerTokenOAuth2(ctx context.Context, challenge challenge, - scopes []authScope) (*bearerToken, error) { +// getBearerTokenOAuth2 obtains an "Authorization: Bearer" token using a pre-existing identity token per +// https://github.com/distribution/distribution/blob/main/docs/spec/auth/oauth.md for challenge and scopes, +// and writes it into dest. +func (c *dockerClient) getBearerTokenOAuth2(ctx context.Context, dest *bearerToken, challenge challenge, + scopes []authScope) error { realm, ok := challenge.Parameters["realm"] if !ok { - return nil, errors.New("missing realm in bearer auth challenge") + return errors.New("missing realm in bearer auth challenge") } authReq, err := http.NewRequestWithContext(ctx, http.MethodPost, realm, nil) if err != nil { - return nil, err + return err } // Make the form data required against the oauth2 authentication @@ -769,31 +769,34 @@ func (c *dockerClient) getBearerTokenOAuth2(ctx context.Context, challenge chall logrus.Debugf("%s %s", authReq.Method, authReq.URL.Redacted()) res, err := c.client.Do(authReq) if err != nil { - return nil, err + return err } defer res.Body.Close() if err := httpResponseToError(res, "Trying to obtain access token"); err != nil { - return nil, err + return err } tokenBlob, err := iolimits.ReadAtMost(res.Body, iolimits.MaxAuthTokenBodySize) if err != nil { - return nil, err + return err } - return newBearerTokenFromJSONBlob(tokenBlob) + return dest.readFromJSONBlob(tokenBlob) } -func (c *dockerClient) getBearerToken(ctx context.Context, challenge challenge, - scopes []authScope) (*bearerToken, error) { +// getBearerToken obtains an "Authorization: Bearer" token using a GET request, per +// https://github.com/distribution/distribution/blob/main/docs/spec/auth/token.md for challenge and scopes, +// and writes it into dest. +func (c *dockerClient) getBearerToken(ctx context.Context, dest *bearerToken, challenge challenge, + scopes []authScope) error { realm, ok := challenge.Parameters["realm"] if !ok { - return nil, errors.New("missing realm in bearer auth challenge") + return errors.New("missing realm in bearer auth challenge") } authReq, err := http.NewRequestWithContext(ctx, http.MethodGet, realm, nil) if err != nil { - return nil, err + return err } params := authReq.URL.Query() @@ -821,18 +824,18 @@ func (c *dockerClient) getBearerToken(ctx context.Context, challenge challenge, logrus.Debugf("%s %s", authReq.Method, authReq.URL.Redacted()) res, err := c.client.Do(authReq) if err != nil { - return nil, err + return err } defer res.Body.Close() if err := httpResponseToError(res, "Requesting bearer token"); err != nil { - return nil, err + return err } tokenBlob, err := iolimits.ReadAtMost(res.Body, iolimits.MaxAuthTokenBodySize) if err != nil { - return nil, err + return err } - return newBearerTokenFromJSONBlob(tokenBlob) + return dest.readFromJSONBlob(tokenBlob) } // detectPropertiesHelper performs the work of detectProperties which executes diff --git a/docker/docker_client_test.go b/docker/docker_client_test.go index c48d7bb574..f232194c20 100644 --- a/docker/docker_client_test.go +++ b/docker/docker_client_test.go @@ -92,7 +92,7 @@ func TestDockerCertDir(t *testing.T) { } } -func TestNewBearerTokenFromJSONBlob(t *testing.T) { +func TestBearerTokenReadFromJSONBlob(t *testing.T) { for _, c := range []struct { input string expected *bearerToken // or nil on failure @@ -111,7 +111,8 @@ func TestNewBearerTokenFromJSONBlob(t *testing.T) { &bearerToken{token: "IAmAToken", expirationTime: time.Unix(1514800802+60, 0)}, }, } { - token, err := newBearerTokenFromJSONBlob([]byte(c.input)) + token := &bearerToken{} + err := token.readFromJSONBlob([]byte(c.input)) if c.expected == nil { assert.Error(t, err, c.input) } else { @@ -123,11 +124,12 @@ func TestNewBearerTokenFromJSONBlob(t *testing.T) { } } -func TestNewBearerTokenIssuedAtZeroFromJsonBlob(t *testing.T) { +func TestBearerTokenReadFromJSONBlobIssuedAtZeroFrom(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) + token := &bearerToken{} + err := token.readFromJSONBlob(tokenBlob) require.NoError(t, err) expectedExpiration := now.Add(time.Duration(100) * time.Second) require.False(t, token.expirationTime.Before(expectedExpiration),