feat(collab): закрытие api-key→collab отмывания (fail-closed дискриминатор)
api_key-принципал стал полноправным REST-принципалом → мог начеканить 24-ч
COLLAB-токен, который AuthenticationExtension не сверял с api_keys, и ревокация
не доставала websocket-редактирование. Блокировать /collab-token для api_key
нельзя (это правки агентов). Решение fail-closed:
- JwtCollabPayload расширен полем principal ('session'|'api_key') + apiKeyId.
generateCollabToken штампует его в КАЖДЫЙ токен: 'api_key'+apiKeyId, когда
чеканит api-key-принципал (внешний MCP-агент), иначе 'session'. Дискриминатор
ключуется на ОРИГИНЕ (наличии api-key), НЕ на actor:'agent' — внутренний
session-backed AI-агент остаётся 'session' и на no-check пути.
- /auth/collab-token читает подписью-выведенные req.raw.authType/apiKeyId
(не тело) и прокидывает origin через getCollabToken → generateCollabToken.
- doAuthenticate: при principal='api_key' на connect прогоняет
ApiKeyService.validate (отозванный ключ → новых collab-подключений нет; инфра
пробрасывается). api_key без apiKeyId — reject. Claimless-токен доверяется
только В ПРЕДЕЛАХ 24-ч grace-окна роллаута (легаси session-токен, api_key его
начеканить не мог); после окна всякий валидный токен обязан нести
дискриминатор, поэтому claimless — баг и отвергается (не молча-доверие 24ч).
CollaborationModule импортирует ApiKeyModule.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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],
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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;
|
||||
|
||||
|
||||
@@ -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 })
|
||||
|
||||
@@ -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 = {
|
||||
|
||||
@@ -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 };
|
||||
}
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
|
||||
@@ -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<string> {
|
||||
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 }
|
||||
: {}),
|
||||
|
||||
Reference in New Issue
Block a user