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

Facebook Authentication should do a patch not an update. #174

Closed
DenJohX opened this Issue Apr 25, 2016 · 4 comments

Comments

Projects
None yet
4 participants
@DenJohX
Copy link

DenJohX commented Apr 25, 2016

Using a feathers-cli generated app with mongodb, if the user model has more fields (other than facebook-id and facebook data), all those fields gets erased when the user authenticates.

This happens because the authentication proccess updates the social information of the user by sending a full update to the user service, thus eliminating any other field the user may have. I think should be better to patch the user instead of updating it.

@ekryski ekryski added the Bug label Apr 25, 2016

@beevelop

This comment has been minimized.

Copy link
Contributor

beevelop commented Apr 27, 2016

I can confirm that using update leads to the described side-effects. This bug is very likely to be independent of the used database – however I can reproduce it for MySQL too.

@marshallswain

This comment has been minimized.

Copy link
Member

marshallswain commented Apr 27, 2016

Do one of you want to try changing this line to do patch instead of update?: https://github.com/feathersjs/feathers-authentication/blob/master/src/services/oauth2/index.js#L63

And see if it resolves the issue? Thanks.

@beevelop

This comment has been minimized.

Copy link
Contributor

beevelop commented Apr 27, 2016

Yeah, I already did so: beevelop/feathers-authentication@a7300f8
I can confirm this results in the excepted behaviour (by not crashing your user entry).

@marshallswain

This comment has been minimized.

Copy link
Member

marshallswain commented Apr 27, 2016

Cool! Would you mind making a PR out of that?

beevelop added a commit to beevelop/feathers-authentication that referenced this issue Apr 27, 2016

fix(oauth2): Use patch to update user in oauthCallback
Using update() leads to conflicts with non-default user models and effectively in loosing the user's
data. By using patch the data gets merged properly.

fixes feathersjs#174

beevelop added a commit to beevelop/feathers-authentication that referenced this issue Apr 27, 2016

fix(oauth2): Use patch to update user in oauthCallback
Using update() leads to conflicts with non-default user models and effectively in loosing the user's
data. By using patch the data gets merged properly.

fixes feathersjs#174

@ekryski ekryski closed this in #183 Apr 28, 2016

ekryski added a commit that referenced this issue Apr 28, 2016

Merge pull request #183 from beevelop/oauth-patch. Closes #174
fix(oauth2): Use patch to update user in oauthCallback
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.