Credentials return AccessToken by value (Azure#17838)

Author: chlowell

sdk/azidentity/CHANGELOG.md

@@ -11,6 +11,7 @@
 * Removed `AuthorizationCodeCredential`. Use `InteractiveBrowserCredential` instead
   to authenticate a user with the authorization code flow.
 * Instances of `AuthenticationFailedError` are now returned by pointer.
+* `GetToken()` returns `azcore.AccessToken` by value
 
 ### Bugs Fixed
 * `AzureCLICredential` panics after receiving an unexpected error type

sdk/azidentity/azure_cli_credential.go

@@ -62,26 +62,26 @@ func NewAzureCLICredential(options *AzureCLICredentialOptions) (*AzureCLICredent
 
 // GetToken requests a token from the Azure CLI. This credential doesn't cache tokens, so every call invokes the CLI.
 // This method is called automatically by Azure SDK clients.
-func (c *AzureCLICredential) GetToken(ctx context.Context, opts policy.TokenRequestOptions) (*azcore.AccessToken, error) {
+func (c *AzureCLICredential) GetToken(ctx context.Context, opts policy.TokenRequestOptions) (azcore.AccessToken, error) {
 	if len(opts.Scopes) != 1 {
-		return nil, errors.New(credNameAzureCLI + ": GetToken() requires exactly one scope")
+		return azcore.AccessToken{}, errors.New(credNameAzureCLI + ": GetToken() requires exactly one scope")
 	}
 	// CLI expects an AAD v1 resource, not a v2 scope
 	scope := strings.TrimSuffix(opts.Scopes[0], defaultSuffix)
 	at, err := c.authenticate(ctx, scope)
 	if err != nil {
-		return nil, err
+		return azcore.AccessToken{}, err
 	}
 	logGetTokenSuccess(c, opts)
 	return at, nil
 }
 
 const timeoutCLIRequest = 10 * time.Second
 
-func (c *AzureCLICredential) authenticate(ctx context.Context, resource string) (*azcore.AccessToken, error) {
+func (c *AzureCLICredential) authenticate(ctx context.Context, resource string) (azcore.AccessToken, error) {
 	output, err := c.tokenProvider(ctx, resource, c.tenantID)
 	if err != nil {
-		return nil, err
+		return azcore.AccessToken{}, err
 	}
 
 	return c.createAccessToken(output)
@@ -137,7 +137,7 @@ func defaultTokenProvider() func(ctx context.Context, resource string, tenantID
 	}
 }
 
-func (c *AzureCLICredential) createAccessToken(tk []byte) (*azcore.AccessToken, error) {
+func (c *AzureCLICredential) createAccessToken(tk []byte) (azcore.AccessToken, error) {
 	t := struct {
 		AccessToken      string `json:"accessToken"`
 		Authority        string `json:"_authority"`
@@ -152,15 +152,15 @@ func (c *AzureCLICredential) createAccessToken(tk []byte) (*azcore.AccessToken,
 	}{}
 	err := json.Unmarshal(tk, &t)
 	if err != nil {
-		return nil, err
+		return azcore.AccessToken{}, err
 	}
 
 	tokenExpirationDate, err := parseExpirationDate(t.ExpiresOn)
 	if err != nil {
-		return nil, fmt.Errorf("Error parsing Token Expiration Date %q: %+v", t.ExpiresOn, err)
+		return azcore.AccessToken{}, fmt.Errorf("Error parsing Token Expiration Date %q: %+v", t.ExpiresOn, err)
 	}
 
-	converted := &azcore.AccessToken{
+	converted := azcore.AccessToken{
 		Token:     t.AccessToken,
 		ExpiresOn: *tokenExpirationDate,
 	}

sdk/azidentity/chained_token_credential.go

@@ -59,7 +59,7 @@ func NewChainedTokenCredential(sources []azcore.TokenCredential, options *Chaine
 
 // GetToken calls GetToken on the chained credentials in turn, stopping when one returns a token.
 // This method is called automatically by Azure SDK clients.
-func (c *ChainedTokenCredential) GetToken(ctx context.Context, opts policy.TokenRequestOptions) (*azcore.AccessToken, error) {
+func (c *ChainedTokenCredential) GetToken(ctx context.Context, opts policy.TokenRequestOptions) (azcore.AccessToken, error) {
 	if !c.retrySources {
 		// ensure only one goroutine at a time iterates the sources and perhaps sets c.successfulCredential
 		c.cond.L.Lock()
@@ -80,7 +80,7 @@ func (c *ChainedTokenCredential) GetToken(ctx context.Context, opts policy.Token
 
 	var err error
 	var errs []error
-	var token *azcore.AccessToken
+	var token azcore.AccessToken
 	var successfulCredential azcore.TokenCredential
 	for _, cred := range c.sources {
 		token, err = cred.GetToken(ctx, opts)

sdk/azidentity/chained_token_credential_test.go

@@ -17,7 +17,7 @@ import (
 )
 
 type fakeCredentialResponse struct {
-	token *azcore.AccessToken
+	token azcore.AccessToken
 	err   error
 }
 
@@ -32,19 +32,19 @@ func NewFakeCredential() *fakeCredential {
 	return &fakeCredential{mut: &sync.Mutex{}}
 }
 
-func (c *fakeCredential) SetResponse(tk *azcore.AccessToken, err error) {
+func (c *fakeCredential) SetResponse(tk azcore.AccessToken, err error) {
 	c.mut.Lock()
 	defer c.mut.Unlock()
 	c.static = &fakeCredentialResponse{tk, err}
 }
 
-func (c *fakeCredential) AppendResponse(tk *azcore.AccessToken, err error) {
+func (c *fakeCredential) AppendResponse(tk azcore.AccessToken, err error) {
 	c.mut.Lock()
 	defer c.mut.Unlock()
 	c.responses = append(c.responses, fakeCredentialResponse{tk, err})
 }
 
-func (c *fakeCredential) GetToken(ctx context.Context, opts policy.TokenRequestOptions) (token *azcore.AccessToken, err error) {
+func (c *fakeCredential) GetToken(ctx context.Context, opts policy.TokenRequestOptions) (azcore.AccessToken, error) {
 	c.mut.Lock()
 	defer c.mut.Unlock()
 	c.getTokenCalls += 1
@@ -56,7 +56,7 @@ func (c *fakeCredential) GetToken(ctx context.Context, opts policy.TokenRequestO
 	return response.token, response.err
 }
 
-func testGoodGetTokenResponse(t *testing.T, token *azcore.AccessToken, err error) {
+func testGoodGetTokenResponse(t *testing.T, token azcore.AccessToken, err error) {
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -81,9 +81,9 @@ func TestChainedTokenCredential_NilSource(t *testing.T) {
 
 func TestChainedTokenCredential_GetTokenSuccess(t *testing.T) {
 	c1 := NewFakeCredential()
-	c1.SetResponse(nil, newCredentialUnavailableError("test", "something went wrong"))
+	c1.SetResponse(azcore.AccessToken{}, newCredentialUnavailableError("test", "something went wrong"))
 	c2 := NewFakeCredential()
-	c2.SetResponse(&azcore.AccessToken{Token: tokenValue, ExpiresOn: time.Now().Add(time.Hour)}, nil)
+	c2.SetResponse(azcore.AccessToken{Token: tokenValue, ExpiresOn: time.Now().Add(time.Hour)}, nil)
 	cred, err := NewChainedTokenCredential([]azcore.TokenCredential{c1, c2}, nil)
 	if err != nil {
 		t.Fatal(err)
@@ -108,7 +108,7 @@ func TestChainedTokenCredential_GetTokenSuccess(t *testing.T) {
 
 func TestChainedTokenCredential_GetTokenFail(t *testing.T) {
 	c := NewFakeCredential()
-	c.SetResponse(nil, newAuthenticationFailedError("test", "something went wrong", nil))
+	c.SetResponse(azcore.AccessToken{}, newAuthenticationFailedError("test", "something went wrong", nil))
 	cred, err := NewChainedTokenCredential([]azcore.TokenCredential{c}, nil)
 	if err != nil {
 		t.Fatal(err)
@@ -128,11 +128,11 @@ func TestChainedTokenCredential_GetTokenFail(t *testing.T) {
 
 func TestChainedTokenCredential_MultipleCredentialsGetTokenUnavailable(t *testing.T) {
 	c1 := NewFakeCredential()
-	c1.SetResponse(nil, newCredentialUnavailableError("unavailableCredential1", "Unavailable expected error"))
+	c1.SetResponse(azcore.AccessToken{}, newCredentialUnavailableError("unavailableCredential1", "Unavailable expected error"))
 	c2 := NewFakeCredential()
-	c2.SetResponse(nil, newCredentialUnavailableError("unavailableCredential2", "Unavailable expected error"))
+	c2.SetResponse(azcore.AccessToken{}, newCredentialUnavailableError("unavailableCredential2", "Unavailable expected error"))
 	c3 := NewFakeCredential()
-	c3.SetResponse(nil, newCredentialUnavailableError("unavailableCredential3", "Unavailable expected error"))
+	c3.SetResponse(azcore.AccessToken{}, newCredentialUnavailableError("unavailableCredential3", "Unavailable expected error"))
 	cred, err := NewChainedTokenCredential([]azcore.TokenCredential{c1, c2, c3}, nil)
 	if err != nil {
 		t.Fatal(err)
@@ -157,11 +157,11 @@ Attempted credentials:
 
 func TestChainedTokenCredential_MultipleCredentialsGetTokenAuthenticationFailed(t *testing.T) {
 	c1 := NewFakeCredential()
-	c1.SetResponse(nil, newCredentialUnavailableError("unavailableCredential1", "Unavailable expected error"))
+	c1.SetResponse(azcore.AccessToken{}, newCredentialUnavailableError("unavailableCredential1", "Unavailable expected error"))
 	c2 := NewFakeCredential()
-	c2.SetResponse(nil, newCredentialUnavailableError("unavailableCredential2", "Unavailable expected error"))
+	c2.SetResponse(azcore.AccessToken{}, newCredentialUnavailableError("unavailableCredential2", "Unavailable expected error"))
 	c3 := NewFakeCredential()
-	c3.SetResponse(nil, newAuthenticationFailedError("authenticationFailedCredential3", "Authentication failed expected error", nil))
+	c3.SetResponse(azcore.AccessToken{}, newAuthenticationFailedError("authenticationFailedCredential3", "Authentication failed expected error", nil))
 	cred, err := NewChainedTokenCredential([]azcore.TokenCredential{c1, c2, c3}, nil)
 	if err != nil {
 		t.Fatal(err)
@@ -186,7 +186,7 @@ Attempted credentials:
 
 func TestChainedTokenCredential_MultipleCredentialsGetTokenCustomName(t *testing.T) {
 	c := NewFakeCredential()
-	c.SetResponse(nil, newCredentialUnavailableError("unavailableCredential1", "Unavailable expected error"))
+	c.SetResponse(azcore.AccessToken{}, newCredentialUnavailableError("unavailableCredential1", "Unavailable expected error"))
 	cred, err := NewChainedTokenCredential([]azcore.TokenCredential{c}, nil)
 	if err != nil {
 		t.Fatal(err)
@@ -210,9 +210,9 @@ Attempted credentials:
 
 func TestChainedTokenCredential_RepeatedGetTokenWithSuccessfulCredential(t *testing.T) {
 	failedCredential := NewFakeCredential()
-	failedCredential.SetResponse(nil, newCredentialUnavailableError("MockCredential", "Mocking a credential unavailable error"))
+	failedCredential.SetResponse(azcore.AccessToken{}, newCredentialUnavailableError("MockCredential", "Mocking a credential unavailable error"))
 	successfulCredential := NewFakeCredential()
-	successfulCredential.SetResponse(&azcore.AccessToken{Token: tokenValue, ExpiresOn: time.Now()}, nil)
+	successfulCredential.SetResponse(azcore.AccessToken{Token: tokenValue, ExpiresOn: time.Now()}, nil)
 
 	cred, err := NewChainedTokenCredential([]azcore.TokenCredential{failedCredential, successfulCredential}, nil)
 	if err != nil {
@@ -241,9 +241,9 @@ func TestChainedTokenCredential_RepeatedGetTokenWithSuccessfulCredential(t *test
 
 func TestChainedTokenCredential_RepeatedGetTokenWithSuccessfulCredentialWithRetrySources(t *testing.T) {
 	failedCredential := NewFakeCredential()
-	failedCredential.SetResponse(nil, newCredentialUnavailableError("MockCredential", "Mocking a credential unavailable error"))
+	failedCredential.SetResponse(azcore.AccessToken{}, newCredentialUnavailableError("MockCredential", "Mocking a credential unavailable error"))
 	successfulCredential := NewFakeCredential()
-	successfulCredential.SetResponse(&azcore.AccessToken{Token: tokenValue, ExpiresOn: time.Now()}, nil)
+	successfulCredential.SetResponse(azcore.AccessToken{Token: tokenValue, ExpiresOn: time.Now()}, nil)
 
 	cred, err := NewChainedTokenCredential([]azcore.TokenCredential{failedCredential, successfulCredential}, &ChainedTokenCredentialOptions{RetrySources: true})
 	if err != nil {
@@ -272,11 +272,11 @@ func TestChainedTokenCredential_RepeatedGetTokenWithSuccessfulCredentialWithRetr
 
 func TestChainedTokenCredential_Race(t *testing.T) {
 	successFake := NewFakeCredential()
-	successFake.SetResponse(&azcore.AccessToken{Token: "*", ExpiresOn: time.Now().Add(time.Hour)}, nil)
+	successFake.SetResponse(azcore.AccessToken{Token: "*", ExpiresOn: time.Now().Add(time.Hour)}, nil)
 	authFailFake := NewFakeCredential()
-	authFailFake.SetResponse(nil, newAuthenticationFailedError("", "", nil))
+	authFailFake.SetResponse(azcore.AccessToken{}, newAuthenticationFailedError("", "", nil))
 	unavailableFake := NewFakeCredential()
-	unavailableFake.SetResponse(nil, newCredentialUnavailableError("", ""))
+	unavailableFake.SetResponse(azcore.AccessToken{}, newCredentialUnavailableError("", ""))
 	tro := policy.TokenRequestOptions{Scopes: []string{liveTestScope}}
 
 	for _, b := range []bool{true, false} {

sdk/azidentity/client_certificate_credential.go

@@ -80,22 +80,22 @@ func NewClientCertificateCredential(tenantID string, clientID string, certs []*x
 }
 
 // GetToken requests an access token from Azure Active Directory. This method is called automatically by Azure SDK clients.
-func (c *ClientCertificateCredential) GetToken(ctx context.Context, opts policy.TokenRequestOptions) (*azcore.AccessToken, error) {
+func (c *ClientCertificateCredential) GetToken(ctx context.Context, opts policy.TokenRequestOptions) (azcore.AccessToken, error) {
 	if len(opts.Scopes) == 0 {
-		return nil, errors.New(credNameCert + ": GetToken() requires at least one scope")
+		return azcore.AccessToken{}, errors.New(credNameCert + ": GetToken() requires at least one scope")
 	}
 	ar, err := c.client.AcquireTokenSilent(ctx, opts.Scopes)
 	if err == nil {
 		logGetTokenSuccess(c, opts)
-		return &azcore.AccessToken{Token: ar.AccessToken, ExpiresOn: ar.ExpiresOn.UTC()}, err
+		return azcore.AccessToken{Token: ar.AccessToken, ExpiresOn: ar.ExpiresOn.UTC()}, err
 	}
 
 	ar, err = c.client.AcquireTokenByCredential(ctx, opts.Scopes)
 	if err != nil {
-		return nil, newAuthenticationFailedErrorFromMSALError(credNameCert, err)
+		return azcore.AccessToken{}, newAuthenticationFailedErrorFromMSALError(credNameCert, err)
 	}
 	logGetTokenSuccess(c, opts)
-	return &azcore.AccessToken{Token: ar.AccessToken, ExpiresOn: ar.ExpiresOn.UTC()}, err
+	return azcore.AccessToken{Token: ar.AccessToken, ExpiresOn: ar.ExpiresOn.UTC()}, err
 }
 
 // ParseCertificates loads certificates and a private key, in PEM or PKCS12 format, for use with NewClientCertificateCredential.

sdk/azidentity/client_certificate_credential_test.go

@@ -10,6 +10,7 @@ import (
 	"errors"
 	"log"
 	"os"
+	"reflect"
 	"testing"
 
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore"
@@ -229,8 +230,8 @@ func TestClientCertificateCredential_InvalidCertLive(t *testing.T) {
 	}
 
 	tk, err := cred.GetToken(context.Background(), policy.TokenRequestOptions{Scopes: []string{liveTestScope}})
-	if tk != nil {
-		t.Fatal("GetToken returned a token")
+	if !reflect.ValueOf(tk).IsZero() {
+		t.Fatal("expected a zero value AccessToken")
 	}
 	var e *AuthenticationFailedError
 	if !errors.As(err, &e) {

sdk/azidentity/client_secret_credential.go

@@ -52,22 +52,22 @@ func NewClientSecretCredential(tenantID string, clientID string, clientSecret st
 }
 
 // GetToken requests an access token from Azure Active Directory. This method is called automatically by Azure SDK clients.
-func (c *ClientSecretCredential) GetToken(ctx context.Context, opts policy.TokenRequestOptions) (*azcore.AccessToken, error) {
+func (c *ClientSecretCredential) GetToken(ctx context.Context, opts policy.TokenRequestOptions) (azcore.AccessToken, error) {
 	if len(opts.Scopes) == 0 {
-		return nil, errors.New(credNameSecret + ": GetToken() requires at least one scope")
+		return azcore.AccessToken{}, errors.New(credNameSecret + ": GetToken() requires at least one scope")
 	}
 	ar, err := c.client.AcquireTokenSilent(ctx, opts.Scopes)
 	if err == nil {
 		logGetTokenSuccess(c, opts)
-		return &azcore.AccessToken{Token: ar.AccessToken, ExpiresOn: ar.ExpiresOn.UTC()}, err
+		return azcore.AccessToken{Token: ar.AccessToken, ExpiresOn: ar.ExpiresOn.UTC()}, err
 	}
 
 	ar, err = c.client.AcquireTokenByCredential(ctx, opts.Scopes)
 	if err != nil {
-		return nil, newAuthenticationFailedErrorFromMSALError(credNameSecret, err)
+		return azcore.AccessToken{}, newAuthenticationFailedErrorFromMSALError(credNameSecret, err)
 	}
 	logGetTokenSuccess(c, opts)
-	return &azcore.AccessToken{Token: ar.AccessToken, ExpiresOn: ar.ExpiresOn.UTC()}, err
+	return azcore.AccessToken{Token: ar.AccessToken, ExpiresOn: ar.ExpiresOn.UTC()}, err
 }
 
 var _ azcore.TokenCredential = (*ClientSecretCredential)(nil)

sdk/azidentity/client_secret_credential_test.go

@@ -6,6 +6,7 @@ package azidentity
 import (
 	"context"
 	"errors"
+	"reflect"
 	"testing"
 
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore/policy"
@@ -55,8 +56,8 @@ func TestClientSecretCredential_InvalidSecretLive(t *testing.T) {
 		t.Fatalf("failed to construct credential: %v", err)
 	}
 	tk, err := cred.GetToken(context.Background(), policy.TokenRequestOptions{Scopes: []string{liveTestScope}})
-	if tk != nil {
-		t.Fatal("GetToken returned a token")
+	if !reflect.ValueOf(tk).IsZero() {
+		t.Fatal("expected a zero value AccessToken")
 	}
 	var e *AuthenticationFailedError
 	if !errors.As(err, &e) {

sdk/azidentity/default_azure_credential.go

@@ -90,7 +90,7 @@ func NewDefaultAzureCredential(options *DefaultAzureCredentialOptions) (*Default
 }
 
 // GetToken requests an access token from Azure Active Directory. This method is called automatically by Azure SDK clients.
-func (c *DefaultAzureCredential) GetToken(ctx context.Context, opts policy.TokenRequestOptions) (token *azcore.AccessToken, err error) {
+func (c *DefaultAzureCredential) GetToken(ctx context.Context, opts policy.TokenRequestOptions) (azcore.AccessToken, error) {
 	return c.chain.GetToken(ctx, opts)
 }
 
@@ -119,11 +119,11 @@ type defaultCredentialErrorReporter struct {
 	err      error
 }
 
-func (d *defaultCredentialErrorReporter) GetToken(ctx context.Context, opts policy.TokenRequestOptions) (token *azcore.AccessToken, err error) {
+func (d *defaultCredentialErrorReporter) GetToken(ctx context.Context, opts policy.TokenRequestOptions) (azcore.AccessToken, error) {
 	if _, ok := d.err.(*credentialUnavailableError); ok {
-		return nil, d.err
+		return azcore.AccessToken{}, d.err
 	}
-	return nil, newCredentialUnavailableError(d.credType, d.err.Error())
+	return azcore.AccessToken{}, newCredentialUnavailableError(d.credType, d.err.Error())
 }
 
 var _ azcore.TokenCredential = (*defaultCredentialErrorReporter)(nil)

sdk/azidentity/device_code_credential.go

@@ -95,33 +95,33 @@ func NewDeviceCodeCredential(options *DeviceCodeCredentialOptions) (*DeviceCodeC
 
 // GetToken requests an access token from Azure Active Directory. It will begin the device code flow and poll until the user completes authentication.
 // This method is called automatically by Azure SDK clients.
-func (c *DeviceCodeCredential) GetToken(ctx context.Context, opts policy.TokenRequestOptions) (*azcore.AccessToken, error) {
+func (c *DeviceCodeCredential) GetToken(ctx context.Context, opts policy.TokenRequestOptions) (azcore.AccessToken, error) {
 	if len(opts.Scopes) == 0 {
-		return nil, errors.New(credNameDeviceCode + ": GetToken() requires at least one scope")
+		return azcore.AccessToken{}, errors.New(credNameDeviceCode + ": GetToken() requires at least one scope")
 	}
 	ar, err := c.client.AcquireTokenSilent(ctx, opts.Scopes, public.WithSilentAccount(c.account))
 	if err == nil {
-		return &azcore.AccessToken{Token: ar.AccessToken, ExpiresOn: ar.ExpiresOn.UTC()}, err
+		return azcore.AccessToken{Token: ar.AccessToken, ExpiresOn: ar.ExpiresOn.UTC()}, err
 	}
 	dc, err := c.client.AcquireTokenByDeviceCode(ctx, opts.Scopes)
 	if err != nil {
-		return nil, newAuthenticationFailedErrorFromMSALError(credNameDeviceCode, err)
+		return azcore.AccessToken{}, newAuthenticationFailedErrorFromMSALError(credNameDeviceCode, err)
 	}
 	err = c.userPrompt(ctx, DeviceCodeMessage{
 		UserCode:        dc.Result.UserCode,
 		VerificationURL: dc.Result.VerificationURL,
 		Message:         dc.Result.Message,
 	})
 	if err != nil {
-		return nil, err
+		return azcore.AccessToken{}, err
 	}
 	ar, err = dc.AuthenticationResult(ctx)
 	if err != nil {
-		return nil, newAuthenticationFailedErrorFromMSALError(credNameDeviceCode, err)
+		return azcore.AccessToken{}, newAuthenticationFailedErrorFromMSALError(credNameDeviceCode, err)
 	}
 	c.account = ar.Account
 	logGetTokenSuccess(c, opts)
-	return &azcore.AccessToken{Token: ar.AccessToken, ExpiresOn: ar.ExpiresOn.UTC()}, err
+	return azcore.AccessToken{Token: ar.AccessToken, ExpiresOn: ar.ExpiresOn.UTC()}, err
 }
 
 var _ azcore.TokenCredential = (*DeviceCodeCredential)(nil)