aspnet / MicrosoftConfigurationBuilders

Microsoft.Configuration.Builders
MIT License
118 stars 61 forks source link

AzureAppConfigurationBuilder uses hardcoded DefaultAzureCredential() to read Key Vault but should use existing GetCredential() #230

Open Juraj2 opened 10 months ago

Juraj2 commented 10 months ago

AzureAppConfigurationBuilder uses hardcoded DefaultAzureCredential() to read Key Vault but should use existing GetCredential()

AzureAppConfigurationBuilder.cs always uses DefaultAzureCredential() when reading App Configuration Key-value references to Key Vault. It should use already existing virtual method GetCredential() instead.

Functional impact

Classes that inherit from AzureAppConfigurationBuilder and override the protected virtual TokenCredential GetCredential() still cannot influence which TokenCredential is used when reading App Configuration values that are referencing Key Vault.

Expected result

When classes that inherit from AzureAppConfigurationBuilder override the protected virtual TokenCredential GetCredential() then the GetCredential() should also be used for App Configuration Key-value references to Key Vault

Actual result

When classes that inherit from AzureAppConfigurationBuilder override the protected virtual TokenCredential GetCredential() then the GetCredential() is only used to read Key-values from App Configuration. But when reading App Configuration Key-value references that reference Key Vault then always the hardcoded new DefaultAzureCredential() is used.

Further technical details

There is a bug in the code in AzureAppConfigurationBuilder.cs in private SecretClient GetSecretClient() method. It should use already existing virtual method GetCredential() instead of hardcoded new DefaultAzureCredential()- the same way as it is used in AzureKeyVaultConfigBuilder.cs

skwazza09 commented 9 months ago

Facing the same issue with AzureAppConfigurationBuilder. Able to load the AppConfig values just fine, however when reading App Configuration Key-value references that reference Key Vault it always uses the default credential which throws a Azure.RequestFailedException.

We are using a CheinedTokenCredential to gain access to the Azure Resource, but unable to use the overridden GetCredential() method as it defaults to the DefaultAzureCredential().

masonhuemmer commented 9 months ago

Any progress made on this issue?

Juraj2 commented 9 months ago

I implemented the fix for it. I am waiting for the pull request approval so it gets to the next release.