address pr comments

Author: GladwinJohnson

src/client/Microsoft.Identity.Client/Internal/Requests/ManagedIdentityAuthRequest.cs

@@ -39,12 +39,15 @@ protected override async Task<AuthenticationResult> ExecuteAsync(CancellationTok
             // 1. FIRST, handle ForceRefresh
             if (_managedIdentityParameters.ForceRefresh)
             {
+                //log a warning if Claims are also set
+                if (!string.IsNullOrEmpty(AuthenticationRequestParameters.Claims))
+                {
+                    logger.Warning("[ManagedIdentityRequest] Both ForceRefresh and Claims are set. Using ForceRefresh to skip cache.");
+                }
+
                 AuthenticationRequestParameters.RequestContext.ApiEvent.CacheInfo = CacheRefreshReason.ForceRefreshOrClaims;
                 logger.Info("[ManagedIdentityRequest] Skipped using the cache because ForceRefresh was set.");
 
-                // We still respect claims if present
-                _managedIdentityParameters.Claims = AuthenticationRequestParameters.Claims;
-
                 // Straight to the MI endpoint
                 authResult = await GetAccessTokenAsync(cancellationToken, logger).ConfigureAwait(false);
                 return authResult;
@@ -60,19 +63,19 @@ protected override async Task<AuthenticationResult> ExecuteAsync(CancellationTok
                 _managedIdentityParameters.Claims = AuthenticationRequestParameters.Claims;
                 AuthenticationRequestParameters.RequestContext.ApiEvent.CacheInfo = CacheRefreshReason.ForceRefreshOrClaims;
 
-                // If there is a cached token, compute its hash for the “bad token” scenario
+                // If there is a cached token, compute its hash for the “revoked token” scenario
                 if (cachedAccessTokenItem != null)
                 {
                     string cachedTokenHash = _cryptoManager.CreateSha256HashHex(cachedAccessTokenItem.Secret);
                     _managedIdentityParameters.RevokedTokenHash = cachedTokenHash;
 
-                    logger.Info("[ManagedIdentityRequest] Claims are present. Computed hash of the cached (bad) token. " +
+                    logger.Info("[ManagedIdentityRequest] Claims are present. Computed hash of the cached (revoked) token. " +
                                 "Will now request a fresh token from the MI endpoint.");
                 }
                 else
                 {
                     logger.Info("[ManagedIdentityRequest] Claims are present, but no cached token was found. " +
-                                "Requesting a fresh token from the MI endpoint without a bad-token hash.");
+                                "Requesting a fresh token from the MI endpoint without a revoked-token hash.");
                 }
 
                 // In both cases, we skip using the cached token and get a new one

src/client/Microsoft.Identity.Client/ManagedIdentity/AbstractManagedIdentity.cs

@@ -57,6 +57,15 @@ public virtual async Task<ManagedIdentityResponse> AuthenticateAsync(
 
             ManagedIdentityRequest request = CreateRequest(resource, parameters);
 
+            // Automatically add claims / capabilities if this MI source supports them
+            if (_sourceType.SupportsClaimsAndCapabilities())
+            {
+                request.AddClaimsAndCapabilities(
+                    _requestContext.ServiceBundle.Config.ClientCapabilities,
+                    parameters,
+                    _requestContext.Logger);
+            }
+
             _requestContext.Logger.Info("[Managed Identity] Sending request to managed identity endpoints.");
 
             IRetryPolicy retryPolicy = _requestContext.ServiceBundle.Config.RetryPolicyFactory.GetRetryPolicy(request.RequestType);
@@ -308,23 +317,5 @@ private static void CreateAndThrowException(string errorCode,
 
             throw exception;
         }
-
-        /// <summary>
-        /// Sets the request parameter in either the query or body based on the request method.
-        /// </summary>
-        /// <param name="request"></param>
-        /// <param name="key"></param>
-        /// <param name="value"></param>
-        protected void SetRequestParameter(ManagedIdentityRequest request, string key, string value)
-        {
-            if (request.Method == HttpMethod.Post)
-            {
-                request.BodyParameters[key] = value;
-            }
-            else
-            {
-                request.QueryParameters[key] = value;
-            }
-        }
     }
 }

src/client/Microsoft.Identity.Client/ManagedIdentity/ManagedIdentityRequest.cs

@@ -61,7 +61,7 @@ internal void AddClaimsAndCapabilities(
                 !string.IsNullOrEmpty(parameters.RevokedTokenHash))
             {
                 QueryParameters["token_sha256_to_refresh"] = parameters.RevokedTokenHash;
-                logger.Info("[Managed Identity] Passing SHA-256 of the 'bad' token to Managed Identity endpoint.");
+                logger.Info("[Managed Identity] Passing SHA-256 of the 'revoked' token to Managed Identity endpoint.");
             }
         }
     }

src/client/Microsoft.Identity.Client/ManagedIdentity/ManagedIdentitySourceExtensions.cs

@@ -0,0 +1,19 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+using System.Collections.Generic;
+
+namespace Microsoft.Identity.Client.ManagedIdentity
+{
+    internal static class ManagedIdentitySourceExtensions
+    {
+        private static readonly HashSet<ManagedIdentitySource> s_supportsClaimsAndCaps =
+            [
+            // add other sources here as they light up
+            ManagedIdentitySource.ServiceFabric,
+            ];
+
+        internal static bool SupportsClaimsAndCapabilities(
+            this ManagedIdentitySource source) => s_supportsClaimsAndCaps.Contains(source);
+    }
+}

src/client/Microsoft.Identity.Client/ManagedIdentity/ServiceFabricManagedIdentitySource.cs

@@ -85,10 +85,6 @@ protected override ManagedIdentityRequest CreateRequest(string resource,
             request.QueryParameters["api-version"] = ServiceFabricMsiApiVersion;
             request.QueryParameters["resource"] = resource;
 
-            request.AddClaimsAndCapabilities(
-                _requestContext.ServiceBundle.Config.ClientCapabilities, 
-                parameters, _requestContext.Logger);
-
             switch (_requestContext.ServiceBundle.Config.ManagedIdentityId.IdType)
             {
                 case AppConfig.ManagedIdentityIdType.ClientId:

src/client/Microsoft.Identity.Client/Utils/CoreHelpers.cs

@@ -103,7 +103,7 @@ public static Dictionary<string, string> ParseKeyValueList(
                     // The value is everything after the first '=' (including any trailing '=')
                     string value = queryPair.Substring(idx + 1);
 
-                    // If urlDecode == true, decode both key and value
+                    // Url decoding is needed for parsing OAuth response, but not for parsing WWW-Authenticate header in 401 challenge
                     if (urlDecode)
                     {
                         key = UrlDecode(key);

tests/Microsoft.Identity.Test.Common/Core/Mocks/MockHttpManagerExtensions.cs

@@ -486,30 +486,53 @@ private static MockHttpMessageHandler BuildMockHandlerForManagedIdentitySource(
 
             var manager = new CommonCryptographyManager();
 
-            // If capabilityEnabled, add "xms_cc": "cp1"
-            if (capabilityEnabled)
+            // ---------------------------------------------------------------------------
+            // Client-capabilities (xms_cc) and revoked-token hash
+            // ---------------------------------------------------------------------------
+
+            bool sourceSupportsExtras = managedIdentitySourceType.SupportsClaimsAndCapabilities();
+
+            // ----- xms_cc --------------------------------------------------------------
+            if (sourceSupportsExtras)
             {
-                if (managedIdentitySourceType == ManagedIdentitySource.ServiceFabric)
+                if (capabilityEnabled)
                 {
+                    // This source should send xms_cc
                     expectedQueryParams.Add("xms_cc", "cp1,cp2");
                 }
+                else
+                {
+                    // Capability flag disabled → xms_cc must NOT be present
+                    notExpectedQueryParams.Add("xms_cc", "cp1,cp2");
+                }
             }
             else
             {
+                // Source does not support capabilities → never expect xms_cc
                 notExpectedQueryParams.Add("xms_cc", "cp1,cp2");
             }
 
-            if (claimsEnabled)
+            // ----- token_sha256_to_refresh --------------------------------------------
+            if (sourceSupportsExtras)
             {
-                if (managedIdentitySourceType == ManagedIdentitySource.ServiceFabric)
+                string hash = manager.CreateSha256HashHex(TestConstants.ATSecret);
+
+                if (claimsEnabled)
+                {
+                    // Claims path active → expect the hash
+                    expectedQueryParams.Add("token_sha256_to_refresh", hash);
+                }
+                else
                 {
-                    expectedQueryParams.Add("token_sha256_to_refresh", 
-                        manager.CreateSha256HashHex(TestConstants.ATSecret));
+                    // No claims → hash must be absent
+                    notExpectedQueryParams.Add("token_sha256_to_refresh", hash);
                 }
             }
             else
             {
-                notExpectedQueryParams.Add("token_sha256_to_refresh", 
+                // Source does not support hash param → ensure it's absent
+                notExpectedQueryParams.Add(
+                    "token_sha256_to_refresh",
                     manager.CreateSha256HashHex(TestConstants.ATSecret));
             }