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.

Sign up for GitHub

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

Jump to bottom

Facebook issues #5785

Closed
deek87 opened this issue Aug 2, 2017 · 3 comments
Closed

Facebook issues #5785

deek87 opened this issue Aug 2, 2017 · 3 comments
Labels
Status:Available Reviewed issue, it’s real, we’d review a pull request. Type:Enhancement A need for something new.

Comments

@deek87
Copy link
Contributor

deek87 commented Aug 2, 2017 •
edited

Currently on a default install of 8.2.x there are currently quite a number of issues with Facebook authentication.

Facebook authentication is linked to 1 account but you can add it to another.
Facebook automatic registration doesn't work - seems like nothing is actually parsed apart from the userid of the facebook account. - this is due to changes in the API calls /me no longer returns email address (it must be added to the URL)
There is no handling for restricting scopes, if I link a facebook account but disallow C5 to share email, it will just fail and it wont re-request the scope.
There is showing that an account is linked to a facebook account, other than the fact you no longer have the option to link an account.
You can not de-link the account except from facebook itself
After linking an account it just shows the login screen and asks you attach again
screen shot 2017-08-02 at 13 01 59

This I'm not 100% sure on but is the callback handler able to handle de-auth requests made from facebook? - edit: No it seems we have no method for de-auth at all.
@katzueno
Copy link
Contributor

katzueno commented Aug 2, 2017

Thanks Derek!

We're building a community base website, and we want users to be able to log-in smoothly.

Although there are not many people who tried to auth a single Facebook user to c5's multiple accounts..., it would still be important that we can prevent that.

@aembler aembler added Status:Available Reviewed issue, it’s real, we’d review a pull request. priority:like to have Type:Enhancement A need for something new. labels Aug 9, 2017
@aembler aembler added this to the Future milestone Aug 9, 2017
biplobice added a commit to biplobice/concretecms that referenced this issue Oct 5, 2017
@biplobice
concretecms#5785 Facebook authentication register issue fix
f095a7c
@biplobice
Copy link
Contributor

This commit fix also #4354

@biplobice biplobice mentioned this issue Oct 5, 2017
Closed
5 tasks
@deek87 deek87 mentioned this issue Oct 5, 2017
Merged
KorvinSzanto pushed a commit that referenced this issue Feb 15, 2018
@biplobice @KorvinSzanto
\#5785 Facebook authentication register issue fix
5c900f7 
Add sns authentication detach option

Oops, missed the view file

Update facebook form

Updating facebook controller to accept revoke access callbacks

Updated as code review

Prevent facebook attach with multiple account

Updating default Oauth controller in case of missing email fields. Also added Google/Twitter De-authorise methods.

Adding return to home on authentication forms
@katzueno
Copy link
Contributor

@KorvinSzanto Thanks for the merge. But, you should add @deek87 as an author, too.

@aembler aembler closed this as completed Feb 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Status:Available Reviewed issue, it’s real, we’d review a pull request. Type:Enhancement A need for something new.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@katzueno @aembler @biplobice @deek87