feat(auth): verifyJwtOneOf + чеканка api-key токенов без exp

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) <noreply@anthropic.com>
2026-07-12 00:25:16 +03:00
parent bfb6a52eea
commit d738780370
2 changed files with 191 additions and 4 deletions
@@ -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<string, unknown>;
+    // 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<string, unknown>).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';
+}
@@ -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<string> {
-    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;
+  }
 }