From c1fc8c2c2540db19b56e73e88b8fb00add7995e5 Mon Sep 17 00:00:00 2001 From: Charles Lowell Date: Fri, 8 Oct 2021 14:20:59 -0700 Subject: [PATCH 1/3] Add DefaultAzureCredential struct --- sdk/azidentity/default_azure_credential.go | 30 +++++++++++++++++----- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/sdk/azidentity/default_azure_credential.go b/sdk/azidentity/default_azure_credential.go index 73f266934750..b4f14009bea0 100644 --- a/sdk/azidentity/default_azure_credential.go +++ b/sdk/azidentity/default_azure_credential.go @@ -4,8 +4,10 @@ package azidentity import ( + "context" + "github.com/Azure/azure-sdk-for-go/sdk/azcore" - "github.com/Azure/azure-sdk-for-go/sdk/internal/log" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" ) const ( @@ -23,13 +25,19 @@ type DefaultAzureCredentialOptions struct { AuthorityHost AuthorityHost } -// NewDefaultAzureCredential provides a default ChainedTokenCredential configuration for applications that will be deployed to Azure. The following credential -// types will be tried, in the following order: +// DefaultAzureCredential is a default credential chain for applications that will be deployed to Azure. +// It combines credentials suitable for deployed applications with credentials suitable in local development. +// It attempts to authenticate with each of these credential types, in the following order: // - EnvironmentCredential // - ManagedIdentityCredential // - AzureCLICredential -// Consult the documentation for these credential types for more information on how they attempt authentication. -func NewDefaultAzureCredential(options *DefaultAzureCredentialOptions) (*ChainedTokenCredential, error) { +// Consult the documentation for these credential types for more information on how they authenticate. +type DefaultAzureCredential struct { + chain *ChainedTokenCredential +} + +// NewDefaultAzureCredential creates a default credential chain for applications that will be deployed to Azure. +func NewDefaultAzureCredential(options *DefaultAzureCredentialOptions) (*DefaultAzureCredential, error) { var creds []azcore.TokenCredential errMsg := "" @@ -67,6 +75,14 @@ func NewDefaultAzureCredential(options *DefaultAzureCredentialOptions) (*Chained logCredentialError(err.credentialType, err) return nil, err } - log.Write(EventCredential, "Azure Identity => NewDefaultAzureCredential() invoking NewChainedTokenCredential()") - return NewChainedTokenCredential(creds, nil) + chain, err := NewChainedTokenCredential(creds, nil) + if err != nil { + return nil, err + } + return &DefaultAzureCredential{chain: chain}, nil +} + +// GetToken attempts to acquire a token from each of the default chain's credentials, stopping when one provides a token. +func (c *DefaultAzureCredential) GetToken(ctx context.Context, opts policy.TokenRequestOptions) (token *azcore.AccessToken, err error) { + return c.chain.GetToken(ctx, opts) } From af8771451ef75038c4410891b02f36cac3dd4f2e Mon Sep 17 00:00:00 2001 From: Charles Lowell Date: Fri, 8 Oct 2021 14:21:02 -0700 Subject: [PATCH 2/3] update tests --- sdk/azidentity/azidentity_test.go | 35 +++++++++------- .../client_secret_credential_test.go | 2 +- .../default_azure_credential_test.go | 40 +++++++------------ 3 files changed, 35 insertions(+), 42 deletions(-) diff --git a/sdk/azidentity/azidentity_test.go b/sdk/azidentity/azidentity_test.go index 1d6698f3b722..84a54280db19 100644 --- a/sdk/azidentity/azidentity_test.go +++ b/sdk/azidentity/azidentity_test.go @@ -35,25 +35,30 @@ const ( customHostString = "https://custommock.com/" ) -// Set AZURE_AUTHORITY_HOST for the duration of a test. Restore its prior value -// after the test completes. Prevents tests which set the variable from breaking live -// tests in sovereign clouds. Obviated by 1.17's T.Setenv -func setEnvAuthorityHost(host string, t *testing.T) { - originalHost := os.Getenv("AZURE_AUTHORITY_HOST") - err := os.Setenv("AZURE_AUTHORITY_HOST", host) - if err != nil { - t.Fatalf("Unexpected error setting AZURE_AUTHORITY_HOST: %v", err) +// Set environment variables for the duration of a test. Restore their prior values +// after the test completes. Obviated by 1.17's T.Setenv +func setEnvironmentVariables(t *testing.T, vars map[string]string) { + priorValues := make(map[string]string, len(vars)) + for k, v := range vars { + priorValues[k] = os.Getenv(k) + err := os.Setenv(k, v) + if err != nil { + t.Fatalf("Unexpected error setting %s: %v", k, err) + } } + t.Cleanup(func() { - err = os.Setenv("AZURE_AUTHORITY_HOST", originalHost) - if err != nil { - t.Fatalf("Unexpected error resetting AZURE_AUTHORITY_HOST: %v", err) + for k, v := range priorValues { + err := os.Setenv(k, v) + if err != nil { + t.Fatalf("Unexpected error resetting %s: %v", k, err) + } } }) } func Test_SetEnvAuthorityHost(t *testing.T) { - setEnvAuthorityHost(envHostString, t) + setEnvironmentVariables(t, map[string]string{"AZURE_AUTHORITY_HOST": envHostString}) authorityHost, err := setAuthorityHost("") if err != nil { t.Fatal(err) @@ -64,7 +69,7 @@ func Test_SetEnvAuthorityHost(t *testing.T) { } func Test_CustomAuthorityHost(t *testing.T) { - setEnvAuthorityHost(envHostString, t) + setEnvironmentVariables(t, map[string]string{"AZURE_AUTHORITY_HOST": envHostString}) authorityHost, err := setAuthorityHost(customHostString) if err != nil { t.Fatal(err) @@ -76,7 +81,7 @@ func Test_CustomAuthorityHost(t *testing.T) { } func Test_DefaultAuthorityHost(t *testing.T) { - setEnvAuthorityHost("", t) + setEnvironmentVariables(t, map[string]string{"AZURE_AUTHORITY_HOST": ""}) authorityHost, err := setAuthorityHost("") if err != nil { t.Fatal(err) @@ -87,7 +92,7 @@ func Test_DefaultAuthorityHost(t *testing.T) { } func Test_NonHTTPSAuthorityHost(t *testing.T) { - setEnvAuthorityHost("", t) + setEnvironmentVariables(t, map[string]string{"AZURE_AUTHORITY_HOST": ""}) authorityHost, err := setAuthorityHost("http://foo.com") if err == nil { t.Fatal("Expected an error but did not receive one.") diff --git a/sdk/azidentity/client_secret_credential_test.go b/sdk/azidentity/client_secret_credential_test.go index e449a12eda1a..9814b6ce5a7a 100644 --- a/sdk/azidentity/client_secret_credential_test.go +++ b/sdk/azidentity/client_secret_credential_test.go @@ -18,7 +18,7 @@ import ( const ( tenantID = "expected-tenant" badTenantID = "bad_tenant" - clientID = "expected_client" + clientID = "expected-client-id" secret = "secret" wrongSecret = "wrong_secret" tokenValue = "new_token" diff --git a/sdk/azidentity/default_azure_credential_test.go b/sdk/azidentity/default_azure_credential_test.go index 8cc5f6a3bd8c..7031a1c8dfed 100644 --- a/sdk/azidentity/default_azure_credential_test.go +++ b/sdk/azidentity/default_azure_credential_test.go @@ -4,38 +4,26 @@ package azidentity import ( - "errors" + "context" "testing" -) -const ( - lengthOfChainOneExcluded = 2 - lengthOfChainFull = 3 + "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" + "github.com/Azure/azure-sdk-for-go/sdk/internal/mock" ) -func TestDefaultAzureCredential_NilOptions(t *testing.T) { - resetEnvironmentVarsForTest() - err := initEnvironmentVarsForTest() +func TestDefaultAzureCredential_GetTokenSuccess(t *testing.T) { + env := map[string]string{"AZURE_TENANT_ID": tenantID, "AZURE_CLIENT_ID": clientID, "AZURE_CLIENT_SECRET": secret} + setEnvironmentVariables(t, env) + srv, close := mock.NewTLSServer() + defer close() + srv.AppendResponse(mock.WithBody([]byte(accessTokenRespSuccess))) + + cred, err := NewDefaultAzureCredential(&DefaultAzureCredentialOptions{AuthorityHost: AuthorityHost(srv.URL()), ClientOptions: policy.ClientOptions{Transport: srv}}) if err != nil { - t.Fatalf("Unexpected error when initializing environment variables: %v", err) + t.Fatalf("Unable to create credential. Received: %v", err) } - cred, err := NewDefaultAzureCredential(nil) + _, err = cred.GetToken(context.Background(), policy.TokenRequestOptions{Scopes: []string{"scope"}}) if err != nil { - t.Fatalf("Did not expect to receive an error in creating the credential") - } - c := newManagedIdentityClient(&ManagedIdentityCredentialOptions{}) - // if the test is running in a MSI environment then the length of sources would be two since it will include environment credential and managed identity credential - if msiType, err := c.getMSIType(); !(msiType == msiTypeUnavailable || msiType == msiTypeUnknown) { - if len(cred.sources) != lengthOfChainFull { - t.Fatalf("Length of ChainedTokenCredential sources for DefaultAzureCredential. Expected: %d, Received: %d", lengthOfChainFull, len(cred.sources)) - } - //if a credential unavailable error is received or msiType is unknown then only the environment credential will be added - } else if unavailableErr := (*CredentialUnavailableError)(nil); errors.As(err, &unavailableErr) || msiType == msiTypeUnknown { - if len(cred.sources) != lengthOfChainOneExcluded { - t.Fatalf("Length of ChainedTokenCredential sources for DefaultAzureCredential. Expected: %d, Received: %d", lengthOfChainOneExcluded, len(cred.sources)) - } - // if there is some other unexpected error then we fail here - } else if err != nil { - t.Fatalf("Received an error when trying to determine MSI type: %v", err) + t.Fatalf("GetToken error: %v", err) } } From 0a2b48c91082d51937ed9ff5228c53194195471f Mon Sep 17 00:00:00 2001 From: Charles Lowell Date: Fri, 8 Oct 2021 14:23:50 -0700 Subject: [PATCH 3/3] changelog --- sdk/azidentity/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sdk/azidentity/CHANGELOG.md b/sdk/azidentity/CHANGELOG.md index 1b29a4058557..d4b78bd6f0cb 100644 --- a/sdk/azidentity/CHANGELOG.md +++ b/sdk/azidentity/CHANGELOG.md @@ -57,6 +57,8 @@ * `AuthenticationFailedError.RawResponse()` returns the HTTP response motivating the error, if available +### Other Changes +* `NewDefaultAzureCredential()` returns `*DefaultAzureCredential` instead of `*ChainedTokenCredential` ## 0.11.0 (2021-09-08) ### Breaking Changes