Updating handling of ssl error received in Android WebView's onReceivedSslError callback, Fixes AB#3268908

changelog.txt

@@ -1,5 +1,6 @@
 vNext
 ----------
+- [MINOR] Updating handling of ssl error received in Android WebView's onReceivedSslError callback (#2691)
 - [MINOR] Fixing the sign in screens when edge to edge is enabled (#2665)
 - [MINOR] Showing webcp flow in webview (#2673)
 - [MINOR] Native auth: Make native auth MFA feature more backward compatible(#2669)

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

@@ -22,6 +22,16 @@
 // THE SOFTWARE.
 package com.microsoft.identity.common.internal.providers.oauth2;
 
+import static com.microsoft.identity.common.adal.internal.AuthenticationConstants.AuthorizationIntentKey.AUTH_INTENT;
+import static com.microsoft.identity.common.adal.internal.AuthenticationConstants.AuthorizationIntentKey.POST_PAGE_LOADED_URL;
+import static com.microsoft.identity.common.adal.internal.AuthenticationConstants.AuthorizationIntentKey.REDIRECT_URI;
+import static com.microsoft.identity.common.adal.internal.AuthenticationConstants.AuthorizationIntentKey.REQUEST_HEADERS;
+import static com.microsoft.identity.common.adal.internal.AuthenticationConstants.AuthorizationIntentKey.REQUEST_URL;
+import static com.microsoft.identity.common.adal.internal.AuthenticationConstants.AuthorizationIntentKey.WEB_VIEW_ZOOM_CONTROLS_ENABLED;
+import static com.microsoft.identity.common.adal.internal.AuthenticationConstants.AuthorizationIntentKey.WEB_VIEW_ZOOM_ENABLED;
+import static com.microsoft.identity.common.java.AuthenticationConstants.SdkPlatformFields.PRODUCT;
+import static com.microsoft.identity.common.java.AuthenticationConstants.SdkPlatformFields.VERSION;
+
 import android.annotation.SuppressLint;
 import android.app.Activity;
 import android.content.Context;
@@ -46,40 +56,30 @@
 import androidx.fragment.app.FragmentActivity;
 
 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.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;
 import com.microsoft.identity.common.adal.internal.util.StringExtensions;
+import com.microsoft.identity.common.internal.fido.LegacyFido2ApiObject;
+import com.microsoft.identity.common.internal.fido.LegacyFidoActivityResultContract;
 import com.microsoft.identity.common.internal.ui.webview.AzureActiveDirectoryWebViewClient;
+import com.microsoft.identity.common.internal.ui.webview.ISendResultCallback;
 import com.microsoft.identity.common.internal.ui.webview.OnPageLoadedCallback;
+import com.microsoft.identity.common.internal.ui.webview.ProcessUtil;
 import com.microsoft.identity.common.internal.ui.webview.WebViewUtil;
+import com.microsoft.identity.common.internal.ui.webview.switchbrowser.SwitchBrowserProtocolCoordinator;
+import com.microsoft.identity.common.java.WarningType;
 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.flighting.CommonFlightsManager;
-import com.microsoft.identity.common.java.ui.webview.authorization.IAuthorizationCompletionCallback;
 import com.microsoft.identity.common.java.providers.RawAuthorizationResult;
+import com.microsoft.identity.common.java.ui.webview.authorization.IAuthorizationCompletionCallback;
 import com.microsoft.identity.common.java.util.ClientExtraSku;
 import com.microsoft.identity.common.logging.Logger;
 
-import java.util.Arrays;
-import java.util.HashMap;
 import java.io.UnsupportedEncodingException;
 import java.net.URLEncoder;
-
-import static com.microsoft.identity.common.adal.internal.AuthenticationConstants.AuthorizationIntentKey.AUTH_INTENT;
-import static com.microsoft.identity.common.adal.internal.AuthenticationConstants.AuthorizationIntentKey.POST_PAGE_LOADED_URL;
-import static com.microsoft.identity.common.adal.internal.AuthenticationConstants.AuthorizationIntentKey.REDIRECT_URI;
-import static com.microsoft.identity.common.adal.internal.AuthenticationConstants.AuthorizationIntentKey.REQUEST_HEADERS;
-import static com.microsoft.identity.common.adal.internal.AuthenticationConstants.AuthorizationIntentKey.REQUEST_URL;
-import static com.microsoft.identity.common.adal.internal.AuthenticationConstants.AuthorizationIntentKey.WEB_VIEW_ZOOM_CONTROLS_ENABLED;
-import static com.microsoft.identity.common.adal.internal.AuthenticationConstants.AuthorizationIntentKey.WEB_VIEW_ZOOM_ENABLED;
-import static com.microsoft.identity.common.java.AuthenticationConstants.SdkPlatformFields.PRODUCT;
-import static com.microsoft.identity.common.java.AuthenticationConstants.SdkPlatformFields.VERSION;
+import java.util.Arrays;
+import java.util.HashMap;
 
 import io.opentelemetry.api.trace.SpanContext;
 

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

@@ -22,6 +22,8 @@
 // THE SOFTWARE.
 package com.microsoft.identity.common.internal.ui.webview;
 
+import static com.microsoft.identity.common.adal.internal.AuthenticationConstants.Browser.SSL_HELP_URL;
+
 import android.app.Activity;
 import android.graphics.Bitmap;
 import android.net.Uri;
@@ -41,23 +43,33 @@
 
 import com.microsoft.identity.common.adal.internal.AuthenticationConstants;
 import com.microsoft.identity.common.internal.ui.webview.challengehandlers.ChallengeFactory;
-import com.microsoft.identity.common.java.ui.webview.authorization.IAuthorizationCompletionCallback;
 import com.microsoft.identity.common.internal.ui.webview.challengehandlers.IChallengeHandler;
 import com.microsoft.identity.common.internal.ui.webview.challengehandlers.NtlmChallenge;
 import com.microsoft.identity.common.internal.ui.webview.challengehandlers.NtlmChallengeHandler;
 import com.microsoft.identity.common.internal.util.StringUtil;
 import com.microsoft.identity.common.java.exception.ClientException;
+import com.microsoft.identity.common.java.flighting.CommonFlight;
+import com.microsoft.identity.common.java.flighting.CommonFlightsManager;
+import com.microsoft.identity.common.java.logging.DiagnosticContext;
+import com.microsoft.identity.common.java.opentelemetry.AttributeName;
+import com.microsoft.identity.common.java.opentelemetry.OTelUtility;
 import com.microsoft.identity.common.java.providers.RawAuthorizationResult;
+import com.microsoft.identity.common.java.ui.webview.authorization.IAuthorizationCompletionCallback;
 import com.microsoft.identity.common.logging.Logger;
 
-import static com.microsoft.identity.common.adal.internal.AuthenticationConstants.Browser.SSL_HELP_URL;
-
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+import io.opentelemetry.api.common.Attributes;
+import io.opentelemetry.api.metrics.LongCounter;
 
 public abstract class OAuth2WebViewClient extends WebViewClient {
     /* constants */
     private static final String TAG = OAuth2WebViewClient.class.getSimpleName();
 
+    private static final LongCounter sWebViewSslErrorCount = OTelUtility.createLongCounter(
+            "web_view_ssl_error_count",
+            "Number of SSL errors received in onReceivedSslError"
+    );
+
     private final IAuthorizationCompletionCallback mCompletionCallback;
     private final OnPageLoadedCallback mPageLoadedCallback;
     private final Activity mActivity;
@@ -89,12 +101,13 @@ IAuthorizationCompletionCallback getCompletionCallback() {
      */
     OAuth2WebViewClient(@NonNull final Activity activity,
                         @NonNull final IAuthorizationCompletionCallback completionCallback,
-                        @NonNull final OnPageLoadedCallback pageLoadedCallback) {
-        //the validation of redirect url and authorization request should be in upper level before launching the webview.
+                        @NonNull final OnPageLoadedCallback pageLoadedCallback
+    ) {
+        // the validation of redirect url and authorization request should be in upper level before launching the webview.
         mActivity = activity;
         mCompletionCallback = completionCallback;
         mPageLoadedCallback = pageLoadedCallback;
-     }
+    }
 
     @Override
     public void onReceivedHttpAuthRequest(WebView view, final HttpAuthHandler handler,
@@ -171,17 +184,21 @@ public void onReceivedSslError(final WebView view,
                                    final SslErrorHandler handler,
                                    final SslError error) {
         // Developer does not have option to control this for now
+        final String methodTag = TAG + ":onReceivedSslError";
         super.onReceivedSslError(view, handler, error);
-        handler.cancel();
-
-        final String errMsg = "Received SSL Error during request. For more info see: " + SSL_HELP_URL;
-
-        Logger.error(TAG + ":onReceivedSslError", errMsg, null);
-
-        // Send the result back to the calling activity
-        mCompletionCallback.onChallengeResponseReceived(
-                RawAuthorizationResult.fromException(
-                        new ClientException("Code:" + ERROR_FAILED_SSL_HANDSHAKE, error.toString())));
+        final String errMsg = "Received SSL Error during request. For more info see: " + SSL_HELP_URL + ". Error: " + error.toString();
+
+        Logger.warn(methodTag, errMsg);
+        final Attributes attributes = Attributes.builder()
+                .put(AttributeName.web_view_ssl_primary_error_code.name(), error.getPrimaryError())
+                .build();
+        sWebViewSslErrorCount.add(1, attributes);
+        if (!CommonFlightsManager.INSTANCE.getFlightsProvider().isFlightEnabled(CommonFlight.SHOULD_PRESERVE_WEBVIEW_FLOW_ON_SSL_ERROR)) {
+            // Send the result back to the calling activity
+            mCompletionCallback.onChallengeResponseReceived(
+                    RawAuthorizationResult.fromException(
+                            new ClientException("Code:" + ERROR_FAILED_SSL_HANDSHAKE, error.toString())));
+        }
     }
 
     @Override

common/src/test/java/com/microsoft/identity/common/internal/ui/webview/AzureActiveDirectoryWebViewClientTest.java

@@ -22,8 +22,20 @@
 // THE SOFTWARE.
 package com.microsoft.identity.common.internal.ui.webview;
 
+import static com.microsoft.identity.common.adal.internal.AuthenticationConstants.Broker.AUTHENTICATOR_MFA_LINKING_PREFIX;
+import static com.microsoft.identity.common.adal.internal.AuthenticationConstants.Broker.COMPANY_PORTAL_APP_PACKAGE_NAME;
+import static com.microsoft.identity.common.adal.internal.AuthenticationConstants.Broker.PLAY_STORE_INSTALL_PREFIX;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.when;
+
 import android.app.Activity;
 import android.content.Context;
+import android.net.http.SslError;
+import android.webkit.SslErrorHandler;
 import android.webkit.WebView;
 
 import androidx.annotation.NonNull;
@@ -33,14 +45,15 @@
 import com.microsoft.identity.common.internal.mocks.MockCommonFlightsManager;
 import com.microsoft.identity.common.internal.ui.DualScreenActivity;
 import com.microsoft.identity.common.internal.ui.webview.challengehandlers.ReAttachPrtHeaderHandler;
+import com.microsoft.identity.common.internal.ui.webview.challengehandlers.SwitchBrowserRequestHandler;
 import com.microsoft.identity.common.java.exception.ClientException;
 import com.microsoft.identity.common.java.flighting.CommonFlight;
 import com.microsoft.identity.common.java.flighting.CommonFlightsManager;
+import com.microsoft.identity.common.java.flighting.IFlightsManager;
 import com.microsoft.identity.common.java.flighting.IFlightsProvider;
+import com.microsoft.identity.common.java.providers.RawAuthorizationResult;
 import com.microsoft.identity.common.java.providers.microsoft.azureactivedirectory.AzureActiveDirectory;
-import com.microsoft.identity.common.internal.ui.webview.challengehandlers.SwitchBrowserRequestHandler;
 import com.microsoft.identity.common.java.ui.webview.authorization.IAuthorizationCompletionCallback;
-import com.microsoft.identity.common.java.providers.RawAuthorizationResult;
 import com.microsoft.identity.common.shadows.ShadowProcessUtil;
 
 import org.junit.Assert;
@@ -52,18 +65,13 @@
 import org.robolectric.RobolectricTestRunner;
 import org.robolectric.annotation.Config;
 
-import static com.microsoft.identity.common.adal.internal.AuthenticationConstants.Broker.AUTHENTICATOR_MFA_LINKING_PREFIX;
-import static com.microsoft.identity.common.adal.internal.AuthenticationConstants.Broker.COMPANY_PORTAL_APP_PACKAGE_NAME;
-import static com.microsoft.identity.common.adal.internal.AuthenticationConstants.Broker.PLAY_STORE_INSTALL_PREFIX;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
-import static org.mockito.Mockito.when;
-
 import java.util.HashMap;
 
 import io.opentelemetry.api.trace.Span;
 
-
+/**
+ * Tests for {@link AzureActiveDirectoryWebViewClient}.
+ */
 @RunWith(RobolectricTestRunner.class)
 public class AzureActiveDirectoryWebViewClientTest {
     private WebView mMockWebView;
@@ -129,7 +137,8 @@ public void onPageLoaded(final String url) {
                     }
                 },
                 TEST_REDIRECT_URI,
-                Mockito.mock(SwitchBrowserRequestHandler.class));
+                Mockito.mock(SwitchBrowserRequestHandler.class)
+        );
         HashMap<String, String> dummyHeaders = new HashMap<>();
         dummyHeaders.put("key", "value");
         mWebViewClient.setRequestHeaders(dummyHeaders);
@@ -374,12 +383,67 @@ public void onPageLoaded(final String url) {
                         }
                     },
                     TEST_REDIRECT_URI,
-                    Mockito.mock(SwitchBrowserRequestHandler.class));
+                    Mockito.mock(SwitchBrowserRequestHandler.class)
+            );
             mWebViewClient.shouldOverrideUrlLoading(mMockWebView, TEST_PASSKEY_REDIRECT_URL);
         } catch (ClassCastException e) {
             Assert.fail("Failure is not expected. The class checks should have prevented this." + e);
         } catch (Exception e) {
             Assert.fail("Failure is not expected." + e);
         }
     }
+
+    @Test
+    public void testOnReceivedSslError_Legacy() {
+        final String mockActiveUrl = "https://login.microsoftonline.com/organizations/oAuth2/v2.0/authorize";
+        final SslErrorHandler mockHandler = Mockito.mock(android.webkit.SslErrorHandler.class);
+        final SslError mockError = Mockito.mock(android.net.http.SslError.class);
+        final IAuthorizationCompletionCallback mockCallback = Mockito.mock(IAuthorizationCompletionCallback.class);
+        when(mockError.getUrl()).thenReturn("https://example.com");
+        final AzureActiveDirectoryWebViewClient mockWebViewClient = new AzureActiveDirectoryWebViewClient(
+                mActivity,
+                mockCallback,
+                url -> {},
+                TEST_REDIRECT_URI,
+                Mockito.mock(SwitchBrowserRequestHandler.class));
+        final WebView mockWebView = new WebView(mContext);
+        mockWebView.setWebViewClient(mockWebViewClient);
+
+        // act
+        mockWebViewClient.onReceivedSslError(mockWebView, mockHandler, mockError);
+
+        Mockito.verify(mockHandler, Mockito.times(1)).cancel();
+        Mockito.verify(mockCallback, Mockito.times(1)).onChallengeResponseReceived(any());
+    }
+
+    @Test
+    public void testOnReceivedSslError() {
+        final IFlightsManager mockFlightsManager = Mockito.mock(IFlightsManager.class);
+        final IFlightsProvider mockFlightsProvider = Mockito.mock(IFlightsProvider.class);
+        when(mockFlightsProvider.isFlightEnabled(eq(CommonFlight.SHOULD_PRESERVE_WEBVIEW_FLOW_ON_SSL_ERROR))).thenReturn(true);
+        when(mockFlightsManager.getFlightsProvider()).thenReturn(mockFlightsProvider);
+        CommonFlightsManager.INSTANCE.initializeCommonFlightsManager(mockFlightsManager);
+        final SslErrorHandler mockHandler = Mockito.mock(android.webkit.SslErrorHandler.class);
+        final SslError mockError = Mockito.mock(android.net.http.SslError.class);
+        final IAuthorizationCompletionCallback mockCallback = Mockito.mock(IAuthorizationCompletionCallback.class);
+        when(mockError.getUrl()).thenReturn("https://example.com");
+        final AzureActiveDirectoryWebViewClient mockWebViewClient = new AzureActiveDirectoryWebViewClient(
+                mActivity,
+                mockCallback,
+                url -> {},
+                TEST_REDIRECT_URI,
+                Mockito.mock(SwitchBrowserRequestHandler.class)
+        );
+        final WebView mockWebView = new WebView(mContext);
+        mockWebView.setWebViewClient(mockWebViewClient);
+
+        // act
+        mockWebViewClient.onReceivedSslError(mockWebView, mockHandler, mockError);
+
+        // verify that the handler is cancelled and the callback is invoked
+        Mockito.verify(mockHandler, Mockito.times(1)).cancel();
+        Mockito.verify(mockCallback, never()).onChallengeResponseReceived(any());
+
+        CommonFlightsManager.INSTANCE.resetFlightsManager();
+    }
 }

common4j/src/main/com/microsoft/identity/common/java/flighting/CommonFlight.java

@@ -139,7 +139,13 @@ public enum CommonFlight implements IFlightConfig {
     /**
      * Flight to enable the Web CP for a tenant list.
      */
-    TENANT_LIST_TO_ENABLE_WEB_CP_IN_WEBVIEW("TenantListToEnableWebCpInWebView", "");
+    TENANT_LIST_TO_ENABLE_WEB_CP_IN_WEBVIEW("TenantListToEnableWebCpInWebView", ""),
+
+    /**
+     * Flight to enable the WebView flow to not cancel and preserve WebView flow on SSL errors.
+     * The web resource running into SSL will itself not be loaded.
+     */
+    SHOULD_PRESERVE_WEBVIEW_FLOW_ON_SSL_ERROR("ShouldPreserveWebViewFlowOnSslError", false);
 
     private String key;
     private Object defaultValue;

common4j/src/main/com/microsoft/identity/common/java/opentelemetry/AttributeName.java

@@ -378,5 +378,11 @@ public enum AttributeName {
     /**
      * Records if the webcp is enabled in webview.
      */
-    is_webcp_in_webview_enabled
+    is_webcp_in_webview_enabled,
+
+    /**
+     * Records the if webview received an SSL error and
+     * corresponding primary error code.
+     */
+    web_view_ssl_primary_error_code
 }