Skip to content
This repository has been archived by the owner on Aug 30, 2021. It is now read-only.

passport facebook (facebook api v2 & email permission disallowed) #202

Closed
richmonkeys opened this issue Oct 12, 2014 · 5 comments · Fixed by #842
Closed

passport facebook (facebook api v2 & email permission disallowed) #202

richmonkeys opened this issue Oct 12, 2014 · 5 comments · Fixed by #842

Comments

@richmonkeys
Copy link

Facebook API v2 no longer returns username, hence on FacebookStrategy callback we'll get undefined for profile.username.
Currently I'm using the below in config/strategies/facebook.js as my workaround.
line 32: username: profile.username || profile.id

Another thing is when user chose not to allow the access of email from Facebook there'll be a small error which causes the app to skip saving/updating the user.
My workaround in config/strategies/facebook.js
line 31: email: (profile.emails)? profile.emails[0].value: ''

@ilanbiala
Copy link
Member

@jcaspian has this been fixed? We've done a lot of OAuth restructuring, so I think some things may have been taken care of.

@ilanbiala ilanbiala self-assigned this Mar 10, 2015
@richmonkeys
Copy link
Author

@ilanbiala I can see that currently FacebookStrategy is using the user's registered Facebook email address for the profile.username field. E.g. [email protected], "jeff_wong" will be used.

As for the second issue mentioned regarding Facebook permission, the problem seems to be still there.

To reproduce:

  1. Open MeanJS app.
  2. Sign up an account via Facebook Login
  3. During Facebook permission prompt, disallow access to email
  4. You'll be redirected back to the app
  5. Try to sign up via Facebook Login again
  6. Facebook automatically response with the Permission decided earlier (disallow access to email)
  7. Redirected back to the app, without the permission prompt from facebook

From what I understand this is actually not an issue of MeanJS but developers like us need to deal with it ourselves. For example if user chose not to allow email access, we'll need to tell user the reason that account could not be created because email is required. Then we'll need to revoke the user app permission so that next time when user choose to sign up via Facebook Login again, the permission prompt will be shown.

In the end, if these "if_else" handles are baked into meanjs out of the box would be awesome.

@ilanbiala
Copy link
Member

@jcaspian is this only for Facebook or for the others as well?

@richmonkeys
Copy link
Author

@ilanbiala I'm sorry I have only tested with FacebookStrategy but not the other passport providers but I believe if the other providers have the permission features then this issue may exists in them as well.

I have done some research earlier today and I found that we can pass a parameter to Facebook while making Login request to re-request permissions that are previously denied.

The parameter is auth_type. I saw it in the Facebook javascript sdk, for more information you may check it here: https://developers.facebook.com/docs/reference/javascript/FB.login/v2.2#options

If every Facebook Login request is sent with this parameter auth_type: 'rerequest', Facebook will check if all the permissions are granted, if not it will prompt user to request the permissions. If all is granted, Facebook will automatically return the normal response back to MeanJS.

rhutchison added a commit to Gym/mean that referenced this issue Aug 23, 2015
…l not be mapped. Username will be generated from first initial of first name and last name.

.jshint latedef set to nofunc.
@Yaoyusina
Copy link

Yaoyusina commented Apr 20, 2016

I find when I use my facebook account "**[email protected]" login, the terminal log is "/authentication/signin?err=Email%20already%20exists" and stay signin page;
and if I use my facebook account "
*****@gmail.com" (without "." in prefix), then the signin is working.
so oddly!

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

Successfully merging a pull request may close this issue.

5 participants