From 4b71ab07db0eae658107f812a60f74fbb79c00ac Mon Sep 17 00:00:00 2001 From: Adam Johnson Date: Tue, 11 Aug 2020 14:54:16 -0700 Subject: [PATCH] Use newly created nullsage equalsIgnoreCase. In reference to https://github.com/AzureAD/azure-activedirectory-library-for-android/pull/1550 * The only way I can understand the tokenCacheItem to be null is if something mutates the map unsafely. * Create a nullsafe equalsIgnoreCase utility in common and use it here. --- .../aad/adal/AcquireTokenRequest.java | 2 +- .../aad/adal/MemoryTokenCacheStore.java | 31 +++++-------------- .../aad/adal/TokenCacheAccessor.java | 3 +- 3 files changed, 11 insertions(+), 25 deletions(-) diff --git a/adal/src/main/java/com/microsoft/aad/adal/AcquireTokenRequest.java b/adal/src/main/java/com/microsoft/aad/adal/AcquireTokenRequest.java index 17d3d2f12..8149e37f8 100644 --- a/adal/src/main/java/com/microsoft/aad/adal/AcquireTokenRequest.java +++ b/adal/src/main/java/com/microsoft/aad/adal/AcquireTokenRequest.java @@ -727,7 +727,7 @@ private void verifyBrokerRedirectUri(final AuthenticationRequest request) throws throw new UsageAuthenticationException(ADALError.DEVELOPER_REDIRECTURI_INVALID, "The redirectUri is null or blank."); } - if (inputUri.equalsIgnoreCase(AuthenticationConstants.Broker.BROKER_REDIRECT_URI)) { + if (AuthenticationConstants.Broker.BROKER_REDIRECT_URI.equalsIgnoreCase(inputUri)) { // TODO: Clean this up once we migrate all Logger functions to the common one. com.microsoft.identity.common.internal.logging.Logger.info(TAG + methodName, "This is a broker redirectUri. Bypass the check."); return; diff --git a/adal/src/main/java/com/microsoft/aad/adal/MemoryTokenCacheStore.java b/adal/src/main/java/com/microsoft/aad/adal/MemoryTokenCacheStore.java index 1e89a57f6..7073ae0ce 100644 --- a/adal/src/main/java/com/microsoft/aad/adal/MemoryTokenCacheStore.java +++ b/adal/src/main/java/com/microsoft/aad/adal/MemoryTokenCacheStore.java @@ -26,9 +26,9 @@ import java.io.IOException; import java.io.ObjectInputStream; import java.io.ObjectOutputStream; -import java.util.HashMap; import java.util.Iterator; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; /** * tokenCacheItem is not persisted. Memory cache does not keep static items. @@ -42,9 +42,7 @@ public class MemoryTokenCacheStore implements ITokenCacheStore { private static final String TAG = "MemoryTokenCacheStore"; - private final Map mCache = new HashMap<>(); - - private transient Object mCacheLock = new Object(); + private final Map mCache = new ConcurrentHashMap<>(16, 0.75f, 1); /** * Creates MemoryTokenCacheStore. @@ -59,9 +57,7 @@ public TokenCacheItem getItem(String key) { } Logger.i(TAG, "Get Item from cache. ", "Key:" + key); - synchronized (mCacheLock) { - return mCache.get(key); - } + return mCache.get(key); } @Override @@ -75,9 +71,7 @@ public void setItem(String key, TokenCacheItem item) { } Logger.i(TAG, "Set Item to cache. ", "Key: " + key); - synchronized (mCacheLock) { - mCache.put(key, item); - } + mCache.put(key, item); } @Override @@ -87,17 +81,13 @@ public void removeItem(String key) { } Logger.i(TAG, "Remove Item from cache. ", "Key:" + key.hashCode()); - synchronized (mCacheLock) { - mCache.remove(key); - } + mCache.remove(key); } @Override public void removeAll() { Logger.v(TAG, "Remove all items from cache."); - synchronized (mCacheLock) { - mCache.clear(); - } + mCache.clear(); } private synchronized void writeObject(ObjectOutputStream out) throws IOException { @@ -108,7 +98,6 @@ private void readObject(ObjectInputStream inputStream) throws IOException, ClassNotFoundException { inputStream.defaultReadObject(); - mCacheLock = new Object(); } @Override @@ -118,16 +107,12 @@ public boolean contains(String key) { } Logger.i(TAG, "contains Item from cache.", "Key: " + key); - synchronized (mCacheLock) { - return mCache.get(key) != null; - } + return mCache.get(key) != null; } @Override public Iterator getAll() { Logger.v(TAG, "Retrieving all items from cache. "); - synchronized (mCacheLock) { - return mCache.values().iterator(); - } + return mCache.values().iterator(); } } diff --git a/adal/src/main/java/com/microsoft/aad/adal/TokenCacheAccessor.java b/adal/src/main/java/com/microsoft/aad/adal/TokenCacheAccessor.java index 3e5f7b402..0e3d6740e 100644 --- a/adal/src/main/java/com/microsoft/aad/adal/TokenCacheAccessor.java +++ b/adal/src/main/java/com/microsoft/aad/adal/TokenCacheAccessor.java @@ -45,6 +45,7 @@ import com.microsoft.identity.common.internal.providers.microsoft.azureactivedirectory.AzureActiveDirectoryOAuth2Configuration; import com.microsoft.identity.common.internal.providers.microsoft.azureactivedirectory.AzureActiveDirectoryOAuth2Strategy; import com.microsoft.identity.common.internal.providers.microsoft.azureactivedirectory.AzureActiveDirectoryTokenResponse; +import com.microsoft.identity.common.internal.util.StringUtil; import java.net.MalformedURLException; import java.net.URL; @@ -443,7 +444,7 @@ boolean isMultipleMRRTsMatchingGivenApp(final String clientId) { final List mrrtsMatchingRequest = new ArrayList<>(); while (allItems.hasNext()) { final TokenCacheItem tokenCacheItem = allItems.next(); - if (tokenCacheItem.getAuthority().equalsIgnoreCase(mAuthority) && tokenCacheItem.getClientId().equalsIgnoreCase(clientId) + if (StringUtil.equalsIgnoreCase(tokenCacheItem.getAuthority(), mAuthority) && StringUtil.equalsIgnoreCase(tokenCacheItem.getClientId(), clientId) && (tokenCacheItem.getIsMultiResourceRefreshToken() || StringExtensions.isNullOrBlank(tokenCacheItem.getResource()))) { mrrtsMatchingRequest.add(tokenCacheItem); }