Potentially dangerous code

[DScheglov]

Example/application/api/auth.2/signin.js

Line 7 in 8a50545

const valid = await metarhia.metautil.validatePassword(password, hash);

The plain password must not be transferred to the 3rd-party code.
This code must use node-embedded functions to get hash and compare it with the stored one.
By the way the password must be salted.

[tshemsedinov]

@DScheglov Thanks for reporting but matautil is not 3rd party code, it's a part of metarhia https://github.com/metarhia/metautil/blob/f3f7adee3b8a66e5375270e9363da9e0272fc2f1/lib/crypto.js#L111-L123 it uses node native crypto.scrypt. Parameter hash contains serialized salt, see: https://github.com/metarhia/metautil/blob/f3f7adee3b8a66e5375270e9363da9e0272fc2f1/lib/crypto.js#L75-L84 as well as hash algorithm name, and its options. Here is an example: $scrypt$N=32768,r=8,p=1,maxmem=67108864$XcD5Zfk+BVIGEyiksBjjy9LL42AFOOqlhEB650woECs$3CNOs25gOVV8AZMYQc6bFnrYdM+3xP996shxJEq5LxGt4gs1g9cocZmi/SYg/H16egY4j7qxTD/oygyEI80cgg

[DScheglov]

@tshemsedinov

Sure, from Metarhia point of view metautils is not a 3rd party code.
But for users of impress and metarhia -- it is exactly third part code.

It is great that password is really salted before saving -- so it is not a concern any more.
However the naming is (a concern).

Also, it is recommended to use a pepper to mitigate risks of leaking all parts of the hash function input in case of db leak. The leak of whole hash function input doesn't allow to restore passwords for all large user base, but it allows to do that for a specific user or for any relatively small group of users.

It is obvious that pepper could be added to the password, but it is also is not safe, because sensitive parameter is passed to the 3rd party code.

To use or not to use the pepper is a matter of the user (of your framwork) risk policy, but the framework should not lock or even provocate avoiding this approach.

Finally, It is ok that metrhia provides password verification service, however, considering license agreement (decline of responsibility) correspondent methods should be named as _insecure to inform users about potential risks.

At least the example code must contain the correspondent remark with explicite decline of responsibility.