Skip to content

Commit 6f36921

Browse files
authored
Update Java Script Api Exposure, Fixes AB#3385083 (#2773)
This PR will move the code that exposes the java script api from handleUrl on pageStarted. This will make it so we use the url that we are actually planning to load in the webview to validate whether or not to exposue the JS interface. previously, most pages were being validated correctly in handleUrl, but in certain redirects, we may have been missing some urls that should have had the url exposed. This change addresses those gaps by checking every url the webview is about to load. [AB#3385083](https://identitydivision.visualstudio.com/fac9d424-53d2-45c0-91b5-ef6ba7a6bf26/_workitems/edit/3385083)
1 parent 910e48f commit 6f36921

File tree

5 files changed

+70
-52
lines changed

5 files changed

+70
-52
lines changed

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

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1223,14 +1223,24 @@ public static String computeMaxHostBrokerProtocol() {
12231223
public static final String REDIRECT_PREFIX = "msauth";
12241224

12251225
/**
1226-
* Prefix for AAD urls
1226+
* Suffix for global AAD urls
12271227
*/
1228-
public static final String AAD_URL_HOST_PREFIX = "login.microsoftonline.";
1228+
public static final String AAD_GLOBAL_URL_HOST_SUFFIX = ".microsoftonline.com";
12291229

12301230
/**
1231-
* Prefix for MSA urls
1231+
* Suffix for China AAD urls
12321232
*/
1233-
public static final String MSA_URL_HOST_PREFIX = "login.live.";
1233+
public static final String AAD_CHINA_URL_HOST_SUFFIX = ".microsoftonline.cn";
1234+
1235+
/**
1236+
* Suffix for US AAD urls
1237+
*/
1238+
public static final String AAD_US_URL_HOST_SUFFIX = ".microsoftonline.us";
1239+
1240+
/**
1241+
* Suffix for Intune MDM urls
1242+
*/
1243+
public static final String AAD_INTUNE_MDM_URL_HOST_SUFFIX = ".microsoft.com";
12341244

12351245
/**
12361246
* Encoded delimiter for redirect.

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

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,8 @@ import com.microsoft.identity.common.internal.numberMatch.NumberMatchHelper
3232
import com.microsoft.identity.common.java.opentelemetry.AttributeName
3333
import com.microsoft.identity.common.java.opentelemetry.SpanExtension
3434
import com.microsoft.identity.common.logging.Logger
35-
import java.net.MalformedURLException
36-
import java.net.URL
35+
import java.net.URI
36+
import java.net.URISyntaxException
3737

3838
/**
3939
* JavaScript API to receive JSON string payloads from AuthUX in order to facilitate calling various
@@ -53,32 +53,32 @@ class AuthUxJavaScriptInterface {
5353
}
5454

5555
/**
56-
* Helper method to determine if url is a valid Url for the JS Interface
57-
* @param url url being loaded
58-
* @return true if url is a valid, safe url, false otherwise
56+
* Helper method to determine if uri is a valid Uri for the JS Interface
57+
* @param uriString uri being loaded
58+
* @return true if uri is a valid, safe uri, false otherwise
5959
*/
60-
fun isValidUrlForInterface(urlString: String?): Boolean {
61-
// If url is null, return false
62-
if (urlString.isNullOrEmpty()) {
60+
fun isValidUriForInterface(uriString: String?): Boolean {
61+
// If uri is null or empty, return false
62+
if (uriString.isNullOrEmpty()) {
6363
return false
6464
}
6565

66-
val url: URL
66+
val uri: URI
6767
try {
68-
url = URL(urlString)
69-
} catch (e: MalformedURLException) {
70-
// If url is not a valid URL, return false
71-
Logger.warn(TAG, "Malformed URL passed.")
68+
uri = URI(uriString)
69+
} catch (e: URISyntaxException) {
70+
Logger.warn(TAG, "URISyntaxException received. uri: $uriString, Message: ${e.message}")
7271
return false
73-
7472
}
7573

76-
val host = url.host
74+
val host = uri.host
7775

78-
// Otherwise, make sure url is a valid url
79-
// We only want to allow URLs that have the AAD or MSA url hosts
80-
return host.startsWith(AuthenticationConstants.Broker.AAD_URL_HOST_PREFIX) ||
81-
host.startsWith(AuthenticationConstants.Broker.MSA_URL_HOST_PREFIX)
76+
// Otherwise, make sure uri is a valid uri
77+
// We only want to allow URIs that have the AAD or MSA uri hosts
78+
return host.endsWith(AuthenticationConstants.Broker.AAD_GLOBAL_URL_HOST_SUFFIX) ||
79+
host.endsWith(AuthenticationConstants.Broker.AAD_INTUNE_MDM_URL_HOST_SUFFIX) ||
80+
host.endsWith(AuthenticationConstants.Broker.AAD_US_URL_HOST_SUFFIX) ||
81+
host.endsWith(AuthenticationConstants.Broker.AAD_CHINA_URL_HOST_SUFFIX)
8282
}
8383
}
8484

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

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,6 @@ public class AzureActiveDirectoryWebViewClient extends OAuth2WebViewClient {
134134
private final SwitchBrowserRequestHandler mSwitchBrowserRequestHandler;
135135
private HashMap<String, String> mRequestHeaders;
136136
private String mRequestUrl;
137-
private boolean mAuthUxJavaScriptInterfaceAdded = false;
138137
private boolean mInWebCpFlow = false;
139138

140139
private final String mUtid;
@@ -166,12 +165,6 @@ public void initializeAuthUxJavaScriptApi(@NonNull final WebView view, final Str
166165
}
167166
}
168167

169-
private boolean shouldExposeJavaScriptInterface(final String url) {
170-
return ProcessUtil.isRunningOnAuthService(getActivity().getApplicationContext())
171-
&& AuthUxJavaScriptInterface.Companion.isValidUrlForInterface(url)
172-
&& CommonFlightsManager.INSTANCE.getFlightsProvider().isFlightEnabled(CommonFlight.ENABLE_JS_API_FOR_AUTHUX);
173-
}
174-
175168
@Override
176169
public void onPageFinished(final WebView view,
177170
final String url) {
@@ -249,19 +242,6 @@ private boolean handleUrl(final WebView view, final String url) {
249242
final String methodTag = TAG + ":handleUrl";
250243
final String formattedURL = url.toLowerCase(Locale.US);
251244

252-
// Re-evaluate adding AuthUx JavaScript Interface
253-
if (shouldExposeJavaScriptInterface(url)) {
254-
// If broker request, and a valid url, expose JavaScript API
255-
Logger.info(methodTag, "Adding AuthUx JavaScript Interface");
256-
view.addJavascriptInterface(new AuthUxJavaScriptInterface(), AuthUxJavaScriptInterface.Companion.getInterfaceName());
257-
mAuthUxJavaScriptInterfaceAdded = true;
258-
} else if (mAuthUxJavaScriptInterfaceAdded) {
259-
// Remove AuthUx JavaScript Interface
260-
Logger.info(methodTag, "Removing AuthUx JavaScript Interface");
261-
view.removeJavascriptInterface(AuthUxJavaScriptInterface.Companion.getInterfaceName());
262-
mAuthUxJavaScriptInterfaceAdded = false;
263-
}
264-
265245
try {
266246
if (isPkeyAuthUrl(formattedURL)) {
267247
Logger.info(methodTag,"WebView detected request for pkeyauth challenge.");

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

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
import androidx.annotation.VisibleForTesting;
4343

4444
import com.microsoft.identity.common.adal.internal.AuthenticationConstants;
45+
import com.microsoft.identity.common.internal.broker.AuthUxJavaScriptInterface;
4546
import com.microsoft.identity.common.internal.ui.webview.challengehandlers.ChallengeFactory;
4647
import com.microsoft.identity.common.internal.ui.webview.challengehandlers.IChallengeHandler;
4748
import com.microsoft.identity.common.internal.ui.webview.challengehandlers.NtlmChallenge;
@@ -50,7 +51,6 @@
5051
import com.microsoft.identity.common.java.exception.ClientException;
5152
import com.microsoft.identity.common.java.flighting.CommonFlight;
5253
import com.microsoft.identity.common.java.flighting.CommonFlightsManager;
53-
import com.microsoft.identity.common.java.logging.DiagnosticContext;
5454
import com.microsoft.identity.common.java.opentelemetry.AttributeName;
5555
import com.microsoft.identity.common.java.opentelemetry.OTelUtility;
5656
import com.microsoft.identity.common.java.providers.RawAuthorizationResult;
@@ -78,6 +78,8 @@ public abstract class OAuth2WebViewClient extends WebViewClient {
7878
@VisibleForTesting
7979
public static ExpectedPage mExpectedPage = null;
8080

81+
protected boolean mAuthUxJavaScriptInterfaceAdded = false;
82+
8183
/**
8284
* @return context
8385
*/
@@ -222,6 +224,20 @@ public void onPageStarted(final WebView view,
222224
final Bitmap favicon) {
223225
final String methodTag = TAG + ":onPageStarted";
224226
checkStartUrl(url);
227+
228+
// Re-evaluate adding AuthUx JavaScript Interface
229+
if (shouldExposeJavaScriptInterface(url)) {
230+
// If broker request, and a valid url, expose JavaScript API
231+
Logger.info(methodTag, "Adding AuthUx JavaScript Interface");
232+
view.addJavascriptInterface(new AuthUxJavaScriptInterface(), AuthUxJavaScriptInterface.Companion.getInterfaceName());
233+
mAuthUxJavaScriptInterfaceAdded = true;
234+
} else if (mAuthUxJavaScriptInterfaceAdded) {
235+
// Remove AuthUx JavaScript Interface
236+
Logger.info(methodTag, "Removing AuthUx JavaScript Interface");
237+
view.removeJavascriptInterface(AuthUxJavaScriptInterface.Companion.getInterfaceName());
238+
mAuthUxJavaScriptInterfaceAdded = false;
239+
}
240+
225241
Logger.info(methodTag,"WebView starts loading.");
226242
super.onPageStarted(view, url, favicon);
227243
}
@@ -248,4 +264,10 @@ private void checkStartUrl(final String url) {
248264
+ " Path: " + uri.getPath());
249265
}
250266
}
267+
268+
protected boolean shouldExposeJavaScriptInterface(final String url) {
269+
return ProcessUtil.isRunningOnAuthService(getActivity().getApplicationContext())
270+
&& AuthUxJavaScriptInterface.Companion.isValidUriForInterface(url)
271+
&& CommonFlightsManager.INSTANCE.getFlightsProvider().isFlightEnabled(CommonFlight.ENABLE_JS_API_FOR_AUTHUX);
272+
}
251273
}

common/src/test/java/com/microsoft/identity/common/internal/broker/AuthUxJavaScriptInterfaceTest.kt

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -86,32 +86,38 @@ class AuthUxJavaScriptInterfaceTest {
8686
}
8787

8888
@Test
89-
fun `test isValidUrlForInterface with valid AAD URL`() {
89+
fun `test isValidUrlForInterface with valid AAD Global URL`() {
9090
val validUrl = "https://login.microsoftonline.com/common/oauth2/authorize"
91-
Assert.assertTrue(AuthUxJavaScriptInterface.isValidUrlForInterface(validUrl))
91+
Assert.assertTrue(AuthUxJavaScriptInterface.isValidUriForInterface(validUrl))
9292
}
9393

9494
@Test
95-
fun `test isValidUrlForInterface with valid MSA URL`() {
96-
val validUrl = "https://login.live.com/oauth20_authorize.srf"
97-
Assert.assertTrue(AuthUxJavaScriptInterface.isValidUrlForInterface(validUrl))
95+
fun `test isValidUrlForInterface with valid AAD US URL`() {
96+
val validUrl = "https://login.microsoftonline.us/common/oauth2/authorize"
97+
Assert.assertTrue(AuthUxJavaScriptInterface.isValidUriForInterface(validUrl))
98+
}
99+
100+
@Test
101+
fun `test isValidUrlForInterface with valid AAD China URL`() {
102+
val validUrl = "https://login.microsoftonline.cn/common/oauth2/authorize"
103+
Assert.assertTrue(AuthUxJavaScriptInterface.isValidUriForInterface(validUrl))
98104
}
99105

100106
@Test
101107
fun `test isValidUrlForInterface with null URL`() {
102108
val nullUrl: String? = null
103-
Assert.assertFalse(AuthUxJavaScriptInterface.isValidUrlForInterface(nullUrl))
109+
Assert.assertFalse(AuthUxJavaScriptInterface.isValidUriForInterface(nullUrl))
104110
}
105111

106112
@Test
107113
fun `test isValidUrlForInterface with invalid URL`() {
108114
val invalidUrl = "https://example.com"
109-
Assert.assertFalse(AuthUxJavaScriptInterface.isValidUrlForInterface(invalidUrl))
115+
Assert.assertFalse(AuthUxJavaScriptInterface.isValidUriForInterface(invalidUrl))
110116
}
111117

112118
@Test
113119
fun `test isValidUrlForInterface with empty URL`() {
114120
val emptyUrl = ""
115-
Assert.assertFalse(AuthUxJavaScriptInterface.isValidUrlForInterface(emptyUrl))
121+
Assert.assertFalse(AuthUxJavaScriptInterface.isValidUriForInterface(emptyUrl))
116122
}
117123
}

0 commit comments

Comments
 (0)