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

Updating User Attached to Params in Client #102

Closed
arjun-io opened this Issue Mar 14, 2016 · 9 comments

Comments

Projects
None yet
4 participants
@arjun-io
Copy link

arjun-io commented Mar 14, 2016

I'm a little lost about how the user gets attached to the request params on the client. Right now, I'm updating my user and then saving that updated user to storage:

await this.app.service('storage').patch('user', {value: userResponse});

When I send another request to my server, the updated user isn't attached via params, instead it's still the old user data.

I'm using socketio on React Native if that's any help.

@daffl

This comment has been minimized.

Copy link
Member

daffl commented Mar 14, 2016

There is a difference between the user on the client and params.user on the server. The user on the client is mainly just for using it in your hooks and services before sending a request to the server. The only user information that is shared with the server is the JWT which contains the user id.

To update the user on the server you have to send a patch to /users/<userid>.

@ekryski

This comment has been minimized.

Copy link
Member

ekryski commented Mar 14, 2016

I can totally see how this is confusing on the client though. If you want to update the user on the server you have to do as @daffl said. If you want to update the user on the client you have to persist your changes back to localStorage. That's about to get easier with #101.

@arjun-io

This comment has been minimized.

Copy link
Author

arjun-io commented Mar 14, 2016

Sorry, I don't think I explained my flow fully. Basically what I'm doing is

Send updates to the server -> If the response is not an error, persist that update in my clients localStorage.

Then, when I send a another request to the server, the hook.params.user field seems to already be populated on the server side hook (i.e. Logging hook.params shows a user before any other Auth hooks are run), but it is populated with an non-updated user. I guess my question is, how is that user being populated in the server side hook? It seems that it exists even before auth.populateUser runs.

@ekryski

This comment has been minimized.

Copy link
Member

ekryski commented Mar 15, 2016

@arao456 hmmm that does seem weird. I figured out what's going on though. It only effects sockets.

The user gets populated when the socket authenticates on the server side. With REST it happens with every request via the populateUser hook but because with sockets the user is already attached via middleware, this line is causing it to use the original user object at authentication time.

Man, good find here! Sorry for the nasty bug. I will patch that tonight or tomorrow.

@ekryski ekryski added the Bug label Mar 15, 2016

@arjun-io

This comment has been minimized.

Copy link
Author

arjun-io commented Mar 15, 2016

@ekryski Glad I could help! I saw that line so I suspected the user was already attached to params, I just couldn't figure out where it was being attached. Good to know that it was via the middleware

@arjun-io

This comment has been minimized.

Copy link
Author

arjun-io commented Mar 18, 2016

@ekryski Any updates on this? Or alternatively, I'd be happy to make a PR myself if you can point me in the direction of the fix. One option it seems is to just remove that line and force the re-retrieval of the user. Another option I'm seeing is here: https://github.com/feathersjs/feathers-authentication/blob/master/src/client/index.js#L37. Is this what is setting the user in the socket for future params?

@ekryski

This comment has been minimized.

Copy link
Member

ekryski commented Mar 18, 2016

@arao456 sorry mate. We got hit by the Product Hunt and Hacker News tsunamis. I'm looking into this right meow 😸

@ekryski ekryski modified the milestone: 1.0 release Mar 19, 2016

ekryski added a commit that referenced this issue Mar 21, 2016

@ekryski ekryski referenced this issue Mar 21, 2016

Merged

v0.6 - Bugs fixes, new hooks, and hook tests #109

10 of 10 tasks complete

@ekryski ekryski closed this in 8a3f377 Mar 24, 2016

@ekryski ekryski modified the milestones: 1.0, 0.6 Mar 26, 2016

@jerfowler

This comment has been minimized.

Copy link
Contributor

jerfowler commented May 11, 2017

This looks like it's broke again in Auk, params.user isn't being updated on socket.io connections.

@ekryski

This comment has been minimized.

Copy link
Member

ekryski commented May 14, 2017

@jerfowler you are right. We have another issue tracking that #293. It's a totally new architecture. This is a known issue that @daffl was planning on tackling but I think got lost in the shuffle. I'm working on a bunch of auth fixes right now that are going to have to be a major release so we might try and get this in as part of it.

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.