Skip to content

Commit dabefab

Browse files
siddhijainCopilotCopilot
authored
Fixes AB#3588022 Optimize load() and getIdTokensForAccountRecord() with filter-then-clone (#3100)
Fixes [AB#3588022](https://identitydivision.visualstudio.com/fac9d424-53d2-45c0-91b5-ef6ba7a6bf26/_workitems/edit/3588022) Apply filter-then-clone optimization to MsalOAuth2TokenCache.load() and getIdTokensForAccountRecord() methods behind the existing ENABLE_FILTER_THEN_CLONE_IN_MEMORY_CACHE flight. When the flight is enabled, these methods now call the direct (non-input-list) getCredentialsFilteredBy overloads which filter on uncloned in-memory references first, then clone only the matching credentials. This avoids cloning all cached credentials on every AcquireTokenSilent cache-hit path. Changes: - Add new getCredentialsFilteredBy overload with kid parameter to IAccountCredentialCache, SharedPreferencesAccountCredentialCache, and SharedPreferencesAccountCredentialCacheWithMemoryCache - Gate load() to skip preload pattern and use direct filtered lookups when flight is enabled - Gate getIdTokensForAccountRecord() with same optimization pattern - Add tests for new kid overload (flight enabled/disabled) - Add MsalOAuth2TokenCache-level tests for flight-gated load() and getIdTokensForAccountRecord() paths --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: siddhijain <30181294+siddhijain@users.noreply.github.com>
1 parent a68f495 commit dabefab

7 files changed

Lines changed: 599 additions & 84 deletions

File tree

‎changelog.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
vNext
22
----------
3+
- [PATCH] Extend filter-then-clone optimization to load() and getIdTokensForAccountRecord() in MsalOAuth2TokenCache: when ENABLE_FILTER_THEN_CLONE_IN_MEMORY_CACHE flight is enabled, skip clone-all preload and call direct flight-gated overloads that clone only matching credentials; add new getCredentialsFilteredBy overload with kid support (#3100)
34
- [PATCH] Move Multiple Listening apps check to the authorization layer (#3070)
45

56
Version 24.2.0

‎common/src/test/java/com/microsoft/identity/common/MsalOAuth2TokenCacheTest.java‎

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929

3030
import com.microsoft.identity.common.components.AndroidPlatformComponentsFactory;
3131
import com.microsoft.identity.common.components.MockPlatformComponentsFactory;
32+
import com.microsoft.identity.common.internal.mocks.MockCommonFlightsManager;
3233
import com.microsoft.identity.common.internal.platform.AndroidPlatformUtil;
3334
import com.microsoft.identity.common.java.interfaces.IPlatformComponents;
3435
import com.microsoft.identity.common.java.providers.microsoft.microsoftsts.MicrosoftStsOAuth2Strategy;
@@ -40,13 +41,17 @@
4041
import com.microsoft.identity.common.java.cache.ICacheRecord;
4142
import com.microsoft.identity.common.java.cache.MsalOAuth2TokenCache;
4243
import com.microsoft.identity.common.java.cache.SharedPreferencesAccountCredentialCache;
44+
import com.microsoft.identity.common.java.cache.SharedPreferencesAccountCredentialCacheWithMemoryCache;
4345
import com.microsoft.identity.common.java.dto.AccessTokenRecord;
4446
import com.microsoft.identity.common.java.dto.AccountRecord;
4547
import com.microsoft.identity.common.java.dto.Credential;
4648
import com.microsoft.identity.common.java.dto.CredentialType;
4749
import com.microsoft.identity.common.java.dto.IdTokenRecord;
4850
import com.microsoft.identity.common.java.dto.PrimaryRefreshTokenRecord;
4951
import com.microsoft.identity.common.java.dto.RefreshTokenRecord;
52+
import com.microsoft.identity.common.java.flighting.CommonFlight;
53+
import com.microsoft.identity.common.java.flighting.CommonFlightsManager;
54+
import com.microsoft.identity.common.java.flighting.IFlightsProvider;
5055
import com.microsoft.identity.common.java.interfaces.INameValueStorage;
5156
import com.microsoft.identity.common.java.providers.microsoft.MicrosoftAccount;
5257
import com.microsoft.identity.common.java.providers.microsoft.MicrosoftRefreshToken;
@@ -59,6 +64,7 @@
5964
import org.junit.Before;
6065
import org.junit.Test;
6166
import org.junit.runner.RunWith;
67+
import org.mockito.Mockito;
6268
import org.powermock.api.mockito.PowerMockito;
6369
import org.robolectric.RobolectricTestRunner;
6470
import org.robolectric.annotation.Config;
@@ -1607,4 +1613,155 @@ public void testGetFamilyRefreshTokenForHomeAccountIdNoAccountWithHomeAccountId(
16071613
getFamilyRefreshTokenForHomeAccountId("26685724-1f8e-4b97-a0ca-1863e33b9fb1"); // different home account id
16081614
assertNull(refreshTokenRecord);
16091615
}
1616+
1617+
// =====================================================================
1618+
// Flight-gated tests for filter-then-clone optimization in load() and getIdTokensForAccountRecord()
1619+
// =====================================================================
1620+
1621+
private MsalOAuth2TokenCache<MicrosoftStsOAuth2Strategy, MicrosoftStsAuthorizationRequest,
1622+
MicrosoftStsTokenResponse, MicrosoftAccount, MicrosoftRefreshToken>
1623+
createCacheWithMemoryCache(final IPlatformComponents components) {
1624+
final ICacheKeyValueDelegate keyValueDelegate = new CacheKeyValueDelegate();
1625+
final INameValueStorage<String> storage = components.getStorageSupplier().getEncryptedNameValueStore(
1626+
"test_prefs_memory_cache",
1627+
String.class
1628+
);
1629+
final IAccountCredentialCache memoryCache = new SharedPreferencesAccountCredentialCacheWithMemoryCache(
1630+
keyValueDelegate,
1631+
storage
1632+
);
1633+
return new MsalOAuth2TokenCache<>(
1634+
components,
1635+
memoryCache,
1636+
mockCredentialAdapter
1637+
);
1638+
}
1639+
1640+
private void enableFilterThenCloneFlight() {
1641+
CommonFlightsManager.INSTANCE.resetFlightsManager();
1642+
final IFlightsProvider mockFlightsProvider = Mockito.mock(IFlightsProvider.class);
1643+
when(mockFlightsProvider.isFlightEnabled(CommonFlight.ENABLE_FILTER_THEN_CLONE_IN_MEMORY_CACHE))
1644+
.thenReturn(true);
1645+
final MockCommonFlightsManager mockFlightsManager = new MockCommonFlightsManager();
1646+
mockFlightsManager.setMockCommonFlightsProvider(mockFlightsProvider);
1647+
CommonFlightsManager.INSTANCE.initializeCommonFlightsManager(mockFlightsManager);
1648+
}
1649+
1650+
private void resetFlight() {
1651+
CommonFlightsManager.INSTANCE.resetFlightsManager();
1652+
}
1653+
1654+
@Test
1655+
public void loadTokens_flightEnabled_returnsCorrectCacheRecord() throws ClientException {
1656+
enableFilterThenCloneFlight();
1657+
try {
1658+
final IPlatformComponents components = MockPlatformComponentsFactory.getNonFunctionalBuilder().build();
1659+
final MsalOAuth2TokenCache<MicrosoftStsOAuth2Strategy, MicrosoftStsAuthorizationRequest,
1660+
MicrosoftStsTokenResponse, MicrosoftAccount, MicrosoftRefreshToken>
1661+
memoryCacheTokenCache = createCacheWithMemoryCache(components);
1662+
1663+
configureMocksForTestBundle(defaultTestBundleV2);
1664+
final ICacheRecord result = memoryCacheTokenCache.save(
1665+
mockStrategy,
1666+
mockRequest,
1667+
mockResponse
1668+
);
1669+
1670+
final ICacheRecord secondaryLoad = memoryCacheTokenCache.load(
1671+
CLIENT_ID,
1672+
APPLICATION_IDENTIFIER_SHA512,
1673+
MAM_ENROLLMENT_IDENTIFIER,
1674+
TARGET,
1675+
defaultTestBundleV2.mGeneratedAccount,
1676+
BEARER_AUTHENTICATION_SCHEME
1677+
);
1678+
1679+
assertEquals(result.getAccount(), secondaryLoad.getAccount());
1680+
assertEquals(result.getAccessToken(), secondaryLoad.getAccessToken());
1681+
assertEquals(result.getRefreshToken(), secondaryLoad.getRefreshToken());
1682+
assertEquals(result.getIdToken(), secondaryLoad.getIdToken());
1683+
} finally {
1684+
resetFlight();
1685+
}
1686+
}
1687+
1688+
@Test
1689+
public void loadTokensV1Compat_flightEnabled_returnsCorrectCacheRecord() throws ClientException {
1690+
enableFilterThenCloneFlight();
1691+
try {
1692+
final IPlatformComponents components = MockPlatformComponentsFactory.getNonFunctionalBuilder().build();
1693+
final MsalOAuth2TokenCache<MicrosoftStsOAuth2Strategy, MicrosoftStsAuthorizationRequest,
1694+
MicrosoftStsTokenResponse, MicrosoftAccount, MicrosoftRefreshToken>
1695+
memoryCacheTokenCache = createCacheWithMemoryCache(components);
1696+
1697+
configureMocksForTestBundle(defaultTestBundleV1);
1698+
final ICacheRecord result = memoryCacheTokenCache.save(
1699+
mockStrategy,
1700+
mockRequest,
1701+
mockResponse
1702+
);
1703+
1704+
final ICacheRecord secondaryLoad = memoryCacheTokenCache.load(
1705+
CLIENT_ID,
1706+
APPLICATION_IDENTIFIER_SHA512,
1707+
MAM_ENROLLMENT_IDENTIFIER,
1708+
TARGET,
1709+
defaultTestBundleV1.mGeneratedAccount,
1710+
BEARER_AUTHENTICATION_SCHEME
1711+
);
1712+
1713+
assertEquals(result.getAccount(), secondaryLoad.getAccount());
1714+
assertEquals(result.getAccessToken(), secondaryLoad.getAccessToken());
1715+
assertEquals(result.getRefreshToken(), secondaryLoad.getRefreshToken());
1716+
assertEquals(result.getV1IdToken(), secondaryLoad.getV1IdToken());
1717+
} finally {
1718+
resetFlight();
1719+
}
1720+
}
1721+
1722+
@Test
1723+
public void getIdTokensForAccountRecord_flightEnabled_returnsCorrectIdTokens() throws ClientException {
1724+
enableFilterThenCloneFlight();
1725+
try {
1726+
final IPlatformComponents components = MockPlatformComponentsFactory.getNonFunctionalBuilder().build();
1727+
final MsalOAuth2TokenCache<MicrosoftStsOAuth2Strategy, MicrosoftStsAuthorizationRequest,
1728+
MicrosoftStsTokenResponse, MicrosoftAccount, MicrosoftRefreshToken>
1729+
memoryCacheTokenCache = createCacheWithMemoryCache(components);
1730+
1731+
configureMocksForTestBundle(defaultTestBundleV2);
1732+
memoryCacheTokenCache.save(
1733+
mockStrategy,
1734+
mockRequest,
1735+
mockResponse
1736+
);
1737+
1738+
final List<IdTokenRecord> idTokens = memoryCacheTokenCache.getIdTokensForAccountRecord(
1739+
CLIENT_ID,
1740+
defaultTestBundleV2.mGeneratedAccount
1741+
);
1742+
1743+
assertEquals(1, idTokens.size());
1744+
assertEquals(defaultTestBundleV2.mGeneratedIdToken, idTokens.get(0));
1745+
} finally {
1746+
resetFlight();
1747+
}
1748+
}
1749+
1750+
@Test
1751+
public void getIdTokensForAccountRecord_flightDisabled_returnsCorrectIdTokens() throws ClientException {
1752+
// Ensure the existing non-flighted path works unchanged
1753+
final ICacheRecord result = mOauth2TokenCache.save(
1754+
mockStrategy,
1755+
mockRequest,
1756+
mockResponse
1757+
);
1758+
1759+
final List<IdTokenRecord> idTokens = mOauth2TokenCache.getIdTokensForAccountRecord(
1760+
CLIENT_ID,
1761+
defaultTestBundleV2.mGeneratedAccount
1762+
);
1763+
1764+
assertEquals(1, idTokens.size());
1765+
assertEquals(defaultTestBundleV2.mGeneratedIdToken, idTokens.get(0));
1766+
}
16101767
}

‎common/src/test/java/com/microsoft/identity/common/SharedPreferencesAccountCredentialCacheWithMemoryCacheTest.java‎

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2843,4 +2843,125 @@ public void removeCredential_doesNotCallKeySetOrGetAll() throws Exception {
28432843
assertEquals("removeCredential() must not call keySet()", 0, trackingStorage.getKeySetCallCount());
28442844
assertEquals("removeCredential() must not call getAll()", 0, trackingStorage.getAllCallCount());
28452845
}
2846+
2847+
// =====================================================================
2848+
// Tests for new getCredentialsFilteredBy overload with kid (no input list)
2849+
// =====================================================================
2850+
2851+
@Test
2852+
public void getCredentialsFilteredByWithKid_flightDisabled_returnsClonesFromAllCredentials() {
2853+
// Save an access token with kid = "kid1"
2854+
final AccessTokenRecord at = new AccessTokenRecord();
2855+
at.setCredentialType(CredentialType.AccessToken.name());
2856+
at.setHomeAccountId(HOME_ACCOUNT_ID);
2857+
at.setEnvironment(ENVIRONMENT);
2858+
at.setClientId(CLIENT_ID);
2859+
at.setRealm(REALM);
2860+
at.setTarget(TARGET);
2861+
at.setCachedAt(CACHED_AT);
2862+
at.setExpiresOn(EXPIRES_ON);
2863+
at.setSecret(SECRET);
2864+
at.setKid("kid1");
2865+
mSharedPreferencesAccountCredentialCache.saveCredential(at);
2866+
2867+
// Also save a non-matching credential type
2868+
final RefreshTokenRecord rt = buildDefaultRefreshToken();
2869+
mSharedPreferencesAccountCredentialCache.saveCredential(rt);
2870+
2871+
// Matching kid = "kid1" should return the AT
2872+
final List<Credential> matchingKid = mSharedPreferencesAccountCredentialCache
2873+
.getCredentialsFilteredBy(
2874+
HOME_ACCOUNT_ID, ENVIRONMENT, CredentialType.AccessToken,
2875+
CLIENT_ID, null, null, REALM, TARGET,
2876+
null, null, "kid1");
2877+
assertEquals(1, matchingKid.size());
2878+
assertEquals(CredentialType.AccessToken.name(), matchingKid.get(0).getCredentialType());
2879+
2880+
// Non-matching kid = "kid2" should return nothing
2881+
final List<Credential> nonMatchingKid = mSharedPreferencesAccountCredentialCache
2882+
.getCredentialsFilteredBy(
2883+
HOME_ACCOUNT_ID, ENVIRONMENT, CredentialType.AccessToken,
2884+
CLIENT_ID, null, null, REALM, TARGET,
2885+
null, null, "kid2");
2886+
assertEquals(0, nonMatchingKid.size());
2887+
2888+
// kid = null should return the AT (no kid filter applied)
2889+
final List<Credential> nullKid = mSharedPreferencesAccountCredentialCache
2890+
.getCredentialsFilteredBy(
2891+
HOME_ACCOUNT_ID, ENVIRONMENT, CredentialType.AccessToken,
2892+
CLIENT_ID, null, null, REALM, TARGET,
2893+
null, null, (String) null);
2894+
assertEquals(1, nullKid.size());
2895+
assertEquals(CredentialType.AccessToken.name(), nullKid.get(0).getCredentialType());
2896+
2897+
// Returned objects should be clones — mutating should not affect cache
2898+
matchingKid.get(0).setCachedAt("mutated");
2899+
final List<Credential> afterMutation = mSharedPreferencesAccountCredentialCache
2900+
.getCredentialsFilteredBy(
2901+
HOME_ACCOUNT_ID, ENVIRONMENT, CredentialType.AccessToken,
2902+
CLIENT_ID, null, null, REALM, TARGET,
2903+
null, null, "kid1");
2904+
assertNotEquals("mutated", afterMutation.get(0).getCachedAt());
2905+
}
2906+
2907+
@Test
2908+
public void getCredentialsFilteredByWithKid_flightEnabled_returnsClonedMatchesOnly() {
2909+
enableFilterThenCloneFlight();
2910+
try {
2911+
// Save an access token with kid = "kid1"
2912+
final AccessTokenRecord at = new AccessTokenRecord();
2913+
at.setCredentialType(CredentialType.AccessToken.name());
2914+
at.setHomeAccountId(HOME_ACCOUNT_ID);
2915+
at.setEnvironment(ENVIRONMENT);
2916+
at.setClientId(CLIENT_ID);
2917+
at.setRealm(REALM);
2918+
at.setTarget(TARGET);
2919+
at.setCachedAt(CACHED_AT);
2920+
at.setExpiresOn(EXPIRES_ON);
2921+
at.setSecret(SECRET);
2922+
at.setKid("kid1");
2923+
mSharedPreferencesAccountCredentialCache.saveCredential(at);
2924+
2925+
// Also save a non-matching credential type
2926+
final RefreshTokenRecord rt = buildDefaultRefreshToken();
2927+
mSharedPreferencesAccountCredentialCache.saveCredential(rt);
2928+
2929+
// Matching kid = "kid1" should return the AT
2930+
final List<Credential> matchingKid = mSharedPreferencesAccountCredentialCache
2931+
.getCredentialsFilteredBy(
2932+
HOME_ACCOUNT_ID, ENVIRONMENT, CredentialType.AccessToken,
2933+
CLIENT_ID, null, null, REALM, TARGET,
2934+
null, null, "kid1");
2935+
assertEquals(1, matchingKid.size());
2936+
assertEquals(CredentialType.AccessToken.name(), matchingKid.get(0).getCredentialType());
2937+
2938+
// Non-matching kid = "kid2" should return nothing
2939+
final List<Credential> nonMatchingKid = mSharedPreferencesAccountCredentialCache
2940+
.getCredentialsFilteredBy(
2941+
HOME_ACCOUNT_ID, ENVIRONMENT, CredentialType.AccessToken,
2942+
CLIENT_ID, null, null, REALM, TARGET,
2943+
null, null, "kid2");
2944+
assertEquals(0, nonMatchingKid.size());
2945+
2946+
// kid = null should return the AT (no kid filter applied)
2947+
final List<Credential> nullKid = mSharedPreferencesAccountCredentialCache
2948+
.getCredentialsFilteredBy(
2949+
HOME_ACCOUNT_ID, ENVIRONMENT, CredentialType.AccessToken,
2950+
CLIENT_ID, null, null, REALM, TARGET,
2951+
null, null, (String) null);
2952+
assertEquals(1, nullKid.size());
2953+
assertEquals(CredentialType.AccessToken.name(), nullKid.get(0).getCredentialType());
2954+
2955+
// Returned objects should be clones — mutating should not affect cache
2956+
matchingKid.get(0).setCachedAt("mutated");
2957+
final List<Credential> afterMutation = mSharedPreferencesAccountCredentialCache
2958+
.getCredentialsFilteredBy(
2959+
HOME_ACCOUNT_ID, ENVIRONMENT, CredentialType.AccessToken,
2960+
CLIENT_ID, null, null, REALM, TARGET,
2961+
null, null, "kid1");
2962+
assertNotEquals("mutated", afterMutation.get(0).getCachedAt());
2963+
} finally {
2964+
resetFlight();
2965+
}
2966+
}
28462967
}

‎common4j/src/main/com/microsoft/identity/common/java/cache/IAccountCredentialCache.java‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,38 @@ List<Credential> getCredentialsFilteredBy(
290290
final String kid
291291
);
292292

293+
/**
294+
* Returns all of the Credentials matching the supplied criteria, including kid filtering.
295+
* Unlike the input-list overload, this method reads directly from the cache and
296+
* benefits from filter-then-clone optimization when the corresponding flight is enabled.
297+
*
298+
* @param homeAccountId The homeAccountId used to match Credential cache keys.
299+
* @param environment The environment used to match Credential cache keys.
300+
* @param credentialType The sought CredentialType.
301+
* @param clientId The clientId used to match Credential cache keys.
302+
* @param applicationIdentifier The physical identifier of the application (Android: packageName/signature)
303+
* @param mamEnrollmentIdentifier The Mobile Application Management or Intune App Protection enrollment identifier (Android Only)
304+
* @param realm The realm used to match Credential cache keys.
305+
* @param target The target used to match Credential cache keys.
306+
* @param authScheme The auth scheme used to match Credential cache keys.
307+
* @param requestedClaims The requested claims used to match Credential cache keys.
308+
* @param kid Kid value used to match access token record.
309+
* @return A mutable List of Credentials matching the supplied criteria.
310+
*/
311+
List<Credential> getCredentialsFilteredBy(
312+
final String homeAccountId,
313+
final String environment,
314+
final CredentialType credentialType,
315+
final String clientId,
316+
final String applicationIdentifier,
317+
final String mamEnrollmentIdentifier,
318+
final String realm,
319+
final String target,
320+
final String authScheme,
321+
final String requestedClaims,
322+
final String kid
323+
);
324+
293325
/**
294326
* Removes the supplied Account from the cache.
295327
*

0 commit comments

Comments
 (0)