From c4d3c81ff25dccdc729f585f583eda2598e361db Mon Sep 17 00:00:00 2001 From: fadidurah Date: Wed, 2 Apr 2025 01:00:20 -0400 Subject: [PATCH 01/15] add javascript api --- .../NumberMatchJavaScriptInterface.kt | 73 +++++++++++++++++++ .../oauth2/WebViewAuthorizationFragment.java | 13 +++- 2 files changed, 84 insertions(+), 2 deletions(-) create mode 100644 common/src/main/java/com/microsoft/identity/common/internal/numberMatch/NumberMatchJavaScriptInterface.kt diff --git a/common/src/main/java/com/microsoft/identity/common/internal/numberMatch/NumberMatchJavaScriptInterface.kt b/common/src/main/java/com/microsoft/identity/common/internal/numberMatch/NumberMatchJavaScriptInterface.kt new file mode 100644 index 0000000000..81256e46c5 --- /dev/null +++ b/common/src/main/java/com/microsoft/identity/common/internal/numberMatch/NumberMatchJavaScriptInterface.kt @@ -0,0 +1,73 @@ +// 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.numberMatch + +import android.webkit.JavascriptInterface +import com.microsoft.identity.common.logging.Logger +import com.microsoft.identity.common.nativeauth.internal.commands.GetAuthMethodsCommand + +/** + * Java Script API to be exposed as part of brokered WebView requests. + * Will be used by AuthUX as part of number match feature. + * When authenticator is installed, and phone uses MFA or PSI in an interactive flow, a number + * matching challenge is issued, where used is given a number and asked to open authenticator and check + * for the same number in authenticator UI. This feature cuts out one UI step, where this API is used to + * supply the number match value and store it in ephemeral storage (kept as long as current broker + * process is alive), where Authenticator can call a broker API to fetch the number match, and immediately + * prompt user for consent, rather than first asking them to check the number match. + */ +class NumberMatchJavaScriptInterface { + + // Store number matches in a static hash map + // No need to persist this storage beyond the current broker process, but we need to keep them + // long enough for AuthApp to call the broker api to fetch the number match + // TODO: Is this a security concern? + companion object { + val TAG = NumberMatchJavaScriptInterface::class.java.simpleName + val numberMatchMap: HashMap = HashMap() + } + + /** + * Method to add a key:value pair of sessionID:numberMatch to static hashmap. This hashmap will be accessed + * by broker api to get the number match for a particular sessionID. + * + * TODO: Check, I feel params should be nullable, and should fallback to null check in AuthApp code. + * Not sure if this API will only called with non-null values + */ + @JavascriptInterface + fun postCodeMatch(sessionID : String?, numberMatch : String?) { + val methodTag = "$TAG:postCodeMatch" + Logger.info( + methodTag, + "Adding entry in NumberMatch hashmap for session ID: $sessionID" + ) + numberMatchMap[sessionID] = numberMatch + } + + /** + * Clear existing number match key:value pairs + */ + fun clearNumberMatchMap() { + numberMatchMap.clear() + } +} \ No newline at end of file diff --git a/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java b/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java index ab0495e2db..2272a0f42c 100644 --- a/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java +++ b/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java @@ -52,6 +52,7 @@ import com.microsoft.identity.common.R; import com.microsoft.identity.common.internal.fido.LegacyFidoActivityResultContract; import com.microsoft.identity.common.internal.fido.LegacyFido2ApiObject; +import com.microsoft.identity.common.internal.numberMatch.NumberMatchJavaScriptInterface; import com.microsoft.identity.common.internal.ui.webview.ISendResultCallback; import com.microsoft.identity.common.internal.ui.webview.switchbrowser.SwitchBrowserProtocolCoordinator; import com.microsoft.identity.common.java.WarningType; @@ -126,6 +127,8 @@ public class WebViewAuthorizationFragment extends AuthorizationFragment { // This is used by the switch browser protocol to handle the resume of the flow. private SwitchBrowserProtocolCoordinator mSwitchBrowserProtocolCoordinator = null; + private boolean isBrokerRequest = false; + @Override public void onCreate(@Nullable Bundle savedInstanceState) { super.onCreate(savedInstanceState); @@ -214,6 +217,9 @@ void extractState(@NonNull final Bundle state) { mAuthIntent = state.getParcelable(AUTH_INTENT); mPkeyAuthStatus = state.getBoolean(PKEYAUTH_STATUS, false); mAuthorizationRequestUrl = state.getString(REQUEST_URL); + if (mAuthorizationRequestUrl != null) { + isBrokerRequest = checkBrokerRequest(mAuthorizationRequestUrl); + } mRedirectUri = state.getString(REDIRECT_URI); mRequestHeaders = getRequestHeaders(state); mPostPageLoadedJavascript = state.getString(POST_PAGE_LOADED_URL); @@ -294,6 +300,9 @@ private void setUpWebView(@NonNull final View view, mWebView.getSettings().setUserAgentString( userAgent + AuthenticationConstants.Broker.CLIENT_TLS_NOT_SUPPORTED); mWebView.getSettings().setJavaScriptEnabled(true); + if (isBrokerRequest) { + mWebView.addJavascriptInterface(new NumberMatchJavaScriptInterface(), "Android"); + } mWebView.requestFocus(View.FOCUS_DOWN); // Set focus to the view for touch event @@ -480,7 +489,7 @@ private HashMap getRequestHeaders(final Bundle state) { } // Attach client extras header for ESTS telemetry. Only done for broker requests - if (isBrokerRequest(this.mAuthorizationRequestUrl)) { + if (isBrokerRequest) { final ClientExtraSku clientExtraSku = ClientExtraSku.builder() .srcSku(state.getString(PRODUCT)) .srcSkuVer(state.getString(VERSION)) @@ -502,7 +511,7 @@ public ActivityResultLauncher getFidoLauncher() { * Helper method to check if the authorization request is being made through broker. * Done by checking for broker version key in the url */ - private boolean isBrokerRequest(final String authorizationUrl) { + private boolean checkBrokerRequest(final String authorizationUrl) { return authorizationUrl.contains(Device.PlatformIdParameters.BROKER_VERSION); } From 1ffa13968e862161bf86603b791bbe3b019bff53 Mon Sep 17 00:00:00 2001 From: fadidurah Date: Wed, 2 Apr 2025 02:18:56 -0400 Subject: [PATCH 02/15] changelog --- changelog.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/changelog.txt b/changelog.txt index 7a894885d1..1ba5e85f41 100644 --- a/changelog.txt +++ b/changelog.txt @@ -1,5 +1,6 @@ vNext ---------- +- [MINOR] Expose a JavaScript API in brokered Webviews to facilitate Improved Same Device NumberMatch (#2617) - [PATCH] Improve logs and error reporting SIWG flow (#2616) - [PATCH] Only show QR +PIN rationale when the request origin is from Microsoft (#2613) - [MINOR] Add an extra index for MSAL CPP core in the extra client sku header (#2610) From ccfe0d2acda19fbd9f5459a9398f366fa0825534 Mon Sep 17 00:00:00 2001 From: fadidurah Date: Thu, 10 Apr 2025 02:10:16 -0400 Subject: [PATCH 03/15] comments --- .../NumberMatchJavaScriptInterface.kt | 23 ++++++++++--------- .../oauth2/WebViewAuthorizationFragment.java | 5 ++-- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/common/src/main/java/com/microsoft/identity/common/internal/numberMatch/NumberMatchJavaScriptInterface.kt b/common/src/main/java/com/microsoft/identity/common/internal/numberMatch/NumberMatchJavaScriptInterface.kt index 81256e46c5..a525b03bc8 100644 --- a/common/src/main/java/com/microsoft/identity/common/internal/numberMatch/NumberMatchJavaScriptInterface.kt +++ b/common/src/main/java/com/microsoft/identity/common/internal/numberMatch/NumberMatchJavaScriptInterface.kt @@ -41,7 +41,6 @@ class NumberMatchJavaScriptInterface { // Store number matches in a static hash map // No need to persist this storage beyond the current broker process, but we need to keep them // long enough for AuthApp to call the broker api to fetch the number match - // TODO: Is this a security concern? companion object { val TAG = NumberMatchJavaScriptInterface::class.java.simpleName val numberMatchMap: HashMap = HashMap() @@ -50,18 +49,20 @@ class NumberMatchJavaScriptInterface { /** * Method to add a key:value pair of sessionID:numberMatch to static hashmap. This hashmap will be accessed * by broker api to get the number match for a particular sessionID. - * - * TODO: Check, I feel params should be nullable, and should fallback to null check in AuthApp code. - * Not sure if this API will only called with non-null values */ @JavascriptInterface fun postCodeMatch(sessionID : String?, numberMatch : String?) { - val methodTag = "$TAG:postCodeMatch" - Logger.info( - methodTag, - "Adding entry in NumberMatch hashmap for session ID: $sessionID" - ) - numberMatchMap[sessionID] = numberMatch + // If both parameters are non-null, add a new entry to the hashmap + if (sessionID != null && numberMatch != null) { + val methodTag = "$TAG:postCodeMatch" + Logger.info( + methodTag, + "Adding entry in NumberMatch hashmap for session ID: $sessionID" + ) + numberMatchMap[sessionID] = numberMatch + } + + // If either parameter is null, do nothing } /** @@ -70,4 +71,4 @@ class NumberMatchJavaScriptInterface { fun clearNumberMatchMap() { numberMatchMap.clear() } -} \ No newline at end of file +} diff --git a/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java b/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java index 2272a0f42c..9290aabcfd 100644 --- a/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java +++ b/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java @@ -54,6 +54,7 @@ import com.microsoft.identity.common.internal.fido.LegacyFido2ApiObject; import com.microsoft.identity.common.internal.numberMatch.NumberMatchJavaScriptInterface; import com.microsoft.identity.common.internal.ui.webview.ISendResultCallback; +import com.microsoft.identity.common.internal.ui.webview.ProcessUtil; import com.microsoft.identity.common.internal.ui.webview.switchbrowser.SwitchBrowserProtocolCoordinator; import com.microsoft.identity.common.java.WarningType; import com.microsoft.identity.common.adal.internal.AuthenticationConstants; @@ -217,8 +218,8 @@ void extractState(@NonNull final Bundle state) { mAuthIntent = state.getParcelable(AUTH_INTENT); mPkeyAuthStatus = state.getBoolean(PKEYAUTH_STATUS, false); mAuthorizationRequestUrl = state.getString(REQUEST_URL); - if (mAuthorizationRequestUrl != null) { - isBrokerRequest = checkBrokerRequest(mAuthorizationRequestUrl); + if (getContext() != null) { + isBrokerRequest = ProcessUtil.isRunningOnAuthService(getContext()); } mRedirectUri = state.getString(REDIRECT_URI); mRequestHeaders = getRequestHeaders(state); From 2945a5f99b423df0724bb49cd0efa75335a8f311 Mon Sep 17 00:00:00 2001 From: fadidurah Date: Tue, 15 Apr 2025 18:45:43 -0400 Subject: [PATCH 04/15] adjust api --- .../broker/AuthUxJavaScriptInterface.kt | 71 +++++++++++++++++++ ...criptInterface.kt => NumberMatchHelper.kt} | 59 ++++++++------- .../oauth2/WebViewAuthorizationFragment.java | 13 +++- 3 files changed, 111 insertions(+), 32 deletions(-) create mode 100644 common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterface.kt rename common/src/main/java/com/microsoft/identity/common/internal/numberMatch/{NumberMatchJavaScriptInterface.kt => NumberMatchHelper.kt} (62%) diff --git a/common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterface.kt b/common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterface.kt new file mode 100644 index 0000000000..e57a4449f6 --- /dev/null +++ b/common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterface.kt @@ -0,0 +1,71 @@ +// 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 + +import android.webkit.JavascriptInterface +import com.microsoft.identity.common.internal.numberMatch.NumberMatchHelper +import com.microsoft.identity.common.java.util.JsonUtil +import com.microsoft.identity.common.logging.Logger + +/** + * JavaScript API to receive JSON string payloads from AuthUX in order to facilitate calling various + * broker methods. + */ +class AuthUxJavaScriptInterface { + + // Store number matches in a static hash map + // No need to persist this storage beyond the current broker process, but we need to keep them + // long enough for AuthApp to call the broker api to fetch the number match + companion object { + val TAG = AuthUxJavaScriptInterface::class.java.simpleName + private const val JAVASCRIPT_INTERFACE_NAME = "BrokerJS" + + fun getInterfaceName() : String { + return JAVASCRIPT_INTERFACE_NAME + } + } + + @JavascriptInterface + fun postToBroker(jsonPayload: String) { + val methodTag = "$TAG:postToBroker" + Logger.info(methodTag, "Received a payload from AuthUX through JavaScript API.") + val jsonMap : Map = JsonUtil.extractJsonObjectIntoMap(jsonPayload) + val function = jsonMap["function"] + val dataMap : Map = JsonUtil.extractJsonObjectIntoMap(jsonMap["data"]) + Logger.info(methodTag, "Function name: [$function]") + + when (function) { + FunctionNames.NUMBER_MATCH.name -> + NumberMatchHelper.storeNumberMatch(dataMap[NumberMatchHelper.SESSION_ID_ATTRIBUTE_NAME], dataMap[NumberMatchHelper.NUMBER_MATCH_ATTRIBUTE_NAME]) + else -> + Logger.warn(methodTag, "Payload from AuthUX contained an unknown function name.") + } + } + + /** + * Enum class to hold function names + */ + enum class FunctionNames { + NUMBER_MATCH + } +} diff --git a/common/src/main/java/com/microsoft/identity/common/internal/numberMatch/NumberMatchJavaScriptInterface.kt b/common/src/main/java/com/microsoft/identity/common/internal/numberMatch/NumberMatchHelper.kt similarity index 62% rename from common/src/main/java/com/microsoft/identity/common/internal/numberMatch/NumberMatchJavaScriptInterface.kt rename to common/src/main/java/com/microsoft/identity/common/internal/numberMatch/NumberMatchHelper.kt index a525b03bc8..457de46737 100644 --- a/common/src/main/java/com/microsoft/identity/common/internal/numberMatch/NumberMatchJavaScriptInterface.kt +++ b/common/src/main/java/com/microsoft/identity/common/internal/numberMatch/NumberMatchHelper.kt @@ -22,13 +22,11 @@ // THE SOFTWARE. package com.microsoft.identity.common.internal.numberMatch -import android.webkit.JavascriptInterface import com.microsoft.identity.common.logging.Logger -import com.microsoft.identity.common.nativeauth.internal.commands.GetAuthMethodsCommand + /** - * Java Script API to be exposed as part of brokered WebView requests. - * Will be used by AuthUX as part of number match feature. + * Helper to facilitate NumberMatchFlow. Used in conjunction with {@link AuthUxJavaScriptInterface} * When authenticator is installed, and phone uses MFA or PSI in an interactive flow, a number * matching challenge is issued, where used is given a number and asked to open authenticator and check * for the same number in authenticator UI. This feature cuts out one UI step, where this API is used to @@ -36,39 +34,40 @@ import com.microsoft.identity.common.nativeauth.internal.commands.GetAuthMethods * process is alive), where Authenticator can call a broker API to fetch the number match, and immediately * prompt user for consent, rather than first asking them to check the number match. */ -class NumberMatchJavaScriptInterface { +class NumberMatchHelper { // Store number matches in a static hash map // No need to persist this storage beyond the current broker process, but we need to keep them // long enough for AuthApp to call the broker api to fetch the number match companion object { - val TAG = NumberMatchJavaScriptInterface::class.java.simpleName - val numberMatchMap: HashMap = HashMap() - } + val TAG = NumberMatchHelper::class.java.simpleName + val numberMatchMap: HashMap = HashMap() + const val SESSION_ID_ATTRIBUTE_NAME = "sessionID" + const val NUMBER_MATCH_ATTRIBUTE_NAME = "numberMatch" - /** - * Method to add a key:value pair of sessionID:numberMatch to static hashmap. This hashmap will be accessed - * by broker api to get the number match for a particular sessionID. - */ - @JavascriptInterface - fun postCodeMatch(sessionID : String?, numberMatch : String?) { - // If both parameters are non-null, add a new entry to the hashmap - if (sessionID != null && numberMatch != null) { - val methodTag = "$TAG:postCodeMatch" - Logger.info( - methodTag, - "Adding entry in NumberMatch hashmap for session ID: $sessionID" - ) - numberMatchMap[sessionID] = numberMatch - } + /** + * Method to add a key:value pair of sessionID:numberMatch to static hashmap. This hashmap will be accessed + * by broker api to get the number match for a particular sessionID. + */ + fun storeNumberMatch(sessionID:String?, numberMatch:String?) { + // If both parameters are non-null, add a new entry to the hashmap + if (sessionID != null && numberMatch != null) { + val methodTag = "$TAG:storeNumberMatch" + Logger.info( + methodTag, + "Adding entry in NumberMatch hashmap for session ID: $sessionID" + ) + numberMatchMap[sessionID] = numberMatch + } - // If either parameter is null, do nothing - } + // If either parameter is null, do nothing + } - /** - * Clear existing number match key:value pairs - */ - fun clearNumberMatchMap() { - numberMatchMap.clear() + /** + * Clear existing number match key:value pairs + */ + fun clearNumberMatchMap() { + numberMatchMap.clear() + } } } diff --git a/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java b/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java index 9290aabcfd..6e0a5c097b 100644 --- a/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java +++ b/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java @@ -36,6 +36,7 @@ import android.view.View; import android.view.ViewGroup; import android.webkit.PermissionRequest; +import android.webkit.ValueCallback; import android.webkit.WebChromeClient; import android.webkit.WebSettings; import android.webkit.WebView; @@ -52,7 +53,7 @@ import com.microsoft.identity.common.R; import com.microsoft.identity.common.internal.fido.LegacyFidoActivityResultContract; import com.microsoft.identity.common.internal.fido.LegacyFido2ApiObject; -import com.microsoft.identity.common.internal.numberMatch.NumberMatchJavaScriptInterface; +import com.microsoft.identity.common.internal.broker.AuthUxJavaScriptInterface; import com.microsoft.identity.common.internal.ui.webview.ISendResultCallback; import com.microsoft.identity.common.internal.ui.webview.ProcessUtil; import com.microsoft.identity.common.internal.ui.webview.switchbrowser.SwitchBrowserProtocolCoordinator; @@ -302,7 +303,7 @@ private void setUpWebView(@NonNull final View view, userAgent + AuthenticationConstants.Broker.CLIENT_TLS_NOT_SUPPORTED); mWebView.getSettings().setJavaScriptEnabled(true); if (isBrokerRequest) { - mWebView.addJavascriptInterface(new NumberMatchJavaScriptInterface(), "Android"); + mWebView.addJavascriptInterface(new AuthUxJavaScriptInterface(), AuthUxJavaScriptInterface.Companion.getInterfaceName()); } mWebView.requestFocus(View.FOCUS_DOWN); @@ -448,6 +449,14 @@ public void run() { // Therefore, we'll show a spinner here, and hides it when mAuthorizationRequestUrl is successfully loaded. // After that, progress bar will be displayed by MSA/AAD. mProgressBar.setVisibility(View.VISIBLE); + +// TODO: REMOVE THIS BEFORE MERGING, Using this to test JS API + mWebView.evaluateJavascript(AuthUxJavaScriptInterface.Companion.getInterfaceName() +".postToBroker('{function: NUMBER_MATCH,data: {sessionID: id, numberMatch: number}}')", new ValueCallback() { + @Override + public void onReceiveValue(String value) { + int x = 0; + } + }); } }); } From 2bd13c03e8f66500dc8b1fb186502b1dbb95d343 Mon Sep 17 00:00:00 2001 From: fadidurah Date: Tue, 15 Apr 2025 19:00:50 -0400 Subject: [PATCH 05/15] move some parameters around --- .../broker/AuthUxJavaScriptInterface.kt | 4 +-- .../internal/numberMatch/NumberMatchHelper.kt | 35 +++++++++++++------ 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterface.kt b/common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterface.kt index e57a4449f6..eca7333431 100644 --- a/common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterface.kt +++ b/common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterface.kt @@ -51,12 +51,12 @@ class AuthUxJavaScriptInterface { Logger.info(methodTag, "Received a payload from AuthUX through JavaScript API.") val jsonMap : Map = JsonUtil.extractJsonObjectIntoMap(jsonPayload) val function = jsonMap["function"] - val dataMap : Map = JsonUtil.extractJsonObjectIntoMap(jsonMap["data"]) + val dataString = jsonMap["data"] Logger.info(methodTag, "Function name: [$function]") when (function) { FunctionNames.NUMBER_MATCH.name -> - NumberMatchHelper.storeNumberMatch(dataMap[NumberMatchHelper.SESSION_ID_ATTRIBUTE_NAME], dataMap[NumberMatchHelper.NUMBER_MATCH_ATTRIBUTE_NAME]) + NumberMatchHelper.storeNumberMatch(dataString) else -> Logger.warn(methodTag, "Payload from AuthUX contained an unknown function name.") } diff --git a/common/src/main/java/com/microsoft/identity/common/internal/numberMatch/NumberMatchHelper.kt b/common/src/main/java/com/microsoft/identity/common/internal/numberMatch/NumberMatchHelper.kt index 457de46737..ecb2b13864 100644 --- a/common/src/main/java/com/microsoft/identity/common/internal/numberMatch/NumberMatchHelper.kt +++ b/common/src/main/java/com/microsoft/identity/common/internal/numberMatch/NumberMatchHelper.kt @@ -22,7 +22,9 @@ // THE SOFTWARE. package com.microsoft.identity.common.internal.numberMatch +import com.microsoft.identity.common.java.util.JsonUtil import com.microsoft.identity.common.logging.Logger +import org.json.JSONException /** @@ -49,18 +51,31 @@ class NumberMatchHelper { * Method to add a key:value pair of sessionID:numberMatch to static hashmap. This hashmap will be accessed * by broker api to get the number match for a particular sessionID. */ - fun storeNumberMatch(sessionID:String?, numberMatch:String?) { - // If both parameters are non-null, add a new entry to the hashmap - if (sessionID != null && numberMatch != null) { - val methodTag = "$TAG:storeNumberMatch" - Logger.info( - methodTag, - "Adding entry in NumberMatch hashmap for session ID: $sessionID" - ) - numberMatchMap[sessionID] = numberMatch + fun storeNumberMatch(data: String?) { + val methodTag = "$TAG:storeNumberMatch" + if (data == null) { + // If we didn't receive a data string, don't store anything + Logger.warn(methodTag, "data String received was null") + return } - // If either parameter is null, do nothing + try { + val dataMap = JsonUtil.extractJsonObjectIntoMap(data) + val sessionId = dataMap[SESSION_ID_ATTRIBUTE_NAME] + val numberMatch = dataMap[NUMBER_MATCH_ATTRIBUTE_NAME] + // If both parameters are non-null, add a new entry to the hashmap + if (sessionId != null && numberMatch != null) { + Logger.info( + methodTag, + "Adding entry in NumberMatch hashmap for session ID: $sessionId" + ) + numberMatchMap[sessionId] = numberMatch + } + + // If either parameter is null, do nothing + } catch (e: JSONException) { + Logger.warn(methodTag, "data String was malform during JSON extraction") + } } /** From f478f412802c30ef8ca3a5613bebbfa92510671a Mon Sep 17 00:00:00 2001 From: fadidurah Date: Tue, 15 Apr 2025 19:45:35 -0400 Subject: [PATCH 06/15] comment out test code --- .../oauth2/WebViewAuthorizationFragment.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java b/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java index 6e0a5c097b..653199e7a4 100644 --- a/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java +++ b/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java @@ -451,12 +451,12 @@ public void run() { mProgressBar.setVisibility(View.VISIBLE); // TODO: REMOVE THIS BEFORE MERGING, Using this to test JS API - mWebView.evaluateJavascript(AuthUxJavaScriptInterface.Companion.getInterfaceName() +".postToBroker('{function: NUMBER_MATCH,data: {sessionID: id, numberMatch: number}}')", new ValueCallback() { - @Override - public void onReceiveValue(String value) { - int x = 0; - } - }); +// mWebView.evaluateJavascript(AuthUxJavaScriptInterface.Companion.getInterfaceName() +".postToBroker('{function: NUMBER_MATCH,data: {sessionID: id, numberMatch: number}}')", new ValueCallback() { +// @Override +// public void onReceiveValue(String value) { +// int x = 0; +// } +// }); } }); } From caf538c55ed0f278e3a2d9e9c8743f251801d2ea Mon Sep 17 00:00:00 2001 From: fadidurah Date: Tue, 15 Apr 2025 19:46:15 -0400 Subject: [PATCH 07/15] remove helper method --- .../providers/oauth2/WebViewAuthorizationFragment.java | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java b/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java index 653199e7a4..aff46a1f33 100644 --- a/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java +++ b/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java @@ -516,15 +516,7 @@ private HashMap getRequestHeaders(final Bundle state) { public ActivityResultLauncher getFidoLauncher() { return mFidoLauncher; } - - /** - * Helper method to check if the authorization request is being made through broker. - * Done by checking for broker version key in the url - */ - private boolean checkBrokerRequest(final String authorizationUrl) { - return authorizationUrl.contains(Device.PlatformIdParameters.BROKER_VERSION); - } - + class AuthorizationCompletionCallback implements IAuthorizationCompletionCallback { @Override public void onChallengeResponseReceived(@NonNull final RawAuthorizationResult response) { From ee7a7c7e3b22d9baef58cb8904db98e8de54f1aa Mon Sep 17 00:00:00 2001 From: fadidurah Date: Thu, 15 May 2025 12:54:59 -0400 Subject: [PATCH 08/15] dev --- changelog.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/changelog.txt b/changelog.txt index 668c4fdfdb..2dfea8dd47 100644 --- a/changelog.txt +++ b/changelog.txt @@ -1,7 +1,6 @@ vNext ---------- - [MINOR] Expose a JavaScript API in brokered Webviews to facilitate Improved Same Device NumberMatch (#2617) -- [MINOR] Enable the new Broker discovery on MSAL by default (#2618) Version 21.1.0 ---------- From 8ae18e1e01794fd182653fae731e32ed71adc93f Mon Sep 17 00:00:00 2001 From: fadidurah Date: Thu, 15 May 2025 18:25:23 -0400 Subject: [PATCH 09/15] update to new JSON Schema --- .../broker/AuthUxJavaScriptInterface.kt | 51 ++++++++--- .../internal/numberMatch/NumberMatchHelper.kt | 34 +++----- .../broker/AuthUxJavaScriptInterfaceTest.kt | 67 +++++++++++++++ .../numberMatch/NumberMatchHelperTest.kt | 84 +++++++++++++++++++ 4 files changed, 202 insertions(+), 34 deletions(-) create mode 100644 common/src/test/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterfaceTest.kt create mode 100644 common/src/test/java/com/microsoft/identity/common/internal/numberMatch/NumberMatchHelperTest.kt diff --git a/common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterface.kt b/common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterface.kt index eca7333431..f4bb73040d 100644 --- a/common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterface.kt +++ b/common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterface.kt @@ -23,6 +23,7 @@ package com.microsoft.identity.common.internal.broker import android.webkit.JavascriptInterface +import com.google.gson.stream.MalformedJsonException import com.microsoft.identity.common.internal.numberMatch.NumberMatchHelper import com.microsoft.identity.common.java.util.JsonUtil import com.microsoft.identity.common.logging.Logger @@ -38,7 +39,7 @@ class AuthUxJavaScriptInterface { // long enough for AuthApp to call the broker api to fetch the number match companion object { val TAG = AuthUxJavaScriptInterface::class.java.simpleName - private const val JAVASCRIPT_INTERFACE_NAME = "BrokerJS" + private const val JAVASCRIPT_INTERFACE_NAME = "ClientBrokerJS" fun getInterfaceName() : String { return JAVASCRIPT_INTERFACE_NAME @@ -46,19 +47,45 @@ class AuthUxJavaScriptInterface { } @JavascriptInterface - fun postToBroker(jsonPayload: String) { - val methodTag = "$TAG:postToBroker" + fun postMessageToBroker(jsonPayload: String) { + val methodTag = "$TAG:postMessageToBroker" Logger.info(methodTag, "Received a payload from AuthUX through JavaScript API.") - val jsonMap : Map = JsonUtil.extractJsonObjectIntoMap(jsonPayload) - val function = jsonMap["function"] - val dataString = jsonMap["data"] - Logger.info(methodTag, "Function name: [$function]") - when (function) { - FunctionNames.NUMBER_MATCH.name -> - NumberMatchHelper.storeNumberMatch(dataString) - else -> - Logger.warn(methodTag, "Payload from AuthUX contained an unknown function name.") + try { + val parsedJson = JsonUtil.extractJsonObjectIntoMap(jsonPayload) + + val correlationID = parsedJson["correlationID"] + Logger.info(methodTag, "Correlation ID during JavaScript Call: [$correlationID]") + + // TODO: Leaving these here, as these will be relevant for next WebCP feature + // val actionName = parsedJson["action_name"] + // val actionComponent = parsedJson["action_component"] + + val parameters = JsonUtil.extractJsonObjectIntoMap(parsedJson["params"]) + val function = parameters["function"] + val data = JsonUtil.extractJsonObjectIntoMap(parameters["data"]) + Logger.info(methodTag, "Function name: [$function]") + + when (function) { + FunctionNames.NUMBER_MATCH.name -> + NumberMatchHelper.storeNumberMatch( + data[NumberMatchHelper.SESSION_ID_ATTRIBUTE_NAME], + data[NumberMatchHelper.NUMBER_MATCH_ATTRIBUTE_NAME]) + else -> + Logger.warn(methodTag, "Payload from AuthUX contained an unknown function name.") + } + } catch (e: Exception) { // If we run into exceptions, we don't want to kill the broker + when (e) { + is NullPointerException -> { + Logger.warn(methodTag, "Payload with missing mandatory fields sent through JavaScriptInterface") + } + is MalformedJsonException -> { + Logger.warn(methodTag, "Malformed JSON payload sent through JavaScriptInterface") + } + else -> { + Logger.warn(methodTag, "Unknown error occurred while processing the payload.") + } + } } } diff --git a/common/src/main/java/com/microsoft/identity/common/internal/numberMatch/NumberMatchHelper.kt b/common/src/main/java/com/microsoft/identity/common/internal/numberMatch/NumberMatchHelper.kt index ecb2b13864..c366514ce0 100644 --- a/common/src/main/java/com/microsoft/identity/common/internal/numberMatch/NumberMatchHelper.kt +++ b/common/src/main/java/com/microsoft/identity/common/internal/numberMatch/NumberMatchHelper.kt @@ -51,30 +51,20 @@ class NumberMatchHelper { * Method to add a key:value pair of sessionID:numberMatch to static hashmap. This hashmap will be accessed * by broker api to get the number match for a particular sessionID. */ - fun storeNumberMatch(data: String?) { + fun storeNumberMatch(sessionId: String?, numberMatch: String?) { val methodTag = "$TAG:storeNumberMatch" - if (data == null) { - // If we didn't receive a data string, don't store anything - Logger.warn(methodTag, "data String received was null") - return - } - - try { - val dataMap = JsonUtil.extractJsonObjectIntoMap(data) - val sessionId = dataMap[SESSION_ID_ATTRIBUTE_NAME] - val numberMatch = dataMap[NUMBER_MATCH_ATTRIBUTE_NAME] - // If both parameters are non-null, add a new entry to the hashmap - if (sessionId != null && numberMatch != null) { - Logger.info( - methodTag, - "Adding entry in NumberMatch hashmap for session ID: $sessionId" - ) - numberMatchMap[sessionId] = numberMatch - } + Logger.info(methodTag, + "Adding entry in NumberMatch hashmap for session ID: $sessionId") - // If either parameter is null, do nothing - } catch (e: JSONException) { - Logger.warn(methodTag, "data String was malform during JSON extraction") + // If both parameters are non-null, add a new entry to the hashmap + if (sessionId != null && numberMatch != null) { + numberMatchMap[sessionId] = numberMatch + } + // If either parameter is null, do nothing + else { + Logger.warn(methodTag, + "Either session ID or number match is null. Nothing to add for number match." + ) } } diff --git a/common/src/test/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterfaceTest.kt b/common/src/test/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterfaceTest.kt new file mode 100644 index 0000000000..dded8e4fd9 --- /dev/null +++ b/common/src/test/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterfaceTest.kt @@ -0,0 +1,67 @@ +// 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 + +import com.microsoft.identity.common.internal.numberMatch.NumberMatchHelper +import org.junit.Before +import org.junit.Test + +class AuthUxJavaScriptInterfaceTest { + + private lateinit var authUxJavaScriptInterface: AuthUxJavaScriptInterface + + private val mockSessionId = "1234" + private val mockNumberMatchValue = "00" + + private val numberMatchTestPayload = """ + { + "correlationID": "SOME_CORRELATION_ID" , + "action_name":"write_data", + "action_component":"broker", + "params": + { + "function": "NUMBER_MATCH", + "data": + { + "sessionID": "$mockSessionId", + "numberMatch": "$mockNumberMatchValue" + } + } + } + """.trimIndent() + + @Before + fun setUp() { + authUxJavaScriptInterface = AuthUxJavaScriptInterface() + } + + @Test + fun `test postMessageToBroker with NUMBER_MATCH function`() { + // Call the method + authUxJavaScriptInterface.postMessageToBroker(numberMatchTestPayload) + + // Verify that the data was stored in NumberMatchHelper + val storedValue = NumberMatchHelper.numberMatchMap[mockSessionId] + assert(storedValue == mockNumberMatchValue) + } +} diff --git a/common/src/test/java/com/microsoft/identity/common/internal/numberMatch/NumberMatchHelperTest.kt b/common/src/test/java/com/microsoft/identity/common/internal/numberMatch/NumberMatchHelperTest.kt new file mode 100644 index 0000000000..6c8a264d24 --- /dev/null +++ b/common/src/test/java/com/microsoft/identity/common/internal/numberMatch/NumberMatchHelperTest.kt @@ -0,0 +1,84 @@ +// 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.numberMatch + +import org.junit.Assert.* +import org.junit.Before +import org.junit.Test + +class NumberMatchHelperTest { + + private val mockSessionId = "1234" + private val mockNumberMatchValue = "00" + + @Before + fun setUp() { + // Clear the map before each test to ensure a clean state + NumberMatchHelper.clearNumberMatchMap() + } + + @Test + fun `test storeNumberMatch with valid inputs`() { + // Store a valid session ID and number match + val sessionId = mockSessionId + val numberMatch = mockNumberMatchValue + NumberMatchHelper.storeNumberMatch(sessionId, numberMatch) + + // Verify that the map contains the correct value + assertEquals(mockNumberMatchValue, NumberMatchHelper.numberMatchMap[sessionId]) + } + + @Test + fun `test storeNumberMatch with null sessionId`() { + // Attempt to store a null session ID + val numberMatch = mockNumberMatchValue + NumberMatchHelper.storeNumberMatch(null, numberMatch) + + // Verify that the map is still empty + assertTrue(NumberMatchHelper.numberMatchMap.isEmpty()) + } + + @Test + fun `test storeNumberMatch with null numberMatch`() { + // Attempt to store a null number match + val sessionId = mockSessionId + NumberMatchHelper.storeNumberMatch(sessionId, null) + + // Verify that the map is still empty + assertTrue(NumberMatchHelper.numberMatchMap.isEmpty()) + } + + @Test + fun `test clearNumberMatchMap`() { + // Add an entry to the map + val sessionId = mockSessionId + val numberMatch = mockNumberMatchValue + NumberMatchHelper.storeNumberMatch(sessionId, numberMatch) + + // Clear the map + NumberMatchHelper.clearNumberMatchMap() + + // Verify that the map is empty + assertTrue(NumberMatchHelper.numberMatchMap.isEmpty()) + } +} From bb18c6da871a50c0b2606657799513c05babc4f1 Mon Sep 17 00:00:00 2001 From: fadidurah Date: Thu, 15 May 2025 22:06:39 -0400 Subject: [PATCH 10/15] add check for ests url --- .../broker/AuthUxJavaScriptInterface.kt | 22 +++++++++++++++++++ .../internal/numberMatch/NumberMatchHelper.kt | 3 --- .../oauth2/WebViewAuthorizationFragment.java | 14 +++++------- 3 files changed, 27 insertions(+), 12 deletions(-) diff --git a/common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterface.kt b/common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterface.kt index f4bb73040d..64e7cccd72 100644 --- a/common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterface.kt +++ b/common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterface.kt @@ -46,6 +46,28 @@ class AuthUxJavaScriptInterface { } } + /** + * Method to receive a JSON string payload from AuthUX through JavaScript API. + * Schema for the Json Payload: + * { + * "correlationID": "SOME_CORRELATION_ID" , + * "action_name":"write_data", + * "action_component":"broker", + * "params": + * { + * "function": "NUMBER_MATCH", + * "data": + * { + * "sessionID": "$mockSessionId", + * "numberMatch": "$mockNumberMatchValue" + * } + * } + * } + * TODO: This is currently the schema set for numberMatch, there may be some additions made for + * the more generalized JSON Schema for future Server-side to broker communication through JS. + * + * https://microsoft-my.sharepoint-df.com/:w:/p/veenasoman/EY1AZIeT8X5KrXVz97Vx520B3Jj0fBLSPlklnoRvcmbh0Q?e=VzNFd1&ovuser=72f988bf-86f1-41af-91ab-2d7cd011db47%2Cfadidurah%40microsoft.com&clickparams=eyJBcHBOYW1lIjoiVGVhbXMtRGVza3RvcCIsIkFwcFZlcnNpb24iOiI0OS8yNTA1MDQwMTYwOSIsIkhhc0ZlZGVyYXRlZFVzZXIiOmZhbHNlfQ%3D%3D + */ @JavascriptInterface fun postMessageToBroker(jsonPayload: String) { val methodTag = "$TAG:postMessageToBroker" diff --git a/common/src/main/java/com/microsoft/identity/common/internal/numberMatch/NumberMatchHelper.kt b/common/src/main/java/com/microsoft/identity/common/internal/numberMatch/NumberMatchHelper.kt index c366514ce0..3f2cca28c1 100644 --- a/common/src/main/java/com/microsoft/identity/common/internal/numberMatch/NumberMatchHelper.kt +++ b/common/src/main/java/com/microsoft/identity/common/internal/numberMatch/NumberMatchHelper.kt @@ -22,10 +22,7 @@ // THE SOFTWARE. package com.microsoft.identity.common.internal.numberMatch -import com.microsoft.identity.common.java.util.JsonUtil import com.microsoft.identity.common.logging.Logger -import org.json.JSONException - /** * Helper to facilitate NumberMatchFlow. Used in conjunction with {@link AuthUxJavaScriptInterface} diff --git a/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java b/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java index 205e63be62..1609f4c663 100644 --- a/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java +++ b/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java @@ -126,6 +126,7 @@ public class WebViewAuthorizationFragment extends AuthorizationFragment { private SwitchBrowserProtocolCoordinator mSwitchBrowserProtocolCoordinator = null; private boolean isBrokerRequest = false; + private boolean isEstsRequest = false; @Override public void onCreate(@Nullable Bundle savedInstanceState) { @@ -216,6 +217,9 @@ void extractState(@NonNull final Bundle state) { mAuthIntent = state.getParcelable(AUTH_INTENT); mPkeyAuthStatus = state.getBoolean(PKEYAUTH_STATUS, false); mAuthorizationRequestUrl = state.getString(REQUEST_URL); + if (mAuthorizationRequestUrl != null) { + isEstsRequest = mAuthorizationRequestUrl.startsWith("https://login.microsoftonline.com"); + } if (getContext() != null) { isBrokerRequest = ProcessUtil.isRunningOnAuthService(getContext()); } @@ -298,7 +302,7 @@ private void setUpWebView(@NonNull final View view, mWebView.getSettings().setUserAgentString( userAgent + AuthenticationConstants.Broker.CLIENT_TLS_NOT_SUPPORTED); mWebView.getSettings().setJavaScriptEnabled(true); - if (isBrokerRequest) { + if (isBrokerRequest && isEstsRequest) { mWebView.addJavascriptInterface(new AuthUxJavaScriptInterface(), AuthUxJavaScriptInterface.Companion.getInterfaceName()); } mWebView.requestFocus(View.FOCUS_DOWN); @@ -367,14 +371,6 @@ public void run() { // Therefore, we'll show a spinner here, and hides it when mAuthorizationRequestUrl is successfully loaded. // After that, progress bar will be displayed by MSA/AAD. mProgressBar.setVisibility(View.VISIBLE); - -// TODO: REMOVE THIS BEFORE MERGING, Using this to test JS API -// mWebView.evaluateJavascript(AuthUxJavaScriptInterface.Companion.getInterfaceName() +".postToBroker('{function: NUMBER_MATCH,data: {sessionID: id, numberMatch: number}}')", new ValueCallback() { -// @Override -// public void onReceiveValue(String value) { -// int x = 0; -// } -// }); } }); } From 0415ee7c9dd60e25f96f6fba521bd26bda6291c6 Mon Sep 17 00:00:00 2001 From: fadidurah Date: Tue, 20 May 2025 00:07:03 -0400 Subject: [PATCH 11/15] update to DTO --- .../broker/AuthUxJavaScriptInterface.kt | 43 +++++++--- .../broker/AuthUxJsonPayloadObject.kt | 86 +++++++++++++++++++ .../broker/AuthUxJavaScriptInterfaceTest.kt | 23 +++++ .../broker/AuthUxJsonPayloadObjectTest.kt | 75 ++++++++++++++++ 4 files changed, 214 insertions(+), 13 deletions(-) create mode 100644 common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJsonPayloadObject.kt create mode 100644 common/src/test/java/com/microsoft/identity/common/internal/broker/AuthUxJsonPayloadObjectTest.kt diff --git a/common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterface.kt b/common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterface.kt index 64e7cccd72..ca51163aa1 100644 --- a/common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterface.kt +++ b/common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterface.kt @@ -25,8 +25,9 @@ package com.microsoft.identity.common.internal.broker import android.webkit.JavascriptInterface import com.google.gson.stream.MalformedJsonException import com.microsoft.identity.common.internal.numberMatch.NumberMatchHelper -import com.microsoft.identity.common.java.util.JsonUtil import com.microsoft.identity.common.logging.Logger +import com.google.gson.Gson +import com.google.gson.JsonSyntaxException /** * JavaScript API to receive JSON string payloads from AuthUX in order to facilitate calling various @@ -74,25 +75,36 @@ class AuthUxJavaScriptInterface { Logger.info(methodTag, "Received a payload from AuthUX through JavaScript API.") try { - val parsedJson = JsonUtil.extractJsonObjectIntoMap(jsonPayload) + val payloadObject = parseJsonToAuthUxJsonPayloadObject(jsonPayload) + + Logger.info(methodTag, "Correlation ID during JavaScript Call: [${payloadObject.correlationId}]") - val correlationID = parsedJson["correlationID"] - Logger.info(methodTag, "Correlation ID during JavaScript Call: [$correlationID]") // TODO: Leaving these here, as these will be relevant for next WebCP feature - // val actionName = parsedJson["action_name"] - // val actionComponent = parsedJson["action_component"] + // val actionName = payloadObject.actionName + // val actionComponent = payloadObject.actionComponent + + val parameters = payloadObject.params + if (parameters == null) { + Logger.warn(methodTag, "Payload from AuthUX contained no \"params\" field.") + return + } + + val function = parameters.function - val parameters = JsonUtil.extractJsonObjectIntoMap(parsedJson["params"]) - val function = parameters["function"] - val data = JsonUtil.extractJsonObjectIntoMap(parameters["data"]) Logger.info(methodTag, "Function name: [$function]") + val data = parameters.data + if (data == null) { + Logger.warn(methodTag, "Payload from AuthUX contained no \"data\" field.") + return + } + when (function) { FunctionNames.NUMBER_MATCH.name -> NumberMatchHelper.storeNumberMatch( - data[NumberMatchHelper.SESSION_ID_ATTRIBUTE_NAME], - data[NumberMatchHelper.NUMBER_MATCH_ATTRIBUTE_NAME]) + data.sessionId, + data.numberMatch) else -> Logger.warn(methodTag, "Payload from AuthUX contained an unknown function name.") } @@ -101,8 +113,8 @@ class AuthUxJavaScriptInterface { is NullPointerException -> { Logger.warn(methodTag, "Payload with missing mandatory fields sent through JavaScriptInterface") } - is MalformedJsonException -> { - Logger.warn(methodTag, "Malformed JSON payload sent through JavaScriptInterface") + is MalformedJsonException, is JsonSyntaxException -> { + Logger.warn(methodTag, "Error Parsing JSON payload sent through JavaScriptInterface") } else -> { Logger.warn(methodTag, "Unknown error occurred while processing the payload.") @@ -111,6 +123,11 @@ class AuthUxJavaScriptInterface { } } + private fun parseJsonToAuthUxJsonPayloadObject(jsonString: String): AuthUxJsonPayloadObject { + val gson = Gson() + return gson.fromJson(jsonString, AuthUxJsonPayloadObject::class.java) + } + /** * Enum class to hold function names */ diff --git a/common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJsonPayloadObject.kt b/common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJsonPayloadObject.kt new file mode 100644 index 0000000000..87b3409f87 --- /dev/null +++ b/common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJsonPayloadObject.kt @@ -0,0 +1,86 @@ +// 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 + +import com.google.gson.annotations.SerializedName + +/** + * Data class representing the JSON payload object received from AuthUX. + * + * @property correlationId The correlation ID for the request. + * @property actionName The name of the action being performed. + * @property actionComponent The component responsible for the action. + * @property params The parameters for the action, including function and data. + */ +data class AuthUxJsonPayloadObject( + @SerializedName(SerializedNames.CORRELATIONID) + val correlationId: String?, + + @SerializedName(SerializedNames.ACTION_NAME) + val actionName: String?, + + @SerializedName(SerializedNames.ACTION_COMPONENT) + val actionComponent: String?, + + @SerializedName(SerializedNames.PARAMS) + val params: AuthUxParams? +) + +/** + * Data class representing the parameters for the action, including function and data. + * + * @property function The function to be executed. + * @property data The data associated with the function. + */ +data class AuthUxParams( + @SerializedName(SerializedNames.FUNCTION) + val function: String?, + + @SerializedName(SerializedNames.DATA) + val data: AuthUxData? +) + +/** + * Data class representing the data associated with the JS API call. + * + * @property sessionId The session ID for the request. + * @property numberMatch The number match value. + */ +data class AuthUxData( + @SerializedName(SerializedNames.SESSIONID) + val sessionId: String?, + + @SerializedName(SerializedNames.NUMBERMATCH) + val numberMatch: String? +) + +object SerializedNames { + const val CORRELATIONID = "correlationID" + const val ACTION_NAME = "action_name" + const val ACTION_COMPONENT = "action_component" + const val PARAMS = "params" + const val FUNCTION = "function" + const val DATA = "data" + const val SESSIONID = "sessionID" + const val NUMBERMATCH = "numberMatch" +} \ No newline at end of file diff --git a/common/src/test/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterfaceTest.kt b/common/src/test/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterfaceTest.kt index dded8e4fd9..a8956649b0 100644 --- a/common/src/test/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterfaceTest.kt +++ b/common/src/test/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterfaceTest.kt @@ -23,6 +23,7 @@ package com.microsoft.identity.common.internal.broker import com.microsoft.identity.common.internal.numberMatch.NumberMatchHelper +import org.junit.After import org.junit.Before import org.junit.Test @@ -55,6 +56,12 @@ class AuthUxJavaScriptInterfaceTest { authUxJavaScriptInterface = AuthUxJavaScriptInterface() } + @After + fun tearDown() { + // Clear the static map after each test + NumberMatchHelper.numberMatchMap.clear() + } + @Test fun `test postMessageToBroker with NUMBER_MATCH function`() { // Call the method @@ -64,4 +71,20 @@ class AuthUxJavaScriptInterfaceTest { val storedValue = NumberMatchHelper.numberMatchMap[mockSessionId] assert(storedValue == mockNumberMatchValue) } + + @Test + fun `test postMessageToBroker with empty json`() { + // Call the method + authUxJavaScriptInterface.postMessageToBroker("{}") + + // Should not get an exception + } + + @Test + fun `test postMessageToBroker with non-json string`() { + // Call the method + authUxJavaScriptInterface.postMessageToBroker("NotAJson") + + // Should not get an exception + } } diff --git a/common/src/test/java/com/microsoft/identity/common/internal/broker/AuthUxJsonPayloadObjectTest.kt b/common/src/test/java/com/microsoft/identity/common/internal/broker/AuthUxJsonPayloadObjectTest.kt new file mode 100644 index 0000000000..527130a77f --- /dev/null +++ b/common/src/test/java/com/microsoft/identity/common/internal/broker/AuthUxJsonPayloadObjectTest.kt @@ -0,0 +1,75 @@ +package com.microsoft.identity.common.internal.broker + +import com.google.gson.Gson +import org.junit.Assert.* +import org.junit.Test + +class AuthUxJsonPayloadObjectTest { + + private val gson = Gson() + + @Test + fun `test deserialization of valid JSON`() { + val json = """ + { + "correlationID": "12345", + "action_name": "write_data", + "action_component": "broker", + "params": { + "function": "NUMBER_MATCH", + "data": { + "sessionID": "67890", + "numberMatch": "123456" + } + } + } + """.trimIndent() + + val payload = gson.fromJson(json, AuthUxJsonPayloadObject::class.java) + + assertNotNull(payload) + assertEquals("12345", payload.correlationId) + assertEquals("write_data", payload.actionName) + assertEquals("broker", payload.actionComponent) + + val params = payload.params + assertNotNull(params) + assertEquals("NUMBER_MATCH", params?.function) + + val data = params?.data + assertNotNull(data) + assertEquals("67890", data?.sessionId) + assertEquals("123456", data?.numberMatch) + } + + @Test + fun `test deserialization of JSON with missing fields`() { + val json = """ + { + "correlationID": "12345", + "action_name": "write_data" + } + """.trimIndent() + + val payload = gson.fromJson(json, AuthUxJsonPayloadObject::class.java) + + assertNotNull(payload) + assertEquals("12345", payload.correlationId) + assertEquals("write_data", payload.actionName) + assertNull(payload.actionComponent) + assertNull(payload.params) + } + + @Test + fun `test deserialization of empty JSON`() { + val json = "{}" + + val payload = gson.fromJson(json, AuthUxJsonPayloadObject::class.java) + + assertNotNull(payload) + assertNull(payload.correlationId) + assertNull(payload.actionName) + assertNull(payload.actionComponent) + assertNull(payload.params) + } +} \ No newline at end of file From 9fe183c6a0a1f299d8d8f6714d99d8170fe3fb9b Mon Sep 17 00:00:00 2001 From: fadidurah Date: Tue, 20 May 2025 12:52:23 -0400 Subject: [PATCH 12/15] spotbug fix --- .../providers/oauth2/WebViewAuthorizationFragment.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java b/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java index 1609f4c663..865d93b78e 100644 --- a/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java +++ b/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java @@ -24,6 +24,7 @@ import android.annotation.SuppressLint; import android.app.Activity; +import android.content.Context; import android.content.Intent; import android.graphics.Bitmap; import android.os.Build; @@ -220,8 +221,10 @@ void extractState(@NonNull final Bundle state) { if (mAuthorizationRequestUrl != null) { isEstsRequest = mAuthorizationRequestUrl.startsWith("https://login.microsoftonline.com"); } - if (getContext() != null) { - isBrokerRequest = ProcessUtil.isRunningOnAuthService(getContext()); + + final Context context = getContext(); + if (context != null) { + isBrokerRequest = ProcessUtil.isRunningOnAuthService(context); } mRedirectUri = state.getString(REDIRECT_URI); mRequestHeaders = getRequestHeaders(state); From 0f43c26285a85c4d910910808d8836ea946c0f88 Mon Sep 17 00:00:00 2001 From: fadidurah Date: Wed, 21 May 2025 21:58:08 -0400 Subject: [PATCH 13/15] address comments, update js exposure --- .../internal/AuthenticationConstants.java | 10 ++++++ .../broker/AuthUxJavaScriptInterface.kt | 26 ++++++++++++--- ...nPayloadObject.kt => AuthUxJsonPayload.kt} | 16 ++++----- .../oauth2/WebViewAuthorizationFragment.java | 12 ++----- .../AzureActiveDirectoryWebViewClient.java | 26 +++++++++++++++ .../broker/AuthUxJavaScriptInterfaceTest.kt | 33 ++++++++++++++++++- ...ObjectTest.kt => AuthUxJsonPayloadTest.kt} | 10 +++--- 7 files changed, 105 insertions(+), 28 deletions(-) rename common/src/main/java/com/microsoft/identity/common/internal/broker/{AuthUxJsonPayloadObject.kt => AuthUxJsonPayload.kt} (89%) rename common/src/test/java/com/microsoft/identity/common/internal/broker/{AuthUxJsonPayloadObjectTest.kt => AuthUxJsonPayloadTest.kt} (91%) diff --git a/common/src/main/java/com/microsoft/identity/common/adal/internal/AuthenticationConstants.java b/common/src/main/java/com/microsoft/identity/common/adal/internal/AuthenticationConstants.java index c0f95d8655..9bcd7ca128 100644 --- a/common/src/main/java/com/microsoft/identity/common/adal/internal/AuthenticationConstants.java +++ b/common/src/main/java/com/microsoft/identity/common/adal/internal/AuthenticationConstants.java @@ -1222,6 +1222,16 @@ public static String computeMaxHostBrokerProtocol() { */ public static final String REDIRECT_PREFIX = "msauth"; + /** + * Prefix for AAD urls + */ + public static final String AAD_URL_PREFIX = "https://login.microsoftonline."; + + /** + * Prefix for MSA urls + */ + public static final String MSA_URL_PREFIX = "https://login.live.com/"; + /** * Encoded delimiter for redirect. */ diff --git a/common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterface.kt b/common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterface.kt index ca51163aa1..838d184f77 100644 --- a/common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterface.kt +++ b/common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterface.kt @@ -23,11 +23,12 @@ package com.microsoft.identity.common.internal.broker import android.webkit.JavascriptInterface +import com.google.gson.Gson +import com.google.gson.JsonSyntaxException import com.google.gson.stream.MalformedJsonException +import com.microsoft.identity.common.adal.internal.AuthenticationConstants import com.microsoft.identity.common.internal.numberMatch.NumberMatchHelper import com.microsoft.identity.common.logging.Logger -import com.google.gson.Gson -import com.google.gson.JsonSyntaxException /** * JavaScript API to receive JSON string payloads from AuthUX in order to facilitate calling various @@ -45,6 +46,23 @@ class AuthUxJavaScriptInterface { fun getInterfaceName() : String { return JAVASCRIPT_INTERFACE_NAME } + + /** + * Helper method to determine if url is a valid Url for the JS Interface + * @param url url being loaded + * @return true if url is a valid, safe url, false otherwise + */ + fun isValidUrlForInterface(url: String?): Boolean { + // If url is null, return false + if (url == null) { + return false + } + + // Otherwise, make sure url is a valid url + // We only want to allow URLs that start with the AAD URL prefix or the MSA URL prefix + return url.startsWith(AuthenticationConstants.Broker.AAD_URL_PREFIX) || + url.startsWith(AuthenticationConstants.Broker.MSA_URL_PREFIX) + } } /** @@ -123,9 +141,9 @@ class AuthUxJavaScriptInterface { } } - private fun parseJsonToAuthUxJsonPayloadObject(jsonString: String): AuthUxJsonPayloadObject { + private fun parseJsonToAuthUxJsonPayloadObject(jsonString: String): AuthUxJsonPayload { val gson = Gson() - return gson.fromJson(jsonString, AuthUxJsonPayloadObject::class.java) + return gson.fromJson(jsonString, AuthUxJsonPayload::class.java) } /** diff --git a/common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJsonPayloadObject.kt b/common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJsonPayload.kt similarity index 89% rename from common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJsonPayloadObject.kt rename to common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJsonPayload.kt index 87b3409f87..b04d51d0b1 100644 --- a/common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJsonPayloadObject.kt +++ b/common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJsonPayload.kt @@ -32,8 +32,8 @@ import com.google.gson.annotations.SerializedName * @property actionComponent The component responsible for the action. * @property params The parameters for the action, including function and data. */ -data class AuthUxJsonPayloadObject( - @SerializedName(SerializedNames.CORRELATIONID) +data class AuthUxJsonPayload( + @SerializedName(SerializedNames.CORRELATION_ID) val correlationId: String?, @SerializedName(SerializedNames.ACTION_NAME) @@ -67,20 +67,20 @@ data class AuthUxParams( * @property numberMatch The number match value. */ data class AuthUxData( - @SerializedName(SerializedNames.SESSIONID) + @SerializedName(SerializedNames.SESSION_ID) val sessionId: String?, - @SerializedName(SerializedNames.NUMBERMATCH) + @SerializedName(SerializedNames.NUMBER_MATCH) val numberMatch: String? ) object SerializedNames { - const val CORRELATIONID = "correlationID" + const val CORRELATION_ID = "correlationID" const val ACTION_NAME = "action_name" const val ACTION_COMPONENT = "action_component" const val PARAMS = "params" const val FUNCTION = "function" const val DATA = "data" - const val SESSIONID = "sessionID" - const val NUMBERMATCH = "numberMatch" -} \ No newline at end of file + const val SESSION_ID = "sessionID" + const val NUMBER_MATCH = "numberMatch" +} diff --git a/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java b/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java index 865d93b78e..c55ab14959 100644 --- a/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java +++ b/common/src/main/java/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java @@ -62,7 +62,6 @@ import com.microsoft.identity.common.java.constants.FidoConstants; import com.microsoft.identity.common.java.exception.ClientException; import com.microsoft.identity.common.java.flighting.CommonFlight; -import com.microsoft.identity.common.java.platform.Device; import com.microsoft.identity.common.java.flighting.CommonFlightsManager; import com.microsoft.identity.common.java.ui.webview.authorization.IAuthorizationCompletionCallback; import com.microsoft.identity.common.java.providers.RawAuthorizationResult; @@ -127,7 +126,6 @@ public class WebViewAuthorizationFragment extends AuthorizationFragment { private SwitchBrowserProtocolCoordinator mSwitchBrowserProtocolCoordinator = null; private boolean isBrokerRequest = false; - private boolean isEstsRequest = false; @Override public void onCreate(@Nullable Bundle savedInstanceState) { @@ -218,10 +216,6 @@ void extractState(@NonNull final Bundle state) { mAuthIntent = state.getParcelable(AUTH_INTENT); mPkeyAuthStatus = state.getBoolean(PKEYAUTH_STATUS, false); mAuthorizationRequestUrl = state.getString(REQUEST_URL); - if (mAuthorizationRequestUrl != null) { - isEstsRequest = mAuthorizationRequestUrl.startsWith("https://login.microsoftonline.com"); - } - final Context context = getContext(); if (context != null) { isBrokerRequest = ProcessUtil.isRunningOnAuthService(context); @@ -271,6 +265,7 @@ public void onPageLoaded(final String url) { getSwitchBrowserCoordinator().getSwitchBrowserRequestHandler() ); setUpWebView(view, mAADWebViewClient); + mAADWebViewClient.initializeAuthUxJavaScriptApi(mWebView, mAuthorizationRequestUrl); launchWebView(mAuthorizationRequestUrl, mRequestHeaders); return view; } @@ -305,9 +300,6 @@ private void setUpWebView(@NonNull final View view, mWebView.getSettings().setUserAgentString( userAgent + AuthenticationConstants.Broker.CLIENT_TLS_NOT_SUPPORTED); mWebView.getSettings().setJavaScriptEnabled(true); - if (isBrokerRequest && isEstsRequest) { - mWebView.addJavascriptInterface(new AuthUxJavaScriptInterface(), AuthUxJavaScriptInterface.Companion.getInterfaceName()); - } mWebView.requestFocus(View.FOCUS_DOWN); // Set focus to the view for touch event @@ -433,7 +425,7 @@ private HashMap getRequestHeaders(final Bundle state) { public ActivityResultLauncher getFidoLauncher() { return mFidoLauncher; } - + class AuthorizationCompletionCallback implements IAuthorizationCompletionCallback { @Override public void onChallengeResponseReceived(@NonNull final RawAuthorizationResult response) { diff --git a/common/src/main/java/com/microsoft/identity/common/internal/ui/webview/AzureActiveDirectoryWebViewClient.java b/common/src/main/java/com/microsoft/identity/common/internal/ui/webview/AzureActiveDirectoryWebViewClient.java index 2db5879bcc..18a73c141b 100644 --- a/common/src/main/java/com/microsoft/identity/common/internal/ui/webview/AzureActiveDirectoryWebViewClient.java +++ b/common/src/main/java/com/microsoft/identity/common/internal/ui/webview/AzureActiveDirectoryWebViewClient.java @@ -41,6 +41,7 @@ import com.microsoft.identity.common.adal.internal.AuthenticationConstants; import com.microsoft.identity.common.adal.internal.util.StringExtensions; +import com.microsoft.identity.common.internal.broker.AuthUxJavaScriptInterface; import com.microsoft.identity.common.internal.broker.PackageHelper; import com.microsoft.identity.common.internal.fido.CredManFidoManager; import com.microsoft.identity.common.internal.fido.FidoChallenge; @@ -130,6 +131,19 @@ public AzureActiveDirectoryWebViewClient(@NonNull final Activity activity, mSwitchBrowserRequestHandler = switchBrowserRequestHandler; } + /** + * This method is used to initialize the JavaScript API for the AuthUx JavaScript interface. + * It checks if the current process is running on the AuthService and if the URL is valid for the interface. + * If both conditions are met, it adds the JavaScript interface to the WebView. + */ + public void initializeAuthUxJavaScriptApi(@NonNull final WebView view, final String url) { + if (ProcessUtil.isRunningOnAuthService(getActivity().getApplicationContext()) && AuthUxJavaScriptInterface.Companion.isValidUrlForInterface(url)) { + // If broker request, and a valid url, expose JavaScript API + Logger.info(TAG, "Adding AuthUx JavaScript Interface"); + view.addJavascriptInterface(new AuthUxJavaScriptInterface(), AuthUxJavaScriptInterface.Companion.getInterfaceName()); + } + } + /** * Give the host application a chance to take over the control when a new url is about to be loaded in the current WebView. * This method was deprecated in API level 24. @@ -191,6 +205,18 @@ private boolean handleUrl(final WebView view, final String url) { final String methodTag = TAG + ":handleUrl"; final String formattedURL = url.toLowerCase(Locale.US); + // Re-evaluate adding AuthUx JavaScript Interface + if (ProcessUtil.isRunningOnAuthService(getActivity().getApplicationContext()) && AuthUxJavaScriptInterface.Companion.isValidUrlForInterface(url)) { + // If broker request, and a valid url, expose JavaScript API + Logger.info(methodTag, "Adding AuthUx JavaScript Interface"); + view.addJavascriptInterface(new AuthUxJavaScriptInterface(), AuthUxJavaScriptInterface.Companion.getInterfaceName()); + } else { + // Remove AuthUx JavaScript Interface + Logger.info(methodTag, "Removing AuthUx JavaScript Interface"); + view.removeJavascriptInterface(AuthUxJavaScriptInterface.Companion.getInterfaceName()); + } + + try { if (isPkeyAuthUrl(formattedURL)) { Logger.info(methodTag,"WebView detected request for pkeyauth challenge."); diff --git a/common/src/test/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterfaceTest.kt b/common/src/test/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterfaceTest.kt index a8956649b0..fb5eb2c538 100644 --- a/common/src/test/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterfaceTest.kt +++ b/common/src/test/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterfaceTest.kt @@ -24,6 +24,7 @@ package com.microsoft.identity.common.internal.broker import com.microsoft.identity.common.internal.numberMatch.NumberMatchHelper import org.junit.After +import org.junit.Assert import org.junit.Before import org.junit.Test @@ -69,7 +70,7 @@ class AuthUxJavaScriptInterfaceTest { // Verify that the data was stored in NumberMatchHelper val storedValue = NumberMatchHelper.numberMatchMap[mockSessionId] - assert(storedValue == mockNumberMatchValue) + Assert.assertTrue(storedValue == mockNumberMatchValue) } @Test @@ -87,4 +88,34 @@ class AuthUxJavaScriptInterfaceTest { // Should not get an exception } + + @Test + fun `test isValidUrlForInterface with valid AAD URL`() { + val validUrl = "https://login.microsoftonline.com/common/oauth2/authorize" + Assert.assertTrue(AuthUxJavaScriptInterface.isValidUrlForInterface(validUrl)) + } + + @Test + fun `test isValidUrlForInterface with valid MSA URL`() { + val validUrl = "https://login.live.com/oauth20_authorize.srf" + Assert.assertTrue(AuthUxJavaScriptInterface.isValidUrlForInterface(validUrl)) + } + + @Test + fun `test isValidUrlForInterface with null URL`() { + val nullUrl: String? = null + Assert.assertFalse(AuthUxJavaScriptInterface.isValidUrlForInterface(nullUrl)) + } + + @Test + fun `test isValidUrlForInterface with invalid URL`() { + val invalidUrl = "https://example.com" + Assert.assertFalse(AuthUxJavaScriptInterface.isValidUrlForInterface(invalidUrl)) + } + + @Test + fun `test isValidUrlForInterface with empty URL`() { + val emptyUrl = "" + Assert.assertFalse(AuthUxJavaScriptInterface.isValidUrlForInterface(emptyUrl)) + } } diff --git a/common/src/test/java/com/microsoft/identity/common/internal/broker/AuthUxJsonPayloadObjectTest.kt b/common/src/test/java/com/microsoft/identity/common/internal/broker/AuthUxJsonPayloadTest.kt similarity index 91% rename from common/src/test/java/com/microsoft/identity/common/internal/broker/AuthUxJsonPayloadObjectTest.kt rename to common/src/test/java/com/microsoft/identity/common/internal/broker/AuthUxJsonPayloadTest.kt index 45db351034..5c4f3ae9cb 100644 --- a/common/src/test/java/com/microsoft/identity/common/internal/broker/AuthUxJsonPayloadObjectTest.kt +++ b/common/src/test/java/com/microsoft/identity/common/internal/broker/AuthUxJsonPayloadTest.kt @@ -26,7 +26,7 @@ import com.google.gson.Gson import org.junit.Assert.* import org.junit.Test -class AuthUxJsonPayloadObjectTest { +class AuthUxJsonPayloadTest { private val gson = Gson() @@ -47,7 +47,7 @@ class AuthUxJsonPayloadObjectTest { } """.trimIndent() - val payload = gson.fromJson(json, AuthUxJsonPayloadObject::class.java) + val payload = gson.fromJson(json, AuthUxJsonPayload::class.java) assertNotNull(payload) assertEquals("12345", payload.correlationId) @@ -73,7 +73,7 @@ class AuthUxJsonPayloadObjectTest { } """.trimIndent() - val payload = gson.fromJson(json, AuthUxJsonPayloadObject::class.java) + val payload = gson.fromJson(json, AuthUxJsonPayload::class.java) assertNotNull(payload) assertEquals("12345", payload.correlationId) @@ -86,7 +86,7 @@ class AuthUxJsonPayloadObjectTest { fun `test deserialization of empty JSON`() { val json = "{}" - val payload = gson.fromJson(json, AuthUxJsonPayloadObject::class.java) + val payload = gson.fromJson(json, AuthUxJsonPayload::class.java) assertNotNull(payload) assertNull(payload.correlationId) @@ -94,4 +94,4 @@ class AuthUxJsonPayloadObjectTest { assertNull(payload.actionComponent) assertNull(payload.params) } -} \ No newline at end of file +} From b25499323daa3f488824508b6ed0d43533f3de3a Mon Sep 17 00:00:00 2001 From: fadidurah Date: Fri, 23 May 2025 13:55:58 -0400 Subject: [PATCH 14/15] adjust to comments, nonnull serialization --- .../build-consumers.yml | 2 +- .../internal/AuthenticationConstants.java | 4 +- .../broker/AuthUxJavaScriptInterface.kt | 41 +++++++++++----- .../internal/broker/AuthUxJsonPayload.kt | 44 +++++++++++++---- .../internal/broker/AuthUxJsonPayloadTest.kt | 47 +++++++++++++------ 5 files changed, 98 insertions(+), 40 deletions(-) diff --git a/azure-pipelines/pull-request-validation/build-consumers.yml b/azure-pipelines/pull-request-validation/build-consumers.yml index fd8eabcb14..dfee1bce10 100644 --- a/azure-pipelines/pull-request-validation/build-consumers.yml +++ b/azure-pipelines/pull-request-validation/build-consumers.yml @@ -186,7 +186,7 @@ stages: value: $[ dependencies.setupBranch.outputs['setvarStep.commonBranch'] ] # map in the variable condition: and( succeeded(), not(contains(variables['prLabels'], variables['skipConsumerValidationLabel'])) ) pool: - vmImage: 'ubuntu-20.04' + vmImage: 'ubuntu-22.04' steps: - checkout: broker displayName: Checkout broker repository diff --git a/common/src/main/java/com/microsoft/identity/common/adal/internal/AuthenticationConstants.java b/common/src/main/java/com/microsoft/identity/common/adal/internal/AuthenticationConstants.java index 9bcd7ca128..6a4854a9d7 100644 --- a/common/src/main/java/com/microsoft/identity/common/adal/internal/AuthenticationConstants.java +++ b/common/src/main/java/com/microsoft/identity/common/adal/internal/AuthenticationConstants.java @@ -1225,12 +1225,12 @@ public static String computeMaxHostBrokerProtocol() { /** * Prefix for AAD urls */ - public static final String AAD_URL_PREFIX = "https://login.microsoftonline."; + public static final String AAD_URL_HOST_PREFIX = "login.microsoftonline."; /** * Prefix for MSA urls */ - public static final String MSA_URL_PREFIX = "https://login.live.com/"; + public static final String MSA_URL_HOST_PREFIX = "login.live."; /** * Encoded delimiter for redirect. diff --git a/common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterface.kt b/common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterface.kt index 838d184f77..09978a5b86 100644 --- a/common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterface.kt +++ b/common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterface.kt @@ -23,12 +23,15 @@ package com.microsoft.identity.common.internal.broker import android.webkit.JavascriptInterface -import com.google.gson.Gson +import com.google.gson.GsonBuilder +import com.google.gson.JsonParseException import com.google.gson.JsonSyntaxException import com.google.gson.stream.MalformedJsonException import com.microsoft.identity.common.adal.internal.AuthenticationConstants import com.microsoft.identity.common.internal.numberMatch.NumberMatchHelper import com.microsoft.identity.common.logging.Logger +import java.net.MalformedURLException +import java.net.URL /** * JavaScript API to receive JSON string payloads from AuthUX in order to facilitate calling various @@ -52,16 +55,28 @@ class AuthUxJavaScriptInterface { * @param url url being loaded * @return true if url is a valid, safe url, false otherwise */ - fun isValidUrlForInterface(url: String?): Boolean { + fun isValidUrlForInterface(urlString: String?): Boolean { // If url is null, return false - if (url == null) { + if (urlString.isNullOrEmpty()) { return false } + val url : URL + try { + url = URL(urlString) + } catch (e: MalformedURLException) { + // If url is not a valid URL, return false + Logger.warn(TAG, "Malformed URL passed.") + return false + + } + + val host = url.host + // Otherwise, make sure url is a valid url - // We only want to allow URLs that start with the AAD URL prefix or the MSA URL prefix - return url.startsWith(AuthenticationConstants.Broker.AAD_URL_PREFIX) || - url.startsWith(AuthenticationConstants.Broker.MSA_URL_PREFIX) + // We only want to allow URLs that have the AAD or MSA url hosts + return host.startsWith(AuthenticationConstants.Broker.AAD_URL_HOST_PREFIX) || + host.startsWith(AuthenticationConstants.Broker.MSA_URL_HOST_PREFIX) } } @@ -129,20 +144,22 @@ class AuthUxJavaScriptInterface { } catch (e: Exception) { // If we run into exceptions, we don't want to kill the broker when (e) { is NullPointerException -> { - Logger.warn(methodTag, "Payload with missing mandatory fields sent through JavaScriptInterface") + Logger.error(methodTag, "Payload with missing mandatory fields sent through JavaScriptInterface", e) } - is MalformedJsonException, is JsonSyntaxException -> { - Logger.warn(methodTag, "Error Parsing JSON payload sent through JavaScriptInterface") + is MalformedJsonException, is JsonSyntaxException, is JsonParseException -> { + Logger.error(methodTag, "Error Parsing JSON payload sent through JavaScriptInterface", e) } else -> { - Logger.warn(methodTag, "Unknown error occurred while processing the payload.") + Logger.error(methodTag, "Unknown error occurred while processing the payload.", e) } } } } - private fun parseJsonToAuthUxJsonPayloadObject(jsonString: String): AuthUxJsonPayload { - val gson = Gson() + private fun parseJsonToAuthUxJsonPayloadObject(jsonString: String): AuthUxJsonPayload{ + val gson = GsonBuilder() + .registerTypeAdapter(AuthUxJsonPayload::class.java, AuthUxJsonPayloadKTDeserializer()) + .create() return gson.fromJson(jsonString, AuthUxJsonPayload::class.java) } diff --git a/common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJsonPayload.kt b/common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJsonPayload.kt index b04d51d0b1..668f795522 100644 --- a/common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJsonPayload.kt +++ b/common/src/main/java/com/microsoft/identity/common/internal/broker/AuthUxJsonPayload.kt @@ -22,7 +22,12 @@ // THE SOFTWARE. package com.microsoft.identity.common.internal.broker +import com.google.gson.JsonDeserializationContext +import com.google.gson.JsonDeserializer +import com.google.gson.JsonElement +import com.google.gson.JsonParseException import com.google.gson.annotations.SerializedName +import java.lang.reflect.Type /** * Data class representing the JSON payload object received from AuthUX. @@ -33,16 +38,9 @@ import com.google.gson.annotations.SerializedName * @property params The parameters for the action, including function and data. */ data class AuthUxJsonPayload( - @SerializedName(SerializedNames.CORRELATION_ID) - val correlationId: String?, - - @SerializedName(SerializedNames.ACTION_NAME) - val actionName: String?, - - @SerializedName(SerializedNames.ACTION_COMPONENT) - val actionComponent: String?, - - @SerializedName(SerializedNames.PARAMS) + val correlationId: String, + val actionName: String, + val actionComponent: String, val params: AuthUxParams? ) @@ -74,6 +72,32 @@ data class AuthUxData( val numberMatch: String? ) +class AuthUxJsonPayloadKTDeserializer : JsonDeserializer { + override fun deserialize(json: JsonElement, typeOfT: Type, context: JsonDeserializationContext): AuthUxJsonPayload { + val jsonObject = json.asJsonObject + + // Validate required fields + val correlationId = jsonObject.get(SerializedNames.CORRELATION_ID)?.asString + ?: throw JsonParseException("correlationID is required and cannot be null") + val actionName = jsonObject.get(SerializedNames.ACTION_NAME)?.asString + ?: throw JsonParseException("action_name is required and cannot be null") + val actionComponent = jsonObject.get(SerializedNames.ACTION_COMPONENT)?.asString + ?: throw JsonParseException("action_component is required and cannot be null") + + // Deserialize params if present + val params = jsonObject.get("params")?.let { + context.deserialize(it, AuthUxParams::class.java) + } + + return AuthUxJsonPayload( + correlationId = correlationId, + actionName = actionName, + actionComponent = actionComponent, + params = params + ) + } +} + object SerializedNames { const val CORRELATION_ID = "correlationID" const val ACTION_NAME = "action_name" diff --git a/common/src/test/java/com/microsoft/identity/common/internal/broker/AuthUxJsonPayloadTest.kt b/common/src/test/java/com/microsoft/identity/common/internal/broker/AuthUxJsonPayloadTest.kt index 5c4f3ae9cb..82bf4f322a 100644 --- a/common/src/test/java/com/microsoft/identity/common/internal/broker/AuthUxJsonPayloadTest.kt +++ b/common/src/test/java/com/microsoft/identity/common/internal/broker/AuthUxJsonPayloadTest.kt @@ -21,14 +21,16 @@ // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN // THE SOFTWARE. package com.microsoft.identity.common.internal.broker - -import com.google.gson.Gson +import com.google.gson.GsonBuilder +import com.google.gson.JsonParseException import org.junit.Assert.* import org.junit.Test class AuthUxJsonPayloadTest { - private val gson = Gson() + private val gson = GsonBuilder() + .registerTypeAdapter(AuthUxJsonPayload::class.java, AuthUxJsonPayloadKTDeserializer()) + .create() @Test fun `test deserialization of valid JSON`() { @@ -65,11 +67,15 @@ class AuthUxJsonPayloadTest { } @Test - fun `test deserialization of JSON with missing fields`() { + fun `test deserialization of JSON with missing optional fields`() { val json = """ { "correlationID": "12345", - "action_name": "write_data" + "action_name": "write_data", + "action_component": "broker", + "params" : { + "invalidField": "invalidField" + } } """.trimIndent() @@ -78,20 +84,31 @@ class AuthUxJsonPayloadTest { assertNotNull(payload) assertEquals("12345", payload.correlationId) assertEquals("write_data", payload.actionName) - assertNull(payload.actionComponent) - assertNull(payload.params) + assertEquals("broker", payload.actionComponent) + assertNotNull(payload.params) + assertNull(payload.params?.data) + assertNull(payload.params?.function) } - @Test - fun `test deserialization of empty JSON`() { + @Test(expected = JsonParseException::class) + fun `test deserialization of JSON with missing mandatory fields, exception expected`() { + val json = """ + { + "correlationID": "12345", + "action_name": "write_data" + } + """.trimIndent() + + // This should throw an exception because "action_component" and "params" is missing + gson.fromJson(json, AuthUxJsonPayload::class.java) + + } + + @Test(expected = JsonParseException::class) + fun `test deserialization of empty JSON, exception expected`() { val json = "{}" + // This should throw an exception because the JSON is empty val payload = gson.fromJson(json, AuthUxJsonPayload::class.java) - - assertNotNull(payload) - assertNull(payload.correlationId) - assertNull(payload.actionName) - assertNull(payload.actionComponent) - assertNull(payload.params) } } From 56ff04dac426d3d3295e3e6881b1fc7e1c9303ea Mon Sep 17 00:00:00 2001 From: fadidurah Date: Fri, 23 May 2025 20:28:25 -0400 Subject: [PATCH 15/15] remove small detail --- .../identity/common/internal/broker/AuthUxJsonPayloadTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/src/test/java/com/microsoft/identity/common/internal/broker/AuthUxJsonPayloadTest.kt b/common/src/test/java/com/microsoft/identity/common/internal/broker/AuthUxJsonPayloadTest.kt index 82bf4f322a..951e60c740 100644 --- a/common/src/test/java/com/microsoft/identity/common/internal/broker/AuthUxJsonPayloadTest.kt +++ b/common/src/test/java/com/microsoft/identity/common/internal/broker/AuthUxJsonPayloadTest.kt @@ -109,6 +109,6 @@ class AuthUxJsonPayloadTest { val json = "{}" // This should throw an exception because the JSON is empty - val payload = gson.fromJson(json, AuthUxJsonPayload::class.java) + gson.fromJson(json, AuthUxJsonPayload::class.java) } }