Skip to content

Conversation

@mohitc1
Copy link
Contributor

@mohitc1 mohitc1 commented Jun 25, 2025

Summary:
This PR updates the handling of SSL errors in the Android WebView used for authentication. The change is in response to an incident where the MSA sign-up flow failed due to an expired certificate on an MSA UX web page. The expired certificate was associated with a resource loaded by the page; however, the existing SSL error handling in Common’s WebView would cancel the loading of that resource and subsequently terminate the entire authentication flow. (See PBI for details.)

Solution
Option 1
We simply log and stop loading the resource that caused the error by calling SssHandler.cancel() but not stop the flow by erroring out. This is a simple change. The only thing is the main URL (the url loading in the WebView) itself has SSL error then the flow would end up in blank page. This is as bad as finishing the flow but does not inform calling app of the error via exception.

But if sub-resource has the error, then the flow continues.

This is a low cost solution and easy to implement with slightly poorer UX.

Option 2 (Changes in this PR)
The objective of this update is to improve SSL error handling so that the flow is only cancelled if an SSL error occurs on the main frame resource (the primary page being loaded). If an SSL error affects a sub-resource, only the loading of that specific resource is cancelled, and the overall authentication flow continues. This approach helps prevent unnecessary failures caused by SSL errors on secondary or non-critical resources.

Changes introduced:

  1. WebView and WebViewClient Update:

    • Introduces a new AzureActiveDirectoryWebView class, which extends Android's WebView and tracks the active URL being loaded, along with AzureActiveDirectoryWebViewClient
    • Tracking is done by maintaining the next URL (called active url) that's going to be loaded in WebView. We cannot fully rely on WebViewClient's callbacks like onPageStarted because webpage can run into error even before onPageStarted is called. So, we track the URL ourselves by
      1. Setting active Url to be loaded in shouldOverrideUrlLoading(..) callback. This is called before URL is about to get loaded
      2. When WebView's loadUrl is called explicitly. This is case where shouldOverrideUrlLoading() is not called. Since there are multiple places webView.loadUrl is called, added AzureActiveDirectoryWebView which keeps track of the URL being loaded in loadUrl and then continues with webView.loadUrl
  2. Handling error in onReceivedSslError

  • onReceivedSslError checks if the SSL error is for the main frame (i.e., the main page being loaded). It does so by comparing the active url against the error url on host/domain. If error is for main frame, it cancels the flow and calls the completion callback with an error; otherwise, it just cancels the faulty resource, and let the flow continue.
  • For purpose of enabling/disabling new flow, a boolean is added. This should be temporary till changes are stable.

Flighting

  1. New SSL Error Handler Toggle:
    • Adds a new intent key (WEB_VIEW_NEW_SSL_ERROR_HANDLER_ENABLED) in AuthenticationConstants.java to control whether the new SSL error handler logic is enabled. This can be used by OneAuth to opt-in or opt-out.
    • Adds a new flight value is well defaulting to true. If the key is not passed. If the key passed, that will be used (primarily meant for OneAuth/MSALCPP). If not, flight would be used (meant for Broker). MSAL would also use default flight value, as there's no remote flighting there.

@github-actions
Copy link

✅ Work item link check complete. Description contains link AB#3268908 to an Azure Boards work item.

@github-actions github-actions bot changed the title Updating handling of SSL error in WebView Updating handling of SSL error in WebView, Fixes AB#3268908 Jun 25, 2025
@mohitc1 mohitc1 added Skip-Consumers-Check Only include this if making a breaking change purposefully, and there is an MSAL/ADAL/Broker PR labels Jun 26, 2025
@github-actions
Copy link

❌ Work item link check failed. Description does not contain AB#{ID}.

Click here to Learn more.

mohitc1 added a commit that referenced this pull request Jun 30, 2025
…edSslError callback, Fixes AB#3268908 (#2691)

Update:
After talking to @iulico-1 from OneAuth.
1. First rolling out with Broker and keeping disabled for OneAuth (and
MSAL Android). Added SSL error telemetry
2. Enable for oneauth after we see no issues in Broker. So, the toggle
code to let OneAuth enable/disable is removed from the PR.
3. Investigate how we can pass the error to OneAuth to track SSL failure
and User canceled the flow because of SSL error.

=====================

Fixes
[AB#3268908](https://identitydivision.visualstudio.com/fac9d424-53d2-45c0-91b5-ef6ba7a6bf26/_workitems/edit/3268908)

**Summary:**  
This PR updates the handling of SSL errors in the Android WebView used
for authentication. The change is in response to an incident where the
MSA sign-up flow failed due to an expired certificate on an MSA UX web
page. The expired certificate was associated with a resource loaded by
the page; however, the existing SSL error handling in Common’s WebView
would cancel the loading of that resource and subsequently terminate the
entire authentication flow. (See PBI for details.)

**Solution**
**Option 1**
We simply log and stop loading the resource that caused the error by
calling `SssHandler.cancel()` but not stop the flow by erroring out.
This is a simple change. The only thing is the main URL (the url loading
in the WebView) itself has SSL error then the flow would end up in
*blank page*. This is as bad as finishing the flow but does not inform
calling app of the error via exception.

But if sub-resource has the error, then the flow continues.

**Flighting**
**New SSL Error Handler**: 
- ~~Adds a new intent key (`WEB_VIEW_NEW_SSL_ERROR_HANDLER_ENABLED`) in
`AuthenticationConstants.java` to control whether the new SSL error
handler logic is enabled. This can be used by OneAuth to opt-in or
opt-out. If the key is not passed, then Common's default logic based on
flighting will be used. If the key passed, that will be used (primarily
meant for OneAuth/MSALCPP).~~. The toggle would be added later if
required. This simplifies current logic to only rely on Common/broker's
flighting.
- Adds a new flight value is well defaulting to `false`. So, OneAuth and
MSAL would also use default flight value and continue using current
behavior. For Broker, we would enable new flow with default `true`
(AzureAD/ad-accounts-for-android#3141), which
can be controlled remotely to turn off if required.

**Option 2** (Not this PR, added for reference)
The objective of this update is to improve SSL error handling so that
the flow is only cancelled if an SSL error occurs on the main frame
resource (the primary page being loaded). If an SSL error affects a
sub-resource, only the loading of that specific resource is cancelled,
and the overall authentication flow continues. This approach helps
prevent unnecessary failures caused by SSL errors on secondary or
non-critical resources.
(#2688)

The con of the approach is client needs to track the active url that's
about and being loaded in WebView. The tracking adds cost of developing
and maintaining to solution without significant benefit over option 1.
@mohitc1 mohitc1 closed this Jul 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Skip-Consumers-Check Only include this if making a breaking change purposefully, and there is an MSAL/ADAL/Broker PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants