Refactor get-account-by-local-account-id FixesAB#3397557, Fixes AB#3397557

changelog.txt

@@ -1,9 +1,13 @@
 vNext
 ----------
 
+- [MINOR] Refactor getAccountByLocalAccountId (#2781)
+- [MINOR] Add OTel Benchmarker (#2786)
+- [MINOR] WebApps AccountId Registry (#2787)
+- [MINOR] Take flight value for whether to show webcp flow in weview or not in brokerless scenarios. (#2784)
+
 Version 23.0.2
 ----------
-- [MINOR] Add OTel Benchmarker (#2786)
 - [MAJOR] Add KeyStoreBackedSecretKeyProvider (#2674)
 - [MINOR] Add Open Id configuration issuer validation reporting in OpenIdProviderConfigurationClient (#2751)
 - [MINOR] Add helper method to record elapsed time (#2768)
@@ -16,8 +20,6 @@ Version 23.0.2
 - [MINOR] Added error handling when webcp redirects have browser protocol #2767
 - [PATCH] Fix for app link redirect from CCT due to forced browser preference (#2775)
 - [MINOR] getAllSsoTokens method for Edge (#2774)
-- [MINOR] WebApps AccountId Registry (#2787)
-- [MINOR] Take flight value for whether to show webcp flow in weview or not in brokerless scenarios. (#2784)
 
 Version 22.1.3
 ----------

common4j/src/main/com/microsoft/identity/common/java/cache/AbstractAccountCredentialCache.java

@@ -113,23 +113,11 @@ protected List<AccountRecord> getAccountsFilteredByInternal(@Nullable final Stri
         final List<AccountRecord> matchingAccounts = new ArrayList<>();
 
         for (final AccountRecord account : allAccounts) {
-            boolean matches = true;
+            if (mustMatchOnHomeAccountId && !StringUtil.equalsIgnoreCaseTrimBoth(homeAccountId, account.getHomeAccountId())) continue;
+            if (mustMatchOnEnvironment && !StringUtil.equalsIgnoreCaseTrimBoth(environment, account.getEnvironment())) continue;
+            if (mustMatchOnRealm && !StringUtil.equalsIgnoreCaseTrimBoth(realm, account.getRealm())) continue;
 
-            if (mustMatchOnHomeAccountId) {
-                matches = StringUtil.equalsIgnoreCaseTrimBoth(homeAccountId, account.getHomeAccountId());
-            }
-
-            if (mustMatchOnEnvironment) {
-                matches = matches && StringUtil.equalsIgnoreCaseTrimBoth(environment, account.getEnvironment());
-            }
-
-            if (mustMatchOnRealm) {
-                matches = matches && StringUtil.equalsIgnoreCaseTrimBoth(realm, account.getRealm());
-            }
-
-            if (matches) {
-                matchingAccounts.add(account);
-            }
+            matchingAccounts.add(account);
         }
 
         Logger.verbose(
@@ -191,28 +179,18 @@ protected List<Credential> getCredentialsFilteredByInternal(@NonNull final List<
         final List<Credential> matchingCredentials = new ArrayList<>();
 
         for (final Credential credential : allCredentials) {
-            boolean matches = true;
-
-            if (mustMatchOnHomeAccountId) {
-                matches = StringUtil.equalsIgnoreCaseTrimBoth(homeAccountId, credential.getHomeAccountId());
-            }
+            if (mustMatchOnHomeAccountId && !StringUtil.equalsIgnoreCaseTrimBoth(homeAccountId, credential.getHomeAccountId())) continue;
 
-            if (mustMatchOnEnvironment) {
-                matches = matches && StringUtil.equalsIgnoreCaseTrimBoth(environment, credential.getEnvironment());
-            }
+            if (mustMatchOnEnvironment && !StringUtil.equalsIgnoreCaseTrimBoth(environment, credential.getEnvironment())) continue;
 
-            if (mustMatchOnCredentialType) {
-                matches = matches && StringUtil.equalsIgnoreCaseTrimBoth(credentialType.name(), credential.getCredentialType());
-            }
+            if (mustMatchOnCredentialType && !StringUtil.equalsIgnoreCaseTrimBoth(credentialType.name(), credential.getCredentialType())) continue;
 
-            if (mustMatchOnClientId) {
-                matches = matches && StringUtil.equalsIgnoreCaseTrimBoth(clientId, credential.getClientId());
-            }
+            if (mustMatchOnClientId && !StringUtil.equalsIgnoreCaseTrimBoth(clientId, credential.getClientId())) continue;
 
             if (mustMatchOnApplicationIdentifier) {
                 if (credential instanceof AccessTokenRecord) {
                     final AccessTokenRecord accessToken = (AccessTokenRecord) credential;
-                    matches = matches && StringUtil.equalsIgnoreCaseTrimBoth(applicationIdentifier, accessToken.getApplicationIdentifier());
+                    if(!StringUtil.equalsIgnoreCaseTrimBoth(applicationIdentifier, accessToken.getApplicationIdentifier())) continue;
                 } else {
                     Logger.verbose(TAG, "Query specified applicationIdentifier match, but credential type does not have application identifier");
                 }
@@ -221,29 +199,32 @@ protected List<Credential> getCredentialsFilteredByInternal(@NonNull final List<
             if (mustMatchOnMamEnrollmentIdentifier) {
                 if (credential instanceof AccessTokenRecord) {
                     final AccessTokenRecord accessToken = (AccessTokenRecord) credential;
-                    matches = matches && StringUtil.equalsIgnoreCaseTrimBoth(mamEnrollmentIdentifier, accessToken.getMamEnrollmentIdentifier());
+                    if(!StringUtil.equalsIgnoreCaseTrimBoth(mamEnrollmentIdentifier, accessToken.getMamEnrollmentIdentifier())) continue;
                 } else {
                     Logger.verbose(TAG, "Query specified mamEnrollmentIdentifier match, but credential type does not have MAM enrollment identifier");
                 }
             }
 
-            if (mustMatchOnRealm && credential instanceof AccessTokenRecord) {
-                final AccessTokenRecord accessToken = (AccessTokenRecord) credential;
-                matches = matches && StringUtil.equalsIgnoreCaseTrimBoth(realm, accessToken.getRealm());
-            }
-
-            if (mustMatchOnRealm && credential instanceof IdTokenRecord) {
-                final IdTokenRecord idToken = (IdTokenRecord) credential;
-                matches = matches && StringUtil.equalsIgnoreCaseTrimBoth(realm, idToken.getRealm());
+            if (mustMatchOnRealm) {
+                String tokenRecordRealm = "";
+                if (credential instanceof AccessTokenRecord) {
+                    final AccessTokenRecord accessToken = (AccessTokenRecord) credential;
+                    tokenRecordRealm = accessToken.getRealm();
+                    if(!StringUtil.equalsIgnoreCaseTrimBoth(realm, tokenRecordRealm)) continue;
+                } else if (credential instanceof IdTokenRecord) {
+                    final IdTokenRecord idToken = (IdTokenRecord) credential;
+                    tokenRecordRealm = idToken.getRealm();
+                    if(!StringUtil.equalsIgnoreCaseTrimBoth(realm, tokenRecordRealm)) continue;
+                }
             }
 
             if (mustMatchOnTarget) {
                 if (credential instanceof AccessTokenRecord) {
                     final AccessTokenRecord accessToken = (AccessTokenRecord) credential;
-                    matches = matches && targetsIntersect(target, accessToken.getTarget(), true);
+                    if(!targetsIntersect(target, accessToken.getTarget(), true)) continue;
                 } else if (credential instanceof RefreshTokenRecord) {
                     final RefreshTokenRecord refreshToken = (RefreshTokenRecord) credential;
-                    matches = matches && targetsIntersect(target, refreshToken.getTarget(), true);
+                    if(!targetsIntersect(target, refreshToken.getTarget(), true)) continue;
                 } else {
                     Logger.verbose(TAG, "Query specified target-match, but no target to match.");
                 }
@@ -252,40 +233,35 @@ protected List<Credential> getCredentialsFilteredByInternal(@NonNull final List<
             if (mustMatchOnAuthScheme && credential instanceof AccessTokenRecord) {
                 final AccessTokenRecord accessToken = (AccessTokenRecord) credential;
                 String atType = accessToken.getAccessTokenType();
-
                 if (null != atType) {
                     atType = atType.trim();
                 }
 
                 if (TokenRequest.TokenType.POP.equalsIgnoreCase(atType)) {
-                    matches = matches && (
-                            authScheme.equalsIgnoreCase(PopAuthenticationSchemeWithClientKeyInternal.SCHEME_POP_WITH_CLIENT_KEY)
-                                    || authScheme.equalsIgnoreCase(PopAuthenticationSchemeInternal.SCHEME_POP)
-                    );
-                } else {
-                    matches = matches && authScheme.equalsIgnoreCase(atType);
-                }
+                    if (!(authScheme.equalsIgnoreCase(PopAuthenticationSchemeWithClientKeyInternal.SCHEME_POP_WITH_CLIENT_KEY)
+                            || authScheme.equalsIgnoreCase(PopAuthenticationSchemeInternal.SCHEME_POP))) {
+                        continue;
+                    }
+                } else if (!authScheme.equalsIgnoreCase(atType)) continue;
             }
 
             if(mustMatchOnKid && credential instanceof AccessTokenRecord) {
                 final AccessTokenRecord accessToken = (AccessTokenRecord) credential;
-                matches = matches && kid.equalsIgnoreCase(accessToken.getKid());
+                if (!kid.equalsIgnoreCase(accessToken.getKid())) continue;
             }
 
             if (mustMatchOnRequestedClaims || mustMatchExactClaims) {
                 if (credential instanceof AccessTokenRecord) {
                     final AccessTokenRecord accessToken = (AccessTokenRecord) credential;
                     if(!(mustMatchExactClaims && StringUtil.isNullOrEmpty(requestedClaims) && StringUtil.isNullOrEmpty(accessToken.getRequestedClaims()))) {
-                        matches = matches && StringUtil.equalsIgnoreCaseTrimBoth(requestedClaims, accessToken.getRequestedClaims());
+                        if(!StringUtil.equalsIgnoreCaseTrimBoth(requestedClaims, accessToken.getRequestedClaims())) continue;
                     }
                 } else {
                     Logger.verbose(TAG, "Query specified requested_claims-match, but attempted to match with non-AT credential type.");
                 }
             }
 
-            if (matches) {
-                matchingCredentials.add(credential);
-            }
+            matchingCredentials.add(credential);
         }
 
         return matchingCredentials;

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

@@ -66,7 +66,6 @@
 import java.util.List;
 import java.util.Locale;
 import java.util.Set;
-import java.util.stream.Collectors;
 
 import edu.umd.cs.findbugs.annotations.Nullable;
 import lombok.NonNull;
@@ -1092,17 +1091,42 @@ public AccountRecord getAccountByLocalAccountId(@Nullable final String environme
                                                     @NonNull final String clientId,
                                                     @NonNull final String localAccountId) {
         final String methodName = ":getAccountByLocalAccountId";
-
-        final List<AccountRecord> accounts = getAccounts(environment, clientId);
-
         Logger.verbosePII(
                 TAG + methodName,
                 "LocalAccountId: [" + localAccountId + "]"
         );
 
-        for (final AccountRecord account : accounts) {
-            if (localAccountId.equals(account.getLocalAccountId())) {
-                return account;
+        final List<AccountRecord> accountRecordList = mAccountCredentialCache.getAccounts();
+        if (accountRecordList.isEmpty()) {
+            Logger.warn(
+                    TAG + methodName,
+                    "No accounts found in the cache."
+            );
+            return null;
+        }
+
+        final Set<CredentialType> credentialTypes = new HashSet<>(
+                Arrays.asList(IdToken, V1IdToken, RefreshToken)
+        );
+
+        final List<Credential> appCredentials = mAccountCredentialCache.getCredentialsFilteredBy(
+                null, // homeAccountId
+                environment,
+                credentialTypes,
+                clientId,
+                null,
+                null,
+                null,
+                null,
+                null,
+                null
+        );
+
+        for (final AccountRecord accountRecord: accountRecordList) {
+            if (accountRecord.getLocalAccountId().equals(localAccountId)
+                    && accountRecord.getEnvironment().equals(environment)
+                        && accountHasCredential(accountRecord, appCredentials)) {
+                return accountRecord;
             }
         }
 
@@ -1229,6 +1253,10 @@ public List<AccountRecord> getAccounts(@Nullable final String environment,
                 "Found " + accountsForEnvironment.size() + " accounts for this environment"
         );
 
+        if (accountsForEnvironment.isEmpty()) {
+            return Collections.unmodifiableList(accountsForThisApp);
+        }
+
         final Set<CredentialType> credentialTypes = new HashSet<>(
                 Arrays.asList(IdToken, V1IdToken, RefreshToken)
         );
@@ -1345,8 +1373,9 @@ private CredentialType getAccessTokenCredentialTypeForAuthenticationScheme(
     }
 
     /**
-     * Evaluates the supplied list of Credentials. Returns true if the provided Account
-     * 'owns' any one of these tokens.
+     * Evaluates the supplied list of Credentials. Returns true if the provided Account's
+     * homeAccountId matches with any of the credentials' homeAccountId.
+     * This does not filter on environment, as that is expected to be pre-filtered.
      *
      * @param account        The Account whose credential ownership should be evaluated.
      * @param appCredentials The Credentials to evaluate.
@@ -1355,20 +1384,17 @@ private CredentialType getAccessTokenCredentialTypeForAuthenticationScheme(
     private boolean accountHasCredential(@NonNull final AccountRecord account,
                                          @NonNull final List<Credential> appCredentials) {
         final String methodName = ":accountHasCredential";
-
         final String accountHomeId = account.getHomeAccountId();
-        final String accountEnvironment = account.getEnvironment();
 
         Logger.verbosePII(
                 TAG + methodName,
                 "HomeAccountId: [" + accountHomeId + "]"
-                        + "\n"
-                        + "Environment: [" + accountEnvironment + "]"
         );
 
+        // Since we already filtered accounts and credentials by environment, there is no need to check
+        // environment again
         for (final Credential credential : appCredentials) {
-            if (accountHomeId.equals(credential.getHomeAccountId())
-                    && accountEnvironment.equals(credential.getEnvironment())) {
+            if (accountHomeId.equals(credential.getHomeAccountId())) {
                 Logger.verbose(
                         TAG + methodName,
                         "Credentials located for account."