Refactor CCA instance management in agentic flows

[Avery-Dunn]

This PR is meant to address this comment in the main agentic refactoring PR: #3842 (comment)

Managing CCA instances is the main expected pain point of the new agentic behavior supported in recent versions of MSAL. This sort of per-agent CCA instance management will become irrelevant once MSAL extensibility APIs are improved, however until those improvements are made and released ID Web should manage the per-agent CCA instances in a proper way.

Concern

The original PR introduced a dedicated GetOrBuildAgentUserFicCcaAsync that built agent CCAs via ConfidentialClientApplicationBuilder.Create(agentAppId) with manual .WithAuthority(), .WithHttpClientFactory(), and .WithCacheOptions() calls — bypassing the normal BuildConfidentialClientApplicationAsync path. This meant agent CCAs missed:

• CreateWithApplicationOptions() (AzureRegion, ClientCapabilities, ClientName/Version)
• .WithLogging() (structured logging)
• _tokenCacheProvider.Initialize() (distributed cache support)
• B2C/CIAM authority handling
• .WithExperimentalFeatures()

In addition, there are existing GetOrBuildConfidentialClientApplicationAsync / _applicationsByAuthorityClientId infrastructure that we could adjust to build the CCAs rather than keeping a parallel track for the per agent CCAs.

Changes compared to parent PR

1. Cache key suffix

GetOrBuildConfidentialClientApplicationAsync now accepts an optional agentAppId parameter. When set, the cache key becomes {blueprintKey}:agent:{agentAppId}, storing agent CCAs in the shared _applicationsByAuthorityClientId dictionary alongside normal CCAs.

The authenticationScheme is not explicitly in the agent suffix because different schemes resolve to different MergedOptions, which produce different GetApplicationKey() base keys — so scheme isolation is implicit in the blueprint key portion.

2. Credential customization callback

BuildConfidentialClientApplicationAsync now accepts an optional clientAssertionProvider callback (Func<AssertionRequestOptions, Task<string>>). When provided, the builder calls .WithClientAssertion(clientAssertionProvider) instead of .WithClientCredentialsAsync(...). All other configuration is shared:

• CreateWithApplicationOptions(ccaOptions) — authority, AzureRegion, ClientCapabilities, ClientName/Version
• .WithHttpClientFactory(), .WithLogging(), .WithExperimentalFeatures()
• Authority handling (OIDC/B2C/standard)
• Cache options (shared + distributed)
• _tokenCacheProvider.Initialize() on both AppTokenCache and UserTokenCache

Deviation from suggestion: The reviewer proposed Func<ConfidentialClientApplicationBuilder, string, Task> (exposing the builder + authority). We use Func<AssertionRequestOptions, Task<string>> (the MSAL assertion callback directly), which keeps the builder encapsulated — GetOrBuildAgentUserFicCcaAsync constructs the callback and passes it to the unified builder rather than receiving a raw builder to configure.

3. Clone/derive blueprint options

For agent CCAs, BuildConfidentialClientApplicationAsync creates a fresh ConfidentialClientApplicationOptions, populates it via MergedOptions.UpdateConfidentialClientApplicationOptionsFromMergedOptions(mergedOptions, ccaOptions), then sets ccaOptions.ClientId = agentAppId. The blueprint's lazily-cached ConfidentialClientApplicationOptions instance is never mutated.

4. Earlier agent flow detection

GetAuthenticationResultForUserInternalAsync now calls TryGetAuthenticationResultForAgentUserFicAsync before GetOrBuildConfidentialClientApplicationAsync. For agent requests, the blueprint CCA is only built lazily inside the assertion callback when Leg 1 actually fires. The IConfidentialClientApplication application parameter was removed from TryGetAuthenticationResultForAgentUserFicAsync entirely since the agent CCA gets its authority from MergedOptions.

Additional changes beyond the original comment

• Selective agent eviction: DOS eviction only removes entries with :agent: in the key — the blueprint CCA is preserved. The old separate-dictionary design cleared everything.
• Removed _agentUserFicCcas and _agentCcaSemaphores: Two entire ConcurrentDictionary fields eliminated. Agent CCAs reuse _applicationsByAuthorityClientId and _appSemaphores.
• Agent redirect URI skip: Agent CCAs skip WithRedirectUri() since redirect URIs aren't meaningful for client-credential + assertion flows.
• _applicationsByAuthorityClientId visibility: Changed from private to internal to support test assertions on shared dictionary contents.

How this compares to the original (master) agent flow

The master branch already stored agent CCAs in _applicationsByAuthorityClientId via the normal builder path — but it did so through a fundamentally different mechanism:

Aspect	Master (original)	This refactor
CCA key differentiation	GetMergedOptions() creates a new MergedOptions with the agent's ClientId, which naturally produces a different GetApplicationKey()	Explicit :agent:{agentAppId} suffix appended to the blueprint's key
Configuration completeness	GetMergedOptions() creates a bare new MergedOptions() with only ~7 fields copied (missing ClientName, ClientVersion, LogLevel, EnablePiiLogging, EnableCacheSynchronization, etc.)	UpdateConfidentialClientApplicationOptionsFromMergedOptions copies all fields from the fully-populated blueprint MergedOptions
Token flow	Piggybacked on ROPC (AcquireTokenByUsernamePassword with password = "password")	Native MSAL UserFIC API (AcquireTokenByUserFederatedIdentityCredential)
Leg 1 (FMI token)	OidcIdpSignedAssertionProvider → re-enters TokenAcquisition via ITokenAcquirerFactory	Direct assertion callback → blueprint CCA's AcquireTokenForClient().WithFmiPath()
Silent cache lookup	Depended on ClaimsPrincipal.GetMsalAccountId() (typically null in agentic scenarios — this was the reported bug)	_agentUserFicAccountIds dictionary tracks MSAL account identifiers independently
DOS protection	None	Threshold eviction clears only :agent: entries, preserving the blueprint CCA

In short, the master design used the shared dictionary but with incomplete configuration cloning and a semantically wrong token flow (ROPC). This refactor restores the shared-dictionary approach while fixing both the configuration completeness (full options cloning) and the token flow (native UserFIC API).

Risks and unknowns

1. Key format coupling: The :agent: sentinel in cache keys is a string convention. If GetApplicationKey() ever produces keys containing :agent: for non-agent scenarios, the eviction logic would incorrectly clear those entries. This is unlikely given the current key format (authority + clientId + azureRegion + credentialId), but worth noting.

2. _agentUserFicAccountIds remains separate: Unlike CCA instances, account identifiers are not stored in the shared dictionary. This is intentional — they map (agent, user, tenant) tuples to MSAL account identifiers, which is a fundamentally different data structure from the CCA cache. There's no existing mechanism for this in the normal flow (which uses ClaimsPrincipal instead).

3. Blueprint CCA built lazily: For agent-only traffic, the blueprint CCA is not built until the assertion callback fires during Leg 1. If the assertion callback fails (e.g., credential loading error), the error surfaces inside the agent CCA's token acquisition rather than during CCA construction. This matches the old behavior where OidcIdpSignedAssertionProvider could also fail during callback execution.

Test coverage

• AgentCca_StoredInSharedDictionary_WithAgentKeySegment — verifies agent and blueprint CCAs coexist in _applicationsByAuthorityClientId with correct key segments
• AgentCcaEviction_DoesNotAffect_BlueprintCca — verifies selective eviction preserves blueprint entries
• All existing agent tests (UPN/OID acquisition, silent cache, shared cache survival, eviction threshold) pass unchanged
• Full test suite (992-1020 tests across TFMs) passes with no regressions

[bgavrilMS]

This should be part of GetApplicationKey

[Avery-Dunn]

GetApplicationKey now accepts an optional agentAppId parameter and appends the :agent:{agentAppId}suffix internally. The manual suffix concatenation in GetOrBuildConfidentialClientApplicationAsync has been removed

[bgavrilMS]

This is O(n) operation, it can be quite expensive. How about just clearing the entire dictionary if it reaches 10000. Tokens are cached anyway.

[Avery-Dunn]

Replaced the O(n) agent-only scan with a simpler _applicationsByAuthorityClientId.Clear().

This should be safe because _applicationsByAuthorityClientId should only be populated by CCA instances that use the shared cache.

[Copilot]

Pull request overview

This PR refactors agentic User FIC confidential client application (CCA) instance management so agent CCAs are built through the same unified builder/configuration path as “blueprint” CCAs, while still maintaining per-agent CCA isolation and selective eviction behavior.

Changes:

• Unifies agent CCA construction with GetOrBuildConfidentialClientApplicationAsync / BuildConfidentialClientApplicationAsync via an agent-specific cache-key suffix and an MSAL client-assertion callback.
• Moves agent-flow detection earlier so the blueprint CCA is built lazily (only when Leg 1 actually executes inside the assertion callback).
• Updates and adds tests to validate agent entries coexist in the shared CCA dictionary and that agent eviction does not remove the blueprint CCA.

Reviewed changes

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

File	Description
tests/Microsoft.Identity.Web.Test/TokenAcquisitionTests.cs	Updates eviction-related tests to target agent entries within the shared CCA dictionary and adds coverage for shared-dictionary storage + blueprint preservation.
src/Microsoft.Identity.Web.TokenAcquisition/TokenAcquisition.cs	Refactors agent CCA caching/building into the shared CCA infrastructure, adds agent key suffixing + selective eviction, and introduces an assertion-provider hook to reuse the standard builder path.

[bgavrilMS]

This clientAssertionProvider is not part of the app key. I think it only makes sense for this callback to exist when an agentAppid is also defined (which implicitly defines the key). In other words, we expect the CCA for 1 agentAppId to map to a single clientAssertionProvider. In other words - agentAppId and clientAssertionProvider need to both be NULL or both be defined.

[bgavrilMS]

Another way of doing this would be have GetOrBuildConfidentialClientApplicationAsync(mergedOptions, isTokenBiding, (agentAppId, blueprintOptions) and for GetOrBuildConfidentialClientApplicationAsync to create the FUNC on the fly. This allows it you to have a key that includes details of blueprintOptions as well.

But it can be overkill.

[Avery-Dunn]

Added an ArgumentException guard at the top of BuildConfidentialClientApplicationAsync that throws if exactly one of agentAppId/clientAssertionProvider is null.

The overload with a tuple shouldn't be necessary, the guard should adequately enforce them being paired.

[bgavrilMS]

You're calling this on every token acquisition. Isn't this expensive? The callback captures external variables etc.

[Avery-Dunn]

Added an early cache-hit check in GetOrBuildAgentUserFicCcaAsync that returns the cached CCA before constructing the closure.

Some quick performance testing showed the closure allocation itself was pretty negligible compared to the total size of the call, but the early return avoids entering the async state machine in GetOrBuildConfidentialClientApplicationAsync, yielding ~15% time improvement on cache-hit calls compared to the original design (~105 µs to ~88 µs)

[bgavrilMS]

You can further optimize allocations here by using a StringBuilder

[Avery-Dunn]

Good point, adjusted in latest commit.

[bgavrilMS]

rename to agenticAssertionProvider to make it clear this is not a general purpose assertion provider.

[Avery-Dunn]

Renamed in latest commit

[bgavrilMS]

Approved with comments.