Skip to content

Commit

Permalink
fix(authentication): Improve logout and disconnect connection handling (
Browse files Browse the repository at this point in the history
  • Loading branch information
daffl authored Oct 18, 2022
1 parent 1b97f14 commit dd77379
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 12 deletions.
3 changes: 0 additions & 3 deletions packages/authentication/src/hooks/connection.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { HookContext, NextFunction } from '@feathersjs/feathers'
import omit from 'lodash/omit'
import { AuthenticationBase, ConnectionEvent } from '../core'

export default (event: ConnectionEvent) => async (context: HookContext, next: NextFunction) => {
Expand All @@ -13,8 +12,6 @@ export default (event: ConnectionEvent) => async (context: HookContext, next: Ne
if (connection) {
const service = context.service as unknown as AuthenticationBase

Object.assign(connection, omit(result, 'accessToken', 'authentication'))

await service.handleConnection(event, connection, result)
}
}
18 changes: 11 additions & 7 deletions packages/authentication/src/jwt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,14 @@ export class JWTStrategy extends AuthenticationBaseStrategy {
connection.authentication.accessToken === authResult.accessToken

const { accessToken } = authResult || {}
const { entity } = this.configuration

if (accessToken && event === 'login') {
debug('Adding authentication information to connection')
const { exp } = await this.authentication.verifyAccessToken(accessToken)
const { exp } =
authResult?.authentication?.payload || (await this.authentication.verifyAccessToken(accessToken))
// The time (in ms) until the token expires
const duration = exp * 1000 - Date.now()
// 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 = lt.setTimeout(() => this.app.emit('disconnect', connection), duration)

debug(`Registering connection expiration timer for ${duration}ms`)
Expand All @@ -61,13 +61,17 @@ export class JWTStrategy extends AuthenticationBaseStrategy {
strategy: this.name,
accessToken
}
connection[entity] = authResult[entity]
} else if (event === 'disconnect' || isValidLogout) {
debug('Removing authentication information and expiration timer from connection')

const { entity } = this.configuration

delete connection[entity]
delete connection.authentication
await new Promise((resolve) =>
process.nextTick(() => {
delete connection[entity]
delete connection.authentication
resolve(connection)
})
)

lt.clearTimeout(this.expirationTimers.get(connection))
this.expirationTimers.delete(connection)
Expand Down
36 changes: 35 additions & 1 deletion packages/authentication/test/jwt.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ describe('authentication/jwt', () => {
})

describe('handleConnection', () => {
it('adds authentication information on create', async () => {
it('adds entity and authentication information on create', async () => {
const connection: any = {}

await app.service('authentication').create(
Expand Down Expand Up @@ -197,6 +197,40 @@ describe('authentication/jwt', () => {
assert.ok(!connection.user)
})

it('deletes authentication information on disconnect but maintains it in event handler', async () => {
const connection: any = {}

await app.service('authentication').create(
{
strategy: 'jwt',
accessToken
},
{ connection }
)

assert.ok(connection.authentication)
assert.ok(connection.user)

const disconnectPromise = new Promise((resolve, reject) =>
app.once('disconnect', (connection) => {
try {
assert.ok(connection.authentication)
assert.ok(connection.user)
resolve(connection)
} catch (error) {
reject(error)
}
})
)
app.emit('disconnect', connection)

await disconnectPromise
await new Promise((resolve) => process.nextTick(resolve))

assert.ok(!connection.authentication)
assert.ok(!connection.user)
})

it('does not remove if accessToken does not match', async () => {
const connection: any = {}

Expand Down
2 changes: 1 addition & 1 deletion packages/socketio/src/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export interface FeathersSocket extends Socket {

export const disconnect =
(app: Application, getParams: ParamsGetter) => (socket: FeathersSocket, next: NextFunction) => {
socket.once('disconnect', () => app.emit('disconnect', getParams(socket)))
socket.on('disconnect', () => app.emit('disconnect', getParams(socket)))
next()
}

Expand Down

0 comments on commit dd77379

Please sign in to comment.