Skip to content

Commit

Permalink
Move creation of bearerToken to obtainBearerToken
Browse files Browse the repository at this point in the history 
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č <[email protected]>
  • Loading branch information
@mtrmac
mtrmac committed Jul 11, 2024
1 parent e7155e1 commit f8d6195
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 33 deletions.
60 changes: 31 additions & 29 deletions docker/docker_client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -760,20 +760,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()
Expand All @@ -783,16 +781,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
Expand All @@ -817,26 +818,29 @@ 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
}

return newBearerTokenFromHTTPResponseBody(res)
return dest.readFromHTTPResponseBody(res)
}

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()
Expand Down Expand Up @@ -864,22 +868,22 @@ 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
}

return newBearerTokenFromHTTPResponseBody(res)
return dest.readFromHTTPResponseBody(res)
}

// newBearerTokenFromHTTPResponseBody parses a http.Response to obtain a bearerToken.
// readFromHTTPResponseBody sets token data by parsing a http.Response.
// The caller is still responsible for ensuring res.Body is closed.
func newBearerTokenFromHTTPResponseBody(res *http.Response) (*bearerToken, error) {
func (bt *bearerToken) readFromHTTPResponseBody(res *http.Response) error {
blob, err := iolimits.ReadAtMost(res.Body, iolimits.MaxAuthTokenBodySize)
if err != nil {
return nil, err
return err
}

var token struct {
Expand All @@ -895,12 +899,10 @@ func newBearerTokenFromHTTPResponseBody(res *http.Response) (*bearerToken, error
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)
return fmt.Errorf("decoding bearer token (last URL %q, body start %q): %w", res.Request.URL.Redacted(), string(bodySample), err)
}

bt := &bearerToken{
token: token.Token,
}
bt.token = token.Token
if bt.token == "" {
bt.token = token.AccessToken
}
Expand All @@ -913,7 +915,7 @@ func newBearerTokenFromHTTPResponseBody(res *http.Response) (*bearerToken, error
token.IssuedAt = time.Now().UTC()
}
bt.expirationTime = token.IssuedAt.Add(time.Duration(token.ExpiresIn) * time.Second)
return bt, nil
return nil
}

// detectPropertiesHelper performs the work of detectProperties which executes
Expand Down
10 changes: 6 additions & 4 deletions docker/docker_client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ func testTokenHTTPResponse(t *testing.T, body string) *http.Response {
}
}

func TestNewBearerTokenFromHTTPResponseBody(t *testing.T) {
func TestBearerTokenReadFromHTTPResponseBody(t *testing.T) {
for _, c := range []struct {
input string
expected *bearerToken // or nil if a failure is expected
Expand All @@ -128,7 +128,8 @@ func TestNewBearerTokenFromHTTPResponseBody(t *testing.T) {
expected: &bearerToken{token: "IAmAToken", expirationTime: time.Unix(1514800802+60, 0)},
},
} {
token, err := newBearerTokenFromHTTPResponseBody(testTokenHTTPResponse(t, c.input))
token := &bearerToken{}
err := token.readFromHTTPResponseBody(testTokenHTTPResponse(t, c.input))
if c.expected == nil {
assert.Error(t, err, c.input)
} else {
Expand All @@ -140,11 +141,12 @@ func TestNewBearerTokenFromHTTPResponseBody(t *testing.T) {
}
}

func TestNewBearerTokenFromHTTPResponseBodyIssuedAtZero(t *testing.T) {
func TestBearerTokenReadFromHTTPResponseBodyIssuedAtZero(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, tokenBlob))
token := &bearerToken{}
err := token.readFromHTTPResponseBody(testTokenHTTPResponse(t, tokenBlob))
require.NoError(t, err)
expectedExpiration := now.Add(time.Duration(100) * time.Second)
require.False(t, token.expirationTime.Before(expectedExpiration),
Expand Down

0 comments on commit f8d6195

Please sign in to comment.