Merge pull request #226 from jd1378/skip_hash_password

claustres · web-flow · commit f1e81d2f48bf · 2024-07-08T14:30:16.000+02:00
feat: add skipPasswordHash option
diff --git a/src/methods/password-change.ts b/src/methods/password-change.ts
@@ -40,6 +40,7 @@ export default async function passwordChange (
     app,
     identifyUserProps,
     passwordField,
+    skipPasswordHash,
     sanitizeUserForClient,
     service,
     notifier
@@ -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);
diff --git a/src/methods/reset-password.ts b/src/methods/reset-password.ts
@@ -85,6 +85,7 @@ async function resetPassword (
     skipIsVerifiedCheck,
     reuseResetToken,
     passwordField,
+    skipPasswordHash,
     sanitizeUserForClient,
     notifier
   } = options;
@@ -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,
diff --git a/src/methods/verify-signup-set-password.ts b/src/methods/verify-signup-set-password.ts
@@ -85,6 +85,7 @@ async function verifySignupSetPassword (
   const {
     app,
     passwordField,
+    skipPasswordHash,
     sanitizeUserForClient,
     service,
     notifier
@@ -117,6 +118,7 @@ async function verifySignupSetPassword (
     isDateAfterNow(user.verifyExpires),
     user.verifyChanges || {},
     password,
+    skipPasswordHash,
     params
   );
 
@@ -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(
diff --git a/src/options.ts b/src/options.ts
@@ -26,6 +26,7 @@ export const optionsDefault: AuthenticationManagementServiceOptions = {
   sanitizeUserForClient,
   skipIsVerifiedCheck: false,
   passwordField: 'password',
+  skipPasswordHash: false,
   passParams: undefined
 };
 
diff --git a/src/services/PasswordChangeService.ts b/src/services/PasswordChangeService.ts
@@ -21,6 +21,7 @@ export class PasswordChangeService
       'identifyUserProps',
       'sanitizeUserForClient',
       'passwordField',
+      'skipPasswordHash',
       'passParams'
     ]);
     this.options = Object.assign(defaultOptions, options);
diff --git a/src/services/ResetPwdLongService.ts b/src/services/ResetPwdLongService.ts
@@ -21,6 +21,7 @@ export class ResetPwdLongService
       'reuseResetToken',
       'sanitizeUserForClient',
       'passwordField',
+      'skipPasswordHash',
       'passParams'
     ]);
     this.options = Object.assign(defaultOptions, options);
diff --git a/src/services/ResetPwdShortService.ts b/src/services/ResetPwdShortService.ts
@@ -22,6 +22,7 @@ export class ResetPwdShortService
       'reuseResetToken',
       'sanitizeUserForClient',
       'passwordField',
+      'skipPasswordHash',
       'identifyUserProps',
       'passParams'
     ]);
diff --git a/src/services/VerifySignupSetPasswordLongService.ts b/src/services/VerifySignupSetPasswordLongService.ts
@@ -18,6 +18,7 @@ export class VerifySignupSetPasswordLongService
       'notifier',
       'sanitizeUserForClient',
       'passwordField',
+      'skipPasswordHash',
       'passParams'
     ]);
     this.options = Object.assign(defaultOptions, options);
diff --git a/src/services/VerifySignupSetPasswordShortService.ts b/src/services/VerifySignupSetPasswordShortService.ts
@@ -19,6 +19,7 @@ export class VerifySignupSetPasswordShortService
       'notifier',
       'sanitizeUserForClient',
       'passwordField',
+      'skipPasswordHash',
       'identifyUserProps',
       'passParams'
     ]);
diff --git a/src/types.ts b/src/types.ts
@@ -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>
 }
@@ -139,6 +141,7 @@ export type VerifySignupSetPasswordLongServiceOptions = Pick<AuthenticationManag
 'sanitizeUserForClient' |
 'notifier' |
 'passwordField' |
+'skipPasswordHash' |
 'passParams'>;
 export type VerifySignupSetPasswordOptions = VerifySignupSetPasswordLongServiceOptions & { app: Application };
 
@@ -148,6 +151,7 @@ export type PasswordChangeServiceOptions = Pick<AuthenticationManagementServiceO
 'notifier' |
 'sanitizeUserForClient' |
 'passwordField' |
+'skipPasswordHash' |
 'passParams'>;
 export type PasswordChangeOptions = PasswordChangeServiceOptions & { app: Application };
 
@@ -162,6 +166,7 @@ export type ResetPasswordServiceOptions = Pick<AuthenticationManagementServiceOp
 'notifier' |
 'sanitizeUserForClient' |
 'passwordField' |
+'skipPasswordHash' |
 'passParams'>;
 export type ResetPasswordOptions = ResetPasswordServiceOptions & { app: Application };
 
diff --git a/test/methods/password-change.test.ts b/test/methods/password-change.test.ts
@@ -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');
+          });
+ 
+        });
+      });
+    })
   });
 });
 
diff --git a/test/methods/reset-pwd-short.test.ts b/test/methods/reset-pwd-short.test.ts
@@ -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'
+            );
+          });
+        });
       });
     });
   });
diff --git a/test/methods/verify-signup-set-password-short.test.ts b/test/methods/verify-signup-set-password-short.test.ts
diff --git a/test/scaffolding.test.ts b/test/scaffolding.test.ts
