From 81957e4f493addebcdfa86150232dba99c0f10a0 Mon Sep 17 00:00:00 2001 From: Charles Lowell Date: Thu, 19 Aug 2021 16:44:38 -0700 Subject: [PATCH 1/2] ManagedIdentityCredential doesn't mutate scopes --- sdk/azidentity/managed_identity_credential.go | 6 ++--- .../managed_identity_credential_test.go | 25 +++++++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/sdk/azidentity/managed_identity_credential.go b/sdk/azidentity/managed_identity_credential.go index ba6deab6aa0c..251202a3e33b 100644 --- a/sdk/azidentity/managed_identity_credential.go +++ b/sdk/azidentity/managed_identity_credential.go @@ -95,9 +95,9 @@ func (c *ManagedIdentityCredential) GetToken(ctx context.Context, opts azcore.To addGetTokenFailureLogs("Managed Identity Credential", err, true) return nil, err } - // The following code will remove the /.default suffix from any scopes passed into the method since ManagedIdentityCredentials expect a resource string instead of a scope string - opts.Scopes[0] = strings.TrimSuffix(opts.Scopes[0], defaultSuffix) - tk, err := c.client.authenticate(ctx, c.id, opts.Scopes) + // managed identity endpoints require an AADv1 resource (i.e. token audience), not a v2 scope, so we remove "/.default" here + scopes := []string{strings.TrimSuffix(opts.Scopes[0], defaultSuffix)} + tk, err := c.client.authenticate(ctx, c.id, scopes) if err != nil { addGetTokenFailureLogs("Managed Identity Credential", err, true) return nil, err diff --git a/sdk/azidentity/managed_identity_credential_test.go b/sdk/azidentity/managed_identity_credential_test.go index 90352f5f6811..08750aec219a 100644 --- a/sdk/azidentity/managed_identity_credential_test.go +++ b/sdk/azidentity/managed_identity_credential_test.go @@ -512,6 +512,31 @@ func TestManagedIdentityCredential_GetTokenNilResource(t *testing.T) { } } +func TestManagedIdentityCredential_ScopesImmutable(t *testing.T) { + resetEnvironmentVarsForTest() + srv, close := mock.NewServer() + defer close() + srv.AppendResponse(mock.WithBody([]byte(expiresOnIntResp))) + _ = os.Setenv(msiEndpoint, srv.URL()) + defer clearEnvVars(msiEndpoint) + options := ManagedIdentityCredentialOptions{ + HTTPClient: srv, + } + cred, err := NewManagedIdentityCredential("", &options) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + scope := "https://localhost/.default" + scopes := []string{scope} + _, err = cred.GetToken(context.Background(), azcore.TokenRequestOptions{Scopes: scopes}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if scopes[0] != scope { + t.Fatalf("GetToken shouldn't mutate arguments") + } +} + func TestManagedIdentityCredential_GetTokenMultipleResources(t *testing.T) { resetEnvironmentVarsForTest() srv, close := mock.NewServer() From 614b21d8d32d2cfe0d2bfc509ea422c6ec583ea7 Mon Sep 17 00:00:00 2001 From: Charles Lowell Date: Thu, 19 Aug 2021 16:44:41 -0700 Subject: [PATCH 2/2] changelog --- sdk/azidentity/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/azidentity/CHANGELOG.md b/sdk/azidentity/CHANGELOG.md index 1b55d068c445..12e52785bf5c 100644 --- a/sdk/azidentity/CHANGELOG.md +++ b/sdk/azidentity/CHANGELOG.md @@ -7,6 +7,7 @@ ### Breaking Changes ### Bugs Fixed +* `ManagedIdentityCredential.GetToken` no longer mutates its `opts.Scopes` ### Other Changes