Skip to content

Commit 439a4fd

Browse files
rpdomerpdome
authored
Merge branch 'dev' into rapong/reject
2 parents 84b5ca6 + 16b1bc8 commit 439a4fd

8 files changed

Lines changed: 159 additions & 6 deletions

File tree

‎.github/copilot-instructions.md‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -554,6 +554,11 @@ See [changelog.txt](../changelog.txt) for the changelog format.
554554

555555
## 11. Dependencies & Versioning
556556

557+
When any change is made to a `common/build.gradle` file or the shared dependency versions file at `gradle/versions.gradle` that adds a new dependency or changes a dependency version, always display the following warning message:
558+
> ⚠️ **Warning:** Changes detected ⚠️
559+
>
560+
> Please follow the recommendations in [Adding or Updating Gradle Dependencies](https://eng.ms/docs/microsoft-security/identity/entra-developer-application-platform/auth-client/authn-sdk-msal-android/android-auth-libraries/how-tos/adding-or-updating-gradle-dependencies).
561+
557562
Flag:
558563
- Security library downgrade.
559564
- Major upgrade without referenced release notes.
@@ -674,4 +679,4 @@ Always cite specific code and give a minimal, actionable fix; use an assumption
674679

675680
---
676681

677-
Thank you for contributing to this project!
682+
Thank you for contributing to this project!

‎azure-pipelines/pull-request-validation/build-consumers.yml‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,37 @@ stages:
132132
tasks: msal:testLocalDebugUnitTest -Plabtest -PlabSecret=$(LabVaultAppCert) -ProbolectricSdkVersion=${{variables.robolectricSdkVersion}} -PmockApiUrl=$(MOCK_API_URL) -PnativeAuthConfigString=$(NATIVE_AUTH_CONFIG_STRING)
133133
jdkArchitecture: x64
134134
jdkVersionOption: "1.17"
135+
# msalautomationapp
136+
- job: msalAutomationAppValidation
137+
displayName: MSAL Automation App
138+
dependsOn:
139+
- setupBranch
140+
variables:
141+
commonBranch: $[ dependencies.setupBranch.outputs['setvarStep.commonBranch'] ] # map in the variable
142+
condition: and( succeeded(), not(contains(variables['prLabels'], variables['skipConsumerValidationLabel'])) )
143+
steps:
144+
- checkout: msal
145+
displayName: Checkout msal repository
146+
clean: true
147+
submodules: recursive
148+
persistCredentials: True
149+
- task: CmdLine@2
150+
displayName: Checkout common submodule $(commonBranch)
151+
inputs:
152+
script: |
153+
git fetch
154+
git checkout $(commonBranch)
155+
git pull
156+
git status
157+
git rev-parse HEAD
158+
workingDirectory: $(Agent.BuildDirectory)/s/common
159+
- template: ../templates/steps/automation-cert.yml
160+
- task: Gradle@3
161+
displayName: Assemble msalautomationapp
162+
inputs:
163+
jdkArchitecture: x64
164+
jdkVersionOption: "1.17"
165+
tasks: clean msalautomationapp:assembleLocal
135166
# broker
136167
- job: brokerValidation
137168
displayName: Broker

‎changelog.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ vNext
33
- [PATCH] Handle app_link Intent redirection by validating broker install links and rejecting unsupported redirect URIs with appropriate error responses (#3102)
44
- [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)
55
- [PATCH] Move Multiple Listening apps check to the authorization layer (#3070)
6+
- [PATCH] Edge TB: Fix lookup mode (#3108)
67

78
Version 24.2.0
89
----------

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

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
// THE SOFTWARE.
2323
package com.microsoft.identity.common;
2424

25+
import com.microsoft.identity.common.java.AuthenticationConstants;
2526
import com.microsoft.identity.common.java.authorities.Authority;
2627
import com.microsoft.identity.common.java.base64.Base64Flags;
2728
import com.microsoft.identity.common.java.base64.Base64Util;
@@ -39,6 +40,7 @@
3940
import com.microsoft.identity.common.java.providers.microsoft.microsoftsts.MicrosoftStsAuthorizationRequest;
4041
import com.microsoft.identity.common.java.providers.microsoft.microsoftsts.MicrosoftStsTokenResponse;
4142
import com.microsoft.identity.common.java.request.SdkType;
43+
import com.microsoft.identity.common.java.util.SchemaUtil;
4244
import com.microsoft.identity.common.java.util.StringUtil;
4345
import com.nimbusds.jose.JOSEException;
4446
import com.nimbusds.jose.JWSAlgorithm;
@@ -288,4 +290,34 @@ static String createRawClientInfo(final String uid, final String utid) {
288290
final String claims = "{\"uid\":\"" + uid + "\",\"utid\":\"" + utid + "\"}";
289291
return Base64Util.encodeToString(StringUtil.toByteArray(claims), Base64Flags.URL_SAFE);
290292
}
293+
294+
@Test
295+
public void testCreateAccountRecord_LookupModeWithRealIdToken_ParsesUsername() throws ServiceException {
296+
// In lookup mode, if the ID token is a real JWT (not "none"), username should still be parsed.
297+
// This covers the interactive fallback scenario where lookup mode params are preserved
298+
// but eSTS returns a real ID token.
299+
when(mockParameters.isLookupMode()).thenReturn(true);
300+
when(mockResponse.getIdToken()).thenReturn(MOCK_ID_TOKEN_WITH_CLAIMS);
301+
302+
final AccountRecord account = mAccountCredentialAdapter.createAccountRecord(
303+
mockParameters, SdkType.MSAL, mockResponse);
304+
305+
assertNotNull(account);
306+
assertEquals("Username should be parsed from real ID token even in lookup mode",
307+
MOCK_PREFERRED_USERNAME, account.getUsername());
308+
}
309+
310+
@Test
311+
public void testCreateAccountRecord_LookupModeWithNoneIdToken_UsernameIsMissing() throws ServiceException {
312+
// In lookup mode with id_token="none", username cannot be derived from the token.
313+
when(mockParameters.isLookupMode()).thenReturn(true);
314+
when(mockResponse.getIdToken()).thenReturn(AuthenticationConstants.Broker.LOOKUP_MODE_ID_TOKEN_VALUE);
315+
316+
final AccountRecord account = mAccountCredentialAdapter.createAccountRecord(
317+
mockParameters, SdkType.MSAL, mockResponse);
318+
319+
assertNotNull(account);
320+
assertEquals("Username should be MISSING_FROM_THE_TOKEN_RESPONSE when id_token is 'none'",
321+
SchemaUtil.MISSING_FROM_THE_TOKEN_RESPONSE, account.getUsername());
322+
}
291323
}

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

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1642,6 +1642,8 @@ private void enableFilterThenCloneFlight() {
16421642
final IFlightsProvider mockFlightsProvider = Mockito.mock(IFlightsProvider.class);
16431643
when(mockFlightsProvider.isFlightEnabled(CommonFlight.ENABLE_FILTER_THEN_CLONE_IN_MEMORY_CACHE))
16441644
.thenReturn(true);
1645+
when(mockFlightsProvider.isFlightEnabled(CommonFlight.USE_IN_MEMORY_CACHE_FOR_ACCOUNTS_AND_CREDENTIALS))
1646+
.thenReturn(true);
16451647
final MockCommonFlightsManager mockFlightsManager = new MockCommonFlightsManager();
16461648
mockFlightsManager.setMockCommonFlightsProvider(mockFlightsProvider);
16471649
CommonFlightsManager.INSTANCE.initializeCommonFlightsManager(mockFlightsManager);
@@ -1764,4 +1766,72 @@ public void getIdTokensForAccountRecord_flightDisabled_returnsCorrectIdTokens()
17641766
assertEquals(1, idTokens.size());
17651767
assertEquals(defaultTestBundleV2.mGeneratedIdToken, idTokens.get(0));
17661768
}
1769+
1770+
/**
1771+
* Regression test for ClassCastException: both flights enabled but the cache was constructed
1772+
* with a plain {@link SharedPreferencesAccountCredentialCache} (not the memory-cache subclass).
1773+
* The instanceof guard must cause load() to fall back to the legacy path without throwing.
1774+
*/
1775+
@Test
1776+
public void load_flightEnabled_withNonMemoryCache_doesNotThrowAndReturnsCorrectResult()
1777+
throws ClientException {
1778+
enableFilterThenCloneFlight();
1779+
try {
1780+
// mOauth2TokenCache uses SharedPreferencesAccountCredentialCache (non-memory) from setUp()
1781+
configureMocksForTestBundle(defaultTestBundleV2);
1782+
final ICacheRecord saved = mOauth2TokenCache.save(
1783+
mockStrategy,
1784+
mockRequest,
1785+
mockResponse
1786+
);
1787+
1788+
final ICacheRecord loaded = mOauth2TokenCache.load(
1789+
CLIENT_ID,
1790+
APPLICATION_IDENTIFIER_SHA512,
1791+
MAM_ENROLLMENT_IDENTIFIER,
1792+
TARGET,
1793+
defaultTestBundleV2.mGeneratedAccount,
1794+
BEARER_AUTHENTICATION_SCHEME
1795+
);
1796+
1797+
assertNotNull(loaded);
1798+
assertEquals(saved.getAccount(), loaded.getAccount());
1799+
assertEquals(saved.getAccessToken(), loaded.getAccessToken());
1800+
assertEquals(saved.getRefreshToken(), loaded.getRefreshToken());
1801+
assertEquals(saved.getIdToken(), loaded.getIdToken());
1802+
} finally {
1803+
resetFlight();
1804+
}
1805+
}
1806+
1807+
/**
1808+
* Regression test for ClassCastException: both flights enabled but the cache was constructed
1809+
* with a plain {@link SharedPreferencesAccountCredentialCache} (not the memory-cache subclass).
1810+
* The instanceof guard must cause getIdTokensForAccountRecord() to fall back to the legacy path
1811+
* without throwing.
1812+
*/
1813+
@Test
1814+
public void getIdTokensForAccountRecord_flightEnabled_withNonMemoryCache_doesNotThrowAndReturnsCorrectResult()
1815+
throws ClientException {
1816+
enableFilterThenCloneFlight();
1817+
try {
1818+
// mOauth2TokenCache uses SharedPreferencesAccountCredentialCache (non-memory) from setUp()
1819+
configureMocksForTestBundle(defaultTestBundleV2);
1820+
mOauth2TokenCache.save(
1821+
mockStrategy,
1822+
mockRequest,
1823+
mockResponse
1824+
);
1825+
1826+
final List<IdTokenRecord> idTokens = mOauth2TokenCache.getIdTokensForAccountRecord(
1827+
CLIENT_ID,
1828+
defaultTestBundleV2.mGeneratedAccount
1829+
);
1830+
1831+
assertEquals(1, idTokens.size());
1832+
assertEquals(defaultTestBundleV2.mGeneratedIdToken, idTokens.get(0));
1833+
} finally {
1834+
resetFlight();
1835+
}
1836+
}
17671837
}

‎common4j/src/main/com/microsoft/identity/common/java/AuthenticationConstants.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -624,6 +624,11 @@ public static final class Broker {
624624
* Value for nativebroker mode sent in the extra query param by ESTS when the request is from lookup.
625625
*/
626626
public static final String LOOKUP_MODE_VALUE = "Lookup";
627+
628+
/**
629+
* When a Lookup mode request is forwarded to ESTS, the ID token in the response will have the value "none".
630+
*/
631+
public static final String LOOKUP_MODE_ID_TOKEN_VALUE = "none";
627632
}
628633

629634
@NoArgsConstructor(access = AccessLevel.PRIVATE)

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626

2727
import com.google.gson.JsonObject;
2828
import com.google.gson.JsonParser;
29+
import com.microsoft.identity.common.java.AuthenticationConstants;
2930
import com.microsoft.identity.common.java.authorities.Authority;
3031
import com.microsoft.identity.common.java.authscheme.AbstractAuthenticationScheme;
3132
import com.microsoft.identity.common.java.authscheme.PopAuthenticationSchemeInternal;
@@ -306,7 +307,7 @@ public AccountRecord createAccountRecord(
306307
} else {
307308
final String idTokenValue = microsoftStsTokenResponse.getIdToken();
308309
final MicrosoftStsAccount microsoftStsAccount = new MicrosoftStsAccount(
309-
parameters.isLookupMode() ?
310+
parameters.isLookupMode() && AuthenticationConstants.Broker.LOOKUP_MODE_ID_TOKEN_VALUE.equals(idTokenValue) ?
310311
IDToken.createForLookup(idTokenValue) : new IDToken(idTokenValue),
311312
clientInfo
312313
);

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

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -675,11 +675,16 @@ public ICacheRecord load(@NonNull final String clientId,
675675
final List<Credential> v1IdTokens;
676676
List<Credential> refreshTokens;
677677

678-
if (useFilterThenClone) {
678+
// The instanceof guard is necessary because the cache type is fixed at construction time,
679+
// but flights are evaluated at runtime. For example, BrokerClientIdRefreshTokenAccessor
680+
// always calls MsalOAuth2TokenCache.create(components) with useInMemoryCache=false,
681+
// producing a plain SharedPreferencesAccountCredentialCache regardless of flight state.
682+
// If both flights are later enabled at runtime, casting without this guard would throw
683+
// a ClassCastException.
684+
if (useFilterThenClone
685+
&& mAccountCredentialCache instanceof SharedPreferencesAccountCredentialCacheWithMemoryCache) {
679686
// Wrap all reads under one lock to ensure a consistent snapshot and prevent
680687
// a concurrent removal from causing partial/inconsistent cache records.
681-
// Safe to cast — USE_IN_MEMORY_CACHE_FOR_ACCOUNTS_AND_CREDENTIALS guarantees
682-
// mAccountCredentialCache is SharedPreferencesAccountCredentialCacheWithMemoryCache.
683688
final Object cacheLock = ((SharedPreferencesAccountCredentialCacheWithMemoryCache)
684689
mAccountCredentialCache).getCacheLock();
685690
synchronized (cacheLock) {
@@ -965,7 +970,10 @@ public List<IdTokenRecord> getIdTokensForAccountRecord(@Nullable String clientId
965970

966971
final List<Credential> idTokens;
967972

968-
if (useFilterThenClone) {
973+
// See the same instanceof guard in load() for a full explanation of why the cache type
974+
// may not match the flight state at runtime.
975+
if (useFilterThenClone
976+
&& mAccountCredentialCache instanceof SharedPreferencesAccountCredentialCacheWithMemoryCache) {
969977
// Wrap under one lock for snapshot consistency (same rationale as load()).
970978
final Object cacheLock = ((SharedPreferencesAccountCredentialCacheWithMemoryCache)
971979
mAccountCredentialCache).getCacheLock();

0 commit comments

Comments
 (0)