Invalidate _cachedId in all property setters that affect Id computation

[Copilot]

Invalidate _cachedId in all property setters that affect Id computation

Analysis

Based on the Id property getter, the following properties are used in computing the cached ID based on the SourceType:

• ✅ Certificate (already invalidates)
• ✅ CachedValue (already invalidates)
• ✅ ClientSecret (already invalidates)
• ✅ KeyVaultUrl (added invalidation)
• ✅ KeyVaultCertificateName (added invalidation)
• ✅ Base64EncodedValue (added invalidation)
• ✅ CertificateDiskPath (added invalidation)
• ✅ CertificateStorePath (added invalidation)
• ✅ CertificateThumbprint (added invalidation)
• ✅ CertificateDistinguishedName (added invalidation)
• ✅ ManagedIdentityClientId (added invalidation)
• ✅ SignedAssertionFileDiskPath (added invalidation)
• ✅ CustomSignedAssertionProviderName (added invalidation)
• ✅ CustomSignedAssertionProviderData (added invalidation)

Changes Made

• Review and understand the existing CredentialDescription.cs file structure
• Add _cachedId = null to all relevant property setters (10 properties updated)
• Create comprehensive unit tests to verify cache invalidation (14 new test methods)
• Build and test the changes (all 92 tests passed)
• Run CodeQL security scan (no issues found)
• Request code review (no issues found)
• Fix certificate test to properly verify cache invalidation by setting cert to null

Summary

This PR fixes a bug where the cached Id value could become stale if certain properties were modified after the Id was first computed. All property setters that contribute to the Id computation now properly invalidate the cache by setting _cachedId = null. Comprehensive unit tests have been added to verify the fix.

The CachedId_InvalidatedWhen_Certificate_Changes test has been updated to properly verify cache invalidation by changing from a certificate to null, which produces different IDs and confirms the cache is recomputed.

Fixes: Issue about _cachedId invalidation in CredentialDescription
Related PR: #238
Discussion: #238 (comment)

Original prompt

---

This section details on the original issue you should resolve

<issue_title>Invalidate _cachedId in all relevant property setters in CredentialDescription</issue_title>
<issue_description>Currently, only the setters for Certificate, CachedValue, and ClientSecret in CredentialDescription invalidate _cachedId by setting it to null. However, the Id computation depends on additional properties based on the value of SourceType. If any of these other properties are changed after the Id is cached, the cached value may become stale and incorrect.

Properties involved (depending on source type):

• KeyVaultUrl, KeyVaultCertificateName (for KeyVault)
• Base64EncodedValue (for Base64Encoded)
• CertificateDiskPath (for Path)
• CertificateStorePath, CertificateThumbprint (for StoreWithThumbprint)
• CertificateStorePath, CertificateDistinguishedName (for StoreWithDistinguishedName)
• ManagedIdentityClientId (for SignedAssertionFromManagedIdentity)
• SignedAssertionFileDiskPath (for SignedAssertionFilePath)
• CustomSignedAssertionProviderName, CustomSignedAssertionProviderData (for CustomSignedAssertion)

There is a risk that Id could be out of sync if these properties are updated and _cachedId is not cleared.

Suggested Action:

• Review all property setters in CredentialDescription. Any property that impacts the result of Id should also invalidate the cached value by setting _cachedId = null.
• Add unit tests

---

Related PR: #238
Discussion: #238 (comment)

---

Coordinating with: @jmprieur</issue_description>

Comments on the Issue (you are @copilot in this section)

• Fixes Invalidate _cachedId in all relevant property setters in CredentialDescription #239

---

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

[jmprieur]

LGTM
Thanks Copilot

[Copilot]

Pull request overview

This pull request ensures that the cached Id for CredentialDescription remains consistent when any property that participates in the Id computation is modified, and adds regression tests around that behavior. It closes a bug where _cachedId could become stale when certain properties were updated after first access.

Changes:

• Updated multiple CredentialDescription properties (KeyVaultUrl, KeyVaultCertificateName, Base64EncodedValue, CertificateDiskPath, CertificateStorePath, CertificateThumbprint, CertificateDistinguishedName, ManagedIdentityClientId, SignedAssertionFileDiskPath, CustomSignedAssertionProviderName, CustomSignedAssertionProviderData) to use backing fields and clear _cachedId in their setters.
• Left existing invalidation behavior for Certificate, ClientSecret, and CachedValue intact while aligning patterns with the new properties.
• Added 14 xUnit tests in CredentialDescriptionIdTest to confirm that updating each relevant property causes the Id to change or otherwise reflect the new state.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
src/Microsoft.Identity.Abstractions/ApplicationOptions/CredentialDescription.cs	Adds backing fields and cache invalidation to all properties whose values are used in CredentialDescription.Id, ensuring IDs are recomputed after mutations.
test/Microsoft.Identity.Abstractions.Tests/CredentialDescriptionIdTest.cs	Adds targeted unit tests for each property affecting Id, though the certificate-based test currently does not truly validate cache invalidation due to using two identical certificates.

[jmprieur]

@copilot there are suggestions to address. Thanks!

[Copilot]

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• bcnvsblobprodcus378.vsblob.vsassets.io
  • Triggering command: /usr/bin/dotnet dotnet build test/Microsoft.Identity.Abstractions.Tests/Microsoft.Identity.Abstractions.Tests.csproj --no-restore (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

[Copilot]

@copilot there are suggestions to address. Thanks!

I've already addressed the bot's feedback about the certificate test in commit 010706a. Could you please clarify which other suggestions need to be addressed? I don't see any additional actionable review comments in the thread.