feat(api-key): фаза 3A — снять kill-switch API_KEYS_ENABLED + копируемый ключ (reveal под step-up)
Removes the API_KEYS_ENABLED kill-switch entirely and makes api-keys copyable via a deterministic re-mint revealed under a password step-up (#557, epic #556). Part 1 — remove the kill-switch (keys are now always on): - environment.service.ts: delete isApiKeysEnabled()/getApiKeysEnabledRaw(). - environment.validation.ts: delete the API_KEYS_ENABLED field + decorators (validation has no forbidNonWhitelisted, so a stray env value is ignored). - api-key.service.ts: drop OnModuleInit boot-log, the validate kill-switch branch, and the now-unused EnvironmentService injection. - api-key.controller.ts: drop assertEnabled() + its call sites and the EnvironmentService injection. - .env.example: delete the API_KEYS_ENABLED block. Part 2 — deterministic mint: apiKeyJwtService gets noTimestamp:true, so the api-key token carries neither exp nor iat and re-minting the same key is byte-identical (basis of copyable reveal). Part 3 — POST /api-keys/reveal { id, password }: JwtAuthGuard + rejectApiKeyPrincipal (403) + AUTH throttler; step-up via AuthService.verifyUserCredentials BEFORE any key lookup (401, no oracle); owner-only even for admin; uniform 404 for absent/revoked/expired/other-owner/ disabled-creator (ForbiddenException from generateApiToken normalized to 404); re-mint via the existing deterministic path; token never persisted; durable AuditEvent.API_KEY_REVEALED + structured log without token material. Part 4 — UI: replace the show-once modal with a per-row Copy action -> password prompt -> reveal -> clipboard write + toast. The token never enters React state, localStorage or the query cache (gcTime:0 + reset-after-read, mirroring #506). i18n en+ru added; immediate-revoke behavior kept. Tests: server api-key + token.service specs (reveal, deterministic no-exp/no-iat, byte-identical, kill-switch-removal) and client api-key vitest (copy flow, no-leak, wrong-password) all green. Mutation-verified: skipping the owner check reds the non-owner-404 test, skipping step-up reds the wrong-password test, dropping noTimestamp reds the deterministic/no-iat tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -22,6 +22,9 @@ export const AuditEvent = {
|
||||
API_KEY_CREATED: 'api_key.created',
|
||||
API_KEY_UPDATED: 'api_key.updated',
|
||||
API_KEY_DELETED: 'api_key.deleted',
|
||||
// A copyable key was re-minted and returned to its owner under a password
|
||||
// step-up (see ApiKeyController.reveal). Durable via DatabaseAuditService.
|
||||
API_KEY_REVEALED: 'api_key.revealed',
|
||||
|
||||
// SCIM Tokens
|
||||
SCIM_TOKEN_CREATED: 'scim_token.created',
|
||||
|
||||
@@ -1,4 +1,8 @@
|
||||
import { ForbiddenException, NotFoundException } from '@nestjs/common';
|
||||
import {
|
||||
ForbiddenException,
|
||||
NotFoundException,
|
||||
UnauthorizedException,
|
||||
} from '@nestjs/common';
|
||||
import { ApiKeyController } from './api-key.controller';
|
||||
import {
|
||||
WorkspaceCaslAction,
|
||||
@@ -12,7 +16,8 @@ import {
|
||||
* — GitHub-PAT semantics closing post-revocation laundering.
|
||||
* - admin (CASL Manage on API) sees/revokes all workspace keys; a member only
|
||||
* their own.
|
||||
* - kill-switch OFF -> the surface 404s (looks like the feature does not exist).
|
||||
* - reveal: password step-up (401 on wrong password), owner-only even for admin
|
||||
* (404), api_key principal 403 — the copyable-key surface (#557).
|
||||
*/
|
||||
|
||||
function makeController(over: any = {}) {
|
||||
@@ -22,6 +27,7 @@ function makeController(over: any = {}) {
|
||||
key: { id: 'k1', name: 'n', expiresAt: null, createdAt: new Date() },
|
||||
}),
|
||||
revoke: jest.fn().mockResolvedValue(undefined),
|
||||
reveal: jest.fn().mockResolvedValue('remint.tok'),
|
||||
...(over.apiKeyService ?? {}),
|
||||
};
|
||||
const apiKeyRepo = {
|
||||
@@ -36,16 +42,16 @@ function makeController(over: any = {}) {
|
||||
}),
|
||||
...(over.workspaceAbility ?? {}),
|
||||
};
|
||||
const environmentService = {
|
||||
isApiKeysEnabled: jest.fn().mockReturnValue(over.enabled ?? true),
|
||||
...(over.environmentService ?? {}),
|
||||
const authService = {
|
||||
verifyUserCredentials: jest.fn().mockResolvedValue({ id: 'u-1' }),
|
||||
...(over.authService ?? {}),
|
||||
};
|
||||
const auditService = { log: jest.fn() };
|
||||
const controller = new ApiKeyController(
|
||||
apiKeyService as any,
|
||||
apiKeyRepo as any,
|
||||
workspaceAbility as any,
|
||||
environmentService as any,
|
||||
authService as any,
|
||||
auditService as any,
|
||||
);
|
||||
return {
|
||||
@@ -53,12 +59,12 @@ function makeController(over: any = {}) {
|
||||
apiKeyService,
|
||||
apiKeyRepo,
|
||||
workspaceAbility,
|
||||
environmentService,
|
||||
authService,
|
||||
auditService,
|
||||
};
|
||||
}
|
||||
|
||||
const user = { id: 'u-1', workspaceId: 'ws-1' } as any;
|
||||
const user = { id: 'u-1', email: 'u@x.io', workspaceId: 'ws-1' } as any;
|
||||
const workspace = { id: 'ws-1' } as any;
|
||||
const reqAccess = () => ({ raw: {}, ip: '1.2.3.4', socket: {} }) as any;
|
||||
const reqApiKey = () =>
|
||||
@@ -86,20 +92,86 @@ describe('ApiKeyController — a token cannot manage tokens', () => {
|
||||
controller.revoke({ id: 'k1' } as any, user, workspace, reqApiKey()),
|
||||
).rejects.toBeInstanceOf(ForbiddenException);
|
||||
});
|
||||
|
||||
it('403 on reveal for an api_key principal (a key cannot reveal its siblings)', async () => {
|
||||
const { controller, authService, apiKeyService } = makeController();
|
||||
await expect(
|
||||
controller.reveal(
|
||||
{ id: 'k1', password: 'pw' } as any,
|
||||
user,
|
||||
workspace,
|
||||
reqApiKey(),
|
||||
),
|
||||
).rejects.toBeInstanceOf(ForbiddenException);
|
||||
// Rejected BEFORE the step-up and the key lookup even run.
|
||||
expect(authService.verifyUserCredentials).not.toHaveBeenCalled();
|
||||
expect(apiKeyService.reveal).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('ApiKeyController — kill-switch OFF → 404', () => {
|
||||
it('create/list/revoke all 404 when disabled', async () => {
|
||||
const { controller } = makeController({ enabled: false });
|
||||
describe('ApiKeyController — reveal (copyable key under step-up)', () => {
|
||||
it('re-mints and returns the token + logs API_KEY_REVEALED on success', async () => {
|
||||
const { controller, apiKeyService, authService, auditService } =
|
||||
makeController();
|
||||
const res = await controller.reveal(
|
||||
{ id: 'k1', password: 'correct-horse' } as any,
|
||||
user,
|
||||
workspace,
|
||||
reqAccess(),
|
||||
);
|
||||
|
||||
expect(res).toEqual({ token: 'remint.tok' });
|
||||
// Step-up ran with the caller's email + supplied password.
|
||||
expect(authService.verifyUserCredentials).toHaveBeenCalledWith(
|
||||
{ email: 'u@x.io', password: 'correct-horse' },
|
||||
'ws-1',
|
||||
);
|
||||
expect(apiKeyService.reveal).toHaveBeenCalledWith({
|
||||
apiKeyId: 'k1',
|
||||
user,
|
||||
workspaceId: 'ws-1',
|
||||
});
|
||||
expect(auditService.log).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ event: 'api_key.revealed' }),
|
||||
);
|
||||
});
|
||||
|
||||
it('401 on a wrong password — step-up runs BEFORE the key lookup (no oracle)', async () => {
|
||||
const { controller, apiKeyService } = makeController({
|
||||
authService: {
|
||||
verifyUserCredentials: jest
|
||||
.fn()
|
||||
.mockRejectedValue(new UnauthorizedException()),
|
||||
},
|
||||
});
|
||||
await expect(
|
||||
controller.create({ name: 'x' } as any, user, reqAccess()),
|
||||
).rejects.toBeInstanceOf(NotFoundException);
|
||||
controller.reveal(
|
||||
{ id: 'k1', password: 'wrong' } as any,
|
||||
user,
|
||||
workspace,
|
||||
reqAccess(),
|
||||
),
|
||||
).rejects.toBeInstanceOf(UnauthorizedException);
|
||||
// The key is never looked up / re-minted when step-up fails.
|
||||
expect(apiKeyService.reveal).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("propagates the service's uniform 404 (absent/revoked/expired/non-owner)", async () => {
|
||||
const { controller, auditService } = makeController({
|
||||
apiKeyService: {
|
||||
reveal: jest.fn().mockRejectedValue(new NotFoundException()),
|
||||
},
|
||||
});
|
||||
await expect(
|
||||
controller.list(user, workspace, reqAccess()),
|
||||
).rejects.toBeInstanceOf(NotFoundException);
|
||||
await expect(
|
||||
controller.revoke({ id: 'k1' } as any, user, workspace, reqAccess()),
|
||||
controller.reveal(
|
||||
{ id: 'k1', password: 'correct' } as any,
|
||||
user,
|
||||
workspace,
|
||||
reqAccess(),
|
||||
),
|
||||
).rejects.toBeInstanceOf(NotFoundException);
|
||||
// No audit event on a failed reveal.
|
||||
expect(auditService.log).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -2,6 +2,7 @@ import {
|
||||
Body,
|
||||
Controller,
|
||||
ForbiddenException,
|
||||
forwardRef,
|
||||
HttpCode,
|
||||
HttpStatus,
|
||||
Inject,
|
||||
@@ -17,6 +18,7 @@ import { ApiKeyService } from './api-key.service';
|
||||
import { ApiKeyRepo } from '@docmost/db/repos/api-key/api-key.repo';
|
||||
import { CreateApiKeyDto } from './dto/create-api-key.dto';
|
||||
import { RevokeApiKeyDto } from './dto/revoke-api-key.dto';
|
||||
import { RevealApiKeyDto } from './dto/reveal-api-key.dto';
|
||||
import { AuthUser } from '../../common/decorators/auth-user.decorator';
|
||||
import { AuthWorkspace } from '../../common/decorators/auth-workspace.decorator';
|
||||
import { JwtAuthGuard } from '../../common/guards/jwt-auth.guard';
|
||||
@@ -31,7 +33,7 @@ import {
|
||||
AUDIT_SERVICE,
|
||||
IAuditService,
|
||||
} from '../../integrations/audit/audit.service';
|
||||
import { EnvironmentService } from '../../integrations/environment/environment.service';
|
||||
import { AuthService } from '../auth/services/auth.service';
|
||||
import { UserThrottlerGuard } from '../../integrations/throttle/user-throttler.guard';
|
||||
import { AUTH_THROTTLER } from '../../integrations/throttle/throttler-names';
|
||||
|
||||
@@ -44,18 +46,13 @@ export class ApiKeyController {
|
||||
private readonly apiKeyService: ApiKeyService,
|
||||
private readonly apiKeyRepo: ApiKeyRepo,
|
||||
private readonly workspaceAbility: WorkspaceAbilityFactory,
|
||||
private readonly environmentService: EnvironmentService,
|
||||
// forwardRef breaks the AuthModule <-> ApiKeyModule cycle (AuthModule imports
|
||||
// ApiKeyModule for JwtStrategy). Only used for the reveal step-up.
|
||||
@Inject(forwardRef(() => AuthService))
|
||||
private readonly authService: AuthService,
|
||||
@Inject(AUDIT_SERVICE) private readonly auditService: IAuditService,
|
||||
) {}
|
||||
|
||||
// Kill-switch OFF: the issuance surface disappears entirely (404), not a 403 —
|
||||
// it must look like the feature does not exist.
|
||||
private assertEnabled(): void {
|
||||
if (!this.environmentService.isApiKeysEnabled()) {
|
||||
throw new NotFoundException();
|
||||
}
|
||||
}
|
||||
|
||||
// A token cannot manage tokens (GitHub-PAT semantics): an api-key principal is
|
||||
// refused on the management surface, so a leaked key cannot mint a replacement
|
||||
// or revoke the keys that would lock it out (closes post-revocation laundering).
|
||||
@@ -78,7 +75,6 @@ export class ApiKeyController {
|
||||
@AuthUser() user: User,
|
||||
@Req() req: FastifyRequest,
|
||||
) {
|
||||
this.assertEnabled();
|
||||
this.rejectApiKeyPrincipal(req);
|
||||
|
||||
// undefined -> service default (1 year); null -> unlimited; string -> Date.
|
||||
@@ -100,8 +96,9 @@ export class ApiKeyController {
|
||||
resourceType: AuditResource.API_KEY,
|
||||
resourceId: key.id,
|
||||
});
|
||||
// The durable trail lives in container logs (AUDIT_SERVICE is a Noop in this
|
||||
// build). No token material — the JWT is only ever returned in the response.
|
||||
// Durable audit is via DatabaseAuditService (#496); this structured log is a
|
||||
// second, container-log trail. No token material — the JWT is only ever
|
||||
// returned in the response.
|
||||
this.logger.log(
|
||||
`API key created: id=${key.id} name=${JSON.stringify(
|
||||
key.name,
|
||||
@@ -131,7 +128,6 @@ export class ApiKeyController {
|
||||
@AuthWorkspace() workspace: Workspace,
|
||||
@Req() req: FastifyRequest,
|
||||
) {
|
||||
this.assertEnabled();
|
||||
this.rejectApiKeyPrincipal(req);
|
||||
|
||||
const ability = this.workspaceAbility.createForUser(user, workspace);
|
||||
@@ -163,7 +159,6 @@ export class ApiKeyController {
|
||||
@AuthWorkspace() workspace: Workspace,
|
||||
@Req() req: FastifyRequest,
|
||||
) {
|
||||
this.assertEnabled();
|
||||
this.rejectApiKeyPrincipal(req);
|
||||
|
||||
const key = await this.apiKeyRepo.findById(dto.id, workspace.id);
|
||||
@@ -198,4 +193,64 @@ export class ApiKeyController {
|
||||
|
||||
return { success: true };
|
||||
}
|
||||
|
||||
/**
|
||||
* Reveal (re-mint) a copyable token for an existing key, under a password
|
||||
* step-up. The token is hashed nowhere and stored nowhere — "reveal" re-mints
|
||||
* the SAME deterministic value (see TokenService.generateApiToken /
|
||||
* ApiKeyService.reveal) and returns it in the body only.
|
||||
*
|
||||
* Defence layers (triple, against exfiltration through a hijacked session):
|
||||
* - rejectApiKeyPrincipal: an api_key Bearer caller is 403 — a leaked key must
|
||||
* NOT be able to reveal its siblings (key laundering); a token doesn't manage
|
||||
* tokens.
|
||||
* - step-up: the password is re-verified (verifyUserCredentials) BEFORE any key
|
||||
* lookup, so a wrong/absent password is a uniform 401 that leaks nothing about
|
||||
* the key. SSO/MFA-only users (no local password) are out of scope (homelab).
|
||||
* - AUTH throttler: same limiter as create.
|
||||
*
|
||||
* Owner-only, EVEN for an admin, and every key-state negative (absent / revoked /
|
||||
* expired / another user's / creator disabled) is a UNIFORM 404 (see
|
||||
* ApiKeyService.reveal) — no existence/state oracle.
|
||||
*/
|
||||
@HttpCode(HttpStatus.OK)
|
||||
@UseGuards(JwtAuthGuard, UserThrottlerGuard)
|
||||
@Throttle({ [AUTH_THROTTLER]: { limit: 10, ttl: 60_000 } })
|
||||
@Post('reveal')
|
||||
async reveal(
|
||||
@Body() dto: RevealApiKeyDto,
|
||||
@AuthUser() user: User,
|
||||
@AuthWorkspace() workspace: Workspace,
|
||||
@Req() req: FastifyRequest,
|
||||
) {
|
||||
this.rejectApiKeyPrincipal(req);
|
||||
|
||||
// Step-up FIRST — before any key lookup — so the 401 for a bad password never
|
||||
// depends on (and never leaks) the key's existence/state. Reuses the exact
|
||||
// timing-safe, SSO-aware credential check used by login (throws a uniform
|
||||
// 401 on wrong/absent/SSO-only password).
|
||||
await this.authService.verifyUserCredentials(
|
||||
{ email: user.email, password: dto.password },
|
||||
workspace.id,
|
||||
);
|
||||
|
||||
const token = await this.apiKeyService.reveal({
|
||||
apiKeyId: dto.id,
|
||||
user,
|
||||
workspaceId: workspace.id,
|
||||
});
|
||||
|
||||
// Durable audit (DatabaseAuditService, #496) + a structured container-log
|
||||
// trail. NEVER the token material.
|
||||
this.auditService.log({
|
||||
event: AuditEvent.API_KEY_REVEALED,
|
||||
resourceType: AuditResource.API_KEY,
|
||||
resourceId: dto.id,
|
||||
});
|
||||
this.logger.log(
|
||||
`API key revealed: id=${dto.id} actor=${user.id} ip=${this.clientIp(req)}`,
|
||||
);
|
||||
|
||||
return { token };
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,7 +1,8 @@
|
||||
import { Module } from '@nestjs/common';
|
||||
import { forwardRef, Module } from '@nestjs/common';
|
||||
import { ApiKeyService } from './api-key.service';
|
||||
import { ApiKeyController } from './api-key.controller';
|
||||
import { TokenModule } from '../auth/token.module';
|
||||
import { AuthModule } from '../auth/auth.module';
|
||||
|
||||
// Core (non-EE) API-key feature: issuance REST endpoints + the shared validator
|
||||
// consumed by jwt.strategy (REST) and McpService (the /mcp Bearer router).
|
||||
@@ -11,7 +12,10 @@ import { TokenModule } from '../auth/token.module';
|
||||
// jwt.strategy) and McpModule (for the /mcp router) can inject it directly,
|
||||
// replacing the absent EE `ee/api-key` dynamic require.
|
||||
@Module({
|
||||
imports: [TokenModule],
|
||||
// forwardRef(AuthModule): the reveal endpoint's password step-up uses
|
||||
// AuthService.verifyUserCredentials. AuthModule already imports ApiKeyModule
|
||||
// (for JwtStrategy), so the two form a cycle that forwardRef resolves.
|
||||
imports: [TokenModule, forwardRef(() => AuthModule)],
|
||||
controllers: [ApiKeyController],
|
||||
providers: [ApiKeyService],
|
||||
exports: [ApiKeyService],
|
||||
|
||||
@@ -1,4 +1,8 @@
|
||||
import { UnauthorizedException } from '@nestjs/common';
|
||||
import {
|
||||
ForbiddenException,
|
||||
NotFoundException,
|
||||
UnauthorizedException,
|
||||
} from '@nestjs/common';
|
||||
import { ApiKeyService } from './api-key.service';
|
||||
import { JwtType } from '../auth/dto/jwt-payload';
|
||||
|
||||
@@ -16,8 +20,6 @@ import { JwtType } from '../auth/dto/jwt-payload';
|
||||
* - No validate cache (each call re-reads the row → immediate revocation).
|
||||
*/
|
||||
|
||||
const APP_SECRET = 'secret';
|
||||
|
||||
function makeDeps(over: Partial<Record<string, any>> = {}) {
|
||||
const apiKeyRepo = {
|
||||
findById: jest.fn(),
|
||||
@@ -38,20 +40,13 @@ function makeDeps(over: Partial<Record<string, any>> = {}) {
|
||||
generateApiToken: jest.fn().mockResolvedValue('minted.jwt.token'),
|
||||
...(over.tokenService ?? {}),
|
||||
};
|
||||
const environmentService = {
|
||||
isApiKeysEnabled: jest.fn().mockReturnValue(true),
|
||||
getApiKeysEnabledRaw: jest.fn().mockReturnValue(undefined),
|
||||
getAppSecret: jest.fn().mockReturnValue(APP_SECRET),
|
||||
...(over.environmentService ?? {}),
|
||||
};
|
||||
const service = new (ApiKeyService as unknown as new (...a: any[]) => ApiKeyService)(
|
||||
apiKeyRepo,
|
||||
userRepo,
|
||||
workspaceRepo,
|
||||
tokenService,
|
||||
environmentService,
|
||||
);
|
||||
return { service, apiKeyRepo, userRepo, workspaceRepo, tokenService, environmentService };
|
||||
return { service, apiKeyRepo, userRepo, workspaceRepo, tokenService };
|
||||
}
|
||||
|
||||
const payload = (over: Record<string, any> = {}) => ({
|
||||
@@ -148,10 +143,6 @@ describe('ApiKeyService.validate', () => {
|
||||
d.workspaceRepo.findById.mockResolvedValue(undefined);
|
||||
},
|
||||
],
|
||||
[
|
||||
'kill-switch OFF',
|
||||
(d) => d.environmentService.isApiKeysEnabled.mockReturnValue(false),
|
||||
],
|
||||
[
|
||||
'malformed payload (missing apiKeyId)',
|
||||
() => undefined,
|
||||
@@ -175,14 +166,16 @@ describe('ApiKeyService.validate', () => {
|
||||
},
|
||||
);
|
||||
|
||||
it('kill-switch OFF denies BEFORE any DB read', async () => {
|
||||
const { service, apiKeyRepo, environmentService } = makeDeps();
|
||||
environmentService.isApiKeysEnabled.mockReturnValue(false);
|
||||
// --- Kill-switch removed: keys work with no env variable ------------------
|
||||
it('validates with no API_KEYS_ENABLED env (kill-switch fully removed)', async () => {
|
||||
// The service no longer takes EnvironmentService — there is no env gate left.
|
||||
const { service, apiKeyRepo, userRepo } = makeDeps();
|
||||
apiKeyRepo.findById.mockResolvedValue(activeRow());
|
||||
userRepo.findById.mockResolvedValue(activeUser());
|
||||
|
||||
await expect(service.validate(payload() as any)).rejects.toBeInstanceOf(
|
||||
UnauthorizedException,
|
||||
);
|
||||
expect(apiKeyRepo.findById).not.toHaveBeenCalled();
|
||||
await expect(service.validate(payload() as any)).resolves.toMatchObject({
|
||||
user: { id: 'u-1' },
|
||||
});
|
||||
});
|
||||
|
||||
// --- Infra error propagates (5xx), NOT masked as 401 ---------------------
|
||||
@@ -297,3 +290,90 @@ describe('ApiKeyService.create (mint-then-insert, no exp)', () => {
|
||||
expect(apiKeyRepo.insert).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* ApiKeyService.reveal — the KEY-STATE gate for the copyable-key flow. Owner-only
|
||||
* (even for an admin), and EVERY negative is a uniform NotFoundException so there
|
||||
* is no existence/state oracle. Re-mint is deterministic (delegated to
|
||||
* TokenService); here we only assert the gate + normalization.
|
||||
*/
|
||||
describe('ApiKeyService.reveal', () => {
|
||||
const caller = (over: Record<string, any> = {}) =>
|
||||
({ id: 'u-1', email: 'u@x.io', workspaceId: 'ws-1', ...over }) as any;
|
||||
|
||||
it('re-mints and returns the token for a live key owned by the caller', async () => {
|
||||
const { service, apiKeyRepo, tokenService } = makeDeps();
|
||||
apiKeyRepo.findById.mockResolvedValue(activeRow());
|
||||
tokenService.generateApiToken.mockResolvedValue('remint.tok');
|
||||
|
||||
const token = await service.reveal({
|
||||
apiKeyId: 'key-1',
|
||||
user: caller(),
|
||||
workspaceId: 'ws-1',
|
||||
});
|
||||
|
||||
expect(token).toBe('remint.tok');
|
||||
// Re-mints against the SAME id + the caller (== creator) so the value matches.
|
||||
expect(tokenService.generateApiToken).toHaveBeenCalledWith({
|
||||
apiKeyId: 'key-1',
|
||||
user: expect.objectContaining({ id: 'u-1' }),
|
||||
workspaceId: 'ws-1',
|
||||
});
|
||||
});
|
||||
|
||||
it('404 for a missing/revoked key (findById returns undefined)', async () => {
|
||||
const { service, apiKeyRepo, tokenService } = makeDeps();
|
||||
apiKeyRepo.findById.mockResolvedValue(undefined);
|
||||
|
||||
await expect(
|
||||
service.reveal({ apiKeyId: 'key-1', user: caller(), workspaceId: 'ws-1' }),
|
||||
).rejects.toBeInstanceOf(NotFoundException);
|
||||
// No re-mint on any negative.
|
||||
expect(tokenService.generateApiToken).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("404 for another user's key — owner-only, even for an admin", async () => {
|
||||
const { service, apiKeyRepo, tokenService } = makeDeps();
|
||||
// The caller is a workspace admin (irrelevant here — reveal ignores CASL).
|
||||
apiKeyRepo.findById.mockResolvedValue(activeRow({ creatorId: 'someone-else' }));
|
||||
|
||||
await expect(
|
||||
service.reveal({ apiKeyId: 'key-1', user: caller(), workspaceId: 'ws-1' }),
|
||||
).rejects.toBeInstanceOf(NotFoundException);
|
||||
expect(tokenService.generateApiToken).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('404 for an expired key (expiry read from the row) — no dead token issued', async () => {
|
||||
const { service, apiKeyRepo, tokenService } = makeDeps();
|
||||
apiKeyRepo.findById.mockResolvedValue(
|
||||
activeRow({ expiresAt: new Date(Date.now() - 1000) }),
|
||||
);
|
||||
|
||||
await expect(
|
||||
service.reveal({ apiKeyId: 'key-1', user: caller(), workspaceId: 'ws-1' }),
|
||||
).rejects.toBeInstanceOf(NotFoundException);
|
||||
expect(tokenService.generateApiToken).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('normalizes a disabled-creator ForbiddenException (403) to a uniform 404', async () => {
|
||||
const { service, apiKeyRepo, tokenService } = makeDeps();
|
||||
apiKeyRepo.findById.mockResolvedValue(activeRow());
|
||||
// generateApiToken throws ForbiddenException when isUserDisabled(user).
|
||||
tokenService.generateApiToken.mockRejectedValue(new ForbiddenException());
|
||||
|
||||
await expect(
|
||||
service.reveal({ apiKeyId: 'key-1', user: caller(), workspaceId: 'ws-1' }),
|
||||
).rejects.toBeInstanceOf(NotFoundException);
|
||||
});
|
||||
|
||||
it('propagates an UNEXPECTED re-mint error (not masked as 404)', async () => {
|
||||
const { service, apiKeyRepo, tokenService } = makeDeps();
|
||||
apiKeyRepo.findById.mockResolvedValue(activeRow());
|
||||
const boom = new Error('signer exploded');
|
||||
tokenService.generateApiToken.mockRejectedValue(boom);
|
||||
|
||||
await expect(
|
||||
service.reveal({ apiKeyId: 'key-1', user: caller(), workspaceId: 'ws-1' }),
|
||||
).rejects.toBe(boom);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,7 +1,8 @@
|
||||
import {
|
||||
ForbiddenException,
|
||||
Injectable,
|
||||
Logger,
|
||||
OnModuleInit,
|
||||
NotFoundException,
|
||||
UnauthorizedException,
|
||||
} from '@nestjs/common';
|
||||
import { v7 as uuid7 } from 'uuid';
|
||||
@@ -9,7 +10,6 @@ import { ApiKeyRepo } from '@docmost/db/repos/api-key/api-key.repo';
|
||||
import { UserRepo } from '@docmost/db/repos/user/user.repo';
|
||||
import { WorkspaceRepo } from '@docmost/db/repos/workspace/workspace.repo';
|
||||
import { TokenService } from '../auth/services/token.service';
|
||||
import { EnvironmentService } from '../../integrations/environment/environment.service';
|
||||
import { JwtApiKeyPayload } from '../auth/dto/jwt-payload';
|
||||
import { ApiKey, User, Workspace } from '@docmost/db/types/entity.types';
|
||||
import { isUserDisabled } from '../../common/helpers';
|
||||
@@ -31,7 +31,7 @@ const LAST_USED_THROTTLE_MS = 60 * 60 * 1000;
|
||||
* key's lifetime and revocation — never the JWT, which carries no `exp` claim.
|
||||
*/
|
||||
@Injectable()
|
||||
export class ApiKeyService implements OnModuleInit {
|
||||
export class ApiKeyService {
|
||||
private readonly logger = new Logger(ApiKeyService.name);
|
||||
|
||||
constructor(
|
||||
@@ -39,20 +39,8 @@ export class ApiKeyService implements OnModuleInit {
|
||||
private readonly userRepo: UserRepo,
|
||||
private readonly workspaceRepo: WorkspaceRepo,
|
||||
private readonly tokenService: TokenService,
|
||||
private readonly environmentService: EnvironmentService,
|
||||
) {}
|
||||
|
||||
onModuleInit() {
|
||||
// Boot log so the kill-switch state after each deploy is verifiable in logs.
|
||||
const enabled = this.environmentService.isApiKeysEnabled();
|
||||
const raw = this.environmentService.getApiKeysEnabledRaw();
|
||||
this.logger.log(
|
||||
`API keys: ${enabled ? 'ENABLED' : 'DISABLED'} (API_KEYS_ENABLED=${
|
||||
raw ?? 'unset'
|
||||
})`,
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Mint a new key for `user`. mint-then-insert ordering (R1):
|
||||
* 1. generate the id first — it must be in the JWT payload before the row.
|
||||
@@ -101,8 +89,8 @@ export class ApiKeyService implements OnModuleInit {
|
||||
* path returns) so the AuthUser/AuthWorkspace decorators and MCP identity work
|
||||
* unchanged.
|
||||
*
|
||||
* Failure semantics (R4, anti-enumeration): a DEFINITE negative fact — feature
|
||||
* disabled, missing/revoked/expired row, workspace mismatch, disabled user —
|
||||
* Failure semantics (R4, anti-enumeration): a DEFINITE negative fact —
|
||||
* missing/revoked/expired row, workspace mismatch, disabled user —
|
||||
* throws a bare `UnauthorizedException` (a single generic 401 for every case;
|
||||
* an agent cannot distinguish expired from revoked, and its reaction is
|
||||
* identical). An UNEXPECTED (infra) error is NOT caught here: it propagates so
|
||||
@@ -114,12 +102,6 @@ export class ApiKeyService implements OnModuleInit {
|
||||
async validate(
|
||||
payload: JwtApiKeyPayload,
|
||||
): Promise<{ user: User; workspace: Workspace }> {
|
||||
// Kill-switch OFF: deny unconditionally (same generic 401). Note the
|
||||
// endpoints additionally 404 at the controller; here we deny the token.
|
||||
if (!this.environmentService.isApiKeysEnabled()) {
|
||||
throw new UnauthorizedException();
|
||||
}
|
||||
|
||||
if (!payload?.apiKeyId || !payload?.sub || !payload?.workspaceId) {
|
||||
throw new UnauthorizedException();
|
||||
}
|
||||
@@ -174,6 +156,67 @@ export class ApiKeyService implements OnModuleInit {
|
||||
await this.apiKeyRepo.softDelete(id, workspaceId);
|
||||
}
|
||||
|
||||
/**
|
||||
* Re-mint (reveal) the token for an EXISTING key so its owner can copy it again.
|
||||
* The token material is never stored, so "reveal" = re-generate the same value:
|
||||
* generateApiToken is deterministic (no `iat`/`exp`, fixed payload), so the
|
||||
* re-minted token is byte-identical to the original for the same key.
|
||||
*
|
||||
* Authorization/step-up (principal rejection + password) is the caller's job
|
||||
* (the controller). This method owns the KEY-STATE gate and the re-mint. Every
|
||||
* negative is a UNIFORM `NotFoundException` — there is NO existence/state oracle:
|
||||
* a caller cannot tell "absent" from "revoked" from "expired" from "another
|
||||
* user's key" from "creator disabled". Only a live key owned by `user` re-mints.
|
||||
*
|
||||
* `user` is the authenticated caller AND (after the owner check) the key's
|
||||
* creator, so the re-minted token's `sub` matches the original mint — passing
|
||||
* `user` directly avoids re-fetching the creator row.
|
||||
*/
|
||||
async reveal(opts: {
|
||||
apiKeyId: string;
|
||||
user: User;
|
||||
workspaceId: string;
|
||||
}): Promise<string> {
|
||||
const { apiKeyId, user, workspaceId } = opts;
|
||||
|
||||
const row = await this.apiKeyRepo.findById(apiKeyId, workspaceId);
|
||||
// Absent = revoked (soft-deleted), orphaned, or never existed -> uniform 404.
|
||||
if (!row) {
|
||||
throw new NotFoundException();
|
||||
}
|
||||
|
||||
// Owner-only, EVEN for an admin: a working revealed token is an impersonation
|
||||
// of the creator, so unlike list/revoke, admin must NOT reveal others' keys.
|
||||
// Someone else's key looks exactly like a missing one -> uniform 404.
|
||||
if (row.creatorId !== user.id) {
|
||||
throw new NotFoundException();
|
||||
}
|
||||
|
||||
// Expiry is read from the ROW (findById filters only deletedAt, so an expired
|
||||
// row is still returned). A dead key must not hand out a working token, and a
|
||||
// uniform 404 keeps the anti-enumeration property.
|
||||
if (row.expiresAt && row.expiresAt.getTime() <= Date.now()) {
|
||||
throw new NotFoundException();
|
||||
}
|
||||
|
||||
try {
|
||||
// Deterministic re-mint (byte-identical to the original for this key).
|
||||
return await this.tokenService.generateApiToken({
|
||||
apiKeyId: row.id,
|
||||
user,
|
||||
workspaceId,
|
||||
});
|
||||
} catch (err) {
|
||||
// A disabled creator makes generateApiToken throw ForbiddenException (403).
|
||||
// Normalize to the SAME 404 so reveal never leaks "user is blocked" vs "key
|
||||
// does not exist". Unexpected errors propagate (5xx), never masked.
|
||||
if (err instanceof ForbiddenException) {
|
||||
throw new NotFoundException();
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
||||
// Throttled best-effort last_used_at bump: skip if it was touched within the
|
||||
// window; otherwise fire-and-forget so a stamp write never fails or slows the
|
||||
// request (mirrors SessionActivityService.trackActivity).
|
||||
|
||||
@@ -0,0 +1,14 @@
|
||||
import { IsNotEmpty, IsString, IsUUID } from 'class-validator';
|
||||
|
||||
// Step-up payload for `POST /api-keys/reveal`: the key to re-mint + the caller's
|
||||
// current password. The password is re-verified server-side (AuthService.
|
||||
// verifyUserCredentials) before the key is ever looked up, so a copyable,
|
||||
// non-expiring token cannot be exfiltrated through a merely-hijacked session.
|
||||
export class RevealApiKeyDto {
|
||||
@IsUUID()
|
||||
id: string;
|
||||
|
||||
@IsString()
|
||||
@IsNotEmpty()
|
||||
password: string;
|
||||
}
|
||||
@@ -1,4 +1,4 @@
|
||||
import { Module } from '@nestjs/common';
|
||||
import { forwardRef, Module } from '@nestjs/common';
|
||||
import { AuthController } from './auth.controller';
|
||||
import { AuthService } from './services/auth.service';
|
||||
import { JwtStrategy } from './strategies/jwt.strategy';
|
||||
@@ -10,8 +10,9 @@ import { ApiKeyModule } from '../api-key/api-key.module';
|
||||
@Module({
|
||||
// ApiKeyModule supplies ApiKeyService, injected into JwtStrategy so an
|
||||
// api_key Bearer/cookie token is validated directly (replacing the absent EE
|
||||
// `ee/api-key` dynamic require).
|
||||
imports: [TokenModule, WorkspaceModule, ApiKeyModule],
|
||||
// `ee/api-key` dynamic require). forwardRef: ApiKeyModule imports AuthModule
|
||||
// back (for the reveal step-up), so the two form a cycle.
|
||||
imports: [TokenModule, WorkspaceModule, forwardRef(() => ApiKeyModule)],
|
||||
controllers: [AuthController],
|
||||
providers: [AuthService, SignupService, JwtStrategy],
|
||||
exports: [SignupService, AuthService],
|
||||
|
||||
@@ -306,6 +306,41 @@ describe('TokenService.generateApiToken (no exp claim ever)', () => {
|
||||
});
|
||||
});
|
||||
|
||||
// #557: the copyable-key contract. noTimestamp suppresses `iat`, so the token
|
||||
// is a pure deterministic function of (payload, secret) — re-minting the SAME
|
||||
// key yields a BYTE-IDENTICAL value, which is what makes "reveal" a safe
|
||||
// re-mint rather than a stored secret.
|
||||
it('mints an api-key JWT with NEITHER exp NOR iat (deterministic)', 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>;
|
||||
expect(decoded.exp).toBeUndefined();
|
||||
expect(decoded.iat).toBeUndefined();
|
||||
});
|
||||
|
||||
it('two mints of the SAME key are byte-identical (re-mint = reveal)', async () => {
|
||||
const { service } = makeRealSignerService();
|
||||
const opts = {
|
||||
apiKeyId: 'key-1',
|
||||
user: user as never,
|
||||
workspaceId: 'ws-1',
|
||||
};
|
||||
|
||||
const first = await service.generateApiToken(opts);
|
||||
// A different time (and a fresh signer instance) must not change the bytes.
|
||||
await new Promise((r) => setTimeout(r, 1100));
|
||||
const { service: service2 } = makeRealSignerService();
|
||||
const second = await service2.generateApiToken(opts);
|
||||
|
||||
expect(second).toBe(first);
|
||||
});
|
||||
|
||||
it('demonstrates the live bug it guards: the SHARED signer WOULD add exp', () => {
|
||||
const { JwtService } = require('@nestjs/jwt');
|
||||
const sharedJwt = new JwtService({
|
||||
|
||||
@@ -155,19 +155,31 @@ export class TokenService {
|
||||
// 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.
|
||||
//
|
||||
// The dedicated signer ALSO sets `noTimestamp: true`, so the token carries
|
||||
// neither `exp` nor `iat`. The payload is a fixed literal { sub, apiKeyId,
|
||||
// workspaceId, type } in a stable order, so the HS256 signature is a pure
|
||||
// deterministic function of (sub, apiKeyId, workspaceId, APP_SECRET): minting
|
||||
// the SAME key twice yields a BYTE-IDENTICAL token. This is what makes the key
|
||||
// "copyable" — the reveal endpoint (#557) re-mints the same value under a
|
||||
// step-up without ever persisting the token material.
|
||||
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`.
|
||||
// `exp` claim. `noTimestamp: true` additionally suppresses the default `iat`
|
||||
// claim jsonwebtoken would otherwise add, making the minted token a fully
|
||||
// deterministic function of its payload + secret (byte-identical re-mints, the
|
||||
// basis of the copyable/reveal flow). Built once and cached. Verification still
|
||||
// goes through the shared verifier (same secret); `verifyAsync` does not
|
||||
// require an `exp` or `iat`.
|
||||
private _apiKeyJwtService?: JwtService;
|
||||
private apiKeyJwtService(): JwtService {
|
||||
if (!this._apiKeyJwtService) {
|
||||
this._apiKeyJwtService = new JwtService({
|
||||
secret: this.environmentService.getAppSecret(),
|
||||
signOptions: { issuer: 'Docmost' },
|
||||
signOptions: { issuer: 'Docmost', noTimestamp: true },
|
||||
});
|
||||
}
|
||||
return this._apiKeyJwtService;
|
||||
|
||||
@@ -70,23 +70,6 @@ export class EnvironmentService {
|
||||
return this.configService.get<string>('JWT_TOKEN_EXPIRES_IN', '90d');
|
||||
}
|
||||
|
||||
// Kill-switch for the agent API-key feature. Default ON (unset -> enabled): a
|
||||
// deploy that never sets the variable must NOT silently kill every agent. The
|
||||
// parse is STRICT — the value is validated by environment.validation to be
|
||||
// exactly 'true' or 'false' (or absent), so a typo like `=0`/`=off`/`=False`
|
||||
// fails at boot rather than being read as "enabled" (which would leave an
|
||||
// operator who yanked the switch during an incident with it still on). When
|
||||
// OFF: validate() denies every api-key token and the issuance endpoints 404.
|
||||
isApiKeysEnabled(): boolean {
|
||||
return this.configService.get<string>('API_KEYS_ENABLED', 'true') !== 'false';
|
||||
}
|
||||
|
||||
// Raw value (or undefined) for the boot log, so the state after each deploy is
|
||||
// verifiable in container logs: `API keys: ENABLED/DISABLED (API_KEYS_ENABLED=...)`.
|
||||
getApiKeysEnabledRaw(): string | undefined {
|
||||
return this.configService.get<string>('API_KEYS_ENABLED');
|
||||
}
|
||||
|
||||
getCookieExpiresIn(): Date {
|
||||
const expiresInStr = this.getJwtTokenExpiresIn();
|
||||
let msUntilExpiry: number;
|
||||
|
||||
@@ -103,15 +103,6 @@ export class EnvironmentVariables {
|
||||
@IsString()
|
||||
TYPESENSE_LOCALE: string;
|
||||
|
||||
// Agent API-key kill-switch. Optional (absent -> default ON). STRICT: only the
|
||||
// literals 'true'/'false' are accepted, so `=0`/`=off`/`=False` fail at boot
|
||||
// instead of being silently read as "enabled" — the switch must actually flip
|
||||
// when an operator flips it. See EnvironmentService.isApiKeysEnabled.
|
||||
@IsOptional()
|
||||
@IsIn(['true', 'false'])
|
||||
@IsString()
|
||||
API_KEYS_ENABLED: string;
|
||||
|
||||
@IsOptional()
|
||||
@ValidateIf((obj) => obj.AI_DRIVER)
|
||||
@IsIn(['openai', 'openai-compatible', 'gemini', 'ollama'])
|
||||
|
||||
Reference in New Issue
Block a user