Skip to content

Support Facebook Login without email #624

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

Merged

Conversation

puilp0502
Copy link
Contributor

@puilp0502 puilp0502 commented Mar 6, 2017

Addresses #623.

I didn't know if I should bump up the version, so I left it as it is.

@samtstern
Copy link
Contributor

@puilp0502 just wanted to say I know this is lingering, I have been caught up on some other stuff and will test this soon. Thank you for doing it!

@samtstern samtstern changed the base branch from master to version-2.0.0-dev March 9, 2017 19:26
@samtstern samtstern changed the base branch from version-2.0.0-dev to master March 9, 2017 19:27
@samtstern samtstern changed the base branch from master to version-2.0.0-dev March 9, 2017 19:28
@samtstern
Copy link
Contributor

@puilp0502 FYI I changed the target branch to version-2.0.0-dev as that's where the next version will come from. You may have to change your local setup a bit, let me know if that's confusing.

@samtstern
Copy link
Contributor

samtstern commented Mar 9, 2017

Ok I just went and got a new phone number (thanks Google Voice) and then made a Facebook account that has no email. Then I signed in and got the following (see below).

I am surprised that signing in like that even works! I guess email is not a hard requirement for Facebook accounts (learn something new about your own products every day haha).

I am fine to merge this change, but @amandle can you chime in and let me know if this could cause any potential problems?

Console:
screenshot from 2017-03-09 12 02 03

App:
no_email

@amandle
Copy link
Contributor

amandle commented Mar 15, 2017

LGTM, Twitter also doesn't require email be shared, so I don't foresee this change causing problems

@samtstern samtstern merged commit 1b1cdb5 into firebase:version-2.0.0-dev Mar 15, 2017
@samtstern samtstern added this to the 2.0.0 milestone Mar 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants