Loopback: When updating the user password, the password is stored in not hashed

[LoicMahieu]

Step to reproduce:

• Create a sample app slc loopback
• Expose User model with public: true
• Create a new user: POST /Users { "password": "foo", "email": "[email protected]" }
• Try to authenticate with password foo
• It success
• Update the user with new password: PUT /Users/1 { "password": "bar" }
• Try to authenticate with password bar
• It fails
  

  Diagnostic:

It seems that the setter results (see UserModel.setter.password) is not used as "data" for the connector, see: DataAccessObject.prototype.updateAttributes . So I think that setter results is never saved in db.

It is really critic because it causes the User model not usable.

Versions:

├─┬ [email protected]
│ ├── [email protected]
│ ├── [email protected]
│ └── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
└── [email protected]

[0candy]

I am not sure if you are using explorer. But if you are, after you authenticate the first time, you need to set the access token at the top of the explorer page before you can update the password.
https://docs.strongloop.com/display/public/LB/Introduction+to+User+model+authentication

Same with if you do it in the code, you need to have the authorization token for the user to submit the request.
https://docs.strongloop.com/display/public/LB/Logging+in+users

The title of your issue is different from what you're asking in the description. Are you saying that the password value is not being stored as hash or are you having issues updating the user password?

[ghost]

I'm seeing this same issue as well. The POST method hashes the password field and saves the entry correctly. The PUT method calls the UserModel.setter.password method on the User which hashes the password but the database entry is stored in plain text. This obviously breaks the users ability to log into the system.

Loopback version 2.26.2

[LoicMahieu]

@0candy Sorry if it wasn't clear. I describe a "login" procedure to illustrate the problem. I am aware about token mechanism of loopback.

To simplify:

• Update the user with new password: PUT /Users/1 { "password": "bar" }
• Look at the DB, the password is stored in clear.

[jonicas]

+1

[0candy]

@LoicMahieu Thank you for pointing out the issue. We will take a look.

[arealmaas]

+1

Which version was this introduced?

[glesage]

Maybe something in this commit: https://github.com/strongloop/loopback-datasource-juggler/commit/8ca67c3d48952b972bc2e2c1ee4c8c93b389f99c ?

[jnowland]

+1 It's locking out users.

[shuckepco]

+1

Worked around this issue by calling hashPassword prior to updateAttributes in the remote method.

[juanscolari]

+1

[notbrain]

+1 this is Really Bad. How did this pass tests?

Reverting to "loopback-datasource-juggler": "2.41.1" was the fix for us. Just removed the ^ prefix (so much for semantic versioning!).

[arealmaas]

2.44.0 should work as well.

[ebarault]

god, we just hist this issue, anyone trying to reset his password is locked out...
confirmed: 2.44.0 works

[bajtos]

This is an unintended regression, please accept our apologies. I have published [email protected] that should fix the problem.

+1 this is Really Bad. How did this pass tests?

As it turns out, there is no test for this specific use case. I'd say this whole situation is a good educational example for all of us (both maintainers and community contributors), why it's important to maintain high test coverage.

I'd like to keep this issue open until we add unit-tests that would have failed on 2.45.0, @Amir-61 offered to write them.

[LoicMahieu]

@bajtos Thanks for having fixed the issue.
I suppose the commit solving the issue is: https://github.com/strongloop/loopback-datasource-juggler/commit/dbdf9153346ed3aabc925eddf6280e19df519cd3

Yes, it demonstrate well that coverage is essential. Furthermore when it's related to core feature of loopback.

[vicky-gonsalves]

+1

[Amir-61]

Connect to https://github.com/strongloop/loopback-datasource-juggler/pull/893