Skip to content
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

feat: add skipPasswordHash option #226

Merged
merged 4 commits into from
Jul 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/methods/password-change.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export default async function passwordChange (
app,
identifyUserProps,
passwordField,
skipPasswordHash,
sanitizeUserForClient,
service,
notifier
Expand Down Expand Up @@ -67,7 +68,7 @@ export default async function passwordChange (
}

const patchedUser = await usersService.patch(user[usersServiceId] as Id, {
password: await hashPassword(app, password, passwordField)
password: skipPasswordHash ? password : await hashPassword(app, password, passwordField)
}, Object.assign({}, params)) as User;

const userResult = await notify(notifier, 'passwordChange', patchedUser, notifierOptions);
Expand Down
3 changes: 2 additions & 1 deletion src/methods/reset-password.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ async function resetPassword (
skipIsVerifiedCheck,
reuseResetToken,
passwordField,
skipPasswordHash,
sanitizeUserForClient,
notifier
} = options;
Expand Down Expand Up @@ -160,7 +161,7 @@ async function resetPassword (
}

const patchedUser = await usersService.patch(user[usersServiceId] as Id, {
[passwordField]: await hashPassword(app, password, passwordField),
[passwordField]: skipPasswordHash ? password : await hashPassword(app, password, passwordField),
resetExpires: null,
resetAttempts: null,
resetToken: null,
Expand Down
8 changes: 5 additions & 3 deletions src/methods/verify-signup-set-password.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ async function verifySignupSetPassword (
const {
app,
passwordField,
skipPasswordHash,
sanitizeUserForClient,
service,
notifier
Expand Down Expand Up @@ -117,6 +118,7 @@ async function verifySignupSetPassword (
isDateAfterNow(user.verifyExpires),
user.verifyChanges || {},
password,
skipPasswordHash,
params
);

Expand Down Expand Up @@ -149,17 +151,17 @@ async function verifySignupSetPassword (
isVerified: boolean,
verifyChanges: VerifyChanges,
password: string,
skipPasswordHash: boolean,
params?: Params
): Promise<User> {
const hashedPassword = await hashPassword(app, password, passwordField);


const patchData = Object.assign({}, verifyChanges || {}, {
isVerified,
verifyToken: null,
verifyShortToken: null,
verifyExpires: null,
verifyChanges: {},
[passwordField]: hashedPassword
[passwordField]: skipPasswordHash ? password : await hashPassword(app, password, passwordField)
});

const result = await usersService.patch(
Expand Down
1 change: 1 addition & 0 deletions src/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export const optionsDefault: AuthenticationManagementServiceOptions = {
sanitizeUserForClient,
skipIsVerifiedCheck: false,
passwordField: 'password',
skipPasswordHash: false,
passParams: undefined
};

Expand Down
1 change: 1 addition & 0 deletions src/services/PasswordChangeService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export class PasswordChangeService
'identifyUserProps',
'sanitizeUserForClient',
'passwordField',
'skipPasswordHash',
'passParams'
]);
this.options = Object.assign(defaultOptions, options);
Expand Down
1 change: 1 addition & 0 deletions src/services/ResetPwdLongService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export class ResetPwdLongService
'reuseResetToken',
'sanitizeUserForClient',
'passwordField',
'skipPasswordHash',
'passParams'
]);
this.options = Object.assign(defaultOptions, options);
Expand Down
1 change: 1 addition & 0 deletions src/services/ResetPwdShortService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export class ResetPwdShortService
'reuseResetToken',
'sanitizeUserForClient',
'passwordField',
'skipPasswordHash',
'identifyUserProps',
'passParams'
]);
Expand Down
1 change: 1 addition & 0 deletions src/services/VerifySignupSetPasswordLongService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export class VerifySignupSetPasswordLongService
'notifier',
'sanitizeUserForClient',
'passwordField',
'skipPasswordHash',
'passParams'
]);
this.options = Object.assign(defaultOptions, options);
Expand Down
1 change: 1 addition & 0 deletions src/services/VerifySignupSetPasswordShortService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export class VerifySignupSetPasswordShortService
'notifier',
'sanitizeUserForClient',
'passwordField',
'skipPasswordHash',
'identifyUserProps',
'passParams'
]);
Expand Down
5 changes: 5 additions & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,8 @@ export interface AuthenticationManagementServiceOptions {
/** Property name of the password field on your `'/users'` service
* @default 'password' */
passwordField: string
/** Should we skip hashing password for `passwordField` ? If `true`, password won't be hashed by feathers-authentication-management when patching the user. This must be set to `true` if you are hashing your password field using resolvers. */
skipPasswordHash: boolean
/** Pass params from f-a-m service to `/users` service */
passParams: (params) => Params | Promise<Params>
}
Expand All @@ -139,6 +141,7 @@ export type VerifySignupSetPasswordLongServiceOptions = Pick<AuthenticationManag
'sanitizeUserForClient' |
'notifier' |
'passwordField' |
'skipPasswordHash' |
'passParams'>;
export type VerifySignupSetPasswordOptions = VerifySignupSetPasswordLongServiceOptions & { app: Application };

Expand All @@ -148,6 +151,7 @@ export type PasswordChangeServiceOptions = Pick<AuthenticationManagementServiceO
'notifier' |
'sanitizeUserForClient' |
'passwordField' |
'skipPasswordHash' |
'passParams'>;
export type PasswordChangeOptions = PasswordChangeServiceOptions & { app: Application };

Expand All @@ -162,6 +166,7 @@ export type ResetPasswordServiceOptions = Pick<AuthenticationManagementServiceOp
'notifier' |
'sanitizeUserForClient' |
'passwordField' |
'skipPasswordHash' |
'passParams'>;
export type ResetPasswordOptions = ResetPasswordServiceOptions & { app: Application };

Expand Down
95 changes: 95 additions & 0 deletions test/methods/password-change.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,101 @@ describe('password-change.ts', function () {
});
})
});

[{
name: "authManagement.create",
callMethod: (app: Application, data: DataPasswordChange, params?: ParamsTest) => {
return app.service("authManagement").create(withAction(data), params);
}
}, {
name: "authManagement.passwordChange",
callMethod: (app: Application, data: DataPasswordChange, params?: ParamsTest) => {
return app.service("authManagement").passwordChange(data, params);
}
}, {
name: "authManagement/password-change",
callMethod: (app: Application, data: DataPasswordChange, params?: ParamsTest) => {
return app.service("authManagement/password-change").create(data, params);
}
}].forEach(({ name, callMethod }) => {
describe(`password-change.test.ts ${idType} skipPasswordHash ${name}`, () => {
describe('standard', () => {
let app: Application;
let usersService: MemoryService;

beforeEach(async () => {
app = feathers();
app.use('authentication', authService(app));

const optionsUsers: Partial<MemoryServiceOptions> = {
multi: true,
id: idType
};

app.use("users", new MemoryService(optionsUsers))

app.setup();

usersService = app.service('users');
await usersService.remove(null);
await usersService.create(clone(users));
});

it('with skipPasswordHash false', async () => {
app.configure(authLocalMgnt({
passParams: params => params,
skipPasswordHash: false,
}));
app.use("authManagement/password-change", new PasswordChangeService(app, {
passParams: params => params,
skipPasswordHash: false,
}))


const userRec = clone(users[1]);

const result = await callMethod(app, {
user: {
email: userRec.email
},
oldPassword: userRec.plainPassword,
password: userRec.plainNewPassword
}) as User;
const user = await usersService.get(result[idType]);

assert.strictEqual(result.isVerified, true, 'isVerified not true');
assert.notStrictEqual(user.password, result.plainNewPassword, 'password was not hashed');
});

it('with skipPasswordHash true', async () => {
app.configure(authLocalMgnt({
passParams: params => params,
skipPasswordHash: true,
}));
app.use("authManagement/password-change", new PasswordChangeService(app, {
passParams: params => params,
skipPasswordHash: true,
}))


const userRec = clone(users[1]);

const result = await callMethod(app, {
user: {
email: userRec.email
},
oldPassword: userRec.plainPassword,
password: userRec.plainNewPassword
}) as User;
const user = await usersService.get(result[idType]);

assert.strictEqual(result.isVerified, true, 'isVerified not true');
assert.strictEqual(user.password, result.plainNewPassword, 'password was hashed');
});

});
});
})
});
});

Expand Down
83 changes: 83 additions & 0 deletions test/methods/reset-pwd-short.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,89 @@ const withAction = (
});
});
});

describe('with skipPasswordHash', () => {
let app: Application;
let usersService: MemoryService;
let authLocalMgntService: AuthenticationManagementService;

beforeEach(async () => {
app = feathers();
app.use('authentication', authService(app));

const optionsUsers: Partial<MemoryServiceOptions> = {
multi: true,
id: idType
};
if (pagination === "paginated") {
optionsUsers.paginate = { default: 10, max: 50 };
}
app.use("users", new MemoryService(optionsUsers))

app.setup();

usersService = app.service('users');
await usersService.remove(null);
await usersService.create(clone(users));
});

it("hashes password when skipPasswordHash is false", async () => {
app.configure(
authLocalMgnt({
skipPasswordHash: false,
})
);
app.use("authManagement/reset-password-short", new ResetPwdShortService(app, {
skipPasswordHash: false,
}));
authLocalMgntService = app.service('authManagement');


const result = await callMethod(app, {
token: '00099',
password: '123456',
user: {
email: users[0].email
},
notifierOptions: {transport: 'sms'},
}) as User;
const user = await usersService.get(result[idType]);

assert.notStrictEqual(
user.password,
'123456',
'password was not hashed'
);
});

it("does not hash password when skipPasswordHash is true", async () => {
app.configure(
authLocalMgnt({
skipPasswordHash: true,
})
);
app.use("authManagement/reset-password-short", new ResetPwdShortService(app, {
skipPasswordHash: true,
}));
authLocalMgntService = app.service('authManagement');

const result = await callMethod(app, {
token: '00099',
password: '123456',
user: {
email: users[0].email
},
notifierOptions: {transport: 'sms'},
}) as User;
const user = await usersService.get(result[idType]);

assert.strictEqual(
user.password,
'123456',
'password was hashed'
);
});
});
});
});
});
Expand Down
Loading
Loading