From 647f65f273345768db24d5e3f16d7df54ca5e10b Mon Sep 17 00:00:00 2001 From: dcshzj <27919917+dcshzj@users.noreply.github.com> Date: Mon, 4 Mar 2024 11:12:02 +0800 Subject: [PATCH 01/11] feat: add site audit logs generator --- .aws/deploy/backend-task-definition.prod.json | 4 + .../backend-task-definition.staging.json | 4 + .../predeploy/06_fetch_ssm_parameters.sh | 1 + src/config/config.ts | 7 + src/constants/constants.ts | 4 + src/errors/AuditLogsError.ts | 11 + src/routes/formsg/formsgSiteAuditLogs.ts | 128 +++++ src/server.js | 17 +- src/services/admin/AuditLogsService.ts | 500 ++++++++++++++++++ .../admin/__tests__/AuditLogsService.spec.ts | 0 src/types/auditLog.ts | 17 + 11 files changed, 692 insertions(+), 1 deletion(-) create mode 100644 src/errors/AuditLogsError.ts create mode 100644 src/routes/formsg/formsgSiteAuditLogs.ts create mode 100644 src/services/admin/AuditLogsService.ts create mode 100644 src/services/admin/__tests__/AuditLogsService.spec.ts create mode 100644 src/types/auditLog.ts diff --git a/.aws/deploy/backend-task-definition.prod.json b/.aws/deploy/backend-task-definition.prod.json index 42a0f2aab..d155a0118 100644 --- a/.aws/deploy/backend-task-definition.prod.json +++ b/.aws/deploy/backend-task-definition.prod.json @@ -185,6 +185,10 @@ { "name": "SITE_CHECKER_FORM_KEY", "valueFrom": "PROD_SITE_CHECKER_FORM_KEY" + }, + { + "name": "SITE_AUDIT_LOGS_FORM_KEY", + "valueFrom": "PROD_SITE_AUDIT_LOGS_FORM_KEY" } ], "logConfiguration": { diff --git a/.aws/deploy/backend-task-definition.staging.json b/.aws/deploy/backend-task-definition.staging.json index 8eaad219e..3ca7c9d20 100644 --- a/.aws/deploy/backend-task-definition.staging.json +++ b/.aws/deploy/backend-task-definition.staging.json @@ -194,6 +194,10 @@ { "name": "SITE_CHECKER_FORM_KEY", "valueFrom": "STAGING_SITE_CHECKER_FORM_KEY" + }, + { + "name": "SITE_AUDIT_LOGS_FORM_KEY", + "valueFrom": "STAGING_SITE_AUDIT_LOGS_FORM_KEY" } ], "logConfiguration": { diff --git a/.platform/hooks/predeploy/06_fetch_ssm_parameters.sh b/.platform/hooks/predeploy/06_fetch_ssm_parameters.sh index 1e8a9c8f2..5dd2ba233 100644 --- a/.platform/hooks/predeploy/06_fetch_ssm_parameters.sh +++ b/.platform/hooks/predeploy/06_fetch_ssm_parameters.sh @@ -84,6 +84,7 @@ ENV_VARS=( "UPTIME_ROBOT_API_KEY" "GGS_REPAIR_FORM_KEY" "SITE_CHECKER_FORM_KEY" + "SITE_AUDIT_LOGS_FORM_KEY" ) echo "Set AWS region" diff --git a/src/config/config.ts b/src/config/config.ts index 775bc7ddf..e02c3ee5f 100644 --- a/src/config/config.ts +++ b/src/config/config.ts @@ -314,6 +314,13 @@ const config = convict({ format: "required-string", default: "", }, + siteAuditLogsFormKey: { + doc: "FormSG API key for site audit logs form", + env: "SITE_AUDIT_LOGS_FORM_KEY", + sensitive: true, + format: "required-string", + default: "", + }, }, postman: { apiKey: { diff --git a/src/constants/constants.ts b/src/constants/constants.ts index a13069b38..e8504c3a8 100644 --- a/src/constants/constants.ts +++ b/src/constants/constants.ts @@ -87,6 +87,10 @@ export const EFS_VOL_PATH_STAGING_LITE = path.join( config.get("aws.efs.volPath"), "repos-lite" ) +export const EFS_VOL_PATH_AUDIT_LOGS = path.join( + config.get("aws.efs.volPath"), + "audit-logs" +) export const STAGING_BRANCH = "staging" export const STAGING_LITE_BRANCH = "staging-lite" export const PLACEHOLDER_FILE_NAME = ".keep" diff --git a/src/errors/AuditLogsError.ts b/src/errors/AuditLogsError.ts new file mode 100644 index 000000000..634475e70 --- /dev/null +++ b/src/errors/AuditLogsError.ts @@ -0,0 +1,11 @@ +import { BaseIsomerError } from "./BaseError" + +export default class AuditLogsError extends BaseIsomerError { + constructor(message: string) { + super({ + status: 500, + code: "AuditLogsError", + message, + }) + } +} diff --git a/src/routes/formsg/formsgSiteAuditLogs.ts b/src/routes/formsg/formsgSiteAuditLogs.ts new file mode 100644 index 000000000..7b76fb934 --- /dev/null +++ b/src/routes/formsg/formsgSiteAuditLogs.ts @@ -0,0 +1,128 @@ +/* eslint-disable import/prefer-default-export */ +import type { DecryptedContentAndAttachments } from "@opengovsg/formsg-sdk/dist/types" +import express, { RequestHandler } from "express" + +import { config } from "@root/config/config" +import InitializationError from "@root/errors/InitializationError" +import logger from "@root/logger/logger" +import { attachFormSGHandler } from "@root/middleware" +import AuditLogsService from "@root/services/admin/AuditLogsService" +import { getField, getFieldsFromTable } from "@root/utils/formsg-utils" + +interface FormsgSiteAuditLogsRouterProps { + auditLogsService: AuditLogsService +} + +const SITE_AUDIT_LOGS_FORM_KEY = config.get("formSg.siteAuditLogsFormKey") + +const REQUESTER_EMAIL_FIELD = "Where should we send the email address to?" +const REPO_NAME_FIELD = + "What is the name of the Isomer site that you need logs for? (Repo Name (in GitHub))" +const LOGS_TIMEFRAME_FIELD = "I need a log of edits made in:" +const LOGS_TIMEFRAME_START_FIELD = "Start date" +const LOGS_TIMEFRAME_END_FIELD = "End date" + +export class FormsgSiteAuditLogsRouter { + private readonly auditLogsService: FormsgSiteAuditLogsRouterProps["auditLogsService"] + + constructor({ auditLogsService }: FormsgSiteAuditLogsRouterProps) { + this.auditLogsService = auditLogsService + } + + getAuditLogsHandler: RequestHandler< + never, + Record, + { data: { submissionId: string } }, + never, + { submission: DecryptedContentAndAttachments } + > = async (req, res) => { + let startDate = "1970-01-01" + let endDate = new Date().toISOString().split("T")[0] + const repoNames: Set = new Set() + + const { responses } = res.locals.submission.content + + const requesterEmail = getField(responses, REQUESTER_EMAIL_FIELD) + + if (!requesterEmail) { + logger.error( + "No requester email was provided in site audit logs form submission" + ) + return + } + + const repoNamesFromTable = getFieldsFromTable(responses, REPO_NAME_FIELD) + + if (!repoNamesFromTable) { + logger.error( + "No repo names were provided in site audit logs form submission" + ) + return + } + + repoNamesFromTable.forEach((repoName) => { + if (typeof repoName === "string") { + // actually wont happen based on our formsg form, but this code + // is added defensively + repoNames.add(repoName) + } else { + repoNames.add(repoName[0]) + } + }) + + const logsTimeframe = getField(responses, LOGS_TIMEFRAME_FIELD) + + if (logsTimeframe === "The past calendar year") { + startDate = `${new Date().getFullYear() - 1}-01-01` + endDate = `${new Date().getFullYear() - 1}-12-31` + } else if (logsTimeframe === "The past calendar month") { + const startDateObject = new Date() + startDateObject.setMonth(startDateObject.getMonth() - 1) + const endDateObject = new Date() + endDateObject.setDate(0) + + startDate = `${startDateObject.getFullYear()}-${startDateObject + .getMonth() + .toString() + .padStart(2, "0")}-01` + endDate = `${endDateObject.getFullYear()}-${endDateObject + .getMonth() + .toString() + .padStart(2, "0")}-${endDateObject.getDate()}` + } else { + const startDateField = getField(responses, LOGS_TIMEFRAME_START_FIELD) + const endDateField = getField(responses, LOGS_TIMEFRAME_END_FIELD) + if (startDateField && endDateField) { + startDate = startDateField + endDate = endDateField + } + } + + res.sendStatus(200) + + this.auditLogsService.getAuditLogsViaFormsg( + requesterEmail, + Array.from(repoNames), + startDate, + endDate, + req.body.data.submissionId + ) + } + + getRouter() { + const router = express.Router({ mergeParams: true }) + if (!SITE_AUDIT_LOGS_FORM_KEY) { + throw new InitializationError( + "Required SITE_AUDIT_LOGS_FORM_KEY environment variable is not defined" + ) + } + + router.post( + "/audit-logs", + attachFormSGHandler(SITE_AUDIT_LOGS_FORM_KEY), + this.getAuditLogsHandler + ) + + return router + } +} diff --git a/src/server.js b/src/server.js index 1be8605e0..a5db7531f 100644 --- a/src/server.js +++ b/src/server.js @@ -74,10 +74,12 @@ import { mailer } from "@services/utilServices/MailClient" import { database } from "./database/config" import { apiLogger } from "./middleware/apiLogger" import { NotificationOnEditHandler } from "./middleware/notificationOnEditHandler" +import { FormsgSiteAuditLogsRouter } from "./routes/formsg/formsgSiteAuditLogs" import getAuthenticatedSubrouter from "./routes/v2/authenticated" import { ReviewsRouter } from "./routes/v2/authenticated/review" import getAuthenticatedSitesSubrouter from "./routes/v2/authenticatedSites" import { SgidAuthRouter } from "./routes/v2/sgidAuth" +import AuditLogsService from "./services/admin/AuditLogsService" import RepoManagementService from "./services/admin/RepoManagementService" import GitFileCommitService from "./services/db/GitFileCommitService" import GitFileSystemService from "./services/db/GitFileSystemService" @@ -88,8 +90,8 @@ import CollaboratorsService from "./services/identity/CollaboratorsService" import LaunchClient from "./services/identity/LaunchClient" import LaunchesService from "./services/identity/LaunchesService" import DynamoDBDocClient from "./services/infra/DynamoDBClient" -import ReviewCommentService from "./services/review/ReviewCommentService" import RepoCheckerService from "./services/review/RepoCheckerService" +import ReviewCommentService from "./services/review/ReviewCommentService" import { rateLimiter } from "./services/utilServices/RateLimiter" import SgidAuthService from "./services/utilServices/SgidAuthService" import { isSecure } from "./utils/auth-utils" @@ -315,6 +317,14 @@ const repoCheckerService = new RepoCheckerService({ git: simpleGitInstance, }) +const auditLogsService = new AuditLogsService({ + collaboratorsService, + isomerAdminsService, + reviewRequestService, + sitesService, + usersService, +}) + // poller site launch updates infraService.pollMessages() @@ -399,6 +409,10 @@ const formsgSiteCheckerRouter = new FormsgSiteCheckerRouter({ repoCheckerService, }) +const formsgSiteAuditLogsRouter = new FormsgSiteAuditLogsRouter({ + auditLogsService, +}) + const app = express() if (isSecure) { @@ -440,6 +454,7 @@ app.use("/formsg", formsgSiteCreateRouter.getRouter()) app.use("/formsg", formsgSiteLaunchRouter.getRouter()) app.use("/formsg", formsgGGsRepairRouter.getRouter()) app.use("/formsg", formsgSiteCheckerRouter.getRouter()) +app.use("/formsg", formsgSiteAuditLogsRouter.getRouter()) // catch unknown routes app.use((req, res, next) => { diff --git a/src/services/admin/AuditLogsService.ts b/src/services/admin/AuditLogsService.ts new file mode 100644 index 000000000..a0ccf358d --- /dev/null +++ b/src/services/admin/AuditLogsService.ts @@ -0,0 +1,500 @@ +import fs from "fs" +import path from "path" + +import { Octokit } from "@octokit/rest" +import { ResultAsync, errAsync, okAsync } from "neverthrow" + +import UserWithSiteSessionData from "@root/classes/UserWithSiteSessionData" +import { CollaboratorRoles, EFS_VOL_PATH_AUDIT_LOGS } from "@root/constants" +import AuditLogsError from "@root/errors/AuditLogsError" +import DatabaseError from "@root/errors/DatabaseError" +import { ForbiddenError } from "@root/errors/ForbiddenError" +import logger from "@root/logger/logger" +import { AuditLog, AuditableActivity } from "@root/types/auditLog" +import { IsomerCommitMessage } from "@root/types/github" +import { tokenServiceInstance } from "@services/db/TokenService" +import CollaboratorsService from "@services/identity/CollaboratorsService" +import IsomerAdminsService from "@services/identity/IsomerAdminsService" +import SitesService from "@services/identity/SitesService" +import UsersService from "@services/identity/UsersService" +import ReviewRequestService from "@services/review/ReviewRequestService" +import { mailer } from "@services/utilServices/MailClient" + +interface AuditLogsServiceProps { + collaboratorsService: CollaboratorsService + isomerAdminsService: IsomerAdminsService + reviewRequestService: ReviewRequestService + sitesService: SitesService + usersService: UsersService +} + +class AuditLogsService { + private readonly collaboratorsService: AuditLogsServiceProps["collaboratorsService"] + + private readonly isomerAdminsService: AuditLogsServiceProps["isomerAdminsService"] + + private readonly reviewRequestService: AuditLogsServiceProps["reviewRequestService"] + + private readonly sitesService: AuditLogsServiceProps["sitesService"] + + private readonly usersService: AuditLogsServiceProps["usersService"] + + constructor({ + collaboratorsService, + isomerAdminsService, + reviewRequestService, + sitesService, + usersService, + }: AuditLogsServiceProps) { + this.collaboratorsService = collaboratorsService + this.isomerAdminsService = isomerAdminsService + this.reviewRequestService = reviewRequestService + this.sitesService = sitesService + this.usersService = usersService + } + + getAuditLogActorNameFromId(userId: string) { + return ResultAsync.fromPromise( + this.isomerAdminsService.isUserIsomerAdmin(userId), + (error) => { + logger.error( + `Error getting user's Isomer admin status from the database: ${error}` + ) + + return new DatabaseError( + "Error getting user's Isomer admin status from the database" + ) + } + ) + .andThen((isIsomerAdmin) => { + if (isIsomerAdmin) { + return okAsync("Isomer Admin") + } + + return errAsync(false) + }) + .orElse(() => + ResultAsync.fromPromise(this.usersService.findById(userId), (error) => { + logger.error( + `Error getting user data from the database using the user ID (${userId}): ${error}` + ) + + return new DatabaseError( + "Error getting user data from the database using the user ID" + ) + }).andThen((user) => { + if (user && user.email) { + return okAsync(user.email) + } + + return errAsync(false) + }) + ) + } + + getAuditLogActorNameFromGitHubId(gitHubId: string) { + return ResultAsync.fromPromise( + this.usersService.findByGitHubId(gitHubId), + (error) => { + logger.error( + `Error getting user data from the database using the GitHub ID (${gitHubId}): ${error}` + ) + + return new DatabaseError( + "Error getting user data from the database using the GitHub ID" + ) + } + ) + .andThen((user) => { + if (!user) { + return errAsync(false) + } + + return ResultAsync.combine([ + okAsync(user), + ResultAsync.fromPromise( + this.isomerAdminsService.isUserIsomerAdmin(user.id.toString()), + (error) => { + logger.error( + `Error getting user's Isomer admin status from the database: ${error}` + ) + + return new DatabaseError( + "Error getting user's Isomer admin status from the database" + ) + } + ), + ]) + }) + .andThen(([user, isIsomerAdmin]) => { + if (isIsomerAdmin) { + return okAsync("Isomer Admin") + } + + if (user.email) { + return okAsync(user.email) + } + + return errAsync(false) + }) + } + + getAuditLogs( + sessionData: UserWithSiteSessionData, + sinceDate: string = new Date("1970-01-01").toISOString(), + untilDate: string = new Date().toISOString() + ): ResultAsync { + const octokit = new Octokit({ + auth: sessionData.accessToken, + }) + + logger.info( + `Getting site audit logs for site ${sessionData.siteName}, from ${sinceDate} to ${untilDate}` + ) + + return ResultAsync.fromPromise( + octokit.paginate(octokit.repos.listCommits, { + owner: "isomerpages", + repo: sessionData.siteName, + since: sinceDate, + until: untilDate, + per_page: 100, + }), + (error) => { + logger.error( + `Error occurred when getting the list of commits for the site ${ + sessionData.siteName + } from GitHub: ${JSON.stringify(error)}` + ) + + return new AuditLogsError( + `Error occurred when getting the list of commits for the site ${sessionData.siteName}` + ) + } + ) + .andThen((commits) => + ResultAsync.combine( + commits + .filter( + (commit) => + // Note: We can ignore merge commits since we are on the staging branch + !commit.commit.message.startsWith("Merge ") + ) + .map((commit) => { + const { message } = commit.commit + + if (message.startsWith("{")) { + try { + const parsedMessage: IsomerCommitMessage = JSON.parse(message) + + return this.getAuditLogActorNameFromId(parsedMessage.userId) + .orElse((error) => { + if (typeof error === "boolean") { + return okAsync( + commit.commit.author?.name || "Unknown author" + ) + } + + return errAsync(error) + }) + .andThen((actorEmail) => + okAsync({ + timestamp: new Date(commit.commit.author?.date || ""), + activity: AuditableActivity.SavedChanges, + actor: actorEmail, + page: parsedMessage.fileName || "", + remarks: parsedMessage.message, + }) + ) + } catch (error) { + logger.error( + `Error parsing JSON in commit ${commit.sha} from ${sessionData.siteName}: ${error}\n` + ) + return errAsync( + new AuditLogsError("Error parsing JSON in commit") + ) + } + } + + return this.getAuditLogActorNameFromGitHubId( + commit.commit.author?.name ?? "" + ) + .orElse((error) => { + if (typeof error === "boolean") { + return okAsync( + commit.commit.author?.name || "Unknown author" + ) + } + + return errAsync(error) + }) + .andThen((actorEmail) => + okAsync({ + timestamp: new Date(commit.commit.author?.date || ""), + activity: AuditableActivity.SavedChanges, + actor: actorEmail, + page: "", + remarks: message, + }) + ) + }) + ) + ) + .andThen((auditLogs) => + ResultAsync.fromPromise( + octokit.paginate(octokit.pulls.list, { + owner: "isomerpages", + repo: sessionData.siteName, + since: sinceDate, + until: untilDate, + per_page: 100, + state: "closed", + base: "master", + head: "staging", + }), + (error) => { + logger.error( + `Error occurred when getting the list of pull requests for the site ${ + sessionData.siteName + } from GitHub: ${JSON.stringify(error)}` + ) + + return new AuditLogsError( + `Error occurred when getting the list of pull requests for the site ${sessionData.siteName}` + ) + } + ) + .andThen((pulls) => okAsync(pulls.filter((pull) => pull.merged_at))) + .andThen((pulls) => + this.sitesService + .getBySiteName(sessionData.siteName) + .andThen((site) => + ResultAsync.combine( + pulls.map((pull) => + ResultAsync.fromPromise( + this.reviewRequestService.getReviewRequest( + site, + pull.number + ), + (error) => { + logger.error( + `Error occurred while retrieving review request data from the database for pull request ${pull.number} of site ${sessionData.siteName}: ${error}` + ) + return new DatabaseError( + "Error occurred while retrieving review request data from the database" + ) + } + ).map((reviewRequest) => ({ + timestamp: new Date(pull.merged_at || ""), + activity: AuditableActivity.PublishedChanges, + actor: + "requestor" in reviewRequest && + reviewRequest.requestor.email + ? reviewRequest.requestor.email + : pull.user?.login || "Unknown user", + page: "", + remarks: `GitHub Pull Request ID #${pull.number}`, + })) + ) + ) + ) + ) + .andThen((publishedChanges) => + okAsync( + publishedChanges + .concat(auditLogs) + .sort((a, b) => a.timestamp.getTime() - b.timestamp.getTime()) + ) + ) + ) + } + + getAuditLogsViaFormsg( + email: string, + repoNames: string[], + sinceDate: string, + untilDate: string, + formSubmissionId: string + ): ResultAsync { + const emailSubject = `[Isomer] Site Audit Logs (submission ID: ${formSubmissionId})` + logger.info( + `Received request to get audit logs, submission ID: ${formSubmissionId}` + ) + + // Step 1: Check if the user exists + return ResultAsync.fromPromise(this.usersService.findByEmail(email), () => { + logger.error( + `Error occurred while retrieving user data from the database for email ${email}` + ) + + return new DatabaseError( + "Error occurred while retrieving user data from the database" + ) + }) + .andThen((user) => { + if (!user) { + logger.warn(`Email address ${email} is not registered on Isomer CMS`) + + return errAsync( + new ForbiddenError("Email address is not registered on Isomer CMS") + ) + } + + return okAsync(user) + }) + .andThen((user) => + // Step 2: Check if the user is a collaborator of ALL given repos + ResultAsync.combine([ + okAsync(user), + ...repoNames.map((repoName) => + ResultAsync.fromPromise( + this.collaboratorsService.getRole(repoName, user.id.toString()), + () => + new DatabaseError( + `Error retrieving user's role for the repo ${repoName}` + ) + ) + ), + ]) + ) + .andThen(([user, ...roles]) => { + const isUserNotAdminCollaborator = roles.some( + (role) => + !role || + (role !== CollaboratorRoles.Admin && + role !== CollaboratorRoles.IsomerAdmin) + ) + + if (isUserNotAdminCollaborator) { + logger.warn( + `User ${email} is not an admin collaborator of all the given repos ${JSON.stringify( + repoNames + )}` + ) + + return errAsync( + new ForbiddenError( + "User is not an admin collaborator of all the given repos" + ) + ) + } + + return okAsync(user) + }) + .andThen((user) => + // Step 3: Obtain a token for the user + ResultAsync.combine([ + okAsync(user), + tokenServiceInstance.getAccessToken(), + ]) + ) + .andThen(([user, accessToken]) => + // Step 4: Construct a fake user session data object and get audit logs + ResultAsync.combine( + repoNames.map((repoName) => { + const userSessionData = new UserWithSiteSessionData({ + githubId: user.githubId, + accessToken, + isomerUserId: user.id.toString(), + email, + siteName: repoName, + }) + + return this.getAuditLogs(userSessionData, sinceDate, untilDate).map( + (auditLogs) => ({ + siteName: repoName, + auditLogs, + snapshotTime: new Date(), + }) + ) + }) + ) + ) + .andThen((auditLogDtos) => { + // Step 5: Prepare the audit log CSV files for each repo + const auditLogHeader = "Date,Time,Activity,User,Page,Remarks\n" + + return ResultAsync.combine( + auditLogDtos.map(({ siteName, auditLogs, snapshotTime }) => { + const csvContent = auditLogs.reduce( + (csv, { timestamp, activity, actor, page, remarks }) => { + const recordDate = timestamp.toISOString().split("T")[0] + const recordHour = timestamp + .getUTCHours() + .toString() + .padStart(2, "0") + const recordMinute = timestamp + .getUTCMinutes() + .toString() + .padStart(2, "0") + const recordSecond = timestamp + .getUTCSeconds() + .toString() + .padStart(2, "0") + const recordTime = `${recordHour}:${recordMinute}:${recordSecond}` + + return `${csv}${recordDate},${recordTime},${activity},"${actor}","${page}","${remarks}"\n` + }, + auditLogHeader + ) + const csvFileDate = snapshotTime.toISOString().split("T")[0] + const csvFileName = `audit-logs_${siteName}_${csvFileDate}_${formSubmissionId}.csv` + const csvFilePath = path.join(EFS_VOL_PATH_AUDIT_LOGS, csvFileName) + + return ResultAsync.fromPromise( + fs.promises.writeFile(csvFilePath, csvContent, "utf-8"), + (error) => { + logger.error( + `Error occurred while writing audit log CSV file for repo ${siteName}: ${JSON.stringify( + error + )}` + ) + return new AuditLogsError( + `Error occurred while writing audit log CSV file for repo ${siteName}` + ) + } + ).map(() => csvFilePath) + }) + ) + }) + .andThen((csvFilePaths) => { + // Step 6: Send the audit log CSV files to the user via email + const emailBody = `

Please find the attached audit log CSV files for the requested repos:

+
    + ${repoNames.map((repoName) => `
  • ${repoName}
    • `)} +
+

Isomer Support Team

` + + return ResultAsync.fromPromise( + mailer.sendMail(email, emailSubject, emailBody, csvFilePaths), + () => + new AuditLogsError( + `Error occurred while sending audit log CSV files to ${email} (submission ID: ${formSubmissionId}). Requested repos: ${repoNames.join( + ", " + )}` + ) + ) + }) + .orElse((error) => { + const emailBody = `

An error occurred when getting the audit log CSV files for the requested repos:

+
    + ${repoNames.map((repoName) => `
  • ${repoName}
    • `)} +
+

This was the error that was received:

+

${JSON.stringify(error)}

+

Isomer Support Team

` + + return ResultAsync.fromPromise( + mailer.sendMail(email, emailSubject, emailBody), + () => + new AuditLogsError( + `Error occurred while sending an error email to ${email} (submission ID: ${formSubmissionId}). Requested repos: ${repoNames.join( + ", " + )}` + ) + ) + }) + } +} + +export default AuditLogsService diff --git a/src/services/admin/__tests__/AuditLogsService.spec.ts b/src/services/admin/__tests__/AuditLogsService.spec.ts new file mode 100644 index 000000000..e69de29bb diff --git a/src/types/auditLog.ts b/src/types/auditLog.ts new file mode 100644 index 000000000..d5855de59 --- /dev/null +++ b/src/types/auditLog.ts @@ -0,0 +1,17 @@ +export enum AuditableActivity { + SavedChanges = "Saved Changes", + PublishedChanges = "Published Changes", + RequestedReview = "Requested Review", + ApprovedReview = "Approved Review", + CancelledReview = "Cancelled Review", + AddedCollaborator = "Added Collaborator", + RemovedCollaborator = "Removed Collaborator", +} + +export type AuditLog = { + timestamp: Date + activity: AuditableActivity + actor: string + page: string + remarks: string +} From e2e5945610095b97601db747de098c86d7afa52a Mon Sep 17 00:00:00 2001 From: dcshzj <27919917+dcshzj@users.noreply.github.com> Date: Mon, 4 Mar 2024 13:05:46 +0800 Subject: [PATCH 02/11] chore: add UTC identifier to timestamp --- src/services/admin/AuditLogsService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/admin/AuditLogsService.ts b/src/services/admin/AuditLogsService.ts index a0ccf358d..d07d50cbb 100644 --- a/src/services/admin/AuditLogsService.ts +++ b/src/services/admin/AuditLogsService.ts @@ -431,7 +431,7 @@ class AuditLogsService { .getUTCSeconds() .toString() .padStart(2, "0") - const recordTime = `${recordHour}:${recordMinute}:${recordSecond}` + const recordTime = `${recordHour}:${recordMinute}:${recordSecond} (UTC)` return `${csv}${recordDate},${recordTime},${activity},"${actor}","${page}","${remarks}"\n` }, From 40651faced6f13a89932e0ce9fd4b8e391348b6d Mon Sep 17 00:00:00 2001 From: dcshzj <27919917+dcshzj@users.noreply.github.com> Date: Tue, 5 Mar 2024 10:29:41 +0800 Subject: [PATCH 03/11] chore: remove empty AuditLogsService test --- src/services/admin/__tests__/AuditLogsService.spec.ts | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 src/services/admin/__tests__/AuditLogsService.spec.ts diff --git a/src/services/admin/__tests__/AuditLogsService.spec.ts b/src/services/admin/__tests__/AuditLogsService.spec.ts deleted file mode 100644 index e69de29bb..000000000 From 6a46e291197690df3fc3825f46e0efc311bab2be Mon Sep 17 00:00:00 2001 From: dcshzj <27919917+dcshzj@users.noreply.github.com> Date: Tue, 5 Mar 2024 10:39:44 +0800 Subject: [PATCH 04/11] fix: define types for error state --- src/services/admin/AuditLogsService.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/services/admin/AuditLogsService.ts b/src/services/admin/AuditLogsService.ts index d07d50cbb..4843b08bf 100644 --- a/src/services/admin/AuditLogsService.ts +++ b/src/services/admin/AuditLogsService.ts @@ -6,6 +6,7 @@ import { ResultAsync, errAsync, okAsync } from "neverthrow" import UserWithSiteSessionData from "@root/classes/UserWithSiteSessionData" import { CollaboratorRoles, EFS_VOL_PATH_AUDIT_LOGS } from "@root/constants" +import { User } from "@root/database/models/User" import AuditLogsError from "@root/errors/AuditLogsError" import DatabaseError from "@root/errors/DatabaseError" import { ForbiddenError } from "@root/errors/ForbiddenError" @@ -345,7 +346,7 @@ class AuditLogsService { .andThen((user) => // Step 2: Check if the user is a collaborator of ALL given repos ResultAsync.combine([ - okAsync(user), + okAsync(user), ...repoNames.map((repoName) => ResultAsync.fromPromise( this.collaboratorsService.getRole(repoName, user.id.toString()), From 06dbe93cdd89d76d2ed8be5280d966bbdf2a47db Mon Sep 17 00:00:00 2001 From: dcshzj <27919917+dcshzj@users.noreply.github.com> Date: Tue, 5 Mar 2024 10:49:58 +0800 Subject: [PATCH 05/11] chore: avoid using enums --- src/services/admin/AuditLogsService.ts | 12 ++++++------ src/types/auditLog.ts | 22 ++++++++++++---------- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/services/admin/AuditLogsService.ts b/src/services/admin/AuditLogsService.ts index 4843b08bf..34a51108f 100644 --- a/src/services/admin/AuditLogsService.ts +++ b/src/services/admin/AuditLogsService.ts @@ -11,7 +11,7 @@ import AuditLogsError from "@root/errors/AuditLogsError" import DatabaseError from "@root/errors/DatabaseError" import { ForbiddenError } from "@root/errors/ForbiddenError" import logger from "@root/logger/logger" -import { AuditLog, AuditableActivity } from "@root/types/auditLog" +import { AuditLog, AuditableActivityNames } from "@root/types/auditLog" import { IsomerCommitMessage } from "@root/types/github" import { tokenServiceInstance } from "@services/db/TokenService" import CollaboratorsService from "@services/identity/CollaboratorsService" @@ -201,7 +201,7 @@ class AuditLogsService { .andThen((actorEmail) => okAsync({ timestamp: new Date(commit.commit.author?.date || ""), - activity: AuditableActivity.SavedChanges, + activity: AuditableActivityNames.SavedChanges, actor: actorEmail, page: parsedMessage.fileName || "", remarks: parsedMessage.message, @@ -230,9 +230,9 @@ class AuditLogsService { return errAsync(error) }) .andThen((actorEmail) => - okAsync({ + okAsync({ timestamp: new Date(commit.commit.author?.date || ""), - activity: AuditableActivity.SavedChanges, + activity: AuditableActivityNames.SavedChanges, actor: actorEmail, page: "", remarks: message, @@ -285,9 +285,9 @@ class AuditLogsService { "Error occurred while retrieving review request data from the database" ) } - ).map((reviewRequest) => ({ + ).map((reviewRequest) => ({ timestamp: new Date(pull.merged_at || ""), - activity: AuditableActivity.PublishedChanges, + activity: AuditableActivityNames.PublishedChanges, actor: "requestor" in reviewRequest && reviewRequest.requestor.email diff --git a/src/types/auditLog.ts b/src/types/auditLog.ts index d5855de59..790509d38 100644 --- a/src/types/auditLog.ts +++ b/src/types/auditLog.ts @@ -1,16 +1,18 @@ -export enum AuditableActivity { - SavedChanges = "Saved Changes", - PublishedChanges = "Published Changes", - RequestedReview = "Requested Review", - ApprovedReview = "Approved Review", - CancelledReview = "Cancelled Review", - AddedCollaborator = "Added Collaborator", - RemovedCollaborator = "Removed Collaborator", -} +export const AuditableActivityNames = { + SavedChanges: "Saved Changes", + PublishedChanges: "Published Changes", + RequestedReview: "Requested Review", + ApprovedReview: "Approved Review", + CancelledReview: "Cancelled Review", + AddedCollaborator: "Added Collaborator", + RemovedCollaborator: "Removed Collaborator", +} as const + +export type AuditableActivity = keyof typeof AuditableActivityNames export type AuditLog = { timestamp: Date - activity: AuditableActivity + activity: typeof AuditableActivityNames[AuditableActivity] actor: string page: string remarks: string From 7808d66cb12ce2b739de197649aa016a3f68edab Mon Sep 17 00:00:00 2001 From: dcshzj <27919917+dcshzj@users.noreply.github.com> Date: Tue, 5 Mar 2024 12:29:47 +0800 Subject: [PATCH 06/11] fix: use moment to specify start and end of day --- src/services/admin/AuditLogsService.ts | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/services/admin/AuditLogsService.ts b/src/services/admin/AuditLogsService.ts index 34a51108f..e7b298f34 100644 --- a/src/services/admin/AuditLogsService.ts +++ b/src/services/admin/AuditLogsService.ts @@ -2,6 +2,7 @@ import fs from "fs" import path from "path" import { Octokit } from "@octokit/rest" +import moment from "moment-timezone" import { ResultAsync, errAsync, okAsync } from "neverthrow" import UserWithSiteSessionData from "@root/classes/UserWithSiteSessionData" @@ -157,8 +158,8 @@ class AuditLogsService { octokit.paginate(octokit.repos.listCommits, { owner: "isomerpages", repo: sessionData.siteName, - since: sinceDate, - until: untilDate, + since: moment(sinceDate).startOf("day").toISOString(), + until: moment(untilDate).endOf("day").toISOString(), per_page: 100, }), (error) => { @@ -246,8 +247,6 @@ class AuditLogsService { octokit.paginate(octokit.pulls.list, { owner: "isomerpages", repo: sessionData.siteName, - since: sinceDate, - until: untilDate, per_page: 100, state: "closed", base: "master", @@ -265,7 +264,16 @@ class AuditLogsService { ) } ) - .andThen((pulls) => okAsync(pulls.filter((pull) => pull.merged_at))) + .andThen((pulls) => + okAsync( + pulls.filter( + (pull) => + pull.merged_at && + moment(sinceDate).startOf("day").isBefore(pull.merged_at) && + moment(untilDate).endOf("day").isAfter(pull.merged_at) + ) + ) + ) .andThen((pulls) => this.sitesService .getBySiteName(sessionData.siteName) From 6099a98e85cc5ac42377e4e649d6727234a1cb33 Mon Sep 17 00:00:00 2001 From: dcshzj <27919917+dcshzj@users.noreply.github.com> Date: Tue, 5 Mar 2024 13:52:03 +0800 Subject: [PATCH 07/11] feat: add ability to get RR creation and approval info --- src/server.js | 1 + src/services/admin/AuditLogsService.ts | 96 ++++++++++++++----- src/services/identity/NotificationsService.ts | 44 +++++++++ 3 files changed, 116 insertions(+), 25 deletions(-) diff --git a/src/server.js b/src/server.js index a5db7531f..27daeb279 100644 --- a/src/server.js +++ b/src/server.js @@ -320,6 +320,7 @@ const repoCheckerService = new RepoCheckerService({ const auditLogsService = new AuditLogsService({ collaboratorsService, isomerAdminsService, + notificationsService, reviewRequestService, sitesService, usersService, diff --git a/src/services/admin/AuditLogsService.ts b/src/services/admin/AuditLogsService.ts index e7b298f34..cdc46b907 100644 --- a/src/services/admin/AuditLogsService.ts +++ b/src/services/admin/AuditLogsService.ts @@ -17,6 +17,7 @@ import { IsomerCommitMessage } from "@root/types/github" import { tokenServiceInstance } from "@services/db/TokenService" import CollaboratorsService from "@services/identity/CollaboratorsService" import IsomerAdminsService from "@services/identity/IsomerAdminsService" +import NotificationsService from "@services/identity/NotificationsService" import SitesService from "@services/identity/SitesService" import UsersService from "@services/identity/UsersService" import ReviewRequestService from "@services/review/ReviewRequestService" @@ -25,6 +26,7 @@ import { mailer } from "@services/utilServices/MailClient" interface AuditLogsServiceProps { collaboratorsService: CollaboratorsService isomerAdminsService: IsomerAdminsService + notificationsService: NotificationsService reviewRequestService: ReviewRequestService sitesService: SitesService usersService: UsersService @@ -35,6 +37,8 @@ class AuditLogsService { private readonly isomerAdminsService: AuditLogsServiceProps["isomerAdminsService"] + private readonly notificationsService: AuditLogsServiceProps["notificationsService"] + private readonly reviewRequestService: AuditLogsServiceProps["reviewRequestService"] private readonly sitesService: AuditLogsServiceProps["sitesService"] @@ -44,12 +48,14 @@ class AuditLogsService { constructor({ collaboratorsService, isomerAdminsService, + notificationsService, reviewRequestService, sitesService, usersService, }: AuditLogsServiceProps) { this.collaboratorsService = collaboratorsService this.isomerAdminsService = isomerAdminsService + this.notificationsService = notificationsService this.reviewRequestService = reviewRequestService this.sitesService = sitesService this.usersService = usersService @@ -279,33 +285,73 @@ class AuditLogsService { .getBySiteName(sessionData.siteName) .andThen((site) => ResultAsync.combine( - pulls.map((pull) => - ResultAsync.fromPromise( - this.reviewRequestService.getReviewRequest( - site, - pull.number + pulls.flatMap((pull) => + ResultAsync.combine([ + ResultAsync.fromPromise( + this.reviewRequestService.getReviewRequest( + site, + pull.number + ), + (error) => { + logger.error( + `Error occurred while retrieving review request data from the database for pull request ${pull.number} of site ${sessionData.siteName}: ${error}` + ) + return new DatabaseError( + "Error occurred while retrieving review request data from the database" + ) + } ), - (error) => { - logger.error( - `Error occurred while retrieving review request data from the database for pull request ${pull.number} of site ${sessionData.siteName}: ${error}` + this.notificationsService.findAllForSite({ + siteName: sessionData.siteName, + }), + ]).map(([reviewRequest, notifications]) => [ + // When pull/review request is published/merged + { + timestamp: new Date(pull.merged_at || ""), + activity: AuditableActivityNames.PublishedChanges, + actor: + "requestor" in reviewRequest && + reviewRequest.requestor.email + ? reviewRequest.requestor.email + : pull.user?.login || "Unknown user", + page: "", + remarks: `GitHub Pull Request ID #${pull.number}`, + }, + + // When review request is created + ...notifications + .filter( + (notification) => + notification.link.endsWith(`/${pull.number}`) && + notification.type === "request_created" ) - return new DatabaseError( - "Error occurred while retrieving review request data from the database" + .map((notification) => ({ + timestamp: notification.createdAt, + activity: AuditableActivityNames.RequestedReview, + actor: notification.sourceUsername, + page: "", + remarks: `GitHub Pull Request ID #${pull.number}`, + })) + .slice(0, 1), + + // When review request is approved + ...notifications + .filter( + (notification) => + notification.link.endsWith(`/${pull.number}`) && + notification.type === "request_approved" ) - } - ).map((reviewRequest) => ({ - timestamp: new Date(pull.merged_at || ""), - activity: AuditableActivityNames.PublishedChanges, - actor: - "requestor" in reviewRequest && - reviewRequest.requestor.email - ? reviewRequest.requestor.email - : pull.user?.login || "Unknown user", - page: "", - remarks: `GitHub Pull Request ID #${pull.number}`, - })) + .map((notification) => ({ + timestamp: notification.createdAt, + activity: AuditableActivityNames.ApprovedReview, + actor: notification.sourceUsername, + page: "", + remarks: `GitHub Pull Request ID #${pull.number}`, + })) + .slice(0, 1), + ]) ) - ) + ).map((pullsAuditLogs) => pullsAuditLogs.flat()) ) ) .andThen((publishedChanges) => @@ -421,7 +467,7 @@ class AuditLogsService { ) .andThen((auditLogDtos) => { // Step 5: Prepare the audit log CSV files for each repo - const auditLogHeader = "Date,Time,Activity,User,Page,Remarks\n" + const auditLogHeader = "Date,Time (UTC),Activity,User,Page,Remarks\n" return ResultAsync.combine( auditLogDtos.map(({ siteName, auditLogs, snapshotTime }) => { @@ -440,7 +486,7 @@ class AuditLogsService { .getUTCSeconds() .toString() .padStart(2, "0") - const recordTime = `${recordHour}:${recordMinute}:${recordSecond} (UTC)` + const recordTime = `${recordHour}:${recordMinute}:${recordSecond}` return `${csv}${recordDate},${recordTime},${activity},"${actor}","${page}","${remarks}"\n` }, diff --git a/src/services/identity/NotificationsService.ts b/src/services/identity/NotificationsService.ts index fcbde7221..fa4261c4c 100644 --- a/src/services/identity/NotificationsService.ts +++ b/src/services/identity/NotificationsService.ts @@ -1,6 +1,9 @@ +import { ResultAsync } from "neverthrow" import { FindOptions, ModelStatic, Op, Sequelize } from "sequelize" import { Notification, Site, Repo, SiteMember } from "@database/models" +import DatabaseError from "@root/errors/DatabaseError" +import logger from "@root/logger/logger" import { NotificationType, getNotificationExpiryDate, @@ -47,6 +50,47 @@ class NotificationsService { })) } + findAllForSite({ + siteName, + findOptions, + }: { + siteName: string + findOptions?: FindOptions + }): ResultAsync { + return ResultAsync.fromPromise( + this.repository.findAll({ + include: [ + { + model: Site, + as: "site", + required: true, + include: [ + { + model: Repo, + required: true, + where: { + name: siteName, + }, + }, + ], + }, + ], + ...findOptions, + }), + (error) => { + logger.error( + `Error finding notifications for site ${siteName}: ${JSON.stringify( + error + )}` + ) + + return new DatabaseError( + `Error finding notifications for site ${siteName}` + ) + } + ) + } + async findAll({ siteName, userId, From c9f56b7caad419be32ca53b53bacea6db4d09908 Mon Sep 17 00:00:00 2001 From: dcshzj <27919917+dcshzj@users.noreply.github.com> Date: Wed, 6 Mar 2024 10:00:17 +0800 Subject: [PATCH 08/11] fix: return status 400 if fields are missing --- src/routes/formsg/formsgSiteAuditLogs.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/routes/formsg/formsgSiteAuditLogs.ts b/src/routes/formsg/formsgSiteAuditLogs.ts index 7b76fb934..a1aae3af1 100644 --- a/src/routes/formsg/formsgSiteAuditLogs.ts +++ b/src/routes/formsg/formsgSiteAuditLogs.ts @@ -48,7 +48,7 @@ export class FormsgSiteAuditLogsRouter { logger.error( "No requester email was provided in site audit logs form submission" ) - return + return res.sendStatus(400) } const repoNamesFromTable = getFieldsFromTable(responses, REPO_NAME_FIELD) @@ -57,7 +57,7 @@ export class FormsgSiteAuditLogsRouter { logger.error( "No repo names were provided in site audit logs form submission" ) - return + return res.sendStatus(400) } repoNamesFromTable.forEach((repoName) => { From 85d9b84f4f7a3953776fec96f23fba80f6ef790f Mon Sep 17 00:00:00 2001 From: dcshzj <27919917+dcshzj@users.noreply.github.com> Date: Wed, 6 Mar 2024 10:01:03 +0800 Subject: [PATCH 09/11] chore: switch to use papaparse instead --- src/services/admin/AuditLogsService.ts | 28 +++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/src/services/admin/AuditLogsService.ts b/src/services/admin/AuditLogsService.ts index cdc46b907..340cc20d2 100644 --- a/src/services/admin/AuditLogsService.ts +++ b/src/services/admin/AuditLogsService.ts @@ -4,6 +4,7 @@ import path from "path" import { Octokit } from "@octokit/rest" import moment from "moment-timezone" import { ResultAsync, errAsync, okAsync } from "neverthrow" +import Papa from "papaparse" import UserWithSiteSessionData from "@root/classes/UserWithSiteSessionData" import { CollaboratorRoles, EFS_VOL_PATH_AUDIT_LOGS } from "@root/constants" @@ -467,12 +468,19 @@ class AuditLogsService { ) .andThen((auditLogDtos) => { // Step 5: Prepare the audit log CSV files for each repo - const auditLogHeader = "Date,Time (UTC),Activity,User,Page,Remarks\n" + const auditLogHeaders = [ + "Date", + "Time (UTC)", + "Activity", + "User", + "Page", + "Remarks", + ] return ResultAsync.combine( auditLogDtos.map(({ siteName, auditLogs, snapshotTime }) => { - const csvContent = auditLogs.reduce( - (csv, { timestamp, activity, actor, page, remarks }) => { + const csvContent = auditLogs.map( + ({ timestamp, activity, actor, page, remarks }) => { const recordDate = timestamp.toISOString().split("T")[0] const recordHour = timestamp .getUTCHours() @@ -488,16 +496,22 @@ class AuditLogsService { .padStart(2, "0") const recordTime = `${recordHour}:${recordMinute}:${recordSecond}` - return `${csv}${recordDate},${recordTime},${activity},"${actor}","${page}","${remarks}"\n` - }, - auditLogHeader + return [recordDate, recordTime, activity, actor, page, remarks] + } ) const csvFileDate = snapshotTime.toISOString().split("T")[0] const csvFileName = `audit-logs_${siteName}_${csvFileDate}_${formSubmissionId}.csv` const csvFilePath = path.join(EFS_VOL_PATH_AUDIT_LOGS, csvFileName) return ResultAsync.fromPromise( - fs.promises.writeFile(csvFilePath, csvContent, "utf-8"), + fs.promises.writeFile( + csvFilePath, + Papa.unparse({ + fields: auditLogHeaders, + data: csvContent, + }), + "utf-8" + ), (error) => { logger.error( `Error occurred while writing audit log CSV file for repo ${siteName}: ${JSON.stringify( From 24658ec51af72594c7da2c6368ebc0f7be5d5c52 Mon Sep 17 00:00:00 2001 From: dcshzj <27919917+dcshzj@users.noreply.github.com> Date: Wed, 6 Mar 2024 10:01:29 +0800 Subject: [PATCH 10/11] chore: add common prefix to all error logs --- src/services/admin/AuditLogsService.ts | 55 ++++++++++++++++---------- 1 file changed, 35 insertions(+), 20 deletions(-) diff --git a/src/services/admin/AuditLogsService.ts b/src/services/admin/AuditLogsService.ts index 340cc20d2..53edf7559 100644 --- a/src/services/admin/AuditLogsService.ts +++ b/src/services/admin/AuditLogsService.ts @@ -67,11 +67,13 @@ class AuditLogsService { this.isomerAdminsService.isUserIsomerAdmin(userId), (error) => { logger.error( - `Error getting user's Isomer admin status from the database: ${error}` + `Site audit log error: Unable to get user's Isomer admin status from the database: ${JSON.stringify( + error + )}` ) return new DatabaseError( - "Error getting user's Isomer admin status from the database" + "Error getting user's permissions from the database" ) } ) @@ -85,7 +87,9 @@ class AuditLogsService { .orElse(() => ResultAsync.fromPromise(this.usersService.findById(userId), (error) => { logger.error( - `Error getting user data from the database using the user ID (${userId}): ${error}` + `Site audit log error: Unable to get user data from the database using the user ID (${userId}): ${JSON.stringify( + error + )}` ) return new DatabaseError( @@ -106,7 +110,9 @@ class AuditLogsService { this.usersService.findByGitHubId(gitHubId), (error) => { logger.error( - `Error getting user data from the database using the GitHub ID (${gitHubId}): ${error}` + `Site audit log error: Unable to get user data from the database using the GitHub ID (${gitHubId}): ${JSON.stringify( + error + )}` ) return new DatabaseError( @@ -125,11 +131,13 @@ class AuditLogsService { this.isomerAdminsService.isUserIsomerAdmin(user.id.toString()), (error) => { logger.error( - `Error getting user's Isomer admin status from the database: ${error}` + `Site audit log error: Unable to get user's Isomer admin status from the database: ${JSON.stringify( + error + )}` ) return new DatabaseError( - "Error getting user's Isomer admin status from the database" + "Error getting user's permissions from the database" ) } ), @@ -171,7 +179,7 @@ class AuditLogsService { }), (error) => { logger.error( - `Error occurred when getting the list of commits for the site ${ + `Site audit log error: Unable to get the list of commits for the site ${ sessionData.siteName } from GitHub: ${JSON.stringify(error)}` ) @@ -217,7 +225,9 @@ class AuditLogsService { ) } catch (error) { logger.error( - `Error parsing JSON in commit ${commit.sha} from ${sessionData.siteName}: ${error}\n` + `Site audit log error: Unable to parse JSON in commit ${ + commit.sha + } from ${sessionData.siteName}: ${JSON.stringify(error)}\n` ) return errAsync( new AuditLogsError("Error parsing JSON in commit") @@ -261,7 +271,7 @@ class AuditLogsService { }), (error) => { logger.error( - `Error occurred when getting the list of pull requests for the site ${ + `Site audit log error: Unable to get the list of pull requests for the site ${ sessionData.siteName } from GitHub: ${JSON.stringify(error)}` ) @@ -295,7 +305,7 @@ class AuditLogsService { ), (error) => { logger.error( - `Error occurred while retrieving review request data from the database for pull request ${pull.number} of site ${sessionData.siteName}: ${error}` + `Site audit log error: Unable to retrieve review request data from the database for pull request ${pull.number} of site ${sessionData.siteName}: ${error}` ) return new DatabaseError( "Error occurred while retrieving review request data from the database" @@ -378,15 +388,20 @@ class AuditLogsService { ) // Step 1: Check if the user exists - return ResultAsync.fromPromise(this.usersService.findByEmail(email), () => { - logger.error( - `Error occurred while retrieving user data from the database for email ${email}` - ) + return ResultAsync.fromPromise( + this.usersService.findByEmail(email), + (error) => { + logger.error( + `Site audit log error: Unable to retrieve user data from the database for email ${email}: ${JSON.stringify( + error + )}` + ) - return new DatabaseError( - "Error occurred while retrieving user data from the database" - ) - }) + return new DatabaseError( + "Error occurred while retrieving user data from the database" + ) + } + ) .andThen((user) => { if (!user) { logger.warn(`Email address ${email} is not registered on Isomer CMS`) @@ -514,12 +529,12 @@ class AuditLogsService { ), (error) => { logger.error( - `Error occurred while writing audit log CSV file for repo ${siteName}: ${JSON.stringify( + `Site audit log error: Unable to write audit log CSV file for repo ${siteName}: ${JSON.stringify( error )}` ) return new AuditLogsError( - `Error occurred while writing audit log CSV file for repo ${siteName}` + `Unable to write audit log CSV file for repo ${siteName}` ) } ).map(() => csvFilePath) From ae8410a1d315b913370a8333c2427fd174c9c067 Mon Sep 17 00:00:00 2001 From: dcshzj <27919917+dcshzj@users.noreply.github.com> Date: Wed, 6 Mar 2024 16:05:42 +0800 Subject: [PATCH 11/11] chore: use a fake GitHub user ID --- src/services/admin/AuditLogsService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/admin/AuditLogsService.ts b/src/services/admin/AuditLogsService.ts index 53edf7559..2394fe3ae 100644 --- a/src/services/admin/AuditLogsService.ts +++ b/src/services/admin/AuditLogsService.ts @@ -464,7 +464,7 @@ class AuditLogsService { ResultAsync.combine( repoNames.map((repoName) => { const userSessionData = new UserWithSiteSessionData({ - githubId: user.githubId, + githubId: "isomeradmin", // Fake GitHub ID, no real need for this here accessToken, isomerUserId: user.id.toString(), email,