Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Currently not possible to use setAllowNewAccounts(false) with email/link authentication #1762

Closed
theHilikus opened this issue Mar 25, 2020 · 7 comments

Comments

@theHilikus
Copy link

theHilikus commented Mar 25, 2020

It's currently not possible to create a user with the firebase admin sdk (or any other way outside of firebase UI) and have that user use email/link authentication. It seems that if you want to use email/link, you are forced to enable client registration (setAllowNewAccounts(true)), which is not always desirable.

I traced the code and the issue is that an externally created user always is created with a password provider id. This causes issues in com.firebase.ui.auth.util.data.ProviderUtils#fetchSortedProviders, which ends up throwing a developer error.

This is related to #1735 but there it was mentioned something slightly different: once you enable email/link, previous users with email/password won't be able to log in, meaning to use email/link, ALL users must use it, which looks like a design decision that requires a new feature to be improved. The situation here looks to me more like a bug: a completely new user that has never logged in cannot login at all, even if it is the first user in a blank auth db or all the users were created after enabling email/link auth. Also, the fact that the application accepts an invalid configuration (setAllowNewAccounts(false) with .enableEmailLinkSignIn(), which will never work together) also makes it look more like a corner-case bug

  • FirebaseUI version: 6.2.0

Steps to reproduce

  • Create a user outside of firebaseUI
  • Configure FirebaseUI with the following options
List<AuthUI.IdpConfig> providers = getAvailableProviders();
Intent loginIntent = AuthUI.getInstance()
                .createSignInIntentBuilder()
                .setAvailableProviders(providers)
                .build();
startActivityForResult(loginIntent, LOG_IN_REQUEST_CODE);

private List<AuthUI.IdpConfig> getAvailableProviders() {
        ActionCodeSettings actionCodeSettings = ActionCodeSettings.newBuilder()
                .setAndroidPackageName(BuildConfig.APPLICATION_ID, true, null)
                .setIOSBundleId(iOsBundleId)
                .setHandleCodeInApp(true)
                .setUrl("https://foo")
                .build();

        return Collections.singletonList(
                new EmailBuilder()
                        .setAllowNewAccounts(false)
                        .enableEmailLinkSignIn()
                        .setActionCodeSettings(actionCodeSettings)
                        .build());

    }
  • Try to log in. It will fail

Finally, all this worked fine if using pure firebase-auth: you can create a user externally and log it in using email/link

@samtstern
Copy link
Contributor

@theHilikus thanks for filing. Sounds like there are two actions items here:

First we need to stop allowing this combination:

.setAllowNewAccounts(false)
.enableEmailLinkSignIn()

We should error out immediately as that's not going to work.

Can you explain a little more about this:

Finally, all this worked fine if using pure firebase-auth: you can create a user externally and log it in using email/link

If you can help me recreate that scenario that would be really useful.

@samtstern samtstern added the auth label Mar 25, 2020
@samtstern samtstern added this to the 6.3.0 milestone Mar 25, 2020
@theHilikus
Copy link
Author

Hi @samtstern
What i meant by the last bit is that if you create a user externally (e.g. using the admin sdk), you can still log it in using pure firebase-auth as described here using the sample code that's there (auth.sendSignInLinkToEmail). But if you create another user and follow the procedure to log it in using firebaseUI instead of pure firebase-auth, it doesn't work. you hit the problem i mentioned.

I hope that makes it clearer but let me know if you still have quesitons

@samtstern
Copy link
Contributor

@theHilikus I know it's been a while but I finally sat down to address this and at first glance the UI is working for me. This is what I see in the email link flow when allow new accounts is false:

Screen Shot 2020-07-20 at 2 04 23 PM

Do you see something else?

@theHilikus
Copy link
Author

theHilikus commented Jul 20, 2020

Hi @samtstern
I see that as well. the problem is not when the user doesn't exist. it is when the user DOES exist but it was not created by firebaseUI, so .setAllowNewAccounts is set to false so that users can never be created by firebaseUI. But it does need to exist to reproduce the bug

I create the user via the firebase admin sdk, but i'm pretty sure you'll be able to reproduce the bug if you create the auth user via the firebase-auth web console

Conceptually, think of a by-invitation-only system. An admin has to create your account before you can log in; you cannot signup by yourself. This is the workflow that is currently not possible: the combination of email-link signin with setAllowNewAccounts = false

Ideally this workflow would be allowed, but if it's not allowed as a design choice and not a bug, it would be nice that at least the builder throws an exception when you build that configuration of .setAllowNewAccounts(false).enableEmailLinkSignIn()

@theHilikus
Copy link
Author

theHilikus commented Jul 20, 2020

btw, I fixed this issue in my personal fork. I never opened a PR because i don't understand the whole project enough to guarantee that there are no unintended regressions, but if we can fix this over here and everyone benefits from the change, that would be great

theHilikus@ce69ff9

The logic in my fix is, if you explicitly configure your builder to use email-link (i.e. enableEmailLinkSignIn()) but the provider only has email-password, force-add the email-link provider as the first option. My rationale being, if the dev is adding the email-link provider to the builder is because they know that email-link is allowed.

@samtstern
Copy link
Contributor

@theHilikus thanks for sharing that commit! I tested it out and it seems fine, the worst case is a DEVELOPER_ERROR if you try and do something wrong which is fine with me.

I'm gonna patch that in.

@samtstern
Copy link
Contributor

This issue is fixed and released in version 6.3.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants