-
Notifications
You must be signed in to change notification settings - Fork 12k
feat: add initial AuditLogService and /audit-logs API endpoint #22578
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,12 @@ | ||||||||||||||||||||||||||||||||
| import type { AuditLogEvent } from "~/lib/types"; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| export const AuditLogService = { | ||||||||||||||||||||||||||||||||
| async logEvent(event: AuditLogEvent): Promise<void> { | ||||||||||||||||||||||||||||||||
| console.log("Audit Log Event:", event); | ||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| async getEvents(filter?: any): Promise<AuditLogEvent[]> { | ||||||||||||||||||||||||||||||||
| // Returning Expty array for now | ||||||||||||||||||||||||||||||||
| return []; | ||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||
|
Comment on lines
+8
to
+11
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Improve type safety for filter parameter. The +interface AuditLogFilter {
+ orgId?: string;
+ userId?: string;
+ action?: string;
+ [key: string]: any;
+}
+
- async getEvents(filter?: any): Promise<AuditLogEvent[]> {
+ async getEvents(filter?: AuditLogFilter): Promise<AuditLogEvent[]> {📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,6 @@ | ||||||||||||||||||||||||
| import { AuditLogService } from "~/lib/audit-log.service"; | ||||||||||||||||||||||||
| import type { AuditLogEvent } from "~/lib/types"; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| export async function EmitAuditLogEvent(event: AuditLogEvent) { | ||||||||||||||||||||||||
| await AuditLogService.logEvent(event); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
Comment on lines
+4
to
+6
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add error handling to the wrapper function. The wrapper function should handle potential errors from the underlying service call, especially since audit logging failures shouldn't typically break the main application flow. -export async function EmitAuditLogEvent(event: AuditLogEvent) {
- await AuditLogService.logEvent(event);
-}
+export async function EmitAuditLogEvent(event: AuditLogEvent) {
+ try {
+ await AuditLogService.logEvent(event);
+ } catch (error) {
+ console.error("Failed to emit audit log event:", error);
+ // Consider whether to throw or handle gracefully based on requirements
+ }
+}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -146,6 +146,18 @@ interface EventTypeExtended extends Omit<EventType, "recurringEvent" | "location | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| | any; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| //Audit Logs for Teams | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export type AuditLogAction = "RENAME" | "DELETE" | "BOOK" | "CANCEL" | "CREATE"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export interface AuditLogEvent { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| timestamp: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| userId: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| orgId: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| action: AuditLogAction; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| resource: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| details: Record<string, any>; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+152
to
+159
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Address interface inconsistencies and improve type safety. The
export interface AuditLogEvent {
+ id: string;
timestamp: string;
userId: string;
orgId: string;
+ teamId?: string;
action: AuditLogAction;
- resource: string;
+ resourceId: string;
+ resourceType: string;
+ description?: string;
- details: Record<string, any>;
+ details: Record<string, unknown>;
}The 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
Comment on lines
+152
to
+159
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interface definition needs alignment with PR objectives. The interface is missing several fields mentioned in the PR objectives and has some type considerations:
Apply this diff to align with PR objectives: export interface AuditLogEvent {
+ id: string;
- timestamp: string;
+ timestamp: Date;
userId: string;
- orgId: string;
+ orgId?: string;
+ teamId?: string;
action: AuditLogAction;
resource: string;
+ description?: string;
- details: Record<string, any>;
+ details?: Record<string, any>;
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // EventType | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export type EventTypeResponse = BaseResponse & { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| event_type?: Partial<EventType | EventTypeExtended>; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| import type { NextApiRequest, NextApiResponse } from "next"; | ||
|
|
||
| import { HttpError } from "@calcom/lib/http-error"; | ||
| import { defaultResponder } from "@calcom/lib/server/defaultResponder"; | ||
|
|
||
| import { AuditLogService } from "~/lib/audit-log.service"; | ||
| import type { AuditLogEvent } from "~/lib/types"; | ||
|
|
||
| async function handler(req: NextApiRequest, res: NextApiResponse) { | ||
| const { orgId, userId, action } = req.query; | ||
|
|
||
| const filter: Record<string, string | undefined> = { | ||
| orgId: typeof orgId === "string" ? orgId : undefined, | ||
| userId: typeof userId === "string" ? userId : undefined, | ||
| action: typeof action === "string" ? action : undefined, | ||
| }; | ||
|
|
||
| try { | ||
| const events: AuditLogEvent[] = await AuditLogService.getEvents?.(filter); | ||
| return res.status(200).json({ events }); | ||
| } catch (error) { | ||
| throw new HttpError({ | ||
| statusCode: 500, | ||
| message: "Failed to fetch audit logs", | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| export default defaultResponder(handler); |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,37 @@ | ||||||||||||||||||||||
| import type { NextApiRequest, NextApiResponse } from "next"; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| import { HttpError } from "@calcom/lib/http-error"; | ||||||||||||||||||||||
| import { defaultResponder } from "@calcom/lib/server/defaultResponder"; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| import { AuditLogService } from "~/lib/audit-log.service"; | ||||||||||||||||||||||
| import type { AuditLogEvent } from "~/lib/types"; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| async function handler(req: NextApiRequest, res: NextApiResponse) { | ||||||||||||||||||||||
| const body = req.body as Partial<AuditLogEvent>; | ||||||||||||||||||||||
| if (!body || typeof body !== "object") { | ||||||||||||||||||||||
| throw new HttpError({ | ||||||||||||||||||||||
| statusCode: 400, | ||||||||||||||||||||||
| message: "Invalid/Missing JSON body", | ||||||||||||||||||||||
| }); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // Required audit event fields | ||||||||||||||||||||||
| const requiredFields = ["timestamp", "userId", "orgId", "action", "resource", "details"]; | ||||||||||||||||||||||
| for (const field of requiredFields) { | ||||||||||||||||||||||
| if (!body[field as keyof AuditLogEvent]) { | ||||||||||||||||||||||
| throw new HttpError({ | ||||||||||||||||||||||
| statusCode: 400, | ||||||||||||||||||||||
| message: `Missing required field: ${field}`, | ||||||||||||||||||||||
| }); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| try { | ||||||||||||||||||||||
| await AuditLogService.logEvent(body as AuditLogEvent); | ||||||||||||||||||||||
| return res.status(201).json({ ok: true }); | ||||||||||||||||||||||
| } catch (error: unknown) { | ||||||||||||||||||||||
| throw error; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
Comment on lines
+29
to
+34
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Remove unnecessary try/catch clause. The try/catch block that only rethrows the original error is unnecessary and can be confusing. The - try {
- await AuditLogService.logEvent(body as AuditLogEvent);
- return res.status(201).json({ ok: true });
- } catch (error: unknown) {
- throw error;
- }
+ await AuditLogService.logEvent(body as AuditLogEvent);
+ return res.status(201).json({ ok: true });📝 Committable suggestion
Suggested change
🧰 Tools🪛 Biome (1.9.4)[error] 33-33: The catch clause that only rethrows the original error is useless. An unnecessary catch clause can be confusing. (lint/complexity/noUselessCatch) 🤖 Prompt for AI Agents |
||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
Comment on lines
+29
to
+35
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Remove unnecessary try-catch block. The catch clause only rethrows the original error without adding any value. Since the - try {
- await AuditLogService.logEvent(body as AuditLogEvent);
- return res.status(201).json({ ok: true });
- } catch (error: unknown) {
- throw error;
- }
+ await AuditLogService.logEvent(body as AuditLogEvent);
+ return res.status(201).json({ ok: true });📝 Committable suggestion
Suggested change
🧰 Tools🪛 Biome (1.9.4)[error] 33-33: The catch clause that only rethrows the original error is useless. An unnecessary catch clause can be confusing. (lint/complexity/noUselessCatch) 🤖 Prompt for AI Agents |
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| export default defaultResponder(handler); | ||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| import { defaultHandler } from "@calcom/lib/server/defaultHandler"; | ||
|
|
||
| import { withMiddleware } from "~/lib/helpers/withMiddleware"; | ||
|
|
||
| export default withMiddleware()( | ||
| defaultHandler({ | ||
| POST: import("./_post"), | ||
| GET: import("./_get"), | ||
| }) | ||
| ); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| export const AUDIT_LOG_EVENT = "audit.log"; | ||
|
|
||
| export type AuditLogPayload = { | ||
| teamId: number; | ||
| actorId: number; | ||
| action: string; | ||
| targetType: string; | ||
| targetId: string; | ||
| metadata?: Record<string, unknown>; | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| import { AuditLogPayload } from "@/ee/audit-logs/lib/audit-log.events"; | ||
| import { Injectable } from "@nestjs/common"; | ||
| import { Prisma } from "@prisma/client"; | ||
| import { PrismaWriteService } from "src/modules/prisma/prisma-write.service"; | ||
|
|
||
| export abstract class AuditLogService { | ||
| abstract log(payload: AuditLogPayload): Promise<void>; | ||
| } | ||
|
|
||
| @Injectable() | ||
| export class PrismaAuditLogService extends AuditLogService { | ||
| // Injecting the PrismaWriteService class | ||
| constructor(private readonly prismaWriteService: PrismaWriteService) { | ||
| super(); | ||
| } | ||
|
|
||
| async log(payload: AuditLogPayload): Promise<void> { | ||
| await this.prismaWriteService.prisma.auditLog.create({ | ||
| data: { | ||
| teamId: payload.teamId, | ||
| actorId: payload.actorId, | ||
| action: payload.action, | ||
| targetType: payload.targetType, | ||
| targetId: payload.targetId, | ||
| metadata: (payload.metadata || {}) as Prisma.InputJsonValue, | ||
| }, | ||
| }); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| -- CreateTable | ||
| CREATE TABLE "AuditLog" ( | ||
| "id" SERIAL NOT NULL, | ||
| "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, | ||
| "teamId" INTEGER NOT NULL, | ||
| "actorId" INTEGER NOT NULL, | ||
| "action" TEXT NOT NULL, | ||
| "targetType" TEXT NOT NULL, | ||
| "targetId" TEXT NOT NULL, | ||
| "metadata" JSONB NOT NULL, | ||
|
|
||
| CONSTRAINT "AuditLog_pkey" PRIMARY KEY ("id") | ||
| ); | ||
|
|
||
| -- CreateIndex | ||
| CREATE INDEX "AuditLog_teamId_idx" ON "AuditLog"("teamId"); | ||
|
|
||
| -- CreateIndex | ||
| CREATE INDEX "AuditLog_actorId_idx" ON "AuditLog"("actorId"); | ||
|
|
||
| -- CreateIndex | ||
| CREATE INDEX "AuditLog_targetType_targetId_idx" ON "AuditLog"("targetType", "targetId"); | ||
|
|
||
| -- AddForeignKey | ||
| ALTER TABLE "AuditLog" ADD CONSTRAINT "AuditLog_teamId_fkey" FOREIGN KEY ("teamId") REFERENCES "Team"("id") ON DELETE CASCADE ON UPDATE CASCADE; | ||
|
|
||
| -- AddForeignKey | ||
| ALTER TABLE "AuditLog" ADD CONSTRAINT "AuditLog_actorId_fkey" FOREIGN KEY ("actorId") REFERENCES "users"("id") ON DELETE CASCADE ON UPDATE CASCADE; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider production-ready logging implementation.
Console logging is not suitable for production environments. Consider implementing proper structured logging with log levels, error handling, and persistence.
📝 Committable suggestion
🤖 Prompt for AI Agents