Rank providers by usability when picking top provider (#1362)

SUPERCILEX · samtstern · commit 0da111bcd138 · 2018-08-28T08:54:47.000-07:00
diff --git a/auth/src/main/java/com/firebase/ui/auth/data/model/FlowParameters.java b/auth/src/main/java/com/firebase/ui/auth/data/model/FlowParameters.java
@@ -40,7 +40,7 @@ public class FlowParameters implements Parcelable {
     public final String appName;
 
     @NonNull
-    public final List<IdpConfig> providerInfo;
+    public final List<IdpConfig> providers;
 
     @StyleRes
     public final int themeId;
@@ -60,7 +60,7 @@ public class FlowParameters implements Parcelable {
 
     public FlowParameters(
             @NonNull String appName,
-            @NonNull List<IdpConfig> providerInfo,
+            @NonNull List<IdpConfig> providers,
             @StyleRes int themeId,
             @DrawableRes int logoId,
             @Nullable String termsOfServiceUrl,
@@ -69,8 +69,8 @@ public FlowParameters(
             boolean enableHints,
             boolean enableAnonymousUpgrade) {
         this.appName = Preconditions.checkNotNull(appName, "appName cannot be null");
-        this.providerInfo = Collections.unmodifiableList(
-                Preconditions.checkNotNull(providerInfo, "providerInfo cannot be null"));
+        this.providers = Collections.unmodifiableList(
+                Preconditions.checkNotNull(providers, "providers cannot be null"));
         this.themeId = themeId;
         this.logoId = logoId;
         this.termsOfServiceUrl = termsOfServiceUrl;
@@ -90,7 +90,7 @@ public static FlowParameters fromIntent(Intent intent) {
     @Override
     public void writeToParcel(Parcel dest, int flags) {
         dest.writeString(appName);
-        dest.writeTypedList(providerInfo);
+        dest.writeTypedList(providers);
         dest.writeInt(themeId);
         dest.writeInt(logoId);
         dest.writeString(termsOfServiceUrl);
@@ -137,7 +137,7 @@ public FlowParameters[] newArray(int size) {
     };
 
     public boolean isSingleProviderFlow() {
-        return providerInfo.size() == 1;
+        return providers.size() == 1;
     }
 
     public boolean isTermsOfServiceUrlProvided() {
diff --git a/auth/src/main/java/com/firebase/ui/auth/data/remote/SignInKickstarter.java b/auth/src/main/java/com/firebase/ui/auth/data/remote/SignInKickstarter.java
@@ -56,7 +56,7 @@ public SignInKickstarter(Application application) {
     public void start() {
         // Only support password credentials if email auth is enabled
         boolean supportPasswords = ProviderUtils.getConfigFromIdps(
-                getArguments().providerInfo, EmailAuthProvider.PROVIDER_ID) != null;
+                getArguments().providers, EmailAuthProvider.PROVIDER_ID) != null;
         List<String> accountTypes = getCredentialAccountTypes();
 
         // If the request will be empty, avoid the step entirely
@@ -97,7 +97,7 @@ public void onComplete(@NonNull Task<CredentialRequestResponse> task) {
     private void startAuthMethodChoice() {
         // If there is only one provider selected, launch the flow directly
         if (getArguments().isSingleProviderFlow()) {
-            AuthUI.IdpConfig firstIdpConfig = getArguments().providerInfo.get(0);
+            AuthUI.IdpConfig firstIdpConfig = getArguments().providers.get(0);
             String firstProvider = firstIdpConfig.getProviderId();
             switch (firstProvider) {
                 case EmailAuthProvider.PROVIDER_ID:
@@ -157,7 +157,7 @@ private void redirectSignIn(String provider, String id) {
 
     private List<String> getCredentialAccountTypes() {
         List<String> accounts = new ArrayList<>();
-        for (AuthUI.IdpConfig idpConfig : getArguments().providerInfo) {
+        for (AuthUI.IdpConfig idpConfig : getArguments().providers) {
             @AuthUI.SupportedProvider String providerId = idpConfig.getProviderId();
             if (providerId.equals(GoogleAuthProvider.PROVIDER_ID)) {
                 accounts.add(ProviderUtils.providerIdToAccountType(providerId));
diff --git a/auth/src/main/java/com/firebase/ui/auth/ui/email/CheckEmailHandler.java b/auth/src/main/java/com/firebase/ui/auth/ui/email/CheckEmailHandler.java
@@ -33,7 +33,7 @@ public void fetchCredential() {
 
     public void fetchProvider(final String email) {
         setResult(Resource.<User>forLoading());
-        ProviderUtils.fetchTopProvider(getAuth(), email)
+        ProviderUtils.fetchTopProvider(getAuth(), getArguments(), email)
                 .addOnCompleteListener(new OnCompleteListener<String>() {
                     @Override
                     public void onComplete(@NonNull Task<String> task) {
@@ -53,7 +53,7 @@ public void onActivityResult(int requestCode, int resultCode, @Nullable Intent d
         setResult(Resource.<User>forLoading());
         final Credential credential = data.getParcelableExtra(Credential.EXTRA_KEY);
         final String email = credential.getId();
-        ProviderUtils.fetchTopProvider(getAuth(), email)
+        ProviderUtils.fetchTopProvider(getAuth(), getArguments(), email)
                 .addOnCompleteListener(new OnCompleteListener<String>() {
                     @Override
                     public void onComplete(@NonNull Task<String> task) {
diff --git a/auth/src/main/java/com/firebase/ui/auth/ui/email/EmailActivity.java b/auth/src/main/java/com/firebase/ui/auth/ui/email/EmailActivity.java
@@ -111,7 +111,7 @@ public void onNewUser(User user) {
         TextInputLayout emailLayout = findViewById(R.id.email_layout);
 
         AuthUI.IdpConfig emailConfig = ProviderUtils.getConfigFromIdpsOrThrow(
-                getFlowParams().providerInfo, EmailAuthProvider.PROVIDER_ID);
+                getFlowParams().providers, EmailAuthProvider.PROVIDER_ID);
         if (emailConfig.getParams().getBoolean(ExtraConstants.ALLOW_NEW_EMAILS, true)) {
             RegisterEmailFragment fragment = RegisterEmailFragment.newInstance(user);
             FragmentTransaction ft = getSupportFragmentManager().beginTransaction()
diff --git a/auth/src/main/java/com/firebase/ui/auth/ui/email/RegisterEmailFragment.java b/auth/src/main/java/com/firebase/ui/auth/ui/email/RegisterEmailFragment.java
@@ -147,7 +147,7 @@ public void onViewCreated(@NonNull View view, @Nullable Bundle savedInstanceStat
 
         // Get configuration
         AuthUI.IdpConfig emailConfig = ProviderUtils.getConfigFromIdpsOrThrow(
-                getFlowParams().providerInfo, EmailAuthProvider.PROVIDER_ID);
+                getFlowParams().providers, EmailAuthProvider.PROVIDER_ID);
         boolean requireName = emailConfig.getParams()
                 .getBoolean(ExtraConstants.REQUIRE_NAME, true);
         mPasswordFieldValidator = new PasswordFieldValidator(
diff --git a/auth/src/main/java/com/firebase/ui/auth/ui/idp/AuthMethodPickerActivity.java b/auth/src/main/java/com/firebase/ui/auth/ui/idp/AuthMethodPickerActivity.java
@@ -89,7 +89,7 @@ protected void onCreate(@Nullable Bundle savedInstanceState) {
         mHandler = ViewModelProviders.of(this).get(SocialProviderResponseHandler.class);
         mHandler.init(params);
 
-        populateIdpList(params.providerInfo, mHandler);
+        populateIdpList(params.providers, mHandler);
 
         int logoId = params.logoId;
         if (logoId == AuthUI.NO_LOGO) {
diff --git a/auth/src/main/java/com/firebase/ui/auth/ui/idp/SingleSignInActivity.java b/auth/src/main/java/com/firebase/ui/auth/ui/idp/SingleSignInActivity.java
@@ -47,7 +47,7 @@ protected void onCreate(@Nullable Bundle savedInstanceState) {
         String provider = user.getProviderId();
 
         AuthUI.IdpConfig providerConfig =
-                ProviderUtils.getConfigFromIdps(getFlowParams().providerInfo, provider);
+                ProviderUtils.getConfigFromIdps(getFlowParams().providers, provider);
         if (providerConfig == null) {
             finish(RESULT_CANCELED, IdpResponse.getErrorIntent(new FirebaseUiException(
                     ErrorCodes.DEVELOPER_ERROR,
diff --git a/auth/src/main/java/com/firebase/ui/auth/ui/idp/WelcomeBackIdpPrompt.java b/auth/src/main/java/com/firebase/ui/auth/ui/idp/WelcomeBackIdpPrompt.java
@@ -98,7 +98,7 @@ protected void onCreate(@Nullable Bundle savedInstanceState) {
 
         String providerId = existingUser.getProviderId();
         AuthUI.IdpConfig config =
-                ProviderUtils.getConfigFromIdps(getFlowParams().providerInfo, providerId);
+                ProviderUtils.getConfigFromIdps(getFlowParams().providers, providerId);
         if (config == null) {
             finish(RESULT_CANCELED, IdpResponse.getErrorIntent(new FirebaseUiException(
                     ErrorCodes.DEVELOPER_ERROR,
diff --git a/auth/src/main/java/com/firebase/ui/auth/util/data/ProviderUtils.java b/auth/src/main/java/com/firebase/ui/auth/util/data/ProviderUtils.java
@@ -21,7 +21,7 @@
 
 import com.firebase.ui.auth.AuthUI;
 import com.firebase.ui.auth.IdpResponse;
-import com.firebase.ui.auth.util.Preconditions;
+import com.firebase.ui.auth.data.model.FlowParameters;
 import com.google.android.gms.auth.api.credentials.IdentityProviders;
 import com.google.android.gms.tasks.Continuation;
 import com.google.android.gms.tasks.Task;
@@ -36,6 +36,7 @@
 import com.google.firebase.auth.SignInMethodQueryResult;
 import com.google.firebase.auth.TwitterAuthProvider;
 
+import java.util.ArrayList;
 import java.util.List;
 
 @RestrictTo(RestrictTo.Scope.LIBRARY_GROUP)
@@ -154,41 +155,63 @@ public static AuthUI.IdpConfig getConfigFromIdpsOrThrow(List<AuthUI.IdpConfig> i
         return config;
     }
 
-    public static Task<String> fetchTopProvider(FirebaseAuth auth, @NonNull String email) {
+    public static Task<List<String>> fetchSortedProviders(@NonNull FirebaseAuth auth,
+                                                          @NonNull final FlowParameters params,
+                                                          @NonNull String email) {
         if (TextUtils.isEmpty(email)) {
             return Tasks.forException(new NullPointerException("Email cannot be empty"));
         }
 
         return auth.fetchSignInMethodsForEmail(email)
-                .continueWith(new Continuation<SignInMethodQueryResult, String>() {
+                .continueWith(new Continuation<SignInMethodQueryResult, List<String>>() {
                     @Override
-                    public String then(@NonNull Task<SignInMethodQueryResult> task) {
-                        if (!task.isSuccessful()) return null;
-
+                    public List<String> then(@NonNull Task<SignInMethodQueryResult> task) {
                         List<String> methods = task.getResult().getSignInMethods();
-                        return methods == null || methods.isEmpty()
-                                ? null : methods.get(methods.size() - 1);
+                        if (methods == null) { methods = new ArrayList<>(); }
+
+                        List<String> allowedProviders = new ArrayList<>(params.providers.size());
+                        for (AuthUI.IdpConfig provider : params.providers) {
+                            allowedProviders.add(provider.getProviderId());
+                        }
+
+                        List<String> lastSignedInProviders = new ArrayList<>(methods.size());
+                        for (String method : methods) {
+                            String id = signInMethodToProviderId(method);
+                            if (allowedProviders.contains(id)) {
+                                lastSignedInProviders.add(0, id);
+                            }
+                        }
+
+                        // Reorder providers from most to least usable. Usability is determined by
+                        // how many steps a user needs to perform to log in.
+                        maximizePriority(lastSignedInProviders, GoogleAuthProvider.PROVIDER_ID);
+
+                        return lastSignedInProviders;
+                    }
+
+                    private void maximizePriority(List<String> providers, String id) {
+                        if (providers.remove(id)) { providers.add(0, id); }
                     }
-                }).continueWith(new Continuation<String, String>() {
+                });
+    }
+
+    public static Task<String> fetchTopProvider(
+            @NonNull FirebaseAuth auth,
+            @NonNull FlowParameters params,
+            @NonNull String email) {
+        return fetchSortedProviders(auth, params, email)
+                .continueWith(new Continuation<List<String>, String>() {
                     @Override
-                    public String then(@NonNull Task<String> task) {
-                        String method = task.getResult();
-                        if (method == null) {
+                    public String then(@NonNull Task<List<String>> task) {
+                        if (!task.isSuccessful()) return null;
+                        List<String> providers = task.getResult();
+
+                        if (providers.isEmpty()) {
                             return null;
                         } else {
-                            return signInMethodToProviderId(method);
+                            return providers.get(0);
                         }
                     }
                 });
     }
-
-    public static String getTopProvider(@NonNull List<String> providers) {
-        return providers == null || providers.isEmpty() ? null :
-                providers.get(providers.size() - 1);
-    }
-
-    public static boolean isExistingProvider(@NonNull List<String> providers, String provider) {
-        if (providers == null) throw new IllegalArgumentException("The list of providers is null.");
-        return providers.contains(provider);
-    }
 }
diff --git a/auth/src/main/java/com/firebase/ui/auth/viewmodel/email/EmailProviderResponseHandler.java b/auth/src/main/java/com/firebase/ui/auth/viewmodel/email/EmailProviderResponseHandler.java
@@ -69,7 +69,7 @@ public void onFailure(@NonNull Exception e) {
                                 // Collision with existing user email without anonymous upgrade
                                 // it should be very hard for the user to even get to this error
                                 // due to CheckEmailFragment.
-                                ProviderUtils.fetchTopProvider(getAuth(), email)
+                                ProviderUtils.fetchTopProvider(getAuth(), getArguments(), email)
                                         .addOnSuccessListener(new StartWelcomeBackFlow(email))
                                         .addOnFailureListener(new OnFailureListener() {
                                             @Override
diff --git a/auth/src/main/java/com/firebase/ui/auth/viewmodel/idp/SocialProviderResponseHandler.java b/auth/src/main/java/com/firebase/ui/auth/viewmodel/idp/SocialProviderResponseHandler.java
@@ -27,7 +27,6 @@
 import com.google.firebase.auth.AuthResult;
 import com.google.firebase.auth.EmailAuthProvider;
 import com.google.firebase.auth.FirebaseAuthUserCollisionException;
-import com.google.firebase.auth.SignInMethodQueryResult;
 
 import java.util.List;
 
@@ -64,7 +63,7 @@ public void onSuccess(AuthResult result) {
                 })
                 .addOnFailureListener(new OnFailureListener() {
                     @Override
-                    public void onFailure(@NonNull final Exception e) {
+                    public void onFailure(@NonNull Exception e) {
                         if (e instanceof FirebaseAuthUserCollisionException) {
                             final String email = response.getEmail();
                             if (email == null) {
@@ -80,20 +79,22 @@ public void onFailure(@NonNull final Exception e) {
                             // existing user.
                             // CASE 3: CASE 2 with an anonymous user. We link the new IDP to the
                             // same account before handling invoking a merge failure.
-                            getAuth().fetchSignInMethodsForEmail(email)
-                                    .addOnSuccessListener(new OnSuccessListener<SignInMethodQueryResult>() {
+                            ProviderUtils.fetchSortedProviders(getAuth(), getArguments(), email)
+                                    .addOnSuccessListener(new OnSuccessListener<List<String>>() {
                                         @Override
-                                        public void onSuccess(SignInMethodQueryResult result) {
-                                            List<String> providers = result.getSignInMethods();
-                                            if (ProviderUtils.isExistingProvider(providers,
-                                                    response.getProviderType())) {
+                                        public void onSuccess(List<String> providers) {
+                                            if (providers.contains(response.getProviderType())) {
                                                 // Case 1
                                                 handleMergeFailure(credential);
+                                            } else if (providers.isEmpty()) {
+                                                setResult(Resource.<IdpResponse>forFailure(
+                                                        new FirebaseUiException(
+                                                                ErrorCodes.DEVELOPER_ERROR,
+                                                                "No supported providers.")));
                                             } else {
                                                 // Case 2 & 3 - we need to link
                                                 startWelcomeBackFlowForLinking(
-                                                        ProviderUtils.getTopProvider(providers),
-                                                        response);
+                                                        providers.get(0), response);
                                             }
                                         }
                                     })
diff --git a/auth/src/test/java/com/firebase/ui/auth/AuthUITest.java b/auth/src/test/java/com/firebase/ui/auth/AuthUITest.java
@@ -49,9 +49,9 @@ public void testCreateStartIntent_shouldHaveEmailAsDefaultProvider() {
                 .createSignInIntentBuilder()
                 .build()
                 .getParcelableExtra(ExtraConstants.FLOW_PARAMS);
-        assertEquals(1, flowParameters.providerInfo.size());
+        assertEquals(1, flowParameters.providers.size());
         assertEquals(EmailAuthProvider.PROVIDER_ID,
-                flowParameters.providerInfo.get(0).getProviderId());
+                flowParameters.providers.get(0).getProviderId());
     }
 
     @Test(expected = IllegalArgumentException.class)
@@ -75,7 +75,7 @@ public void testCreatingStartIntent() {
                 .build()
                 .getParcelableExtra(ExtraConstants.FLOW_PARAMS);
 
-        assertEquals(4, flowParameters.providerInfo.size());
+        assertEquals(4, flowParameters.providers.size());
         assertEquals(TestHelper.MOCK_APP.getName(), flowParameters.appName);
         assertEquals(TestConstants.TOS_URL, flowParameters.termsOfServiceUrl);
         assertEquals(TestConstants.PRIVACY_URL, flowParameters.privacyPolicyUrl);
diff --git a/auth/src/test/java/com/firebase/ui/auth/viewmodel/SocialProviderResponseHandlerTest.java b/auth/src/test/java/com/firebase/ui/auth/viewmodel/SocialProviderResponseHandlerTest.java
@@ -311,8 +311,7 @@ public void testSignInIdp_anonymousUserUpgradeEnabledAndExistingPasswordUserWith
 
     private void setupAnonymousUpgrade() {
         // enableAnonymousUpgrade must be set to true
-        FlowParameters testParams = TestHelper.getFlowParameters(Collections.singletonList(
-                EmailAuthProvider.PROVIDER_ID), /* enableAnonymousUpgrade */ true);
+        FlowParameters testParams = TestHelper.getFlowParameters(AuthUI.SUPPORTED_PROVIDERS, /* enableAnonymousUpgrade */ true);
         mHandler.initializeForTesting(testParams, mMockAuth, null, null);
 
         when(mUser.isAnonymous()).thenReturn(true);
