From 14da2951852386fb448412128aa2e5fa162ad3b2 Mon Sep 17 00:00:00 2001 From: agent_coder Date: Sat, 11 Jul 2026 03:25:56 +0300 Subject: [PATCH] =?UTF-8?q?fix(auth):=20=D0=BF=D1=80=D0=BE=D0=B2=D0=B5?= =?UTF-8?q?=D0=BD=D0=B0=D0=BD=D1=81=20=D0=B4=D0=BB=D1=8F=20API-key=20?= =?UTF-8?q?=D0=BF=D1=83=D1=82=D0=B8=20(#486)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit validateApiKey возвращал результат до резолва провенанса — REST-записи по is_agent API-ключу не получали маркер 'agent'. Перенести «выше» нельзя: payload API-ключа не несёт подписанный actor-клейм, а user (с isAgent) неизвестен до валидации ключа. Резолвим провенанс от возвращённого user: isAgent -> 'agent', иначе 'user'; aiChatId у API-ключа всегда null (нет ai_chats-строки). Загрузка EE ApiKeyService вынесена в переопределяемый шов resolveApiKeyService для юнит- теста без EE-бандла. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../core/auth/strategies/jwt.strategy.spec.ts | 99 +++++++++++++++++++ .../src/core/auth/strategies/jwt.strategy.ts | 51 +++++++--- 2 files changed, 135 insertions(+), 15 deletions(-) diff --git a/apps/server/src/core/auth/strategies/jwt.strategy.spec.ts b/apps/server/src/core/auth/strategies/jwt.strategy.spec.ts index 11544df7..20a669ca 100644 --- a/apps/server/src/core/auth/strategies/jwt.strategy.spec.ts +++ b/apps/server/src/core/auth/strategies/jwt.strategy.spec.ts @@ -120,3 +120,102 @@ describe('JwtStrategy — provenance derivation', () => { expect(req.raw.actor).toBeUndefined(); }); }); + +/** + * Provenance derivation on the API-KEY path (jwt.strategy.validateApiKey, #486). + * + * The access-token path stamped provenance; the API-key path returned early + * WITHOUT it, so an is_agent API key's REST writes recorded no 'agent' marker. + * The API-key payload carries no signed claim, so provenance is resolved from the + * SERVER-SIDE user returned by ApiKeyService.validateApiKey: isAgent -> 'agent', + * otherwise 'user'; aiChatId is always null (an API key has no ai_chats row). + * + * The enterprise ApiKeyService is not bundled in the OSS build, so the strategy + * loads it through an overridable `resolveApiKeyService` seam that we stub here. + */ +describe('JwtStrategy — API-key provenance derivation (#486)', () => { + function makeApiKeyStrategy(validateApiKeyImpl: (p: any) => Promise) { + const userRepo: any = { findById: jest.fn() }; + const workspaceRepo: any = { findById: jest.fn() }; + const userSessionRepo: any = { findActiveById: jest.fn() }; + const sessionActivityService: any = { trackActivity: jest.fn() }; + const environmentService: any = { getAppSecret: () => 'test-secret' }; + const moduleRef: any = {}; + + const strategy = new JwtStrategy( + userRepo, + workspaceRepo, + userSessionRepo, + sessionActivityService, + environmentService, + moduleRef, + ); + // Stub the EE ApiKeyService seam (the real module is not in the OSS build). + const validateApiKey = jest.fn(validateApiKeyImpl); + jest + .spyOn(strategy as any, 'resolveApiKeyService') + .mockReturnValue({ validateApiKey }); + return { strategy, validateApiKey }; + } + + const makeReq = () => ({ raw: {} as Record }); + const apiKeyPayload = () => ({ + sub: 'svc-1', + workspaceId: 'ws-1', + apiKeyId: 'key-1', + type: JwtType.API_KEY, + }); + + it("stamps actor='agent' for an is_agent API key (from the validated user)", async () => { + const validated = { + user: { id: 'svc-1', isAgent: true }, + workspace: { id: 'ws-1' }, + }; + const { strategy, validateApiKey } = makeApiKeyStrategy( + async () => validated, + ); + const req = makeReq(); + + const result = await strategy.validate(req, apiKeyPayload() as any); + + expect(validateApiKey).toHaveBeenCalledTimes(1); + expect(req.raw.actor).toBe('agent'); + // API keys carry no internal ai_chats row -> null. + expect(req.raw.aiChatId).toBeNull(); + // The validated auth object is returned unchanged (req.user shape preserved). + expect(result).toBe(validated); + }); + + it("stamps actor='user' for an ordinary (non-agent) API key", async () => { + const { strategy } = makeApiKeyStrategy(async () => ({ + user: { id: 'u-1', isAgent: false }, + workspace: { id: 'ws-1' }, + })); + const req = makeReq(); + + await strategy.validate(req, apiKeyPayload() as any); + + expect(req.raw.actor).toBe('user'); + expect(req.raw.aiChatId).toBeNull(); + }); + + it('throws Unauthorized (and stamps nothing) when the EE module is missing', async () => { + const userRepo: any = { findById: jest.fn() }; + const strategy = new JwtStrategy( + userRepo, + { findById: jest.fn() } as any, + { findActiveById: jest.fn() } as any, + { trackActivity: jest.fn() } as any, + { getAppSecret: () => 'test-secret' } as any, + {} as any, + ); + // EE not bundled: the seam returns null. + jest.spyOn(strategy as any, 'resolveApiKeyService').mockReturnValue(null); + const req = makeReq(); + + await expect( + strategy.validate(req, apiKeyPayload() as any), + ).rejects.toThrow(UnauthorizedException); + expect(req.raw.actor).toBeUndefined(); + }); +}); diff --git a/apps/server/src/core/auth/strategies/jwt.strategy.ts b/apps/server/src/core/auth/strategies/jwt.strategy.ts index 9e5604a9..2abd65c2 100644 --- a/apps/server/src/core/auth/strategies/jwt.strategy.ts +++ b/apps/server/src/core/auth/strategies/jwt.strategy.ts @@ -102,28 +102,49 @@ export class JwtStrategy extends PassportStrategy(Strategy, 'jwt') { } private async validateApiKey(req: any, payload: JwtApiKeyPayload) { - let ApiKeyModule: any; - let isApiKeyModuleReady = false; + const apiKeyService = this.resolveApiKeyService(); + if (!apiKeyService) { + throw new UnauthorizedException('Enterprise API Key module missing'); + } + const result = await apiKeyService.validateApiKey(payload); + + // Stamp the agent-edit provenance for the API-KEY path too (#486). Unlike the + // access-token path above, it CANNOT be resolved before this point: the + // API-key payload carries no signed actor/aiChatId claim, and the user (with + // its isAgent flag) is unknown until the key is validated. Claim semantics for + // API keys: an is_agent API key (an agent service account) stamps 'agent' on + // every REST write; an ordinary API key resolves to 'user'. An API key has no + // internal ai_chats row, so aiChatId is always null. Derived from the + // SERVER-SIDE user (never a client field), so an 'agent' badge is unspoofable + // — mirroring the access-token path. Passing `null` for the claim means the + // actor is decided solely by user.isAgent. + const provenance = resolveProvenance((result as any)?.user, null); + req.raw.actor = provenance.actor; + req.raw.aiChatId = provenance.aiChatId; + + return result; + } + + /** + * Resolve the enterprise ApiKeyService, or `null` when the EE module is not + * bundled in this build (community build). Extracted as an overridable seam so + * the API-key provenance stamping can be unit-tested without the EE package + * present (docmost is OSS + a separate EE bundle; `require` of the EE path + * throws here). Any load/resolve error is treated as "module missing". + */ + protected resolveApiKeyService(): { + validateApiKey: (payload: JwtApiKeyPayload) => Promise; + } | null { try { // eslint-disable-next-line @typescript-eslint/no-require-imports - ApiKeyModule = require('./../../../ee/api-key/api-key.service'); - isApiKeyModuleReady = true; + const ApiKeyModule = require('./../../../ee/api-key/api-key.service'); + return this.moduleRef.get(ApiKeyModule.ApiKeyService, { strict: false }); } catch (err) { this.logger.debug( 'API Key module requested but enterprise module not bundled in this build', ); - isApiKeyModuleReady = false; + return null; } - - if (isApiKeyModuleReady) { - const ApiKeyService = this.moduleRef.get(ApiKeyModule.ApiKeyService, { - strict: false, - }); - - return ApiKeyService.validateApiKey(payload); - } - - throw new UnauthorizedException('Enterprise API Key module missing'); } }