{"action":"created","issue":{"url":"https://api.github.com/repos/feathersjs/feathers-authentication-oauth2/issues/19","repository_url":"https://api.github.com/repos/feathersjs/feathers-authentication-oauth2","labels_url":"https://api.github.com/repos/feathersjs/feathers-authentication-oauth2/issues/19/labels{/name}","comments_url":"https://api.github.com/repos/feathersjs/feathers-authentication-oauth2/issues/19/comments","events_url":"https://api.github.com/repos/feathersjs/feathers-authentication-oauth2/issues/19/events","html_url":"https://github.com/feathersjs/feathers-authentication-oauth2/issues/19","id":223633075,"number":19,"title":"Update existing user in verifier will change the user password hash from an already hashed password.","user":{"login":"skinnyworm","id":7065428,"avatar_url":"https://avatars0.githubusercontent.com/u/7065428?v=3","gravatar_id":"","url":"https://api.github.com/users/skinnyworm","html_url":"https://github.com/skinnyworm","followers_url":"https://api.github.com/users/skinnyworm/followers","following_url":"https://api.github.com/users/skinnyworm/following{/other_user}","gists_url":"https://api.github.com/users/skinnyworm/gists{/gist_id}","starred_url":"https://api.github.com/users/skinnyworm/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/skinnyworm/subscriptions","organizations_url":"https://api.github.com/users/skinnyworm/orgs","repos_url":"https://api.github.com/users/skinnyworm/repos","events_url":"https://api.github.com/users/skinnyworm/events{/privacy}","received_events_url":"https://api.github.com/users/skinnyworm/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":2,"created_at":"2017-04-23T12:35:20Z","updated_at":"2017-04-26T04:28:26Z","closed_at":null,"body":"### Steps to reproduce\r\n\r\nGiven a user is already authenticated by oauth2. \r\n\r\nAnd then he updated his password. \r\n\r\nAfter he authenticated via oauth2 next time, he will not be able to authenticate by password.\r\n\r\n### Root cause of this issue\r\n\r\nIn default OAuth2Verifier, it will try to update existing entity by merging the entity with oauth profile and id. \r\n\r\n```javascript\r\n_updateEntity (entity, data) {\r\n    const options = this.options;\r\n    const name = options.name;\r\n    const id = entity[this.service.id];\r\n    debug(`Updating ${options.entity}: ${id}`);\r\n\r\n    const newData = {\r\n      [options.idField]: data.profile.id,\r\n      [name]: data\r\n    };\r\n\r\n    // Merge existing user data with new profile data\r\n    const updated = merge({}, entity, newData);\r\n    return this.service.update(id, updated, { oauth: { provider: name } });\r\n  }\r\n```\r\n\r\nSince normally the update method of user service will have a before hook to hash the password. In above case, the entity is fetched from user service which contain a hashed password. Update it will cause the hashed password to be hashed again. So the original password can no longer be authenticated.\r\n\r\n### Suggestion\r\n\r\nMaybe we should use __patch__ method instead of __update___ method."},"comment":{"url":"https://api.github.com/repos/feathersjs/feathers-authentication-oauth2/issues/comments/297234695","html_url":"https://github.com/feathersjs/feathers-authentication-oauth2/issues/19#issuecomment-297234695","issue_url":"https://api.github.com/repos/feathersjs/feathers-authentication-oauth2/issues/19","id":297234695,"user":{"login":"ekryski","id":599019,"avatar_url":"https://avatars0.githubusercontent.com/u/599019?v=3","gravatar_id":"","url":"https://api.github.com/users/ekryski","html_url":"https://github.com/ekryski","followers_url":"https://api.github.com/users/ekryski/followers","following_url":"https://api.github.com/users/ekryski/following{/other_user}","gists_url":"https://api.github.com/users/ekryski/gists{/gist_id}","starred_url":"https://api.github.com/users/ekryski/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ekryski/subscriptions","organizations_url":"https://api.github.com/users/ekryski/orgs","repos_url":"https://api.github.com/users/ekryski/repos","events_url":"https://api.github.com/users/ekryski/events{/privacy}","received_events_url":"https://api.github.com/users/ekryski/received_events","type":"User","site_admin":false},"created_at":"2017-04-26T04:28:26Z","updated_at":"2017-04-26T04:28:26Z","body":"Hmm... can't remember if I had a reason for using `update`. Usually if it's something quirky I leave comments in the code so I don't think I did have reason.\r\n\r\nThe only reason I have the `merge` in here manually is to handle deep objects. I can't remember if I started with `patch` and ran into an issue with that or was just using `update` from the get-go but I'm not sure if `patch` handles deep merging in every DB adapter. @daffl can remember off hand?\r\n\r\nAs @skinnyworm suggests I think `patch` is much better here, we just need to make sure that entities with deep object changes (ie. entity.facebook.friends) are respected."}}
{"actor":{"display_login":"ekryski"}}