Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
siddhijain merged 15 commits into from
Oct 27, 2025
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
88f6918
Refactor get-account-by-local-account-id
siddhijain Oct 9, 2025
7309e5c
remove method created to test
siddhijain Oct 9, 2025
6728e2f
Fixing spacing issue
siddhijain Oct 9, 2025
c28e18b
Update common4j/src/main/com/microsoft/identity/common/java/cache/Msa…
siddhijain Oct 9, 2025
6e909fa
Merge branch 'dev' into sjain/refactor-get-account-by-local-account-id
siddhijain Oct 9, 2025
45d5e58
remove a variable to simplify condition
siddhijain Oct 9, 2025
557a281
Merge branch 'dev' into sjain/refactor-get-account-by-local-account-id
siddhijain Oct 10, 2025
0a103f6
Removed variable to simplify condition
siddhijain Oct 21, 2025
d04ff77
Merge branch 'dev' into sjain/refactor-get-account-by-local-account-id
siddhijain Oct 23, 2025
cd8ac32
incorporating PR comments
siddhijain Oct 24, 2025
94a5961
Merge branch 'dev' into sjain/refactor-get-account-by-local-account-id
siddhijain Oct 24, 2025
74847af
updated changelog.txt
siddhijain Oct 24, 2025
87d16ff
Merge branch 'dev' into sjain/refactor-get-account-by-local-account-id
siddhijain Oct 26, 2025
338f399
Fix failing tests
siddhijain Oct 27, 2025
847e9d4
updated documentation
siddhijain Oct 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 30 additions & 15 deletions ...n4j/src/main/com/microsoft/identity/common/java/cache/AbstractAccountCredentialCache.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,21 +115,24 @@ protected List<AccountRecord> getAccountsFilteredByInternal(@Nullable final Stri
for (final AccountRecord account : allAccounts) {
boolean matches = true;

// once matches is false, it will never be true for this account
if (mustMatchOnHomeAccountId) {
matches = StringUtil.equalsIgnoreCaseTrimBoth(homeAccountId, account.getHomeAccountId());
if(!matches) continue;
}

if (mustMatchOnEnvironment) {
matches = matches && StringUtil.equalsIgnoreCaseTrimBoth(environment, account.getEnvironment());
if(!matches) continue;
}

if (mustMatchOnRealm) {
matches = matches && StringUtil.equalsIgnoreCaseTrimBoth(realm, account.getRealm());
if(!matches) continue;
Copy link

Copilot AI Oct 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consistent spacing issue: missing space after 'if' keyword throughout the file. Should be 'if (!matches) continue;' for all instances.

Suggested change
if(!matches) continue;
}
if (mustMatchOnEnvironment) {
matches = matches && StringUtil.equalsIgnoreCaseTrimBoth(environment, account.getEnvironment());
if(!matches) continue;
}
if (mustMatchOnRealm) {
matches = matches && StringUtil.equalsIgnoreCaseTrimBoth(realm, account.getRealm());
if(!matches) continue;
if (!matches) continue;
}
if (mustMatchOnEnvironment) {
matches = matches && StringUtil.equalsIgnoreCaseTrimBoth(environment, account.getEnvironment());
if (!matches) continue;
}
if (mustMatchOnRealm) {
matches = matches && StringUtil.equalsIgnoreCaseTrimBoth(realm, account.getRealm());
if (!matches) continue;

Copilot uses AI. Check for mistakes.
}

if (matches) {
matchingAccounts.add(account);
}
// if it reaches here, it means matches is true
matchingAccounts.add(account);
}

Logger.verbose(
Expand Down Expand Up @@ -195,58 +198,67 @@ protected List<Credential> getCredentialsFilteredByInternal(@NonNull final List<

if (mustMatchOnHomeAccountId) {
matches = StringUtil.equalsIgnoreCaseTrimBoth(homeAccountId, credential.getHomeAccountId());
if(!matches) continue;
Copy link

Copilot AI Oct 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consistent spacing issue: missing space after 'if' keyword throughout the file. Should be 'if (!matches) continue;' for all instances.

Copilot uses AI. Check for mistakes.
}

if (mustMatchOnEnvironment) {
matches = matches && StringUtil.equalsIgnoreCaseTrimBoth(environment, credential.getEnvironment());
matches = StringUtil.equalsIgnoreCaseTrimBoth(environment, credential.getEnvironment());
if(!matches) continue;
Copy link

Copilot AI Oct 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consistent spacing issue: missing space after 'if' keyword throughout the file. Should be 'if (!matches) continue;' for all instances.

Copilot uses AI. Check for mistakes.
}

if (mustMatchOnCredentialType) {
matches = matches && StringUtil.equalsIgnoreCaseTrimBoth(credentialType.name(), credential.getCredentialType());
matches = StringUtil.equalsIgnoreCaseTrimBoth(credentialType.name(), credential.getCredentialType());
if(!matches) continue;
Copy link

Copilot AI Oct 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consistent spacing issue: missing space after 'if' keyword throughout the file. Should be 'if (!matches) continue;' for all instances.

Copilot uses AI. Check for mistakes.
}

if (mustMatchOnClientId) {
matches = matches && StringUtil.equalsIgnoreCaseTrimBoth(clientId, credential.getClientId());
matches = StringUtil.equalsIgnoreCaseTrimBoth(clientId, credential.getClientId());
if(!matches) continue;
Copy link

Copilot AI Oct 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consistent spacing issue: missing space after 'if' keyword throughout the file. Should be 'if (!matches) continue;' for all instances.

Copilot uses AI. Check for mistakes.
}

if (mustMatchOnApplicationIdentifier) {
if (credential instanceof AccessTokenRecord) {
final AccessTokenRecord accessToken = (AccessTokenRecord) credential;
matches = matches && StringUtil.equalsIgnoreCaseTrimBoth(applicationIdentifier, accessToken.getApplicationIdentifier());
matches = StringUtil.equalsIgnoreCaseTrimBoth(applicationIdentifier, accessToken.getApplicationIdentifier());
} else {
Logger.verbose(TAG, "Query specified applicationIdentifier match, but credential type does not have application identifier");
}
if(!matches) continue;
Copy link

Copilot AI Oct 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consistent spacing issue: missing space after 'if' keyword throughout the file. Should be 'if (!matches) continue;' for all instances.

Copilot uses AI. Check for mistakes.
}

if (mustMatchOnMamEnrollmentIdentifier) {
if (credential instanceof AccessTokenRecord) {
final AccessTokenRecord accessToken = (AccessTokenRecord) credential;
matches = matches && StringUtil.equalsIgnoreCaseTrimBoth(mamEnrollmentIdentifier, accessToken.getMamEnrollmentIdentifier());
matches = StringUtil.equalsIgnoreCaseTrimBoth(mamEnrollmentIdentifier, accessToken.getMamEnrollmentIdentifier());
} else {
Logger.verbose(TAG, "Query specified mamEnrollmentIdentifier match, but credential type does not have MAM enrollment identifier");
}
if(!matches) continue;
Copy link

Copilot AI Oct 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consistent spacing issue: missing space after 'if' keyword throughout the file. Should be 'if (!matches) continue;' for all instances.

Copilot uses AI. Check for mistakes.
}

if (mustMatchOnRealm && credential instanceof AccessTokenRecord) {
final AccessTokenRecord accessToken = (AccessTokenRecord) credential;
matches = matches && StringUtil.equalsIgnoreCaseTrimBoth(realm, accessToken.getRealm());
matches = StringUtil.equalsIgnoreCaseTrimBoth(realm, accessToken.getRealm());
if(!matches) continue;
Copy link

Copilot AI Oct 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consistent spacing issue: missing space after 'if' keyword throughout the file. Should be 'if (!matches) continue;' for all instances.

Copilot uses AI. Check for mistakes.
}

if (mustMatchOnRealm && credential instanceof IdTokenRecord) {
final IdTokenRecord idToken = (IdTokenRecord) credential;
matches = matches && StringUtil.equalsIgnoreCaseTrimBoth(realm, idToken.getRealm());
matches = StringUtil.equalsIgnoreCaseTrimBoth(realm, idToken.getRealm());
if(!matches) continue;
}

if (mustMatchOnTarget) {
if (credential instanceof AccessTokenRecord) {
final AccessTokenRecord accessToken = (AccessTokenRecord) credential;
matches = matches && targetsIntersect(target, accessToken.getTarget(), true);
matches = targetsIntersect(target, accessToken.getTarget(), true);
} else if (credential instanceof RefreshTokenRecord) {
final RefreshTokenRecord refreshToken = (RefreshTokenRecord) credential;
matches = matches && targetsIntersect(target, refreshToken.getTarget(), true);
matches = targetsIntersect(target, refreshToken.getTarget(), true);
} else {
Logger.verbose(TAG, "Query specified target-match, but no target to match.");
}
if(!matches) continue;
}

if (mustMatchOnAuthScheme && credential instanceof AccessTokenRecord) {
Expand All @@ -263,24 +275,27 @@ protected List<Credential> getCredentialsFilteredByInternal(@NonNull final List<
|| authScheme.equalsIgnoreCase(PopAuthenticationSchemeInternal.SCHEME_POP)
);
} else {
matches = matches && authScheme.equalsIgnoreCase(atType);
matches = authScheme.equalsIgnoreCase(atType);
}
if(!matches) continue;
}

if(mustMatchOnKid && credential instanceof AccessTokenRecord) {
final AccessTokenRecord accessToken = (AccessTokenRecord) credential;
matches = matches && kid.equalsIgnoreCase(accessToken.getKid());
matches = kid.equalsIgnoreCase(accessToken.getKid());
if(!matches) 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());
matches = StringUtil.equalsIgnoreCaseTrimBoth(requestedClaims, accessToken.getRequestedClaims());
}
} else {
Logger.verbose(TAG, "Query specified requested_claims-match, but attempted to match with non-AT credential type.");
}
if(!matches) continue;
}

if (matches) {
Expand Down
103 changes: 91 additions & 12 deletions common4j/src/main/com/microsoft/identity/common/java/cache/MsalOAuth2TokenCache.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import com.microsoft.identity.common.java.dto.IdTokenRecord;
import com.microsoft.identity.common.java.dto.RefreshTokenRecord;
import com.microsoft.identity.common.java.exception.ClientException;
import com.microsoft.identity.common.java.flighting.CommonFlight;
import com.microsoft.identity.common.java.interfaces.INameValueStorage;
import com.microsoft.identity.common.java.interfaces.IPlatformComponents;
import com.microsoft.identity.common.java.logging.Logger;
Expand All @@ -58,6 +59,7 @@
import com.microsoft.identity.common.java.telemetry.events.CacheEndEvent;
import com.microsoft.identity.common.java.telemetry.events.CacheStartEvent;
import com.microsoft.identity.common.java.util.StringUtil;
import com.microsoft.identity.common.java.flighting.CommonFlightsManager;

import java.util.ArrayList;
import java.util.Arrays;
Expand All @@ -66,7 +68,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;
Expand Down Expand Up @@ -1092,16 +1093,45 @@ 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())) {
List<AccountRecord> accountsFilteredByEnvAndLocalAccountId = new ArrayList<>();
List<AccountRecord> accountRecordList = mAccountCredentialCache.getAccounts();
for (AccountRecord accountRecord: accountRecordList) {
if (accountRecord.getLocalAccountId().equals(localAccountId)
&& accountRecord.getEnvironment().equals(environment)) {
accountsFilteredByEnvAndLocalAccountId.add(accountRecord);
}
}

Logger.verbose(
TAG + methodName,
"Found " + accountsFilteredByEnvAndLocalAccountId.size() + " accounts for this environment" +
"and localAccountId"
);

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 account : accountsFilteredByEnvAndLocalAccountId) {
if (accountHasCredential(account, appCredentials)) {
return account;
}
}
Expand Down Expand Up @@ -1202,6 +1232,58 @@ private void setToCacheRecord(@NonNull final CacheRecord.CacheRecordBuilder targ
}
}

private AccountRecord getAccountByLocalAccountId1(@Nullable final String environment,
@NonNull final String clientId,
final String localAccountId) {
final String methodName = ":getAccounts";

Logger.verbosePII(
TAG + methodName,
"Environment: [" + environment + "]"
+ "\n"
+ "ClientId: [" + clientId + "]"
);

List<AccountRecord> accountsFilteredByEnvAndLocalAccountId = new ArrayList<>();
List<AccountRecord> accountRecordList = mAccountCredentialCache.getAccounts();
for (AccountRecord accountRecord: accountRecordList) {
if (accountRecord.getLocalAccountId().equals(localAccountId)
&& accountRecord.getEnvironment().equals(environment)) {
accountsFilteredByEnvAndLocalAccountId.add(accountRecord);
}
}

Logger.verbose(
TAG + methodName,
"Found " + accountsFilteredByEnvAndLocalAccountId.size() + " accounts for this environment"
);

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 account : accountsFilteredByEnvAndLocalAccountId) {
if (accountHasCredential(account, appCredentials)) {
return account;
}
}

return null;
}

@Override
public List<AccountRecord> getAccounts(@Nullable final String environment,
@NonNull final String clientId) {
Expand Down Expand Up @@ -1355,20 +1437,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."
Expand Down
Loading