From 63407f83d0529043e561d50c4975dc3c90e107bd Mon Sep 17 00:00:00 2001 From: Adrien Maret Date: Tue, 21 Sep 2021 21:45:37 +0200 Subject: [PATCH] 2.14.4 - Hotfix unsubscribe from Embedded SDK (#2144) When unsubscribing, Kuzzle trigger an event containing (among other information) the `kuid` of the user who made the subscription. The EmbeddedSDK is not necessarily using an user to execute requests. This case is now handled ### Other changes - Convert related files to typescript --- .eslintignore | 2 + .gitignore | 2 + .../auth/{tokenManager.js => tokenManager.ts} | 188 ++++++++++-------- lib/core/realtime/hotelClerk.js | 4 +- lib/core/security/tokenRepository.js | 2 +- lib/kuzzle/kuzzle.js | 2 +- lib/model/security/{token.js => token.ts} | 44 +++- test/api/controllers/authController.test.js | 2 +- test/api/funnel/checkRights.test.js | 2 +- test/core/auth/tokenManager.test.js | 12 +- .../realtime/hotelClerk/unsubscribe.test.js | 4 +- test/core/security/tokenRepository.test.js | 2 +- test/mocks/kuzzle.mock.js | 2 +- 13 files changed, 159 insertions(+), 109 deletions(-) rename lib/core/auth/{tokenManager.js => tokenManager.ts} (61%) rename lib/model/security/{token.js => token.ts} (63%) diff --git a/.eslintignore b/.eslintignore index ac2f206d16..1339411ebc 100644 --- a/.eslintignore +++ b/.eslintignore @@ -63,3 +63,5 @@ lib/util/mutex.js lib/util/inflector.js lib/util/koncordeCompat.js features/support/application/functional-tests-app.js +lib/model/security/token.js +lib/core/auth/tokenManager.js diff --git a/.gitignore b/.gitignore index d59504a537..fa4385f7cb 100644 --- a/.gitignore +++ b/.gitignore @@ -137,3 +137,5 @@ lib/util/koncordeCompat.js features-sdk/support/application/functional-tests-app.js docker/scripts/functional-tests-controller.js docker/scripts/start-kuzzle-dev.js +lib/model/security/token.js +lib/core/auth/tokenManager.js diff --git a/lib/core/auth/tokenManager.js b/lib/core/auth/tokenManager.ts similarity index 61% rename from lib/core/auth/tokenManager.js rename to lib/core/auth/tokenManager.ts index b6f939b738..511015eebb 100644 --- a/lib/core/auth/tokenManager.js +++ b/lib/core/auth/tokenManager.ts @@ -19,11 +19,46 @@ * limitations under the License. */ -'use strict'; +import SortedArray from 'sorted-array'; -const SortedArray = require('sorted-array'); +import '../../types/Global'; +import { Token } from '../../model/security/token'; -const Token = require('../../model/security/token'); +interface ISortedArray { + array: T[]; + + search (item: any): number; + + insert (item: T): void; +} + +/** + * Extends the Token model with a set of linked connection IDs. + */ +class ManagedToken extends Token { + /** + * Unique string to identify the token and sort it by expiration date + */ + idx: string; + + /** + * Set of connection ID that use this token. + */ + connectionIds: Set; + + constructor (token: Token, connectionIds: Set) { + super(token); + + this.connectionIds = connectionIds; + } + + /** + * Returns an unique string that identify a token and allows to sort them by expiration date. + */ + static indexFor (token: Token) { + return `${token.expiresAt};${token._id}`; + } +} /* Maximum delay of a setTimeout call. If larger than this value, @@ -36,16 +71,18 @@ const Token = require('../../model/security/token'); const TIMEOUT_MAX = Math.pow(2, 31) - 1; /** - * Maintains a list of valid tokens used by real-time subscriptions + * Maintains a list of valid tokens used by connected protocols. + * * When a token expires, this module cleans up the corresponding connection's - * subscriptions, and notify the user - * - * @class TokenManager + * subscriptions if any, and notify the user */ -class TokenManager { - constructor () { - this.anonymousUserId = null; +export class TokenManager { + private tokens: ISortedArray; + private anonymousUserId: string = null; + private tokensByConnection = new Map(); + private timer: NodeJS.Timeout = null; + constructor () { /* * Tokens are sorted by their expiration date * @@ -74,9 +111,6 @@ class TokenManager { return a.idx < b.idx ? -1 : 1; }); - this.tokensByConnection = new Map(); - - this.timer = null; } async init () { @@ -99,56 +133,57 @@ class TokenManager { /** * Link a connection and a token. * If one or another expires, associated subscriptions are cleaned up - * @param {Token} token - * @param {String} connectionId + * @param token + * @param connectionId */ - link (token, connectionId) { + link (token: Token, connectionId: string) { // Embedded SDK does not use tokens if (! token || token._id === this.anonymousUserId) { return; } - const idx = getTokenIndex(token); + const idx = ManagedToken.indexFor(token); const currentToken = this.tokensByConnection.get(connectionId); if (currentToken) { if (currentToken._id === token._id) { return; // Connection and Token already linked } - this._removeConnectionLinkedToToken(connectionId, currentToken); + this.removeConnectionLinkedToToken(connectionId, currentToken); } - const pos = this.tokens.search({idx}); + const pos = this.tokens.search({ idx }); if (pos === -1) { - this._add(token, [connectionId]); + this.add(token, new Set([connectionId])); } else { - const data = this.tokens.array[pos]; - data.connectionIds.add(connectionId); - this.tokensByConnection.set(connectionId, data); + const managedToken = this.tokens.array[pos]; + managedToken.connectionIds.add(connectionId); + + this.tokensByConnection.set(connectionId, managedToken); } } /** * Unlink a connection from its associated token * - * @param {Token} token - * @param {String} connectionId + * @param token + * @param connectionId */ - unlink (token, connectionId) { + unlink (token: Token, connectionId: string) { // Embedded SDK does not use tokens if (! token || token._id === this.anonymousUserId) { return; } - const idx = getTokenIndex(token); + const idx = ManagedToken.indexFor(token); const pos = this.tokens.search({ idx }); if (pos === -1) { return; } - this._removeConnectionLinkedToToken(connectionId, this.tokens.array[pos]); + this.removeConnectionLinkedToToken(connectionId, this.tokens.array[pos]); const currentToken = this.tokensByConnection.get(connectionId); if (currentToken && currentToken._id === token._id) { @@ -164,36 +199,35 @@ class TokenManager { * * @param token */ - async expire (token) { + async expire (token: Token) { if (token._id === this.anonymousUserId) { return; } - const idx = getTokenIndex(token); + const idx = ManagedToken.indexFor(token); const searchResult = this.tokens.search({idx}); if (searchResult > -1) { - const data = this.tokens.array[searchResult]; + const managedToken = this.tokens.array[searchResult]; - for (const connectionId of data.connectionIds) { + for (const connectionId of managedToken.connectionIds) { this.tokensByConnection.delete(connectionId); await global.kuzzle.ask('core:realtime:user:remove', connectionId); } - this._deleteByIndex(searchResult); + this.deleteByIndex(searchResult); } } /** * Refresh an existing token with a new one * - * @param {Token} oldToken - * @param {Token} newToken + * @param oldToken + * @param newToken */ - refresh (oldToken, newToken) { - const - oldIndex = getTokenIndex(oldToken), - pos = this.tokens.search({idx: oldIndex}); + refresh (oldToken: Token, newToken: Token) { + const oldIndex = ManagedToken.indexFor(oldToken); + const pos = this.tokens.search({ idx: oldIndex }); // If the old token has been created and then refreshed within the same // second, then it has the exact same characteristics than the new one. @@ -205,14 +239,14 @@ class TokenManager { if (pos > -1 && oldToken._id !== newToken._id) { const connectionIds = this.tokens.array[pos].connectionIds; - this._add(newToken, connectionIds); + this.add(newToken, connectionIds); // Delete old token - this._deleteByIndex(pos); + this.deleteByIndex(pos); } } - async checkTokensValidity() { + async checkTokensValidity () { const arr = this.tokens.array; // API key can never expire (-1) @@ -224,7 +258,9 @@ class TokenManager { for (const connectionId of connectionIds) { await global.kuzzle.ask('core:realtime:tokenExpired:notify', connectionId); } + setImmediate(() => this.checkTokensValidity()); + return; } @@ -235,76 +271,64 @@ class TokenManager { /** * Gets the token matching user & connection if any - * - * @param {string} userId - * @param {string} connectionId - * @returns {Token} */ - getConnectedUserToken(userId, connectionId) { - const data = this.tokensByConnection.get(connectionId); + getConnectedUserToken (userId: string, connectionId: string): Token | null { + const token = this.tokensByConnection.get(connectionId); - return data && data.userId === userId - ? new Token({...data, connectionId}) - : null; + return token && token.userId === userId ? token : null; } /** - * Returns the token associated to a connection + * Returns the kuid associated to a connection */ - getToken (connectionId) { - return this.tokensByConnection.get(connectionId); + getKuidFromConnection (connectionId: string): string | null { + const token = this.tokensByConnection.get(connectionId); + + if (! token) { + return null; + } + + return token.userId; } /** * Adds token to internal collections * - * @param {Token} token - * @param {string} connectionId - * @private + * @param token + * @param connectionId */ - _add(token, connectionIds) { - const data = Object.assign({}, token, { + private add (token: Token, connectionIds: Set) { + const orderedToken = Object.assign({}, token, { connectionIds: new Set(connectionIds), - idx: getTokenIndex(token) + idx: ManagedToken.indexFor(token) }); for (const connectionId of connectionIds) { - this.tokensByConnection.set(connectionId, data); + this.tokensByConnection.set(connectionId, orderedToken); } - this.tokens.insert(data); + this.tokens.insert(orderedToken); - if (this.tokens.array[0].idx === data.idx) { + if (this.tokens.array[0].idx === orderedToken.idx) { this.runTimer(); } } - _removeConnectionLinkedToToken(connectionId, token) { - token.connectionIds.delete(connectionId); + private removeConnectionLinkedToToken (connectionId: string, managedToken: ManagedToken) { + managedToken.connectionIds.delete(connectionId); - if (token.connectionIds.size === 0) { - const pos = this.tokens.search({ idx: token.idx }); - this._deleteByIndex(pos); + if (managedToken.connectionIds.size === 0) { + const pos = this.tokens.search({ idx: managedToken.idx }); + this.deleteByIndex(pos); } } - _deleteByIndex(index) { - const data = this.tokens.array[index]; + private deleteByIndex (index: number) { + const orderedToken = this.tokens.array[index]; - if (!data) { + if (! orderedToken) { return; } this.tokens.array.splice(index, 1); } } - -/** - * Calculate a simple sortable token index - * @param token - * @returns {string} - */ -function getTokenIndex(token) { - return `${token.expiresAt};${token._id}`; -} - -module.exports = TokenManager; diff --git a/lib/core/realtime/hotelClerk.js b/lib/core/realtime/hotelClerk.js index 37bdc77f88..5dc0d55c32 100644 --- a/lib/core/realtime/hotelClerk.js +++ b/lib/core/realtime/hotelClerk.js @@ -551,7 +551,7 @@ class HotelClerk { }); } - const { userId } = global.kuzzle.tokenManager.getToken(connectionId); + const kuid = global.kuzzle.tokenManager.getKuidFromConnection(connectionId); const subscription = new Subscription( room.index, @@ -559,7 +559,7 @@ class HotelClerk { undefined, roomId, connectionId, - { _id: userId }); + { _id: kuid }); global.kuzzle.emit('core:realtime:user:unsubscribe:after', { /* @deprecated */ diff --git a/lib/core/security/tokenRepository.js b/lib/core/security/tokenRepository.js index dcdc43a126..71e25e986f 100644 --- a/lib/core/security/tokenRepository.js +++ b/lib/core/security/tokenRepository.js @@ -28,7 +28,7 @@ const Bluebird = require('bluebird'); const ApiKey = require('../../model/storage/apiKey'); const { UnauthorizedError } = require('../../kerror/errors'); -const Token = require('../../model/security/token'); +const { Token } = require('../../model/security/token'); const Repository = require('../shared/repository'); const kerror = require('../../kerror'); const debug = require('../../util/debug')('kuzzle:bootstrap:tokens'); diff --git a/lib/kuzzle/kuzzle.js b/lib/kuzzle/kuzzle.js index 207ff45c3b..4af07e2b75 100644 --- a/lib/kuzzle/kuzzle.js +++ b/lib/kuzzle/kuzzle.js @@ -38,7 +38,7 @@ const PassportWrapper = require('../core/auth/passportWrapper'); const PluginsManager = require('../core/plugin/pluginsManager'); const Router = require('../core/network/router'); const Statistics = require('../core/statistics'); -const TokenManager = require('../core/auth/tokenManager'); +const { TokenManager } = require('../core/auth/tokenManager'); const Validation = require('../core/validation'); const Logger = require('./log'); const vault = require('./vault'); diff --git a/lib/model/security/token.js b/lib/model/security/token.ts similarity index 63% rename from lib/model/security/token.js rename to lib/model/security/token.ts index b5a546f91a..565dce558a 100644 --- a/lib/model/security/token.js +++ b/lib/model/security/token.ts @@ -19,31 +19,53 @@ * limitations under the License. */ -'use strict'; +export interface TokenContent { + /** + * Token ID (also Redis key) + * + * @example `${userId}#${jwt}` + */ + _id?: string; + expiresAt?: number; + ttl?: number; + userId?: string; + connectionIds?: string[]; + jwt?: string; + refreshed?: boolean; +} /** - * @class Token + * Represents a token that identify an user. */ -class Token { - constructor(data = {}) { +export class Token implements TokenContent { + _id: string; + expiresAt: number; + ttl: number; + userId: string; + jwt: string; + refreshed: boolean; + + constructor(data: TokenContent = {}) { this._id = data._id || null; this.expiresAt = data.expiresAt || null; this.ttl = data.ttl || null; this.userId = data.userId || null; - this.connectionId = data.connectionId || null; this.jwt = data.jwt || null; this.refreshed = Boolean(data.refreshed); } - get type() { + get type (): 'apiKey' | 'authToken' { if (this.jwt && this.jwt.startsWith(Token.APIKEY_PREFIX)) { return 'apiKey'; } + return 'authToken'; } -} -Token.AUTH_PREFIX = 'kauth-'; -Token.APIKEY_PREFIX = 'kapikey-'; - -module.exports = Token; + static get AUTH_PREFIX () { + return 'kauth-'; + } + static get APIKEY_PREFIX () { + return 'kapikey-'; + } +} diff --git a/test/api/controllers/authController.test.js b/test/api/controllers/authController.test.js index 78e5876861..5032a76766 100644 --- a/test/api/controllers/authController.test.js +++ b/test/api/controllers/authController.test.js @@ -15,7 +15,7 @@ const { const KuzzleMock = require('../../mocks/kuzzle.mock'); const AuthController = require('../../../lib/api/controllers/authController'); -const Token = require('../../../lib/model/security/token'); +const { Token } = require('../../../lib/model/security/token'); const User = require('../../../lib/model/security/user'); const { NativeController } = require('../../../lib/api/controllers/baseController'); diff --git a/test/api/funnel/checkRights.test.js b/test/api/funnel/checkRights.test.js index c7f2eeccf0..1901fdb684 100644 --- a/test/api/funnel/checkRights.test.js +++ b/test/api/funnel/checkRights.test.js @@ -11,7 +11,7 @@ const { const KuzzleMock = require('../../mocks/kuzzle.mock'); const FunnelController = require('../../../lib/api/funnel'); -const Token = require('../../../lib/model/security/token'); +const { Token } = require('../../../lib/model/security/token'); const User = require('../../../lib/model/security/user'); describe('funnel.checkRights', () => { diff --git a/test/core/auth/tokenManager.test.js b/test/core/auth/tokenManager.test.js index 84648b498a..e07097dbe7 100644 --- a/test/core/auth/tokenManager.test.js +++ b/test/core/auth/tokenManager.test.js @@ -5,8 +5,8 @@ const sinon = require('sinon'); const KuzzleMock = require('../../mocks/kuzzle.mock'); -const Token = require('../../../lib/model/security/token'); -const TokenManager = require('../../../lib/core/auth/tokenManager'); +const { Token } = require('../../../lib/model/security/token'); +const { TokenManager } = require('../../../lib/core/auth/tokenManager'); describe('Test: token manager core component', () => { const anonymousToken = new Token({ _id: '-1' }); @@ -466,15 +466,15 @@ describe('Test: token manager core component', () => { it('should return a matching token', () => { tokenManager.link(token, 'foo'); - const response = tokenManager.getConnectedUserToken(token.userId, 'foo'); - should(response).be.an.instanceOf(Token); - should(response).match({ + const ret = tokenManager.getConnectedUserToken(token.userId, 'foo'); + + should(ret).match({ _id: token._id, expiresAt: token.expiresAt, ttl: null, userId: token.userId, jwt: token.jwt, - connectionId: 'foo', + connectionIds: new Set(['foo']), }); }); diff --git a/test/core/realtime/hotelClerk/unsubscribe.test.js b/test/core/realtime/hotelClerk/unsubscribe.test.js index a5c27b3655..e333b56e68 100644 --- a/test/core/realtime/hotelClerk/unsubscribe.test.js +++ b/test/core/realtime/hotelClerk/unsubscribe.test.js @@ -45,7 +45,7 @@ describe('Test: hotelClerk.unsubscribe', () => { hotelClerk._removeRoomFromRealtimeEngine = sinon.spy(); - kuzzle.tokenManager.getToken.returns({}); + kuzzle.tokenManager.getKuidFromConnection.returns(null); return hotelClerk.init(); }); @@ -89,7 +89,7 @@ describe('Test: hotelClerk.unsubscribe', () => { }); it('should remove the room from the customer list and remove the connection entry if empty', async () => { - kuzzle.tokenManager.getToken.returns({ userId: 'Umraniye' }); + kuzzle.tokenManager.getKuidFromConnection.returns('Umraniye'); hotelClerk.customers.set(connectionId, new Map([ [ roomId, null ] ])); diff --git a/test/core/security/tokenRepository.test.js b/test/core/security/tokenRepository.test.js index c0147a5dd5..7d8e904755 100644 --- a/test/core/security/tokenRepository.test.js +++ b/test/core/security/tokenRepository.test.js @@ -15,7 +15,7 @@ const { const KuzzleMock = require('../../mocks/kuzzle.mock'); const MutexMock = require('../../mocks/mutex.mock'); -const Token = require('../../../lib/model/security/token'); +const { Token } = require('../../../lib/model/security/token'); const User = require('../../../lib/model/security/user'); const Repository = require('../../../lib/core/shared/repository'); const ApiKey = require('../../../lib/model/storage/apiKey'); diff --git a/test/mocks/kuzzle.mock.js b/test/mocks/kuzzle.mock.js index 53ba462f45..bae0824e2f 100644 --- a/test/mocks/kuzzle.mock.js +++ b/test/mocks/kuzzle.mock.js @@ -171,7 +171,7 @@ class KuzzleMock extends KuzzleEventEmitter { link: sinon.stub(), refresh: sinon.stub(), unlink: sinon.stub(), - getToken: sinon.stub(), + getKuidFromConnection: sinon.stub(), }; this.validation = {