fix(hooks): Migrate built-in hooks and allow backwards compatibility (#…

…2358)
feathersjs · May 22, 2021 · 759c5a1 · 759c5a1
1 parent 88e1329
commit 759c5a1
Show file tree

Hide file tree

Showing 14 changed files with 122 additions and 157 deletions.
diff --git a/packages/authentication-client/src/hooks/authentication.ts b/packages/authentication-client/src/hooks/authentication.ts
@@ -1,20 +1,18 @@
-import { HookContext } from '@feathersjs/feathers';
+import { HookContext, NextFunction } from '@feathersjs/feathers';
 import { stripSlashes } from '@feathersjs/commons';
 
 export const authentication = () => {
-  return (context: HookContext) => {
+  return (context: HookContext, next: NextFunction) => {
     const { app, params, path, method, app: { authentication: service } } = context;
 
     if (stripSlashes(service.options.path) === path && method === 'create') {
-      return context;
+      return next();
     }
 
     return Promise.resolve(app.get('authentication')).then(authResult => {
       if (authResult) {
         context.params = Object.assign({}, authResult, params);
       }
-
-      return context;
-    });
+    }).then(next);
   };
 };
diff --git a/packages/authentication-client/src/hooks/populate-header.ts b/packages/authentication-client/src/hooks/populate-header.ts
@@ -1,7 +1,7 @@
-import { HookContext } from '@feathersjs/feathers';
+import { HookContext, NextFunction } from '@feathersjs/feathers';
 
 export const populateHeader = () => {
-  return (context: HookContext) => {
+  return (context: HookContext, next: NextFunction) => {
     const { app, params: { accessToken } } = context;
     const authentication = app.authentication;
 
@@ -15,6 +15,6 @@ export const populateHeader = () => {
       }, context.params.headers);
     }
 
-    return context;
+    return next();
   };
 };
diff --git a/packages/authentication-client/src/index.ts b/packages/authentication-client/src/index.ts
@@ -52,14 +52,10 @@ const init = (_options: Partial<AuthenticationClientOptions> = {}) => {
     app.reAuthenticate = authentication.reAuthenticate.bind(authentication);
     app.logout = authentication.logout.bind(authentication);
 
-    app.hooks({
-      before: {
-        all: [
-          hooks.authentication(),
-          hooks.populateHeader()
-        ]
-      }
-    });
+    app.hooks([
+      hooks.authentication(),
+      hooks.populateHeader()
+    ]);
   };
 };
 

diff --git a/packages/authentication-local/src/hooks/hash-password.ts b/packages/authentication-local/src/hooks/hash-password.ts
@@ -3,7 +3,7 @@ import set from 'lodash/set';
 import cloneDeep from 'lodash/cloneDeep';
 import { BadRequest } from '@feathersjs/errors';
 import { createDebug } from '@feathersjs/commons';
-import { HookContext } from '@feathersjs/feathers';
+import { HookContext, NextFunction } from '@feathersjs/feathers';
 import { LocalStrategy } from '../strategy';
 
 const debug = createDebug('@feathersjs/authentication-local/hooks/hash-password');
@@ -18,47 +18,42 @@ export default function hashPassword (field: string, options: HashPasswordOption
     throw new Error('The hashPassword hook requires a field name option');
   }
 
-  return async (context: HookContext<any, any>) => {
-    if (context.type !== 'before') {
-      throw new Error('The \'hashPassword\' hook should only be used as a \'before\' hook');
-    }
-
+  return async (context: HookContext<any, any>, next?: NextFunction) => {
     const { app, data, params } = context;
 
-    if (data === undefined) {
-      debug('hook.data is undefined. Skipping hashPassword hook.');
-      return context;
-    }
+    if (data !== undefined) {
+      const authService = app.defaultAuthentication(options.authentication);
+      const { strategy = 'local' } = options;
 
-    const authService = app.defaultAuthentication(options.authentication);
-    const { strategy = 'local' } = options;
+      if (!authService || typeof authService.getStrategies !== 'function') {
+        throw new BadRequest('Could not find an authentication service to hash password');
+      }
 
-    if (!authService || typeof authService.getStrategies !== 'function') {
-      throw new BadRequest('Could not find an authentication service to hash password');
-    }
+      const [ localStrategy ] = authService.getStrategies(strategy) as LocalStrategy[];
 
-    const [ localStrategy ] = authService.getStrategies(strategy) as LocalStrategy[];
+      if (!localStrategy || typeof localStrategy.hashPassword !== 'function') {
+        throw new BadRequest(`Could not find '${strategy}' strategy to hash password`);
+      }
 
-    if (!localStrategy || typeof localStrategy.hashPassword !== 'function') {
-      throw new BadRequest(`Could not find '${strategy}' strategy to hash password`);
-    }
+      const addHashedPassword = async (data: any) => {
+        const password = get(data, field);
 
-    const addHashedPassword = async (data: any) => {
-      const password = get(data, field);
+        if (password === undefined) {
+          debug(`hook.data.${field} is undefined, not hashing password`);
+          return data;
+        }
 
-      if (password === undefined) {
-        debug(`hook.data.${field} is undefined, not hashing password`);
-        return data;
-      }
+        const hashedPassword: string = await localStrategy.hashPassword(password, params);
 
-      const hashedPassword: string = await localStrategy.hashPassword(password, params);
+        return set(cloneDeep(data), field, hashedPassword);
+      }
 
-      return set(cloneDeep(data), field, hashedPassword);
+      context.data = Array.isArray(data) ? await Promise.all(data.map(addHashedPassword)) :
+        await addHashedPassword(data);
     }
 
-    context.data = Array.isArray(data) ? await Promise.all(data.map(addHashedPassword)) :
-      await addHashedPassword(data);
-
-    return context;
+    if (typeof next === 'function') {
+      await next();
+    }
   };
 }
diff --git a/packages/authentication-local/src/hooks/protect.ts b/packages/authentication-local/src/hooks/protect.ts
@@ -1,8 +1,7 @@
 import omit from 'lodash/omit';
-import { HookContext } from '@feathersjs/feathers';
+import { HookContext, NextFunction } from '@feathersjs/feathers';
 
-export default (...fields: string[]) => (context: HookContext<any, any>) => {
-  const result = context.dispatch || context.result;
+export default (...fields: string[]) => async (context: HookContext<any, any>, next?: NextFunction) => {
   const o = (current: any) => {
     if (typeof current === 'object' && !Array.isArray(current)) {
       const data = typeof current.toJSON === 'function'
@@ -14,23 +13,25 @@ export default (...fields: string[]) => (context: HookContext<any, any>) => {
     return current;
   };
 
-  if (!result) {
-    return context;
+  if (typeof next === 'function') {
+    await next();
   }
 
-  if (Array.isArray(result)) {
-    context.dispatch = result.map(o);
-  } else if (result.data && context.method === 'find') {
-    context.dispatch = Object.assign({}, result, {
-      data: result.data.map(o)
-    });
-  } else {
-    context.dispatch = o(result);
-  }
+  const result = context.dispatch || context.result;
 
-  if (context.params && context.params.provider) {
-    context.result = context.dispatch;
-  }
+  if (result) {
+    if (Array.isArray(result)) {
+      context.dispatch = result.map(o);
+    } else if (result.data && context.method === 'find') {
+      context.dispatch = Object.assign({}, result, {
+        data: result.data.map(o)
+      });
+    } else {
+      context.dispatch = o(result);
+    }
 
-  return context;
+    if (context.params && context.params.provider) {
+      context.result = context.dispatch;
+    }
+  }
 };
diff --git a/packages/authentication-local/test/fixture.ts b/packages/authentication-local/test/fixture.ts
@@ -37,20 +37,20 @@ export function createApplication (app = feathers<ServiceTypes>()) {
     }
   }));
 
+  app.service('users').hooks([
+    protect('password')
+  ]);
   app.service('users').hooks({
-    before: {
-      create: [ hashPassword('password') ]
-    },
-    after: {
-      all: [ protect('password') ],
-      get: [context => {
-        if (context.params.provider) {
-          context.result.fromGet = true;
-        }
-
-        return context;
-      }]
-    }
+    create: [
+      hashPassword('password')
+    ],
+    get: [ async (context, next) => {
+      await next();
+
+      if (context.params.provider) {
+        context.result.fromGet = true;
+      }
+    }]
   });
 
   return app;

diff --git a/packages/authentication-local/test/hooks/hash-password.test.ts b/packages/authentication-local/test/hooks/hash-password.test.ts
@@ -55,28 +55,6 @@ describe('@feathersjs/authentication-local/hooks/hash-password', () => {
     }
   });
 
-  it('errors when authentication strategy does not exist', async () => {
-    const users = app.service('users');
-
-    users.hooks({
-      after: {
-        create: hashPassword('password')
-      }
-    });
-
-    try {
-      await users.create({
-        email: 'dave@hashpassword.com',
-        password: 'supersecret'
-      });
-      assert.fail('Should never get here');
-    } catch (error) {
-      assert.strictEqual(error.message,
-        'The \'hashPassword\' hook should only be used as a \'before\' hook'
-      );
-    }
-  });
-
   it('hashes password on field', async () => {
     const password = 'supersecret';