From 2707c3331721be6a4a0a943b0fbc118379a1ad2a Mon Sep 17 00:00:00 2001 From: David Luecke Date: Sun, 18 Aug 2019 12:12:16 -0700 Subject: [PATCH] fix: Expire and remove authenticated real-time connections (#1512) --- packages/authentication/src/core.ts | 15 +++++- .../authentication/src/hooks/connection.ts | 11 ++-- packages/authentication/src/hooks/event.ts | 7 +-- packages/authentication/src/jwt.ts | 54 +++++++++++-------- packages/authentication/src/service.ts | 7 ++- packages/authentication/test/jwt.test.ts | 23 +++++++- packages/socketio/test/index.test.js | 2 +- .../transport-commons/src/socket/index.ts | 19 +++++-- 8 files changed, 93 insertions(+), 45 deletions(-) diff --git a/packages/authentication/src/core.ts b/packages/authentication/src/core.ts index b2fb2ad0ae..a08600a929 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, HookContext } from '@feathersjs/feathers'; +import { Application, Params } from '@feathersjs/feathers'; import { IncomingMessage, ServerResponse } from 'http'; import defaultOptions from './options'; @@ -21,6 +21,8 @@ export interface AuthenticationRequest { [key: string]: any; } +export type ConnectionEvent = 'login'|'logout'|'disconnect'; + export interface AuthenticationStrategy { /** * Implement this method to get access to the AuthenticationService @@ -55,7 +57,7 @@ export interface AuthenticationStrategy { * @param connection The real-time connection * @param context The hook context */ - handleConnection? (connection: any, context: HookContext): Promise; + handleConnection? (event: ConnectionEvent, connection: any, authResult?: AuthenticationResult): Promise; /** * Parse a basic HTTP request and response for authentication request information. * @param req The HTTP request @@ -223,6 +225,15 @@ export class AuthenticationBase { }); } + async handleConnection (event: ConnectionEvent, connection: any, authResult?: AuthenticationResult) { + const strategies = this.getStrategies(...Object.keys(this.strategies)) + .filter(current => typeof current.handleConnection === 'function'); + + for (const strategy of strategies) { + await strategy.handleConnection(event, connection, authResult); + } + } + /** * Parse an 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 896c0b28f2..66d185bcbb 100644 --- a/packages/authentication/src/hooks/connection.ts +++ b/packages/authentication/src/hooks/connection.ts @@ -1,9 +1,8 @@ import { HookContext } from '@feathersjs/feathers'; import { omit } from 'lodash'; +import { AuthenticationBase, ConnectionEvent } from '../core'; -import { AuthenticationBase } from '../core'; - -export default () => async (context: HookContext) => { +export default (event: ConnectionEvent) => async (context: HookContext) => { const { result, params: { connection } } = context; if (!connection) { @@ -11,14 +10,10 @@ export default () => async (context: HookContext) => { } const service = context.service as unknown as AuthenticationBase; - const strategies = service.getStrategies(...Object.keys(service.strategies)) - .filter(current => typeof current.handleConnection === 'function'); Object.assign(connection, omit(result, 'accessToken', 'authentication')); - for (const strategy of strategies) { - await strategy.handleConnection(connection, context); - } + await service.handleConnection(event, connection, result); return context; }; diff --git a/packages/authentication/src/hooks/event.ts b/packages/authentication/src/hooks/event.ts index a363a46017..1520f76f15 100644 --- a/packages/authentication/src/hooks/event.ts +++ b/packages/authentication/src/hooks/event.ts @@ -1,12 +1,13 @@ import Debug from 'debug'; import { HookContext } from '@feathersjs/feathers'; +import { ConnectionEvent } from '../core'; const debug = Debug('@feathersjs/authentication/hooks/connection'); -export default (event: string) => (context: HookContext) => { - const { type, app, result, params } = context; +export default (event: ConnectionEvent) => async (context: HookContext) => { + const { app, result, params } = context; - if (type === 'after' && params.provider && result) { + if (params.provider && result) { debug(`Sending authentication event '${event}'`); app.emit(event, result, params, context); } diff --git a/packages/authentication/src/jwt.ts b/packages/authentication/src/jwt.ts index d7f1cacb24..641937241e 100644 --- a/packages/authentication/src/jwt.ts +++ b/packages/authentication/src/jwt.ts @@ -1,16 +1,18 @@ -import { NotAuthenticated } from '@feathersjs/errors'; -import { IncomingMessage } from 'http'; -import { omit } from 'lodash'; import Debug from 'debug'; -import { Params, HookContext } from '@feathersjs/feathers'; +import { omit } from 'lodash'; +import { IncomingMessage } from 'http'; +import { NotAuthenticated } from '@feathersjs/errors'; +import { Params } from '@feathersjs/feathers'; import { AuthenticationBaseStrategy } from './strategy'; -import { AuthenticationRequest, AuthenticationResult } from './core'; +import { AuthenticationRequest, AuthenticationResult, ConnectionEvent } from './core'; const debug = Debug('@feathersjs/authentication/jwt'); const SPLIT_HEADER = /(\S+)\s+(\S+)/; export class JWTStrategy extends AuthenticationBaseStrategy { + expirationTimers = new WeakMap(); + get configuration () { const authConfig = this.authentication.configuration; const config = super.configuration; @@ -24,23 +26,33 @@ 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; - } + async handleConnection (event: ConnectionEvent, connection: any, authResult?: AuthenticationResult): Promise { + const isValidLogout = event === 'logout' && connection.authentication && authResult && + connection.authentication.accessToken === authResult.accessToken; + + if (authResult && event === 'login') { + const { accessToken } = authResult; + const { exp } = await this.authentication.verifyAccessToken(accessToken); + // The time (in ms) until the token expires + const duration = (exp * 1000) - new Date().getTime(); + // This may have to be a `logout` event but right now we don't want + // the whole context object lingering around until the timer is gone + const timer = setTimeout(() => this.app.emit('disconnect', connection), duration); + + debug(`Registering connection expiration timer for ${duration}ms`); + this.expirationTimers.set(connection, timer); + + debug('Adding authentication information to connection'); + connection.authentication = { + strategy: this.name, + accessToken + }; + } else if (event === 'disconnect' || isValidLogout) { + debug('Removing authentication information and expiration timer from connection'); + + delete connection.authentication; + clearTimeout(this.expirationTimers.get(connection)); } - - return context; } verifyConfiguration () { diff --git a/packages/authentication/src/service.ts b/packages/authentication/src/service.ts index 0914f84233..807e3e76c1 100644 --- a/packages/authentication/src/service.ts +++ b/packages/authentication/src/service.ts @@ -155,10 +155,9 @@ export class AuthenticationService extends AuthenticationBase implements Partial // @ts-ignore this.hooks({ - after: [ connection() ], - finally: { - create: [ event('login') ], - remove: [ event('logout') ] + after: { + create: [ connection('login'), event('login') ], + remove: [ connection('logout'), event('logout') ] } }); } diff --git a/packages/authentication/test/jwt.test.ts b/packages/authentication/test/jwt.test.ts index 39491a5337..f78f7ad359 100644 --- a/packages/authentication/test/jwt.test.ts +++ b/packages/authentication/test/jwt.test.ts @@ -92,7 +92,7 @@ describe('authentication/jwt', () => { }); }); - describe('updateConnection', () => { + describe('handleConnection', () => { it('adds authentication information on create', async () => { const connection: any = {}; @@ -108,6 +108,27 @@ describe('authentication/jwt', () => { }); }); + it('sends disconnect event when connection token expires and removes authentication', async () => { + const connection: any = {}; + const token: string = await app.service('authentication').createAccessToken({}, { + subject: `${user.id}`, + expiresIn: '1s' + }); + + const result = await app.service('authentication').create({ + strategy: 'jwt', + accessToken: token + }, { connection }); + + assert.ok(connection.authentication); + + assert.strictEqual(result.accessToken, token); + + const disconnection = await new Promise(resolve => app.once('disconnect', resolve)); + + assert.strictEqual(disconnection, connection); + }); + it('deletes authentication information on remove', async () => { const connection: any = {}; diff --git a/packages/socketio/test/index.test.js b/packages/socketio/test/index.test.js index c3f71895be..7295b9824d 100644 --- a/packages/socketio/test/index.test.js +++ b/packages/socketio/test/index.test.js @@ -10,7 +10,7 @@ const methodTests = require('./methods.js'); const eventTests = require('./events'); const socketio = require('../lib'); -describe.only('@feathersjs/socketio', () => { +describe('@feathersjs/socketio', () => { let app; let server; let socket; diff --git a/packages/transport-commons/src/socket/index.ts b/packages/transport-commons/src/socket/index.ts index 7c1272d1b4..83b0ee2af0 100644 --- a/packages/transport-commons/src/socket/index.ts +++ b/packages/transport-commons/src/socket/index.ts @@ -1,8 +1,8 @@ +import { Application, Params } from '@feathersjs/feathers'; import Debug from 'debug'; import { channels } from '../channels'; import { routing } from '../routing'; import { getDispatcher, runMethod } from './utils'; -import { Application } from '@feathersjs/feathers'; import { RealTimeConnection } from '../channels/channel/base'; const debug = Debug('@feathersjs/transport-commons'); @@ -16,15 +16,24 @@ export interface SocketOptions { export function socket ({ done, emit, socketMap, getParams }: SocketOptions) { return (app: Application) => { + const leaveChannels = (connection: RealTimeConnection) => { + const { channels } = app; + + if (channels.length) { + app.channel(app.channels).leave(connection); + } + }; + app.configure(channels()); app.configure(routing()); app.on('publish', getDispatcher(emit, socketMap)); - app.on('disconnect', connection => { - const { channels } = app; + app.on('disconnect', leaveChannels); + app.on('logout', (_authResult: any, params: Params) => { + const { connection } = params; - if (channels.length) { - app.channel(app.channels).leave(connection); + if (connection) { + leaveChannels(connection); } });