From 890bdbafcbe364f55675c6c20740bc580773984c Mon Sep 17 00:00:00 2001 From: prsaminathan Date: Thu, 16 Jul 2026 14:43:02 -0700 Subject: [PATCH 1/3] Common: gate silent shared-FOCI refresh-token redemption on caller authorization (AB#3687466) Problem ------- BaseController.getAccountWithFRTIfAvailable redeems the device-wide shared family-of-client-id (FOCI) refresh token for any silent request whose named client id belongs to a token family, without checking whether the calling app is authorized to share FOCI tokens. On the brokered silent path this lets an unauthorized local app mint a token from another app's shared FOCI refresh token by naming a first-party family client id. Phased delivery --------------- AB#3687466 is delivered in phases, each as its own independently reviewable and independently flighted PR: - Phase 1: re-pin the silent bound-service caller on acquireTokenSilently. - Phase 2: gate device-wide FOCI account enumeration on getAccounts. - Phase 3 (this PR): gate silent shared-FOCI token redemption (broker-side backstop). - Phase 4: staged flight rollout (observe -> enforce). Cross-repo changes merge common-first, then broker; this Common change is the base dependency for the corresponding broker PR. Solution -------- Carry the caller's FOCI authorization on the silent command parameters and gate the shared-FOCI redemption on it, while leaving the caller's own UID-partitioned cache path untouched: - Add SilentTokenCommandParameters.callerAuthorizedForFoci, a @Builder.Default boolean defaulting to true. Default-true keeps non-brokered silent flows (e.g. LocalMSALController, which owns its tokens and has no cross-app "authorized to share" concept) and every existing caller unaffected; no interface change. - In BaseController.getAccountWithFRTIfAvailable, before redeeming the shared family refresh token, return null when callerAuthorizedForFoci is false. This falls through to the caller's own UID-partitioned cache only (same as the "no FRT found" path), so an unauthorized caller never has the device-wide shared FOCI RT redeemed on its behalf. The authorization decision itself is evaluated in the broker layer and set on the parameters; this change is a pure, backward-compatible controller-layer gate with no schema change. Testing ------- Unit tests (BaseControllerFociRedemptionGateTest): - callerAuthorizedForFoci = false -> shared FoCI RT redemption skipped (returns null, falls through to the caller's own cache); - callerAuthorizedForFoci = true -> redemption proceeds (no regression for legitimate family apps and non-brokered callers). Work item: AB#3687466 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0436fb4a-df54-48de-a086-55aadb1eb200 --- changelog.txt | 1 + .../SilentTokenCommandParameters.java | 14 +++ .../java/controllers/BaseController.java | 10 ++ .../BaseControllerFociRedemptionGateTest.java | 111 ++++++++++++++++++ 4 files changed, 136 insertions(+) create mode 100644 common4j/src/test/com/microsoft/identity/common/java/controllers/BaseControllerFociRedemptionGateTest.java diff --git a/changelog.txt b/changelog.txt index e14f90c817..a6d7e7631b 100644 --- a/changelog.txt +++ b/changelog.txt @@ -1,5 +1,6 @@ vNext ---------- +- [MINOR] Add SilentTokenCommandParameters.callerAuthorizedForFoci (default true) and gate shared FoCI refresh-token redemption in BaseController.getAccountWithFRTIfAvailable on it: an unauthorized caller no longer has the device-wide shared family refresh token redeemed on its behalf (falls through to its own UID-partitioned cache). Default true keeps non-brokered callers unaffected (AB#3687466) (#TBD) Version 24.5.0 ---------- diff --git a/common4j/src/main/com/microsoft/identity/common/java/commands/parameters/SilentTokenCommandParameters.java b/common4j/src/main/com/microsoft/identity/common/java/commands/parameters/SilentTokenCommandParameters.java index 055b6f2d24..db6ddcd32a 100644 --- a/common4j/src/main/com/microsoft/identity/common/java/commands/parameters/SilentTokenCommandParameters.java +++ b/common4j/src/main/com/microsoft/identity/common/java/commands/parameters/SilentTokenCommandParameters.java @@ -31,6 +31,7 @@ import com.microsoft.identity.common.java.exception.ClientException; import com.microsoft.identity.common.java.logging.Logger; +import lombok.Builder; import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.experimental.SuperBuilder; @@ -42,6 +43,19 @@ public class SilentTokenCommandParameters extends TokenCommandParameters { private static final String TAG = SilentTokenCommandParameters.class.getSimpleName(); + /** + * Whether the calling app is authorized to redeem the device-wide shared family-of-client-id (FoCI) + * refresh token. When {@code false}, the controller must not redeem the shared FoCI refresh token on + * this caller's behalf and falls through to the caller's own UID-partitioned cache only. + *

+ * Defaults to {@code true} (permissive) so non-brokered silent flows (e.g. {@code LocalMSALController}, + * which owns its tokens and has no cross-app "authorized to share" concept) are unaffected. The Broker + * explicitly sets this from {@code isAuthorizedToShareTokens(callingUid)} for brokered silent requests. + * See AB#3687466. + */ + @Builder.Default + private boolean callerAuthorizedForFoci = true; + @Override public void validate() throws ArgumentException, ClientException { super.validate(); diff --git a/common4j/src/main/com/microsoft/identity/common/java/controllers/BaseController.java b/common4j/src/main/com/microsoft/identity/common/java/controllers/BaseController.java index 03aea0aee7..10e8a4b47f 100644 --- a/common4j/src/main/com/microsoft/identity/common/java/controllers/BaseController.java +++ b/common4j/src/main/com/microsoft/identity/common/java/controllers/BaseController.java @@ -1086,6 +1086,16 @@ private AccountRecord getAccountWithFRTIfAvailable(@NonNull final SilentTokenCom .getFamilyRefreshTokenForHomeAccountId(homeAccountId); if (refreshTokenRecord != null) { + // Gate the device-wide shared FoCI refresh-token redemption: an unauthorized caller must not + // have the shared family refresh token redeemed on its behalf. Fall through to the caller's own + // UID-partitioned cache only (return null, same as the "no FRT found" path). The Broker sets + // this flag from isAuthorizedToShareTokens(callingUid); non-brokered callers default to true and + // are unaffected. See AB#3687466. + if (!parameters.isCallerAuthorizedForFoci()) { + Logger.info(methodTag, + "Caller not authorized to share FoCI tokens; skipping shared FoCI RT redemption."); + return null; + } try { // foci token is available, make a request to service to see if the client id is FOCI and save the tokens FociQueryUtilities.tryFociTokenWithGivenClientId( diff --git a/common4j/src/test/com/microsoft/identity/common/java/controllers/BaseControllerFociRedemptionGateTest.java b/common4j/src/test/com/microsoft/identity/common/java/controllers/BaseControllerFociRedemptionGateTest.java new file mode 100644 index 0000000000..1c44f30de3 --- /dev/null +++ b/common4j/src/test/com/microsoft/identity/common/java/controllers/BaseControllerFociRedemptionGateTest.java @@ -0,0 +1,111 @@ +// Copyright (c) Microsoft Corporation. +// All rights reserved. +// +// This code is licensed under the MIT License. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files(the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and / or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions : +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. +package com.microsoft.identity.common.java.controllers; + +import static org.junit.Assert.assertNull; +import static org.mockito.ArgumentMatchers.any; + +import com.microsoft.identity.common.java.cache.MsalOAuth2TokenCache; +import com.microsoft.identity.common.java.commands.parameters.SilentTokenCommandParameters; +import com.microsoft.identity.common.java.dto.AccountRecord; +import com.microsoft.identity.common.java.dto.RefreshTokenRecord; +import com.microsoft.identity.common.java.foci.FociQueryUtilities; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; +import org.mockito.MockedStatic; +import org.mockito.Mockito; + +/** + * Unit tests for the silent shared-FoCI refresh-token redemption gate reached via + * {@link BaseController#getCachedAccountRecordFromAllCaches} (implemented in the private + * {@code getAccountWithFRTIfAvailable}) — AB#3687466. + * + *

The gate skips redemption of the device-wide shared family refresh token when the caller is not + * authorized to share FoCI tokens ({@code callerAuthorizedForFoci == false}), falling through to the + * caller's own UID-partitioned cache (returns {@code null}) instead of calling + * {@link FociQueryUtilities#tryFociTokenWithGivenClientId}. + */ +@RunWith(JUnit4.class) +public class BaseControllerFociRedemptionGateTest { + + private static final String HOME_ACCOUNT_ID = "home.account.id"; + private static final String LOCAL_ACCOUNT_ID = "local.account.id"; + private static final String CLIENT_ID = "test-client-id"; + private static final String REDIRECT_URI = "msauth://redirect"; + + private static SilentTokenCommandParameters paramsWithSharedFociRt(final boolean callerAuthorizedForFoci, + final MsalOAuth2TokenCache cache) { + // A shared family refresh token exists for the home account id. + Mockito.when(cache.getFamilyRefreshTokenForHomeAccountId(HOME_ACCOUNT_ID)) + .thenReturn(new RefreshTokenRecord()); + + final AccountRecord account = new AccountRecord(); + account.setHomeAccountId(HOME_ACCOUNT_ID); + account.setLocalAccountId(LOCAL_ACCOUNT_ID); + + final SilentTokenCommandParameters parameters = Mockito.mock(SilentTokenCommandParameters.class); + Mockito.when(parameters.getOAuth2TokenCache()).thenReturn(cache); + Mockito.when(parameters.getAccount()).thenReturn(account); + Mockito.when(parameters.getClientId()).thenReturn(CLIENT_ID); + Mockito.when(parameters.getRedirectUri()).thenReturn(REDIRECT_URI); + Mockito.when(parameters.isCallerAuthorizedForFoci()).thenReturn(callerAuthorizedForFoci); + return parameters; + } + + @Test + public void getCachedAccountRecordFromAllCaches_callerNotAuthorizedForFoci_skipsFrtRedemption() + throws Exception { + final MsalOAuth2TokenCache cache = Mockito.mock(MsalOAuth2TokenCache.class); + final SilentTokenCommandParameters parameters = paramsWithSharedFociRt(false, cache); + final BaseController controller = Mockito.mock(BaseController.class, Mockito.CALLS_REAL_METHODS); + + try (final MockedStatic foci = Mockito.mockStatic(FociQueryUtilities.class)) { + final AccountRecord result = controller.getCachedAccountRecordFromAllCaches(parameters); + + assertNull(result); + // The shared FoCI refresh token must NOT be redeemed for an unauthorized caller. + foci.verify( + () -> FociQueryUtilities.tryFociTokenWithGivenClientId(any(), any(), any(), any(), any()), + Mockito.never()); + } + } + + @Test + public void getCachedAccountRecordFromAllCaches_callerAuthorizedForFoci_attemptsFrtRedemption() + throws Exception { + final MsalOAuth2TokenCache cache = Mockito.mock(MsalOAuth2TokenCache.class); + final SilentTokenCommandParameters parameters = paramsWithSharedFociRt(true, cache); + final BaseController controller = Mockito.mock(BaseController.class, Mockito.CALLS_REAL_METHODS); + + try (final MockedStatic foci = Mockito.mockStatic(FociQueryUtilities.class)) { + controller.getCachedAccountRecordFromAllCaches(parameters); + + // An authorized caller proceeds to redeem the shared FoCI refresh token (pre-fix behavior). + foci.verify( + () -> FociQueryUtilities.tryFociTokenWithGivenClientId(any(), any(), any(), any(), any()), + Mockito.times(1)); + } + } +} From 95a6d16764648cc3e127e82fc507557c1a6f3f79 Mon Sep 17 00:00:00 2001 From: prsaminathan Date: Thu, 16 Jul 2026 15:56:41 -0700 Subject: [PATCH 2/3] Common: fill in PR number (#3188) in changelog for the silent shared-FoCI redemption gate (AB#3687466) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0436fb4a-df54-48de-a086-55aadb1eb200 --- changelog.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.txt b/changelog.txt index a6d7e7631b..7215ffe612 100644 --- a/changelog.txt +++ b/changelog.txt @@ -1,6 +1,6 @@ vNext ---------- -- [MINOR] Add SilentTokenCommandParameters.callerAuthorizedForFoci (default true) and gate shared FoCI refresh-token redemption in BaseController.getAccountWithFRTIfAvailable on it: an unauthorized caller no longer has the device-wide shared family refresh token redeemed on its behalf (falls through to its own UID-partitioned cache). Default true keeps non-brokered callers unaffected (AB#3687466) (#TBD) +- [MINOR] Add SilentTokenCommandParameters.callerAuthorizedForFoci (default true) and gate shared FoCI refresh-token redemption in BaseController.getAccountWithFRTIfAvailable on it: an unauthorized caller no longer has the device-wide shared family refresh token redeemed on its behalf (falls through to its own UID-partitioned cache). Default true keeps non-brokered callers unaffected (AB#3687466) (#3188) Version 24.5.0 ---------- From ba9b400431c399bc560eb52e1f2123d4dc8807f1 Mon Sep 17 00:00:00 2001 From: prsaminathan Date: Tue, 21 Jul 2026 10:58:38 -0700 Subject: [PATCH 3/3] Common: address PR #3188 review feedback on the silent shared-FOCI redemption gate (AB#3687466) Addresses @shahzaibj's review comments: - Add a real-builder test (silentTokenCommandParameters_builtWithoutCallerAuth_defaultsToAuthorized) that builds a SilentTokenCommandParameters without setting caller-auth and asserts isCallerAuthorizedForFoci() returns true. The existing tests mock the getter, so the security-critical @Builder.Default = true invariant was never exercised; if it were dropped, Lombok's @SuperBuilder would default the primitive to false and silent FOCI redemption would fail closed for every non-brokered caller. This now catches that at test time. - Add @Expose to callerAuthorizedForFoci so the deciding flag appears in the redacted logParameters output (serializeExposedFieldsOfObjectToJsonString), matching callerPackageName/callerSignature. Useful for "why did silent fail for this app" triage without needing PII logging. - Clarify the field KDoc: the gate lives in shared BaseController code that non-brokered flows also execute; those callers never set the flag and inherit the permissive default, so the gate is a no-op for them. Work item: AB#3687466 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0436fb4a-df54-48de-a086-55aadb1eb200 --- .../SilentTokenCommandParameters.java | 10 +++++++--- .../BaseControllerFociRedemptionGateTest.java | 17 +++++++++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/common4j/src/main/com/microsoft/identity/common/java/commands/parameters/SilentTokenCommandParameters.java b/common4j/src/main/com/microsoft/identity/common/java/commands/parameters/SilentTokenCommandParameters.java index db6ddcd32a..b07d70c5da 100644 --- a/common4j/src/main/com/microsoft/identity/common/java/commands/parameters/SilentTokenCommandParameters.java +++ b/common4j/src/main/com/microsoft/identity/common/java/commands/parameters/SilentTokenCommandParameters.java @@ -31,6 +31,7 @@ import com.microsoft.identity.common.java.exception.ClientException; import com.microsoft.identity.common.java.logging.Logger; +import com.google.gson.annotations.Expose; import lombok.Builder; import lombok.EqualsAndHashCode; import lombok.Getter; @@ -48,11 +49,14 @@ public class SilentTokenCommandParameters extends TokenCommandParameters { * refresh token. When {@code false}, the controller must not redeem the shared FoCI refresh token on * this caller's behalf and falls through to the caller's own UID-partitioned cache only. *

- * Defaults to {@code true} (permissive) so non-brokered silent flows (e.g. {@code LocalMSALController}, - * which owns its tokens and has no cross-app "authorized to share" concept) are unaffected. The Broker - * explicitly sets this from {@code isAuthorizedToShareTokens(callingUid)} for brokered silent requests. + * Defaults to {@code true} (permissive). The redemption gate lives in shared {@code BaseController} + * code that non-brokered silent flows (e.g. {@code LocalMSALController}, which owns its tokens and has + * no cross-app "authorized to share" concept) also execute; those callers never set this flag and + * inherit the permissive default, so the gate is a no-op for them. The Broker explicitly sets this from + * {@code isAuthorizedToShareTokens(callingUid)} for brokered silent requests. * See AB#3687466. */ + @Expose() @Builder.Default private boolean callerAuthorizedForFoci = true; diff --git a/common4j/src/test/com/microsoft/identity/common/java/controllers/BaseControllerFociRedemptionGateTest.java b/common4j/src/test/com/microsoft/identity/common/java/controllers/BaseControllerFociRedemptionGateTest.java index 1c44f30de3..f9002a132d 100644 --- a/common4j/src/test/com/microsoft/identity/common/java/controllers/BaseControllerFociRedemptionGateTest.java +++ b/common4j/src/test/com/microsoft/identity/common/java/controllers/BaseControllerFociRedemptionGateTest.java @@ -23,6 +23,7 @@ package com.microsoft.identity.common.java.controllers; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import com.microsoft.identity.common.java.cache.MsalOAuth2TokenCache; @@ -30,6 +31,7 @@ import com.microsoft.identity.common.java.dto.AccountRecord; import com.microsoft.identity.common.java.dto.RefreshTokenRecord; import com.microsoft.identity.common.java.foci.FociQueryUtilities; +import com.microsoft.identity.common.java.interfaces.IPlatformComponents; import org.junit.Test; import org.junit.runner.RunWith; @@ -108,4 +110,19 @@ public void getCachedAccountRecordFromAllCaches_callerAuthorizedForFoci_attempts Mockito.times(1)); } } + + /** + * Guards the secure-by-default invariant: a real {@link SilentTokenCommandParameters} built without + * explicitly setting caller-auth must report {@code true}. If {@code @Builder.Default = true} were + * dropped, Lombok's generated builder would default the primitive to {@code false} and silent FoCI + * redemption would start failing closed for every non-brokered caller. AB#3687466. + */ + @Test + public void silentTokenCommandParameters_builtWithoutCallerAuth_defaultsToAuthorized() { + final SilentTokenCommandParameters parameters = SilentTokenCommandParameters.builder() + .platformComponents(Mockito.mock(IPlatformComponents.class)) + .build(); + + assertTrue(parameters.isCallerAuthorizedForFoci()); + } }