From d738780370b460fe0b45c44b04a1fef7d18bae6c Mon Sep 17 00:00:00 2001 From: agent_coder Date: Sun, 12 Jul 2026 00:25:16 +0300 Subject: [PATCH] =?UTF-8?q?feat(auth):=20verifyJwtOneOf=20+=20=D1=87=D0=B5?= =?UTF-8?q?=D0=BA=D0=B0=D0=BD=D0=BA=D0=B0=20api-key=20=D1=82=D0=BE=D0=BA?= =?UTF-8?q?=D0=B5=D0=BD=D0=BE=D0=B2=20=D0=B1=D0=B5=D0=B7=20exp?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TokenService получает примитив type-routing`verifyJwtOneOf(token, allowed[])`: подпись проверяется один раз, тип обязан входить в явный allowlist, иначе — тот же generic-throw, что и у verifyJwt. Нужен для /mcp-Bearer, который принимает и ACCESS, и API_KEY на одном слоте, но без переиспользуемого footgun'а «verify-and-return-any-type». generateApiToken теперь чеканит api-key токен БЕЗ клейма exp. Единственный источник истины о сроке/отзыве ключа — строка api_keys (проверяется на каждом запросе), не JWT. Общий jwtService зарегистрирован с глобальным signOptions.expiresIn ('90d'), который мержится в любой sign() — даже sign(payload, {}) — а {expiresIn: undefined} бросает; поэтому «бессрочный» ключ через общий signer молча получал бы exp=now+90d. Чеканка идёт через выделенный signer без expiresIn (issuer 'Docmost' для паритета клеймов). Живой баг подтверждён эмпирически и покрыт тестом. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../services/token.service.behavior.spec.ts | 137 ++++++++++++++++++ .../src/core/auth/services/token.service.ts | 58 +++++++- 2 files changed, 191 insertions(+), 4 deletions(-) diff --git a/apps/server/src/core/auth/services/token.service.behavior.spec.ts b/apps/server/src/core/auth/services/token.service.behavior.spec.ts index 32293c27..36ef36ae 100644 --- a/apps/server/src/core/auth/services/token.service.behavior.spec.ts +++ b/apps/server/src/core/auth/services/token.service.behavior.spec.ts @@ -1,4 +1,5 @@ import { ForbiddenException, UnauthorizedException } from '@nestjs/common'; +import * as jwt from 'jsonwebtoken'; import { TokenService } from './token.service'; import { JwtType } from '../dto/jwt-payload'; @@ -214,3 +215,139 @@ describe('TokenService.generateCollabToken', () => { }); }); }); + +/** + * API-key token minting MUST carry NO `exp` claim — the ONLY source of truth for + * a key's lifetime/revocation is its `api_keys` row, checked on every request. + * + * This is a LIVE-bug regression: the shared JwtService is registered with a + * global `signOptions.expiresIn` (default '90d') that merges into every sign(), + * so an api-key minted through it silently gets exp=now+90d and an "unlimited" + * key dies in 90 days. TokenService.generateApiToken mints through a dedicated + * no-expiry signer instead. These tests construct the REAL signer (a real secret + * via the stubbed EnvironmentService) and decode the produced JWT to assert the + * observable property: no `exp`. + */ +describe('TokenService.generateApiToken (no exp claim ever)', () => { + const APP_SECRET_LOCAL = 'apikey-secret'; + + function makeRealSignerService() { + // Give the SHARED jwtService a global expiresIn so a regression (minting + // through it) would show up as an exp claim — the exact live bug. + const { JwtService } = require('@nestjs/jwt'); + const sharedJwt = new JwtService({ + secret: APP_SECRET_LOCAL, + signOptions: { expiresIn: '90d', issuer: 'Docmost' }, + }); + const environmentService = { + getAppSecret: () => APP_SECRET_LOCAL, + }; + const service = new (TokenService as unknown as new ( + ...args: unknown[] + ) => TokenService)(sharedJwt, environmentService); + return { service }; + } + + const user = makeUser({ id: 'svc-1', workspaceId: 'ws-1' }); + + it('mints an api-key JWT with NO exp claim and issuer Docmost', async () => { + const { service } = makeRealSignerService(); + + const token = await service.generateApiToken({ + apiKeyId: 'key-1', + user: user as never, + workspaceId: 'ws-1', + }); + + const decoded = jwt.decode(token) as Record; + // The observable security property: no expiry lives in the JWT. + expect(decoded.exp).toBeUndefined(); + expect(decoded).toMatchObject({ + sub: 'svc-1', + apiKeyId: 'key-1', + workspaceId: 'ws-1', + type: JwtType.API_KEY, + iss: 'Docmost', + }); + }); + + it('demonstrates the live bug it guards: the SHARED signer WOULD add exp', () => { + const { JwtService } = require('@nestjs/jwt'); + const sharedJwt = new JwtService({ + secret: APP_SECRET_LOCAL, + signOptions: { expiresIn: '90d', issuer: 'Docmost' }, + }); + // Even with empty per-call options the global expiresIn merges in. + const leaky = sharedJwt.sign({ sub: 'x', type: JwtType.API_KEY }, {}); + expect((jwt.decode(leaky) as Record).exp).toBeDefined(); + }); + + it('refuses to mint for a disabled user', async () => { + const { service } = makeRealSignerService(); + await expect( + service.generateApiToken({ + apiKeyId: 'key-1', + user: makeUser({ deactivatedAt: new Date() }) as never, + workspaceId: 'ws-1', + }), + ).rejects.toBeInstanceOf(ForbiddenException); + }); +}); + +/** + * verifyJwtOneOf is the type-routing primitive: verify the signature once and + * assert the token type is on an explicit allowlist. It must NOT degrade into a + * "return whatever type" helper — a token whose type is off the allowlist is + * rejected with the same generic error as a single-type mismatch. + */ +describe('TokenService.verifyJwtOneOf (allowlist type-routing)', () => { + it('returns the payload when the type is on the allowlist', async () => { + const verifyAsync = jest + .fn() + .mockResolvedValue({ type: JwtType.API_KEY, sub: 'u-1' }); + const { service } = makeTokenService({ verifyAsync }); + + const payload = await service.verifyJwtOneOf(token123(), [ + JwtType.ACCESS, + JwtType.API_KEY, + ]); + + expect(payload).toMatchObject({ type: JwtType.API_KEY, sub: 'u-1' }); + expect(verifyAsync).toHaveBeenCalledTimes(1); + }); + + it('accepts the OTHER allowed type too', async () => { + const verifyAsync = jest + .fn() + .mockResolvedValue({ type: JwtType.ACCESS, sub: 'u-1' }); + const { service } = makeTokenService({ verifyAsync }); + + await expect( + service.verifyJwtOneOf(token123(), [JwtType.ACCESS, JwtType.API_KEY]), + ).resolves.toMatchObject({ type: JwtType.ACCESS }); + }); + + it('rejects a token whose type is OFF the allowlist (confused-deputy guard)', async () => { + const verifyAsync = jest + .fn() + .mockResolvedValue({ type: JwtType.COLLAB, sub: 'u-1' }); + const { service } = makeTokenService({ verifyAsync }); + + await expect( + service.verifyJwtOneOf(token123(), [JwtType.ACCESS, JwtType.API_KEY]), + ).rejects.toBeInstanceOf(UnauthorizedException); + }); + + it('verifies the signature exactly ONCE', async () => { + const verifyAsync = jest + .fn() + .mockResolvedValue({ type: JwtType.ACCESS }); + const { service } = makeTokenService({ verifyAsync }); + await service.verifyJwtOneOf(token123(), [JwtType.ACCESS]); + expect(verifyAsync).toHaveBeenCalledTimes(1); + }); +}); + +function token123(): string { + return 'a.b.c'; +} diff --git a/apps/server/src/core/auth/services/token.service.ts b/apps/server/src/core/auth/services/token.service.ts index 33a20069..d172075a 100644 --- a/apps/server/src/core/auth/services/token.service.ts +++ b/apps/server/src/core/auth/services/token.service.ts @@ -4,7 +4,6 @@ import { UnauthorizedException, } from '@nestjs/common'; import { JwtService } from '@nestjs/jwt'; -import type { StringValue } from 'ms'; import { EnvironmentService } from '../../../integrations/environment/environment.service'; import { JwtApiKeyPayload, @@ -123,9 +122,8 @@ export class TokenService { apiKeyId: string; user: User; workspaceId: string; - expiresIn?: StringValue | number; }): Promise { - const { apiKeyId, user, workspaceId, expiresIn } = opts; + const { apiKeyId, user, workspaceId } = opts; if (isUserDisabled(user)) { throw new ForbiddenException(); } @@ -137,7 +135,32 @@ export class TokenService { type: JwtType.API_KEY, }; - return this.jwtService.sign(payload, expiresIn ? { expiresIn } : {}); + // API-key tokens carry NO `exp` claim EVER — the ONLY source of truth for a + // key's lifetime and revocation is its `api_keys` row (checked on every + // request), not the JWT. This CANNOT use `this.jwtService`: TokenModule + // registers it with a global `signOptions.expiresIn` (JWT_TOKEN_EXPIRES_IN, + // default '90d'), which merges into EVERY sign() call — even `sign(payload, + // {})` — and `{ expiresIn: undefined }` THROWS rather than stripping it + // (verified empirically). So an "unlimited" key minted through the shared + // signer would silently get exp=now+90d and die in 90 days regardless of its + // row. We mint through a dedicated no-expiry signer, re-stamping only + // `issuer: 'Docmost'` for claim parity with the shared signer. + return this.apiKeyJwtService().sign(payload); + } + + // Lazily-built JWT signer for API-key tokens: same APP_SECRET, same 'Docmost' + // issuer, but WITHOUT the global `expiresIn` — so minted API-key tokens have no + // `exp` claim. Built once and cached. Verification still goes through the + // shared verifier (same secret); `verifyAsync` does not require an `exp`. + private _apiKeyJwtService?: JwtService; + private apiKeyJwtService(): JwtService { + if (!this._apiKeyJwtService) { + this._apiKeyJwtService = new JwtService({ + secret: this.environmentService.getAppSecret(), + signOptions: { issuer: 'Docmost' }, + }); + } + return this._apiKeyJwtService; } async generatePdfRenderToken( @@ -177,4 +200,31 @@ export class TokenService { return payload; } + + /** + * Verify a token's signature ONCE and assert its `type` is one of `allowed`. + * + * This is the type-routing primitive for surfaces that legitimately accept + * more than one token type on the same Bearer slot (the /mcp Bearer path + * accepts both an ACCESS and an API_KEY token). It is deliberately NOT a + * "verify-and-return-whatever-type" helper — that would be a reusable + * confused-deputy footgun (any caller could then feed an attachment/collab + * token where an access token is expected). An explicit allowlist preserves + * the type-pinning property of `verifyJwt`: a token whose `type` is not in the + * allowlist is rejected with the SAME generic error as a type mismatch, and + * the signature is verified exactly once (no double-verify). + */ + async verifyJwtOneOf(token: string, allowed: JwtType[]) { + const payload = await this.jwtService.verifyAsync(token, { + secret: this.environmentService.getAppSecret(), + }); + + if (!allowed.includes(payload.type)) { + throw new UnauthorizedException( + 'Invalid JWT token. Token type does not match.', + ); + } + + return payload; + } }