Skip to content

Commit

Permalink
fix: do not store api keys in internal index, use fingerprint instead
Browse files Browse the repository at this point in the history
  • Loading branch information
Juiced66 committed Oct 30, 2024
1 parent 7b4b336 commit c9cb2b8
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 74 deletions.
93 changes: 35 additions & 58 deletions lib/core/security/tokenRepository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -61,8 +61,6 @@ export class TokenRepository extends ObjectRepository<Token> {
}

async init() {
await this.loadApiKeys();

/**
* Assign an existing token to a user. Stores the token in Kuzzle's cache.
* @param {String} hash - JWT
Expand Down Expand Up @@ -272,10 +270,21 @@ export class TokenRepository extends ObjectRepository<Token> {

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,
Expand Down Expand Up @@ -329,11 +338,13 @@ export class TokenRepository extends ObjectRepository<Token> {
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");
Expand All @@ -350,15 +361,30 @@ export class TokenRepository extends ObjectRepository<Token> {
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);
} catch (err) {
if (err instanceof UnauthorizedError) {
throw err;
}

throw securityError.getFrom(err, "verification_error", err.message);
}

Expand Down Expand Up @@ -457,55 +483,6 @@ export class TokenRepository extends ObjectRepository<Token> {
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
Expand Down
34 changes: 23 additions & 11 deletions lib/model/storage/apiKey.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -107,7 +107,6 @@ class ApiKey extends BaseModel {
description,
expiresAt: token.expiresAt,
fingerprint,
token: token.jwt,
ttl: token.ttl,
userId: user._id,
},
Expand All @@ -116,6 +115,8 @@ class ApiKey extends BaseModel {

await apiKey.save({ refresh, userId: creatorId });

apiKey.token = token.jwt;

return apiKey;
}

Expand All @@ -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}".`,
});
}

/**
Expand Down
3 changes: 1 addition & 2 deletions lib/model/storage/baseModel.js
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,6 @@ class BaseModel {
searchBody,
options,
);

return resp.hits.map((hit) => this._instantiateFromDb(hit));
}

Expand All @@ -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,
Expand Down
12 changes: 9 additions & 3 deletions test/model/storage/apiKey.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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",
},
},
});
});
});

Expand Down

0 comments on commit c9cb2b8

Please sign in to comment.