Feat: Filter allowed CustomTabs browsers #353

lbalmaceda · 2020-09-23T20:55:40Z

Changes

Introduces a BrowserPicker class that can be set through the CustomTabsOptions object used in the WebAuthProvider methods. By constructing a new instance, the developers would be able to filter the browsers used to launch the web auth flow.

Public API

Based on the linked PR, but using builders to make it extensible, more testable, and future proof.

//Step 1: Define the filter
BrowserPicker browserPicker = BrowserPicker.newBuilder()
    .withAllowedPackages(Arrays.asList("com.google.chrome"))
    .build();

//Step 2: Set the filter
CustomTabsOptions customTabsOptions = CustomTabsOptions.newBuilder()
    .withBrowserPicker(browserPicker)
    .build();

//Step 3: Handle possible browser errors in the callback
AuthCallback callback = new AuthCallback() {
    @Override
    public void onFailure(@NonNull Dialog dialog) {
        //...
    }

    @Override
    public void onFailure(@NonNull AuthenticationException exception) {
        if (exception.isBrowserAppNotAvailable()){
            //handle this, e.g. show dialog and suggest to install Chrome 
        }
    }

    @Override
    public void onSuccess(@NonNull Credentials credentials) {
        //...
    }
});

//Step 4: Launch the authentication
WebAuthProvider.init(account)
    .withCustomTabsOptions(customTabsOptions)
    .start(activity, callback);

If the user-selected default browser is custom tabs compatible and included in this "allowed packages" list, it will be picked. Otherwise, the order of the items in the "allowed packages" list matters and will be used to choose the first browser available that is contained there.

References

Supersedes #338

Testing

There is a lot of unit tests involved for keeping the legacy behavior untouched and for testing that the introduced filtering logic is there. I plan to test this manually before publishing it.

This change adds unit test coverage
This change adds integration test coverage
This change has been tested on the latest version of the platform/language or why not

Checklist

I have read the Auth0 general contribution guidelines
I have read the Auth0 Code of Conduct
All existing and new tests complete without errors

…in / Logout Adding support to override the browser packages to be used during Login / Logout

lbalmaceda

proposed public API, subject to change

//Define the filter
BrowserPicker browserPicker = BrowserPicker.newBuilder()
    .withAllowedPackages(Arrays.asList("com.google.chrome"))
    .build();
//Set the filter
CustomTabsOptions customTabsOptions = CustomTabsOptions.newBuilder()
    .withBrowserPicker(browserPicker)
    .build();
//Launch the authentication
WebAuthProvider.init(account)
    .withCustomTabsOptions(customTabsOptions)
    .start(activity, callback, REQUEST_CODE);

auth0/src/main/java/com/auth0/android/provider/AuthenticationActivity.java

lbalmaceda · 2020-09-23T21:13:40Z

auth0/src/test/java/com/auth0/android/provider/WebAuthProviderTest.java

@@ -167,21 +155,6 @@ public void shouldInitWithContext() {
 assertNotNull(WebAuthProvider.getManagerInstance());
 }

- @SuppressWarnings("deprecation")


removed as they are duplicate. see existing tests:

@Test public void shouldFailToResumeLoginWithIntentWithoutFirstInitProvider() { WebAuthProvider.resetManagerInstance(); Intent intent = createAuthIntent(""); assertFalse(WebAuthProvider.resume(intent)); } @SuppressWarnings("deprecation") @Test public void shouldFailToResumeLoginWithRequestCodeWithoutFirstInitProvider() { WebAuthProvider.resetManagerInstance(); Intent intent = createAuthIntent(""); assertFalse(WebAuthProvider.resume(REQUEST_CODE, Activity.RESULT_OK, intent)); }

lbalmaceda · 2020-09-23T22:01:01Z

auth0/src/main/java/com/auth0/android/provider/BrowserPicker.java

+public class BrowserPicker implements Parcelable {
+
+ @NonNull
+ private static final List<String> CHROME_BROWSERS = Arrays.asList(


these are for the legacy scenarios

lbalmaceda · 2020-09-23T22:01:10Z

auth0/src/main/java/com/auth0/android/provider/BrowserPicker.java

+ private Builder() {
+ }
+
+ //TODO: JAVADOCS


will add soon

lbalmaceda · 2020-09-23T22:02:12Z

auth0/src/main/java/com/auth0/android/provider/BrowserPicker.java

+
+ //If the browser packages were filtered, use the allowed packages list as order preference.
+ //A user-selected default browser will always be picked up first.
+ List<String> preferenceList = allowedPackages;


the order of preference is dictated by either the "allowed packages" in the case that the developer has provided those, or the default browser + chrome browsers, in the case of the legacy scenario

lbalmaceda · 2020-09-23T22:05:51Z

auth0/src/test/java/com/auth0/android/provider/OAuthManagerTest.java

 manager.useFullScreen(true);
 assertTrue(manager.useFullScreen());
 }

- @Test
- public void shouldNotHaveCustomTabsOptionsByDefault() {


lbalmaceda · 2020-09-23T22:06:15Z

auth0/src/test/java/com/auth0/android/provider/OAuthManagerTest.java

@@ -121,13 +104,23 @@ public void shouldHaveValidState() {

 @Test
 public void shouldHaveInvalidState() {
- exception.expect(AuthenticationException.class);
- OAuthManager.assertValidState("0987654321", "1234567890");
+ Assert.assertThrows(AuthenticationException.class, new ThrowingRunnable() {


trying out the new way of asserting exceptions. The rule for "ExpectedException.none()" is deprecated

lbalmaceda · 2020-09-23T22:06:41Z

auth0/src/test/java/com/auth0/android/provider/WebAuthProviderTest.java

-
- //Next line is needed to tell a Browser app is installed
- prepareBrowserApp(true, null);
+ BrowserPickerTest.setupBrowserContext(activity, Arrays.asList("com.auth0.browser"), null, null);


tells that 1 browser, not custom tab compatible (doesn't matter), is installed

auth0/src/test/java/com/auth0/android/provider/WebAuthProviderTest.java

lbalmaceda · 2020-09-23T22:09:32Z

auth0/src/test/java/com/auth0/android/provider/WebAuthProviderTest.java

- //** ** ** ** ** ** **//
-
- @Test
- public void shouldHaveBrowserAppInstalled() {


moved to BrowserPicker tests

jimmyjames

Generally looks good, just one question about the removal of the setCustomTabsOptions from the OAuthManager.

auth0/src/main/java/com/auth0/android/provider/OAuthManager.java

lbalmaceda · 2020-09-24T20:10:42Z

auth0/src/main/java/com/auth0/android/provider/BrowserPicker.java

@@ -110,14 +110,13 @@ public BrowserPicker build() {
 @Nullable
 String getBestBrowserPackage(@NonNull PackageManager pm) {
 Intent browserIntent = new Intent(Intent.ACTION_VIEW, Uri.parse("http:https://www.example.com"));
- ResolveInfo webHandler = pm.resolveActivity(browserIntent,
- Build.VERSION.SDK_INT >= Build.VERSION_CODES.M ? PackageManager.MATCH_ALL : PackageManager.MATCH_DEFAULT_ONLY);
+ ResolveInfo webHandler = pm.resolveActivity(browserIntent, PackageManager.MATCH_DEFAULT_ONLY);


Fix: this line's purpose is to find the "default browser app" in the device. "Matching all" would return all the browsers and pick one from there, which is not always the default app.

lbalmaceda · 2020-09-24T20:11:26Z

auth0/src/main/java/com/auth0/android/provider/BrowserPicker.java

 String defaultBrowser = null;
 if (webHandler != null) {
 defaultBrowser = webHandler.activityInfo.packageName;
 }

- final List<ResolveInfo> availableBrowsers = pm.queryIntentActivities(browserIntent, 0);
+ final List<ResolveInfo> availableBrowsers = pm.queryIntentActivities(browserIntent, Build.VERSION.SDK_INT >= Build.VERSION_CODES.M ? PackageManager.MATCH_ALL : 0);


Fix: when listing the installed browsers on marshmallow and above, the ALL flag should be used. Otherwise, the returned list has only one result.

lbalmaceda · 2020-09-24T20:11:56Z

auth0/src/test/java/com/auth0/android/provider/BrowserPickerTest.java

@@ -327,7 +328,7 @@ static void setupBrowserContext(@NonNull Context context, @NonNull List<String>
 PackageManager pm = mock(PackageManager.class);
 when(context.getPackageManager()).thenReturn(pm);
 ResolveInfo defaultPackage = resolveInfoForPackageName(defaultBrowserPackage);
- when(pm.resolveActivity(any(Intent.class), anyInt())).thenReturn(defaultPackage);
+ when(pm.resolveActivity(any(Intent.class), eq(PackageManager.MATCH_DEFAULT_ONLY))).thenReturn(defaultPackage);


better matchers, only by the values that we know we use. Will help catch changes like changing the prod code flags by mistake in the future 👍

nelsonmaia and others added 6 commits September 22, 2020 15:55

Adding support to override the browser packages to be used during Log…

056038f

…in / Logout Adding support to override the browser packages to be used during Login / Logout

Adding support to search all the browsers in the device

7236981

refactor to use extensible BrowserPicker filtering

f3b3b64

Fix tests for changed logic

57badc1

Replace deprecated import with the alternative

1df97f9

add tests for the BrowserPicker class

2070eb9

lbalmaceda commented Sep 23, 2020

View reviewed changes

lbalmaceda requested a review from jimmyjames September 23, 2020 22:11

lbalmaceda added 2 commits September 23, 2020 20:04

add coverage for introduced use cases and javadocs

e1692ff

add CompatibleBrowser tests

4c25864

lbalmaceda added the large Large review label Sep 23, 2020

lbalmaceda added this to the v1-Next milestone Sep 23, 2020

lbalmaceda marked this pull request as ready for review September 23, 2020 23:16

lbalmaceda requested a review from a team September 23, 2020 23:16

lbalmaceda added the CH: Added label Sep 23, 2020

jimmyjames reviewed Sep 24, 2020

View reviewed changes

auth0/src/main/java/com/auth0/android/provider/OAuthManager.java Show resolved Hide resolved

jimmyjames self-requested a review September 24, 2020 15:51

jimmyjames previously approved these changes Sep 24, 2020

View reviewed changes

fix flag passed when querying browser apps for post marshmallow

7985131

lbalmaceda dismissed jimmyjames’s stale review via 7985131 September 24, 2020 20:09

lbalmaceda commented Sep 24, 2020

View reviewed changes

jimmyjames approved these changes Sep 24, 2020

View reviewed changes

lbalmaceda merged commit 77367e0 into master Sep 25, 2020

lbalmaceda modified the milestones: v1-Next, 1.27.0 Sep 25, 2020

lbalmaceda mentioned this pull request Nov 3, 2020

Add custom tabs options to the available Lock settings [SDK-1974] auth0/Lock.Android#601

Merged

7 tasks

lbalmaceda deleted the nelsonmaia-master branch January 20, 2021 09:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat: Filter allowed CustomTabs browsers #353

Feat: Filter allowed CustomTabs browsers #353

lbalmaceda commented Sep 23, 2020 •

edited

Loading

lbalmaceda left a comment

lbalmaceda Sep 23, 2020

lbalmaceda Sep 23, 2020

lbalmaceda Sep 23, 2020

lbalmaceda Sep 23, 2020

lbalmaceda Sep 23, 2020

lbalmaceda Sep 23, 2020

lbalmaceda Sep 23, 2020

lbalmaceda Sep 23, 2020

jimmyjames left a comment

lbalmaceda Sep 24, 2020

lbalmaceda Sep 24, 2020

lbalmaceda Sep 24, 2020

Feat: Filter allowed CustomTabs browsers #353

Feat: Filter allowed CustomTabs browsers #353

Conversation

lbalmaceda commented Sep 23, 2020 • edited Loading

Changes

Public API

References

Testing

Checklist

lbalmaceda left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jimmyjames left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lbalmaceda commented Sep 23, 2020 •

edited

Loading