feat: Add AuthenticationBaseStrategy and make authentication option h…

…andling more explicit (#1284)

package.json

@@ -21,7 +21,7 @@
   "scripts": {
     "install": "lerna bootstrap",
     "publish": "lerna publish",
-    "lint": "tslint 'packages/**/*.js' 'packages/**/*.ts' -c tslint.json",
+    "lint": "tslint 'packages/**/*.js' 'packages/**/*.ts' -c tslint.json --fix",
     "test": "npm run lint && nyc lerna run test",
     "test:client": "grunt"
   },

packages/authentication-client/test/integration/fixture.ts

@@ -11,8 +11,10 @@ export default (app: Application) => {
   const authentication = new AuthenticationService(app);
 
   app.set('authentication', {
+    entity: 'user',
+    service: 'users',
     secret: 'supersecret',
-    strategies: [ 'local', 'jwt' ],
+    jwtStrategies: [ 'local', 'jwt' ],
     local: {
       usernameField: 'email',
       passwordField: 'password'

packages/authentication-local/src/strategy.ts

@@ -2,28 +2,14 @@ import bcrypt from 'bcryptjs';
 import { get, omit } from 'lodash';
 import Debug from 'debug';
 import { NotAuthenticated } from '@feathersjs/errors';
-import { AuthenticationStrategy, AuthenticationBase, AuthenticationRequest } from '@feathersjs/authentication';
-import { Application, Query, Params } from '@feathersjs/feathers';
+import { Query, Params } from '@feathersjs/feathers';
+import {
+  AuthenticationRequest, AuthenticationBaseStrategy
+} from '@feathersjs/authentication';
 
 const debug = Debug('@feathersjs/authentication-local/strategy');
 
-export class LocalStrategy implements AuthenticationStrategy {
-  authentication?: AuthenticationBase;
-  app?: Application;
-  name?: string;
-
-  setAuthentication (auth: AuthenticationBase) {
-    this.authentication = auth;
-  }
-
-  setApplication (app: Application) {
-    this.app = app;
-  }
-
-  setName (name: string) {
-    this.name = name;
-  }
-
+export class LocalStrategy extends AuthenticationBaseStrategy {
   verifyConfiguration () {
     const config = this.configuration;
 
@@ -36,16 +22,17 @@ export class LocalStrategy implements AuthenticationStrategy {
 
   get configuration () {
     const authConfig = this.authentication.configuration;
-    const config = authConfig[this.name] || {};
+    const config = super.configuration || {};
 
-    return Object.assign({}, {
+    return {
       hashSize: 10,
       service: authConfig.service,
       entity: authConfig.entity,
       errorMessage: 'Invalid login',
       entityPasswordField: config.passwordField,
-      entityUsernameField: config.usernameField
-    }, config);
+      entityUsernameField: config.usernameField,
+      ...config
+    };
   }
 
   getEntityQuery (query: Query, _params: Params) {

packages/authentication-local/test/fixture.js

@@ -9,8 +9,10 @@ module.exports = (app = feathers()) => {
   const authentication = new AuthenticationService(app);
 
   app.set('authentication', {
+    entity: 'user',
+    service: 'users',
     secret: 'supersecret',
-    strategies: [ 'local', 'jwt' ],
+    jwtStrategies: [ 'local', 'jwt' ],
     local: {
       usernameField: 'email',
       passwordField: 'password'

packages/authentication/src/core.ts

@@ -48,7 +48,7 @@ export interface AuthenticationStrategy {
    * @param authentication The authentication request
    * @param params The service call parameters
    */
-  authenticate (authentication: AuthenticationRequest, params: Params): Promise<AuthenticationResult>;
+  authenticate? (authentication: AuthenticationRequest, params: Params): Promise<AuthenticationResult>;
   /**
    * Parse a basic HTTP request and response for authentication request information.
    * @param req The HTTP request

packages/authentication/src/index.ts

@@ -10,5 +10,6 @@ export {
   AuthenticationResult,
   AuthenticationStrategy
 } from './core';
+export { AuthenticationBaseStrategy } from './strategy';
 export { AuthenticationService } from './service';
 export { JWTStrategy } from './jwt';

packages/authentication/src/jwt.ts

@@ -1,30 +1,15 @@
 import { NotAuthenticated } from '@feathersjs/errors';
-import { AuthenticationStrategy, AuthenticationBase, AuthenticationRequest } from './core';
-import { Application, Params } from '@feathersjs/feathers';
+import { AuthenticationRequest } from './core';
+import { Params } from '@feathersjs/feathers';
 import { IncomingMessage } from 'http';
+import { AuthenticationBaseStrategy } from './strategy';
 
 const SPLIT_HEADER = /(\S+)\s+(\S+)/;
 
-export class JWTStrategy implements AuthenticationStrategy {
-  authentication?: AuthenticationBase;
-  app?: Application;
-  name?: string;
-
-  setAuthentication (auth: AuthenticationBase): void {
-    this.authentication = auth;
-  }
-
-  setApplication (app: Application) {
-    this.app = app;
-  }
-
-  setName (name: string) {
-    this.name = name;
-  }
-
+export class JWTStrategy extends AuthenticationBaseStrategy {
   get configuration () {
     const authConfig = this.authentication.configuration;
-    const config = authConfig[this.name];
+    const config = super.configuration;
 
     return {
       ...config,
@@ -51,11 +36,10 @@ export class JWTStrategy implements AuthenticationStrategy {
    * @param params Service call parameters
    */
   async getEntity (id: string, params: Params) {
-    const { service } = this.configuration;
-    const entityService = this.app.service(service);
+    const entityService = this.entityService;
 
-    if (!entityService) {
-      throw new NotAuthenticated(`Could not find entity service '${service}'`);
+    if (entityService === null) {
+      throw new NotAuthenticated(`Could not find entity service`);
     }
 
     // @ts-ignore

packages/authentication/src/options.ts

@@ -1,7 +1,6 @@
 export default {
-  entity: 'user',
-  service: 'users',
-  strategies: [],
+  httpStrategies: [ 'jwt' ],
+  jwtStrategies: [],
   jwtOptions: {
     header: { typ: 'access' }, // by default is an access token but can be any type
     audience: 'https://yourdomain.com', // The resource server where the token is processed

packages/authentication/src/service.ts

@@ -54,13 +54,13 @@ export class AuthenticationService extends AuthenticationBase implements Service
    * @param params Service call parameters
    */
   async create (data: AuthenticationRequest, params?: Params) {
-    const { strategies } = this.configuration;
+    const { jwtStrategies } = this.configuration;
 
-    if (!strategies.length) {
-      throw new NotAuthenticated('No authentication strategies allowed for creating a JWT');
+    if (!jwtStrategies.length) {
+      throw new NotAuthenticated('No authentication strategies allowed for creating a JWT (`jwtStrategies`)');
     }
 
-    const authResult = await this.authenticate(data, params, ...strategies);
+    const authResult = await this.authenticate(data, params, ...jwtStrategies);
 
     debug('Got authentication result', authResult);
 
@@ -88,7 +88,7 @@ export class AuthenticationService extends AuthenticationBase implements Service
    */
   async remove (id: null|string, params?: Params) {
     const { authentication } = params;
-    const { strategies } = this.configuration;
+    const { jwtStrategies } = this.configuration;
 
     // When an id is passed it is expected to be the authentication `accessToken`
     if (id !== null && id !== authentication.accessToken) {
@@ -97,7 +97,7 @@ export class AuthenticationService extends AuthenticationBase implements Service
 
     debug('Verifying authentication strategy in remove');
 
-    return this.authenticate(authentication, params, ...strategies);
+    return this.authenticate(authentication, params, ...jwtStrategies);
   }
 
   /**
@@ -106,13 +106,17 @@ export class AuthenticationService extends AuthenticationBase implements Service
   setup () {
     // The setup method checks for valid settings and registers the
     // connection and event (login, logout) hooks
-    const { secret, service, entity, entityId, strategies } = this.configuration;
+    const { secret, service, entity, entityId } = this.configuration;
 
     if (typeof secret !== 'string') {
       throw new Error(`A 'secret' must be provided in your authentication configuration`);
     }
 
     if (entity !== null) {
+      if (service === undefined) {
+        throw new Error(`The 'service' options is not set in the authentication configuration`);
+      }
+
       if (this.app.service(service) === undefined) {
         throw new Error(`The '${service}' entity service does not exist (set to 'null' if it is not required)`);
       }
@@ -122,10 +126,6 @@ export class AuthenticationService extends AuthenticationBase implements Service
       }
     }
 
-    if (strategies.length === 0) {
-      throw new Error(`At least one valid authentication strategy required in '${this.configKey}.strategies'`);
-    }
-
     // @ts-ignore
     this.hooks({ after: [ connection(), events() ] });
   }

packages/authentication/src/strategy.ts

@@ -0,0 +1,34 @@
+import { AuthenticationStrategy, AuthenticationBase } from './core';
+import { Application, Service } from '@feathersjs/feathers';
+
+export class AuthenticationBaseStrategy implements AuthenticationStrategy {
+  authentication?: AuthenticationBase;
+  app?: Application;
+  name?: string;
+
+  setAuthentication (auth: AuthenticationBase) {
+    this.authentication = auth;
+  }
+
+  setApplication (app: Application) {
+    this.app = app;
+  }
+
+  setName (name: string) {
+    this.name = name;
+  }
+
+  get configuration () {
+    return this.authentication.configuration[this.name];
+  }
+
+  get entityService (): Service<any> {
+    const { service } = this.configuration;
+
+    if (!service) {
+      return null;
+    }
+
+    return this.app.service(service) || null;
+  }
+}

packages/authentication/test/core.test.ts

@@ -15,7 +15,10 @@ describe('authentication/core', () => {
   beforeEach(() => {
     app = feathers();
     auth = new AuthenticationBase(app, 'authentication', {
-      secret: 'supersecret'
+      entity: 'user',
+      service: 'users',
+      secret: 'supersecret',
+      first: { hello: 'test' }
     });
 
     auth.register('first', new Strategy1());
@@ -95,6 +98,12 @@ describe('authentication/core', () => {
       assert.strictEqual(first.app, app);
       assert.strictEqual(first.authentication, auth);
     });
+
+    it('strategy configuration getter', () => {
+      const [ first ] = auth.getStrategies('first') as [ Strategy1 ];
+
+      assert.deepStrictEqual(first.configuration, { hello: 'test' });
+    });
   });
 
   describe('authenticate', () => {

packages/authentication/test/fixtures.ts

@@ -1,39 +1,23 @@
 import { NotAuthenticated } from '@feathersjs/errors';
-import { Application, Params } from '@feathersjs/feathers';
+import { Params } from '@feathersjs/feathers';
 
-import { AuthenticationStrategy, AuthenticationRequest } from '../src/core';
-import { AuthenticationService } from '../src/service';
+import { AuthenticationRequest } from '../src/core';
 import { IncomingMessage } from 'http';
+import { AuthenticationBaseStrategy } from '../src/strategy';
 
 export interface MockRequest extends IncomingMessage {
   isDave?: boolean;
   isV2?: boolean;
 }
 
-export class Strategy1 implements AuthenticationStrategy {
+export class Strategy1 extends AuthenticationBaseStrategy {
   static result = {
     user: {
       id: 123,
       name: 'Dave'
     }
   };
 
-  name?: string;
-  app?: Application;
-  authentication?: AuthenticationService;
-
-  setName (name: string) {
-    this.name = name;
-  }
-
-  setApplication (app: Application) {
-    this.app = app;
-  }
-
-  setAuthentication (authentication: AuthenticationService) {
-    this.authentication = authentication;
-  }
-
   async authenticate (authentication: AuthenticationRequest) {
     if (authentication.username === 'David' || authentication.both) {
       return Strategy1.result;
@@ -51,7 +35,7 @@ export class Strategy1 implements AuthenticationStrategy {
   }
 }
 
-export class Strategy2 implements AuthenticationStrategy {
+export class Strategy2 extends AuthenticationBaseStrategy {
   static result = {
     user: {
       name: 'V2',

packages/authentication/test/hooks/authenticate.test.ts

@@ -17,12 +17,16 @@ describe('authentication/hooks/authenticate', () => {
   beforeEach(() => {
     app = feathers();
     app.use('/authentication', new AuthenticationService(app, 'authentication', {
+      entity: 'user',
+      service: 'users',
       secret: 'supersecret',
-      strategies: [ 'first' ]
+      jwtStrategies: [ 'first' ]
     }));
     app.use('/auth-v2', new AuthenticationService(app, 'auth-v2', {
+      entity: 'user',
+      service: 'users',
       secret: 'supersecret',
-      strategies: [ 'test' ]
+      jwtStrategies: [ 'test' ]
     }));
     app.use('/users', {
       async find () {

packages/authentication/test/jwt.test.ts

@@ -24,8 +24,10 @@ describe('authentication/jwt', () => {
     app = feathers();
 
     const authService = new AuthenticationService(app, 'authentication', {
+      entity: 'user',
+      service: 'users',
       secret: 'supersecret',
-      strategies: [ 'jwt' ]
+      jwtStrategies: [ 'jwt' ]
     });
 
     authService.register('jwt', new JWTStrategy());
@@ -101,7 +103,7 @@ describe('authentication/jwt', () => {
         assert.fail('Should never get here');
       } catch (error) {
         assert.strictEqual(error.name, 'NotAuthenticated');
-        assert.strictEqual(error.message, `Could not find entity service 'users'`);
+        assert.strictEqual(error.message, `Could not find entity service`);
       }
     });