feathersjs / feathers

The API and real-time application framework
https://feathersjs.com
MIT License
14.97k stars 744 forks source link

`context.params.user` will differ between rest/socketio calls if user is modified on `authentication/create` #3265

Closed viters closed 9 months ago

viters commented 10 months ago

Due to how socketio and rest authentication are built, and how authenticate hook works, in case you are mutating user result for requests that have provider, you will have different results in context.params.user for socketio and for rest.

Let's consider you have user.secretAdministrativeNote field, which you are able to access internally (when provider == null), but it will not be visible for external providers.

authentication/create is called for both rest and socketio, both will receive params.provider (rest and socketio respectively). This will eventually reach JWTStrategy.authenticate and JWTStrategy.getEntity. getEntity behaves differently if the provider is set or not. For authenticate/create case, provider is set, therefore we will receive user entity in a way that user is able to see that, without secretAdministrativeNote. This is expected, as we are returning that entity after authenticate/create is finished back to user. Although, here lies the trap: for socketio connections, authenticate/create sets the authentication result on connection, via @feathersjs/authentication/src/jwt.ts:64. Therefore, authenticate/create result is set on connection forever, getting context.params.user.secretAdministrativeNote will yield null on socketio transport in all hooks afterwards.

Meanwhile, rest transport works differently, authentication/create will result in a token and user entity, user should not see secretAdministrativeNote on response - that is achieved successfully. Subsequent requests with authenticate hook, will be going through JWTStrategy.authenticate, but without provider (see @feathersjs/authentication/src/hooks/authenticate.ts:52). Therefore, user that should be set on params will be gathered with "administrative" privileges (@feathersjs/authentication/src/jwt.ts:121). Therefore, getting context.params.user.secretAdministrativeNote will yield actual value on rest transport in all hooks afterwards.

This will of course apply to any other modifications on user done on authenticate/create.

I do not have minimal example yet, this are my findings after reading the code for authentication and socketio transport, and debugging similar case today. I think it might be a result of tight coupling between setting context.params.user and authentication/create result.

daffl commented 10 months ago

This is one of the reasons why v5 has introduced external resolvers. They make sure that external requests always receive the safe data without having to check for a provider while internal calls (like the authenticate hook) receive the full data.