From f4f4571dab74c098ed00f1f9824ad53468e03c1a Mon Sep 17 00:00:00 2001 From: Kamakshee Samant Date: Mon, 20 Nov 2023 14:55:27 +1100 Subject: [PATCH 1/5] chore: audit log service --- .env.development | 2 + .env.e2e | 2 + .env.test | 2 + .localstack/dynamodb.sh | 21 +++++- docker-compose.yml | 2 + github-for-jira.sd.yml | 23 +++++++ src/config/env.ts | 2 + src/services/audit-log-service.test.ts | 75 ++++++++++++++++++++++ src/services/audit-log-service.ts | 89 ++++++++++++++++++++++++++ 9 files changed, 217 insertions(+), 1 deletion(-) create mode 100644 src/services/audit-log-service.test.ts create mode 100644 src/services/audit-log-service.ts diff --git a/.env.development b/.env.development index 4c71ab24bc..2b753b0721 100644 --- a/.env.development +++ b/.env.development @@ -17,6 +17,8 @@ GLOBAL_HASH_SECRET=testsecret #Dyamno for deployment status DYNAMO_DEPLOYMENT_HISTORY_CACHE_TABLE_REGION=us-west-1 DYNAMO_DEPLOYMENT_HISTORY_CACHE_TABLE_NAME=deployment-history-cache +DYNAMO_AUDIT_LOG_TABLE_REGION=us-west-1 +DYNAMO_AUDIT_LOG_TABLE_NAME=audit_log # The Postgres URL used to connect to the database and secret for encrypting data DATABASE_URL=postgres://postgres:postgres@127.0.0.1:5432/jira-dev diff --git a/.env.e2e b/.env.e2e index 63d750e0b6..a9b3deae68 100644 --- a/.env.e2e +++ b/.env.e2e @@ -21,6 +21,8 @@ DEBUG=nock.* #Dyamno for deployment status DYNAMO_DEPLOYMENT_HISTORY_CACHE_TABLE_REGION=us-west-1 DYNAMO_DEPLOYMENT_HISTORY_CACHE_TABLE_NAME=deployment-history-cache +DYNAMO_AUDIT_LOG_TABLE_REGION=us-west-1 +DYNAMO_AUDIT_LOG_TABLE_NAME=audit_log MICROS_AWS_REGION=us-west-1 diff --git a/.env.test b/.env.test index d8b673a155..ded4a652a5 100644 --- a/.env.test +++ b/.env.test @@ -26,6 +26,8 @@ DEBUG=nock.* #Dyamno for deployment status DYNAMO_DEPLOYMENT_HISTORY_CACHE_TABLE_REGION=us-west-1 DYNAMO_DEPLOYMENT_HISTORY_CACHE_TABLE_NAME=deployment-history-cache +DYNAMO_AUDIT_LOG_TABLE_REGION=us-west-1 +DYNAMO_AUDIT_LOG_TABLE_NAME=audit_log MICROS_AWS_REGION=us-west-1 diff --git a/.localstack/dynamodb.sh b/.localstack/dynamodb.sh index 14bfc3a75c..2fb73a2ab0 100755 --- a/.localstack/dynamodb.sh +++ b/.localstack/dynamodb.sh @@ -1,5 +1,5 @@ #!/usr/bin/env bash - +# DEPLOYMENT_HISTORY_CACHE_TABLE echo "===== creating dynamo table ${DYNAMO_DEPLOYMENT_HISTORY_CACHE_TABLE_NAME} =====" awslocal dynamodb create-table \ @@ -15,8 +15,27 @@ awslocal dynamodb create-table \ ReadCapacityUnits=10,WriteCapacityUnits=5 echo "===== table ${DYNAMO_DEPLOYMENT_HISTORY_CACHE_TABLE_NAME} created =====" + +# AUDIT_LOG_TABLE +echo "===== creating dynamo table ${DYNAMO_AUDIT_LOG_TABLE_NAME} =====" + +awslocal dynamodb create-table \ + --table-name $DYNAMO_AUDIT_LOG_TABLE_NAME \ + --key-schema \ + AttributeName=Id,KeyType=HASH \ + AttributeName=CreatedAt,KeyType=RANGE \ + --attribute-definitions \ + AttributeName=Id,AttributeType=S \ + AttributeName=CreatedAt,AttributeType=N \ + --region $DYNAMO_AUDIT_LOG_TABLE_REGION \ + --provisioned-throughput \ + ReadCapacityUnits=10,WriteCapacityUnits=5 + +echo "===== table ${DYNAMO_AUDIT_LOG_TABLE_NAME} created =====" + echo "===== checking now =====" +awslocal dynamodb list-tables --region $DYNAMO_AUDIT_LOG_TABLE_REGION awslocal dynamodb list-tables --region $DYNAMO_DEPLOYMENT_HISTORY_CACHE_TABLE_REGION echo "===== check finished =====" diff --git a/docker-compose.yml b/docker-compose.yml index f89324fb97..1245f77ec3 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -31,7 +31,9 @@ services: environment: - DEFAULT_REGION=us-west-1 - DYNAMO_DEPLOYMENT_HISTORY_CACHE_TABLE_NAME=deployment-history-cache + - DYNAMO_AUDIT_LOG_TABLE_NAME=audit_log - DYNAMO_DEPLOYMENT_HISTORY_CACHE_TABLE_REGION=us-west-1 + - DYNAMO_AUDIT_LOG_TABLE_REGION=us-west-1 - LAMBDA_REMOTE_DOCKER=false - LAMBDA_EXECUTOR=local # runs lambda inside temp directory instead of new docker container - SQS_ENDPOINT_STRATEGY=off # sets the SQS queue domain/path to the legacy version diff --git a/github-for-jira.sd.yml b/github-for-jira.sd.yml index 51e6bd9082..9212a20957 100644 --- a/github-for-jira.sd.yml +++ b/github-for-jira.sd.yml @@ -180,6 +180,17 @@ resources: TTLAttributeName: ExpiredAfter dataType: - UGC/PrimaryIdentifier # Sha of a commit that point to user's code + - name: audit_log + type: dynamo-db + attributes: &table-attributes + HashKeyName: Id + HashKeyType: "S" + RangeKeyName: CreatedAt + RangeKeyType: "N" + ReadWriteCapacityMode: ON_DEMAND + TTLAttributeName: ExpiredAfter + dataType: + - UGC/PrimaryIdentifier # Sha of a commit that point to user's code scaling: instance: t2.small @@ -432,6 +443,10 @@ environmentOverrides: type: dynamo-db attributes: <<: *table-attributes + - name: audit_log + type: dynamo-db + attributes: + <<: *table-attributes alarms: overrides: @@ -535,6 +550,10 @@ environmentOverrides: type: dynamo-db attributes: <<: *table-attributes + - name: audit_log + type: dynamo-db + attributes: + <<: *table-attributes - type: globaledge name: proxy @@ -636,6 +655,10 @@ environmentOverrides: type: dynamo-db attributes: <<: *table-attributes + - name: audit_log + type: dynamo-db + attributes: + <<: *table-attributes - type: globaledge name: proxy diff --git a/src/config/env.ts b/src/config/env.ts index 664b97871e..4157be9c1c 100644 --- a/src/config/env.ts +++ b/src/config/env.ts @@ -141,6 +141,8 @@ export interface EnvVars { //DyamoDB for deployment status history DYNAMO_DEPLOYMENT_HISTORY_CACHE_TABLE_REGION: string; DYNAMO_DEPLOYMENT_HISTORY_CACHE_TABLE_NAME: string; + DYNAMO_AUDIT_LOG_TABLE_NAME: string; + DYNAMO_AUDIT_LOG_TABLE_REGION: string; // Micros Lifecycle Env Vars SNS_NOTIFICATION_LIFECYCLE_QUEUE_URL?: string; diff --git a/src/services/audit-log-service.test.ts b/src/services/audit-log-service.test.ts new file mode 100644 index 0000000000..3a4c73e318 --- /dev/null +++ b/src/services/audit-log-service.test.ts @@ -0,0 +1,75 @@ +import { getLogger } from "config/logger"; +import { envVars } from "config/env"; +// import { cacheSuccessfulDeploymentInfo, findLastSuccessDeploymentFromCache } from "./deployment-cache-service"; +import { dynamodb as ddb } from "config/dynamodb"; +import { createHashWithoutSharedSecret } from "utils/encryption"; +import { auditLog } from "./audit-log-service"; + +const logger = getLogger("test"); +const ONE_DAY_IN_MILLISECONDS = 24 * 60 * 60 * 1000; + + +describe("audit log service", () => { + + describe("auditLog", () => { + it("should successfully save DD api call audit info to dynamo db", async () => { + + const createdAt = new Date(); + const subscriptionId = 241412; + const entityId = "25e1008"; + const entityAction = "pushed"; + const entityType = "commit"; + const source = "backfill"; + const issueKey = "ARC-2605"; + const env = "prod"; + const ID = `subID_${subscriptionId}_type_${entityType}_id_${entityId}_action_${entityAction}_issueKey_${issueKey}`; + await auditLog( + { + source, + entityType, + entityAction, + entityId, + subscriptionId, + issueKey, + env, + createdAt + }, + logger + ); + const result = await ddb + .getItem({ + TableName: envVars.DYNAMO_AUDIT_LOG_TABLE_NAME, + Key: { + Id: { S: createHashWithoutSharedSecret(ID) }, + CreatedAt: { N: String(createdAt.getTime()) } + }, + AttributesToGet: [ + "Id", + "CreatedAt", + "ExpiredAfter", + "source", + "entityType", + "entityAction", + "entityId", + "subscriptionId", + "issueKey", + "env" + ] + }) + .promise(); + expect(result.$response.error).toBeNull(); + expect(result.Item).toEqual({ + Id: { "S": createHashWithoutSharedSecret(ID) }, + CreatedAt: { "N": String(createdAt.getTime()) }, + ExpiredAfter: { "N": String(Math.floor((createdAt.getTime() + ONE_DAY_IN_MILLISECONDS) / 1000)) }, + source: { "S": source }, + entityAction: { "S": entityAction }, + entityId: { "S": entityId }, + entityType: { "S": entityType }, + env: { "S": env }, + issueKey: { "S": issueKey }, + subscriptionId: { "N": String(subscriptionId) } + }); + }); + }); +}); diff --git a/src/services/audit-log-service.ts b/src/services/audit-log-service.ts new file mode 100644 index 0000000000..ecc3ee468c --- /dev/null +++ b/src/services/audit-log-service.ts @@ -0,0 +1,89 @@ +import Logger from "bunyan"; +import { envVars } from "config/env"; +import { getLogger } from "config/logger"; +import { dynamodb as ddb } from "config/dynamodb"; +import { createHashWithoutSharedSecret } from "utils/encryption"; + +const defaultLogger = getLogger("DeploymentDynamoLogger"); +export type AuditInfo = { + source: string; + entityType: string; + entityAction: string; + entityId: string; + subscriptionId: number; + issueKey: string; + env: string; + createdAt: Date; +}; + +const ONE_DAY_IN_MILLISECONDS = 24 * 60 * 60 * 1000; + +export const auditLog = async (auditInfo : AuditInfo, logger: Logger) => { + logger.debug("Saving auditInfo to db"); + const { source, entityAction, entityId, entityType, subscriptionId, env, issueKey, createdAt } = auditInfo; + const result = await ddb.putItem({ + TableName: envVars.DYNAMO_AUDIT_LOG_TABLE_NAME, + Item: { + Id: { "S": getKey(auditInfo) }, //partition key + CreatedAt: { "N": String(createdAt.getTime()) }, //sort key + ExpiredAfter: { "N": String(Math.floor((new Date().getTime() + ONE_DAY_IN_MILLISECONDS) / 1000)) }, //ttl + source: { "S": source }, + entityAction: { "S": entityAction }, + entityId: { "S": entityId }, + entityType: { "S": entityType }, + env: { "S": env }, + issueKey: { "S": issueKey }, + subscriptionId: { "N": String(subscriptionId) } + } + }).promise(); + if (result.$response.error) { + throw result.$response.error; + } +}; + +export type LastSuccessfulDeploymentFromCache = { + commitSha: string; + createdAt: Date; +}; + +export const findLog = async( + params: AuditInfo, + logger: Logger = defaultLogger +): Promise => { + logger.debug("Finding audit log for DD call"); + const result = await ddb.query({ + TableName: envVars.DYNAMO_AUDIT_LOG_TABLE_NAME, + KeyConditionExpression: "Id = :id and CreatedAt < :createdAt", + ExpressionAttributeValues: { + ":id": { "S": getKey(params) } + }, + ScanIndexForward: false, + Limit: 1 + }).promise(); + + if (result.$response.error) { + throw result.$response.error; + } + + if (!result.Items?.length) { + return undefined; + } + + const item = result.Items[0]; + + return { + commitSha: item.CommitSha.S || "", + createdAt: new Date(Number(item.CreatedAt.N)) + }; +}; + +/* + * The partition key (return of this function) + range key (creation time of the deployment status) will be the unique identifier of the each entry + * Some assumption here is repositoryId is unique per github base url (cloud and ghes) per env. + * So if multiple jiraHost connect to the same app (like same cloud org but multiple subscription), the data will be shared. + * Sharing that deployment data is okay, because they base on the github deployment result, not our subscription. + */ +const getKey = (auditInfo: AuditInfo) => { + const { entityAction, entityId, entityType, subscriptionId, issueKey } = auditInfo; + return createHashWithoutSharedSecret(`subID_${subscriptionId}_type_${entityType}_id_${entityId}_action_${entityAction}_issueKey_${issueKey}`); +}; From d35ad23d5723c87199a952fae996ce1f589121c9 Mon Sep 17 00:00:00 2001 From: Gary Xue Date: Mon, 20 Nov 2023 16:54:49 +1100 Subject: [PATCH 2/5] ARC-2643 add api router to fetch audit log --- src/routes/api/api-router.ts | 2 + .../api/audit-log/audit-log-api-router.ts | 16 +++++ .../audit-log/audit-log-get-by-sub-id.test.ts | 64 +++++++++++++++++++ .../api/audit-log/audit-log-get-by-sub-id.ts | 28 ++++++++ src/services/audit-log-service.ts | 12 +++- test/utils/database-state-creator.ts | 2 +- 6 files changed, 121 insertions(+), 3 deletions(-) create mode 100644 src/routes/api/audit-log/audit-log-api-router.ts create mode 100644 src/routes/api/audit-log/audit-log-get-by-sub-id.test.ts create mode 100644 src/routes/api/audit-log/audit-log-get-by-sub-id.ts diff --git a/src/routes/api/api-router.ts b/src/routes/api/api-router.ts index a96fe5702b..10f48cc763 100644 --- a/src/routes/api/api-router.ts +++ b/src/routes/api/api-router.ts @@ -30,6 +30,7 @@ import { ApiReplyFailedEntitiesFromDataDepotPost } from "./api-replay-failed-ent import { RepoSyncState } from "models/reposyncstate"; import { ApiResyncFailedTasksPost } from "./api-resync-failed-tasks"; import { GHESVerificationRouter } from "./ghes-app-verification/ghes-app-verification-router"; +import { AuditLogApiRouter } from "./audit-log/audit-log-api-router"; export const ApiRouter = Router(); @@ -226,6 +227,7 @@ ApiRouter.post("/reset-failed-pending-deployment-cursor", ResetFailedAndPendingD ApiRouter.post("/replay-rejected-entities-from-data-depot", ApiReplyFailedEntitiesFromDataDepotPost); ApiRouter.post("/resync-failed-tasks",ApiResyncFailedTasksPost); ApiRouter.use("/verify/githubapp/:gitHubAppId", GHESVerificationRouter); +ApiRouter.use("/audit-log", AuditLogApiRouter); ApiRouter.use("/jira", ApiJiraRouter); ApiRouter.use("/:installationId", param("installationId").isInt(), returnOnValidationError, ApiInstallationRouter); diff --git a/src/routes/api/audit-log/audit-log-api-router.ts b/src/routes/api/audit-log/audit-log-api-router.ts new file mode 100644 index 0000000000..2547c16484 --- /dev/null +++ b/src/routes/api/audit-log/audit-log-api-router.ts @@ -0,0 +1,16 @@ +import { Router } from "express"; +import { ApiAuditLogGetBySubscriptionId } from "./audit-log-get-by-sub-id"; +import { param, query } from "express-validator"; +import { returnOnValidationError } from "../api-utils"; + +export const AuditLogApiRouter = Router({ mergeParams: true }); + +AuditLogApiRouter.get("/subscription/:subscriptionId", + param("subscriptionId").isInt(), + query("entityType").isString(), + query("entityAction").isString(), + query("entityId").isString(), + query("issueKey").isString(), + returnOnValidationError, + ApiAuditLogGetBySubscriptionId +); diff --git a/src/routes/api/audit-log/audit-log-get-by-sub-id.test.ts b/src/routes/api/audit-log/audit-log-get-by-sub-id.test.ts new file mode 100644 index 0000000000..276b9d4a9d --- /dev/null +++ b/src/routes/api/audit-log/audit-log-get-by-sub-id.test.ts @@ -0,0 +1,64 @@ +import supertest from "supertest"; +import { when } from "jest-when"; +import { omit } from "lodash"; +import { getFrontendApp } from "../../../app"; +import { DatabaseStateCreator, CreatorResult } from "test/utils/database-state-creator"; +import { findLog } from "services/audit-log-service"; + +jest.mock("services/audit-log-service"); + +describe("AuditLogApiGetBySubscriptionId", () => { + + const makeApiCall = (subscriptionId: string | number, params: Record) => { + return supertest(getFrontendApp()) + .get(`/api/audit-log/subscription/${subscriptionId}`) + .query(params) + .set("X-Slauth-Mechanism", "test") + .send(); + }; + + let db: CreatorResult; + let params; + + beforeEach(async () => { + db = await new DatabaseStateCreator().forServer().create(); + params = { + issueKey: "ABC-123", + entityType: "commit", + entityAction: "push", + entityId: "abcd-efgh-ijkl" + }; + }); + + describe.each(["issueKey", "entityType", "entityAction", "entityId"])("param validation", (paramKey) => { + it(`should return 422 on missing param ${paramKey}`, async () => { + await makeApiCall(db.subscription.id, omit(params, paramKey)) + .expect(422); + }); + }); + + it(`should return error on missing subscription`, async () => { + await makeApiCall(db.subscription.id + 1, params) + .expect(500); + }); + + + it("should return audit log data successfully", async () => { + + when(findLog).calledWith({ + subscriptionId: db.subscription.id, + issueKey: "ABC-123", + entityType: "commit", + entityAction: "push", + entityId: "abcd-efgh-ijkl" + }, expect.anything()).mockResolvedValue({ + name: "hello" + } as any); + + const result = await makeApiCall(db.subscription.id, params).expect(200); + + expect(result.body).toEqual({ + name: "hello" + }); + }); +}); diff --git a/src/routes/api/audit-log/audit-log-get-by-sub-id.ts b/src/routes/api/audit-log/audit-log-get-by-sub-id.ts new file mode 100644 index 0000000000..bcb887e098 --- /dev/null +++ b/src/routes/api/audit-log/audit-log-get-by-sub-id.ts @@ -0,0 +1,28 @@ +import { Request, Response } from "express"; +import { Subscription } from "models/subscription"; +import { findLog, AuditInfoPK } from "services/audit-log-service"; + +export const ApiAuditLogGetBySubscriptionId = async (req: Request, res: Response): Promise => { + + const { subscriptionId } = req.params; + const { issueKey, entityType, entityAction, entityId } = req.query; + + const subscription = await Subscription.findByPk(subscriptionId); + + if (subscription === null) { + throw new Error("Cannot find subscription by id " + subscriptionId); + } + + const auditInfo: AuditInfoPK = { + subscriptionId: Number(subscriptionId), + issueKey: String(issueKey), + entityType: String(entityType), + entityAction: String(entityAction), + entityId: String(entityId) + }; + + const result = await findLog(auditInfo, req.log); + + res.status(200).json(result); + +}; diff --git a/src/services/audit-log-service.ts b/src/services/audit-log-service.ts index ecc3ee468c..7a6022c0ed 100644 --- a/src/services/audit-log-service.ts +++ b/src/services/audit-log-service.ts @@ -5,6 +5,14 @@ import { dynamodb as ddb } from "config/dynamodb"; import { createHashWithoutSharedSecret } from "utils/encryption"; const defaultLogger = getLogger("DeploymentDynamoLogger"); +export type AuditInfoPK = { + entityType: string; + entityAction: string; + entityId: string; + subscriptionId: number; + issueKey: string; +}; + export type AuditInfo = { source: string; entityType: string; @@ -47,7 +55,7 @@ export type LastSuccessfulDeploymentFromCache = { }; export const findLog = async( - params: AuditInfo, + params: AuditInfoPK, logger: Logger = defaultLogger ): Promise => { logger.debug("Finding audit log for DD call"); @@ -83,7 +91,7 @@ export const findLog = async( * So if multiple jiraHost connect to the same app (like same cloud org but multiple subscription), the data will be shared. * Sharing that deployment data is okay, because they base on the github deployment result, not our subscription. */ -const getKey = (auditInfo: AuditInfo) => { +const getKey = (auditInfo: AuditInfoPK) => { const { entityAction, entityId, entityType, subscriptionId, issueKey } = auditInfo; return createHashWithoutSharedSecret(`subID_${subscriptionId}_type_${entityType}_id_${entityId}_action_${entityAction}_issueKey_${issueKey}`); }; diff --git a/test/utils/database-state-creator.ts b/test/utils/database-state-creator.ts index 3bc146e1ed..b2657429c5 100644 --- a/test/utils/database-state-creator.ts +++ b/test/utils/database-state-creator.ts @@ -7,7 +7,7 @@ import path from "path"; import { getHashedKey } from "models/sequelize"; import { v4 } from "uuid"; -interface CreatorResult { +export interface CreatorResult { installation: Installation; subscription: Subscription; repoSyncState: RepoSyncState | undefined; From ff4c719f545d9797dfb6557d125c9a0ba752554c Mon Sep 17 00:00:00 2001 From: Kamakshee Samant Date: Mon, 20 Nov 2023 15:08:38 +1100 Subject: [PATCH 3/5] chore: audit log service --- src/services/audit-log-service.test.ts | 2 +- src/services/audit-log-service.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/services/audit-log-service.test.ts b/src/services/audit-log-service.test.ts index 3a4c73e318..2f6811427e 100644 --- a/src/services/audit-log-service.test.ts +++ b/src/services/audit-log-service.test.ts @@ -22,7 +22,7 @@ describe("audit log service", () => { const source = "backfill"; const issueKey = "ARC-2605"; const env = "prod"; - const ID = `subID_${subscriptionId}_type_${entityType}_id_${entityId}_action_${entityAction}_issueKey_${issueKey}`; + const ID = `subID_${subscriptionId}_typ_${entityType}_id_${entityId}_act_${entityAction}_issKey_${issueKey}`; await auditLog( { source, diff --git a/src/services/audit-log-service.ts b/src/services/audit-log-service.ts index 7a6022c0ed..deff71797c 100644 --- a/src/services/audit-log-service.ts +++ b/src/services/audit-log-service.ts @@ -93,5 +93,5 @@ export const findLog = async( */ const getKey = (auditInfo: AuditInfoPK) => { const { entityAction, entityId, entityType, subscriptionId, issueKey } = auditInfo; - return createHashWithoutSharedSecret(`subID_${subscriptionId}_type_${entityType}_id_${entityId}_action_${entityAction}_issueKey_${issueKey}`); + return createHashWithoutSharedSecret(`subID_${subscriptionId}_typ_${entityType}_id_${entityId}_act_${entityAction}_issKey_${issueKey}`); }; From 2d762b760e96b97cbdc6913139553138e08b3133 Mon Sep 17 00:00:00 2001 From: Kamakshee Samant Date: Tue, 21 Nov 2023 17:47:58 +1100 Subject: [PATCH 4/5] chore: audit log service --- .env.development | 2 +- .env.e2e | 2 +- .env.test | 2 +- docker-compose.yml | 2 +- github-for-jira.sd.yml | 16 +-- .../api/audit-log/audit-log-api-router.ts | 1 - .../audit-log/audit-log-get-by-sub-id.test.ts | 4 +- .../api/audit-log/audit-log-get-by-sub-id.ts | 3 +- src/services/audit-log-service.test.ts | 5 +- src/services/audit-log-service.ts | 114 ++++++++++-------- test/snapshots/app.test.ts.snap | 2 + 11 files changed, 79 insertions(+), 74 deletions(-) diff --git a/.env.development b/.env.development index 2b753b0721..592343c523 100644 --- a/.env.development +++ b/.env.development @@ -18,7 +18,7 @@ GLOBAL_HASH_SECRET=testsecret DYNAMO_DEPLOYMENT_HISTORY_CACHE_TABLE_REGION=us-west-1 DYNAMO_DEPLOYMENT_HISTORY_CACHE_TABLE_NAME=deployment-history-cache DYNAMO_AUDIT_LOG_TABLE_REGION=us-west-1 -DYNAMO_AUDIT_LOG_TABLE_NAME=audit_log +DYNAMO_AUDIT_LOG_TABLE_NAME=audit-log # The Postgres URL used to connect to the database and secret for encrypting data DATABASE_URL=postgres://postgres:postgres@127.0.0.1:5432/jira-dev diff --git a/.env.e2e b/.env.e2e index a9b3deae68..91a296c9ed 100644 --- a/.env.e2e +++ b/.env.e2e @@ -22,7 +22,7 @@ DEBUG=nock.* DYNAMO_DEPLOYMENT_HISTORY_CACHE_TABLE_REGION=us-west-1 DYNAMO_DEPLOYMENT_HISTORY_CACHE_TABLE_NAME=deployment-history-cache DYNAMO_AUDIT_LOG_TABLE_REGION=us-west-1 -DYNAMO_AUDIT_LOG_TABLE_NAME=audit_log +DYNAMO_AUDIT_LOG_TABLE_NAME=audit-log MICROS_AWS_REGION=us-west-1 diff --git a/.env.test b/.env.test index ded4a652a5..0f3a4bef4d 100644 --- a/.env.test +++ b/.env.test @@ -27,7 +27,7 @@ DEBUG=nock.* DYNAMO_DEPLOYMENT_HISTORY_CACHE_TABLE_REGION=us-west-1 DYNAMO_DEPLOYMENT_HISTORY_CACHE_TABLE_NAME=deployment-history-cache DYNAMO_AUDIT_LOG_TABLE_REGION=us-west-1 -DYNAMO_AUDIT_LOG_TABLE_NAME=audit_log +DYNAMO_AUDIT_LOG_TABLE_NAME=audit-log MICROS_AWS_REGION=us-west-1 diff --git a/docker-compose.yml b/docker-compose.yml index 1245f77ec3..c4013a3aba 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -31,7 +31,7 @@ services: environment: - DEFAULT_REGION=us-west-1 - DYNAMO_DEPLOYMENT_HISTORY_CACHE_TABLE_NAME=deployment-history-cache - - DYNAMO_AUDIT_LOG_TABLE_NAME=audit_log + - DYNAMO_AUDIT_LOG_TABLE_NAME=audit-log - DYNAMO_DEPLOYMENT_HISTORY_CACHE_TABLE_REGION=us-west-1 - DYNAMO_AUDIT_LOG_TABLE_REGION=us-west-1 - LAMBDA_REMOTE_DOCKER=false diff --git a/github-for-jira.sd.yml b/github-for-jira.sd.yml index 9212a20957..96457d6e5f 100644 --- a/github-for-jira.sd.yml +++ b/github-for-jira.sd.yml @@ -180,9 +180,9 @@ resources: TTLAttributeName: ExpiredAfter dataType: - UGC/PrimaryIdentifier # Sha of a commit that point to user's code - - name: audit_log + - name: audit-log type: dynamo-db - attributes: &table-attributes + attributes: &audit-log-table-attributes HashKeyName: Id HashKeyType: "S" RangeKeyName: CreatedAt @@ -443,10 +443,10 @@ environmentOverrides: type: dynamo-db attributes: <<: *table-attributes - - name: audit_log + - name: audit-log type: dynamo-db attributes: - <<: *table-attributes + <<: *audit-log-table-attributes alarms: overrides: @@ -550,10 +550,10 @@ environmentOverrides: type: dynamo-db attributes: <<: *table-attributes - - name: audit_log + - name: audit-log type: dynamo-db attributes: - <<: *table-attributes + <<: *audit-log-table-attributes - type: globaledge name: proxy @@ -655,10 +655,10 @@ environmentOverrides: type: dynamo-db attributes: <<: *table-attributes - - name: audit_log + - name: audit-log type: dynamo-db attributes: - <<: *table-attributes + <<: *audit-log-table-attributes - type: globaledge name: proxy diff --git a/src/routes/api/audit-log/audit-log-api-router.ts b/src/routes/api/audit-log/audit-log-api-router.ts index 2547c16484..c9ada0c046 100644 --- a/src/routes/api/audit-log/audit-log-api-router.ts +++ b/src/routes/api/audit-log/audit-log-api-router.ts @@ -8,7 +8,6 @@ export const AuditLogApiRouter = Router({ mergeParams: true }); AuditLogApiRouter.get("/subscription/:subscriptionId", param("subscriptionId").isInt(), query("entityType").isString(), - query("entityAction").isString(), query("entityId").isString(), query("issueKey").isString(), returnOnValidationError, diff --git a/src/routes/api/audit-log/audit-log-get-by-sub-id.test.ts b/src/routes/api/audit-log/audit-log-get-by-sub-id.test.ts index 276b9d4a9d..09d5b6384f 100644 --- a/src/routes/api/audit-log/audit-log-get-by-sub-id.test.ts +++ b/src/routes/api/audit-log/audit-log-get-by-sub-id.test.ts @@ -25,12 +25,11 @@ describe("AuditLogApiGetBySubscriptionId", () => { params = { issueKey: "ABC-123", entityType: "commit", - entityAction: "push", entityId: "abcd-efgh-ijkl" }; }); - describe.each(["issueKey", "entityType", "entityAction", "entityId"])("param validation", (paramKey) => { + describe.each(["issueKey", "entityType", "entityId"])("param validation", (paramKey) => { it(`should return 422 on missing param ${paramKey}`, async () => { await makeApiCall(db.subscription.id, omit(params, paramKey)) .expect(422); @@ -49,7 +48,6 @@ describe("AuditLogApiGetBySubscriptionId", () => { subscriptionId: db.subscription.id, issueKey: "ABC-123", entityType: "commit", - entityAction: "push", entityId: "abcd-efgh-ijkl" }, expect.anything()).mockResolvedValue({ name: "hello" diff --git a/src/routes/api/audit-log/audit-log-get-by-sub-id.ts b/src/routes/api/audit-log/audit-log-get-by-sub-id.ts index bcb887e098..9e5248acb9 100644 --- a/src/routes/api/audit-log/audit-log-get-by-sub-id.ts +++ b/src/routes/api/audit-log/audit-log-get-by-sub-id.ts @@ -5,7 +5,7 @@ import { findLog, AuditInfoPK } from "services/audit-log-service"; export const ApiAuditLogGetBySubscriptionId = async (req: Request, res: Response): Promise => { const { subscriptionId } = req.params; - const { issueKey, entityType, entityAction, entityId } = req.query; + const { issueKey, entityType, entityId } = req.query; const subscription = await Subscription.findByPk(subscriptionId); @@ -17,7 +17,6 @@ export const ApiAuditLogGetBySubscriptionId = async (req: Request, res: Response subscriptionId: Number(subscriptionId), issueKey: String(issueKey), entityType: String(entityType), - entityAction: String(entityAction), entityId: String(entityId) }; diff --git a/src/services/audit-log-service.test.ts b/src/services/audit-log-service.test.ts index 2f6811427e..d006d49a29 100644 --- a/src/services/audit-log-service.test.ts +++ b/src/services/audit-log-service.test.ts @@ -21,8 +21,7 @@ describe("audit log service", () => { const entityType = "commit"; const source = "backfill"; const issueKey = "ARC-2605"; - const env = "prod"; - const ID = `subID_${subscriptionId}_typ_${entityType}_id_${entityId}_act_${entityAction}_issKey_${issueKey}`; + const ID = `subID_${subscriptionId}_typ_${entityType}_id_${entityId}_issKey_${issueKey}`; await auditLog( { source, @@ -31,7 +30,6 @@ describe("audit log service", () => { entityId, subscriptionId, issueKey, - env, createdAt }, logger @@ -66,7 +64,6 @@ describe("audit log service", () => { entityAction: { "S": entityAction }, entityId: { "S": entityId }, entityType: { "S": entityType }, - env: { "S": env }, issueKey: { "S": issueKey }, subscriptionId: { "N": String(subscriptionId) } }); diff --git a/src/services/audit-log-service.ts b/src/services/audit-log-service.ts index deff71797c..3f2cf2e362 100644 --- a/src/services/audit-log-service.ts +++ b/src/services/audit-log-service.ts @@ -5,93 +5,103 @@ import { dynamodb as ddb } from "config/dynamodb"; import { createHashWithoutSharedSecret } from "utils/encryption"; const defaultLogger = getLogger("DeploymentDynamoLogger"); + export type AuditInfoPK = { entityType: string; - entityAction: string; entityId: string; subscriptionId: number; issueKey: string; }; -export type AuditInfo = { - source: string; - entityType: string; - entityAction: string; - entityId: string; - subscriptionId: number; - issueKey: string; - env: string; +export type AuditInfo = AuditInfoPK & { createdAt: Date; + entityAction: string; + source: string; }; const ONE_DAY_IN_MILLISECONDS = 24 * 60 * 60 * 1000; -export const auditLog = async (auditInfo : AuditInfo, logger: Logger) => { +export const auditLog = async (auditInfo: AuditInfo, logger: Logger) => { logger.debug("Saving auditInfo to db"); - const { source, entityAction, entityId, entityType, subscriptionId, env, issueKey, createdAt } = auditInfo; - const result = await ddb.putItem({ - TableName: envVars.DYNAMO_AUDIT_LOG_TABLE_NAME, - Item: { - Id: { "S": getKey(auditInfo) }, //partition key - CreatedAt: { "N": String(createdAt.getTime()) }, //sort key - ExpiredAfter: { "N": String(Math.floor((new Date().getTime() + ONE_DAY_IN_MILLISECONDS) / 1000)) }, //ttl - source: { "S": source }, - entityAction: { "S": entityAction }, - entityId: { "S": entityId }, - entityType: { "S": entityType }, - env: { "S": env }, - issueKey: { "S": issueKey }, - subscriptionId: { "N": String(subscriptionId) } - } - }).promise(); + const { + source, + entityAction, + entityId, + entityType, + subscriptionId, + issueKey, + createdAt + } = auditInfo; + const result = await ddb + .putItem({ + TableName: envVars.DYNAMO_AUDIT_LOG_TABLE_NAME, + Item: { + Id: { S: getKey(auditInfo) }, //partition key + CreatedAt: { N: String(createdAt.getTime()) }, //sort key + ExpiredAfter: { + N: String( + Math.floor( + (auditInfo.createdAt.getTime() + ONE_DAY_IN_MILLISECONDS) / 1000 + ) + ) + }, //ttl + source: { S: source }, + entityAction: { S: entityAction }, + entityId: { S: entityId }, + entityType: { S: entityType }, + issueKey: { S: issueKey }, + subscriptionId: { N: String(subscriptionId) } + } + }) + .promise(); if (result.$response.error) { throw result.$response.error; } }; -export type LastSuccessfulDeploymentFromCache = { - commitSha: string; - createdAt: Date; -}; - -export const findLog = async( +export const findLog = async ( params: AuditInfoPK, logger: Logger = defaultLogger -): Promise => { +): Promise => { logger.debug("Finding audit log for DD call"); - const result = await ddb.query({ - TableName: envVars.DYNAMO_AUDIT_LOG_TABLE_NAME, - KeyConditionExpression: "Id = :id and CreatedAt < :createdAt", - ExpressionAttributeValues: { - ":id": { "S": getKey(params) } - }, - ScanIndexForward: false, - Limit: 1 - }).promise(); + const result = await ddb + .query({ + TableName: envVars.DYNAMO_AUDIT_LOG_TABLE_NAME, + ExpressionAttributeValues: { + ":id": { S: getKey(params) } + }, + ScanIndexForward: false, + Limit: 1 + }) + .promise(); if (result.$response.error) { throw result.$response.error; } if (!result.Items?.length) { - return undefined; + return []; } - const item = result.Items[0]; + const items = result.Items; - return { - commitSha: item.CommitSha.S || "", + return items.map((item) => ({ + source: String(item.source.S), + entityType: String(item.entityType.S), + entityAction: String(item.entityAction.S), + entityId: String(item.entityId.S), + subscriptionId: Number(item.subscriptionId.N), + issueKey: String(item.issueKey.S), createdAt: new Date(Number(item.CreatedAt.N)) - }; + })); }; /* * The partition key (return of this function) + range key (creation time of the deployment status) will be the unique identifier of the each entry - * Some assumption here is repositoryId is unique per github base url (cloud and ghes) per env. - * So if multiple jiraHost connect to the same app (like same cloud org but multiple subscription), the data will be shared. - * Sharing that deployment data is okay, because they base on the github deployment result, not our subscription. */ const getKey = (auditInfo: AuditInfoPK) => { - const { entityAction, entityId, entityType, subscriptionId, issueKey } = auditInfo; - return createHashWithoutSharedSecret(`subID_${subscriptionId}_typ_${entityType}_id_${entityId}_act_${entityAction}_issKey_${issueKey}`); + const { entityId, entityType, subscriptionId, issueKey } = auditInfo; + return createHashWithoutSharedSecret( + `subID_${subscriptionId}_typ_${entityType}_id_${entityId}_issKey_${issueKey}` + ); }; diff --git a/test/snapshots/app.test.ts.snap b/test/snapshots/app.test.ts.snap index 0653193af7..663fb0d058 100644 --- a/test/snapshots/app.test.ts.snap +++ b/test/snapshots/app.test.ts.snap @@ -97,6 +97,8 @@ exports[`app getFrontendApp please review routes and update snapshot when adding query,expressInit,elapsedTimeMetrics,sentryRequestMiddleware,urlencodedParser,jsonParser,cookieParser,LogMiddleware,urlencodedParser,jsonParser,LogMiddleware,slauthMiddleware,rateLimit,ApiResyncFailedTasksPost :POST ^/?(?=/|$)^/api/?(?=/|$)^/verify/githubapp/(?:([^/]+?))/?(?=/|$)^/verify-get-apps/?$ query,expressInit,elapsedTimeMetrics,sentryRequestMiddleware,urlencodedParser,jsonParser,cookieParser,LogMiddleware,urlencodedParser,jsonParser,LogMiddleware,slauthMiddleware,rateLimit,GHESVerifyGetApps +:GET ^/?(?=/|$)^/api/?(?=/|$)^/audit-log/?(?=/|$)^/subscription/(?:([^/]+?))/?$ + query,expressInit,elapsedTimeMetrics,sentryRequestMiddleware,urlencodedParser,jsonParser,cookieParser,LogMiddleware,urlencodedParser,jsonParser,LogMiddleware,slauthMiddleware,rateLimit,middleware,middleware,middleware,middleware,returnOnValidationError,ApiAuditLogGetBySubscriptionId :POST ^/?(?=/|$)^/api/?(?=/|$)^/jira/?(?=/|$)^/(?:([^/]+?))/verify/?$ query,expressInit,elapsedTimeMetrics,sentryRequestMiddleware,urlencodedParser,jsonParser,cookieParser,LogMiddleware,urlencodedParser,jsonParser,LogMiddleware,slauthMiddleware,rateLimit,middleware,returnOnValidationError,ApiJiraVerifyPost :POST ^/?(?=/|$)^/api/?(?=/|$)^/jira/?(?=/|$)^/(?:([^/]+?))/uninstall/?$ From 1c452d4d8e9d14533a3fd6305836e398dc762ce1 Mon Sep 17 00:00:00 2001 From: Kamakshee Samant Date: Tue, 21 Nov 2023 18:18:59 +1100 Subject: [PATCH 5/5] chore: PR review comments --- src/services/audit-log-service.test.ts | 73 ++++++++++++++++++++------ src/services/audit-log-service.ts | 1 + 2 files changed, 58 insertions(+), 16 deletions(-) diff --git a/src/services/audit-log-service.test.ts b/src/services/audit-log-service.test.ts index d006d49a29..a7866060eb 100644 --- a/src/services/audit-log-service.test.ts +++ b/src/services/audit-log-service.test.ts @@ -1,19 +1,15 @@ import { getLogger } from "config/logger"; import { envVars } from "config/env"; -// import { cacheSuccessfulDeploymentInfo, findLastSuccessDeploymentFromCache } from "./deployment-cache-service"; import { dynamodb as ddb } from "config/dynamodb"; import { createHashWithoutSharedSecret } from "utils/encryption"; -import { auditLog } from "./audit-log-service"; +import { auditLog, findLog } from "./audit-log-service"; const logger = getLogger("test"); const ONE_DAY_IN_MILLISECONDS = 24 * 60 * 60 * 1000; - describe("audit log service", () => { - describe("auditLog", () => { it("should successfully save DD api call audit info to dynamo db", async () => { - const createdAt = new Date(); const subscriptionId = 241412; const entityId = "25e1008"; @@ -50,22 +46,67 @@ describe("audit log service", () => { "entityAction", "entityId", "subscriptionId", - "issueKey", - "env" + "issueKey" ] }) .promise(); expect(result.$response.error).toBeNull(); expect(result.Item).toEqual({ - Id: { "S": createHashWithoutSharedSecret(ID) }, - CreatedAt: { "N": String(createdAt.getTime()) }, - ExpiredAfter: { "N": String(Math.floor((createdAt.getTime() + ONE_DAY_IN_MILLISECONDS) / 1000)) }, - source: { "S": source }, - entityAction: { "S": entityAction }, - entityId: { "S": entityId }, - entityType: { "S": entityType }, - issueKey: { "S": issueKey }, - subscriptionId: { "N": String(subscriptionId) } + Id: { S: createHashWithoutSharedSecret(ID) }, + CreatedAt: { N: String(createdAt.getTime()) }, + ExpiredAfter: { + N: String( + Math.floor((createdAt.getTime() + ONE_DAY_IN_MILLISECONDS) / 1000) + ) + }, + source: { S: source }, + entityAction: { S: entityAction }, + entityId: { S: entityId }, + entityType: { S: entityType }, + issueKey: { S: issueKey }, + subscriptionId: { N: String(subscriptionId) } + }); + }); + + describe("auditLog", () => { + it("should successfully save DD api call audit info to dynamo db", async () => { + const createdAt = new Date(); + const subscriptionId = 241412; + const entityId = "25e1008"; + const entityAction = "pushed"; + const entityType = "commit"; + const source = "backfill"; + const issueKey = "ARC-2605"; + await auditLog( + { + source, + entityType, + entityAction, + entityId, + subscriptionId, + issueKey, + createdAt + }, + logger + ); + const result = await findLog( + { + entityType, + entityId, + subscriptionId, + issueKey + }, + logger + ); + expect(result).toEqual([{ + entityAction, + entityId, + entityType, + issueKey, + source, + subscriptionId, + createdAt + }]); }); }); }); diff --git a/src/services/audit-log-service.ts b/src/services/audit-log-service.ts index 3f2cf2e362..b27be17619 100644 --- a/src/services/audit-log-service.ts +++ b/src/services/audit-log-service.ts @@ -67,6 +67,7 @@ export const findLog = async ( const result = await ddb .query({ TableName: envVars.DYNAMO_AUDIT_LOG_TABLE_NAME, + KeyConditionExpression: "Id = :id", ExpressionAttributeValues: { ":id": { S: getKey(params) } },