Somalaya/release/17.2.1

changelog.txt

@@ -1,5 +1,8 @@
-V.Next
+V.17.2.1
 ---------
+- [PATCH] Move SHOULD_USE_ACCOUNT_MANAGER_UNTIL_EPOCH_MILLISECONDS_KEY to BaseActiveBrokerCache (#2340)
+- [PATCH] Add cache for ContentProviderStrategy.isSupportedByTargetedBroker() (#2338)
+- [PATCH] isSupportedByTargetedBroker should only be invoked in bg thread (#2339)
 - [PATCH] Handle repeated camera requests. (#2332)
 - [MINOR] Clear active broker cache if the (cached) active broker app doesn't support ipc mechanism (#2331)
 - [MINOR] Add support for CIAM custom domain (#2314)

common/build.gradle

@@ -31,7 +31,7 @@ codeCoverageReport {
 
 // In dev, we want to keep the dependencies(common4j, broker4j, common) to 1.0.+ to be able to be consumed by daily dev pipeline.
 // In release/*, we change these to specific versions being consumed.
-def common4jVersion = "1.0.+"
+def common4jVersion = "14.2.1"
 if (project.hasProperty("distCommon4jVersion") && project.distCommon4jVersion != '') {
     common4jVersion = project.distCommon4jVersion
 }

common/src/main/java/com/microsoft/identity/common/internal/activebrokerdiscovery/BrokerDiscoveryClient.kt

@@ -34,6 +34,7 @@ import com.microsoft.identity.common.internal.broker.ipc.IIpcStrategy
 import com.microsoft.identity.common.internal.cache.IClientActiveBrokerCache
 import com.microsoft.identity.common.java.exception.ClientException
 import com.microsoft.identity.common.java.exception.ClientException.ONLY_SUPPORTS_ACCOUNT_MANAGER_ERROR_CODE
+import com.microsoft.identity.common.java.interfaces.IPlatformComponents
 import com.microsoft.identity.common.java.logging.Logger
 import kotlinx.coroutines.*
 import kotlinx.coroutines.sync.Mutex
@@ -162,12 +163,13 @@ class BrokerDiscoveryClient(private val brokerCandidates: Set<BrokerData>,
     }
 
     constructor(context: Context,
+                components: IPlatformComponents,
                 cache: IClientActiveBrokerCache): this(
         brokerCandidates = BrokerData.getKnownBrokerApps(),
         getActiveBrokerFromAccountManager = {
             AccountManagerBrokerDiscoveryUtil(context).getActiveBrokerFromAccountManager()
         },
-        ipcStrategy = ContentProviderStrategy(context),
+        ipcStrategy = ContentProviderStrategy(context, components),
         cache = cache,
         isPackageInstalled = { brokerData ->
             PackageHelper(context).

common/src/main/java/com/microsoft/identity/common/internal/activebrokerdiscovery/BrokerDiscoveryClientFactory.kt

@@ -88,6 +88,7 @@ class BrokerDiscoveryClientFactory {
                     lock.withLock {
                         if (clientSdkInstance == null) {
                             clientSdkInstance = getInstance(context,
+                                platformComponents,
                                 ClientActiveBrokerCache.getClientSdkCache(platformComponents.storageSupplier))
                         }
                     }
@@ -108,6 +109,7 @@ class BrokerDiscoveryClientFactory {
                     lock.withLock {
                         if (brokerSdkInstance == null) {
                             brokerSdkInstance = getInstance(context,
+                                platformComponents,
                                 ClientActiveBrokerCache.getBrokerSdkCache(platformComponents.storageSupplier))
                         }
                     }
@@ -122,11 +124,12 @@ class BrokerDiscoveryClientFactory {
          **/
         @JvmStatic
         private fun getInstance(context: Context,
+                                platformComponents: IPlatformComponents,
                                 cache: IClientActiveBrokerCache) : IBrokerDiscoveryClient{
             val methodTag = "$TAG:getInstance"
             return if (isNewBrokerDiscoveryEnabled()) {
                 Logger.info(methodTag, "Broker Discovery is enabled. Use the new logic on the SDK side")
-                BrokerDiscoveryClient(context, cache)
+                BrokerDiscoveryClient(context, platformComponents, cache)
             } else {
                 Logger.info(methodTag, "Broker Discovery is disabled. Use AccountManager on the SDK side.")
                 LegacyBrokerDiscoveryClient(context)

common/src/main/java/com/microsoft/identity/common/internal/broker/ipc/ContentProviderStatusLoader.kt

@@ -0,0 +1,104 @@
+// Copyright (c) Microsoft Corporation.
+// All rights reserved.
+//
+// This code is licensed under the MIT License.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files(the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and / or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions :
+//
+// The above copyright notice and this permission notice shall be included in
+// all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE.
+package com.microsoft.identity.common.internal.broker.ipc
+
+import android.content.Context
+import android.content.pm.PackageManager
+import android.content.pm.ProviderInfo
+import android.os.Build
+import android.os.DeadObjectException
+import com.microsoft.identity.common.java.interfaces.INameValueStorage
+import com.microsoft.identity.common.java.interfaces.IPlatformComponents
+import com.microsoft.identity.common.logging.Logger
+
+/**
+ * packageManager.queryContentProviders involves an ipc call (we've seen DeadObjectException from it before)
+ *
+ * We'll avoid making unnecessary ipc request by caching them.
+ * The content provider support status should remain the same for a given broker version.
+ **/
+class ContentProviderStatusLoader(
+    private val getVersionCode: (appPackageName: String) -> String,
+    private val supportedByContentProvider: (appPackageName: String) -> Boolean,
+    private val fileManager: INameValueStorage<String>,
+) : IContentProviderStatusLoader {
+
+    companion object {
+        private val TAG = ContentProviderStatusLoader::class.java.simpleName
+        private const val SHARED_PREFERENCE_NAME = "com.microsoft.common.ipc.content.provider.query.cache"
+
+        @Throws(PackageManager.NameNotFoundException::class)
+        private fun getVersionCode(context: Context, appPackageName: String) : String {
+            val packageInfo = context.packageManager.getPackageInfo(appPackageName, 0)
+            return if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) {
+                packageInfo.longVersionCode.toString()
+            } else {
+                packageInfo.versionCode.toString()
+            }
+        }
+
+        @Throws(DeadObjectException::class)
+        fun supportedByContentProvider(context: Context, appPackageName: String) : Boolean {
+            val contentProviderAuthority: String =
+                ContentProviderStrategy.getContentProviderAuthority(appPackageName)
+
+            val providers: List<ProviderInfo> = context.packageManager
+                .queryContentProviders(null, 0, 0)
+
+            for (providerInfo in providers) {
+                if (providerInfo.authority != null && providerInfo.authority == contentProviderAuthority) {
+                    return true
+                }
+            }
+
+            return false
+        }
+    }
+
+    constructor(context: Context, components: IPlatformComponents): this(
+        fileManager = components.storageSupplier.getUnencryptedNameValueStore(
+            SHARED_PREFERENCE_NAME, String::class.java),
+        getVersionCode = { pkgName -> getVersionCode(context, pkgName) },
+        supportedByContentProvider = { pkgName -> supportedByContentProvider(context, pkgName) }
+    )
+
+    override fun supportsContentProvider(packageName: String) : Boolean {
+        val methodTag = "$TAG:getResult"
+        try {
+            // Construct a key for the cache.
+            val key = packageName + ":" + getVersionCode(packageName)
+
+            // Try loading from cache, return if the value is found.
+            fileManager.get(key)?.let {
+                return it.toBoolean()
+            }
+
+            val queriedResult = supportedByContentProvider(packageName)
+            fileManager.put(key, queriedResult.toString())
+            return queriedResult
+        } catch (t: Throwable) {
+            Logger.error(methodTag, t.message, t)
+            return false
+        }
+    }
+}

common/src/main/java/com/microsoft/identity/common/internal/broker/ipc/ContentProviderStrategy.java

@@ -29,7 +29,6 @@
 import static com.microsoft.identity.common.internal.broker.ipc.IIpcStrategy.Type.CONTENT_PROVIDER;
 
 import android.content.Context;
-import android.content.pm.ProviderInfo;
 import android.database.Cursor;
 import android.net.Uri;
 import android.os.Bundle;
@@ -41,28 +40,31 @@
 
 import com.microsoft.identity.common.exception.BrokerCommunicationException;
 import com.microsoft.identity.common.internal.util.ParcelableUtil;
+import com.microsoft.identity.common.java.interfaces.IPlatformComponents;
 import com.microsoft.identity.common.logging.Logger;
 
-import java.util.List;
-
 /**
  * A strategy for communicating with the targeted broker via Content Provider.
  */
 public class ContentProviderStrategy extends AbstractIpcStrategyWithServiceValidation {
 
     private static final String TAG = ContentProviderStrategy.class.getSimpleName();
     private final Context mContext;
+    private final IContentProviderStatusLoader mCache;
 
-    public ContentProviderStrategy(final Context context) {
+    public ContentProviderStrategy(final Context context, final IPlatformComponents components) {
         super(false);
         mContext = context;
+        mCache = new ContentProviderStatusLoader(context, components);
     }
 
     @VisibleForTesting
     protected ContentProviderStrategy(final Context context,
+                                      final IContentProviderStatusLoader cache,
                                       final boolean shouldBypassSupportValidation) {
         super(shouldBypassSupportValidation);
         mContext = context;
+        mCache = cache;
     }
 
     @Override
@@ -142,7 +144,7 @@ private Uri getContentProviderURI(final @NonNull String targetedBrokerPackageNam
     /**
      * Returns content provider authority.
      */
-    private String getContentProviderAuthority(final @NonNull String targetedBrokerPackageName) {
+    public static String getContentProviderAuthority(final @NonNull String targetedBrokerPackageName) {
         return targetedBrokerPackageName + "." + AUTHORITY;
     }
 
@@ -151,22 +153,6 @@ private String getContentProviderAuthority(final @NonNull String targetedBrokerP
      */
     @Override
     public boolean isSupportedByTargetedBroker(final @NonNull String targetedBrokerPackageName) {
-        final String methodTag = TAG + ":isSupportedByTargetedBroker";
-        final String contentProviderAuthority = getContentProviderAuthority(targetedBrokerPackageName);
-
-        final List<ProviderInfo> providers = mContext.getPackageManager()
-                .queryContentProviders(null, 0, 0);
-
-        if (providers == null) {
-            Logger.error(methodTag, "Content Provider not found.", null);
-            return false;
-        }
-
-        for (final ProviderInfo providerInfo : providers) {
-            if (providerInfo.authority != null && providerInfo.authority.equals(contentProviderAuthority)) {
-                return true;
-            }
-        }
-        return false;
+        return mCache.supportsContentProvider(targetedBrokerPackageName);
     }
 }

common/src/main/java/com/microsoft/identity/common/internal/broker/ipc/IContentProviderStatusLoader.kt

@@ -0,0 +1,34 @@
+// Copyright (c) Microsoft Corporation.
+// All rights reserved.
+//
+// This code is licensed under the MIT License.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files(the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and / or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions :
+//
+// The above copyright notice and this permission notice shall be included in
+// all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE.
+package com.microsoft.identity.common.internal.broker.ipc
+
+/**
+ * A wrapper interface around packageManager.queryContentProviders().
+ * */
+interface IContentProviderStatusLoader {
+
+    /**
+     * Determine if the targeted app supports (Broker) Content Provider.
+     **/
+    fun supportsContentProvider(packageName: String) : Boolean
+}

common/src/main/java/com/microsoft/identity/common/internal/cache/BaseActiveBrokerCache.kt

@@ -47,6 +47,22 @@ open class BaseActiveBrokerCache
          * Key of the broker signature hash in the cache.
          **/
         const val ACTIVE_BROKER_CACHE_SIGHASH_KEY = "ACTIVE_BROKER_CACHE_SIGHASH_KEY"
+
+        /**
+         * Returns true if the time has NOT passed the given expiry date.
+         */
+        fun isNotExpired(expiryDate: Long?): Boolean{
+            if (expiryDate == null) {
+                return false
+            }
+            return System.currentTimeMillis() < expiryDate
+        }
+
+        /**
+         * Key for storing time which the client discovery should use AccountManager.
+         **/
+        const val SHOULD_USE_ACCOUNT_MANAGER_UNTIL_EPOCH_MILLISECONDS_KEY =
+            "SHOULD_USE_ACCOUNT_MANAGER_UNTIL_EPOCH_MILLISECONDS_KEY"
     }
 
     override fun getCachedActiveBroker(): BrokerData? {
@@ -63,6 +79,29 @@ open class BaseActiveBrokerCache
         }
     }
 
+    override fun shouldUseAccountManager(): Boolean {
+        return runBlocking {
+            lock.withLock {
+                storage.get(SHOULD_USE_ACCOUNT_MANAGER_UNTIL_EPOCH_MILLISECONDS_KEY)?.let { rawValue ->
+                    rawValue.toLongOrNull()?.let { expiryDate ->
+                        return@runBlocking isNotExpired(expiryDate)
+                    }
+                }
+
+                return@runBlocking false
+            }
+        }
+    }
+
+    override fun setShouldUseAccountManagerForTheNextMilliseconds(timeInMillis: Long) {
+        return runBlocking {
+            lock.withLock {
+                val timeStamp = System.currentTimeMillis() + timeInMillis
+                storage.put(SHOULD_USE_ACCOUNT_MANAGER_UNTIL_EPOCH_MILLISECONDS_KEY, timeStamp.toString())
+            }
+        }
+    }
+
     override fun setCachedActiveBroker(brokerData: BrokerData) {
         return runBlocking {
             lock.withLock {
@@ -74,18 +113,16 @@ open class BaseActiveBrokerCache
     override fun clearCachedActiveBroker() {
         return runBlocking {
             lock.withLock {
-                clearCachedActiveBrokerWithoutLock()
+                storage.remove(ACTIVE_BROKER_CACHE_PACKAGE_NAME_KEY)
+                storage.remove(ACTIVE_BROKER_CACHE_SIGHASH_KEY)
+                storage.remove(SHOULD_USE_ACCOUNT_MANAGER_UNTIL_EPOCH_MILLISECONDS_KEY)
             }
         }
     }
 
     protected open fun setCachedActiveBrokerWithoutLock(brokerData: BrokerData){
         storage.put(ACTIVE_BROKER_CACHE_PACKAGE_NAME_KEY, brokerData.packageName)
         storage.put(ACTIVE_BROKER_CACHE_SIGHASH_KEY, brokerData.signingCertificateThumbprint)
-    }
-
-    protected fun clearCachedActiveBrokerWithoutLock(){
-        storage.remove(ACTIVE_BROKER_CACHE_PACKAGE_NAME_KEY)
-        storage.remove(ACTIVE_BROKER_CACHE_SIGHASH_KEY)
+        storage.remove(SHOULD_USE_ACCOUNT_MANAGER_UNTIL_EPOCH_MILLISECONDS_KEY)
     }
 }