Centralize email configuration options

[samtstern]

Right now we have an email option exposed on SignInIntentBuilder to allow new email accounts. We have multiple feature requests for other email flags:

• 467 - Don't require display name
• 312 - Require verified email address

I'd like to see these implemented but don't think we should keep hanging random options off the sign-in intent builder. Instead we'd like something like this (pseudocode):

new AuthUI.IdpConfig.Builder(AuthUI.EMAIL_PROVIDER)
        .allowNewAccounts(false)
        .requireDisplayName(false)
        .requireEmailVerification(true)
        .build(),

This is a much more scalable way to add flags over time.

[WillieCubed]

I could make a PR if you're not working on this already.

[samtstern]

@TheCraftKid thanks! Not working on this already but do you want to quickly outline what you think the API in this issue before starting a PR? That way we don't have to go back and forth on PR comments.

[WillieCubed]

This appears to be a good idea adding it to the IdpConfig.Builder, but would an option such as requireEmailVerification() on multiple identity providers make sense?

[samtstern]

I am wondering if we could do something tricky.

Maybe AuthUI.IdpConfig.Builder(AuthUI.EMAIL_PROVIDER) would return a different Builder class that's a subclass of the generic one and includes email-specific options.

This pattern would also allow us to do the same for other methods. For instance we could make setPermissions only appear on Google/Facebook/Twitter since it doesn't make sense on Phone/Email.

[WillieCubed]

Yeah, I was initially thinking generic classes would be the way to go. Just a bit more work. 😄

[WillieCubed]

Would this be in 2.1.0, 2.2.0 or the next major release?

[samtstern]

I think this can be done in the next release (which we are now calling 2.1.1 but would be renamed 2.2.0 if we had this change).

We can just mark the current email options as deprecated but not delete them, so it shouldn't be a breaking change.

[WillieCubed]

Okay, LGTM, I'll get to work.

[WillieCubed]

Okay, I want to squash #312, but the auth module is so dense, I don't know where to verify/check if the email is verified. Thoughts?

[samtstern]

@TheCraftKid maybe we split this up into two tasks? The first is the new config object, the second is adding the missing flags.

There are some open questions for #312. I'd say what we want to do is fail email sign-in if we don't see emailVerified on the FirebaseUser but we probably need more UI and also email sending to make that happen. Probably a big task and should be a separate PR from the options object.

[samtstern]

Also I'd prefer to tackle #467 before #312 since firebaseui-web already has the feature and it's much simpler to implement.

[jstimes]

Hey @TheCraftKid are you still working on this? If not I'd be happy to take over.

[WillieCubed]

Yeah, I meant to make sure this made it into version 3.0.0, but I got a bit busy on other projects. Let me make some more changes and push an updated version.

[ryandailey100]

@TheCraftKid When will this be finished by? I would like to have this implemented inside my app before I launch. Thanks 👍

[WillieCubed]

I'm sorry to hold you up, but it'll likely be by the end of the day or tomorrow evening, max.

…

On Tue, Oct 10, 2017, 12:33 PM Ryan Dailey ***@***.***> wrote: @TheCraftKid <https://github.com/thecraftkid> When will this be finished by? I would like to have this implemented inside my app before I launch. Thanks 👍 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#806 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AP3uIp73g_T1C6Tucz_rAuuKBnnHnLNUks5sq6pcgaJpZM4ObuSr> .

[ryandailey100]

Great! Sounds good to me. Thanks again

[WillieCubed]

On second thought, it's possibly likely we're going to lump this in with #962, so there's no telling when that's going to be completed. It may be anywhere from a week to whenever it's done, but we may introduce a minor version bump with a fix for this. I wouldn't bet on that, though.

[samtstern]

This has been fixed and released in version 3.2.0.