From c9cb2b869a1e94ff19a8812415985290d7efb95f Mon Sep 17 00:00:00 2001 From: Juiced66 Date: Wed, 30 Oct 2024 11:39:45 +0100 Subject: [PATCH] fix: do not store api keys in internal index, use fingerprint instead --- lib/core/security/tokenRepository.ts | 93 +++++++++++----------------- lib/model/storage/apiKey.js | 34 ++++++---- lib/model/storage/baseModel.js | 3 +- test/model/storage/apiKey.test.js | 12 +++- 4 files changed, 68 insertions(+), 74 deletions(-) diff --git a/lib/core/security/tokenRepository.ts b/lib/core/security/tokenRepository.ts index b5ce8de283..de916e9b0a 100644 --- a/lib/core/security/tokenRepository.ts +++ b/lib/core/security/tokenRepository.ts @@ -30,8 +30,8 @@ import { Token } from "../../model/security/token"; import { User } from "../../model/security/user"; import ApiKey from "../../model/storage/apiKey"; import debugFactory from "../../util/debug"; -import { Mutex } from "../../util/mutex"; import { ObjectRepository } from "../shared/ObjectRepository"; +import { sha256 } from "../../util/crypto"; const securityError = kerror.wrap("security", "token"); const debug = debugFactory("kuzzle:bootstrap:tokens"); @@ -61,8 +61,6 @@ export class TokenRepository extends ObjectRepository { } async init() { - await this.loadApiKeys(); - /** * Assign an existing token to a user. Stores the token in Kuzzle's cache. * @param {String} hash - JWT @@ -272,10 +270,21 @@ export class TokenRepository extends ObjectRepository { if (type === "apiKey") { encodedToken = Token.APIKEY_PREFIX + encodedToken; - } else { - encodedToken = Token.AUTH_PREFIX + encodedToken; + + // For API keys, we don't persist the token + const expiresAt = + parsedExpiresIn === -1 ? -1 : Date.now() + parsedExpiresIn; + return new Token({ + _id: `${user._id}#${encodedToken}`, + expiresAt, + jwt: encodedToken, + ttl: parsedExpiresIn, + userId: user._id, + }); } + encodedToken = Token.AUTH_PREFIX + encodedToken; + // Persist regular tokens return this.persistForUser(encodedToken, user._id, { singleUse, ttl: parsedExpiresIn, @@ -329,11 +338,13 @@ export class TokenRepository extends ObjectRepository { return this.anonymousToken; } + const isApiKey = token.startsWith(Token.APIKEY_PREFIX); + const tokenWithoutPrefix = this.removeTokenPrefix(token); + let decoded = null; try { - decoded = jwt.verify(this.removeTokenPrefix(token), global.kuzzle.secret); - + decoded = jwt.verify(tokenWithoutPrefix, global.kuzzle.secret); // probably forged token => throw without providing any information if (!decoded._id) { throw new jwt.JsonWebTokenError("Invalid token"); @@ -350,7 +361,23 @@ export class TokenRepository extends ObjectRepository { throw securityError.getFrom(err, "verification_error", err.message); } - let userToken: Token; + if (isApiKey) { + const fingerprint = sha256(token); + + const apiKey = await ApiKey.load(decoded._id, fingerprint); + + const userToken = new Token({ + _id: `${decoded._id}#${token}`, + expiresAt: apiKey.expiresAt, + jwt: token, + ttl: apiKey.ttl, + userId: decoded._id, + }); + + return userToken; + } + + let userToken; try { userToken = await this.loadForUser(decoded._id, token); @@ -358,7 +385,6 @@ export class TokenRepository extends ObjectRepository { if (err instanceof UnauthorizedError) { throw err; } - throw securityError.getFrom(err, "verification_error", err.message); } @@ -457,55 +483,6 @@ export class TokenRepository extends ObjectRepository { await Promise.all(promises); } - /** - * Loads authentication token from API key into Redis - */ - private async loadApiKeys() { - const mutex = new Mutex("ApiKeysBootstrap", { - timeout: -1, - ttl: 30000, - }); - - await mutex.lock(); - - try { - const bootstrapped = await global.kuzzle.ask( - "core:cache:internal:get", - BOOTSTRAP_DONE_KEY, - ); - - if (bootstrapped) { - debug("API keys already in cache. Skip."); - return; - } - - debug("Loading API keys into Redis"); - - const promises = []; - - await ApiKey.batchExecute({ match_all: {} }, (documents) => { - for (const { _source } of documents) { - promises.push( - this.persistForUser(_source.token, _source.userId, { - singleUse: false, - ttl: _source.ttl, - }), - ); - } - }); - - await Promise.all(promises); - - await global.kuzzle.ask( - "core:cache:internal:store", - BOOTSTRAP_DONE_KEY, - 1, - ); - } finally { - await mutex.unlock(); - } - } - /** * The repository main class refreshes automatically the TTL * of accessed entries, letting only unaccessed entries expire diff --git a/lib/model/storage/apiKey.js b/lib/model/storage/apiKey.js index 108d17e336..e7a21bc2a7 100644 --- a/lib/model/storage/apiKey.js +++ b/lib/model/storage/apiKey.js @@ -49,7 +49,7 @@ class ApiKey extends BaseModel { serialize({ includeToken = false } = {}) { const serialized = super.serialize(); - if (!includeToken) { + if (!includeToken && this.token) { delete serialized._source.token; } @@ -107,7 +107,6 @@ class ApiKey extends BaseModel { description, expiresAt: token.expiresAt, fingerprint, - token: token.jwt, ttl: token.ttl, userId: user._id, }, @@ -116,6 +115,8 @@ class ApiKey extends BaseModel { await apiKey.save({ refresh, userId: creatorId }); + apiKey.token = token.jwt; + return apiKey; } @@ -127,16 +128,27 @@ class ApiKey extends BaseModel { * * @returns {ApiKey} */ - static async load(userId, id) { - const apiKey = await super.load(id); - - if (userId !== apiKey.userId) { - throw kerror.get("services", "storage", "not_found", id, { - message: `ApiKey "${id}" not found for user "${userId}".`, - }); + static async load(userId, fingerprint) { + const query = { + term: { + userId, + }, + }; + + const apiKeys = await super.search({ query }); + if (apiKeys.length > 0) { + const apiKey = apiKeys.find((a) => a.fingerprint === fingerprint); + if (userId !== apiKey.userId) { + throw kerror.get("services", "storage", "not_found", fingerprint, { + message: `ApiKey "${fingerprint}" not found for user "${userId}".`, + }); + } + + return apiKey; } - - return apiKey; + throw kerror.get("services", "storage", "not_found", fingerprint, { + message: `ApiKey "${fingerprint}" not found for user "${userId}".`, + }); } /** diff --git a/lib/model/storage/baseModel.js b/lib/model/storage/baseModel.js index bb0ce0a49d..97e014593b 100644 --- a/lib/model/storage/baseModel.js +++ b/lib/model/storage/baseModel.js @@ -218,7 +218,6 @@ class BaseModel { searchBody, options, ); - return resp.hits.map((hit) => this._instantiateFromDb(hit)); } @@ -232,7 +231,7 @@ class BaseModel { static truncate({ refresh } = {}) { return this.deleteByQuery({ match_all: {} }, { refresh }); } - + // ? This looks not in use anymore ? static batchExecute(query, callback) { return global.kuzzle.internalIndex.mExecute( this.collection, diff --git a/test/model/storage/apiKey.test.js b/test/model/storage/apiKey.test.js index ffcc9b876c..8e392cb047 100644 --- a/test/model/storage/apiKey.test.js +++ b/test/model/storage/apiKey.test.js @@ -116,8 +116,8 @@ describe("ApiKey", () => { describe("ApiKey.load", () => { it("should throw if the key does not belong to the provided user", async () => { - const loadStub = sinon - .stub(BaseModel, "load") + const searchStub = sinon + .stub(BaseModel, "search") .resolves({ userId: "mylehuong" }); const promise = ApiKey.load("aschen", "api-key-id"); @@ -126,7 +126,13 @@ describe("ApiKey", () => { id: "services.storage.not_found", }); - should(loadStub).be.calledWith("api-key-id"); + should(searchStub).be.calledWith({ + query: { + term: { + userId: "aschen", + }, + }, + }); }); });