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

Refresh token must have a user ID #419

Merged
merged 3 commits into from Oct 23, 2017

Conversation

Projects
None yet
2 participants
@francisco-sanchez-molina
Copy link

francisco-sanchez-molina commented Feb 13, 2017

No description provided.

@ekryski

This comment has been minimized.

Copy link
Member

ekryski commented Apr 11, 2017

@francisco-sanchez-molina thanks for you the PR! However, this is breaking the tests. Any chance you can update with more details as to what this is fixing and also fix the tests and/or add new ones to cover the use case?

francisco-sanchez-molina added some commits Apr 11, 2017

@francisco-sanchez-molina

This comment has been minimized.

Copy link
Author

francisco-sanchez-molina commented Apr 11, 2017

Hi @ekryski! Thank you for reviewing my PR. The test that fails is not related to my change, and it fails randomly :/.

When you create a token, feathers return a jwt with user id inside payload at first level (payload:
{_id:123}), but when refresh a token user id is located in a sub-level (payload: {data: {_id:123}}):

pasted image at 2017_04_11 09_10 am

And this broke some hooks that try to get user id from token

@ekryski

This comment has been minimized.

Copy link
Member

ekryski commented Jun 8, 2017

Yes I think this is fine. Thanks for updating the PR. I see that the tests are only failing on Node v0.12.0. Since that is no longer a supported version this should be good to be merged. :shipit:

@ekryski ekryski merged commit 5290746 into feathersjs:legacy Oct 23, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
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.