Skip to content

Commit a232b77

Browse files
authored
Fixing error in construction of LegacyFido2ApiManager (#2654)
### Summary While testing for their passkey enablement rollout, the Office team found a bug where if they're using OneAuth with fragment or dialog mode (and thus using their own Activity instead of AuthorizationActivity), they run into a casting exception in the process of creating a LegacyFido2ApiManager, as we need to pass a WebViewAuthorizationFragment into the constructor. The devices which would run into this error would be: - Running on Android 13 - Non-brokered auth - Are using OneAuth, and specifically using fragment/dialog mode - According to OneAuth, this is currently only being used by the Office team, which is why the other apps which rolled out passkey support have not run into this issue. The fix for this is to add explicit type checks before creating the LegacyFido2ApiManager. Because the legacy FIDO2 API does require a Fragment which has a particular object instantiated (which I had added into our WebViewAuthorizationFragment), we can't just pass any fragment from any activity. This means that for Office's case, for Android 13 and below devices, they are going to only use CredMan- which is actually a good thing, since CredMan now has support for security keys for all OS versions (we originally included the legacy API because this wasn't the case a while ago). At some point, it would be best to just remove the legacy API logic, but that can be done once a larger majority of users are on Android 14+. - Company Portal on Android 13 or below with security key -> Passed on real device (thanks to Ben) - Company Portal on Android 13 or below without passkey -> passed on emulator; could still see the passkey dialogs - Company Portal on Android 14 -> passed - Office on Android 13 -> (Waiting for response) - OneAuthTestApp on Android 13 -> (Waiting for response) - MsalTestApp on Android 13 -> Passed on emulator - Test basic brokered and nonbrokered scenarios on Android 14 -> Passed on real device Noting again that brokered auth scenarios should remain unaffected because the legacy API path was never hit (the flight has remained off, and default value is false). The same should be the case for Android 14+.
1 parent f47b86d commit a232b77

File tree

4 files changed

+50
-5
lines changed

4 files changed

+50
-5
lines changed

changelog.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ vNext
44
- [MINOR] Add Get AAD device ID API for resource account (#2644)
55
- [MINOR] Expose a JavaScript API in brokered Webviews to facilitate Improved Same Device NumberMatch (#2617)
66
- [MINOR] Add API for resource account provisioning (API only) (#2640)
7+
- [PATCH] Fixing error in construction of LegacyFido2ApiManager (#2654)
78

89
Version 21.1.0
910
----------

common/src/main/java/com/microsoft/identity/common/internal/fido/CredManFidoManager.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ class CredManFidoManager (val context: Context,
8484
// We're setting preferImmediatelyAvailableCredentials in the CredMan getCredentialRequest object to true if the device OS is Android 13 or lower.
8585
// This ensures the behavior where no dialog from CredMan is shown if no passkey cred is present.
8686
// The end goal is for an Android <= 13 user who only has a security key to see one dialog which will allow them to authenticate.
87-
preferImmediatelyAvailableCredentials = (getFlightsProvider().isFlightEnabled(CommonFlight.ENABLE_LEGACY_FIDO_SECURITY_KEY_LOGIC) && Build.VERSION.SDK_INT < Build.VERSION_CODES.UPSIDE_DOWN_CAKE)
87+
preferImmediatelyAvailableCredentials = (legacyManager != null && getFlightsProvider().isFlightEnabled(CommonFlight.ENABLE_LEGACY_FIDO_SECURITY_KEY_LOGIC) && Build.VERSION.SDK_INT < Build.VERSION_CODES.UPSIDE_DOWN_CAKE)
8888
)
8989
try {
9090
Logger.info(methodTag, "Calling Credential Manager with a GetCredentialRequest.")

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

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -235,11 +235,16 @@ private boolean handleUrl(final WebView view, final String url) {
235235
} else if (CommonFlightsManager.INSTANCE.getFlightsProvider().isFlightEnabled(CommonFlight.ENABLE_PASSKEY_FEATURE) && isPasskeyUrl(formattedURL)) {
236236
Logger.info(methodTag,"WebView detected request for passkey protocol.");
237237
final FidoChallenge challenge = FidoChallenge.createFromRedirectUri(url);
238-
final SpanContext spanContext = getActivity() instanceof AuthorizationActivity ? ((AuthorizationActivity)getActivity()).getSpanContext() : null;
238+
final Activity currentActivity = getActivity();
239+
final SpanContext spanContext = currentActivity instanceof AuthorizationActivity ? ((AuthorizationActivity)currentActivity).getSpanContext() : null;
240+
// The legacyManager should only be getting added if the device is on Android 13 or lower and the library is MSAL/OneAuth with fragment or dialog mode.
241+
// The legacyManager logic should be removed once a larger majority of users are on Android 14+.
239242
final IFidoManager legacyManager =
240-
CommonFlightsManager.INSTANCE.getFlightsProvider().isFlightEnabled(CommonFlight.ENABLE_LEGACY_FIDO_SECURITY_KEY_LOGIC)
243+
currentActivity instanceof AuthorizationActivity
244+
&& ((AuthorizationActivity) currentActivity).getFragment() instanceof WebViewAuthorizationFragment
245+
&& CommonFlightsManager.INSTANCE.getFlightsProvider().isFlightEnabled(CommonFlight.ENABLE_LEGACY_FIDO_SECURITY_KEY_LOGIC)
241246
&& Build.VERSION.SDK_INT < Build.VERSION_CODES.UPSIDE_DOWN_CAKE
242-
? new LegacyFido2ApiManager(view.getContext(), (WebViewAuthorizationFragment)((AuthorizationActivity)getActivity()).getFragment())
247+
? new LegacyFido2ApiManager(view.getContext(), (WebViewAuthorizationFragment)((AuthorizationActivity)currentActivity).getFragment())
243248
: null;
244249
final AuthFidoChallengeHandler challengeHandler = new AuthFidoChallengeHandler(
245250
new CredManFidoManager(

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

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import androidx.test.core.app.ApplicationProvider;
3131

3232
import com.microsoft.identity.common.adal.internal.AuthenticationConstants;
33+
import com.microsoft.identity.common.internal.ui.DualScreenActivity;
3334
import com.microsoft.identity.common.internal.ui.webview.challengehandlers.CrossCloudChallengeHandler;
3435
import com.microsoft.identity.common.java.exception.ClientException;
3536
import com.microsoft.identity.common.java.providers.microsoft.azureactivedirectory.AzureActiveDirectory;
@@ -90,6 +91,7 @@ public class AzureActiveDirectoryWebViewClientTest {
9091
private static final String TEST_NONCE_REDIRECT_URL = "https://login.microsoftonline.com/organizations/oAuth2/v2.0/authorize?&sso_nonce=ABCD";
9192
private static final String TEST_CROSS_CLOUD_REDIRECT_URL = "https://login.microsoftonline.us/organizations/oAuth2/v2.0/authorize?x=10";
9293
private static final String TEST_PUBLIC_CLOUD_REDIRECT_URL = "https://login.microsoftonline.com/organizations/oAuth2/v2.0/authorize?x=10";
94+
private static final String TEST_PASSKEY_REDIRECT_URL = "http-auth:PassKey?challenge=challenge&version=1.0&submitUrl=https://login.microsoftonline.com/common/credential?passKeyAuth=1.0%2fpasskey&context=&relyingPartyIdentifier=login.microsoft.com&allowedCredentials=somevalue";
9395
private static final String TEST_INTENT_INSTALL_BROKER_REDIRECT_URL = "intent://play.google.com/store/apps/details?id=com.azure.authenticator&referrer=%20adjust_reftag%3Dc6f1p4ErudH2C%26utm_source%3DLanding%2BPage%2BOrganic%2B-%2Bapp%2Bstore%2Bbadges%26utm_campaign%3Dappstore_android&pcampaignid=web_auto_redirect&web_logged_in=0&redirect_entry_point=dp#Intent;scheme=https;action=android.intent.action.VIEW;package=com.android.vending;end";
9496

9597
@Before
@@ -236,8 +238,45 @@ public void testProcessCloudRedirectAndPrtHeaderInternalException() {
236238
}
237239
}
238240

239-
@Test
240241
public void testUrlOverrideHandlesIntentRedirectUrl() {
241242
assertTrue(mWebViewClient.shouldOverrideUrlLoading(mMockWebView, TEST_INTENT_INSTALL_BROKER_REDIRECT_URL));
242243
}
244+
245+
public void setTestPasskeyRedirectUrl() {
246+
assertTrue(mWebViewClient.shouldOverrideUrlLoading(mMockWebView, TEST_PASSKEY_REDIRECT_URL));
247+
}
248+
249+
@Test
250+
public void testPasskeyActivityCastingNoException() {
251+
try {
252+
// Putting an explicit non-AuthorizationActivity activity here to test that an exception won't be thrown.
253+
mActivity = Robolectric.buildActivity(DualScreenActivity.class).get();
254+
mWebViewClient = new AzureActiveDirectoryWebViewClient(
255+
mActivity,
256+
new IAuthorizationCompletionCallback() {
257+
@Override
258+
public void onChallengeResponseReceived(@NonNull RawAuthorizationResult response) {
259+
260+
}
261+
262+
@Override
263+
public void setPKeyAuthStatus(boolean status) {
264+
return;
265+
}
266+
},
267+
new OnPageLoadedCallback() {
268+
@Override
269+
public void onPageLoaded(final String url) {
270+
return;
271+
}
272+
},
273+
TEST_REDIRECT_URI,
274+
Mockito.mock(SwitchBrowserRequestHandler.class));
275+
mWebViewClient.shouldOverrideUrlLoading(mMockWebView, TEST_PASSKEY_REDIRECT_URL);
276+
} catch (ClassCastException e) {
277+
Assert.fail("Failure is not expected. The class checks should have prevented this." + e);
278+
} catch (Exception e) {
279+
Assert.fail("Failure is not expected." + e);
280+
}
281+
}
243282
}

0 commit comments

Comments
 (0)