Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/copilot-instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
- If you need to run `npx prisma generate`, run `npm run db:generate` from `apps/backend-services` - it's a special script that writes models into apps/temporal/src and apps/backend-services/src. Don't forget to run migrations as normal if necessary.
- Tables should be designed with a `created_at` timestamp (default to now) and an `updated_at` timestamp (auto-updated on change) for auditing purposes.
- Table names should be singular (e.g., `User`, not `Users`) to align with Prisma conventions.
- **Transactions:** Two or more database writes that must succeed or fail together MUST use a single Prisma transaction. Db-services take optional `tx?: Prisma.TransactionClient` as the last parameter (`const client = tx ?? this.prisma`). Services start cross-module transactions with `prismaService.transaction()` and pass `tx` through — services must not query `tx` directly. Controllers never use transactions. See `docs-md/DATABASE_SERVICES.md`.
- **Audit on mutations:** Every user-initiated mutation and every service-layer mutation transaction MUST emit an audit event (`AuditService.recordEvent` or benchmark `AuditLogService`). Pass `tx` to audit when inside a transaction; otherwise audit after commit (best-effort, non-fatal). See `docs-md/AUDIT.md` and the compliance audit in `docs-md/TRANSACTION_AND_AUDIT_AUDIT.md`.

## Requirements and User Stories
- When finished implementing a user story, check it off in the related user stories file in `feature-docs/002-group-management/user_stories/README.md` and update the acceptance checklist.
Expand Down
4 changes: 3 additions & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,6 @@
- Do not include any document-specific implementation, the system is generic and must support arbitrary workloads
- All backend controllers must have full Swagger/OpenAPI documentation: use specific decorators (`@ApiOkResponse`, `@ApiForbiddenResponse`, `@ApiUnauthorizedResponse`, `@ApiConflictResponse`, etc.) instead of generic `@ApiResponse`, create dedicated DTO classes with `@ApiProperty` decorators for all request/response shapes, and reference those DTOs via the `type` field in response decorators.
- To test API directly, use: `curl -H "x-api-key: 69OrdcwUk4qrB6Pl336PGsloa0L084HFp7X7aX7sSTY" http://localhost:3002/api/...`
- NEVER read secrets from .env files directly, they should not be leaked into chat, terminal, etc., do not operate with secret values directly, only indirectly through variables.
- NEVER read secrets from .env files directly, they should not be leaked into chat, terminal, etc., do not operate with secret values directly, only indirectly through variables.
- **Database transactions:** Any operation that performs two or more database writes that must stay consistent MUST run inside a single Prisma transaction. Db-services accept optional `tx?: Prisma.TransactionClient` as the last parameter and use `const client = tx ?? this.prisma`. Services initiate cross-module transactions via `prismaService.transaction(async (tx) => { ... })` and pass `tx` to db-services (and other services) — never query `tx` directly in services. Controllers never initiate or receive transactions. See [docs-md/DATABASE_SERVICES.md](docs-md/DATABASE_SERVICES.md).
- **Audit on mutations:** Every user-initiated create/update/delete (and every service-layer mutation transaction) MUST record an audit event via `AuditService.recordEvent` (global) or `AuditLogService` / `AuditLogDbService` (benchmark). Pass the same `tx` into audit when the mutation is transactional; otherwise call audit immediately after a successful commit. Audit failures must not fail the main operation unless audit is intentionally in the same transaction. Read/access endpoints follow [docs-md/AUDIT.md](docs-md/AUDIT.md). When reviewing or adding backend code, check [docs-md/TRANSACTION_AND_AUDIT_AUDIT.md](docs-md/TRANSACTION_AND_AUDIT_AUDIT.md) for known gaps.
26 changes: 24 additions & 2 deletions apps/backend-services/src/actor/api-key.service.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { NotFoundException } from "@nestjs/common";
import { Test, TestingModule } from "@nestjs/testing";
import * as bcrypt from "bcrypt";
import { AuditService } from "@/audit/audit.service";
import { AppLoggerService } from "@/logging/app-logger.service";
import { mockAppLogger } from "@/testUtils/mockAppLogger";
import { ApiKeyService } from "./api-key.service";
Expand All @@ -16,6 +17,10 @@ const mockApiKeyDbService = {
updateApiKeyLastUsed: jest.fn(),
};

const mockAuditService = {
recordEvent: jest.fn().mockResolvedValue(undefined),
};

describe("ApiKeyService", () => {
let service: ApiKeyService;

Expand All @@ -33,6 +38,10 @@ describe("ApiKeyService", () => {
provide: ApiKeyDbService,
useValue: mockApiKeyDbService,
},
{
provide: AuditService,
useValue: mockAuditService,
},
],
}).compile();

Expand Down Expand Up @@ -140,16 +149,29 @@ describe("ApiKeyService", () => {

describe("deleteApiKey", () => {
it("should throw NotFoundException if no key exists", async () => {
mockApiKeyDbService.deleteApiKeyById.mockRejectedValue({ code: "P2025" });
await expect(service.deleteApiKey("key123")).rejects.toBeDefined();
mockApiKeyDbService.findApiKeyById.mockResolvedValue(null);
await expect(service.deleteApiKey("key123")).rejects.toThrow(
NotFoundException,
);
});

it("should delete a key by its ID", async () => {
mockApiKeyDbService.findApiKeyById.mockResolvedValue({
id: "key123",
group_id: "group123",
key_prefix: "abcd1234",
});
mockApiKeyDbService.deleteApiKeyById.mockResolvedValue({ id: "key123" });
await service.deleteApiKey("key123");
expect(mockApiKeyDbService.deleteApiKeyById).toHaveBeenCalledWith(
"key123",
);
expect(mockAuditService.recordEvent).toHaveBeenCalledWith(
expect.objectContaining({
event_type: "api_key_deleted",
resource_id: "key123",
}),
);
});
});

Expand Down
33 changes: 31 additions & 2 deletions apps/backend-services/src/actor/api-key.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
ApiKeyInfoDto,
GeneratedApiKeyDto,
} from "@/actor/dto/api-key-info.dto";
import { AuditService } from "@/audit/audit.service";
import type { ValidatedApiKey } from "@/auth/types";
import { AppLoggerService } from "@/logging/app-logger.service";
import { ApiKeyDbService } from "./api-key-db.service";
Expand All @@ -14,6 +15,7 @@ export class ApiKeyService {
constructor(
private readonly apiKeyDb: ApiKeyDbService,
private readonly logger: AppLoggerService,
private readonly auditService: AuditService,
) {}

async getApiKey(groupId: string): Promise<ApiKeyInfoDto | null> {
Expand Down Expand Up @@ -72,6 +74,14 @@ export class ApiKeyService {

this.logger.log(`API key generated for user ${userId} in group ${groupId}`);

await this.auditService.recordEvent({
event_type: "api_key_created",
resource_type: "api_key",
resource_id: apiKey.id,
group_id: groupId,
payload: { key_prefix: keyPrefix, generating_user_id: userId },
});

return {
id: apiKey.id,
key,
Expand All @@ -84,10 +94,21 @@ export class ApiKeyService {
}

async deleteApiKey(keyId: string): Promise<void> {
const deleted = await this.apiKeyDb.deleteApiKeyById(keyId);
if (!deleted) {
const existing = await this.apiKeyDb.findApiKeyById(keyId);
if (!existing) {
throw new NotFoundException("No API key found with this ID");
}

await this.apiKeyDb.deleteApiKeyById(keyId);

await this.auditService.recordEvent({
event_type: "api_key_deleted",
resource_type: "api_key",
resource_id: keyId,
group_id: existing.group_id,
payload: { key_prefix: existing.key_prefix },
});

this.logger.log(`API key ${keyId} deleted`);
}

Expand All @@ -106,6 +127,14 @@ export class ApiKeyService {

this.logger.log(`API key generated for user ${userId} in group ${groupId}`);

await this.auditService.recordEvent({
event_type: "api_key_regenerated",
resource_type: "api_key",
resource_id: apiKey.id,
group_id: groupId,
payload: { key_prefix: keyPrefix, generating_user_id: userId },
});

return {
id: apiKey.id,
key,
Expand Down
80 changes: 75 additions & 5 deletions apps/backend-services/src/audit/audit.service.spec.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Prisma } from "@generated/client";
import { Test, TestingModule } from "@nestjs/testing";
import { AppLoggerService } from "@/logging/app-logger.service";
import * as requestContextModule from "@/logging/request-context";
Expand Down Expand Up @@ -60,6 +61,7 @@ describe("AuditService", () => {
group_id: "grp-1",
request_id: "req-1",
}),
undefined,
);
});

Expand All @@ -74,13 +76,14 @@ describe("AuditService", () => {

expect(mockAuditDb.createAuditEvent).toHaveBeenCalledWith(
expect.objectContaining({ actor_id: null }),
undefined,
);
});

it("should use userId from request context when actor_id is omitted", async () => {
it("should use actorId from request context when actor_id is omitted", async () => {
jest
.spyOn(requestContextModule, "getRequestContext")
.mockReturnValue({ requestId: "req-ctx-1", actorId: undefined });
.mockReturnValue({ requestId: "req-ctx-1", actorId: "actor-ctx-1" });
mockAuditDb.createAuditEvent.mockResolvedValue(undefined);

await service.recordEvent({
Expand All @@ -90,7 +93,8 @@ describe("AuditService", () => {
});

expect(mockAuditDb.createAuditEvent).toHaveBeenCalledWith(
expect.objectContaining({ actor_id: null }),
expect.objectContaining({ actor_id: "actor-ctx-1" }),
undefined,
);
});

Expand All @@ -107,11 +111,34 @@ describe("AuditService", () => {
});

expect(mockAuditDb.createAuditEvent).toHaveBeenCalledWith(
expect.objectContaining({ request_id: "ctx-req-1" }),
expect.objectContaining({
request_id: "ctx-req-1",
actor_id: "u",
}),
undefined,
);
});

it("should prefer explicit actor_id over request context", async () => {
jest
.spyOn(requestContextModule, "getRequestContext")
.mockReturnValue({ requestId: "ctx-req-1", actorId: "ctx-actor" });
mockAuditDb.createAuditEvent.mockResolvedValue(undefined);

await service.recordEvent({
event_type: "TEST",
resource_type: "document",
resource_id: "res-1",
actor_id: "explicit-actor",
});

expect(mockAuditDb.createAuditEvent).toHaveBeenCalledWith(
expect.objectContaining({ actor_id: "explicit-actor" }),
undefined,
);
});

it("should set optional fields to null when omitted and context has no userId", async () => {
it("should set optional fields to null when omitted and context has no actorId", async () => {
jest
.spyOn(requestContextModule, "getRequestContext")
.mockReturnValue({ requestId: "ctx-req-1" });
Expand All @@ -130,8 +157,51 @@ describe("AuditService", () => {
workflow_execution_id: null,
group_id: null,
}),
undefined,
);
});

it("should pass tx to createAuditEvent when provided", async () => {
mockAuditDb.createAuditEvent.mockResolvedValue(undefined);
const tx = {} as Prisma.TransactionClient;

await service.recordEvent(
{
event_type: "TEST",
resource_type: "document",
resource_id: "res-1",
},
tx,
);

expect(mockAuditDb.createAuditEvent).toHaveBeenCalledWith(
expect.objectContaining({
event_type: "TEST",
resource_id: "res-1",
}),
tx,
);
});

it("should propagate errors when tx is provided", async () => {
mockAuditDb.createAuditEvent.mockRejectedValue(
new Error("DB write failed"),
);
const tx = {} as Prisma.TransactionClient;

await expect(
service.recordEvent(
{
event_type: "FAIL",
resource_type: "doc",
resource_id: "r1",
},
tx,
),
).rejects.toThrow("DB write failed");

expect(mockAppLogger.warn).not.toHaveBeenCalled();
});
});

describe("recordEvent - array of events", () => {
Expand Down
28 changes: 20 additions & 8 deletions apps/backend-services/src/audit/audit.service.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Prisma } from "@generated/client";
import { Injectable } from "@nestjs/common";
import { AppLoggerService } from "@/logging/app-logger.service";
import { getRequestContext } from "@/logging/request-context";
Expand All @@ -12,29 +13,40 @@ export class AuditService {
) {}

/**
* Records one or more audit events. Failures are logged and do not throw
* so that audit write failures do not fail the main operation.
* When request_id is omitted, it is filled from the current
* request context (AsyncLocalStorage) when available.
* Records one or more audit events. When `tx` is omitted, failures are logged
* and do not throw so audit write failures do not fail the main operation.
* When `tx` is provided, audit writes participate in the caller's transaction
* and failures propagate (rolling back the transaction).
* When request_id is omitted, it is filled from the current request context
* (AsyncLocalStorage) when available.
*/
async recordEvent(
events: CreateAuditEventInput | CreateAuditEventInput[],
tx?: Prisma.TransactionClient,
): Promise<void> {
const ctx = getRequestContext();
const list = Array.isArray(events) ? events : [events];
for (const e of list) {
try {
await this.auditDb.createAuditEvent({
const write = this.auditDb.createAuditEvent(
{
event_type: e.event_type,
resource_type: e.resource_type,
resource_id: e.resource_id,
actor_id: e.actor_id ?? null,
actor_id: e.actor_id ?? ctx?.actorId ?? null,
document_id: e.document_id ?? null,
workflow_execution_id: e.workflow_execution_id ?? null,
group_id: e.group_id ?? null,
request_id: e.request_id ?? ctx?.requestId ?? null,
payload: e.payload,
});
},
tx,
);
if (tx) {
await write;
continue;
}
try {
await write;
} catch (err) {
const msg = err instanceof Error ? err.message : String(err);
this.logger.warn("Audit event write failed (non-fatal)", {
Expand Down
11 changes: 11 additions & 0 deletions apps/backend-services/src/azure/azure.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,17 @@ export class AzureController {
actorId,
);

await this.auditService.recordEvent({
event_type: "classifier_training_requested",
resource_type: "classifier",
resource_id: name,
actor_id: actorId,
group_id: group_id,
payload: {
classifier_name: name,
},
});

setImmediate(async () => {
try {
// Upload the documents required for training
Expand Down
7 changes: 7 additions & 0 deletions apps/backend-services/src/azure/classifier.service.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Test, TestingModule } from "@nestjs/testing";
import { AuditService } from "@/audit/audit.service";
import { BLOB_STORAGE_CONTAINER_NAME } from "@/blob-storage/blob-storage.module";
import { AppLoggerService } from "@/logging/app-logger.service";
import { mockAppLogger } from "@/testUtils/mockAppLogger";
Expand All @@ -15,6 +16,8 @@ import { ClassifierDbService } from "./classifier-db.service";
const mockClassifierDbService = {
findClassifierModel: jest.fn(),
updateClassifierModel: jest.fn(),
createClassifierModel: jest.fn(),
deleteClassifierModel: jest.fn(),
};
const mockAzureService = {
getClient: jest.fn().mockReturnValue({
Expand Down Expand Up @@ -69,6 +72,10 @@ describe("ClassifierService", () => {
{ provide: AzureStorageService, useValue: azureStorage },
{ provide: BLOB_STORAGE, useValue: blobStorage },
{ provide: BLOB_STORAGE_CONTAINER_NAME, useValue: "document-blobs" },
{
provide: AuditService,
useValue: { recordEvent: jest.fn().mockResolvedValue(undefined) },
},
],
}).compile();
service = module.get<ClassifierService>(ClassifierService);
Expand Down
Loading
Loading