Skip to content

scrypt#verify should not return error on password mismatch #75

@alex94cp

Description

@alex94cp

The current behaviour of scrypt#verify(async) of triggering an error if password mismatch is not coherent with the sync version. I strongly think that scrypt#verify (async) should not invoke the callback with an error in this case.

An error should be triggered in the case that the password verification has not been able to take place, and therefore information about whether password matches or not is basically undefined. Consider, for example, what would happen if an out of memory error were triggered from scrypt (I don't know if it does, it's just an example): I'd have to check err.scrypt_err_code for each particular error code to know if password match info is usable or not; now adding an error to scrypt would make it forward-incompatible (would need a new major version) and my application would be forever tied to a particular scrypt version.

Even from a design point of view, scrypt shouldn't impose to its consumers that a password mismatch should be an error. It MAY be an error in the consumer application, and it would treat is as such like this:

scrypt.verify(pwHass, passGiven, function(err, matches) {
    if (err)
        return cb(err);
    if (!matches)
        return cb(new Error('password mismatch'));
    return cb(null, userInfoOrSomething);
});

But it may be not an error, like in my use case, where a password mismatch does not return an error, and instead triggers an onAccessDenied callback, where the user decides what to do (ie: treating it as anonymous). With the current design, I'm forced to use scrypt like this:

scrypt.verify(pwHash, passGiven, function(err, matches) {
    if (err && err.scrypt_err_code !== 11)
        return cb(err);
    return matches ? onAccessGranted(userInfo) : onAccessDenied(credsGiven);
});

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions