diff --git a/sdk/azidentity/CHANGELOG.md b/sdk/azidentity/CHANGELOG.md index eb0d3cd27688..5cd3bdfaa08f 100644 --- a/sdk/azidentity/CHANGELOG.md +++ b/sdk/azidentity/CHANGELOG.md @@ -9,6 +9,9 @@ ### Bugs Fixed * User credential types inconsistently log access token scopes * `DefaultAzureCredential` skips managed identity in Azure Container Instances +* Credentials having optional tenant IDs such as `AzureCLICredential` and + `InteractiveBrowserCredential` require setting `AdditionallyAllowedTenants` + when used with some clients ### Other Changes * `ChainedTokenCredential` and `DefaultAzureCredential` continue to their next diff --git a/sdk/azidentity/azidentity.go b/sdk/azidentity/azidentity.go index 3ae4697e8c13..40a94154c67a 100644 --- a/sdk/azidentity/azidentity.go +++ b/sdk/azidentity/azidentity.go @@ -105,7 +105,16 @@ func resolveAdditionalTenants(tenants []string) []string { return cp } -// resolveTenant returns the correct tenant for a token request +// resolveTenant returns the correct tenant for a token request, or "" when the calling credential doesn't +// have an explicitly configured tenant and the caller didn't specify a tenant for the token request. +// +// - defaultTenant: tenant set when constructing the credential, if any. "" is valid for credentials +// having an optional or implicit tenant such as dev tool and interactive user credentials. Those +// default to the tool's configured tenant or the user's home tenant, respectively. +// - specified: tenant specified for this token request i.e., TokenRequestOptions.TenantID. May be "". +// - credName: name of the calling credential type; for error messages +// - additionalTenants: optional allow list of tenants the credential may acquire tokens from in +// addition to defaultTenant i.e., the credential's AdditionallyAllowedTenants option func resolveTenant(defaultTenant, specified, credName string, additionalTenants []string) (string, error) { if specified == "" || specified == defaultTenant { return defaultTenant, nil @@ -121,6 +130,17 @@ func resolveTenant(defaultTenant, specified, credName string, additionalTenants return specified, nil } } + if len(additionalTenants) == 0 { + switch defaultTenant { + case "", organizationsTenantID: + // The application didn't specify a tenant or allow list when constructing the credential. Allow the + // tenant specified for this token request because we have nothing to compare it to (i.e., it vacuously + // satisfies the credential's configuration); don't know whether the application is multitenant; and + // don't want to return an error in the common case that the specified tenant matches the credential's + // default tenant determined elsewhere e.g., in some dev tool's configuration. + return specified, nil + } + } return "", fmt.Errorf(`%s isn't configured to acquire tokens for tenant %q. To enable acquiring tokens for this tenant add it to the AdditionallyAllowedTenants on the credential options, or add "*" to allow acquiring tokens for any tenant`, credName, specified) } diff --git a/sdk/azidentity/azidentity_test.go b/sdk/azidentity/azidentity_test.go index 0eadd5ae2909..fabb1d7d9c8c 100644 --- a/sdk/azidentity/azidentity_test.go +++ b/sdk/azidentity/azidentity_test.go @@ -610,230 +610,322 @@ func TestAdditionallyAllowedTenants(t *testing.T) { } tenantA := "A" tenantB := "B" - for _, test := range []struct { - allowed []string - desc, expected, tenant string - err bool + type testCase struct { + allowed []string + desc, expected, ctorTenant, tokenRequestTenant string + err bool + } + cases := []testCase{ + { + desc: "all tenants allowed", + allowed: []string{"*"}, + expected: tenantA, + ctorTenant: fakeTenantID, + tokenRequestTenant: tenantA, + }, + { + desc: "tenant explicitly allowed", + allowed: []string{tenantA, tenantB}, + ctorTenant: fakeTenantID, + expected: tenantA, + tokenRequestTenant: tenantA, + }, + { + desc: "tenant explicitly allowed", + allowed: []string{tenantA, tenantB}, + ctorTenant: fakeTenantID, + expected: tenantB, + tokenRequestTenant: tenantB, + }, + { + desc: "tenant not allowed", + allowed: []string{tenantA}, + ctorTenant: fakeTenantID, + tokenRequestTenant: tenantB, + err: true, + }, + } + for _, credential := range []struct { + ctor func(azcore.ClientOptions, testCase, *testing.T) (azcore.TokenCredential, error) + env func(testCase) map[string]string + name string + tenantOptional bool }{ { - desc: "all tenants allowed", - allowed: []string{"*"}, - expected: tenantA, - tenant: tenantA, + name: credNameAssertion, + ctor: func(co azcore.ClientOptions, tc testCase, _ *testing.T) (azcore.TokenCredential, error) { + o := ClientAssertionCredentialOptions{AdditionallyAllowedTenants: tc.allowed, ClientOptions: co} + return NewClientAssertionCredential(tc.ctorTenant, fakeClientID, func(context.Context) (string, error) { return "...", nil }, &o) + }, }, { - desc: "tenant explicitly allowed", - allowed: []string{tenantA, tenantB}, - expected: tenantA, - tenant: tenantA, + name: credNameAzureCLI, + ctor: func(_ azcore.ClientOptions, tc testCase, t *testing.T) (azcore.TokenCredential, error) { + o := AzureCLICredentialOptions{ + AdditionallyAllowedTenants: tc.allowed, + TenantID: tc.ctorTenant, + tokenProvider: func(ctx context.Context, scopes []string, tenant, subscription string) ([]byte, error) { + require.Equal(t, tc.expected, tenant) + return mockAzTokenProviderSuccess(ctx, scopes, tenant, subscription) + }, + } + return NewAzureCLICredential(&o) + }, + tenantOptional: true, }, { - desc: "tenant explicitly allowed", - allowed: []string{tenantA, tenantB}, - expected: tenantB, - tenant: tenantB, + name: credNameAzureDeveloperCLI, + ctor: func(_ azcore.ClientOptions, tc testCase, t *testing.T) (azcore.TokenCredential, error) { + o := AzureDeveloperCLICredentialOptions{ + AdditionallyAllowedTenants: tc.allowed, + tokenProvider: func(ctx context.Context, scopes []string, tenant string) ([]byte, error) { + if tenant != tc.expected { + t.Errorf("unexpected tenantID %q", tenant) + } + return mockAzdTokenProviderSuccess(ctx, scopes, tenant) + }, + } + return NewAzureDeveloperCLICredential(&o) + }, + tenantOptional: true, }, { - desc: "tenant not allowed", - allowed: []string{tenantA}, - tenant: tenantB, - err: true, + name: credNameAzurePipelines, + ctor: func(co azcore.ClientOptions, tc testCase, _ *testing.T) (azcore.TokenCredential, error) { + o := AzurePipelinesCredentialOptions{AdditionallyAllowedTenants: tc.allowed, ClientOptions: co} + return NewAzurePipelinesCredential(tc.ctorTenant, fakeClientID, "service-connection", tokenValue, &o) + }, + env: func(testCase) map[string]string { + return map[string]string{systemOIDCRequestURI: "https://localhost/instance"} + }, }, { - desc: "no additional tenants allowed", - tenant: tenantA, - err: true, + name: credNameCert, + ctor: func(co azcore.ClientOptions, tc testCase, _ *testing.T) (azcore.TokenCredential, error) { + o := ClientCertificateCredentialOptions{AdditionallyAllowedTenants: tc.allowed, ClientOptions: co} + return NewClientCertificateCredential(tc.ctorTenant, fakeClientID, allCertTests[0].certs, allCertTests[0].key, &o) + }, }, - } { - tro := policy.TokenRequestOptions{Scopes: []string{liveTestScope}, TenantID: test.tenant} - for _, subtest := range []struct { - ctor func(azcore.ClientOptions) (azcore.TokenCredential, error) - env map[string]string - name string - }{ - { - name: credNameAssertion, - ctor: func(co azcore.ClientOptions) (azcore.TokenCredential, error) { - o := ClientAssertionCredentialOptions{AdditionallyAllowedTenants: test.allowed, ClientOptions: co} - return NewClientAssertionCredential(fakeTenantID, fakeClientID, func(context.Context) (string, error) { return "...", nil }, &o) - }, + { + name: credNameDeviceCode, + ctor: func(co azcore.ClientOptions, tc testCase, _ *testing.T) (azcore.TokenCredential, error) { + o := DeviceCodeCredentialOptions{ + AdditionallyAllowedTenants: tc.allowed, + ClientOptions: co, + UserPrompt: func(context.Context, DeviceCodeMessage) error { return nil }, + } + return NewDeviceCodeCredential(&o) }, - { - name: credNameAzureCLI, - ctor: func(azcore.ClientOptions) (azcore.TokenCredential, error) { - o := AzureCLICredentialOptions{ - AdditionallyAllowedTenants: test.allowed, - tokenProvider: func(ctx context.Context, scopes []string, tenant, subscription string) ([]byte, error) { - if tenant != test.expected { - t.Errorf(`unexpected tenantID "%s"`, tenant) - } - return mockAzTokenProviderSuccess(ctx, scopes, tenant, subscription) - }, - } - return NewAzureCLICredential(&o) - }, + tenantOptional: true, + }, + { + name: credNameOBO, + ctor: func(co azcore.ClientOptions, tc testCase, _ *testing.T) (azcore.TokenCredential, error) { + o := OnBehalfOfCredentialOptions{ + AdditionallyAllowedTenants: tc.allowed, + ClientOptions: co, + } + return NewOnBehalfOfCredentialWithSecret(tc.ctorTenant, fakeClientID, "assertion", fakeSecret, &o) }, - { - name: credNameAzureDeveloperCLI, - ctor: func(azcore.ClientOptions) (azcore.TokenCredential, error) { - o := AzureDeveloperCLICredentialOptions{ - AdditionallyAllowedTenants: test.allowed, - tokenProvider: func(ctx context.Context, scopes []string, tenant string) ([]byte, error) { - if tenant != test.expected { - t.Errorf("unexpected tenantID %q", tenant) - } - return mockAzdTokenProviderSuccess(ctx, scopes, tenant) - }, - } - return NewAzureDeveloperCLICredential(&o) - }, + }, + { + name: credNameSecret, + ctor: func(co azcore.ClientOptions, tc testCase, _ *testing.T) (azcore.TokenCredential, error) { + o := ClientSecretCredentialOptions{AdditionallyAllowedTenants: tc.allowed, ClientOptions: co} + return NewClientSecretCredential(tc.ctorTenant, fakeClientID, fakeSecret, &o) }, - { - name: credNameAzurePipelines, - ctor: func(co azcore.ClientOptions) (azcore.TokenCredential, error) { - o := AzurePipelinesCredentialOptions{AdditionallyAllowedTenants: test.allowed, ClientOptions: co} - return NewAzurePipelinesCredential(fakeTenantID, fakeClientID, "service-connection", tokenValue, &o) - }, - env: map[string]string{systemOIDCRequestURI: "https://localhost/instance"}, + }, + { + name: credNameUserPassword, + ctor: func(co azcore.ClientOptions, tc testCase, _ *testing.T) (azcore.TokenCredential, error) { + o := UsernamePasswordCredentialOptions{AdditionallyAllowedTenants: tc.allowed, ClientOptions: co} + return NewUsernamePasswordCredential(tc.ctorTenant, fakeClientID, fakeUsername, "password", &o) }, - { - name: credNameCert, - ctor: func(co azcore.ClientOptions) (azcore.TokenCredential, error) { - o := ClientCertificateCredentialOptions{AdditionallyAllowedTenants: test.allowed, ClientOptions: co} - return NewClientCertificateCredential(fakeTenantID, fakeClientID, allCertTests[0].certs, allCertTests[0].key, &o) - }, + }, + { + name: credNameWorkloadIdentity, + ctor: func(co azcore.ClientOptions, tc testCase, _ *testing.T) (azcore.TokenCredential, error) { + return NewWorkloadIdentityCredential(&WorkloadIdentityCredentialOptions{ + AdditionallyAllowedTenants: tc.allowed, + ClientID: fakeClientID, + ClientOptions: co, + TenantID: tc.ctorTenant, + TokenFilePath: af, + }) }, - { - name: credNameDeviceCode, - ctor: func(co azcore.ClientOptions) (azcore.TokenCredential, error) { - o := DeviceCodeCredentialOptions{ - AdditionallyAllowedTenants: test.allowed, - ClientOptions: co, - UserPrompt: func(context.Context, DeviceCodeMessage) error { return nil }, + }, + { + name: "DefaultAzureCredential/" + credNameAzureCLI, + ctor: func(_ azcore.ClientOptions, tc testCase, t *testing.T) (azcore.TokenCredential, error) { + srv, close := mock.NewTLSServer(mock.WithTransformAllRequestsToTestServerUrl()) + t.Cleanup(close) + srv.SetResponse(mock.WithStatusCode(http.StatusBadRequest)) + + c, err := NewDefaultAzureCredential(&DefaultAzureCredentialOptions{ + AdditionallyAllowedTenants: tc.allowed, + ClientOptions: policy.ClientOptions{Transport: srv}, + }) + require.NoError(t, err) + called := false + t.Cleanup(func() { + if tc.err { + require.False(t, called) + } else { + require.True(t, called) } - return NewDeviceCodeCredential(&o) - }, - }, - { - name: credNameOBO, - ctor: func(co azcore.ClientOptions) (azcore.TokenCredential, error) { - o := OnBehalfOfCredentialOptions{ - AdditionallyAllowedTenants: test.allowed, - ClientOptions: co, + }) + for _, source := range c.chain.sources { + if c, ok := source.(*AzureCLICredential); ok { + c.opts.tokenProvider = func(ctx context.Context, scopes []string, tenant, subscription string) ([]byte, error) { + called = true + require.Equal(t, tc.expected, tenant) + return mockAzTokenProviderSuccess(ctx, scopes, tenant, subscription) + } + break } - return NewOnBehalfOfCredentialWithSecret(fakeTenantID, fakeClientID, "assertion", fakeSecret, &o) - }, - }, - { - name: credNameSecret, - ctor: func(co azcore.ClientOptions) (azcore.TokenCredential, error) { - o := ClientSecretCredentialOptions{AdditionallyAllowedTenants: test.allowed, ClientOptions: co} - return NewClientSecretCredential(fakeTenantID, fakeClientID, fakeSecret, &o) - }, + } + return c, nil }, - { - name: credNameUserPassword, - ctor: func(co azcore.ClientOptions) (azcore.TokenCredential, error) { - o := UsernamePasswordCredentialOptions{AdditionallyAllowedTenants: test.allowed, ClientOptions: co} - return NewUsernamePasswordCredential(fakeTenantID, fakeClientID, fakeUsername, "password", &o) - }, + tenantOptional: true, + }, + { + name: "DefaultAzureCredential/" + credNameAzureDeveloperCLI, + ctor: func(_ azcore.ClientOptions, tc testCase, t *testing.T) (azcore.TokenCredential, error) { + srv, close := mock.NewTLSServer(mock.WithTransformAllRequestsToTestServerUrl()) + t.Cleanup(close) + srv.SetResponse(mock.WithStatusCode(http.StatusBadRequest)) + + c, err := NewDefaultAzureCredential(&DefaultAzureCredentialOptions{ + AdditionallyAllowedTenants: tc.allowed, + ClientOptions: policy.ClientOptions{Transport: srv}, + }) + require.NoError(t, err) + called := false + t.Cleanup(func() { + if tc.err { + require.False(t, called) + } else { + require.True(t, called) + } + }) + for _, source := range c.chain.sources { + switch c := source.(type) { + case *AzureCLICredential: + c.opts.tokenProvider = func(context.Context, []string, string, string) ([]byte, error) { + return nil, newCredentialUnavailableError(credNameAzureCLI, "...") + } + case *AzureDeveloperCLICredential: + c.opts.tokenProvider = func(ctx context.Context, scopes []string, tenant string) ([]byte, error) { + called = true + require.Equal(t, tc.expected, tenant) + return mockAzdTokenProviderSuccess(ctx, scopes, tenant) + } + } + } + return c, nil }, - { - name: credNameWorkloadIdentity, - ctor: func(co azcore.ClientOptions) (azcore.TokenCredential, error) { - return NewWorkloadIdentityCredential(&WorkloadIdentityCredentialOptions{ - AdditionallyAllowedTenants: test.allowed, - ClientID: fakeClientID, - ClientOptions: co, - TenantID: fakeTenantID, - TokenFilePath: af, - }) - }, + tenantOptional: true, + }, + { + name: "DefaultAzureCredential/EnvironmentCredential", + ctor: func(co azcore.ClientOptions, tc testCase, _ *testing.T) (azcore.TokenCredential, error) { + o := DefaultAzureCredentialOptions{ClientOptions: co, TenantID: tc.tokenRequestTenant} + return NewDefaultAzureCredential(&o) }, - { - name: "DefaultAzureCredential/EnvironmentCredential", - ctor: func(co azcore.ClientOptions) (azcore.TokenCredential, error) { - o := DefaultAzureCredentialOptions{ClientOptions: co, TenantID: test.tenant} - return NewDefaultAzureCredential(&o) - }, - env: map[string]string{ - azureAdditionallyAllowedTenants: strings.Join(test.allowed, ";"), + env: func(tc testCase) map[string]string { + return map[string]string{ + azureAdditionallyAllowedTenants: strings.Join(tc.allowed, ";"), azureClientID: fakeClientID, azureClientSecret: fakeSecret, - azureTenantID: fakeTenantID, - }, + azureTenantID: tc.ctorTenant, + } }, - { - name: "DefaultAzureCredential/EnvironmentCredential/option-overrides-env", - ctor: func(co azcore.ClientOptions) (azcore.TokenCredential, error) { - o := DefaultAzureCredentialOptions{AdditionallyAllowedTenants: test.allowed, ClientOptions: co, TenantID: test.tenant} - return NewDefaultAzureCredential(&o) - }, - env: map[string]string{ - azureAdditionallyAllowedTenants: "not-" + test.tenant, + }, + { + name: "DefaultAzureCredential/EnvironmentCredential/option-overrides-env", + ctor: func(co azcore.ClientOptions, tc testCase, _ *testing.T) (azcore.TokenCredential, error) { + o := DefaultAzureCredentialOptions{AdditionallyAllowedTenants: tc.allowed, ClientOptions: co, TenantID: tc.tokenRequestTenant} + return NewDefaultAzureCredential(&o) + }, + env: func(tc testCase) map[string]string { + return map[string]string{ + azureAdditionallyAllowedTenants: "not-" + tc.tokenRequestTenant, azureClientID: fakeClientID, azureClientSecret: fakeSecret, - azureTenantID: fakeTenantID, - }, + azureTenantID: tc.ctorTenant, + } }, - { - name: "DefaultAzureCredential/" + credNameWorkloadIdentity, - ctor: func(co azcore.ClientOptions) (azcore.TokenCredential, error) { - o := DefaultAzureCredentialOptions{AdditionallyAllowedTenants: test.allowed, ClientOptions: co} - return NewDefaultAzureCredential(&o) - }, - env: map[string]string{ - azureAdditionallyAllowedTenants: strings.Join(test.allowed, ";"), + }, + { + name: "DefaultAzureCredential/" + credNameWorkloadIdentity, + ctor: func(co azcore.ClientOptions, tc testCase, _ *testing.T) (azcore.TokenCredential, error) { + o := DefaultAzureCredentialOptions{AdditionallyAllowedTenants: tc.allowed, ClientOptions: co} + return NewDefaultAzureCredential(&o) + }, + env: func(tc testCase) map[string]string { + return map[string]string{ + azureAdditionallyAllowedTenants: strings.Join(tc.allowed, ";"), azureAuthorityHost: "https://login.microsoftonline.com", azureClientID: fakeClientID, azureFederatedTokenFile: af, - azureTenantID: fakeTenantID, - }, + azureTenantID: tc.ctorTenant, + } }, - { - name: "EnvironmentCredential/" + credNameCert, - ctor: func(co azcore.ClientOptions) (azcore.TokenCredential, error) { - return NewEnvironmentCredential(&EnvironmentCredentialOptions{ClientOptions: co}) - }, - env: map[string]string{ - azureAdditionallyAllowedTenants: strings.Join(test.allowed, ";"), + }, + { + name: "EnvironmentCredential/" + credNameCert, + ctor: func(co azcore.ClientOptions, _ testCase, _ *testing.T) (azcore.TokenCredential, error) { + return NewEnvironmentCredential(&EnvironmentCredentialOptions{ClientOptions: co}) + }, + env: func(tc testCase) map[string]string { + return map[string]string{ + azureAdditionallyAllowedTenants: strings.Join(tc.allowed, ";"), azureClientCertificatePath: "testdata/certificate.pem", azureClientID: fakeClientID, - azureTenantID: fakeTenantID, - }, + azureTenantID: tc.ctorTenant, + } }, - { - name: "EnvironmentCredential/" + credNameSecret, - ctor: func(co azcore.ClientOptions) (azcore.TokenCredential, error) { - return NewEnvironmentCredential(&EnvironmentCredentialOptions{ClientOptions: co}) - }, - env: map[string]string{ - azureAdditionallyAllowedTenants: strings.Join(test.allowed, ";"), + }, + { + name: "EnvironmentCredential/" + credNameSecret, + ctor: func(co azcore.ClientOptions, tc testCase, _ *testing.T) (azcore.TokenCredential, error) { + return NewEnvironmentCredential(&EnvironmentCredentialOptions{ClientOptions: co}) + }, + env: func(tc testCase) map[string]string { + return map[string]string{ + azureAdditionallyAllowedTenants: strings.Join(tc.allowed, ";"), azureClientID: fakeClientID, azureClientSecret: fakeSecret, - azureTenantID: fakeTenantID, - }, + azureTenantID: tc.ctorTenant, + } }, - { - name: "EnvironmentCredential/" + credNameUserPassword, - ctor: func(co azcore.ClientOptions) (azcore.TokenCredential, error) { - return NewEnvironmentCredential(&EnvironmentCredentialOptions{ClientOptions: co}) - }, - env: map[string]string{ - azureAdditionallyAllowedTenants: strings.Join(test.allowed, ";"), + }, + { + name: "EnvironmentCredential/" + credNameUserPassword, + ctor: func(co azcore.ClientOptions, tc testCase, _ *testing.T) (azcore.TokenCredential, error) { + return NewEnvironmentCredential(&EnvironmentCredentialOptions{ClientOptions: co}) + }, + env: func(tc testCase) map[string]string { + return map[string]string{ + azureAdditionallyAllowedTenants: strings.Join(tc.allowed, ";"), azureClientID: fakeClientID, azurePassword: "password", - azureTenantID: fakeTenantID, + azureTenantID: tc.ctorTenant, azureUsername: fakeUsername, - }, + } }, - } { - t.Run(fmt.Sprintf("%s/%s", subtest.name, test.desc), func(t *testing.T) { - for k, v := range subtest.env { - t.Setenv(k, v) + }, + } { + for _, test := range cases { + tro := policy.TokenRequestOptions{Scopes: []string{liveTestScope}, TenantID: test.tokenRequestTenant} + t.Run(fmt.Sprintf("%s/%s", credential.name, test.desc), func(t *testing.T) { + if credential.env != nil { + for k, v := range credential.env(test) { + t.Setenv(k, v) + } } sts := mockSTS{ - tenant: test.tenant, + tenant: test.tokenRequestTenant, tokenRequestCallback: func(r *http.Request) *http.Response { if actual := strings.Split(r.URL.Path, "/")[1]; actual != test.expected { t.Fatalf("expected tenant %q, got %q", test.expected, actual) @@ -841,7 +933,7 @@ func TestAdditionallyAllowedTenants(t *testing.T) { return nil }, } - c, err := subtest.ctor(policy.ClientOptions{Transport: &sts}) + c, err := credential.ctor(policy.ClientOptions{Transport: &sts}, test, t) if err != nil { t.Fatal(err) } @@ -868,6 +960,36 @@ func TestAdditionallyAllowedTenants(t *testing.T) { }) } + if credential.tenantOptional { + t.Run(credential.name+"/no tenant specified", func(t *testing.T) { + tc := testCase{tokenRequestTenant: tenantA, expected: tenantA} + c, err := credential.ctor(policy.ClientOptions{Transport: &mockSTS{}}, tc, t) + require.NoError(t, err) + _, err = c.GetToken(ctx, policy.TokenRequestOptions{Scopes: []string{liveTestScope}, TenantID: tc.tokenRequestTenant}) + if tc.err { + require.Error(t, err) + } else { + require.NoError(t, err) + } + }) + } else { + t.Run(credential.name+"/no additionally allowed tenants", func(t *testing.T) { + tc := testCase{err: true, ctorTenant: fakeTenantID, tokenRequestTenant: tenantA} + if credential.env != nil { + for k, v := range credential.env(tc) { + t.Setenv(k, v) + } + } + c, err := credential.ctor(policy.ClientOptions{Transport: &mockSTS{}}, tc, t) + require.NoError(t, err) + _, err = c.GetToken(ctx, policy.TokenRequestOptions{Scopes: []string{liveTestScope}, TenantID: tc.tokenRequestTenant}) + require.Error(t, err) + }) + } + } + + for _, test := range cases { + tro := policy.TokenRequestOptions{Scopes: []string{liveTestScope}, TenantID: test.tokenRequestTenant} t.Run(credNameBrowser, func(t *testing.T) { c, err := NewInteractiveBrowserCredential(&InteractiveBrowserCredentialOptions{ AdditionallyAllowedTenants: test.allowed, @@ -887,65 +1009,6 @@ func TestAdditionallyAllowedTenants(t *testing.T) { require.ErrorAs(t, err, &e) } }) - - for _, credName := range []string{credNameAzureCLI, credNameAzureDeveloperCLI} { - t.Run(fmt.Sprintf("DefaultAzureCredential/%s/%s", credName, test.desc), func(t *testing.T) { - typeName := fmt.Sprintf("%T", &AzureCLICredential{}) - if credName == credNameAzureDeveloperCLI { - typeName = fmt.Sprintf("%T", &AzureDeveloperCLICredential{}) - } - called := false - verifyTenant := func(tenant string) { - called = true - if tenant != test.expected { - t.Fatalf("unexpected tenant %q", tenant) - } - } - - // mock IMDS failure because managed identity precedes CLI in the chain - srv, close := mock.NewTLSServer(mock.WithTransformAllRequestsToTestServerUrl()) - defer close() - srv.SetResponse(mock.WithStatusCode(400)) - o := DefaultAzureCredentialOptions{ - AdditionallyAllowedTenants: test.allowed, - ClientOptions: policy.ClientOptions{Transport: srv}, - } - - c, err := NewDefaultAzureCredential(&o) - if err != nil { - t.Fatal(err) - } - for _, source := range c.chain.sources { - if fmt.Sprintf("%T", source) != typeName { - continue - } - switch c := source.(type) { - case *AzureCLICredential: - c.opts.tokenProvider = func(ctx context.Context, scopes []string, tenant, subscription string) ([]byte, error) { - verifyTenant(tenant) - return mockAzTokenProviderSuccess(ctx, scopes, tenant, subscription) - } - case *AzureDeveloperCLICredential: - c.opts.tokenProvider = func(ctx context.Context, scopes []string, tenant string) ([]byte, error) { - verifyTenant(tenant) - return mockAzdTokenProviderSuccess(ctx, scopes, tenant) - } - } - if _, err := c.GetToken(context.Background(), tro); err != nil { - if test.err { - return - } - t.Fatal(err) - } else if test.err { - t.Fatal("expected an error") - } - if !called { - t.Fatalf("%s wasn't invoked", credName) - } - break - } - }) - } } } @@ -1181,6 +1244,18 @@ func TestResolveTenant(t *testing.T) { } }) } + + t.Run("empty default tenant", func(t *testing.T) { + expected := "expected" + for _, additionalTenants := range [][]string{nil, {"*"}, {expected}} { + actual, err := resolveTenant("", expected, credName, additionalTenants) + require.NoError(t, err, "the specified tenant should be allowed given additionalTenants %v", additionalTenants) + require.Equal(t, expected, actual) + } + actual, err := resolveTenant("", expected, credName, []string{"un" + expected}) + require.Error(t, err, "the specified tenant shouldn't be allowed because additionalTenants isn't empty and doesn't contain it") + require.Empty(t, actual) + }) } func TestConfidentialClientPersistentCache(t *testing.T) { diff --git a/sdk/azidentity/azure_cli_credential.go b/sdk/azidentity/azure_cli_credential.go index b9976f5fedee..e2f371cfd87d 100644 --- a/sdk/azidentity/azure_cli_credential.go +++ b/sdk/azidentity/azure_cli_credential.go @@ -30,9 +30,9 @@ type azTokenProvider func(ctx context.Context, scopes []string, tenant, subscrip // AzureCLICredentialOptions contains optional parameters for AzureCLICredential. type AzureCLICredentialOptions struct { - // AdditionallyAllowedTenants specifies tenants for which the credential may acquire tokens, in addition - // to TenantID. Add the wildcard value "*" to allow the credential to acquire tokens for any tenant the - // logged in account can access. + // AdditionallyAllowedTenants specifies tenants to which the credential may authenticate, in addition to + // TenantID. When TenantID is empty, this option has no effect and the credential will authenticate to + // any requested tenant. Add the wildcard value "*" to allow the credential to authenticate to any tenant. AdditionallyAllowedTenants []string // Subscription is the name or ID of a subscription. Set this to acquire tokens for an account other diff --git a/sdk/azidentity/azure_developer_cli_credential.go b/sdk/azidentity/azure_developer_cli_credential.go index cbe7c4c2db1f..46d0b5519222 100644 --- a/sdk/azidentity/azure_developer_cli_credential.go +++ b/sdk/azidentity/azure_developer_cli_credential.go @@ -30,9 +30,9 @@ type azdTokenProvider func(ctx context.Context, scopes []string, tenant string) // AzureDeveloperCLICredentialOptions contains optional parameters for AzureDeveloperCLICredential. type AzureDeveloperCLICredentialOptions struct { - // AdditionallyAllowedTenants specifies tenants for which the credential may acquire tokens, in addition - // to TenantID. Add the wildcard value "*" to allow the credential to acquire tokens for any tenant the - // logged in account can access. + // AdditionallyAllowedTenants specifies tenants to which the credential may authenticate, in addition to + // TenantID. When TenantID is empty, this option has no effect and the credential will authenticate to + // any requested tenant. Add the wildcard value "*" to allow the credential to authenticate to any tenant. AdditionallyAllowedTenants []string // TenantID identifies the tenant the credential should authenticate in. Defaults to the azd environment, diff --git a/sdk/azidentity/default_azure_credential.go b/sdk/azidentity/default_azure_credential.go index 4f641b6902c3..14af271f6a1f 100644 --- a/sdk/azidentity/default_azure_credential.go +++ b/sdk/azidentity/default_azure_credential.go @@ -23,15 +23,19 @@ type DefaultAzureCredentialOptions struct { // to credential types that authenticate via external tools such as the Azure CLI. azcore.ClientOptions - // AdditionallyAllowedTenants specifies additional tenants for which the credential may acquire tokens. Add - // the wildcard value "*" to allow the credential to acquire tokens for any tenant. This value can also be - // set as a semicolon delimited list of tenants in the environment variable AZURE_ADDITIONALLY_ALLOWED_TENANTS. + // AdditionallyAllowedTenants specifies tenants to which the credential may authenticate, in addition to + // TenantID. When TenantID is empty, this option has no effect and the credential will authenticate to + // any requested tenant. Add the wildcard value "*" to allow the credential to authenticate to any tenant. + // This value can also be set as a semicolon delimited list of tenants in the environment variable + // AZURE_ADDITIONALLY_ALLOWED_TENANTS. AdditionallyAllowedTenants []string + // DisableInstanceDiscovery should be set true only by applications authenticating in disconnected clouds, or // private clouds such as Azure Stack. It determines whether the credential requests Microsoft Entra instance metadata // from https://login.microsoft.com before authenticating. Setting this to true will skip this request, making // the application responsible for ensuring the configured authority is valid and trustworthy. DisableInstanceDiscovery bool + // TenantID sets the default tenant for authentication via the Azure CLI and workload identity. TenantID string } diff --git a/sdk/azidentity/device_code_credential.go b/sdk/azidentity/device_code_credential.go index 53c4c728735f..53ae9767f425 100644 --- a/sdk/azidentity/device_code_credential.go +++ b/sdk/azidentity/device_code_credential.go @@ -21,8 +21,9 @@ const credNameDeviceCode = "DeviceCodeCredential" type DeviceCodeCredentialOptions struct { azcore.ClientOptions - // AdditionallyAllowedTenants specifies additional tenants for which the credential may acquire - // tokens. Add the wildcard value "*" to allow the credential to acquire tokens for any tenant. + // AdditionallyAllowedTenants specifies tenants to which the credential may authenticate, in addition to + // TenantID. When TenantID is empty, this option has no effect and the credential will authenticate to + // any requested tenant. Add the wildcard value "*" to allow the credential to authenticate to any tenant. AdditionallyAllowedTenants []string // AuthenticationRecord returned by a call to a credential's Authenticate method. Set this option diff --git a/sdk/azidentity/interactive_browser_credential.go b/sdk/azidentity/interactive_browser_credential.go index 848db16e4350..ec89de9b5b23 100644 --- a/sdk/azidentity/interactive_browser_credential.go +++ b/sdk/azidentity/interactive_browser_credential.go @@ -20,8 +20,9 @@ const credNameBrowser = "InteractiveBrowserCredential" type InteractiveBrowserCredentialOptions struct { azcore.ClientOptions - // AdditionallyAllowedTenants specifies additional tenants for which the credential may acquire - // tokens. Add the wildcard value "*" to allow the credential to acquire tokens for any tenant. + // AdditionallyAllowedTenants specifies tenants to which the credential may authenticate, in addition to + // TenantID. When TenantID is empty, this option has no effect and the credential will authenticate to + // any requested tenant. Add the wildcard value "*" to allow the credential to authenticate to any tenant. AdditionallyAllowedTenants []string // AuthenticationRecord returned by a call to a credential's Authenticate method. Set this option