DUNA. Prevent phish attack in “switch_browser” flow. , Fixes AB#3250169

changelog.txt

@@ -1,5 +1,6 @@
 vNext
 ----------
+- [MINOR] Adding a state parameter to prevent phish attacks, in “switch_browser” flow. (#2631)
 - [MINOR] Move camera logic to CameraPermissionRequestHandler and add SdmQrPinManager (#2630)
 - [MINOR] Pass Work Profile existence, OS Version, and Manufacturer to ESTS (#2627)
 - [MINOR] Add telemetry for the switch browser protocol (#2612)

common/src/main/java/com/microsoft/identity/common/adal/internal/AuthenticationConstants.java

@@ -560,6 +560,10 @@ public static final class SWITCH_BROWSER {
          */
         public static final String ACTION_URI = "action_uri";
 
+        /**
+         * String Query parameter key for the state blob.
+         */
+        public static final String STATE = "state";
     }
 
     /**

common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java

@@ -178,6 +178,7 @@ private void resumeSwitchBrowser(@NonNull final Bundle extras) {
         try {
             Logger.info(methodTag, "Resuming switch browser flow");
             getSwitchBrowserCoordinator().processSwitchBrowserResume(
+                    mAuthorizationRequestUrl,
                     extras,
                     (switchBrowserResumeUri, switchBrowserResumeHeaders) -> {
                         launchWebView(switchBrowserResumeUri.toString(), switchBrowserResumeHeaders);

common/src/main/java/com/microsoft/identity/common/internal/ui/webview/AzureActiveDirectoryWebViewClient.java

@@ -390,7 +390,7 @@ private void processSwitchBrowserRequest(@NonNull final String url) {
         final String methodTag = TAG + ":processSwitchBrowserRequest";
         try {
             mSwitchBrowserRequestHandler.processChallenge(
-                    SwitchBrowserChallenge.constructFromRedirectUrl(url)
+                    SwitchBrowserChallenge.constructFromRedirectUrl(url, mRequestUrl)
             );
         } catch (final Throwable throwable) {
             Logger.error(methodTag, "Switch browser challenge could not be processed.", throwable);

common/src/main/java/com/microsoft/identity/common/internal/ui/webview/challengehandlers/SwitchBrowserChallenge.kt

@@ -30,7 +30,8 @@ import com.microsoft.identity.common.internal.ui.webview.switchbrowser.SwitchBro
  * It contains the URI to be opened in the new browser.
  */
 data class SwitchBrowserChallenge(
-    val uri: Uri,
+    val processUri: Uri,
+    val authorizationUrl: String
 ) {
 
     companion object {
@@ -46,10 +47,13 @@ data class SwitchBrowserChallenge(
          */
         @JvmStatic
         @Throws(Exception::class)
-        fun constructFromRedirectUrl(redirectUrl: String): SwitchBrowserChallenge {
+        fun constructFromRedirectUrl(redirectUrl: String, authorizationUrl: String): SwitchBrowserChallenge {
             Uri.parse(redirectUrl).let { redirectUri ->
                 SwitchBrowserUriHelper.buildProcessUri(redirectUri).let { processUri ->
-                    return SwitchBrowserChallenge(uri = processUri)
+                    return SwitchBrowserChallenge(
+                        processUri = processUri,
+                        authorizationUrl = authorizationUrl
+                    )
                 }
             }
         }

common/src/main/java/com/microsoft/identity/common/internal/ui/webview/challengehandlers/SwitchBrowserRequestHandler.kt

@@ -28,6 +28,7 @@ import android.content.Intent
 import com.microsoft.identity.common.adal.internal.AuthenticationConstants.SWITCH_BROWSER
 import com.microsoft.identity.common.internal.ui.browser.AndroidBrowserSelector
 import com.microsoft.identity.common.internal.ui.browser.CustomTabsManager
+import com.microsoft.identity.common.internal.ui.webview.switchbrowser.SwitchBrowserUriHelper
 import com.microsoft.identity.common.internal.ui.webview.switchbrowser.SwitchBrowserUriHelper.isSwitchBrowserRedirectUrl
 import com.microsoft.identity.common.java.browser.IBrowserSelector
 import com.microsoft.identity.common.java.exception.ClientException
@@ -83,6 +84,9 @@ class SwitchBrowserRequestHandler(
     override fun processChallenge(switchBrowserChallenge: SwitchBrowserChallenge) {
         SpanExtension.makeCurrentSpan(span).use {
             val methodTag = "$TAG:processChallenge"
+
+            val state = switchBrowserChallenge.processUri.getQueryParameter(SWITCH_BROWSER.STATE)
+            SwitchBrowserUriHelper.statesMatch(switchBrowserChallenge.authorizationUrl, state)
 
             // Select a browser to handle the switch browser challenge
             val browser = browserSelector.selectBrowser(
@@ -125,7 +129,7 @@ class SwitchBrowserRequestHandler(
                 "Launching switch browser request on browser: ${browser.packageName}"
             )
             browserIntent.setPackage(browser.packageName)
-            browserIntent.setData(switchBrowserChallenge.uri)
+            browserIntent.setData(switchBrowserChallenge.processUri)
             activity.startActivity(browserIntent)
             isChallengeHandled = true
             span.setAttribute(

common/src/main/java/com/microsoft/identity/common/internal/ui/webview/switchbrowser/SwitchBrowserProtocolCoordinator.kt

@@ -31,8 +31,6 @@ import com.microsoft.identity.common.adal.internal.AuthenticationConstants.Autho
 import com.microsoft.identity.common.adal.internal.AuthenticationConstants.SWITCH_BROWSER
 import com.microsoft.identity.common.internal.providers.oauth2.BrokerAuthorizationActivity
 import com.microsoft.identity.common.internal.ui.webview.challengehandlers.SwitchBrowserRequestHandler
-import com.microsoft.identity.common.internal.ui.webview.switchbrowser.SwitchBrowserUriHelper.buildResumeUri
-import com.microsoft.identity.common.internal.ui.webview.switchbrowser.SwitchBrowserUriHelper.isSwitchBrowserRedirectUrl
 import com.microsoft.identity.common.java.AuthenticationConstants.AAD.AUTHORIZATION
 import com.microsoft.identity.common.java.exception.ClientException
 import com.microsoft.identity.common.java.opentelemetry.AttributeName
@@ -69,7 +67,7 @@ class SwitchBrowserProtocolCoordinator(
          * Returns `true` if the URL matches the expected pattern, `false` otherwise.
          */
         fun isSwitchBrowserResume(url: String?, redirectUrl: String): Boolean {
-            return isSwitchBrowserRedirectUrl(url, redirectUrl, SWITCH_BROWSER.RESUME_PATH)
+            return SwitchBrowserUriHelper.isSwitchBrowserRedirectUrl(url, redirectUrl, SWITCH_BROWSER.RESUME_PATH)
         }
 
         /**
@@ -94,6 +92,10 @@ class SwitchBrowserProtocolCoordinator(
                 SWITCH_BROWSER.CODE,
                 uri.getQueryParameter(SWITCH_BROWSER.CODE)
             )
+            intent.putExtra(
+                SWITCH_BROWSER.STATE,
+                uri.getQueryParameter(SWITCH_BROWSER.STATE)
+            )
             return intent
         }
     }
@@ -109,24 +111,29 @@ class SwitchBrowserProtocolCoordinator(
      */
     @Throws(ClientException::class)
     fun processSwitchBrowserResume(
+        authorizationRequest: String,
         extras: Bundle,
         onSuccessAction: (Uri, HashMap<String, String>) -> Unit
     ) {
         SpanExtension.makeCurrentSpan(span).use {
             val methodTag = "$TAG:processSwitchBrowserResume"
             val actionUri = extras.getString(SWITCH_BROWSER.ACTION_URI)
             val code = extras.getString(SWITCH_BROWSER.CODE)
+            val state = extras.getString(SWITCH_BROWSER.STATE)
             if (actionUri.isNullOrEmpty() || code.isNullOrEmpty()) {
                 val clientException = ClientException(
                     ClientException.MISSING_PARAMETER,
-                    "Action URI is null/empty: ${actionUri == null}, code is null/empty: ${code == null}"
+                    "Action URI is null/empty: ${actionUri.isNullOrEmpty()}," +
+                            " code is null/empty: ${code.isNullOrEmpty()}."
                 )
                 span.setStatus(StatusCode.ERROR)
                 span.recordException(clientException)
                 span.end()
                 throw clientException
             }
-            val resumeUri = buildResumeUri(actionUri)
+            SwitchBrowserUriHelper.statesMatch(authorizationRequest, state)
+            // Validate the state from auth request and redirect URL is the same
+            val resumeUri = SwitchBrowserUriHelper.buildResumeUri(actionUri, state)
             val headers = hashMapOf(AUTHORIZATION to code)
             onSuccessAction(resumeUri, headers)
             // Reset the challenge state after processing the resume action

common/src/main/java/com/microsoft/identity/common/internal/ui/webview/switchbrowser/SwitchBrowserUriHelper.kt

@@ -24,9 +24,12 @@ package com.microsoft.identity.common.internal.ui.webview.switchbrowser
 
 import android.net.Uri
 import com.microsoft.identity.common.adal.internal.AuthenticationConstants.SWITCH_BROWSER
-import com.microsoft.identity.common.java.AuthenticationConstants.OAuth2
 import com.microsoft.identity.common.java.exception.ClientException
+import com.microsoft.identity.common.java.flighting.CommonFlight
+import com.microsoft.identity.common.java.flighting.CommonFlightsManager
+import com.microsoft.identity.common.java.opentelemetry.SpanExtension
 import com.microsoft.identity.common.logging.Logger
+import io.opentelemetry.api.trace.StatusCode
 
 /**
  * SwitchBrowserUriHelper is a helper class to build URIs for the switch browser challenge.
@@ -35,6 +38,13 @@ object SwitchBrowserUriHelper {
 
     private const val TAG = "SwitchBrowserUriHelper"
 
+    internal val STATE_VALIDATION_REQUIRED: Boolean by lazy {
+        CommonFlightsManager
+            .getFlightsProvider()
+            .isFlightEnabled(CommonFlight.SWITCH_BROWSER_PROTOCOL_REQUIRES_STATE)
+    }
+
+
     /**
      * Build the process uri for the switch browser challenge.
      *
@@ -69,51 +79,32 @@ object SwitchBrowserUriHelper {
             Logger.error(methodTag, errorMessage, exception)
             throw exception
         }
+        val state = uri.getQueryParameter(
+            SWITCH_BROWSER.STATE
+        )
         // Query parameters for the process uri.
         val queryParams = hashMapOf<String, String>()
         queryParams[SWITCH_BROWSER.CODE] = code
+        addStateToQueryParams(queryParams, state, methodTag)
         // Construct the uri to the process endpoint.
         return buildSwitchBrowserUri(actionUri, queryParams)
     }
 
-    /**
-     * Build a generic switch browser uri.
-     *
-     * @param actionUri The action uri to be opened.
-     * @param queryParams The query parameters to be included in the switch browser uri.
-     *
-     * @return The switch browser uri constructed from the action uri and query parameters.
-     */
-    @Throws(IllegalArgumentException::class, NullPointerException::class, UnsupportedOperationException::class)
-    private fun buildSwitchBrowserUri(
-        actionUri: String,
-        queryParams: HashMap<String, String> = hashMapOf()
-    ): Uri {
-        val paths = actionUri.split("/")
-        val authority = paths[0]
-        val uriBuilder = Uri.Builder()
-            .scheme("https")
-            .encodedAuthority(authority)
-        for (i in 1 until paths.size) {
-            uriBuilder.appendPath(paths[i])
-        }
-        for ((key, value) in queryParams.entries) {
-            uriBuilder.appendQueryParameter(key, value)
-        }
-        return uriBuilder.build()
-    }
-
     /**
      * Build the resume uri for the switch browser challenge.
      *
      * @param actionUri The action uri to be opened.
+     * @param state The state to be included in the switch browser uri.
      *
      * @return The resume uri constructed from the bundle.
      * e.g. actionUri
      */
-    fun buildResumeUri(actionUri: String): Uri {
+    fun buildResumeUri(actionUri: String, state: String?): Uri {
+        val methodTag = "$TAG:buildResumeUri"
         // Construct the uri to the resume endpoint.
-        return buildSwitchBrowserUri(actionUri)
+        val queryParams = hashMapOf<String, String>()
+        addStateToQueryParams(queryParams, state, methodTag)
+        return buildSwitchBrowserUri(actionUri, queryParams)
     }
 
     /**
@@ -134,4 +125,95 @@ object SwitchBrowserUriHelper {
         val expectedUrl = "$redirectUrl/$switchBrowserPath"
         return url.startsWith(expectedUrl, ignoreCase = true)
     }
+
+    /**
+     * Check if state in the auth request matches the state provided.
+     */
+    fun statesMatch(authorizationUrl: String, state: String?) {
+        val methodTag = "$TAG:statesMatch"
+        if (!STATE_VALIDATION_REQUIRED) {
+            Logger.info(methodTag, "State validation is not required.")
+            return
+        }
+        val span = SpanExtension.current()
+        // Validate the state from auth request and redirect URL is the same
+        if (state.isNullOrEmpty()) {
+            val clientException = ClientException(
+                ClientException.STATE_MISMATCH,
+                "State is null."
+            )
+            span.setStatus(StatusCode.ERROR)
+            span.recordException(clientException)
+            span.end()
+            throw clientException
+        }
+        val authRequestState = Uri.parse(authorizationUrl).getQueryParameter(SWITCH_BROWSER.STATE)
+        if (authRequestState.isNullOrEmpty()) {
+            val clientException = ClientException(
+                ClientException.STATE_MISMATCH,
+                "Authorization request state is null."
+            )
+            span.setStatus(StatusCode.ERROR)
+            span.recordException(clientException)
+            span.end()
+            throw clientException
+        }
+        if (state != authRequestState) {
+            val clientException = ClientException(
+                ClientException.STATE_MISMATCH,
+                "State does not match with the auth request state."
+            )
+            span.setStatus(StatusCode.ERROR)
+            span.recordException(clientException)
+            span.end()
+            throw clientException
+        }
+        Logger.info(methodTag, "States match.")
+    }
+
+    /**
+     * Add state to the query parameters If STATE_VALIDATION_REQUIRED is enabled.
+     * If STATE_VALIDATION_REQUIRED is disabled, this method does nothing.
+     * If STATE_VALIDATION_REQUIRED is enabled and state is null or empty, this method throws an exception.
+     */
+    private fun addStateToQueryParams(queryParams: HashMap<String, String>, state: String?, methodTag: String) {
+        if (STATE_VALIDATION_REQUIRED) {
+            if (state.isNullOrEmpty()) {
+                // This should never happen, but if it does, we should log it and throw.
+                val errorMessage = "State is null or empty"
+                val exception = ClientException(ClientException.MISSING_PARAMETER, errorMessage)
+                Logger.error(methodTag, errorMessage, exception)
+                throw exception
+            } else {
+                queryParams[SWITCH_BROWSER.STATE] = state
+            }
+        }
+    }
+
+    /**
+     * Build a generic switch browser uri.
+     *
+     * @param actionUri The action uri to be opened.
+     * @param queryParams The query parameters to be included in the switch browser uri.
+     *
+     * @return The switch browser uri constructed from the action uri and query parameters.
+     */
+    @Throws(IllegalArgumentException::class, NullPointerException::class, UnsupportedOperationException::class)
+    private fun buildSwitchBrowserUri(
+        actionUri: String,
+        queryParams: HashMap<String, String> = hashMapOf()
+    ): Uri {
+        val paths = actionUri.split("/")
+        val authority = paths[0]
+        val uriBuilder = Uri.Builder()
+            .scheme("https")
+            .encodedAuthority(authority)
+        for (i in 1 until paths.size) {
+            uriBuilder.appendPath(paths[i])
+        }
+        for ((key, value) in queryParams.entries) {
+            uriBuilder.appendQueryParameter(key, value)
+        }
+        return uriBuilder.build()
+    }
 }