azsecrets v1.3.0 didn't respect home tenant id configured (login-ed) in az cli

[xuxife]

Bug Report

azsecrets v1.3.0 didn't work with azidentity.NewAzureCLICredential(nil), it didn't use the default tenant id login-ed in my az cli.
Error:

panic: AzureCLICredential isn't configured to acquire tokens for tenant "72f988bf-86f1-41af-91ab-2d7cd011db47". 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

I'm using single tenant, didn't use cross tenant keyvault scenario.

---

Minimal repro code:
go.mod

module github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets/bug

go 1.23.3

require (
	github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.8.0
	github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets v1.3.0
)
...

main.go

package main

import (
	"context"
	"fmt"

	"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
	"github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets"
)

func main() {
	cred, err := azidentity.NewAzureCLICredential(nil)
	if err != nil {
		panic(err)
	}
	secretsClient, err := azsecrets.NewClient("https://<SOME VAULT IN TENANT X>.vault.azure.net/", cred, nil)
	if err != nil {
		panic(err)
	}
	resp, err := secretsClient.GetSecret(context.Background(), "<SECRET NAME>", "", nil)
	if err != nil {
		panic(err)
	}
	fmt.Println(*resp.Value)
}

az account show

{
  "environmentName": "AzureCloud",
  "homeTenantId": "72f988bf-86f1-41af-91ab-2d7cd011db47", // TENANT X
  "isDefault": true,
  "managedByTenants": [],
  "state": "Enabled",
  "tenantDefaultDomain": "microsoft.onmicrosoft.com",
  "tenantDisplayName": "Microsoft",
  "tenantId": "72f988bf-86f1-41af-91ab-2d7cd011db47", // TENANT X
  "user": {
    "name": "<ME>@microsoft.com",
    "type": "user"
  }
}

• import path of package in question: github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets
• SDK version e.g. 1.3.0
  • Specify the exact commit if possible; one way to get this is the REVISION
    column output by go list -m <module>, for example go list -m github.com/Azure/azure-sdk-for-go/sdk/azcore.
• output of go version

• What happened?

I cannot authenticate with azsecrets v1.3.0 with azidentity.NewAzureCLICredential(nil)

• What did you expect or want to happen?

Expect it should use default tenant id I logined in az cli, just like comments in azidentity.AzureCLICredentialOptions

// 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 []string

	// Subscription is the name or ID of a subscription. Set this to acquire tokens for an account other
	// than the Azure CLI's current account.
	Subscription string

	// TenantID identifies the tenant the credential should authenticate in.
	// Defaults to the CLI's default tenant, which is typically the home tenant of the logged in user.
	TenantID string
...
}

• How can we reproduce it?

repro code provided

• Anything we should know about your environment.

[github-actions]

Thank you for your feedback. Tagging and routing to the team member best able to assist.

[gracewilcox]

Hi @xuxife! I want to make sure I can understand and isolate your issue. If you roll back to the previous version of azsecrets (v1.2.0), do you still experience this issue?

Are you running this code in the same CLI that you ran the az account show command? That's an Azure CLI command, so it's also possible that your code is not running on your Azure CLI.

Where exactly is your code running? What is your precise environment with the version? If it is running on the Azure CLI, what are the results of running az login? Or alternatively if your code is running on PowerShell, what are the results of running Connect-AzAccount?

[github-actions]

Hi @xuxife. Thank you for opening this issue and giving us the opportunity to assist. To help our team better understand your issue and the details of your scenario please provide a response to the question asked above or the information requested above. This will help us more accurately address your issue.

[bcho]

@gracewilcox hi, the issue doesn't reproduce in verison prior to v1.2.0 . We believe the problem is related to the code here:

azure-sdk-for-go/sdk/azidentity/azidentity.go

Lines 109 to 124 in c5e7bb9

	func resolveTenant(defaultTenant, specified, credName string, additionalTenants []string) (string, error) {
	if specified == "" || specified == defaultTenant {
	return defaultTenant, nil
	}
	if defaultTenant == "adfs" {
	return "", errors.New("ADFS doesn't support tenants")
	}
	if !validTenantID(specified) {
	return "", errInvalidTenantID
	}
	for _, t := range additionalTenants {
	if t == "*" || t == specified {
	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)

If we use "nil" for the cli credentials option object, an empty tenant id will be set in here:

azure-sdk-for-go/sdk/azidentity/azure_cli_credential.go

Lines 42 to 44 in c5e7bb9

	// TenantID identifies the tenant the credential should authenticate in.
	// Defaults to the CLI's default tenant, which is typically the home tenant of the logged in user.
	TenantID string

Prior to v1.2.0, the akv's TokenRequestOptions (tro) object is not being back-filled with the tenant id, hence the tenant id value in tro and cli credentials are both empty. In this case, the resolveTenant will assume the tenant is unspecefied and allow continue invoking the az cli for acquiring the token.

However, the v1.2.0 change back-filles the non-emptuy tenant id from the challenge request to the tro. In this case, the resolveTenant assume the default set tenant id does not match the requested tenant, hence error out in the last part of the funciton. But, giving the az cli credentials object is allowed to have an optional tenant id (which it should infer from the az cli), and the actual tenant from az cli matches akv tenant, we should not error out in this settings.

[gracewilcox]

Hi @bcho! Thank you for the detailed information. We'll look into fixing the issue.

[chlowell]

Thanks for opening this issue! This behavior is actually a security feature. In general, we don't want credentials to authenticate in arbitrary tenants by default because doing so can enable unintentional resource access. The Azure CLI case is awkward because azidentity doesn't know the CLI's default tenant. This means that when you don't specify a tenant in the credential options, the credential has no way to tell whether a token request is for an unexpected tenant. We might be able to make this a little easier, but it will take some careful thought because we don't want to compromise any application's security posture. In the meantime, here are a couple ways to get your application working:

Option 1: specify a tenant for AzureCLICredential

If you only want to access a single tenant, you can specify it when constructing the credential:

cred, err := azidentity.NewAzureCLICredential(&azidentity.AzureCLICredentialOptions{
    TenantID: "72f988bf-86f1-41af-91ab-2d7cd011db47",
})

Option 2: set AdditionallyAllowedTenants

You can specify allowed tenants in the credential options:

cred, err := azidentity.NewAzureCLICredential(&azidentity.AzureCLICredentialOptions{
    AdditionallyAllowedTenants: []string{"72f988bf-86f1-41af-91ab-2d7cd011db47"},
})

If you don't know all the tenants you need to access, you can allow all tenants with a wildcard:

cred, err := azidentity.NewAzureCLICredential(&azidentity.AzureCLICredentialOptions{
    AdditionallyAllowedTenants: []string{"*"},
})

[michvllni]

Thanks for opening this issue! This behavior is actually a security feature. In general, we don't want credentials to authenticate in arbitrary tenants by default because doing so can enable unintentional resource access. The Azure CLI case is awkward because azidentity doesn't know the CLI's default tenant. This means that when you don't specify a tenant in the credential options, the credential has no way to tell whether a token request is for an unexpected tenant. We might be able to make this a little easier, but it will take some careful thought because we don't want to compromise any application's security posture. In the meantime, here are a couple ways to get your application working:

Option 1: specify a tenant for AzureCLICredential

If you only want to access a single tenant, you can specify it when constructing the credential:

cred, err := azidentity.NewAzureCLICredential(&azidentity.AzureCLICredentialOptions{
TenantID: "72f988bf-86f1-41af-91ab-2d7cd011db47",
})

Option 2: set AdditionallyAllowedTenants

You can specify allowed tenants in the credential options:

cred, err := azidentity.NewAzureCLICredential(&azidentity.AzureCLICredentialOptions{
AdditionallyAllowedTenants: []string{"72f988bf-86f1-41af-91ab-2d7cd011db47"},
})
If you don't know all the tenants you need to access, you can allow all tenants with a wildcard:

cred, err := azidentity.NewAzureCLICredential(&azidentity.AzureCLICredentialOptions{
AdditionallyAllowedTenants: []string{"*"},
})

While I can understand your intentions, this is still a breaking change and should be treated as such. Hiding it in a minor causes unneccessary pain.

To make for a smoother transition, you could set the wildcard as default like in your option 2 and emit a warning that will become an error in the future.

[bcho]

@chlowell is that possible to perform an extra call az account show to retrieve the tenant id when the value is not set? This also matches the behavior of the comment here

azure-sdk-for-go/sdk/azidentity/azure_cli_credential.go

Lines 42 to 44 in c5e7bb9

	// TenantID identifies the tenant the credential should authenticate in.
	// Defaults to the CLI's default tenant, which is typically the home tenant of the logged in user.
	TenantID string

[chlowell]

is that possible to perform an extra call az account show to retrieve the tenant id when the value is not set?

It's possible, but there's a couple problems with this approach:

1. dependens on the output format of az account show. I don't know whether CLI guarantees its compatibility across versions
2. adds an additional CLI invocation requiring at least hundreds of ms; can take several seconds

I suppose we can answer the compatibility question but there's no avoiding the additional CLI invocation, which would manifest to apps as a significant perf regression. AzureCLICredential already takes a relatively long time to acquire a token; how would you feel about it taking twice as long?

[bcho]

Agree on the concern of the perf regression. I ran a simple test from my local, a basic az cli invocation takes ~200ms (for account show and get-access-token if local token cache hits)

Giving the akv case is requesting the token using a separated scope, which is likely not hitting the cache at all, in this case, the get-access-token call takes 1 ~ 2s to finish the full token flow in my end. Hence, adding a 200ms call seems to be fine here. And I would expect the az cli token usage is local only, which is also acceptable to have a 1~2s latency for the initial token acquisition here.