From 08f1c4787eca8eafe1bde8fd4b84dda9a26f7b57 Mon Sep 17 00:00:00 2001 From: iamgusain <75644120+iamgusain@users.noreply.github.com> Date: Fri, 15 Mar 2024 12:25:39 -0700 Subject: [PATCH] Give BrokerMsalController higher priority than LocalMsalController if available (#2040) ### What Giving BrokerMsalController higher priority than LocalMsalController if available. This means that if Broker is available, msal will first try to acquire token via broker before trying its localController ### Why In order to push brokered auth as default when available. And reduce usage of regular RT, when PRT can be used which is bound to the device. ### How Changing the order of controllers to put BrokerMsalController (if eligible) before LocalMsalController ### Testing Verified AcquireTokenSilent calls reach Broker first. --- changelog | 1 + .../controllers/MSALControllerFactory.kt | 17 +++-- .../ShadowLegacyBrokerDiscoveryClient.kt | 36 ++++++++++ .../controllers/MSALControllerFactoryTest.kt | 69 +++++++++++++++++++ 4 files changed, 114 insertions(+), 9 deletions(-) create mode 100644 msal/src/test/java/com/microsoft/identity/client/e2e/shadows/ShadowLegacyBrokerDiscoveryClient.kt create mode 100644 msal/src/test/java/com/microsoft/identity/client/internal/controllers/MSALControllerFactoryTest.kt diff --git a/changelog b/changelog index ffce4fae28..0702d29962 100644 --- a/changelog +++ b/changelog @@ -2,6 +2,7 @@ MSAL Wiki : https://github.com/AzureAD/microsoft-authentication-library-for-andr vNext ---------- +- [MINOR] Give BrokerMsalController higher priority than LocalMsalController if available (#2040) - [MINOR] Rename "is QR + PIN available" API to "get preferred auth method". (#2012) - [MINOR] Add support for CIAM custom domain (#2029) - [MINOR] Removing passkey feature flag (#2044) diff --git a/msal/src/main/java/com/microsoft/identity/client/internal/controllers/MSALControllerFactory.kt b/msal/src/main/java/com/microsoft/identity/client/internal/controllers/MSALControllerFactory.kt index a370d9aca9..c52e254da7 100644 --- a/msal/src/main/java/com/microsoft/identity/client/internal/controllers/MSALControllerFactory.kt +++ b/msal/src/main/java/com/microsoft/identity/client/internal/controllers/MSALControllerFactory.kt @@ -89,26 +89,25 @@ class MSALControllerFactory( /** * Returns one or more controllers to address a given request. * - * The order of the response matters. The local controller should be returned first in order to - * ensure that any local refresh tokens are preferred over the use of the broker + * If requests is eligible for broker, {@link BrokerMsalController} will be + * added before {@link LocalMSALController}. otherwise only {@link LocalMSALController} will + * be returned. * - * Only return the broker controller when the following are true: + * Broker eligibility is determined if following conditions are true: * - * 1) The client indicates it wants to use broker - * 2) The authority is AAD - * 3) The audience is not AnyPersonalAccount - * 4) The broker is installed - * 5) The broker redirect URI for the client is registered + * 1) The broker is installed + * 2) The client indicates it wants to use broker + * 3) The authority is AAD */ fun getAllControllers(authority: Authority): List { val activeBroker = getActiveBrokerPackageName() val controllers: MutableList = ArrayList() - controllers.add(LocalMSALController()) if (!activeBroker.isNullOrEmpty() && brokerEligible(authority)) { controllers.add( BrokerMsalController(applicationContext, platformComponents, activeBroker) ) } + controllers.add(LocalMSALController()) return controllers.toList() } diff --git a/msal/src/test/java/com/microsoft/identity/client/e2e/shadows/ShadowLegacyBrokerDiscoveryClient.kt b/msal/src/test/java/com/microsoft/identity/client/e2e/shadows/ShadowLegacyBrokerDiscoveryClient.kt new file mode 100644 index 0000000000..7e9119e73a --- /dev/null +++ b/msal/src/test/java/com/microsoft/identity/client/e2e/shadows/ShadowLegacyBrokerDiscoveryClient.kt @@ -0,0 +1,36 @@ +// 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.client.e2e.shadows + +import com.microsoft.identity.common.internal.activebrokerdiscovery.LegacyBrokerDiscoveryClient +import com.microsoft.identity.common.internal.broker.BrokerData +import org.robolectric.annotation.Implements + +// A Shadow for mocking LegacyBrokerDiscoveryClient +@Implements(LegacyBrokerDiscoveryClient::class) +class ShadowLegacyBrokerDiscoveryClient { + + fun getActiveBroker(shouldSkipCache: Boolean): BrokerData? { + return BrokerData.debugBrokerHost + } +} \ No newline at end of file diff --git a/msal/src/test/java/com/microsoft/identity/client/internal/controllers/MSALControllerFactoryTest.kt b/msal/src/test/java/com/microsoft/identity/client/internal/controllers/MSALControllerFactoryTest.kt new file mode 100644 index 0000000000..f3b25ab331 --- /dev/null +++ b/msal/src/test/java/com/microsoft/identity/client/internal/controllers/MSALControllerFactoryTest.kt @@ -0,0 +1,69 @@ +// 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.client.internal.controllers + +import android.content.Context +import androidx.test.core.app.ApplicationProvider +import com.microsoft.identity.client.PublicClientApplicationConfiguration +import com.microsoft.identity.client.PublicClientApplicationConfigurationFactory +import com.microsoft.identity.client.e2e.shadows.ShadowLegacyBrokerDiscoveryClient +import com.microsoft.identity.common.internal.controllers.BrokerMsalController +import com.microsoft.identity.common.java.authorities.Authority +import com.microsoft.identity.msal.test.R +import org.junit.Assert +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config + +@RunWith(RobolectricTestRunner::class) +@Config(shadows = [ShadowLegacyBrokerDiscoveryClient::class] ) +class MSALControllerFactoryTest { + + private lateinit var pcaConfiguration: PublicClientApplicationConfiguration + private lateinit var msalControllerFactory: MSALControllerFactory + + @Before + fun setup() { + val context : Context = ApplicationProvider.getApplicationContext() + pcaConfiguration = PublicClientApplicationConfigurationFactory.loadConfiguration(context, R.raw.msal_default_config) + pcaConfiguration.appContext = context + Assert.assertTrue(pcaConfiguration.useBroker) + msalControllerFactory = MSALControllerFactory(pcaConfiguration) + } + @Test + fun testGetControllers() { + val testAuthority = Authority.getAuthorityFromAuthorityUrl("https://login.microsoftonline.com/common") + Assert.assertTrue(msalControllerFactory.brokerEligibleAndInstalled(testAuthority)) + Assert.assertEquals(2, msalControllerFactory.getAllControllers(testAuthority).size) + Assert.assertTrue(msalControllerFactory.getAllControllers(testAuthority)[0] is BrokerMsalController ) + } + + @Test + fun testGetDefaultController() { + val testAuthority = Authority.getAuthorityFromAuthorityUrl("https://login.microsoftonline.com/common") + Assert.assertTrue(msalControllerFactory.brokerEligibleAndInstalled(testAuthority)) + Assert.assertTrue(msalControllerFactory.getDefaultController(testAuthority) is BrokerMsalController ) + } +} \ No newline at end of file