Documentation for Key Vault configuration is wrong

[heaths]

Documentation states,

https://github.com/dotnet/AspNetCore.Docs/blob/642a1e53966fe9535fbf6ae870e29ffe119a39e4/aspnetcore/security/key-vault-configuration.md?plain=1#L339

This is now how KeyVaultSecretManager is implemented currently, and based on my comments I think we should change the implementation little to none. Expired secrets may still be necessary to, say, decrypt (if a private or shared key), but I could see excluding disabled secrets since those are explicitly disabled.

/cc @JoshLove-msft

---

Document Details

⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

• ID: 2cc0b77a-df21-7007-5cec-4399d98c6dc6
• Version Independent ID: efe5ae5e-ff64-f1c7-6920-4512fc8f36d1
• Content: Azure Key Vault configuration provider in ASP.NET Core
• Content Source: aspnetcore/security/key-vault-configuration.md
• Product: aspnet-core
• Technology: aspnetcore-security
• GitHub Login: @Rick-Anderson
• Microsoft Alias: riande

[Rick-Anderson]

See Disabled and expired secrets

cc @blowdart

[heaths]

Yes, I'm stating I think the docs are wrong. I own the Key Vault SDK for .NET, and there are certainly cases where expired secrets should be loaded. Whatever the case is, the newer (~3 year old) configuration provider has been like this, and changing it now would be a breaking change. Not returning disabled secrets, however, might be worth it. That's an explicit user (likely admin) action unlike expiration.

[blowdart]

It's not a matter of should Heath, it's a matter of does your SDK do that right now, which it appears to do :)

If, as you say, your library loads expired secrets by default then fine, the docs should reflect that.

I'm not sure I agree with loading expired secrets by default (maybe that should be an sdk option, but that's off topic for docs).

Given it's your SDK @heaths you can update the docs. Might I suggest, rather than attempt to expand and justify why expired secrets get loaded just simplify the section to

Disabled secrets Disabled and secrets are excluded from the configuration provider. To include values for these secrets in app configuration reenable the disabled secret and restart your application.

[Rick-Anderson]

@heaths can you update the doc or should I close this issue? cc @JoshLove-msft

[HomerBart]

Hi @heaths, I think the documentation is still wrong. It states that Disabled and expired secrets are included by default in the configuration provider. Including disabled secrets doesn't make sense to me, so I tested it today and I found that disabled secrets are not actually included. I am using .NET 7, with version 1.2.2 of the Azure.Extensions.AspNetCore.Configuration.Secrets NuGet package. Perhaps the documentation needs to be revisited again. Thank you.

[heaths]

@HomerBart you're right: there's some incongruency here. A disabled key will be listed by Key Vault but Key Vault will not return it if you try to get its secret value e.g.,

$ az keyvault secret list --vault-name $vaultName --query '[].{name: name, enabled: attributes.enabled, expires: attributes.expires}' -o table
Name            Enabled    Expires
--------------  ---------  -------------------------
DisabledSecret  False
ExpiredSecret   True       2023-11-06T22:59:00+00:00
Secret          True

$ az keyvault secret show --vault-name heathskv --name Secret --query value -o tsv
secret

$ az keyvault secret show --vault-name heathskv --name ExpiredSecret --query value -o tsv
expired

$ az keyvault secret show --vault-name heathskv --name DisabledSecret --query value -o tsv
(Forbidden) Operation get is not allowed on a disabled secret.
Code: Forbidden
Message: Operation get is not allowed on a disabled secret.
Inner error: {
    "code": "SecretDisabled"
}

My confusion stemmed from the fact it is enumerated, hence an initial listing of secrets by the KeyVaultSecretManager will fetch it, but it won't be able to get the secret value itself. I'll update the documentation.