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

Juraj2 · 2023-11-15T20:59:30Z

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

The text was updated successfully, but these errors were encountered:

…() instead of hardcoded DefaultAzureCredential() to read values mapped from KeyVault This is fix for issue aspnet#230 Background: In order for AzureAppConfigurationBuilder to read Azure App Configuration values mapped from KeyVault, it gets the TokenCredential by calling `private SecretClient GetSecretClient()`. But GetSecretClient() has hardcoded way to get `TokenCredential` by calling `new DefaultAzureCredential()` but it should honor an already existing virtual method `GetCredential()` instead to get the `TokenCredential`. So the classes that inherit from `AzureAppConfigurationBuilder` and override the `protected virtual TokenCredential GetCredential()` can influence which credential is used when reading app configuration values that are mapped from KeyVault. The bug is that classes that inherit from `AzureAppConfigurationBuilder` and override the `protected virtual TokenCredential GetCredential()` cannot influence which credential is used when reading app configuration values that are mapped from KeyVault.

…spnet#230) So reading of App Configuration Key-value references to Key Vault can be fine tuned by the derived classes.

…tClientOptions in AzureAppConfigurationBuilder (aspnet#230) - Fix for AzureAppConfigurationBuilder to read App Configuration Key-value references to Key Vault in the same way as AzureKeyVaultConfigBuilder Addresses aspnet#230

To spacing what was prior aspnet#230

…onBuilder (aspnet#230)

skwazza09 · 2023-11-28T19:21:25Z

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 · 2023-12-14T18:05:07Z

Any progress made on this issue?

Juraj2 · 2023-12-15T07:08:19Z

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

Juraj2 changed the title ~~AzureAppConfigurationBuilder always uses DefaultAzureCredentail() for SecretClient but should use virtual GetCredential() instead~~ AzureAppConfigurationBuilder always uses DefaultAzureCredential() to read Key Vault but should use existing GetCredential() Nov 16, 2023

Juraj2 mentioned this issue Nov 16, 2023

Fix for Key Vault reading in AzureAppConfigurationBuilder (#230) #232

Open

Juraj2 changed the title ~~AzureAppConfigurationBuilder always uses DefaultAzureCredential() to read Key Vault but should use existing GetCredential()~~ AzureAppConfigurationBuilder uses hardcoded DefaultAzureCredential() to read Key Vault but should use existing GetCredential() Nov 16, 2023

Juraj2 pushed a commit to Juraj2/MicrosoftConfigurationBuilders that referenced this issue Nov 17, 2023

Clean up spacing (aspnet#230)

ba0ce66

To spacing what was prior aspnet#230

Juraj2 pushed a commit to Juraj2/MicrosoftConfigurationBuilders that referenced this issue Nov 27, 2023

Fixing comments in AzureKeyVaultConfigBuilder and AzureAppConfigurati…

7d04b31

…onBuilder (aspnet#230)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

Juraj2 commented Nov 15, 2023 •

edited

skwazza09 commented Nov 28, 2023 •

edited

masonhuemmer commented Dec 14, 2023 •

edited

Juraj2 commented Dec 15, 2023 •

edited

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

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

Comments

Juraj2 commented Nov 15, 2023 • edited