From 5fbbe154b8075ab913047e9206e715a413869416 Mon Sep 17 00:00:00 2001 From: Sowmya Malayanur Date: Mon, 4 Mar 2024 23:13:55 -0800 Subject: [PATCH 1/9] changelog, version.properties, build.gradle updated --- changelog.txt | 2 +- common/build.gradle | 2 +- common4j/versioning/version.properties | 2 +- versioning/version.properties | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/changelog.txt b/changelog.txt index 38608910ec..c02722ebdc 100644 --- a/changelog.txt +++ b/changelog.txt @@ -1,4 +1,4 @@ -V.Next +V.17.2.0 --------- - [PATCH] Handle repeated camera requests. (#2332) - [MINOR] Clear active broker cache if the (cached) active broker app doesn't support ipc mechanism (#2331) diff --git a/common/build.gradle b/common/build.gradle index 9725649797..e7f7b28d2f 100644 --- a/common/build.gradle +++ b/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.0-RC1" if (project.hasProperty("distCommon4jVersion") && project.distCommon4jVersion != '') { common4jVersion = project.distCommon4jVersion } diff --git a/common4j/versioning/version.properties b/common4j/versioning/version.properties index eca23c6bb7..7dd105a9c7 100644 --- a/common4j/versioning/version.properties +++ b/common4j/versioning/version.properties @@ -1,4 +1,4 @@ #Wed May 12 20:08:39 UTC 2021 -versionName=14.1.0 +versionName=14.2.0-RC1 versionCode=1 latestPatchVersion=227 diff --git a/versioning/version.properties b/versioning/version.properties index 87f3b29fc5..9ec073e196 100644 --- a/versioning/version.properties +++ b/versioning/version.properties @@ -1,4 +1,4 @@ #Tue Apr 06 22:55:08 UTC 2021 -versionName=17.1.1 +versionName=17.2.0-RC1 versionCode=1 latestPatchVersion=234 From b23c9a5db0fd18fa692e26ea0684eea16a98d495 Mon Sep 17 00:00:00 2001 From: Sowmya Malayanur Date: Wed, 6 Mar 2024 10:48:25 -0800 Subject: [PATCH 2/9] automation fix --- .../ui/automation/device/settings/BaseSettings.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/uiautomationutilities/src/main/java/com/microsoft/identity/client/ui/automation/device/settings/BaseSettings.java b/uiautomationutilities/src/main/java/com/microsoft/identity/client/ui/automation/device/settings/BaseSettings.java index 28ec48068b..11bce8270f 100644 --- a/uiautomationutilities/src/main/java/com/microsoft/identity/client/ui/automation/device/settings/BaseSettings.java +++ b/uiautomationutilities/src/main/java/com/microsoft/identity/client/ui/automation/device/settings/BaseSettings.java @@ -58,12 +58,12 @@ public void launchDeviceAdminSettingsPage() { context.startActivity(intent); final UiObject deviceAdminPage; - if (android.os.Build.VERSION.SDK_INT == 32) { +// if (android.os.Build.VERSION.SDK_INT == 32) { // This is needed for higher API levels. So far have confirmed this for API 32, but API 30 will use the one above deviceAdminPage = UiAutomatorUtils.obtainUiObjectWithDescription("Device admin apps"); - } else { - deviceAdminPage = UiAutomatorUtils.obtainUiObjectWithText("device admin"); - } +// } else { +// deviceAdminPage = UiAutomatorUtils.obtainUiObjectWithText("device admin"); +// } Assert.assertTrue("Device Admin Settings Page appears", deviceAdminPage.exists()); } From c268be7a84ed58e39a10f63074bd6888434942cb Mon Sep 17 00:00:00 2001 From: melissaahn Date: Wed, 6 Mar 2024 15:38:03 -0800 Subject: [PATCH 3/9] Cherry-pick "Passkey Error Prefix" (#2337) https://github.com/AzureAD/microsoft-authentication-library-common-for-android/pull/2336/files --- .../internal/fido/AuthFidoChallengeHandler.kt | 28 +++++++++++++++++-- .../common/java/constants/FidoConstants.kt | 5 ++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/common/src/main/java/com/microsoft/identity/common/internal/fido/AuthFidoChallengeHandler.kt b/common/src/main/java/com/microsoft/identity/common/internal/fido/AuthFidoChallengeHandler.kt index e39a7d9d5a..01079ed912 100644 --- a/common/src/main/java/com/microsoft/identity/common/internal/fido/AuthFidoChallengeHandler.kt +++ b/common/src/main/java/com/microsoft/identity/common/internal/fido/AuthFidoChallengeHandler.kt @@ -27,6 +27,7 @@ import androidx.lifecycle.LifecycleOwner import androidx.lifecycle.lifecycleScope import com.microsoft.identity.common.internal.ui.webview.challengehandlers.IChallengeHandler import com.microsoft.identity.common.java.constants.FidoConstants +import com.microsoft.identity.common.java.constants.FidoConstants.Companion.PASSKEY_PROTOCOL_ERROR_PREFIX_STRING import com.microsoft.identity.common.java.opentelemetry.AttributeName import com.microsoft.identity.common.java.opentelemetry.OTelUtility import com.microsoft.identity.common.java.opentelemetry.SpanName @@ -178,6 +179,7 @@ class AuthFidoChallengeHandler ( * @param errorMessage Error message string. * @param exception Exception associated with error. Default null. * @param assertion String representing response with signed challenge. + * @param methodTag methodTag of method where exception occurred. */ fun respondToChallengeWithError(submitUrl: String, context: String, @@ -190,10 +192,32 @@ class AuthFidoChallengeHandler ( if (exception != null) { span.recordException(exception) span.setStatus(StatusCode.ERROR) - respondToChallenge(submitUrl, exception.javaClass.name + ": " + exception.message, context, span) } else { span.setStatus(StatusCode.ERROR, errorMessage) - respondToChallenge(submitUrl, errorMessage, context, span) } + respondToChallenge( + submitUrl, + getErrorAssertion( + errorMessage, + exception + ), + context, + span + ) + } + + /** + * Formats the exception/error message to be sent via the assertion header of the passkey protocol. + * + * @param errorMessage Error message string. + * @param exception Exception associated with error. Default null. + */ + private fun getErrorAssertion(errorMessage: String, + exception: Exception? = null + ): String { + if (exception != null) { + return PASSKEY_PROTOCOL_ERROR_PREFIX_STRING + exception.javaClass.name + ": " + exception.message + } + return PASSKEY_PROTOCOL_ERROR_PREFIX_STRING + errorMessage } } diff --git a/common4j/src/main/com/microsoft/identity/common/java/constants/FidoConstants.kt b/common4j/src/main/com/microsoft/identity/common/java/constants/FidoConstants.kt index d7824cb7b2..c77fb56263 100644 --- a/common4j/src/main/com/microsoft/identity/common/java/constants/FidoConstants.kt +++ b/common4j/src/main/com/microsoft/identity/common/java/constants/FidoConstants.kt @@ -105,6 +105,11 @@ class FidoConstants { */ const val PASSKEY_PROTOCOL_HEADER_VALUE = "$PASSKEY_PROTOCOL_VERSION/$PASSKEY_PROTOCOL_KEY_TYPES_SUPPORTED" + /** + * Error messages sent to ESTS via the protocol should have a prefix attached. + */ + const val PASSKEY_PROTOCOL_ERROR_PREFIX_STRING = "ERROR: " + /** * JSON key value of assertion response of authentication response JSON object. */ From b86e7f89570decbc1775aaf8de17e5be702b69c0 Mon Sep 17 00:00:00 2001 From: Dome Pongmongkol <19558668+rpdome@users.noreply.github.com> Date: Tue, 12 Mar 2024 12:22:15 -0500 Subject: [PATCH 4/9] Cherry-pick #2339 (#2342) #2339 --- changelog.txt | 1 + .../controllers/BrokerMsalController.java | 56 ++++++++++++------- 2 files changed, 38 insertions(+), 19 deletions(-) diff --git a/changelog.txt b/changelog.txt index c02722ebdc..a04c6ce00c 100644 --- a/changelog.txt +++ b/changelog.txt @@ -1,5 +1,6 @@ V.17.2.0 --------- +- [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) diff --git a/common/src/main/java/com/microsoft/identity/common/internal/controllers/BrokerMsalController.java b/common/src/main/java/com/microsoft/identity/common/internal/controllers/BrokerMsalController.java index 109bb9f9f0..c1bfe9791a 100644 --- a/common/src/main/java/com/microsoft/identity/common/internal/controllers/BrokerMsalController.java +++ b/common/src/main/java/com/microsoft/identity/common/internal/controllers/BrokerMsalController.java @@ -52,6 +52,7 @@ import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; +import androidx.annotation.WorkerThread; import com.microsoft.identity.common.PropertyBagUtil; import com.microsoft.identity.common.adal.internal.AuthenticationConstants; @@ -133,22 +134,25 @@ public class BrokerMsalController extends BaseController { private ResultFuture mBrokerResultFuture; private final String mActiveBrokerPackageName; - private final BrokerOperationExecutor mBrokerOperationExecutor; private final HelloCache mHelloCache; private final IPlatformComponents mComponents; private final Context mApplicationContext; + // Can be set in unit tests, otherwise this will be null. + @Nullable + private final List ipcStrategies; + + private BrokerOperationExecutor mOperationExecutor; + + @VisibleForTesting public BrokerMsalController(@NonNull final Context applicationContext, @NonNull final IPlatformComponents components, @NonNull final String activeBrokerPackageName, - @NonNull final List ipcStrategies) { + @Nullable final List ipcStrategies) { mComponents = components; mApplicationContext = applicationContext; mActiveBrokerPackageName = activeBrokerPackageName; - mBrokerOperationExecutor = new BrokerOperationExecutor( - ipcStrategies, - new ActiveBrokerCacheUpdater(applicationContext, - ClientActiveBrokerCache.getClientSdkCache(components.getStorageSupplier()))); + this.ipcStrategies = ipcStrategies; mHelloCache = getHelloCache(); } @@ -158,7 +162,21 @@ public BrokerMsalController(@NonNull final Context applicationContext, this(applicationContext, components, activeBrokerPackageName, - OneAuthSharedFunctions.getIpcStrategies(applicationContext, activeBrokerPackageName)); + null); + } + + /** Should only be invoked in Background thread, given that getIpcStrategies could be a long running operation. */ + @WorkerThread + @NonNull + private synchronized BrokerOperationExecutor getBrokerOperationExecutor(){ + if (mOperationExecutor == null) { + mOperationExecutor = new BrokerOperationExecutor( + ipcStrategies != null ? ipcStrategies : + OneAuthSharedFunctions.getIpcStrategies(mApplicationContext, mActiveBrokerPackageName), + new ActiveBrokerCacheUpdater(mApplicationContext, + ClientActiveBrokerCache.getClientSdkCache(mComponents.getStorageSupplier()))); + } + return mOperationExecutor; } @VisibleForTesting @@ -413,7 +431,7 @@ public void onFinishAuthorizationSession(int requestCode, private @NonNull Intent getBrokerAuthorizationIntent( final @NonNull InteractiveTokenCommandParameters parameters) throws BaseException { - return mBrokerOperationExecutor.execute(parameters, + return getBrokerOperationExecutor().execute(parameters, new BrokerOperation() { private String negotiatedBrokerProtocolVersion; @@ -472,7 +490,7 @@ public void putValueInSuccessEvent(final @NonNull ApiEndEvent event, final @NonN public AuthorizationResult deviceCodeFlowAuthRequest(final DeviceCodeFlowCommandParameters parameters) throws BaseException, ClientException { // IPC to Broker : fetch DCF auth result - return mBrokerOperationExecutor.execute(parameters, + return getBrokerOperationExecutor().execute(parameters, new BrokerOperation() { private String negotiatedBrokerProtocolVersion; @@ -525,7 +543,7 @@ public AcquireTokenResult acquireDeviceCodeFlowToken( throws BaseException, ClientException { // IPC to Broker : AcquireTokenWithDCF API in Broker - return mBrokerOperationExecutor.execute(parameters, + return getBrokerOperationExecutor().execute(parameters, new BrokerOperation() { private String negotiatedBrokerProtocolVersion; @@ -597,7 +615,7 @@ public void putValueInSuccessEvent(final @NonNull ApiEndEvent event, final @NonN @Override public @NonNull AcquireTokenResult acquireTokenSilent(final @NonNull SilentTokenCommandParameters parameters) throws BaseException { - return mBrokerOperationExecutor.execute(parameters, + return getBrokerOperationExecutor().execute(parameters, new BrokerOperation() { private String negotiatedBrokerProtocolVersion; @@ -659,7 +677,7 @@ public void putValueInSuccessEvent(final @NonNull ApiEndEvent event, final @NonN @Override public @NonNull List getAccounts(final @NonNull CommandParameters parameters) throws BaseException { - return mBrokerOperationExecutor.execute(parameters, + return getBrokerOperationExecutor().execute(parameters, new BrokerOperation>() { private String negotiatedBrokerProtocolVersion; @@ -718,7 +736,7 @@ public void putValueInSuccessEvent(final @NonNull ApiEndEvent event, final @NonN */ @Override public boolean removeAccount(final @NonNull RemoveAccountCommandParameters parameters) throws BaseException { - return mBrokerOperationExecutor.execute(parameters, + return getBrokerOperationExecutor().execute(parameters, new BrokerOperation() { private String negotiatedBrokerProtocolVersion; @@ -774,7 +792,7 @@ public void putValueInSuccessEvent(final @NonNull ApiEndEvent event, final @NonN @Override public PreferredAuthMethod getPreferredAuthMethod() throws BaseException { final String methodTag = TAG + ":getPreferredAuthMethod"; - return mBrokerOperationExecutor.execute( + return getBrokerOperationExecutor().execute( null, new BrokerOperation() { @@ -838,7 +856,7 @@ public void putValueInSuccessEvent(final @NonNull ApiEndEvent event, final @NonN */ @Override public boolean getDeviceMode(final @NonNull CommandParameters parameters) throws BaseException { - return mBrokerOperationExecutor.execute(parameters, + return getBrokerOperationExecutor().execute(parameters, new BrokerOperation() { @Override public void performPrerequisites(final @NonNull IIpcStrategy strategy) { @@ -898,7 +916,7 @@ List getCurrentAccount(final @NonNull CommandParameters parameters return getAccounts(parameters); } - return mBrokerOperationExecutor.execute(parameters, + return getBrokerOperationExecutor().execute(parameters, new BrokerOperation>() { private String negotiatedBrokerProtocolVersion; @@ -974,7 +992,7 @@ public boolean removeCurrentAccount(final @NonNull RemoveAccountCommandParameter * If everything succeeds on the broker side, it will then * 4. Sign out from default browser. */ - return mBrokerOperationExecutor.execute(parameters, + return getBrokerOperationExecutor().execute(parameters, new BrokerOperation() { private String negotiatedBrokerProtocolVersion; @@ -1028,7 +1046,7 @@ public AcquireTokenResult acquireTokenWithPassword(@lombok.NonNull RopcTokenComm @Override public GenerateShrResult generateSignedHttpRequest(@NonNull final GenerateShrCommandParameters parameters) throws BaseException { - return mBrokerOperationExecutor.execute(parameters, new BrokerOperation() { + return getBrokerOperationExecutor().execute(parameters, new BrokerOperation() { private String negotiatedBrokerProtocolVersion; @@ -1082,7 +1100,7 @@ public void putValueInSuccessEvent(@NonNull final ApiEndEvent event, } public AcquirePrtSsoTokenResult getSsoToken(final @NonNull AcquirePrtSsoTokenCommandParameters parameters) throws BaseException { - return mBrokerOperationExecutor.execute(parameters, new BrokerOperation() { + return getBrokerOperationExecutor().execute(parameters, new BrokerOperation() { private String negotiatedBrokerProtocolVersion; From b62f43ceb474729ec93f95653e15f0bc4ee35cce Mon Sep 17 00:00:00 2001 From: Dome Pongmongkol <19558668+rpdome@users.noreply.github.com> Date: Tue, 12 Mar 2024 13:24:06 -0500 Subject: [PATCH 5/9] Cherry-pick #2338 (#2346) --- changelog.txt | 1 + .../BrokerDiscoveryClient.kt | 4 +- .../BrokerDiscoveryClientFactory.kt | 5 +- .../broker/ipc/ContentProviderStatusLoader.kt | 104 ++++++++++++++ .../broker/ipc/ContentProviderStrategy.java | 30 ++-- .../ipc/IContentProviderStatusLoader.kt | 34 +++++ .../OneAuthSharedFunctions.kt | 24 +++- .../ipc/ContentProviderStatusLoaderTest.kt | 129 ++++++++++++++++++ .../ipc/ContentProviderStrategyTests.java | 14 +- 9 files changed, 319 insertions(+), 26 deletions(-) create mode 100644 common/src/main/java/com/microsoft/identity/common/internal/broker/ipc/ContentProviderStatusLoader.kt create mode 100644 common/src/main/java/com/microsoft/identity/common/internal/broker/ipc/IContentProviderStatusLoader.kt create mode 100644 common/src/test/java/com/microsoft/identity/common/internal/broker/ipc/ContentProviderStatusLoaderTest.kt diff --git a/changelog.txt b/changelog.txt index a04c6ce00c..0d0f59a72e 100644 --- a/changelog.txt +++ b/changelog.txt @@ -1,5 +1,6 @@ V.17.2.0 --------- +- [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) diff --git a/common/src/main/java/com/microsoft/identity/common/internal/activebrokerdiscovery/BrokerDiscoveryClient.kt b/common/src/main/java/com/microsoft/identity/common/internal/activebrokerdiscovery/BrokerDiscoveryClient.kt index 986f9de3b5..4cef6c5a67 100644 --- a/common/src/main/java/com/microsoft/identity/common/internal/activebrokerdiscovery/BrokerDiscoveryClient.kt +++ b/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, } 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). diff --git a/common/src/main/java/com/microsoft/identity/common/internal/activebrokerdiscovery/BrokerDiscoveryClientFactory.kt b/common/src/main/java/com/microsoft/identity/common/internal/activebrokerdiscovery/BrokerDiscoveryClientFactory.kt index b73980af20..0b10ccffbc 100644 --- a/common/src/main/java/com/microsoft/identity/common/internal/activebrokerdiscovery/BrokerDiscoveryClientFactory.kt +++ b/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) diff --git a/common/src/main/java/com/microsoft/identity/common/internal/broker/ipc/ContentProviderStatusLoader.kt b/common/src/main/java/com/microsoft/identity/common/internal/broker/ipc/ContentProviderStatusLoader.kt new file mode 100644 index 0000000000..b8c9a9c790 --- /dev/null +++ b/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, +) : 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 = 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 + } + } +} diff --git a/common/src/main/java/com/microsoft/identity/common/internal/broker/ipc/ContentProviderStrategy.java b/common/src/main/java/com/microsoft/identity/common/internal/broker/ipc/ContentProviderStrategy.java index aeae358d70..9faa406f2c 100644 --- a/common/src/main/java/com/microsoft/identity/common/internal/broker/ipc/ContentProviderStrategy.java +++ b/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,10 +40,9 @@ 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. */ @@ -52,17 +50,21 @@ public class ContentProviderStrategy extends AbstractIpcStrategyWithServiceValid 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 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); } } diff --git a/common/src/main/java/com/microsoft/identity/common/internal/broker/ipc/IContentProviderStatusLoader.kt b/common/src/main/java/com/microsoft/identity/common/internal/broker/ipc/IContentProviderStatusLoader.kt new file mode 100644 index 0000000000..e4cfcfcf07 --- /dev/null +++ b/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 +} diff --git a/common/src/main/java/com/microsoft/identity/common/sharedwithoneauth/OneAuthSharedFunctions.kt b/common/src/main/java/com/microsoft/identity/common/sharedwithoneauth/OneAuthSharedFunctions.kt index 9ce3826168..65186b783b 100644 --- a/common/src/main/java/com/microsoft/identity/common/sharedwithoneauth/OneAuthSharedFunctions.kt +++ b/common/src/main/java/com/microsoft/identity/common/sharedwithoneauth/OneAuthSharedFunctions.kt @@ -23,11 +23,14 @@ package com.microsoft.identity.common.sharedwithoneauth import android.content.Context +import androidx.annotation.WorkerThread +import com.microsoft.identity.common.components.AndroidPlatformComponentsFactory import com.microsoft.identity.common.internal.broker.MicrosoftAuthClient import com.microsoft.identity.common.internal.broker.ipc.AccountManagerAddAccountStrategy import com.microsoft.identity.common.internal.broker.ipc.BoundServiceStrategy import com.microsoft.identity.common.internal.broker.ipc.ContentProviderStrategy import com.microsoft.identity.common.internal.broker.ipc.IIpcStrategy +import com.microsoft.identity.common.java.interfaces.IPlatformComponents import com.microsoft.identity.common.logging.Logger // Functions to be invoked by both OneAuth and MSAL Android @@ -48,13 +51,32 @@ class OneAuthSharedFunctions { fun getIpcStrategies( context: Context, activeBrokerPackageName: String, + ): List { + return getIpcStrategies(context, + AndroidPlatformComponentsFactory.createFromContext(context), + activeBrokerPackageName) + } + + /** + * Constructs a list of [IIpcStrategy] to communicate from + * OneAuth/MSAL to Broker process. + * + * @param context [Context] + * @param components [IPlatformComponents] + * @param activeBrokerPackageName name of the app hosting the broker process to communicate to. + **/ + @JvmStatic + fun getIpcStrategies( + context: Context, + components: IPlatformComponents, + activeBrokerPackageName: String, ): List { val methodTag = "$TAG:getIpcStrategies" val strategies: MutableList = ArrayList() val sb = StringBuilder(100) sb.append("Broker Strategies added : ") - val contentProviderStrategy = ContentProviderStrategy(context) + val contentProviderStrategy = ContentProviderStrategy(context, components) if (contentProviderStrategy.isSupportedByTargetedBroker(activeBrokerPackageName)) { sb.append("ContentProviderStrategy, ") strategies.add(contentProviderStrategy) diff --git a/common/src/test/java/com/microsoft/identity/common/internal/broker/ipc/ContentProviderStatusLoaderTest.kt b/common/src/test/java/com/microsoft/identity/common/internal/broker/ipc/ContentProviderStatusLoaderTest.kt new file mode 100644 index 0000000000..512b279d99 --- /dev/null +++ b/common/src/test/java/com/microsoft/identity/common/internal/broker/ipc/ContentProviderStatusLoaderTest.kt @@ -0,0 +1,129 @@ +// 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.pm.PackageManager +import com.microsoft.identity.common.java.util.ported.InMemoryStorage +import org.junit.Assert +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +@RunWith(RobolectricTestRunner::class) +class ContentProviderStatusLoaderTest { + + @Test + fun testAppNotInstalled(){ + val loader = ContentProviderStatusLoader( + getVersionCode = { throw PackageManager.NameNotFoundException() }, + supportedByContentProvider = { throw IllegalStateException() }, + fileManager = InMemoryStorage() + ) + + Assert.assertFalse(loader.supportsContentProvider("com.microsoft.test")) + } + + @Test + fun testGetValidResultAndPersistInCache(){ + val cache = InMemoryStorage() + val versionKey = "1.2.3456" + + val loader = ContentProviderStatusLoader( + getVersionCode = { pkgName -> + if (pkgName == "com.microsoft.test") + return@ContentProviderStatusLoader versionKey + throw PackageManager.NameNotFoundException() + }, + supportedByContentProvider = { true }, + fileManager = cache + ) + + Assert.assertTrue(loader.supportsContentProvider("com.microsoft.test")) + Assert.assertTrue(cache.get("com.microsoft.test:$versionKey").toBoolean()) + Assert.assertEquals(1, cache.size()) + } + + @Test + fun testGetInalidResultAndPersistInCache(){ + val cache = InMemoryStorage() + val versionKey = "123.456.789" + + val loader = ContentProviderStatusLoader( + getVersionCode = { pkgName -> + if (pkgName == "com.microsoft.test") + return@ContentProviderStatusLoader versionKey + throw PackageManager.NameNotFoundException() + }, + supportedByContentProvider = { false }, + fileManager = cache + ) + + Assert.assertFalse(loader.supportsContentProvider("com.microsoft.test")) + Assert.assertFalse(cache.get("com.microsoft.test:$versionKey").toBoolean()) + Assert.assertEquals(1, cache.size()) + } + + @Test + fun testGetValueFromCache(){ + val cache = InMemoryStorage() + val versionKey = "123.456.789" + cache.put("com.microsoft.test:$versionKey", true.toString()) + + val loader = ContentProviderStatusLoader( + getVersionCode = { pkgName -> + if (pkgName == "com.microsoft.test") + return@ContentProviderStatusLoader versionKey + throw PackageManager.NameNotFoundException() + }, + supportedByContentProvider = { + throw IllegalStateException("This line should not be reached") }, + fileManager = cache + ) + + Assert.assertTrue(loader.supportsContentProvider("com.microsoft.test")) + Assert.assertEquals(1, cache.size()) + } + + @Test + fun testTryGetValueFromCache_VersionMismatch(){ + val cache = InMemoryStorage() + val versionKey = "123.456.789" + cache.put("com.microsoft.test:$versionKey", true.toString()) + val newVersionKey = "1.2.234" + + val loader = ContentProviderStatusLoader( + getVersionCode = { pkgName -> + if (pkgName == "com.microsoft.test") + // Returns a different version + return@ContentProviderStatusLoader newVersionKey + throw PackageManager.NameNotFoundException() + }, + supportedByContentProvider = { false }, + fileManager = cache + ) + + Assert.assertFalse(loader.supportsContentProvider("com.microsoft.test")) + Assert.assertFalse(cache.get("com.microsoft.test:$newVersionKey").toBoolean()) + Assert.assertEquals(2, cache.size()) + } +} diff --git a/common/src/test/java/com/microsoft/identity/common/internal/broker/ipc/ContentProviderStrategyTests.java b/common/src/test/java/com/microsoft/identity/common/internal/broker/ipc/ContentProviderStrategyTests.java index c2120df0ac..d14a9b82a3 100644 --- a/common/src/test/java/com/microsoft/identity/common/internal/broker/ipc/ContentProviderStrategyTests.java +++ b/common/src/test/java/com/microsoft/identity/common/internal/broker/ipc/ContentProviderStrategyTests.java @@ -45,12 +45,24 @@ import static com.microsoft.identity.common.internal.broker.ipc.BrokerOperationBundle.Operation.MSAL_REMOVE_ACCOUNT; import static com.microsoft.identity.common.internal.broker.ipc.BrokerOperationBundle.Operation.MSAL_SIGN_OUT_FROM_SHARED_DEVICE; +import lombok.NonNull; + @RunWith(RobolectricTestRunner.class) @Config(sdk = {Build.VERSION_CODES.N}, shadows = {ShadowContentResolverWithSuccessResult.class}) public class ContentProviderStrategyTests extends IpcStrategyTests { + + static class MockContentProviderStatusLoader implements IContentProviderStatusLoader { + @Override + public boolean supportsContentProvider(@NonNull String packageName) { + return true; + } + } + @Override protected IIpcStrategy getStrategy() { - return new ContentProviderStrategy(ApplicationProvider.getApplicationContext(), true); + return new ContentProviderStrategy(ApplicationProvider.getApplicationContext(), + new MockContentProviderStatusLoader(), + true); } @Test From e228b8ff030d219464a584c402315df058d292d1 Mon Sep 17 00:00:00 2001 From: Dome Pongmongkol <19558668+rpdome@users.noreply.github.com> Date: Tue, 12 Mar 2024 17:02:21 -0500 Subject: [PATCH 6/9] Cherry-pick #2340 (#2347) #2340 --- changelog.txt | 1 + .../internal/cache/BaseActiveBrokerCache.kt | 49 ++++++++++++-- .../internal/cache/ClientActiveBrokerCache.kt | 50 -------------- .../internal/cache/IActiveBrokerCache.kt | 12 ++++ .../cache/IClientActiveBrokerCache.kt | 16 ----- .../InMemoryActiveBrokerCache.kt | 2 +- .../internal/cache/ActiveBrokerCacheTest.kt | 56 ++++++++++++++++ .../cache/ClientActiveBrokerCacheTest.kt | 66 ------------------- 8 files changed, 113 insertions(+), 139 deletions(-) delete mode 100644 common/src/test/java/com/microsoft/identity/common/internal/cache/ClientActiveBrokerCacheTest.kt diff --git a/changelog.txt b/changelog.txt index 0d0f59a72e..97f491f887 100644 --- a/changelog.txt +++ b/changelog.txt @@ -1,5 +1,6 @@ V.17.2.0 --------- +- [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) diff --git a/common/src/main/java/com/microsoft/identity/common/internal/cache/BaseActiveBrokerCache.kt b/common/src/main/java/com/microsoft/identity/common/internal/cache/BaseActiveBrokerCache.kt index 2f4cb9ce84..180846811b 100644 --- a/common/src/main/java/com/microsoft/identity/common/internal/cache/BaseActiveBrokerCache.kt +++ b/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,7 +113,9 @@ 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) } } } @@ -82,10 +123,6 @@ open class BaseActiveBrokerCache 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) } } \ No newline at end of file diff --git a/common/src/main/java/com/microsoft/identity/common/internal/cache/ClientActiveBrokerCache.kt b/common/src/main/java/com/microsoft/identity/common/internal/cache/ClientActiveBrokerCache.kt index 7b8b70fe25..10d60b25d3 100644 --- a/common/src/main/java/com/microsoft/identity/common/internal/cache/ClientActiveBrokerCache.kt +++ b/common/src/main/java/com/microsoft/identity/common/internal/cache/ClientActiveBrokerCache.kt @@ -24,9 +24,7 @@ package com.microsoft.identity.common.internal.cache import com.microsoft.identity.common.java.interfaces.INameValueStorage import com.microsoft.identity.common.java.interfaces.IStorageSupplier -import kotlinx.coroutines.runBlocking import kotlinx.coroutines.sync.Mutex -import kotlinx.coroutines.sync.withLock class ClientActiveBrokerCache internal constructor(private val storage: INameValueStorage, @@ -84,53 +82,5 @@ internal constructor(private val storage: INameValueStorage, lock = sClientSdkSideLock ) } - - /** - * 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 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 clearCachedActiveBroker() { - return runBlocking { - lock.withLock { - clearCachedActiveBrokerWithoutLock() - storage.remove(SHOULD_USE_ACCOUNT_MANAGER_UNTIL_EPOCH_MILLISECONDS_KEY) - } - } } } \ No newline at end of file diff --git a/common/src/main/java/com/microsoft/identity/common/internal/cache/IActiveBrokerCache.kt b/common/src/main/java/com/microsoft/identity/common/internal/cache/IActiveBrokerCache.kt index a4d264e14b..04d28630ee 100644 --- a/common/src/main/java/com/microsoft/identity/common/internal/cache/IActiveBrokerCache.kt +++ b/common/src/main/java/com/microsoft/identity/common/internal/cache/IActiveBrokerCache.kt @@ -49,4 +49,16 @@ interface IActiveBrokerCache { * Clears the active broker from the cache. */ fun clearCachedActiveBroker() + + /** + * Returns true if AccountManager should still be used. + **/ + fun shouldUseAccountManager(): Boolean + + /** + * Set the time span when AccountManager should still be used. + * + * @param timeInMillis Time in milliseconds (from now) + **/ + fun setShouldUseAccountManagerForTheNextMilliseconds(timeInMillis: Long) } \ No newline at end of file diff --git a/common/src/main/java/com/microsoft/identity/common/internal/cache/IClientActiveBrokerCache.kt b/common/src/main/java/com/microsoft/identity/common/internal/cache/IClientActiveBrokerCache.kt index 5b62bfae51..a772061897 100644 --- a/common/src/main/java/com/microsoft/identity/common/internal/cache/IClientActiveBrokerCache.kt +++ b/common/src/main/java/com/microsoft/identity/common/internal/cache/IClientActiveBrokerCache.kt @@ -22,24 +22,8 @@ // THE SOFTWARE. package com.microsoft.identity.common.internal.cache -import com.microsoft.identity.common.internal.activebrokerdiscovery.BrokerDiscoveryClient - /** * An extension of [IActiveBrokerCache]. - * This will allow [BrokerDiscoveryClient] to fall back to AccountManager immediately if it's aware that the broker side - * Does not support account manager (as opposed to trying to make unnecessary IPC call every time). * */ interface IClientActiveBrokerCache: IActiveBrokerCache { - - /** - * Returns true if [BrokerDiscoveryClient] should still use AccountManager. - **/ - fun shouldUseAccountManager(): Boolean - - /** - * Set the time span when [BrokerDiscoveryClient] should just rely on AccountManager. - * - * @param timeInMillis Time in milliseconds (from now) - **/ - fun setShouldUseAccountManagerForTheNextMilliseconds(timeInMillis: Long) } \ No newline at end of file diff --git a/common/src/test/java/com/microsoft/identity/common/internal/activebrokerdiscovery/InMemoryActiveBrokerCache.kt b/common/src/test/java/com/microsoft/identity/common/internal/activebrokerdiscovery/InMemoryActiveBrokerCache.kt index 9ef0374556..341e3128f4 100644 --- a/common/src/test/java/com/microsoft/identity/common/internal/activebrokerdiscovery/InMemoryActiveBrokerCache.kt +++ b/common/src/test/java/com/microsoft/identity/common/internal/activebrokerdiscovery/InMemoryActiveBrokerCache.kt @@ -23,7 +23,7 @@ package com.microsoft.identity.common.internal.activebrokerdiscovery import com.microsoft.identity.common.internal.broker.BrokerData -import com.microsoft.identity.common.internal.cache.ClientActiveBrokerCache.Companion.isNotExpired +import com.microsoft.identity.common.internal.cache.BaseActiveBrokerCache.Companion.isNotExpired import com.microsoft.identity.common.internal.cache.IActiveBrokerCache import com.microsoft.identity.common.internal.cache.IClientActiveBrokerCache import java.time.Instant diff --git a/common/src/test/java/com/microsoft/identity/common/internal/cache/ActiveBrokerCacheTest.kt b/common/src/test/java/com/microsoft/identity/common/internal/cache/ActiveBrokerCacheTest.kt index af0f663cba..2cc47cf4c6 100644 --- a/common/src/test/java/com/microsoft/identity/common/internal/cache/ActiveBrokerCacheTest.kt +++ b/common/src/test/java/com/microsoft/identity/common/internal/cache/ActiveBrokerCacheTest.kt @@ -120,6 +120,10 @@ class BaseActiveBrokerCacheTest { } override fun remove(name: String) { + if (name == BaseActiveBrokerCache.SHOULD_USE_ACCOUNT_MANAGER_UNTIL_EPOCH_MILLISECONDS_KEY) { + return + } + throw UnsupportedOperationException() } @@ -211,4 +215,56 @@ class BaseActiveBrokerCacheTest { cache.clearCachedActiveBroker() Assert.assertNull(cache.getCachedActiveBroker()) } + + @Test + fun testSetSkipToAccountManager(){ + val cache = BaseActiveBrokerCache(InMemoryStorage(), Mutex()) + Assert.assertNull(cache.getCachedActiveBroker()) + + Assert.assertFalse(cache.shouldUseAccountManager()) + + cache.setShouldUseAccountManagerForTheNextMilliseconds( + TimeUnit.SECONDS.toMillis(2) + ) + Assert.assertTrue(cache.shouldUseAccountManager()) + + Thread.sleep(TimeUnit.SECONDS.toMillis(2)) + + Assert.assertFalse(cache.shouldUseAccountManager()) + } + + @Test + fun testSetSkipToAccountManager_ClearValue(){ + val cache = BaseActiveBrokerCache(InMemoryStorage(), Mutex()) + Assert.assertNull(cache.getCachedActiveBroker()) + + Assert.assertFalse(cache.shouldUseAccountManager()) + + cache.setShouldUseAccountManagerForTheNextMilliseconds( + TimeUnit.SECONDS.toMillis(2) + ) + Assert.assertTrue(cache.shouldUseAccountManager()) + + cache.clearCachedActiveBroker() + + Assert.assertFalse(cache.shouldUseAccountManager()) + } + + @Test + fun testSetSkipToAccountManager_ForceSetBroker(){ + val cache = BaseActiveBrokerCache(InMemoryStorage(), Mutex()) + Assert.assertNull(cache.getCachedActiveBroker()) + + Assert.assertFalse(cache.shouldUseAccountManager()) + + cache.setShouldUseAccountManagerForTheNextMilliseconds( + TimeUnit.SECONDS.toMillis(60) + ) + Assert.assertTrue(cache.shouldUseAccountManager()) + + val mockData = BrokerData(MOCK_PACKAGE_NAME, MOCK_HASH) + cache.setCachedActiveBroker(mockData) + + Assert.assertFalse(cache.shouldUseAccountManager()) + } } diff --git a/common/src/test/java/com/microsoft/identity/common/internal/cache/ClientActiveBrokerCacheTest.kt b/common/src/test/java/com/microsoft/identity/common/internal/cache/ClientActiveBrokerCacheTest.kt deleted file mode 100644 index ee82140796..0000000000 --- a/common/src/test/java/com/microsoft/identity/common/internal/cache/ClientActiveBrokerCacheTest.kt +++ /dev/null @@ -1,66 +0,0 @@ -// 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.cache - -import com.microsoft.identity.common.java.util.ported.InMemoryStorage -import kotlinx.coroutines.sync.Mutex -import org.junit.Assert -import org.junit.Test -import java.util.concurrent.TimeUnit - -class ClientActiveBrokerCacheTest { - - @Test - fun testSetSkipToAccountManager(){ - val cache = ClientActiveBrokerCache(InMemoryStorage(), Mutex()) - Assert.assertNull(cache.getCachedActiveBroker()) - - Assert.assertFalse(cache.shouldUseAccountManager()) - - cache.setShouldUseAccountManagerForTheNextMilliseconds( - TimeUnit.SECONDS.toMillis(2) - ) - Assert.assertTrue(cache.shouldUseAccountManager()) - - Thread.sleep(TimeUnit.SECONDS.toMillis(2)) - - Assert.assertFalse(cache.shouldUseAccountManager()) - } - - @Test - fun testSetSkipToAccountManager_ClearValue(){ - val cache = ClientActiveBrokerCache(InMemoryStorage(), Mutex()) - Assert.assertNull(cache.getCachedActiveBroker()) - - Assert.assertFalse(cache.shouldUseAccountManager()) - - cache.setShouldUseAccountManagerForTheNextMilliseconds( - TimeUnit.SECONDS.toMillis(2) - ) - Assert.assertTrue(cache.shouldUseAccountManager()) - - cache.clearCachedActiveBroker() - - Assert.assertFalse(cache.shouldUseAccountManager()) - } -} From 43c3b3c8de90f605d69d44e6d830519659e55da6 Mon Sep 17 00:00:00 2001 From: Sowmya Malayanur Date: Thu, 14 Mar 2024 02:15:59 -0700 Subject: [PATCH 7/9] removed RC tags --- common/build.gradle | 2 +- common4j/versioning/version.properties | 2 +- versioning/version.properties | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/common/build.gradle b/common/build.gradle index e7f7b28d2f..a218380e9e 100644 --- a/common/build.gradle +++ b/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 = "14.2.0-RC1" +def common4jVersion = "14.2.0" if (project.hasProperty("distCommon4jVersion") && project.distCommon4jVersion != '') { common4jVersion = project.distCommon4jVersion } diff --git a/common4j/versioning/version.properties b/common4j/versioning/version.properties index 7dd105a9c7..522ce4ee76 100644 --- a/common4j/versioning/version.properties +++ b/common4j/versioning/version.properties @@ -1,4 +1,4 @@ #Wed May 12 20:08:39 UTC 2021 -versionName=14.2.0-RC1 +versionName=14.2.0 versionCode=1 latestPatchVersion=227 diff --git a/versioning/version.properties b/versioning/version.properties index 9ec073e196..430637938f 100644 --- a/versioning/version.properties +++ b/versioning/version.properties @@ -1,4 +1,4 @@ #Tue Apr 06 22:55:08 UTC 2021 -versionName=17.2.0-RC1 +versionName=17.2.0 versionCode=1 latestPatchVersion=234 From 9efc166ae1bce3767cc5f066d06f660de9fb4ec9 Mon Sep 17 00:00:00 2001 From: Sowmya Malayanur Date: Thu, 14 Mar 2024 02:19:12 -0700 Subject: [PATCH 8/9] removed automation fix --- .../ui/automation/device/settings/BaseSettings.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/uiautomationutilities/src/main/java/com/microsoft/identity/client/ui/automation/device/settings/BaseSettings.java b/uiautomationutilities/src/main/java/com/microsoft/identity/client/ui/automation/device/settings/BaseSettings.java index 11bce8270f..28ec48068b 100644 --- a/uiautomationutilities/src/main/java/com/microsoft/identity/client/ui/automation/device/settings/BaseSettings.java +++ b/uiautomationutilities/src/main/java/com/microsoft/identity/client/ui/automation/device/settings/BaseSettings.java @@ -58,12 +58,12 @@ public void launchDeviceAdminSettingsPage() { context.startActivity(intent); final UiObject deviceAdminPage; -// if (android.os.Build.VERSION.SDK_INT == 32) { + if (android.os.Build.VERSION.SDK_INT == 32) { // This is needed for higher API levels. So far have confirmed this for API 32, but API 30 will use the one above deviceAdminPage = UiAutomatorUtils.obtainUiObjectWithDescription("Device admin apps"); -// } else { -// deviceAdminPage = UiAutomatorUtils.obtainUiObjectWithText("device admin"); -// } + } else { + deviceAdminPage = UiAutomatorUtils.obtainUiObjectWithText("device admin"); + } Assert.assertTrue("Device Admin Settings Page appears", deviceAdminPage.exists()); } From 00df1754fdcbdb5ae8a760a14d3c7e9e03426243 Mon Sep 17 00:00:00 2001 From: Sowmya Malayanur Date: Thu, 14 Mar 2024 12:34:23 -0700 Subject: [PATCH 9/9] changed version as old version is published with wrong branch --- changelog.txt | 2 +- common/build.gradle | 2 +- common4j/versioning/version.properties | 2 +- versioning/version.properties | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/changelog.txt b/changelog.txt index 97f491f887..effd543c9a 100644 --- a/changelog.txt +++ b/changelog.txt @@ -1,4 +1,4 @@ -V.17.2.0 +V.17.2.1 --------- - [PATCH] Move SHOULD_USE_ACCOUNT_MANAGER_UNTIL_EPOCH_MILLISECONDS_KEY to BaseActiveBrokerCache (#2340) - [PATCH] Add cache for ContentProviderStrategy.isSupportedByTargetedBroker() (#2338) diff --git a/common/build.gradle b/common/build.gradle index a218380e9e..f2f45d8f75 100644 --- a/common/build.gradle +++ b/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 = "14.2.0" +def common4jVersion = "14.2.1" if (project.hasProperty("distCommon4jVersion") && project.distCommon4jVersion != '') { common4jVersion = project.distCommon4jVersion } diff --git a/common4j/versioning/version.properties b/common4j/versioning/version.properties index 522ce4ee76..b2b04f3281 100644 --- a/common4j/versioning/version.properties +++ b/common4j/versioning/version.properties @@ -1,4 +1,4 @@ #Wed May 12 20:08:39 UTC 2021 -versionName=14.2.0 +versionName=14.2.1 versionCode=1 latestPatchVersion=227 diff --git a/versioning/version.properties b/versioning/version.properties index 430637938f..e7f3d4ef59 100644 --- a/versioning/version.properties +++ b/versioning/version.properties @@ -1,4 +1,4 @@ #Tue Apr 06 22:55:08 UTC 2021 -versionName=17.2.0 +versionName=17.2.1 versionCode=1 latestPatchVersion=234