restrict FOCI to PCA and improve logging (#5030)

* Fix for 4988 - restrict FOCI to PCA and improve logging * Update test
AzureAD · Jan 1, 2025 · eb59900 · eb59900
1 parent 01e79b1
commit eb59900
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 9 deletions.
diff --git a/src/client/Microsoft.Identity.Client/Internal/Requests/Silent/CacheSilentStrategy.cs b/src/client/Microsoft.Identity.Client/Internal/Requests/Silent/CacheSilentStrategy.cs
@@ -174,13 +174,17 @@ private async Task<AuthenticationResult> CreateAuthenticationResultAsync(MsalAcc
 
         private async Task<MsalTokenResponse> TryGetTokenUsingFociAsync(CancellationToken cancellationToken)
         {
-            if (!ServiceBundle.PlatformProxy.GetFeatureFlags().IsFociEnabled)
+            var logger = AuthenticationRequestParameters.RequestContext.Logger;
+
+            // FOCI is only supported on public client desktop apps
+            if (!(ServiceBundle.PlatformProxy.GetFeatureFlags().IsFociEnabled &&
+                ServiceBundle.Config.IsPublicClient))
             {
+                logger.Verbose(() =>
+                    "[FOCI] Skip FOCI on confidential client and on mobile apps");
                 return null;
             }
 
-            var logger = AuthenticationRequestParameters.RequestContext.Logger;
-
             // If the app was just added to the family, the app metadata will reflect this
             // after the first RT exchanged.
             bool? isFamilyMember = await CacheManager.IsAppFociMemberAsync(TheOnlyFamilyId).ConfigureAwait(false);
@@ -239,7 +243,8 @@ private async Task<MsalRefreshTokenCacheItem> FindRefreshTokenOrFailAsync()
             var msalRefreshTokenItem = await CacheManager.FindRefreshTokenAsync().ConfigureAwait(false);
             if (msalRefreshTokenItem == null)
             {
-                AuthenticationRequestParameters.RequestContext.Logger.Verbose(()=>"No Refresh Token was found in the cache. ");
+                AuthenticationRequestParameters.RequestContext.Logger.Warning(
+                    "No Refresh Token was found in the cache. ");
 
                 throw new MsalUiRequiredException(
                     MsalError.NoTokensFoundError,

diff --git a/src/client/Microsoft.Identity.Client/TokenCache.ITokenCacheInternal.cs b/src/client/Microsoft.Identity.Client/TokenCache.ITokenCacheInternal.cs
@@ -901,7 +901,7 @@ private static void FilterRefreshTokensByHomeAccountIdOrAssertion(
             // version of MSAL which did not record app metadata.
             if (appMetadata == null)
             {
-                logger.Warning("No app metadata found. Returning unknown. ");
+                logger.Verbose(() => "No app metadata found. Returning unknown. ");
                 return null;
             }
 
@@ -917,7 +917,9 @@ async Task<IEnumerable<IAccount>> ITokenCacheInternal.GetAccountsAsync(Authentic
         {
             var logger = requestParameters.RequestContext.Logger;
             var environment = requestParameters.AuthorityInfo.Host;
-            bool filterByClientId = !_featureFlags.IsFociEnabled;
+
+            // FOCI is only enabled on public client desktop apps
+            bool filterByClientId = !(requestParameters.AppConfig.IsPublicClient && _featureFlags.IsFociEnabled);
 
             // this will either be the home account ID or null, it can never be OBO assertion or tenant ID
             string partitionKey = CacheKeyFactory.GetKeyFromRequest(requestParameters);

diff --git a/tests/Microsoft.Identity.Test.Unit/RequestsTests/FociTests.cs b/tests/Microsoft.Identity.Test.Unit/RequestsTests/FociTests.cs
@@ -69,6 +69,39 @@ public async Task FociHappyPathAsync()
             }
         }
 
+        [TestMethod]
+        public async Task NoFociForCca()
+        {
+            // Arrange
+            using (_harness = CreateTestHarness())
+            {
+                InitApps();
+
+                var appC = ConfidentialClientApplicationBuilder
+                    .Create("other_client_id")
+                    .WithClientSecret(TestConstants.ClientSecret)
+                    .WithHttpManager(_harness.HttpManager)
+                    .WithAuthority(TestConstants.AuthorityUtidTenant)
+                    .BuildConcrete();
+
+                ConfigureCacheSerialization(appC);
+
+                // Act
+                await InteractiveAsync(_appA, ServerTokenResponse.FociToken).ConfigureAwait(false);
+                var appAAccount= (await _appA.GetAccountsAsync().ConfigureAwait(false)).Single();
+                var appCAccount = await appC.GetAccountAsync(appAAccount.HomeAccountId.Identifier).ConfigureAwait(false);
+
+                // Assert
+                Assert.IsNull(appCAccount);
+
+                // Act
+                var ex = await AssertException.TaskThrowsAsync<MsalUiRequiredException>(
+                    () => appC.AcquireTokenSilent(TestConstants.s_scope, appAAccount).ExecuteAsync()).ConfigureAwait(false);
+
+                Assert.AreEqual(MsalError.NoTokensFoundError, ex.ErrorCode);
+            }
+        }
+
         /// <summary>
         /// A is part of the family, B is not. B fails gracefully trying to get a token silently
         /// </summary>
@@ -375,15 +408,15 @@ private void InitApps()
             ConfigureCacheSerialization(_appB);
         }
 
-        private void ConfigureCacheSerialization(IPublicClientApplication pca)
+        private void ConfigureCacheSerialization(IClientApplicationBase app)
         {
-            pca.UserTokenCache.SetBeforeAccess(notificationArgs =>
+            app.UserTokenCache.SetBeforeAccess(notificationArgs =>
             {
                 byte[] bytes = Encoding.UTF8.GetBytes(_inMemoryCache);
                 notificationArgs.TokenCache.DeserializeMsalV3(bytes);
             });
 
-            pca.UserTokenCache.SetAfterAccess(notificationArgs =>
+            app.UserTokenCache.SetAfterAccess(notificationArgs =>
             {
                 if (notificationArgs.HasStateChanged)
                 {