feathersjs / feathers

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

Password (hashed) is still passed back to user in /authentication calls. #3345

Closed jamesvillarrubia closed 1 month ago

jamesvillarrubia commented 7 months ago

Steps to reproduce

  1. Generate a basic app with local authentication.
  2. Create a user.
  3. Hit /authentication with proper credentials
  4. Receive response that includes hashed password.
    {
    "accessToken": "eyJh....anQ",
    "authentication": {
        "strategy": "local",
    },
    "user": {
        "id": 1,
        "email": "test@example.com",
        "password": "$2.....f4JHK1fLJ/i",
    }
    }

Expected behavior

Local Auth strategy return user without password hash.

Actual behavior

Local Auth strategy returns hashed password.

Investigation

From what I can tell the content.dispatch is only used in the http library, but in the authentication getEntity function, it's assuming that the resolver has removed sensitive fields. Since the request is internal and never gets to the http line, the user information is passed in whole. https://github.com/feathersjs/feathers/blob/82d30fd37914e61935e068e89fc389f6bf47aaad/packages/transport-commons/src/http.ts#L82

I have added an around hook to compensate but this seems like the wrong strategy.

  app.service(userPath).hooks({
    around: {
      all: [
        async(context, next)=>{
           await next();
           if(context.dispatch && context.params.provider){
             context.result = context.dispatch
           }
        },
        schemaHooks.resolveExternal(userExternalResolver), 
        schemaHooks.resolveResult(userResolver)
      ],

Options

1) Manually strip out the field in the Local Strategy, but leave internal calls with access to the user information, including password 2) Write a hook to set result to dispatch as a default. 3) Use a resolver in the Auth hooks to strip out the password.

daffl commented 7 months ago

The authentication call will also return the safe external representation of the user object. I just tested it with a newly generated application and for me it is returning the data as expected:

Screenshot 2023-11-24 at 16 54 35
Complexlity commented 7 months ago

Just to add @jamesvillarrubia , It'll be a good idea to put up a stackblitz repro or an example repo that could be run locally. Because it works as expected for me as well