Conversation
There was a problem hiding this comment.
Pull request overview
Adds agent identity support to MSAL .NET by introducing a new high-level AcquireTokenForAgent API that orchestrates FMI + UserFIC flows, plus extending UserFIC to support user identification by OID (Guid) in addition to UPN.
Changes:
- Introduces
AgentIdentitymodel andAcquireTokenForAgentpublic API (builder + request pipeline). - Adds a
Guidoverload forAcquireTokenByUserFederatedIdentityCredentialand updates request construction (body + CCS routing) to senduser_id. - Adds telemetry event ID and integration tests covering UPN/OID/app-only agent scenarios.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Microsoft.Identity.Test.Integration.netcore/HeadlessTests/Agentic.cs | Adds integration coverage for agent flows (low-level UserFIC + high-level AcquireTokenForAgent) including UPN/OID/app-only. |
| src/client/Microsoft.Identity.Client/TelemetryCore/Internal/Events/ApiEvent.cs | Adds telemetry API ID for AcquireTokenForAgent. |
| src/client/Microsoft.Identity.Client/PublicApi/netstandard2.0/PublicAPI.Unshipped.txt | Declares new public APIs (AgentIdentity, AcquireTokenForAgent, UserFIC Guid overload) for netstandard2.0. |
| src/client/Microsoft.Identity.Client/PublicApi/net8.0/PublicAPI.Unshipped.txt | Declares new public APIs for net8.0. |
| src/client/Microsoft.Identity.Client/PublicApi/net8.0-ios/PublicAPI.Unshipped.txt | Declares new public APIs for net8.0-ios. |
| src/client/Microsoft.Identity.Client/PublicApi/net8.0-android/PublicAPI.Unshipped.txt | Declares new public APIs for net8.0-android. |
| src/client/Microsoft.Identity.Client/PublicApi/net472/PublicAPI.Unshipped.txt | Declares new public APIs for net472. |
| src/client/Microsoft.Identity.Client/PublicApi/net462/PublicAPI.Unshipped.txt | Declares new public APIs for net462. |
| src/client/Microsoft.Identity.Client/OAuth2/OAuthConstants.cs | Adds user_id parameter constant for UserFIC OID requests. |
| src/client/Microsoft.Identity.Client/Internal/Requests/UserFederatedIdentityCredentialRequest.cs | Sends user_id vs username based on overload and aligns CCS routing hint behavior. |
| src/client/Microsoft.Identity.Client/Internal/Requests/AgentTokenRequest.cs | New internal request orchestrator implementing multi-leg agent flow via internal CCAs. |
| src/client/Microsoft.Identity.Client/IConfidentialClientApplication.cs | Adds AcquireTokenForAgent to the public interface. |
| src/client/Microsoft.Identity.Client/IByUserFederatedIdentityCredential.cs | Adds Guid overload for UserFIC and clarifies UPN vs OID semantics in docs. |
| src/client/Microsoft.Identity.Client/ConfidentialClientApplication.cs | Implements new public APIs (AcquireTokenForAgent + UserFIC Guid overload). |
| src/client/Microsoft.Identity.Client/ApiConfig/Parameters/AcquireTokenForAgentParameters.cs | New parameters bag for agent flow (AgentIdentity, ForceRefresh, SendX5C). |
| src/client/Microsoft.Identity.Client/ApiConfig/Parameters/AcquireTokenByUserFederatedIdentityCredentialParameters.cs | Adds UserObjectId to carry OID into request layer. |
| src/client/Microsoft.Identity.Client/ApiConfig/Executors/IConfidentialClientApplicationExecutor.cs | Adds executor overload for AcquireTokenForAgent. |
| src/client/Microsoft.Identity.Client/ApiConfig/Executors/ConfidentialClientExecutor.cs | Wires executor path to AgentTokenRequest. |
| src/client/Microsoft.Identity.Client/ApiConfig/AcquireTokenForAgentParameterBuilder.cs | New builder for AcquireTokenForAgent options (ForceRefresh, SendX5C) and telemetry. |
| src/client/Microsoft.Identity.Client/ApiConfig/AcquireTokenByUserFederatedIdentityCredentialParameterBuilder.cs | Adds builder factory/constructor for the new Guid overload. |
| src/client/Microsoft.Identity.Client/AgentIdentity.cs | New public model representing agent + (optional) user identity with UPN/OID/app-only options. |
src/client/Microsoft.Identity.Client/Internal/Requests/AgentTokenRequest.cs
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/Internal/Requests/AgentTokenRequest.cs
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/Internal/Requests/AgentTokenRequest.cs
Outdated
Show resolved
Hide resolved
...client/Microsoft.Identity.Client/Internal/Requests/UserFederatedIdentityCredentialRequest.cs
Show resolved
Hide resolved
...client/Microsoft.Identity.Client/Internal/Requests/UserFederatedIdentityCredentialRequest.cs
Show resolved
Hide resolved
| if (blueprintConfig.HttpManager != null) | ||
| { | ||
| builder.WithHttpManager(blueprintConfig.HttpManager); | ||
| } |
There was a problem hiding this comment.
PropagateHttpConfig only copies HttpClientFactory/HttpManager. If the blueprint CCA is configured with token-cache options (e.g., WithCacheOptions / shared cache settings), internal CCAs will be created with default cache options, which can change caching behavior and break expectations that the agent flow follows the blueprint’s cache configuration. Consider propagating relevant cache configuration (at minimum CacheOptions) into the internal CCA builders as well.
| } | |
| } | |
| if (blueprintConfig.CacheOptions != null) | |
| { | |
| builder.WithCacheOptions(blueprintConfig.CacheOptions); | |
| } |
There was a problem hiding this comment.
I don't believe CacheOptions should be propagated: currently the only option in CacheOptions is to use a shared, static cache, which would make the internal CCA instance's cache be shared with those created by customers.
If a customer created a CCA instance with the same app ID as the one set as the AgentIdentity then other acquireToken flows could find the agent-specific tokens cached in the acquireTokenForAgent flow.
src/client/Microsoft.Identity.Client/Internal/Requests/AgentTokenRequest.cs
Outdated
Show resolved
Hide resolved
| /// (and its in-memory token cache) instead of rebuilding from scratch each time. | ||
| /// </summary> | ||
| internal ConcurrentDictionary<string, IConfidentialClientApplication> AgentCcaCache { get; } = new(); | ||
|
|
There was a problem hiding this comment.
AgentCcaCache is an unbounded ConcurrentDictionary keyed by agent application id (and currently reused across authorities). In long-lived processes that call AcquireTokenForAgent with many different agentApplicationIds, this can grow without limit and retain token caches indefinitely. Consider an eviction strategy (e.g., LRU/size cap), a way to clear entries, and/or scoping keys to include authority so entries don’t accumulate across tenants.
| /// <summary> | |
| /// Clears all cached agent <see cref="IConfidentialClientApplication"/> instances. | |
| /// This can be used by long-lived hosts to avoid unbounded growth of the in-memory cache. | |
| /// </summary> | |
| internal void ClearAgentCcaCache() | |
| { | |
| AgentCcaCache.Clear(); | |
| } | |
| /// <summary> | |
| /// Removes a specific agent <see cref="IConfidentialClientApplication"/> instance | |
| /// from the cache by its agent application identifier, if present. | |
| /// Returns <c>true</c> if an entry was removed; otherwise, <c>false</c>. | |
| /// </summary> | |
| /// <param name="agentApplicationId">The agent application identifier used as the cache key.</param> | |
| internal bool RemoveAgentCcaCacheEntry(string agentApplicationId) | |
| { | |
| if (string.IsNullOrEmpty(agentApplicationId)) | |
| { | |
| return false; | |
| } | |
| return AgentCcaCache.TryRemove(agentApplicationId, out _); | |
| } |
There was a problem hiding this comment.
I don't believe this will be a problem: each entry in the cache is pegged to a specific application ID, and customers likely won't have enough Entra apps to cause any memory issues here.
...client/Microsoft.Identity.Client/Internal/Requests/UserFederatedIdentityCredentialRequest.cs
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/ApiConfig/Parameters/AcquireTokenForAgentParameters.cs
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/Internal/Requests/AgentTokenRequest.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/Internal/Requests/AgentTokenRequest.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/Internal/Requests/AgentTokenRequest.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/Internal/Requests/AgentTokenRequest.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/Internal/Requests/AgentTokenRequest.cs
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/Internal/Requests/AgentTokenRequest.cs
Outdated
Show resolved
Hide resolved
| // Instance discovery: honor the Blueprint's custom metadata or disabled discovery | ||
| agentConfig.CustomInstanceDiscoveryMetadata = blueprintConfig.CustomInstanceDiscoveryMetadata; | ||
| agentConfig.CustomInstanceDiscoveryMetadataUri = blueprintConfig.CustomInstanceDiscoveryMetadataUri; | ||
| agentConfig.IsInstanceDiscoveryEnabled = blueprintConfig.IsInstanceDiscoveryEnabled; |
There was a problem hiding this comment.
When building the internal Agent CCA, PropagateBlueprintConfig copies several config fields but does not propagate ServiceBundle.Config.AccessorOptions (set via .WithCacheOptions(...)). This can change token cache behavior (e.g., shared/static internal caches) between the blueprint and the internal Agent CCA, leading to surprising cache misses or memory growth differences. Consider copying AccessorOptions (and any other cache-related config required) into the Agent CCA config so the internal app/user caches respect the blueprint's cache configuration.
| agentConfig.IsInstanceDiscoveryEnabled = blueprintConfig.IsInstanceDiscoveryEnabled; | |
| agentConfig.IsInstanceDiscoveryEnabled = blueprintConfig.IsInstanceDiscoveryEnabled; | |
| // Cache: propagate cache accessor options so Agent CCA respects Blueprint cache configuration | |
| agentConfig.AccessorOptions = blueprintConfig.AccessorOptions; |
src/client/Microsoft.Identity.Client/Internal/Requests/AgentTokenRequest.cs
Show resolved
Hide resolved
| builder.AppendLine("=== AcquireTokenByUserFederatedIdentityCredentialParameters ==="); | ||
| builder.AppendLine("SendX5C: " + SendX5C); | ||
| builder.AppendLine("ForceRefresh: " + ForceRefresh); | ||
| builder.AppendLine("UserIdentifiedByOid: " + UserObjectId.HasValue); |
There was a problem hiding this comment.
Log username as well
| // The user_fic grant identifies the user by either OID (user_id) or UPN (username). | ||
| // The parameter builder enforces that exactly one is set via separate constructors, | ||
| // so both values cannot be populated simultaneously through the public API. | ||
| // OID is checked first because it is immutable and preferred over UPN, which can be renamed. |
There was a problem hiding this comment.
I am not sure if this line is true. Is OID preferred in case of user fic?
There was a problem hiding this comment.
I think username is common and logic is fine. Only the comment seems a bit misleading
This PR adds high-level
AcquireTokenForAgentAPI to perform the series of calls needed for agent identity scenarios, and extends the existing UserFIC behavior (added in #5802) with user OID support.New public APIs:
AgentIdentity: Model class representing an agent app and the user it acts on behalf of (via a UPN or OID), as well as an app-only option with no userIConfidentialClientApplication.AcquireTokenForAgent: High-level API that orchestrates the full multi-leg FMI+UserFIC flow internally. Accepts a blueprint CCA configured with SN+I certificate and handles Leg 1 (FMI credential), Leg 2 (assertion via FMI path), and Leg 3 (UserFIC exchange)AcquireTokenByUserFederatedIdentityCredential(scopes, Guid userObjectId, assertion): New overload using aGuidtype instead of a string, to represent a user's OID instead of their username. Matches similar behavior added to ID Web's AgentIdentitiesExtension.csInternal implementations:
AgentTokenRequest: New class to orchestrate the multi-leg flow using the blueprint CCA to obtain FMI credentials, building internal agent CCAs for assertion and token exchangeUserFederatedIdentityCredentialRequestconditionally sends user_id or username based on which overload was used, with matching CCS routing hintsConfidentialClientApplicationto store internal CCA instances: a different CCA instance is needed for the internal token calls as they use a different client ID and credential, and a simple dictionary in the "main" CCA instance will ensure the internal CCA instances (and their token caches) share the lifetime of the main CCA instancesAgentic.csto cover various agent ID scenarios these changes support: low-level UserFic API vs. high-level AcquireTokenForAgent API, UPN/OID/app-only options, caching and silent call behavior, etc.UserFederatedIdentityCredentialTeststo cover the silent call/caching behavior on both the new agent API and existing FIC API