diff --git a/apps/server/src/collaboration/collaboration.module.ts b/apps/server/src/collaboration/collaboration.module.ts index 66d8001f..94ebba62 100644 --- a/apps/server/src/collaboration/collaboration.module.ts +++ b/apps/server/src/collaboration/collaboration.module.ts @@ -16,6 +16,7 @@ import { TransclusionService } from '../core/page/transclusion/transclusion.serv import { TransclusionModule } from '../core/page/transclusion/transclusion.module'; import { StorageModule } from '../integrations/storage/storage.module'; import { EnvironmentModule } from '../integrations/environment/environment.module'; +import { ApiKeyModule } from '../core/api-key/api-key.module'; @Module({ providers: [ @@ -31,6 +32,7 @@ import { EnvironmentModule } from '../integrations/environment/environment.modul exports: [CollaborationGateway], imports: [ TokenModule, + ApiKeyModule, WatcherModule, StorageModule.forRootAsync({ imports: [EnvironmentModule], diff --git a/apps/server/src/collaboration/extensions/authentication.extension.spec.ts b/apps/server/src/collaboration/extensions/authentication.extension.spec.ts index 585393a4..c22118d5 100644 --- a/apps/server/src/collaboration/extensions/authentication.extension.spec.ts +++ b/apps/server/src/collaboration/extensions/authentication.extension.spec.ts @@ -52,6 +52,7 @@ describe('AuthenticationExtension.onAuthenticate', () => { let pageRepo: { findById: jest.Mock }; let spaceMemberRepo: { getUserSpaceRoles: jest.Mock }; let pagePermissionRepo: { canUserEditPage: jest.Mock }; + let apiKeyService: { validate: jest.Mock }; // Build the hocuspocus onAuthenticate payload. connectionConfig.readOnly // starts false; the extension flips it to true on a read-only downgrade. @@ -79,12 +80,15 @@ describe('AuthenticationExtension.onAuthenticate', () => { }), }; + apiKeyService = { validate: jest.fn().mockResolvedValue({ user: {}, workspace: {} }) }; + ext = new AuthenticationExtension( tokenService as any, userRepo as any, pageRepo as any, spaceMemberRepo as any, pagePermissionRepo as any, + apiKeyService as any, ); // Silence the extension's logger (it warns/debugs on denial branches). jest.spyOn(ext['logger'], 'warn').mockImplementation(() => undefined); @@ -231,4 +235,73 @@ describe('AuthenticationExtension.onAuthenticate', () => { // No internal ai_chats row for an MCP/service-account collab edit → null. expect(ctx.aiChatId).toBeNull(); }); + + // --- #501: api-key laundering guard (fail-closed discriminator) ---------- + describe('api-key laundering guard', () => { + it('api_key principal → row-checks the key on connect (valid key proceeds)', async () => { + tokenService.verifyJwt.mockResolvedValue( + buildJwt({ principal: 'api_key', apiKeyId: 'key-1' }), + ); + const data = buildData(); + await ext.onAuthenticate(data as any); + + expect(apiKeyService.validate).toHaveBeenCalledTimes(1); + expect(apiKeyService.validate).toHaveBeenCalledWith( + expect.objectContaining({ apiKeyId: 'key-1', type: JwtType.API_KEY }), + ); + }); + + it('REVOKED api_key → Unauthorized on connect, BEFORE any page/user lookup', async () => { + tokenService.verifyJwt.mockResolvedValue( + buildJwt({ principal: 'api_key', apiKeyId: 'key-1' }), + ); + // The shared validator denies a revoked key. + apiKeyService.validate.mockRejectedValue(new UnauthorizedException()); + + await expect(ext.onAuthenticate(buildData() as any)).rejects.toThrow( + UnauthorizedException, + ); + // No new collab connection: the key check gates before page access. + expect(pageRepo.findById).not.toHaveBeenCalled(); + }); + + it('api_key principal missing apiKeyId → Unauthorized (malformed)', async () => { + tokenService.verifyJwt.mockResolvedValue(buildJwt({ principal: 'api_key' })); + await expect(ext.onAuthenticate(buildData() as any)).rejects.toThrow( + UnauthorizedException, + ); + expect(apiKeyService.validate).not.toHaveBeenCalled(); + }); + + it('session principal → NO api-key check (session-backed, incl. internal agent)', async () => { + tokenService.verifyJwt.mockResolvedValue(buildJwt({ principal: 'session' })); + await ext.onAuthenticate(buildData() as any); + expect(apiKeyService.validate).not.toHaveBeenCalled(); + }); + + it('claimless token WITHIN the grace window → trusted (legacy pre-rollout)', async () => { + // Default rolloutAt = now, so we are inside the grace window. + tokenService.verifyJwt.mockResolvedValue(buildJwt()); // no principal + await expect(ext.onAuthenticate(buildData() as any)).resolves.toBeDefined(); + expect(apiKeyService.validate).not.toHaveBeenCalled(); + }); + + it('claimless token AFTER the grace window → Unauthorized (fail-closed)', async () => { + // Move the rollout reference far into the past so the grace has elapsed. + (ext as any).rolloutAt = Date.now() - 25 * 60 * 60 * 1000; + tokenService.verifyJwt.mockResolvedValue(buildJwt()); // no principal + await expect(ext.onAuthenticate(buildData() as any)).rejects.toThrow( + UnauthorizedException, + ); + }); + + it('infra error from the api-key row-check propagates (not masked)', async () => { + tokenService.verifyJwt.mockResolvedValue( + buildJwt({ principal: 'api_key', apiKeyId: 'key-1' }), + ); + const boom = new Error('db down'); + apiKeyService.validate.mockRejectedValue(boom); + await expect(ext.onAuthenticate(buildData() as any)).rejects.toBe(boom); + }); + }); }); diff --git a/apps/server/src/collaboration/extensions/authentication.extension.ts b/apps/server/src/collaboration/extensions/authentication.extension.ts index 79b4246c..5f1c3f73 100644 --- a/apps/server/src/collaboration/extensions/authentication.extension.ts +++ b/apps/server/src/collaboration/extensions/authentication.extension.ts @@ -14,20 +14,37 @@ import { findHighestUserSpaceRole } from '@docmost/db/repos/space/utils'; import { SpaceRole } from '../../common/helpers/types/permission'; import { isUserDisabled } from '../../common/helpers'; import { getPageId } from '../collaboration.util'; -import { JwtCollabPayload, JwtType } from '../../core/auth/dto/jwt-payload'; +import { + JwtApiKeyPayload, + JwtCollabPayload, + JwtType, +} from '../../core/auth/dto/jwt-payload'; import { resolveProvenance } from '../../common/decorators/auth-provenance.decorator'; import { observeCollabAuth } from '../../integrations/metrics/metrics.registry'; +import { ApiKeyService } from '../../core/api-key/api-key.service'; + +// Max lifetime of a collab token (generateCollabToken uses expiresIn '24h'). Used +// as the rollout grace window below: once this long has elapsed since this +// process started serving the #501 code, every STILL-VALID collab token was +// necessarily minted post-rollout and MUST carry the `principal` discriminator, +// so a claimless one is a bug and is rejected (fail-closed) rather than trusted. +const COLLAB_TOKEN_GRACE_MS = 24 * 60 * 60 * 1000; @Injectable() export class AuthenticationExtension implements Extension { private readonly logger = new Logger(AuthenticationExtension.name); + // Reference instant for the claimless-rejection grace window. Overridable so a + // unit test can drive the pre-/post-grace boundary without wall-clock waits. + protected rolloutAt = Date.now(); + constructor( private tokenService: TokenService, private userRepo: UserRepo, private pageRepo: PageRepo, private readonly spaceMemberRepo: SpaceMemberRepo, private readonly pagePermissionRepo: PagePermissionRepo, + private readonly apiKeyService: ApiKeyService, ) {} async onAuthenticate(data: onAuthenticatePayload) { @@ -54,6 +71,36 @@ export class AuthenticationExtension implements Extension { throw new UnauthorizedException('Invalid collab token'); } + // #501 — fail-closed api-key laundering guard. A collab token minted by an + // api-key principal carries principal='api_key' + apiKeyId; re-check the key + // on connect so a REVOKED key gets NO new collab connections (a collab token + // outlives its 24h, but a revoked key can no longer open fresh ones). An + // api-key token missing its apiKeyId is malformed → reject. A claimless token + // (no recognized principal) is trusted only DURING the rollout grace window + // (a legacy pre-rollout session token, which api keys could never mint); + // once the grace has elapsed every valid token must carry the discriminator, + // so a claimless one is a bug and is rejected (not silently trusted for 24h). + const principal = jwtPayload.principal; + if (principal === 'api_key') { + if (!jwtPayload.apiKeyId) { + throw new UnauthorizedException(); + } + // Row-check via the SHARED validator: throws Unauthorized on a revoked/ + // expired/disabled key; an infra error propagates (not masked). No new + // connection for a dead key. + await this.apiKeyService.validate({ + sub: jwtPayload.sub, + workspaceId: jwtPayload.workspaceId, + apiKeyId: jwtPayload.apiKeyId, + type: JwtType.API_KEY, + } as JwtApiKeyPayload); + } else if (principal !== 'session') { + // Unrecognized/absent discriminator: reject once past the grace window. + if (Date.now() - this.rolloutAt >= COLLAB_TOKEN_GRACE_MS) { + throw new UnauthorizedException(); + } + } + const userId = jwtPayload.sub; const workspaceId = jwtPayload.workspaceId; diff --git a/apps/server/src/core/auth/auth.controller.ts b/apps/server/src/core/auth/auth.controller.ts index f15d82e4..6991feff 100644 --- a/apps/server/src/core/auth/auth.controller.ts +++ b/apps/server/src/core/auth/auth.controller.ts @@ -207,8 +207,20 @@ export class AuthController { async collabToken( @AuthUser() user: User, @AuthWorkspace() workspace: Workspace, + @Req() req: FastifyRequest, ) { - return this.authService.getCollabToken(user, workspace.id); + // Thread the api-key origin (#501): when the requester authenticated with an + // api key (jwt.strategy stamped req.raw.authType/apiKeyId), the minted collab + // token carries principal='api_key' + apiKeyId so a later revoke of the key + // rejects NEW collab connections. A normal session request mints a + // principal='session' token. Reading the SIGNED-derived req.raw fields (never + // a client body) keeps it unspoofable. + const raw = req.raw as { authType?: string; apiKeyId?: string }; + const apiKey = + raw.authType === 'api_key' && raw.apiKeyId + ? { apiKeyId: raw.apiKeyId } + : undefined; + return this.authService.getCollabToken(user, workspace.id, apiKey); } @SkipThrottle({ [AUTH_THROTTLER]: true }) diff --git a/apps/server/src/core/auth/dto/jwt-payload.ts b/apps/server/src/core/auth/dto/jwt-payload.ts index b6a9f980..495f7610 100644 --- a/apps/server/src/core/auth/dto/jwt-payload.ts +++ b/apps/server/src/core/auth/dto/jwt-payload.ts @@ -33,6 +33,17 @@ export type JwtPayload = { aiChatId?: string | null; }; +// The AUTH-PRINCIPAL kind behind a collab token — distinct from `actor` +// (provenance). Stamped into EVERY newly-minted collab token (#501): 'session' +// for a normal user/session (incl. the internal AI agent, which is session- +// backed), 'api_key' when the token was minted by an api-key principal (an +// external MCP agent). The discriminator keys on the token's ORIGIN, so the +// collab seam can re-check a revoked api key on connect and reject a claimless +// token after the rollout grace window. NOT keyed on `actor:'agent'` — the +// internal agent is 'agent' but session-backed, so it must stay on the no-check +// path. +export type CollabPrincipal = 'session' | 'api_key'; + export type JwtCollabPayload = { sub: string; workspaceId: string; @@ -44,6 +55,13 @@ export type JwtCollabPayload = { // Nullable: an external MCP agent has no internal ai_chats row, so it carries // an 'agent' actor with a null aiChatId. aiChatId?: string | null; + // Auth-principal discriminator (#501). Present on every post-rollout token; + // its absence on a still-valid token past the grace window is treated as an + // error, not trust (fail-closed). + principal?: CollabPrincipal; + // Only when principal === 'api_key': the minting key's id, so the collab seam + // can row-check (and reject) a revoked key on connect. + apiKeyId?: string; }; export type JwtExchangePayload = { diff --git a/apps/server/src/core/auth/services/auth.service.ts b/apps/server/src/core/auth/services/auth.service.ts index 986084b9..e8d35186 100644 --- a/apps/server/src/core/auth/services/auth.service.ts +++ b/apps/server/src/core/auth/services/auth.service.ts @@ -375,10 +375,20 @@ export class AuthService { } } - async getCollabToken(user: User, workspaceId: string) { + async getCollabToken( + user: User, + workspaceId: string, + // Origin of the request minting this collab token (#501). When the caller is + // an api-key principal, its apiKeyId is threaded into the token so the collab + // seam can re-check the key on connect (closing api-key -> long-lived-collab + // laundering). Absent for a normal session/human request. + apiKey?: { apiKeyId: string }, + ) { const token = await this.tokenService.generateCollabToken( user, workspaceId, + undefined, + apiKey, ); return { token }; } 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 36ef36ae..de2f44f0 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 @@ -214,6 +214,41 @@ describe('TokenService.generateCollabToken', () => { aiChatId: 'chat-456', }); }); + + // #501 fail-closed discriminator: EVERY collab token carries a principal. + it("defaults principal to 'session' with NO apiKeyId (normal/internal-agent path)", async () => { + const { service, jwtService } = makeTokenService(); + await service.generateCollabToken(makeUser() as never, 'ws-1'); + const [payload] = jwtService.sign.mock.calls[0]; + expect(payload.principal).toBe('session'); + expect(payload).not.toHaveProperty('apiKeyId'); + }); + + it("the internal agent (provenance, NO apiKey) still gets principal='session'", async () => { + const { service, jwtService } = makeTokenService(); + await service.generateCollabToken( + makeUser() as never, + 'ws-1', + { actor: 'agent', aiChatId: 'chat-1' }, + ); + const [payload] = jwtService.sign.mock.calls[0]; + // Keyed on api-key ORIGIN, not actor: an is_agent session token is 'session'. + expect(payload.principal).toBe('session'); + expect(payload).not.toHaveProperty('apiKeyId'); + }); + + it("stamps principal='api_key' + apiKeyId when minted by an api-key principal", async () => { + const { service, jwtService } = makeTokenService(); + await service.generateCollabToken( + makeUser() as never, + 'ws-1', + undefined, + { apiKeyId: 'key-9' }, + ); + const [payload] = jwtService.sign.mock.calls[0]; + expect(payload.principal).toBe('api_key'); + expect(payload.apiKeyId).toBe('key-9'); + }); }); /** diff --git a/apps/server/src/core/auth/services/token.service.ts b/apps/server/src/core/auth/services/token.service.ts index d172075a..de202346 100644 --- a/apps/server/src/core/auth/services/token.service.ts +++ b/apps/server/src/core/auth/services/token.service.ts @@ -61,6 +61,12 @@ export class TokenService { // token carries no actor/aiChatId and is treated as 'user' downstream. // aiChatId is nullable for an external agent with no internal ai_chats row. provenance?: { actor: 'agent'; aiChatId: string | null }, + // Optional api-key origin (#501). When the collab token is minted by an + // api-key principal (an external MCP agent), the caller passes the key id so + // the token carries principal='api_key' + apiKeyId and the collab seam can + // re-check the key on connect. Absent -> principal='session' (a normal + // user/session, including the internal session-backed AI agent). + apiKey?: { apiKeyId: string }, ): Promise { if (isUserDisabled(user)) { throw new ForbiddenException(); @@ -70,6 +76,10 @@ export class TokenService { sub: user.id, workspaceId, type: JwtType.COLLAB, + // Fail-closed discriminator on EVERY minted token: 'api_key' when minted by + // an api-key principal, else 'session'. + principal: apiKey ? 'api_key' : 'session', + ...(apiKey ? { apiKeyId: apiKey.apiKeyId } : {}), ...(provenance ? { actor: provenance.actor, aiChatId: provenance.aiChatId } : {}),