From a03a28ef636f21a3a8103c99a1c043e6a20ccf04 Mon Sep 17 00:00:00 2001 From: Gero Posmyk-Leinemann Date: Mon, 22 Jul 2024 10:13:30 +0200 Subject: [PATCH] [audit log] Fix audit log persistence (#20054) * [audit] Catch + log errors properly * [audit] Properly serialize BigInts * [public-api] Fix PublicApiConverter toAuditLog by re-using the BigIntToJson.replacer --- .../src/typeorm/entity/db-audit-log.ts | 7 +++++- .../gitpod-db/src/typeorm/transformer.ts | 16 ++++++++++++++ .../gitpod-protocol/src/util/stringify.ts | 22 +++++++++++++++++++ .../src/public-api-converter.ts | 3 ++- components/server/src/api/server.ts | 14 +++++------- .../src/audit/AuditLogService.spec.db.ts | 17 ++++++++++++++ .../server/src/audit/AuditLogService.ts | 4 ++++ .../websocket/websocket-connection-manager.ts | 2 +- 8 files changed, 73 insertions(+), 12 deletions(-) create mode 100644 components/gitpod-protocol/src/util/stringify.ts diff --git a/components/gitpod-db/src/typeorm/entity/db-audit-log.ts b/components/gitpod-db/src/typeorm/entity/db-audit-log.ts index ce4058216cac58..43921b0335c340 100644 --- a/components/gitpod-db/src/typeorm/entity/db-audit-log.ts +++ b/components/gitpod-db/src/typeorm/entity/db-audit-log.ts @@ -7,6 +7,8 @@ import { Entity, Column, PrimaryColumn } from "typeorm"; import { TypeORM } from "../typeorm"; import { AuditLog } from "@gitpod/gitpod-protocol/lib/audit-log"; +import { BigIntToJson } from "@gitpod/gitpod-protocol/lib/util/stringify"; +import { Transformer } from "../transformer"; @Entity() export class DBAuditLog implements AuditLog { @@ -25,6 +27,9 @@ export class DBAuditLog implements AuditLog { @Column("varchar") action: string; - @Column("simple-json") + @Column({ + type: "text", // it's JSON on DB, but we aim to disable the Typeorm-internal JSON-transformer we can't control and can't disable otherwise + transformer: Transformer.SIMPLE_JSON_CUSTOM([], BigIntToJson.replacer, BigIntToJson.reviver), + }) args: object[]; } diff --git a/components/gitpod-db/src/typeorm/transformer.ts b/components/gitpod-db/src/typeorm/transformer.ts index 944e9f72d4fb3b..68e2c536cec26c 100644 --- a/components/gitpod-db/src/typeorm/transformer.ts +++ b/components/gitpod-db/src/typeorm/transformer.ts @@ -81,6 +81,22 @@ export namespace Transformer { }; }; + export const SIMPLE_JSON_CUSTOM = ( + defaultValue: any, + replacer?: (this: any, key: string, value: any) => any, + reviver?: (this: any, key: string, value: any) => any, + ) => { + return { + to(value: any): any { + return JSON.stringify(value || defaultValue, replacer); + }, + from(value: any): any { + // eslint-disable-next-line @typescript-eslint/no-unsafe-argument + return JSON.parse(value, reviver); + }, + }; + }; + export const encrypted = (encryptionServiceProvider: () => EncryptionService): ValueTransformer => { return { to(value: any): any { diff --git a/components/gitpod-protocol/src/util/stringify.ts b/components/gitpod-protocol/src/util/stringify.ts new file mode 100644 index 00000000000000..e8d3adcfd9eb4e --- /dev/null +++ b/components/gitpod-protocol/src/util/stringify.ts @@ -0,0 +1,22 @@ +/** + * Copyright (c) 2024 Gitpod GmbH. All rights reserved. + * Licensed under the GNU Affero General Public License (AGPL). + * See License.AGPL.txt in the project root for license information. + */ + +export namespace BigIntToJson { + export const BIGINT_KEY = "bigint:"; + export function replacer(key: string, value: any) { + if (typeof value === "bigint") { + return `${BIGINT_KEY}${value.toString()}`; + } + return value; + } + export function reviver(key: string, value: any) { + if (typeof value === "string" && value.startsWith(BIGINT_KEY)) { + const v: string = value.substring(7); + return BigInt(v); + } + return value; + } +} diff --git a/components/public-api/typescript-common/src/public-api-converter.ts b/components/public-api/typescript-common/src/public-api-converter.ts index c50c2fc852358f..59b0d8d4322217 100644 --- a/components/public-api/typescript-common/src/public-api-converter.ts +++ b/components/public-api/typescript-common/src/public-api-converter.ts @@ -159,6 +159,7 @@ import { WorkspaceStatus_PrebuildResult, WorkspaceStatus_WorkspaceConditions, } from "@gitpod/public-api/lib/gitpod/v1/workspace_pb"; +import { BigIntToJson } from "@gitpod/gitpod-protocol/lib/util/stringify"; import { getPrebuildLogPath } from "./prebuild-utils"; import { InvalidGitpodYMLError, RepositoryNotFoundError, UnauthorizedRepositoryAccessError } from "./public-api-errors"; const URL = require("url").URL || window.URL; @@ -1604,7 +1605,7 @@ export class PublicAPIConverter { actorId: input.actorId, action: input.action, timestamp: this.toTimestamp(input.timestamp), - args: JSON.stringify(input.args), + args: JSON.stringify(input.args, BigIntToJson.replacer), }); } } diff --git a/components/server/src/api/server.ts b/components/server/src/api/server.ts index 0a829367f91173..11d2d3dac18414 100644 --- a/components/server/src/api/server.ts +++ b/components/server/src/api/server.ts @@ -306,15 +306,11 @@ export class API { const promise = await apply>(); const result = await promise; if (subjectId) { - try { - self.auditLogService.recordAuditLog( - subjectId!.userId()!, - requestContext.requestMethod, - [args[0]], - ); - } catch (error) { - log.error("Failed to record audit log", error); - } + self.auditLogService.asyncRecordAuditLog( + subjectId!.userId()!, + requestContext.requestMethod, + [args[0]], + ); } done(); return result; diff --git a/components/server/src/audit/AuditLogService.spec.db.ts b/components/server/src/audit/AuditLogService.spec.db.ts index cf16b8442527fd..a4bcabfcd2bac4 100644 --- a/components/server/src/audit/AuditLogService.spec.db.ts +++ b/components/server/src/audit/AuditLogService.spec.db.ts @@ -15,6 +15,7 @@ import { createTestContainer } from "../test/service-testing-container-module"; import { UserService } from "../user/user-service"; import { AuditLogService } from "./AuditLogService"; import { Experiments } from "@gitpod/gitpod-protocol/lib/experiments/configcat-server"; +import { Timestamp } from "@bufbuild/protobuf"; describe("AuditLogService", () => { let container: Container; @@ -94,6 +95,22 @@ describe("AuditLogService", () => { expect(logs.length).to.eq(1); }); + it("should record audit log containing timestamp args (like listWorkspaceSessions)", async () => { + const t1 = new Timestamp({ seconds: 100n, nanos: 0 }); + await auditLogService.recordAuditLog(owner.id, "action1", [ + { + organizationId: org.id, + }, + t1, + ]); + + const logs = await auditLogService.listAuditLogs(owner.id, org.id); + + expect(logs.length).to.eq(1); + expect(logs[0].args.length).to.eq(2); + expect(logs[0].args[1]).to.deep.equal(t1); + }); + it("should list audit logs", async () => { await auditLogDB.recordAuditLog({ id: v4(), diff --git a/components/server/src/audit/AuditLogService.ts b/components/server/src/audit/AuditLogService.ts index f5b80ef5845e2f..3b3b5d1085bb97 100644 --- a/components/server/src/audit/AuditLogService.ts +++ b/components/server/src/audit/AuditLogService.ts @@ -23,6 +23,10 @@ export class AuditLogService { @inject(UserDB) private readonly dbUser: UserDB, ) {} + asyncRecordAuditLog(actorId: string, method: string, args: any[]): void { + this.recordAuditLog(actorId, method, args).catch((err) => log.error("failed to record audit log", err)); + } + async recordAuditLog(actorId: string, method: string, args: any[]): Promise { if ( !(await getExperimentsClientForBackend().getValueAsync("audit_logs", false, { diff --git a/components/server/src/websocket/websocket-connection-manager.ts b/components/server/src/websocket/websocket-connection-manager.ts index afdb19ebcb0aad..5ed5836b3c9e57 100644 --- a/components/server/src/websocket/websocket-connection-manager.ts +++ b/components/server/src/websocket/websocket-connection-manager.ts @@ -404,7 +404,7 @@ class GitpodJsonRpcProxyFactory extends JsonRpcProxyFactory const result = await this.internalOnRequest(span, method, ...args); if (userId) { // omit the last argument, which is the cancellation token - this.auditLogService.recordAuditLog(userId, method, args.slice(0, -1)); + this.auditLogService.asyncRecordAuditLog(userId, method, args.slice(0, -1)); } return result; } finally {