-
Notifications
You must be signed in to change notification settings - Fork 98
Password hash on reset differs from account creation #92
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Do you have a hashPassword hook on patch after? If so, do you condition it so it doesn't rehash the password written by the repo, which password is already hashed. |
Consider this: before: {
all: [],
find: [ authenticate('jwt') ],
get: [ authenticate('jwt') ],
create: [
hashPassword(),
verifyHooks.addVerification()
],
update: [
commonHooks.disallow('external')
],
patch: [
commonHooks.iff(
commonHooks.isProvider('external'),
commonHooks.preventChanges(
'email',
'isVerified',
'verifyToken',
'verifyShortToken',
'verifyExpires',
'verifyChanges',
'resetToken',
'resetShortToken',
'resetExpires'
),
hashPassword(),
authenticate('jwt')
)
],
remove: [ authenticate('jwt') ]
},``` |
Ok, I think that was probably it. I was hashing password every time on patch. This is my user.hooks.js file, I'd really appreciate a quick review to see if this all makes sense. I find it hard to get my head around what I really need in here. const { authenticate } = require("@feathersjs/authentication").hooks;
const commonHooks = require("feathers-hooks-common");
const { restrictToOwner } = require("feathers-authentication-hooks");
const {
addVerification,
removeVerification
} = require("feathers-authentication-management").hooks;
const {
hashPassword,
protect
} = require("@feathersjs/authentication-local").hooks;
const restrictToAuthenticatedOwners = [
authenticate("jwt"),
restrictToOwner({
idField: "_id",
ownerField: "_id"
})
];
const preventAuthManagementChanges = commonHooks.iff(
commonHooks.isProvider("external"),
commonHooks.preventChanges(
"email",
"isVerified",
"verifyToken",
"verifyShortToken",
"verifyExpires",
"verifyChanges",
"resetToken",
"resetShortToken",
"resetExpires"
),
hashPassword(),
authenticate("jwt")
);
const sendEmailVerification = require("../../hooks/send-verification-email");
module.exports = {
before: {
all: [],
find: [authenticate("jwt")],
get: [...restrictToAuthenticatedOwners],
create: [
hashPassword(),
addVerification() // adds .isVerified, .verifyExpires, .verifyToken, .verifyChanges
],
update: [preventAuthManagementChanges, ...restrictToAuthenticatedOwners],
patch: [preventAuthManagementChanges, ...restrictToAuthenticatedOwners],
remove: [...restrictToAuthenticatedOwners]
},
after: {
all: [
commonHooks.when(
hook => hook.params.provider, // protect for everything except internal (undefined)
protect("password")
)
],
find: [],
get: [],
create: [
protect("password"),
sendEmailVerification(),
removeVerification() // removes verification/reset fields other than .isVerified
],
update: [],
patch: [],
remove: []
},
error: {
all: [],
find: [],
get: [],
create: [],
update: [],
patch: [],
remove: []
}
}; |
Steps to reproduce
(everything good here)
Expected behavior
After a password reset, user should be able to log in with the new password and the hash should be the same as a good known password hash for the same password
Actual behavior
After a password reset, user cannot log into account with valid password
System configuration
Windows 10, Node 8.9.4
The text was updated successfully, but these errors were encountered: