diff --git a/src/lib/middleware/api-token-middleware.ts b/src/lib/middleware/api-token-middleware.ts index f698219abc..0fceff28db 100644 --- a/src/lib/middleware/api-token-middleware.ts +++ b/src/lib/middleware/api-token-middleware.ts @@ -78,9 +78,7 @@ const apiAccessMiddleware = ( // If we're here, we know that api token middleware was enabled, otherwise we'd returned a no-op middleware // We explicitly only protect client and proxy apis, since admin apis are protected by our permission checker // Reject with 401 - logger.warn( - `Client api request without valid token (${apiToken}), rejecting`, - ); + logger.warn(`No user found for token, rejecting`); res.status(401).send({ message: NO_TOKEN_WHERE_TOKEN_WAS_REQUIRED, }); diff --git a/src/lib/services/api-token-service.test.ts b/src/lib/services/api-token-service.test.ts index 83f3ab13fc..116eb81115 100644 --- a/src/lib/services/api-token-service.test.ts +++ b/src/lib/services/api-token-service.test.ts @@ -12,7 +12,7 @@ import { API_TOKEN_UPDATED, TEST_AUDIT_USER, } from '../types'; -import { addDays, minutesToMilliseconds } from 'date-fns'; +import { addDays, minutesToMilliseconds, subDays } from 'date-fns'; import EventService from '../features/events/event-service'; import FakeFeatureTagStore from '../../test/fixtures/fake-feature-tag-store'; import { createFakeEventsService } from '../../lib/features'; @@ -261,4 +261,18 @@ describe('API token getTokenWithCache', () => { } expect(apiTokenStoreGet).toHaveBeenCalledTimes(2); }); + + test('should not return the token if it has expired and shoud perform only one db query', async () => { + const { apiTokenService, apiTokenStore } = setup(); + const apiTokenStoreGet = jest.spyOn(apiTokenStore, 'get'); + + // valid token not present in cache but expired + apiTokenStore.insert({ ...token, expiresAt: subDays(new Date(), 1) }); + + for (let i = 0; i < 5; i++) { + const found = await apiTokenService.getTokenWithCache(token.secret); + expect(found).toBeUndefined(); + } + expect(apiTokenStoreGet).toHaveBeenCalledTimes(1); + }); }); diff --git a/src/lib/services/api-token-service.ts b/src/lib/services/api-token-service.ts index 2b859eec78..4778697c3f 100644 --- a/src/lib/services/api-token-service.ts +++ b/src/lib/services/api-token-service.ts @@ -145,36 +145,41 @@ export class ApiTokenService { } const nextAllowedQuery = this.queryAfter.get(secret) ?? 0; - this.logger.info( - `Token found in cache: ${Boolean( - token, - )}, next allowed query: ${nextAllowedQuery}`, - ); - if (!token && isPast(nextAllowedQuery)) { - this.logger.info( - `Token not found in cache, querying database for token with secret: ${secret}`, - ); - if (this.queryAfter.size > 1000) { - // establish a max limit for queryAfter size to prevent memory leak + if (!token) { + if (isPast(nextAllowedQuery)) { + this.logger.info(`Token not found in cache, querying database`); + if (this.queryAfter.size > 1000) { + // establish a max limit for queryAfter size to prevent memory leak + this.logger.info( + 'queryAfter size exceeded 1000, clearing cache', + ); + this.queryAfter.clear(); + } + + const stopCacheTimer = this.timer('getTokenWithCache.query'); + token = await this.store.get(secret); + if (token) { + if (token?.expiresAt && isPast(token.expiresAt)) { + this.logger.info('Token has expired'); + // prevent querying the same invalid secret multiple times. Expire after 5 minutes + this.queryAfter.set(secret, addMinutes(new Date(), 5)); + token = undefined; + } else { + this.activeTokens.push(token); + } + } else { + // prevent querying the same invalid secret multiple times. Expire after 5 minutes + this.queryAfter.set(secret, addMinutes(new Date(), 5)); + } + stopCacheTimer(); + } else { this.logger.info( - 'queryAfter size exceeded 1000, clearing cache', + `Not allowed to query this token until: ${this.queryAfter.get( + secret, + )}`, ); - this.queryAfter.clear(); } - // prevent querying the same invalid secret multiple times. Expire after 5 minutes - this.queryAfter.set(secret, addMinutes(new Date(), 5)); - this.logger.info( - `Added ${secret} to queryAfter: ${this.queryAfter.get(secret)}`, - ); - - const stopCacheTimer = this.timer('getTokenWithCache.query'); - token = await this.store.get(secret); - if (token) { - this.activeTokens.push(token); - } - stopCacheTimer(); } - return token; } @@ -213,7 +218,7 @@ export class ApiTokenService { secret: string, ): Promise { const token = await this.getTokenWithCache(secret); - this.logger.info(`getUserForToken ${secret} found: ${token}`); + this.logger.info(`Found user? ${token ? 'yes' : 'no'}`); if (token) { this.lastSeenSecrets.add(token.secret); const apiUser: IApiUser = new ApiUser({