From 4329febfb0936f74f8e16fec829a72b18deb61ce Mon Sep 17 00:00:00 2001 From: David Luecke Date: Sat, 17 Aug 2019 15:01:59 -0700 Subject: [PATCH] feat: Let strategies handle the connection (#1510) --- .../authentication-client/test/index.test.ts | 2 +- packages/authentication/src/core.ts | 9 +- .../authentication/src/hooks/connection.ts | 25 +++-- .../src/hooks/{events.ts => event.ts} | 11 +-- packages/authentication/src/hooks/index.ts | 2 +- packages/authentication/src/jwt.ts | 32 ++++++- packages/authentication/src/service.ts | 10 +- .../test/hooks/connection.test.ts | 92 ------------------- .../hooks/{events.test.ts => event.test.ts} | 5 +- packages/authentication/test/jwt.test.ts | 59 ++++++++++++ 10 files changed, 123 insertions(+), 124 deletions(-) rename packages/authentication/src/hooks/{events.ts => event.ts} (51%) delete mode 100644 packages/authentication/test/hooks/connection.test.ts rename packages/authentication/test/hooks/{events.test.ts => event.test.ts} (94%) diff --git a/packages/authentication-client/test/index.test.ts b/packages/authentication-client/test/index.test.ts index 3142b16f51..125bfc0a96 100644 --- a/packages/authentication-client/test/index.test.ts +++ b/packages/authentication-client/test/index.test.ts @@ -27,7 +27,7 @@ describe('@feathersjs/authentication-client', () => { if (!app.get('authentication')) { throw new Error('Not logged in'); } - + return Promise.resolve({ id }); } }); diff --git a/packages/authentication/src/core.ts b/packages/authentication/src/core.ts index e9f145630f..b2fb2ad0ae 100644 --- a/packages/authentication/src/core.ts +++ b/packages/authentication/src/core.ts @@ -4,7 +4,7 @@ import jsonwebtoken, { SignOptions, Secret, VerifyOptions } from 'jsonwebtoken'; import uuidv4 from 'uuid/v4'; import { NotAuthenticated } from '@feathersjs/errors'; import Debug from 'debug'; -import { Application, Params } from '@feathersjs/feathers'; +import { Application, Params, HookContext } from '@feathersjs/feathers'; import { IncomingMessage, ServerResponse } from 'http'; import defaultOptions from './options'; @@ -49,6 +49,13 @@ export interface AuthenticationStrategy { * @param params The service call parameters */ authenticate? (authentication: AuthenticationRequest, params: Params): Promise; + /** + * Update a real-time connection according to this strategy. + * + * @param connection The real-time connection + * @param context The hook context + */ + handleConnection? (connection: any, context: HookContext): Promise; /** * Parse a basic HTTP request and response for authentication request information. * @param req The HTTP request diff --git a/packages/authentication/src/hooks/connection.ts b/packages/authentication/src/hooks/connection.ts index 53e87dbbb9..896c0b28f2 100644 --- a/packages/authentication/src/hooks/connection.ts +++ b/packages/authentication/src/hooks/connection.ts @@ -1,26 +1,23 @@ import { HookContext } from '@feathersjs/feathers'; -import Debug from 'debug'; +import { omit } from 'lodash'; -const debug = Debug('@feathersjs/authentication/hooks/connection'); +import { AuthenticationBase } from '../core'; -export default (strategy = 'jwt') => (context: HookContext) => { - const { method, result, params: { connection } } = context; - const { accessToken, ...rest } = result; +export default () => async (context: HookContext) => { + const { result, params: { connection } } = context; if (!connection) { return context; } - const { authentication = {} } = connection; + const service = context.service as unknown as AuthenticationBase; + const strategies = service.getStrategies(...Object.keys(service.strategies)) + .filter(current => typeof current.handleConnection === 'function'); - if (method === 'remove' && accessToken === authentication.accessToken) { - debug('Removing authentication information from real-time connection'); - delete connection.authentication; - } else if (method === 'create' && accessToken) { - debug('Adding authentication information to real-time connection'); - Object.assign(connection, rest, { - authentication: { strategy, accessToken } - }); + Object.assign(connection, omit(result, 'accessToken', 'authentication')); + + for (const strategy of strategies) { + await strategy.handleConnection(connection, context); } return context; diff --git a/packages/authentication/src/hooks/events.ts b/packages/authentication/src/hooks/event.ts similarity index 51% rename from packages/authentication/src/hooks/events.ts rename to packages/authentication/src/hooks/event.ts index b45fca232d..a363a46017 100644 --- a/packages/authentication/src/hooks/events.ts +++ b/packages/authentication/src/hooks/event.ts @@ -2,16 +2,11 @@ import Debug from 'debug'; import { HookContext } from '@feathersjs/feathers'; const debug = Debug('@feathersjs/authentication/hooks/connection'); -const EVENTS: { [key: string]: string } = { - create: 'login', - remove: 'logout' -}; -export default () => (context: HookContext) => { - const { method, app, result, params } = context; - const event = EVENTS[method]; +export default (event: string) => (context: HookContext) => { + const { type, app, result, params } = context; - if (event && params.provider && result) { + if (type === 'after' && params.provider && result) { debug(`Sending authentication event '${event}'`); app.emit(event, result, params, context); } diff --git a/packages/authentication/src/hooks/index.ts b/packages/authentication/src/hooks/index.ts index 3870bb495e..775f3e0943 100644 --- a/packages/authentication/src/hooks/index.ts +++ b/packages/authentication/src/hooks/index.ts @@ -1,3 +1,3 @@ export { default as authenticate } from './authenticate'; export { default as connection } from './connection'; -export { default as events } from './events'; +export { default as event } from './event'; diff --git a/packages/authentication/src/jwt.ts b/packages/authentication/src/jwt.ts index a8608ee503..d7f1cacb24 100644 --- a/packages/authentication/src/jwt.ts +++ b/packages/authentication/src/jwt.ts @@ -1,10 +1,13 @@ import { NotAuthenticated } from '@feathersjs/errors'; -import { omit } from 'lodash'; -import { AuthenticationRequest, AuthenticationResult } from './core'; -import { Params } from '@feathersjs/feathers'; import { IncomingMessage } from 'http'; +import { omit } from 'lodash'; +import Debug from 'debug'; +import { Params, HookContext } from '@feathersjs/feathers'; + import { AuthenticationBaseStrategy } from './strategy'; +import { AuthenticationRequest, AuthenticationResult } from './core'; +const debug = Debug('@feathersjs/authentication/jwt'); const SPLIT_HEADER = /(\S+)\s+(\S+)/; export class JWTStrategy extends AuthenticationBaseStrategy { @@ -21,6 +24,25 @@ export class JWTStrategy extends AuthenticationBaseStrategy { }; } + async handleConnection (connection: any, context: HookContext) { + const { result: { accessToken }, method } = context; + + if (accessToken) { + if (method === 'create') { + debug('Adding authentication information to connection'); + connection.authentication = { + strategy: this.name, + accessToken + }; + } else if (method === 'remove' && accessToken === connection.authentication.accessToken) { + debug('Removing authentication information from connection'); + delete connection.authentication; + } + } + + return context; + } + verifyConfiguration () { const allowedKeys = [ 'entity', 'service', 'header', 'schemes' ]; @@ -40,6 +62,8 @@ export class JWTStrategy extends AuthenticationBaseStrategy { const { entity } = this.configuration; const entityService = this.entityService; + debug('Getting entity', id); + if (entityService === null) { throw new NotAuthenticated(`Could not find entity service`); } @@ -96,6 +120,8 @@ export class JWTStrategy extends AuthenticationBaseStrategy { return null; } + debug('Found parsed header value'); + const [ , scheme = null, schemeValue = null ] = headerValue.match(SPLIT_HEADER) || []; const hasScheme = scheme && schemes.some( current => new RegExp(current, 'i').test(scheme) diff --git a/packages/authentication/src/service.ts b/packages/authentication/src/service.ts index 4d036333e7..0914f84233 100644 --- a/packages/authentication/src/service.ts +++ b/packages/authentication/src/service.ts @@ -2,7 +2,7 @@ import Debug from 'debug'; import { merge, get } from 'lodash'; import { NotAuthenticated } from '@feathersjs/errors'; import { AuthenticationBase, AuthenticationResult, AuthenticationRequest } from './core'; -import { connection, events } from './hooks'; +import { connection, event } from './hooks'; import { Application, Params, ServiceMethods } from '@feathersjs/feathers'; const debug = Debug('@feathersjs/authentication/service'); @@ -154,6 +154,12 @@ export class AuthenticationService extends AuthenticationBase implements Partial } // @ts-ignore - this.hooks({ after: [ connection(), events() ] }); + this.hooks({ + after: [ connection() ], + finally: { + create: [ event('login') ], + remove: [ event('logout') ] + } + }); } } diff --git a/packages/authentication/test/hooks/connection.test.ts b/packages/authentication/test/hooks/connection.test.ts deleted file mode 100644 index af3ce1adf0..0000000000 --- a/packages/authentication/test/hooks/connection.test.ts +++ /dev/null @@ -1,92 +0,0 @@ -import assert from 'assert'; -import feathers, { Params, Service } from '@feathersjs/feathers'; - -import { AuthenticationRequest } from '../../src/core'; -import hook from '../../src/hooks/connection'; -import { AuthenticationService } from '../../src'; -import { AuthenticationResult } from '../../src/core'; - -describe('authentication/hooks/connection', () => { - const app = feathers().use('/authentication', { - async create (_data: AuthenticationRequest, params: Params) { - if (params.noAccessToken) { - return {}; - } - - return { - accessToken: '1234', - authentication: { strategy: 'test' }, - additionalParams: true - }; - }, - - async remove () { - return { accessToken: '1234' }; - } - }); - - const service = app.service('authentication') as AuthenticationService & Service; - - service.hooks({ - after: { - all: [ hook() ] - } - }); - - it('create does nothing when there is no connection', async () => { - const result = await service.create({}, {}); - - assert.deepStrictEqual(result, { - accessToken: '1234', - authentication: { strategy: 'test' }, - additionalParams: true - }); - }); - - it('create (login) updates `params.connection.authentication` with all params', async () => { - const connection = {}; - - await service.create({}, { connection }); - - assert.deepStrictEqual(connection, { - authentication: { - strategy: 'jwt', - accessToken: '1234' - }, - additionalParams: true - }); - }); - - it('create (login) does nothing when there is no accessToken', async () => { - const connection = {}; - - await service.create({}, { - connection, - noAccessToken: true - }); - - assert.deepStrictEqual(connection, {}); - }); - - it('remove (logout) deletes `connection.authentication` if token matches', async () => { - const connection = { - authentication: { strategy: 'jwt', accessToken: '1234' } - }; - - await service.remove('test', { connection }); - - assert.deepStrictEqual(connection, {}); - }); - - it('remove (logout) does nothing if token does not match', async () => { - const connection = { - authentication: { strategy: 'jwt', accessToken: '12343' } - }; - - await service.remove('test', { connection }); - - assert.deepStrictEqual(connection, { - authentication: { strategy: 'jwt', accessToken: '12343' } - }); - }); -}); diff --git a/packages/authentication/test/hooks/events.test.ts b/packages/authentication/test/hooks/event.test.ts similarity index 94% rename from packages/authentication/test/hooks/events.test.ts rename to packages/authentication/test/hooks/event.test.ts index b2ce6e161e..ecd212a4fb 100644 --- a/packages/authentication/test/hooks/events.test.ts +++ b/packages/authentication/test/hooks/event.test.ts @@ -1,7 +1,7 @@ import assert from 'assert'; import feathers, { Params, HookContext } from '@feathersjs/feathers'; -import hook from '../../src/hooks/events'; +import hook from '../../src/hooks/event'; import { AuthenticationRequest, AuthenticationResult } from '../../src/core'; describe('authentication/hooks/events', () => { @@ -19,7 +19,8 @@ describe('authentication/hooks/events', () => { service.hooks({ after: { - all: [ hook() ] + create: [ hook('login') ], + remove: [ hook('logout') ] } }); diff --git a/packages/authentication/test/jwt.test.ts b/packages/authentication/test/jwt.test.ts index 8761644e25..39491a5337 100644 --- a/packages/authentication/test/jwt.test.ts +++ b/packages/authentication/test/jwt.test.ts @@ -72,6 +72,7 @@ describe('authentication/jwt', () => { }); payload = await service.verifyAccessToken(accessToken); + app.setup(); }); it('getEntity', async () => { @@ -91,6 +92,64 @@ describe('authentication/jwt', () => { }); }); + describe('updateConnection', () => { + it('adds authentication information on create', async () => { + const connection: any = {}; + + await app.service('authentication').create({ + strategy: 'jwt', + accessToken + }, { connection }); + + assert.deepStrictEqual(connection.user, user); + assert.deepStrictEqual(connection.authentication, { + strategy: 'jwt', + accessToken + }); + }); + + it('deletes authentication information on remove', async () => { + const connection: any = {}; + + await app.service('authentication').create({ + strategy: 'jwt', + accessToken + }, { connection }); + + assert.ok(connection.authentication); + + await app.service('authentication').remove(null, { + authentication: connection.authentication, + connection + }); + + assert.ok(!connection.authentication); + }); + + it('does not remove if accessToken does not match', async () => { + const connection: any = {}; + + await app.service('authentication').create({ + strategy: 'jwt', + accessToken + }, { connection }); + + assert.ok(connection.authentication); + + await app.service('authentication').remove(null, { + authentication: { + strategy: 'jwt', + accessToken: await app.service('authentication').createAccessToken({}, { + subject: `${user.id}` + }) + }, + connection + }); + + assert.ok(connection.authentication); + }); + }); + describe('with authenticate hook', () => { it('fails for protected service and external call when not set', async () => { try {