From acecfca372e7ed971ec93478bc2ff5222fc6a54c Mon Sep 17 00:00:00 2001 From: Stefan Ernst Date: Mon, 4 May 2026 11:54:46 +0200 Subject: [PATCH 1/2] fix(api-key): emit long-lived JWTs for "no expiration" keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit API keys created via Settings → API management with the "no expiration" option were silently issued JWTs that carried a 90-day exp claim. The DB column correctly stored expires_at = NULL and ApiKeyService.validateApiKey honored that, but the JWT verify layer (passport-jwt in JwtStrategy and jwtService.verifyAsync in McpAuthGuard) enforces the standard exp claim first. After 90 days the token starts failing every request even though the DB still considers the key valid forever — and there is no refresh endpoint, so users have to revoke + recreate. Root cause: TokenService.generateApiToken was calling jwtService.sign(payload, expiresIn ? { expiresIn } : {}) When expiresIn is undefined, the empty options object inherits the module-level signOptions.expiresIn default ('90d', set in TokenModule for short-lived access tokens). passing { expiresIn: undefined } would also fail because jsonwebtoken@9 validates options strictly and rejects undefined. Fix: always pass an explicit expiresIn, falling back to '100y' for the no-expiration case so the JWT exp matches the DB's "no expiry" semantics in practice. The DB's apiKey.expiresAt remains the authoritative source of truth. Tests: replaced the smoke-only token.service.spec with two real assertions that wire up a JwtModule mirroring production config and decode the returned token. Locks in the regression — the no-expiry test fails on the unfixed code with TTL = 7,776,000 (exactly 90 days). Note: existing API keys already issued with the bug still have the bad exp baked into their JWT signature and will expire 90 days from creation. Affected users need to revoke and recreate their keys after upgrading. --- .../core/auth/services/token.service.spec.ts | 85 ++++++++++++++++++- .../src/core/auth/services/token.service.ts | 10 ++- 2 files changed, 93 insertions(+), 2 deletions(-) diff --git a/apps/server/src/core/auth/services/token.service.spec.ts b/apps/server/src/core/auth/services/token.service.spec.ts index a5f5d655a7..bd928cedd0 100644 --- a/apps/server/src/core/auth/services/token.service.spec.ts +++ b/apps/server/src/core/auth/services/token.service.spec.ts @@ -1,12 +1,38 @@ +// `common/helpers` transitively imports `nanoid` (ESM-only), which the repo's +// current jest config does not transform. We don't exercise any of those +// helpers in this spec, so stub the barrel out to keep this file runnable +// without touching the global jest config. +jest.mock('../../../common/helpers', () => ({ + isUserDisabled: () => false, +})); + import { Test, TestingModule } from '@nestjs/testing'; +import { JwtModule } from '@nestjs/jwt'; +import * as jwt from 'jsonwebtoken'; import { TokenService } from './token.service'; +import { EnvironmentService } from '../../../integrations/environment/environment.service'; +import { createMockUser } from '../../../test-utils/test-helpers'; describe('TokenService', () => { let service: TokenService; beforeEach(async () => { const module: TestingModule = await Test.createTestingModule({ - providers: [TokenService], + imports: [ + // Mirror production: a module-level expiresIn default exists, intended + // for short-lived access tokens. generateApiToken must NOT inherit it. + JwtModule.register({ + secret: 'test-secret', + signOptions: { expiresIn: '90d', issuer: 'Docmost' }, + }), + ], + providers: [ + TokenService, + { + provide: EnvironmentService, + useValue: { getAppSecret: () => 'test-secret' }, + }, + ], }).compile(); service = module.get(TokenService); @@ -15,4 +41,61 @@ describe('TokenService', () => { it('should be defined', () => { expect(service).toBeDefined(); }); + + describe('generateApiToken', () => { + const user = createMockUser(); + + it('uses a long-lived exp (~100y) when no expiresIn is supplied, instead of inheriting the 90d module default', async () => { + const before = Math.floor(Date.now() / 1000); + + const token = await service.generateApiToken({ + apiKeyId: 'key-id-1', + user, + workspaceId: user.workspaceId, + }); + + const decoded = jwt.decode(token) as { + exp?: number; + iat?: number; + type?: string; + apiKeyId?: string; + sub?: string; + workspaceId?: string; + } | null; + expect(decoded).not.toBeNull(); + expect(decoded!.type).toBe('api_key'); + expect(decoded!.apiKeyId).toBe('key-id-1'); + expect(decoded!.sub).toBe(user.id); + expect(decoded!.workspaceId).toBe(user.workspaceId); + + // Must NOT be the leaky 90d module default (the bug). Anything past one + // year out is fine; we use 100y in production. + const ninetyDaysSeconds = 90 * 24 * 60 * 60; + const oneYearSeconds = 365 * 24 * 60 * 60; + expect(decoded!.iat).toBeDefined(); + expect(decoded!.exp).toBeDefined(); + expect(decoded!.iat!).toBeGreaterThanOrEqual(before); + const ttl = decoded!.exp! - decoded!.iat!; + expect(ttl).toBeGreaterThan(oneYearSeconds); + expect(ttl).not.toBe(ninetyDaysSeconds); + }); + + it('respects an explicit numeric expiresIn (seconds) when supplied', async () => { + const before = Math.floor(Date.now() / 1000); + + const token = await service.generateApiToken({ + apiKeyId: 'key-id-2', + user, + workspaceId: user.workspaceId, + expiresIn: 3600, + }); + + const decoded = jwt.decode(token) as { exp?: number; iat?: number } | null; + expect(decoded).not.toBeNull(); + expect(decoded!.exp).toBeDefined(); + expect(decoded!.iat).toBeDefined(); + expect(decoded!.exp! - decoded!.iat!).toBe(3600); + expect(decoded!.iat!).toBeGreaterThanOrEqual(before); + }); + }); }); diff --git a/apps/server/src/core/auth/services/token.service.ts b/apps/server/src/core/auth/services/token.service.ts index b9035ba36f..1a5e7d2425 100644 --- a/apps/server/src/core/auth/services/token.service.ts +++ b/apps/server/src/core/auth/services/token.service.ts @@ -112,7 +112,15 @@ export class TokenService { type: JwtType.API_KEY, }; - return this.jwtService.sign(payload, expiresIn ? { expiresIn } : {}); + // Always pass an explicit expiresIn so we override the module-level + // signOptions.expiresIn default (currently '90d', intended for short-lived + // access tokens). When the API key was created with "no expiration", fall + // back to a 100-year window — practically equivalent to never expiring, + // and required because jsonwebtoken's option validator rejects an + // `expiresIn: undefined` override. The DB's apiKey.expiresAt column + // remains the authoritative source of truth for API key expiry, enforced + // by ApiKeyService.validateApiKey. + return this.jwtService.sign(payload, { expiresIn: expiresIn ?? '100y' }); } async verifyJwt(token: string, tokenType: string) { From 3ed015211eb31568973a97a104b042d1c5adec40 Mon Sep 17 00:00:00 2001 From: Finn Dittmar Date: Mon, 4 May 2026 12:42:53 +0200 Subject: [PATCH 2/2] shorten comments --- .../src/core/auth/services/token.service.spec.ts | 4 ---- apps/server/src/core/auth/services/token.service.ts | 10 ++-------- 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/apps/server/src/core/auth/services/token.service.spec.ts b/apps/server/src/core/auth/services/token.service.spec.ts index bd928cedd0..c167be35da 100644 --- a/apps/server/src/core/auth/services/token.service.spec.ts +++ b/apps/server/src/core/auth/services/token.service.spec.ts @@ -1,7 +1,3 @@ -// `common/helpers` transitively imports `nanoid` (ESM-only), which the repo's -// current jest config does not transform. We don't exercise any of those -// helpers in this spec, so stub the barrel out to keep this file runnable -// without touching the global jest config. jest.mock('../../../common/helpers', () => ({ isUserDisabled: () => false, })); diff --git a/apps/server/src/core/auth/services/token.service.ts b/apps/server/src/core/auth/services/token.service.ts index 1a5e7d2425..d9edec1c36 100644 --- a/apps/server/src/core/auth/services/token.service.ts +++ b/apps/server/src/core/auth/services/token.service.ts @@ -112,14 +112,8 @@ export class TokenService { type: JwtType.API_KEY, }; - // Always pass an explicit expiresIn so we override the module-level - // signOptions.expiresIn default (currently '90d', intended for short-lived - // access tokens). When the API key was created with "no expiration", fall - // back to a 100-year window — practically equivalent to never expiring, - // and required because jsonwebtoken's option validator rejects an - // `expiresIn: undefined` override. The DB's apiKey.expiresAt column - // remains the authoritative source of truth for API key expiry, enforced - // by ApiKeyService.validateApiKey. + // due to the default value of 90 days, if no expiry is provided + // 100y are set as a maximum length return this.jwtService.sign(payload, { expiresIn: expiresIn ?? '100y' }); }