From 824d6693effae6cc5b637d13eb254581546ee6b7 Mon Sep 17 00:00:00 2001 From: Kishore <42832651+kishore03109@users.noreply.github.com> Date: Thu, 21 Sep 2023 16:25:28 +0800 Subject: [PATCH 1/8] feat(template): add ffs as a manual check-in (#933) * feat(template): add ffs as a manual check-in * feat(template): link do doc for sot --- .github/pull_request_template.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index ed22bd0cc..8dd123d43 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -14,7 +14,7 @@ Closes [insert issue #] - [ ] Yes - this PR contains breaking changes - Details ... -- [ ] No - this PR is backwards compatible +- [ ] No - this PR is backwards compatible with ALL of the following feature flags in this [doc](https://www.notion.so/opengov/Existing-feature-flags-518ad2cdc325420893a105e88c432be5) **Features**: From 75d69820b9175fb11aee32db599c6242ad7ea18b Mon Sep 17 00:00:00 2001 From: Hsu Zhong Jun <27919917+dcshzj@users.noreply.github.com> Date: Thu, 28 Sep 2023 16:26:50 +0800 Subject: [PATCH 2/8] feat: add ability to update repo state for GGS (#949) * chore: update admin repo list and add e2e repos * chore: reorder unit tests to match order on main file * feat: add ability to update repo state for GGS * feat: add unit tests for new functionality --- src/constants/constants.ts | 6 + src/services/db/GitFileSystemService.ts | 68 +- src/services/db/GitHubService.js | 4 +- src/services/db/RepoService.ts | 45 +- .../db/__tests__/GitFileSystemService.spec.ts | 834 ++++++++++++------ .../db/__tests__/GitHubService.spec.ts | 14 + src/services/db/__tests__/RepoService.spec.ts | 470 +++++----- 7 files changed, 923 insertions(+), 518 deletions(-) diff --git a/src/constants/constants.ts b/src/constants/constants.ts index 10672f96c..44d3cb210 100644 --- a/src/constants/constants.ts +++ b/src/constants/constants.ts @@ -40,6 +40,7 @@ export const ISOMER_ADMIN_REPOS = [ "isomercms-backend", "isomercms-frontend", "isomer-redirection", + "isomer-indirection", "isomerpages-template", "isomer-conversion-scripts", "isomer-wysiwyg", @@ -53,6 +54,11 @@ export const ISOMER_ADMIN_REPOS = [ "infra", "markdown-helper", ] +export const ISOMER_E2E_TEST_REPOS = [ + "e2e-test-repo", + "e2e-email-test-repo", + "e2e-notggs-test-repo", +] export const INACTIVE_USER_THRESHOLD_DAYS = 60 export const GITHUB_ORG_REPOS_ENDPOINT = `https://api.github.com/orgs/${ISOMER_GITHUB_ORG_NAME}/repos` diff --git a/src/services/db/GitFileSystemService.ts b/src/services/db/GitFileSystemService.ts index a0dc4c645..8fe1a4332 100644 --- a/src/services/db/GitFileSystemService.ts +++ b/src/services/db/GitFileSystemService.ts @@ -21,6 +21,7 @@ import { config } from "@config/config" import logger from "@logger/logger" +import { BadRequestError } from "@errors/BadRequestError" import { ConflictError } from "@errors/ConflictError" import GitFileSystemError from "@errors/GitFileSystemError" import GitFileSystemNeedsRollbackError from "@errors/GitFileSystemNeedsRollbackError" @@ -155,8 +156,11 @@ export default class GitFileSystemService { }) } - // Ensure that the repository is in the BRANCH_REF branch - ensureCorrectBranch(repoName: string): ResultAsync { + // Ensure that the repository is in the specified branch + ensureCorrectBranch( + repoName: string, + branchName: string + ): ResultAsync { return ResultAsync.fromPromise( this.git .cwd(`${EFS_VOL_PATH}/${repoName}`) @@ -171,11 +175,11 @@ export default class GitFileSystemService { return new GitFileSystemError("An unknown error occurred") } ).andThen((currentBranch) => { - if (currentBranch !== BRANCH_REF) { + if (currentBranch !== branchName) { return ResultAsync.fromPromise( - this.git.cwd(`${EFS_VOL_PATH}/${repoName}`).checkout(BRANCH_REF), + this.git.cwd(`${EFS_VOL_PATH}/${repoName}`).checkout(branchName), (error) => { - logger.error(`Error when checking out ${BRANCH_REF}: ${error}`) + logger.error(`Error when checking out ${branchName}: ${error}`) if (error instanceof GitError) { return new GitFileSystemError("Unable to checkout branch") @@ -346,7 +350,7 @@ export default class GitFileSystemService { ) } - return this.ensureCorrectBranch(repoName).andThen(() => + return this.ensureCorrectBranch(repoName, BRANCH_REF).andThen(() => ResultAsync.fromPromise( this.git.cwd(`${EFS_VOL_PATH}/${repoName}`).pull(), (error) => { @@ -398,6 +402,7 @@ export default class GitFileSystemService { // Push the latest changes to upstream Git hosting provider push( repoName: string, + branchName: string, isForce = false ): ResultAsync { return this.isValidGitRepo(repoName).andThen((isValid) => { @@ -407,7 +412,7 @@ export default class GitFileSystemService { ) } - return this.ensureCorrectBranch(repoName) + return this.ensureCorrectBranch(repoName, branchName) .andThen(() => ResultAsync.fromPromise( isForce @@ -486,7 +491,7 @@ export default class GitFileSystemService { const commitMessage = JSON.stringify(commitMessageObj) - return this.ensureCorrectBranch(repoName) + return this.ensureCorrectBranch(repoName, BRANCH_REF) .andThen(() => { if (skipGitAdd) { // This is necessary when we have performed a git mv @@ -1173,4 +1178,51 @@ export default class GitFileSystemService { ) }) } + + updateRepoState( + repoName: string, + branchName: string, + sha: string + ): ResultAsync { + return this.isValidGitRepo(repoName).andThen((isValid) => { + if (!isValid) { + return errAsync( + new GitFileSystemError(`Folder "${repoName}" is not a valid Git repo`) + ) + } + + return this.ensureCorrectBranch(repoName, branchName) + .andThen(() => + ResultAsync.fromPromise( + this.git.cwd(`${EFS_VOL_PATH}/${repoName}`).catFile(["-t", sha]), + (error) => { + // An error is thrown if the SHA does not exist in the branch + if (error instanceof GitError) { + return new BadRequestError("The provided SHA is invalid") + } + + return new GitFileSystemError("An unknown error occurred") + } + ) + ) + .andThen(() => + ResultAsync.fromPromise( + this.git.cwd(`${EFS_VOL_PATH}/${repoName}`).reset(["--hard", sha]), + (error) => { + logger.error(`Error when updating repo state: ${error}`) + + if (error instanceof GitError) { + return new GitFileSystemError( + `Unable to update repo state to commit SHA ${sha}` + ) + } + + return new GitFileSystemError("An unknown error occurred") + } + ) + ) + .andThen(() => this.push(repoName, branchName, true)) + .map(() => undefined) + }) + } } diff --git a/src/services/db/GitHubService.js b/src/services/db/GitHubService.js index def4aca2d..5ee7b8e74 100644 --- a/src/services/db/GitHubService.js +++ b/src/services/db/GitHubService.js @@ -455,10 +455,10 @@ class GitHubService { return newCommitSha } - async updateRepoState(sessionData, { commitSha }) { + async updateRepoState(sessionData, { commitSha, branchName = BRANCH_REF }) { const { accessToken } = sessionData const { siteName } = sessionData - const refEndpoint = `${siteName}/git/refs/heads/${BRANCH_REF}` + const refEndpoint = `${siteName}/git/refs/heads/${branchName}` const headers = { Authorization: `token ${accessToken}`, } diff --git a/src/services/db/RepoService.ts b/src/services/db/RepoService.ts index 93012ee18..5a0fbd0c6 100644 --- a/src/services/db/RepoService.ts +++ b/src/services/db/RepoService.ts @@ -25,6 +25,8 @@ import { GitHubService } from "./GitHubService" import * as ReviewApi from "./review" const PLACEHOLDER_FILE_NAME = ".keep" +const BRANCH_REF = config.get("github.branchRef") + export default class RepoService extends GitHubService { private readonly gitFileSystemService: GitFileSystemService @@ -172,7 +174,7 @@ export default class RepoService extends GitHubService { throw result.error } - this.gitFileSystemService.push(sessionData.siteName) + this.gitFileSystemService.push(sessionData.siteName, BRANCH_REF) return { sha: result.value.newSha } } return await super.create(sessionData, { @@ -385,7 +387,7 @@ export default class RepoService extends GitHubService { throw result.error } - this.gitFileSystemService.push(sessionData.siteName) + this.gitFileSystemService.push(sessionData.siteName, BRANCH_REF) return { newSha: result.value } } @@ -430,7 +432,7 @@ export default class RepoService extends GitHubService { throw result.error } - this.gitFileSystemService.push(sessionData.siteName) + this.gitFileSystemService.push(sessionData.siteName, BRANCH_REF) return } @@ -497,7 +499,7 @@ export default class RepoService extends GitHubService { throw result.error } - this.gitFileSystemService.push(sessionData.siteName) + this.gitFileSystemService.push(sessionData.siteName, BRANCH_REF) return } @@ -535,7 +537,7 @@ export default class RepoService extends GitHubService { throw result.error } - this.gitFileSystemService.push(sessionData.siteName) + this.gitFileSystemService.push(sessionData.siteName, BRANCH_REF) return { newSha: result.value } } @@ -626,7 +628,7 @@ export default class RepoService extends GitHubService { throw result.error } - this.gitFileSystemService.push(sessionData.siteName) + this.gitFileSystemService.push(sessionData.siteName, BRANCH_REF) return { newSha: result.value } } @@ -737,8 +739,35 @@ export default class RepoService extends GitHubService { }) } - async updateRepoState(sessionData: any, { commitSha }: any): Promise { - return await super.updateRepoState(sessionData, { commitSha }) + async updateRepoState( + sessionData: UserWithSiteSessionData, + { + commitSha, + branchName = BRANCH_REF, + }: { commitSha: string; branchName?: string } + ): Promise { + const { siteName } = sessionData + if ( + this.isRepoWhitelisted( + siteName, + this.getGgsWhitelistedRepos(sessionData.growthbook) + ) + ) { + logger.info( + `Updating repo state for site ${siteName} to ${commitSha} on local Git file system` + ) + const result = await this.gitFileSystemService.updateRepoState( + siteName, + branchName, + commitSha + ) + if (result.isErr()) { + throw result.error + } + return + } + + return await super.updateRepoState(sessionData, { commitSha, branchName }) } async checkHasAccess(sessionData: any): Promise { diff --git a/src/services/db/__tests__/GitFileSystemService.spec.ts b/src/services/db/__tests__/GitFileSystemService.spec.ts index 3e640a4e1..6f96368a6 100644 --- a/src/services/db/__tests__/GitFileSystemService.spec.ts +++ b/src/services/db/__tests__/GitFileSystemService.spec.ts @@ -6,6 +6,7 @@ import { GitError, SimpleGit } from "simple-git" import config from "@config/config" +import { BadRequestError } from "@errors/BadRequestError" import { ConflictError } from "@errors/ConflictError" import GitFileSystemError from "@errors/GitFileSystemError" import GitFileSystemNeedsRollbackError from "@errors/GitFileSystemNeedsRollbackError" @@ -23,11 +24,8 @@ import { GitDirectoryItem, GitFile } from "@root/types/gitfilesystem" import _GitFileSystemService from "@services/db/GitFileSystemService" const MockSimpleGit = { - checkIsRepo: jest.fn(), clone: jest.fn(), cwd: jest.fn(), - remote: jest.fn(), - revparse: jest.fn(), } const GitFileSystemService = new _GitFileSystemService( @@ -365,9 +363,10 @@ describe("GitFileSystemService", () => { }) describe("ensureCorrectBranch", () => { - it("should perform a branch change if the current branch is not the correct branch", async () => { + it("should perform a branch change if the current branch is not the desired branch", async () => { const revparseMock = jest.fn().mockResolvedValueOnce("incorrect-branch") const checkoutMock = jest.fn().mockResolvedValueOnce(undefined) + const anotherBranchRef = "another-branch-ref" MockSimpleGit.cwd.mockReturnValueOnce({ revparse: revparseMock, @@ -376,7 +375,31 @@ describe("GitFileSystemService", () => { checkout: checkoutMock, }) - const result = await GitFileSystemService.ensureCorrectBranch("fake-repo") + const result = await GitFileSystemService.ensureCorrectBranch( + "fake-repo", + anotherBranchRef + ) + + expect(revparseMock).toHaveBeenCalledWith(["--abbrev-ref", "HEAD"]) + expect(checkoutMock).toHaveBeenCalledWith(anotherBranchRef) + expect(result._unsafeUnwrap()).toBeTrue() + }) + + it("should perform a branch change if the current branch is not the default branch", async () => { + const revparseMock = jest.fn().mockResolvedValueOnce("incorrect-branch") + const checkoutMock = jest.fn().mockResolvedValueOnce(undefined) + + MockSimpleGit.cwd.mockReturnValueOnce({ + revparse: revparseMock, + }) + MockSimpleGit.cwd.mockReturnValueOnce({ + checkout: checkoutMock, + }) + + const result = await GitFileSystemService.ensureCorrectBranch( + "fake-repo", + BRANCH_REF + ) expect(revparseMock).toHaveBeenCalledWith(["--abbrev-ref", "HEAD"]) expect(checkoutMock).toHaveBeenCalledWith(BRANCH_REF) @@ -390,7 +413,10 @@ describe("GitFileSystemService", () => { revparse: revparseMock, }) - const result = await GitFileSystemService.ensureCorrectBranch("fake-repo") + const result = await GitFileSystemService.ensureCorrectBranch( + "fake-repo", + BRANCH_REF + ) expect(revparseMock).toHaveBeenCalledWith(["--abbrev-ref", "HEAD"]) expect(MockSimpleGit.cwd).toHaveBeenCalledTimes(1) @@ -402,7 +428,10 @@ describe("GitFileSystemService", () => { revparse: jest.fn().mockRejectedValueOnce(new GitError()), }) - const result = await GitFileSystemService.ensureCorrectBranch("fake-repo") + const result = await GitFileSystemService.ensureCorrectBranch( + "fake-repo", + BRANCH_REF + ) expect(result._unsafeUnwrapErr()).toBeInstanceOf(GitFileSystemError) }) @@ -415,7 +444,10 @@ describe("GitFileSystemService", () => { checkout: jest.fn().mockRejectedValueOnce(new GitError()), }) - const result = await GitFileSystemService.ensureCorrectBranch("fake-repo") + const result = await GitFileSystemService.ensureCorrectBranch( + "fake-repo", + BRANCH_REF + ) expect(result._unsafeUnwrapErr()).toBeInstanceOf(GitFileSystemError) }) @@ -685,7 +717,34 @@ describe("GitFileSystemService", () => { push: jest.fn().mockResolvedValueOnce(undefined), }) - const result = await GitFileSystemService.push("fake-repo") + const result = await GitFileSystemService.push("fake-repo", BRANCH_REF) + + expect(result.isOk()).toBeTrue() + }) + + it("should push successfully for a valid Git repo with a non-standard branch ref", async () => { + const nonStandardBranchRef = "non-standard-branch-ref" + MockSimpleGit.cwd.mockReturnValueOnce({ + checkIsRepo: jest.fn().mockResolvedValueOnce(true), + }) + MockSimpleGit.cwd.mockReturnValueOnce({ + remote: jest + .fn() + .mockResolvedValueOnce( + `git@github.com:${ISOMER_GITHUB_ORG_NAME}/fake-repo.git` + ), + }) + MockSimpleGit.cwd.mockReturnValueOnce({ + revparse: jest.fn().mockResolvedValueOnce(nonStandardBranchRef), + }) + MockSimpleGit.cwd.mockReturnValueOnce({ + push: jest.fn().mockResolvedValueOnce(undefined), + }) + + const result = await GitFileSystemService.push( + "fake-repo", + nonStandardBranchRef + ) expect(result.isOk()).toBeTrue() }) @@ -711,7 +770,7 @@ describe("GitFileSystemService", () => { push: jest.fn().mockResolvedValueOnce(undefined), }) - const result = await GitFileSystemService.push("fake-repo") + const result = await GitFileSystemService.push("fake-repo", BRANCH_REF) expect(result.isOk()).toBeTrue() }) @@ -737,7 +796,7 @@ describe("GitFileSystemService", () => { push: jest.fn().mockRejectedValueOnce(new GitError()), }) - const result = await GitFileSystemService.push("fake-repo") + const result = await GitFileSystemService.push("fake-repo", BRANCH_REF) expect(result._unsafeUnwrapErr()).toBeInstanceOf(GitFileSystemError) }) @@ -747,7 +806,7 @@ describe("GitFileSystemService", () => { checkIsRepo: jest.fn().mockResolvedValueOnce(false), }) - const result = await GitFileSystemService.push("fake-repo") + const result = await GitFileSystemService.push("fake-repo", BRANCH_REF) expect(result._unsafeUnwrapErr()).toBeInstanceOf(GitFileSystemError) }) @@ -1984,294 +2043,493 @@ describe("GitFileSystemService", () => { ) expect(actual._unsafeUnwrapErr()).toBeInstanceOf(GitFileSystemError) }) + }) - describe("deleteFile", () => { - it("should delete a file successfully", async () => { - // getLatestCommitOfBranch - MockSimpleGit.cwd.mockReturnValueOnce({ - log: jest.fn().mockResolvedValueOnce({ - latest: { - author_name: "fake-author", - author_email: "fake-email", - date: "fake-date", - message: "fake-message", - hash: "test-commit-sha", - }, - }), - }) - - // getGitBlobHash - MockSimpleGit.cwd.mockReturnValueOnce({ - revparse: jest.fn().mockResolvedValueOnce("fake-old-hash"), - }) - - // commit - MockSimpleGit.cwd.mockReturnValueOnce({ - checkIsRepo: jest.fn().mockResolvedValueOnce(true), - }) - - // commit - MockSimpleGit.cwd.mockReturnValueOnce({ - remote: jest - .fn() - .mockResolvedValueOnce( - `git@github.com:${ISOMER_GITHUB_ORG_NAME}/fake-repo.git` - ), - }) - - // commit - MockSimpleGit.cwd.mockReturnValueOnce({ - revparse: jest.fn().mockResolvedValueOnce(BRANCH_REF), - }) - - // commit - MockSimpleGit.cwd.mockReturnValueOnce({ - add: jest.fn().mockResolvedValueOnce(undefined), - }) - - // commit - MockSimpleGit.cwd.mockReturnValueOnce({ - commit: jest.fn().mockResolvedValueOnce({ commit: "fake-new-hash" }), - }) - - const actual = await GitFileSystemService.delete( - "fake-repo", - "fake-dir/fake-file", - "fake-old-hash", - "fake-user-id", - false - ) + describe("deleteFile", () => { + it("should delete a file successfully", async () => { + // getLatestCommitOfBranch + MockSimpleGit.cwd.mockReturnValueOnce({ + log: jest.fn().mockResolvedValueOnce({ + latest: { + author_name: "fake-author", + author_email: "fake-email", + date: "fake-date", + message: "fake-message", + hash: "test-commit-sha", + }, + }), + }) + + // getGitBlobHash + MockSimpleGit.cwd.mockReturnValueOnce({ + revparse: jest.fn().mockResolvedValueOnce("fake-old-hash"), + }) + + // commit + MockSimpleGit.cwd.mockReturnValueOnce({ + checkIsRepo: jest.fn().mockResolvedValueOnce(true), + }) - expect(actual._unsafeUnwrap()).toEqual("fake-new-hash") - }) - - it("should return a error if the file is not valid", async () => { - // getLatestCommitOfBranch - MockSimpleGit.cwd.mockReturnValueOnce({ - log: jest.fn().mockResolvedValueOnce({ - latest: { - author_name: "fake-author", - author_email: "fake-email", - date: "fake-date", - message: "fake-message", - hash: "test-commit-sha", - }, - }), - }) - const mockStats = new Stats() - const spyGetFilePathStats = jest - .spyOn(GitFileSystemService, "getFilePathStats") + // commit + MockSimpleGit.cwd.mockReturnValueOnce({ + remote: jest + .fn() .mockResolvedValueOnce( - okAsync({ - ...mockStats, - isFile: () => false, - isDirectory: () => true, - }) - ) - - const actual = await GitFileSystemService.delete( - "fake-repo", - "fake-dir", - "fake-old-hash", - "fake-user-id", - false + `git@github.com:${ISOMER_GITHUB_ORG_NAME}/fake-repo.git` + ), + }) + + // commit + MockSimpleGit.cwd.mockReturnValueOnce({ + revparse: jest.fn().mockResolvedValueOnce(BRANCH_REF), + }) + + // commit + MockSimpleGit.cwd.mockReturnValueOnce({ + add: jest.fn().mockResolvedValueOnce(undefined), + }) + + // commit + MockSimpleGit.cwd.mockReturnValueOnce({ + commit: jest.fn().mockResolvedValueOnce({ commit: "fake-new-hash" }), + }) + + const actual = await GitFileSystemService.delete( + "fake-repo", + "fake-dir/fake-file", + "fake-old-hash", + "fake-user-id", + false + ) + + expect(actual._unsafeUnwrap()).toEqual("fake-new-hash") + }) + + it("should return a error if the file is not valid", async () => { + // getLatestCommitOfBranch + MockSimpleGit.cwd.mockReturnValueOnce({ + log: jest.fn().mockResolvedValueOnce({ + latest: { + author_name: "fake-author", + author_email: "fake-email", + date: "fake-date", + message: "fake-message", + hash: "test-commit-sha", + }, + }), + }) + const mockStats = new Stats() + const spyGetFilePathStats = jest + .spyOn(GitFileSystemService, "getFilePathStats") + .mockResolvedValueOnce( + okAsync({ + ...mockStats, + isFile: () => false, + isDirectory: () => true, + }) + ) + + const actual = await GitFileSystemService.delete( + "fake-repo", + "fake-dir", + "fake-old-hash", + "fake-user-id", + false + ) + expect(spyGetFilePathStats).toBeCalledTimes(1) + expect(actual._unsafeUnwrapErr()).toBeInstanceOf(GitFileSystemError) + }) + + it("should return a error if the file hash does not match", async () => { + // getLatestCommitOfBranch + MockSimpleGit.cwd.mockReturnValueOnce({ + log: jest.fn().mockResolvedValueOnce({ + latest: { + author_name: "fake-author", + author_email: "fake-email", + date: "fake-date", + message: "fake-message", + hash: "wrong-sha", + }, + }), + }) + + const mockStats = new Stats() + jest + .spyOn(GitFileSystemService, "getFilePathStats") + .mockResolvedValueOnce( + okAsync({ + ...mockStats, + isFile: () => true, + isDirectory: () => false, + }) ) - expect(spyGetFilePathStats).toBeCalledTimes(1) - expect(actual._unsafeUnwrapErr()).toBeInstanceOf(GitFileSystemError) - }) - - it("should return a error if the file hash does not match", async () => { - // getLatestCommitOfBranch - MockSimpleGit.cwd.mockReturnValueOnce({ - log: jest.fn().mockResolvedValueOnce({ - latest: { - author_name: "fake-author", - author_email: "fake-email", - date: "fake-date", - message: "fake-message", - hash: "wrong-sha", - }, - }), - }) - - const mockStats = new Stats() - jest - .spyOn(GitFileSystemService, "getFilePathStats") + + const spyGetGitBlobHash = jest + .spyOn(GitFileSystemService, "getGitBlobHash") + .mockReturnValueOnce(okAsync("correct-sha")) + + const actual = await GitFileSystemService.delete( + "fake-repo", + "fake-dir", + "fake-old-hash", + "fake-user-id", + false + ) + expect(spyGetGitBlobHash).toBeCalledTimes(1) + expect(actual._unsafeUnwrapErr()).toBeInstanceOf(ConflictError) + }) + }) + + describe("deleteDirectory", () => { + it("should delete a directory successfully", async () => { + MockSimpleGit.cwd.mockReturnValueOnce({ + log: jest.fn().mockResolvedValueOnce({ + latest: { + author_name: "fake-author", + author_email: "fake-email", + date: "fake-date", + message: "fake-message", + hash: "test-commit-sha", + }, + }), + }) + + // commit + MockSimpleGit.cwd.mockReturnValueOnce({ + checkIsRepo: jest.fn().mockResolvedValueOnce(true), + }) + + // commit + MockSimpleGit.cwd.mockReturnValueOnce({ + remote: jest + .fn() .mockResolvedValueOnce( - okAsync({ - ...mockStats, - isFile: () => true, - isDirectory: () => false, - }) - ) - - const spyGetGitBlobHash = jest - .spyOn(GitFileSystemService, "getGitBlobHash") - .mockReturnValueOnce(okAsync("correct-sha")) - - const actual = await GitFileSystemService.delete( - "fake-repo", - "fake-dir", - "fake-old-hash", - "fake-user-id", - false + `git@github.com:${ISOMER_GITHUB_ORG_NAME}/fake-repo.git` + ), + }) + + // commit + MockSimpleGit.cwd.mockReturnValueOnce({ + revparse: jest.fn().mockResolvedValueOnce(BRANCH_REF), + }) + + // commit + MockSimpleGit.cwd.mockReturnValueOnce({ + add: jest.fn().mockResolvedValueOnce(undefined), + }) + + // commit + MockSimpleGit.cwd.mockReturnValueOnce({ + commit: jest.fn().mockResolvedValueOnce({ commit: "fake-new-hash" }), + }) + + const actual = await GitFileSystemService.delete( + "fake-repo", + "fake-dir", + "", + "fake-user-id", + true + ) + + expect(actual._unsafeUnwrap()).toEqual("fake-new-hash") + }) + + it("should return a error if the directory is not valid", async () => { + // getLatestCommitOfBranch + MockSimpleGit.cwd.mockReturnValueOnce({ + log: jest.fn().mockResolvedValueOnce({ + latest: { + author_name: "fake-author", + author_email: "fake-email", + date: "fake-date", + message: "fake-message", + hash: "test-commit-sha", + }, + }), + }) + const mockStats = new Stats() + const spyGetFilePathStats = jest + .spyOn(GitFileSystemService, "getFilePathStats") + .mockResolvedValueOnce( + okAsync({ + ...mockStats, + isFile: () => true, + isDirectory: () => false, + }) ) - expect(spyGetGitBlobHash).toBeCalledTimes(1) - expect(actual._unsafeUnwrapErr()).toBeInstanceOf(ConflictError) - }) - }) - - describe("deleteDirectory", () => { - it("should delete a directory successfully", async () => { - MockSimpleGit.cwd.mockReturnValueOnce({ - log: jest.fn().mockResolvedValueOnce({ - latest: { - author_name: "fake-author", - author_email: "fake-email", - date: "fake-date", - message: "fake-message", - hash: "test-commit-sha", - }, - }), - }) - - // commit - MockSimpleGit.cwd.mockReturnValueOnce({ - checkIsRepo: jest.fn().mockResolvedValueOnce(true), - }) - - // commit - MockSimpleGit.cwd.mockReturnValueOnce({ - remote: jest - .fn() - .mockResolvedValueOnce( - `git@github.com:${ISOMER_GITHUB_ORG_NAME}/fake-repo.git` - ), - }) - - // commit - MockSimpleGit.cwd.mockReturnValueOnce({ - revparse: jest.fn().mockResolvedValueOnce(BRANCH_REF), - }) - - // commit - MockSimpleGit.cwd.mockReturnValueOnce({ - add: jest.fn().mockResolvedValueOnce(undefined), - }) - - // commit - MockSimpleGit.cwd.mockReturnValueOnce({ - commit: jest.fn().mockResolvedValueOnce({ commit: "fake-new-hash" }), - }) - - const actual = await GitFileSystemService.delete( - "fake-repo", - "fake-dir", - "", - "fake-user-id", - true + + const actual = await GitFileSystemService.delete( + "fake-repo", + "fake-dir", + "", + "fake-user-id", + true + ) + expect(spyGetFilePathStats).toBeCalledTimes(1) + expect(actual._unsafeUnwrapErr()).toBeInstanceOf(GitFileSystemError) + }) + + it("should rollback changes if an error occurred when committing", async () => { + MockSimpleGit.cwd.mockReturnValueOnce({ + log: jest.fn().mockResolvedValueOnce({ + latest: { + author_name: "fake-author", + author_email: "fake-email", + date: "fake-date", + message: "fake-message", + hash: "test-commit-sha", + }, + }), + }) + const mockStats = new Stats() + jest + .spyOn(GitFileSystemService, "getFilePathStats") + .mockResolvedValueOnce( + okAsync({ + ...mockStats, + isFile: () => false, + isDirectory: () => true, + }) ) - expect(actual._unsafeUnwrap()).toEqual("fake-new-hash") - }) - - it("should return a error if the directory is not valid", async () => { - // getLatestCommitOfBranch - MockSimpleGit.cwd.mockReturnValueOnce({ - log: jest.fn().mockResolvedValueOnce({ - latest: { - author_name: "fake-author", - author_email: "fake-email", - date: "fake-date", - message: "fake-message", - hash: "test-commit-sha", - }, - }), - }) - const mockStats = new Stats() - const spyGetFilePathStats = jest - .spyOn(GitFileSystemService, "getFilePathStats") + MockSimpleGit.cwd.mockReturnValueOnce({ + checkIsRepo: jest.fn().mockResolvedValueOnce(true), + }) + MockSimpleGit.cwd.mockReturnValueOnce({ + remote: jest + .fn() .mockResolvedValueOnce( - okAsync({ - ...mockStats, - isFile: () => true, - isDirectory: () => false, - }) - ) - - const actual = await GitFileSystemService.delete( - "fake-repo", - "fake-dir", - "", - "fake-user-id", - true - ) - expect(spyGetFilePathStats).toBeCalledTimes(1) - expect(actual._unsafeUnwrapErr()).toBeInstanceOf(GitFileSystemError) - }) - - it("should rollback changes if an error occurred when committing", async () => { - MockSimpleGit.cwd.mockReturnValueOnce({ - log: jest.fn().mockResolvedValueOnce({ - latest: { - author_name: "fake-author", - author_email: "fake-email", - date: "fake-date", - message: "fake-message", - hash: "test-commit-sha", - }, - }), - }) - const mockStats = new Stats() - jest - .spyOn(GitFileSystemService, "getFilePathStats") + `git@github.com:${ISOMER_GITHUB_ORG_NAME}/fake-repo.git` + ), + }) + MockSimpleGit.cwd.mockReturnValueOnce({ + revparse: jest.fn().mockResolvedValueOnce(BRANCH_REF), + }) + MockSimpleGit.cwd.mockReturnValueOnce({ + add: jest.fn().mockResolvedValueOnce(undefined), + }) + + MockSimpleGit.cwd.mockReturnValueOnce({ + commit: jest.fn().mockRejectedValueOnce(new GitError()), + }) + MockSimpleGit.cwd.mockReturnValueOnce({ + reset: jest.fn().mockReturnValueOnce({ + clean: jest.fn().mockResolvedValueOnce(undefined), + }), + }) + + const spyRollback = jest.spyOn(GitFileSystemService, "rollback") + + const actual = await GitFileSystemService.delete( + "fake-repo", + "fake-dir", + "fake new content", + "fake-user-id", + true + ) + + expect(actual._unsafeUnwrapErr()).toBeInstanceOf(GitFileSystemError) + expect(spyRollback).toHaveBeenCalledWith("fake-repo", "test-commit-sha") + }) + }) + + describe("updateRepoState", () => { + it("should successfully update the repo state for a valid Git repo", async () => { + const mockResetFn = jest.fn().mockResolvedValueOnce("reset") + + MockSimpleGit.cwd.mockReturnValueOnce({ + checkIsRepo: jest.fn().mockResolvedValueOnce(true), + }) + MockSimpleGit.cwd.mockReturnValueOnce({ + remote: jest + .fn() .mockResolvedValueOnce( - okAsync({ - ...mockStats, - isFile: () => false, - isDirectory: () => true, - }) - ) - - MockSimpleGit.cwd.mockReturnValueOnce({ - checkIsRepo: jest.fn().mockResolvedValueOnce(true), - }) - MockSimpleGit.cwd.mockReturnValueOnce({ - remote: jest - .fn() - .mockResolvedValueOnce( - `git@github.com:${ISOMER_GITHUB_ORG_NAME}/fake-repo.git` - ), - }) - MockSimpleGit.cwd.mockReturnValueOnce({ - revparse: jest.fn().mockResolvedValueOnce(BRANCH_REF), - }) - MockSimpleGit.cwd.mockReturnValueOnce({ - add: jest.fn().mockResolvedValueOnce(undefined), - }) - - MockSimpleGit.cwd.mockReturnValueOnce({ - commit: jest.fn().mockRejectedValueOnce(new GitError()), - }) - MockSimpleGit.cwd.mockReturnValueOnce({ - reset: jest.fn().mockReturnValueOnce({ - clean: jest.fn().mockResolvedValueOnce(undefined), - }), - }) - - const spyRollback = jest.spyOn(GitFileSystemService, "rollback") - - const actual = await GitFileSystemService.delete( - "fake-repo", - "fake-dir", - "fake new content", - "fake-user-id", - true - ) + `git@github.com:${ISOMER_GITHUB_ORG_NAME}/fake-repo.git` + ), + }) + MockSimpleGit.cwd.mockReturnValueOnce({ + revparse: jest.fn().mockResolvedValueOnce(BRANCH_REF), + }) + MockSimpleGit.cwd.mockReturnValueOnce({ + catFile: jest.fn().mockResolvedValueOnce("commit"), + }) + MockSimpleGit.cwd.mockReturnValueOnce({ + reset: mockResetFn, + }) + MockSimpleGit.cwd.mockReturnValueOnce({ + checkIsRepo: jest.fn().mockResolvedValueOnce(true), + }) + MockSimpleGit.cwd.mockReturnValueOnce({ + remote: jest + .fn() + .mockResolvedValueOnce( + `git@github.com:${ISOMER_GITHUB_ORG_NAME}/fake-repo.git` + ), + }) + MockSimpleGit.cwd.mockReturnValueOnce({ + revparse: jest.fn().mockResolvedValueOnce(BRANCH_REF), + }) + MockSimpleGit.cwd.mockReturnValueOnce({ + push: jest.fn().mockResolvedValueOnce(undefined), + }) + + const actual = await GitFileSystemService.updateRepoState( + "fake-repo", + BRANCH_REF, + "fake-sha" + ) + + expect(actual._unsafeUnwrap()).toBeUndefined() + expect(mockResetFn).toHaveBeenCalledWith(["--hard", "fake-sha"]) + }) + + it("should successfully update the repo state for a valid Git repo with a non-standard branch", async () => { + const mockResetFn = jest.fn().mockResolvedValueOnce("reset") + const nonStandardBranchRef = "non-standard-branch" + + MockSimpleGit.cwd.mockReturnValueOnce({ + checkIsRepo: jest.fn().mockResolvedValueOnce(true), + }) + MockSimpleGit.cwd.mockReturnValueOnce({ + remote: jest + .fn() + .mockResolvedValueOnce( + `git@github.com:${ISOMER_GITHUB_ORG_NAME}/fake-repo.git` + ), + }) + MockSimpleGit.cwd.mockReturnValueOnce({ + revparse: jest.fn().mockResolvedValueOnce(nonStandardBranchRef), + }) + MockSimpleGit.cwd.mockReturnValueOnce({ + catFile: jest.fn().mockResolvedValueOnce("commit"), + }) + MockSimpleGit.cwd.mockReturnValueOnce({ + reset: mockResetFn, + }) + MockSimpleGit.cwd.mockReturnValueOnce({ + checkIsRepo: jest.fn().mockResolvedValueOnce(true), + }) + MockSimpleGit.cwd.mockReturnValueOnce({ + remote: jest + .fn() + .mockResolvedValueOnce( + `git@github.com:${ISOMER_GITHUB_ORG_NAME}/fake-repo.git` + ), + }) + MockSimpleGit.cwd.mockReturnValueOnce({ + revparse: jest.fn().mockResolvedValueOnce(nonStandardBranchRef), + }) + MockSimpleGit.cwd.mockReturnValueOnce({ + push: jest.fn().mockResolvedValueOnce(undefined), + }) + + const actual = await GitFileSystemService.updateRepoState( + "fake-repo", + nonStandardBranchRef, + "another-fake-sha" + ) + + expect(actual._unsafeUnwrap()).toBeUndefined() + expect(mockResetFn).toHaveBeenCalledWith(["--hard", "another-fake-sha"]) + }) + + it("should return an error if an error occurred when resetting the repo", async () => { + MockSimpleGit.cwd.mockReturnValueOnce({ + checkIsRepo: jest.fn().mockResolvedValueOnce(true), + }) + MockSimpleGit.cwd.mockReturnValueOnce({ + remote: jest + .fn() + .mockResolvedValueOnce( + `git@github.com:${ISOMER_GITHUB_ORG_NAME}/fake-repo.git` + ), + }) + MockSimpleGit.cwd.mockReturnValueOnce({ + revparse: jest.fn().mockResolvedValueOnce(BRANCH_REF), + }) + MockSimpleGit.cwd.mockReturnValueOnce({ + catFile: jest.fn().mockResolvedValueOnce("commit"), + }) + MockSimpleGit.cwd.mockReturnValueOnce({ + reset: jest.fn().mockRejectedValueOnce(new GitError()), + }) + + const actual = await GitFileSystemService.updateRepoState( + "fake-repo", + BRANCH_REF, + "fake-sha" + ) + + expect(actual._unsafeUnwrapErr()).toBeInstanceOf(GitFileSystemError) + }) + + it("should return a BadRequestError if the SHA does not exist on the branch", async () => { + MockSimpleGit.cwd.mockReturnValueOnce({ + checkIsRepo: jest.fn().mockResolvedValueOnce(true), + }) + MockSimpleGit.cwd.mockReturnValueOnce({ + remote: jest + .fn() + .mockResolvedValueOnce( + `git@github.com:${ISOMER_GITHUB_ORG_NAME}/fake-repo.git` + ), + }) + MockSimpleGit.cwd.mockReturnValueOnce({ + revparse: jest.fn().mockResolvedValueOnce(BRANCH_REF), + }) + MockSimpleGit.cwd.mockReturnValueOnce({ + catFile: jest.fn().mockRejectedValueOnce(new GitError()), + }) + + const actual = await GitFileSystemService.updateRepoState( + "fake-repo", + BRANCH_REF, + "fake-sha" + ) + + expect(actual._unsafeUnwrapErr()).toBeInstanceOf(BadRequestError) + }) + + it("should return an error if an error occurred when checking if the SHA exists on the branch", async () => { + MockSimpleGit.cwd.mockReturnValueOnce({ + checkIsRepo: jest.fn().mockResolvedValueOnce(true), + }) + MockSimpleGit.cwd.mockReturnValueOnce({ + remote: jest + .fn() + .mockResolvedValueOnce( + `git@github.com:${ISOMER_GITHUB_ORG_NAME}/fake-repo.git` + ), + }) + MockSimpleGit.cwd.mockReturnValueOnce({ + revparse: jest.fn().mockResolvedValueOnce(BRANCH_REF), + }) + MockSimpleGit.cwd.mockReturnValueOnce({ + catFile: jest.fn().mockRejectedValueOnce(new Error()), + }) - expect(actual._unsafeUnwrapErr()).toBeInstanceOf(GitFileSystemError) - expect(spyRollback).toHaveBeenCalledWith("fake-repo", "test-commit-sha") + const actual = await GitFileSystemService.updateRepoState( + "fake-repo", + BRANCH_REF, + "fake-sha" + ) + + expect(actual._unsafeUnwrapErr()).toBeInstanceOf(GitFileSystemError) + }) + + it("should return an error if the repo is not a valid Git repo", async () => { + MockSimpleGit.cwd.mockReturnValueOnce({ + checkIsRepo: jest.fn().mockResolvedValueOnce(false), }) + + const actual = await GitFileSystemService.updateRepoState( + "fake-repo", + BRANCH_REF, + "fake-sha" + ) + + expect(actual._unsafeUnwrapErr()).toBeInstanceOf(GitFileSystemError) }) }) }) diff --git a/src/services/db/__tests__/GitHubService.spec.ts b/src/services/db/__tests__/GitHubService.spec.ts index 95206a632..26778b547 100644 --- a/src/services/db/__tests__/GitHubService.spec.ts +++ b/src/services/db/__tests__/GitHubService.spec.ts @@ -897,6 +897,20 @@ describe("Github Service", () => { authHeader ) }) + + it("should update a repo state for a non-standard branch correctly", async () => { + const branchName = "test-branch" + const branchRefEndpoint = `${siteName}/git/refs/heads/${branchName}` + await service.updateRepoState(sessionData, { + commitSha: sha, + branchName, + }) + expect(mockAxiosInstance.patch).toHaveBeenCalledWith( + branchRefEndpoint, + { sha, force: true }, + authHeader + ) + }) }) describe("checkHasAccess", () => { diff --git a/src/services/db/__tests__/RepoService.spec.ts b/src/services/db/__tests__/RepoService.spec.ts index c5f2bb685..eb52fb77c 100644 --- a/src/services/db/__tests__/RepoService.spec.ts +++ b/src/services/db/__tests__/RepoService.spec.ts @@ -1,6 +1,8 @@ import { AxiosCacheInstance } from "axios-cache-interceptor" import { okAsync } from "neverthrow" +import config from "@config/config" + import { mockAccessToken, mockEmail, @@ -21,6 +23,8 @@ import _RepoService from "@services/db/RepoService" import { GitHubService } from "../GitHubService" +const BRANCH_REF = config.get("github.branchRef") + const MockAxiosInstance = { put: jest.fn(), get: jest.fn(), @@ -40,6 +44,7 @@ const MockGitFileSystemService = { getLatestCommitOfBranch: jest.fn(), renameSinglePath: jest.fn(), moveFiles: jest.fn(), + updateRepoState: jest.fn(), } const RepoService = new _RepoService( @@ -245,6 +250,81 @@ describe("RepoService", () => { }) }) + describe("readMediaFile", () => { + it("should read image from the local Git file system for whitelisted repos", async () => { + const expected: MediaFileOutput = { + name: "test content", + sha: "test-sha", + mediaUrl: "sampleBase64Img", + mediaPath: "images/test-img.jpeg", + type: "image" as ItemType, + } + MockGitFileSystemService.readMediaFile.mockResolvedValueOnce( + okAsync(expected) + ) + + const actual = await RepoService.readMediaFile( + mockUserWithSiteSessionDataAndGrowthBook, + { + directoryName: "test", + fileName: "test content", + } + ) + + expect(actual).toEqual(expected) + }) + + it("should read image from GitHub for whitelisted repos", async () => { + const sessionData: UserWithSiteSessionData = new UserWithSiteSessionData({ + githubId: mockGithubId, + accessToken: mockAccessToken, + isomerUserId: mockIsomerUserId, + email: mockEmail, + siteName: "not-whitelisted", + }) + + const expected: MediaFileOutput = { + name: "test-image", + sha: "test-sha", + mediaUrl: "http://some-cdn.com/image", + mediaPath: "images/test-img.jpeg", + type: "image" as ItemType, + } + + const gitHubServiceReadDirectory = jest.spyOn( + GitHubService.prototype, + "readDirectory" + ) + const gitHubServiceGetRepoInfo = jest.spyOn( + GitHubService.prototype, + "getRepoInfo" + ) + gitHubServiceReadDirectory.mockResolvedValueOnce([ + { + name: ".keep", + }, + { + name: "test-image", + }, + { + name: "fake-dir", + }, + ]) + gitHubServiceGetRepoInfo.mockResolvedValueOnce({ private: false }) + const getMediaFileInfo = jest + .spyOn(mediaUtils, "getMediaFileInfo") + .mockResolvedValueOnce(expected) + + const actual = await RepoService.readMediaFile(sessionData, { + directoryName: "images", + fileName: "test-image", + }) + + expect(actual).toEqual(expected) + expect(getMediaFileInfo).toBeCalledTimes(1) + }) + }) + describe("readDirectory", () => { it("should read from the local Git file system if the repo is whitelisted", async () => { const expected: GitDirectoryItem[] = [ @@ -329,6 +409,115 @@ describe("RepoService", () => { }) }) + describe("readMediaDirectory", () => { + it("should return an array of files and directories from disk if repo is whitelisted", async () => { + const image: MediaFileOutput = { + name: "image-name", + sha: "test-sha", + mediaUrl: "base64ofimage", + mediaPath: "images/image-name.jpg", + type: "file", + } + const dir: MediaDirOutput = { + name: "imageDir", + type: "dir", + } + const expected = [image, dir] + MockGitFileSystemService.listDirectoryContents.mockResolvedValueOnce( + okAsync([ + { + name: "image-name", + type: "file", + sha: "test-sha", + path: "images/image-name.jpg", + }, + { + name: "imageDir", + type: "dir", + sha: "test-sha", + path: "images/imageDir", + }, + { + name: ".keep", + type: "file", + sha: "test-sha", + path: "images/.keep", + }, + ]) + ) + MockGitFileSystemService.readMediaFile.mockResolvedValueOnce( + okAsync(image) + ) + + const actual = await RepoService.readMediaDirectory( + mockUserWithSiteSessionDataAndGrowthBook, + "images" + ) + + expect(actual).toEqual(expected) + }) + + it("should return an array of files and directories from GitHub if repo is not whitelisted", async () => { + const sessionData: UserWithSiteSessionData = new UserWithSiteSessionData({ + githubId: mockGithubId, + accessToken: mockAccessToken, + isomerUserId: mockIsomerUserId, + email: mockEmail, + siteName: "not-whitelisted", + }) + + const image: MediaFileOutput = { + name: "image-name", + sha: "test-sha", + mediaUrl: "base64ofimage", + mediaPath: "images/image-name.jpg", + type: "file", + } + const dir: MediaDirOutput = { + name: "imageDir", + type: "dir", + } + const expected = [image, dir] + + const gitHubServiceGetRepoInfo = jest + .spyOn(GitHubService.prototype, "getRepoInfo") + .mockResolvedValueOnce({ private: false }) + const gitHubServiceReadDirectory = jest + .spyOn(GitHubService.prototype, "readDirectory") + .mockResolvedValueOnce([ + { + name: "image-name", + type: "file", + sha: "test-sha", + path: "images/image-name.jpg", + }, + { + name: "imageDir", + type: "dir", + sha: "test-sha", + path: "images/imageDir", + }, + { + name: ".keep", + type: "file", + sha: "test-sha", + path: "images/.keep", + }, + ]) + + const repoServiceReadMediaFile = jest + .spyOn(_RepoService.prototype, "readMediaFile") + .mockResolvedValueOnce(image) + + const actual = await RepoService.readMediaDirectory(sessionData, "images") + + expect(actual).toEqual(expected) + expect(gitHubServiceGetRepoInfo).toBeCalledTimes(1) + expect(gitHubServiceReadDirectory).toBeCalledTimes(1) + expect(repoServiceReadMediaFile).toBeCalledTimes(1) + }) + }) + describe("update", () => { it("should update the local Git file system if the repo is whitelisted", async () => { const expectedSha = "fake-commit-sha" @@ -373,6 +562,59 @@ describe("RepoService", () => { }) }) + describe("delete", () => { + it("should delete a file from Git file system when repo is whitelisted", async () => { + MockGitFileSystemService.delete.mockResolvedValueOnce( + okAsync("some-fake-sha") + ) + + await RepoService.delete(mockUserWithSiteSessionDataAndGrowthBook, { + sha: "fake-original-sha", + fileName: "test.md", + directoryName: "pages", + }) + + expect(MockGitFileSystemService.delete).toBeCalledTimes(1) + expect(MockGitFileSystemService.delete).toBeCalledWith( + mockUserWithSiteSessionDataAndGrowthBook.siteName, + "pages/test.md", + "fake-original-sha", + mockUserWithSiteSessionDataAndGrowthBook.isomerUserId, + false + ) + expect(MockGitFileSystemService.push).toBeCalledTimes(1) + expect(MockGitFileSystemService.push).toBeCalledWith( + mockUserWithSiteSessionDataAndGrowthBook.siteName, + BRANCH_REF + ) + }) + + it("should delete a file from GitHub when repo is not whitelisted", async () => { + const sessionData: UserWithSiteSessionData = new UserWithSiteSessionData({ + githubId: mockGithubId, + accessToken: mockAccessToken, + isomerUserId: mockIsomerUserId, + email: mockEmail, + siteName: "not-whitelisted", + }) + + const gitHubServiceDelete = jest.spyOn(GitHubService.prototype, "delete") + + await RepoService.delete(sessionData, { + sha: "fake-original-sha", + fileName: "test.md", + directoryName: "pages", + }) + + expect(gitHubServiceDelete).toBeCalledTimes(1) + expect(gitHubServiceDelete).toBeCalledWith(sessionData, { + sha: "fake-original-sha", + fileName: "test.md", + directoryName: "pages", + }) + }) + }) + describe("renameSinglePath", () => { it("should rename using the local Git file system if the repo is whitelisted", async () => { const expectedSha = "fake-commit-sha" @@ -608,31 +850,24 @@ describe("RepoService", () => { }) }) - describe("readMediaFile", () => { - it("should read image from the local Git file system for whitelisted repos", async () => { - const expected: MediaFileOutput = { - name: "test content", - sha: "test-sha", - mediaUrl: "sampleBase64Img", - mediaPath: "images/test-img.jpeg", - type: "image" as ItemType, - } - MockGitFileSystemService.readMediaFile.mockResolvedValueOnce( - okAsync(expected) + describe("updateRepoState", () => { + it("should update the repo state on the local Git file system if the repo is whitelisted", async () => { + MockGitFileSystemService.updateRepoState.mockResolvedValueOnce( + okAsync(undefined) ) - const actual = await RepoService.readMediaFile( + await RepoService.updateRepoState( mockUserWithSiteSessionDataAndGrowthBook, { - directoryName: "test", - fileName: "test content", + commitSha: "fake-sha", + branchName: "master", } ) - expect(actual).toEqual(expected) + expect(MockGitFileSystemService.updateRepoState).toBeCalledTimes(1) }) - it("should read image from GitHub for whitelisted repos", async () => { + it("should update the repo state on GitHub if the repo is not whitelisted", async () => { const sessionData: UserWithSiteSessionData = new UserWithSiteSessionData({ githubId: mockGithubId, accessToken: mockAccessToken, @@ -640,207 +875,18 @@ describe("RepoService", () => { email: mockEmail, siteName: "not-whitelisted", }) - - const expected: MediaFileOutput = { - name: "test-image", - sha: "test-sha", - mediaUrl: "http://some-cdn.com/image", - mediaPath: "images/test-img.jpeg", - type: "image" as ItemType, - } - - const gitHubServiceReadDirectory = jest.spyOn( - GitHubService.prototype, - "readDirectory" - ) - const gitHubServiceGetRepoInfo = jest.spyOn( + const gitHubServiceUpdateRepoState = jest.spyOn( GitHubService.prototype, - "getRepoInfo" - ) - gitHubServiceReadDirectory.mockResolvedValueOnce([ - { - name: ".keep", - }, - { - name: "test-image", - }, - { - name: "fake-dir", - }, - ]) - gitHubServiceGetRepoInfo.mockResolvedValueOnce({ private: false }) - const getMediaFileInfo = jest - .spyOn(mediaUtils, "getMediaFileInfo") - .mockResolvedValueOnce(expected) - - const actual = await RepoService.readMediaFile(sessionData, { - directoryName: "images", - fileName: "test-image", - }) - - expect(actual).toEqual(expected) - expect(getMediaFileInfo).toBeCalledTimes(1) - }) - }) - - describe("readMediaDirectory", () => { - it("should return an array of files and directories from disk if repo is whitelisted", async () => { - const image: MediaFileOutput = { - name: "image-name", - sha: "test-sha", - mediaUrl: "base64ofimage", - mediaPath: "images/image-name.jpg", - type: "file", - } - const dir: MediaDirOutput = { - name: "imageDir", - type: "dir", - } - const expected = [image, dir] - MockGitFileSystemService.listDirectoryContents.mockResolvedValueOnce( - okAsync([ - { - name: "image-name", - type: "file", - sha: "test-sha", - path: "images/image-name.jpg", - }, - { - name: "imageDir", - type: "dir", - sha: "test-sha", - path: "images/imageDir", - }, - { - name: ".keep", - type: "file", - sha: "test-sha", - path: "images/.keep", - }, - ]) - ) - MockGitFileSystemService.readMediaFile.mockResolvedValueOnce( - okAsync(image) - ) - - const actual = await RepoService.readMediaDirectory( - mockUserWithSiteSessionDataAndGrowthBook, - "images" - ) - - expect(actual).toEqual(expected) - }) - - it("should return an array of files and directories from GitHub if repo is not whitelisted", async () => { - const sessionData: UserWithSiteSessionData = new UserWithSiteSessionData({ - githubId: mockGithubId, - accessToken: mockAccessToken, - isomerUserId: mockIsomerUserId, - email: mockEmail, - siteName: "not-whitelisted", - }) - - const image: MediaFileOutput = { - name: "image-name", - sha: "test-sha", - mediaUrl: "base64ofimage", - mediaPath: "images/image-name.jpg", - type: "file", - } - const dir: MediaDirOutput = { - name: "imageDir", - type: "dir", - } - const expected = [image, dir] - - const gitHubServiceGetRepoInfo = jest - .spyOn(GitHubService.prototype, "getRepoInfo") - .mockResolvedValueOnce({ private: false }) - const gitHubServiceReadDirectory = jest - .spyOn(GitHubService.prototype, "readDirectory") - .mockResolvedValueOnce([ - { - name: "image-name", - type: "file", - sha: "test-sha", - path: "images/image-name.jpg", - }, - { - name: "imageDir", - type: "dir", - sha: "test-sha", - path: "images/imageDir", - }, - { - name: ".keep", - type: "file", - sha: "test-sha", - path: "images/.keep", - }, - ]) - - const repoServiceReadMediaFile = jest - .spyOn(_RepoService.prototype, "readMediaFile") - .mockResolvedValueOnce(image) - - const actual = await RepoService.readMediaDirectory(sessionData, "images") - - expect(actual).toEqual(expected) - expect(gitHubServiceGetRepoInfo).toBeCalledTimes(1) - expect(gitHubServiceReadDirectory).toBeCalledTimes(1) - expect(repoServiceReadMediaFile).toBeCalledTimes(1) - }) - }) - - describe("delete", () => { - it("should delete a file from Git file system when repo is whitelisted", async () => { - MockGitFileSystemService.delete.mockResolvedValueOnce( - okAsync("some-fake-sha") - ) - - await RepoService.delete(mockUserWithSiteSessionDataAndGrowthBook, { - sha: "fake-original-sha", - fileName: "test.md", - directoryName: "pages", - }) - - expect(MockGitFileSystemService.delete).toBeCalledTimes(1) - expect(MockGitFileSystemService.delete).toBeCalledWith( - mockUserWithSiteSessionDataAndGrowthBook.siteName, - "pages/test.md", - "fake-original-sha", - mockUserWithSiteSessionDataAndGrowthBook.isomerUserId, - false - ) - expect(MockGitFileSystemService.push).toBeCalledTimes(1) - expect(MockGitFileSystemService.push).toBeCalledWith( - mockUserWithSiteSessionDataAndGrowthBook.siteName + "updateRepoState" ) - }) - - it("should delete a file from GitHub when repo is not whitelisted", async () => { - const sessionData: UserWithSiteSessionData = new UserWithSiteSessionData({ - githubId: mockGithubId, - accessToken: mockAccessToken, - isomerUserId: mockIsomerUserId, - email: mockEmail, - siteName: "not-whitelisted", - }) - - const gitHubServiceDelete = jest.spyOn(GitHubService.prototype, "delete") + gitHubServiceUpdateRepoState.mockResolvedValueOnce(undefined) - await RepoService.delete(sessionData, { - sha: "fake-original-sha", - fileName: "test.md", - directoryName: "pages", + await RepoService.updateRepoState(sessionData, { + commitSha: "fake-sha", + branchName: "master", }) - expect(gitHubServiceDelete).toBeCalledTimes(1) - expect(gitHubServiceDelete).toBeCalledWith(sessionData, { - sha: "fake-original-sha", - fileName: "test.md", - directoryName: "pages", - }) + expect(gitHubServiceUpdateRepoState).toBeCalledTimes(1) }) }) }) From ef953934707b0924ab23dfb6298ae1188e7c7584 Mon Sep 17 00:00:00 2001 From: Harish Date: Thu, 28 Sep 2023 17:00:03 +0800 Subject: [PATCH 3/8] IS-621: fix issues for staging deploy node 18 (#951) * feat: swap to bcryptjs * feat: fix import * temp: staging test --- .github/workflows/ci.yml | 2 +- package-lock.json | 84 +++++++++++++++++------------ package.json | 4 +- src/services/identity/OtpService.ts | 2 +- 4 files changed, 53 insertions(+), 39 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5be62ceb7..eec3af49c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,7 +10,7 @@ env: STAGING_BRANCH: refs/heads/staging EB_APP: isomer-cms EB_ENV_PRODUCTION: cms-backend-prod-node16 - EB_ENV_STAGING: cms-backend-staging-node16 + EB_ENV_STAGING: cms-backend-staging-node18 COMMIT_MESSAGE: ${{ github.event.head_commit.message }} jobs: diff --git a/package-lock.json b/package-lock.json index 3908b71e5..f4ffbafac 100644 --- a/package-lock.json +++ b/package-lock.json @@ -24,7 +24,7 @@ "axios": "~0.27.2", "axios-cache-interceptor": "^0.10.7", "base-64": "^0.1.0", - "bcrypt": "^5.1.0", + "bcryptjs": "^2.4.3", "bluebird": "^3.7.2", "body-parser": "^1.19.2", "cache-parser": "^1.2.4", @@ -85,7 +85,7 @@ "@octokit/types": "^6.35.0", "@tsconfig/recommended": "^1.0.1", "@types/aws-lambda": "^8.10.106", - "@types/bcrypt": "^5.0.0", + "@types/bcryptjs": "^2.4.4", "@types/bluebird": "^3.5.38", "@types/convict": "^6.1.1", "@types/cookie-parser": "^1.4.3", @@ -2867,6 +2867,7 @@ "version": "1.0.11", "resolved": "https://registry.npmjs.org/@mapbox/node-pre-gyp/-/node-pre-gyp-1.0.11.tgz", "integrity": "sha512-Yhlar6v9WQgUp/He7BdgzOz8lqMQ8sU+jkCq7Wx8Myc5YFJLbEe7lgui/V7G1qB1DJykHSGwreceSaD60Y0PUQ==", + "dev": true, "dependencies": { "detect-libc": "^2.0.0", "https-proxy-agent": "^5.0.0", @@ -4253,14 +4254,11 @@ "@babel/types": "^7.20.7" } }, - "node_modules/@types/bcrypt": { - "version": "5.0.0", - "resolved": "https://registry.npmjs.org/@types/bcrypt/-/bcrypt-5.0.0.tgz", - "integrity": "sha512-agtcFKaruL8TmcvqbndlqHPSJgsolhf/qPWchFlgnW1gECTN/nKbFcoFnvKAQRFfKbh+BO6A3SWdJu9t+xF3Lw==", - "dev": true, - "dependencies": { - "@types/node": "*" - } + "node_modules/@types/bcryptjs": { + "version": "2.4.4", + "resolved": "https://registry.npmjs.org/@types/bcryptjs/-/bcryptjs-2.4.4.tgz", + "integrity": "sha512-9wlJI7k5gRyJEC4yrV7DubzNQFTPiykYxUA6lBtsk5NlOfW9oWLJ1HdIA4YtE+6C3i3mTpDQQEosJ2rVZfBWnw==", + "dev": true }, "node_modules/@types/bluebird": { "version": "3.5.38", @@ -4887,7 +4885,8 @@ "node_modules/abbrev": { "version": "1.1.1", "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz", - "integrity": "sha512-nne9/IiQ/hzIhY6pdDnbBtz7DjPTKrY00P/zvPSm5pOFkl6xuGrGnXn/VtTNNfNtAfZ9/1RtehkszU9qcTii0Q==" + "integrity": "sha512-nne9/IiQ/hzIhY6pdDnbBtz7DjPTKrY00P/zvPSm5pOFkl6xuGrGnXn/VtTNNfNtAfZ9/1RtehkszU9qcTii0Q==", + "dev": true }, "node_modules/accepts": { "version": "1.3.8", @@ -5079,6 +5078,7 @@ "version": "5.0.1", "resolved": "https://registry.npmjs.org/ansi-regex/-/ansi-regex-5.0.1.tgz", "integrity": "sha512-quJQXlTSUGL2LH9SUXo8VwsY4soanhgo6LNSm84E1LBcE8s3O0wpdiRzyR9z/ZZJMlMWv37qOOb9pdJlMUEKFQ==", + "dev": true, "engines": { "node": ">=8" } @@ -5113,12 +5113,14 @@ "node_modules/aproba": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/aproba/-/aproba-2.0.0.tgz", - "integrity": "sha512-lYe4Gx7QT+MKGbDsA+Z+he/Wtef0BiwDOlK/XkBrdfsh9J/jPPXbX0tE9x9cl27Tmu5gg3QUbUrQYa/y+KOHPQ==" + "integrity": "sha512-lYe4Gx7QT+MKGbDsA+Z+he/Wtef0BiwDOlK/XkBrdfsh9J/jPPXbX0tE9x9cl27Tmu5gg3QUbUrQYa/y+KOHPQ==", + "dev": true }, "node_modules/are-we-there-yet": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/are-we-there-yet/-/are-we-there-yet-2.0.0.tgz", "integrity": "sha512-Ci/qENmwHnsYo9xKIcUJN5LeDKdJ6R1Z1j9V/J5wyq8nh/mYPEpIKJbBZXtZjG04HiK7zV/p6Vs9952MrMeUIw==", + "dev": true, "dependencies": { "delegates": "^1.0.0", "readable-stream": "^3.6.0" @@ -5543,18 +5545,10 @@ "resolved": "https://registry.npmjs.org/safe-buffer/-/safe-buffer-5.1.2.tgz", "integrity": "sha512-Gd2UZBJDkXlY7GbJxfsE8/nvKkUEU1G38c1siN6QP6a9PT9MmHB8GnpscSmMJSoF8LOIrt8ud/wPtojys4G6+g==" }, - "node_modules/bcrypt": { - "version": "5.1.0", - "resolved": "https://registry.npmjs.org/bcrypt/-/bcrypt-5.1.0.tgz", - "integrity": "sha512-RHBS7HI5N5tEnGTmtR/pppX0mmDSBpQ4aCBsj7CEQfYXDcO74A8sIBYcJMuCsis2E81zDxeENYhv66oZwLiA+Q==", - "hasInstallScript": true, - "dependencies": { - "@mapbox/node-pre-gyp": "^1.0.10", - "node-addon-api": "^5.0.0" - }, - "engines": { - "node": ">= 10.0.0" - } + "node_modules/bcryptjs": { + "version": "2.4.3", + "resolved": "https://registry.npmjs.org/bcryptjs/-/bcryptjs-2.4.3.tgz", + "integrity": "sha512-V/Hy/X9Vt7f3BbPJEi8BdVFMByHi+jNXrYkW3huaybV/kQ0KJg0Y6PkEMbn+zeT+i+SiKZ/HMqJGIIt4LZDqNQ==" }, "node_modules/before-after-hook": { "version": "2.2.3", @@ -5970,6 +5964,7 @@ "version": "2.0.0", "resolved": "https://registry.npmjs.org/chownr/-/chownr-2.0.0.tgz", "integrity": "sha512-bIomtDF5KGpdogkLd9VspvFzk9KfpyyGlS8YFVZl7TGPBHL5snIOnxeshwVgPteQ9b4Eydl+pVbIyE1DcvCWgQ==", + "dev": true, "engines": { "node": ">=10" } @@ -6137,6 +6132,7 @@ "version": "1.1.3", "resolved": "https://registry.npmjs.org/color-support/-/color-support-1.1.3.tgz", "integrity": "sha512-qiBjkpbMLO/HL68y+lh4q0/O1MZFj2RX6X/KmMa3+gJD3z+WwI1ZzDHysvqHGS3mP6mznPckpXmw1nI9cJjyRg==", + "dev": true, "bin": { "color-support": "bin.js" } @@ -6308,7 +6304,8 @@ "node_modules/console-control-strings": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/console-control-strings/-/console-control-strings-1.1.0.tgz", - "integrity": "sha512-ty/fTekppD2fIwRvnZAVdeOiGd1c7YXEixbgJTNzqcxJWKQnjJ/V1bNEEE6hygpM3WjwHFUVK6HTjWSzV4a8sQ==" + "integrity": "sha512-ty/fTekppD2fIwRvnZAVdeOiGd1c7YXEixbgJTNzqcxJWKQnjJ/V1bNEEE6hygpM3WjwHFUVK6HTjWSzV4a8sQ==", + "dev": true }, "node_modules/content-disposition": { "version": "0.5.4", @@ -6767,7 +6764,8 @@ "node_modules/delegates": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/delegates/-/delegates-1.0.0.tgz", - "integrity": "sha512-bd2L678uiWATM6m5Z1VzNCErI3jiGzt6HGY8OVICs40JQq/HALfbyNJmp0UDakEY4pMMaN0Ly5om/B1VI/+xfQ==" + "integrity": "sha512-bd2L678uiWATM6m5Z1VzNCErI3jiGzt6HGY8OVICs40JQq/HALfbyNJmp0UDakEY4pMMaN0Ly5om/B1VI/+xfQ==", + "dev": true }, "node_modules/depd": { "version": "2.0.0", @@ -6813,6 +6811,7 @@ "version": "2.0.1", "resolved": "https://registry.npmjs.org/detect-libc/-/detect-libc-2.0.1.tgz", "integrity": "sha512-463v3ZeIrcWtdgIg6vI6XUncguvr2TnGl4SzDXinkt9mSLpBJKXT3mW6xT3VQdDN11+WVs29pgvivTc4Lp8v+w==", + "dev": true, "engines": { "node": ">=8" } @@ -6970,7 +6969,8 @@ "node_modules/emoji-regex": { "version": "8.0.0", "resolved": "https://registry.npmjs.org/emoji-regex/-/emoji-regex-8.0.0.tgz", - "integrity": "sha512-MSjYzcWNOA0ewAHpz0MxpYFvwg6yjy1NG3xteoqz644VCo/RPgnr1/GGt+ic3iJTzQ8Eu3TdM14SawnVUmGE6A==" + "integrity": "sha512-MSjYzcWNOA0ewAHpz0MxpYFvwg6yjy1NG3xteoqz644VCo/RPgnr1/GGt+ic3iJTzQ8Eu3TdM14SawnVUmGE6A==", + "dev": true }, "node_modules/enabled": { "version": "2.0.0", @@ -8345,6 +8345,7 @@ "version": "2.1.0", "resolved": "https://registry.npmjs.org/fs-minipass/-/fs-minipass-2.1.0.tgz", "integrity": "sha512-V/JgOLFCS+R6Vcq0slCuaeWEdNC3ouDlJMNIsacH2VtALiu9mV4LPrHc5cDl8k5aw6J8jwgWWpiTo5RYhmIzvg==", + "dev": true, "dependencies": { "minipass": "^3.0.0" }, @@ -8413,6 +8414,7 @@ "version": "3.0.2", "resolved": "https://registry.npmjs.org/gauge/-/gauge-3.0.2.tgz", "integrity": "sha512-+5J6MS/5XksCuXq++uFRsnUd7Ovu1XenbeuIuNRJxYWjgQbPuFhT14lAvsWfqfAmnwluf1OwMjz39HjfLPci0Q==", + "dev": true, "dependencies": { "aproba": "^1.0.3 || ^2.0.0", "color-support": "^1.1.2", @@ -8775,7 +8777,8 @@ "node_modules/has-unicode": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/has-unicode/-/has-unicode-2.0.1.tgz", - "integrity": "sha512-8Rf9Y83NBReMnx0gFzA8JImQACstCYWUplepDa9xprwwtmgEZUF0h/i5xSA625zB/I37EtrswSST6OXxwaaIJQ==" + "integrity": "sha512-8Rf9Y83NBReMnx0gFzA8JImQACstCYWUplepDa9xprwwtmgEZUF0h/i5xSA625zB/I37EtrswSST6OXxwaaIJQ==", + "dev": true }, "node_modules/helmet": { "version": "4.6.0", @@ -9298,6 +9301,7 @@ "version": "3.0.0", "resolved": "https://registry.npmjs.org/is-fullwidth-code-point/-/is-fullwidth-code-point-3.0.0.tgz", "integrity": "sha512-zymm5+u+sCsSWyD9qNaejV3DFvhCKclKdizYaJUuHA83RLjb7nSuGnddCHGv0hk+KY7BMAlsWeK4Ueg6EV6XQg==", + "dev": true, "engines": { "node": ">=8" } @@ -12544,6 +12548,7 @@ "version": "3.1.0", "resolved": "https://registry.npmjs.org/make-dir/-/make-dir-3.1.0.tgz", "integrity": "sha512-g3FeP20LNwhALb/6Cz6Dd4F2ngze0jz7tbzrD2wAV+o9FeNHe4rL+yK2md0J/fiSf1sa1ADhXqi5+oVwOM/eGw==", + "dev": true, "dependencies": { "semver": "^6.0.0" }, @@ -12558,6 +12563,7 @@ "version": "6.3.1", "resolved": "https://registry.npmjs.org/semver/-/semver-6.3.1.tgz", "integrity": "sha512-BR7VvDCVHO+q2xBEWskxS6DJE1qRnb7DxzUrogb71CWoSficBxYsiAGd+Kl0mmq/MprG9yArRkyrQxTO6XjMzA==", + "dev": true, "bin": { "semver": "bin/semver.js" } @@ -12735,6 +12741,7 @@ "version": "3.3.6", "resolved": "https://registry.npmjs.org/minipass/-/minipass-3.3.6.tgz", "integrity": "sha512-DxiNidxSEK+tHG6zOIklvNOwm3hvCrbUrdtzY74U6HKTJxvIDfOUL5W5P2Ghd3DTkhhKPYGqeNUIh5qcM4YBfw==", + "dev": true, "dependencies": { "yallist": "^4.0.0" }, @@ -12816,6 +12823,7 @@ "version": "2.1.2", "resolved": "https://registry.npmjs.org/minizlib/-/minizlib-2.1.2.tgz", "integrity": "sha512-bAxsR8BVfj60DWXHE3u30oHzfl4G7khkSuPW+qvpd7jFRHm7dLxOjUk1EHACJ/hxLY8phGJ0YhYHZo7jil7Qdg==", + "dev": true, "dependencies": { "minipass": "^3.0.0", "yallist": "^4.0.0" @@ -12828,6 +12836,7 @@ "version": "1.0.4", "resolved": "https://registry.npmjs.org/mkdirp/-/mkdirp-1.0.4.tgz", "integrity": "sha512-vVqVZQyf3WLx2Shd0qJ9xuvqgAyKPLAiqITEtqW0oIUjzo3PePDd6fW9iFz30ef7Ysp/oiWqbhszeGWW2T6Gzw==", + "dev": true, "bin": { "mkdirp": "bin/cmd.js" }, @@ -12982,11 +12991,6 @@ "resolved": "https://registry.npmjs.org/node-abort-controller/-/node-abort-controller-3.1.1.tgz", "integrity": "sha512-AGK2yQKIjRuqnc6VkX2Xj5d+QW8xZ87pa1UK6yA6ouUyuxfHuMP6umE5QK7UmTeOAymo+Zx1Fxiuw9rVx8taHQ==" }, - "node_modules/node-addon-api": { - "version": "5.1.0", - "resolved": "https://registry.npmjs.org/node-addon-api/-/node-addon-api-5.1.0.tgz", - "integrity": "sha512-eh0GgfEkpnoWDq+VY8OyvYhFEzBk6jIYbRKdIlyTiAXIVJ8PyBaKb0rp7oDtoddbdoHWhq8wwr+XZ81F1rpNdA==" - }, "node_modules/node-dig-dns": { "version": "0.3.3", "resolved": "https://registry.npmjs.org/node-dig-dns/-/node-dig-dns-0.3.3.tgz", @@ -13143,6 +13147,7 @@ "version": "5.0.0", "resolved": "https://registry.npmjs.org/nopt/-/nopt-5.0.0.tgz", "integrity": "sha512-Tbj67rffqceeLpcRXrT7vKAN8CwfPeIBgM7E6iBkmKLV7bEMwpGgYLGv0jACUsECaa/vuxP0IjEont6umdMgtQ==", + "dev": true, "dependencies": { "abbrev": "1" }, @@ -13166,6 +13171,7 @@ "version": "5.0.1", "resolved": "https://registry.npmjs.org/npmlog/-/npmlog-5.0.1.tgz", "integrity": "sha512-AqZtDUWOMKs1G/8lwylVjrdYgqA4d9nu8hc+0gzRxlDb1I10+FHBGMXs6aiQHFdCUUlqH99MUMuLfzWDNDtfxw==", + "dev": true, "dependencies": { "are-we-there-yet": "^2.0.0", "console-control-strings": "^1.1.0", @@ -14333,6 +14339,7 @@ "version": "3.0.2", "resolved": "https://registry.npmjs.org/rimraf/-/rimraf-3.0.2.tgz", "integrity": "sha512-JZkJMZkAGFFPP2YqXZXPbMlMBgsxzE8ILs4lMIX/2o0L9UBw9O/Y3o6wFw/i9YLapcUJWwqbi3kdxIPdC62TIA==", + "dev": true, "dependencies": { "glob": "^7.1.3" }, @@ -14729,7 +14736,8 @@ "node_modules/set-blocking": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/set-blocking/-/set-blocking-2.0.0.tgz", - "integrity": "sha512-KiKBS8AnWGEyLzofFfmvKwpdPzqiy16LvQfK3yv/fVH7Bj13/wl3JSR1J+rfgRE9q7xUJK4qvgS8raSOeLUehw==" + "integrity": "sha512-KiKBS8AnWGEyLzofFfmvKwpdPzqiy16LvQfK3yv/fVH7Bj13/wl3JSR1J+rfgRE9q7xUJK4qvgS8raSOeLUehw==", + "dev": true }, "node_modules/setprototypeof": { "version": "1.2.0", @@ -14782,7 +14790,8 @@ "node_modules/signal-exit": { "version": "3.0.7", "resolved": "https://registry.npmjs.org/signal-exit/-/signal-exit-3.0.7.tgz", - "integrity": "sha512-wnD2ZE+l+SPC/uoS0vXeE9L1+0wuaMqKlfz9AMUo38JsyLSBWSFcHR1Rri62LZc12vLr1gb3jl7iwQhgwpAbGQ==" + "integrity": "sha512-wnD2ZE+l+SPC/uoS0vXeE9L1+0wuaMqKlfz9AMUo38JsyLSBWSFcHR1Rri62LZc12vLr1gb3jl7iwQhgwpAbGQ==", + "dev": true }, "node_modules/simple-git": { "version": "3.19.1", @@ -15109,6 +15118,7 @@ "version": "4.2.3", "resolved": "https://registry.npmjs.org/string-width/-/string-width-4.2.3.tgz", "integrity": "sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g==", + "dev": true, "dependencies": { "emoji-regex": "^8.0.0", "is-fullwidth-code-point": "^3.0.0", @@ -15181,6 +15191,7 @@ "version": "6.0.1", "resolved": "https://registry.npmjs.org/strip-ansi/-/strip-ansi-6.0.1.tgz", "integrity": "sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A==", + "dev": true, "dependencies": { "ansi-regex": "^5.0.1" }, @@ -15504,6 +15515,7 @@ "version": "6.1.15", "resolved": "https://registry.npmjs.org/tar/-/tar-6.1.15.tgz", "integrity": "sha512-/zKt9UyngnxIT/EAGYuxaMYgOIJiP81ab9ZfkILq4oNLPFX50qyYmu7jRj9qeXoxmJHjGlbH0+cm2uy1WCs10A==", + "dev": true, "dependencies": { "chownr": "^2.0.0", "fs-minipass": "^2.0.0", @@ -15520,6 +15532,7 @@ "version": "5.0.0", "resolved": "https://registry.npmjs.org/minipass/-/minipass-5.0.0.tgz", "integrity": "sha512-3FnjYuehv9k6ovOEbyOswadCDPX1piCfhV8ncmYtHOjuPwylVWsghTLo7rabjC3Rx5xD4HDx8Wm1xnMF7S5qFQ==", + "dev": true, "engines": { "node": ">=8" } @@ -16565,6 +16578,7 @@ "version": "1.1.5", "resolved": "https://registry.npmjs.org/wide-align/-/wide-align-1.1.5.tgz", "integrity": "sha512-eDMORYaPNZ4sQIuuYPDHdQvf4gyCF9rEEV/yPxGfwPkRodwEgiMUUXTx/dex+Me0wxx53S+NgUHaP7y3MGlDmg==", + "dev": true, "dependencies": { "string-width": "^1.0.2 || 2 || 3 || 4" } diff --git a/package.json b/package.json index 9e753cb65..ca42244cc 100644 --- a/package.json +++ b/package.json @@ -40,7 +40,7 @@ "axios": "~0.27.2", "axios-cache-interceptor": "^0.10.7", "base-64": "^0.1.0", - "bcrypt": "^5.1.0", + "bcryptjs": "^2.4.3", "bluebird": "^3.7.2", "body-parser": "^1.19.2", "cache-parser": "^1.2.4", @@ -101,7 +101,7 @@ "@octokit/types": "^6.35.0", "@tsconfig/recommended": "^1.0.1", "@types/aws-lambda": "^8.10.106", - "@types/bcrypt": "^5.0.0", + "@types/bcryptjs": "^2.4.4", "@types/bluebird": "^3.5.38", "@types/convict": "^6.1.1", "@types/cookie-parser": "^1.4.3", diff --git a/src/services/identity/OtpService.ts b/src/services/identity/OtpService.ts index 3583f997e..9afaa2274 100644 --- a/src/services/identity/OtpService.ts +++ b/src/services/identity/OtpService.ts @@ -1,6 +1,6 @@ import crypto from "crypto" -import bcrypt from "bcrypt" +import bcrypt from "bcryptjs" const SALT_TIMES = 10 const TOTP_LENGTH = 6 From a792ef1b9fbd95c03dc2024a2c62dc69c378aa98 Mon Sep 17 00:00:00 2001 From: Hsu Zhong Jun <27919917+dcshzj@users.noreply.github.com> Date: Wed, 4 Oct 2023 10:25:08 +0800 Subject: [PATCH 4/8] feat: add new admin endpoint to reset repository (#950) * feat: introduce new reset repo admin endpoint * feat: add unit tests for resetRepo functionality * fix: add e2e-notggs-test-repo as allowed e2e repo * fix: remove unneeded map to undefined * chore(e2e): rename variable for non-ggs enabled repo --- .../__tests__/RepoManagement.spec.ts | 116 ++++++++++++++++++ src/routes/v2/authenticatedSites/index.js | 9 ++ .../v2/authenticatedSites/repoManagement.ts | 71 +++++++++++ src/server.js | 5 + src/services/admin/RepoManagementService.ts | 53 ++++++++ .../__tests__/RepoManagementService.spec.ts | 60 +++++++++ .../AuthenticationMiddlewareService.ts | 4 +- src/utils/mutex-utils.js | 6 +- 8 files changed, 322 insertions(+), 2 deletions(-) create mode 100644 src/routes/v2/authenticatedSites/__tests__/RepoManagement.spec.ts create mode 100644 src/routes/v2/authenticatedSites/repoManagement.ts create mode 100644 src/services/admin/RepoManagementService.ts create mode 100644 src/services/admin/__tests__/RepoManagementService.spec.ts diff --git a/src/routes/v2/authenticatedSites/__tests__/RepoManagement.spec.ts b/src/routes/v2/authenticatedSites/__tests__/RepoManagement.spec.ts new file mode 100644 index 000000000..2bf790714 --- /dev/null +++ b/src/routes/v2/authenticatedSites/__tests__/RepoManagement.spec.ts @@ -0,0 +1,116 @@ +import express from "express" +import { errAsync, okAsync } from "neverthrow" +import request from "supertest" + +import { BadRequestError } from "@errors/BadRequestError" +import { ForbiddenError } from "@errors/ForbiddenError" +import GitFileSystemError from "@errors/GitFileSystemError" +import GitHubApiError from "@errors/GitHubApiError" + +import { AuthorizationMiddleware } from "@middleware/authorization" +import { attachReadRouteHandlerWrapper } from "@middleware/routeHandler" + +import { + generateRouter, + generateRouterForDefaultUserWithSite, +} from "@fixtures/app" +import RepoManagementService from "@services/admin/RepoManagementService" + +import { RepoManagementRouter } from "../repoManagement" + +describe("RepoManagementRouter", () => { + const mockRepoManagementService = { + resetRepo: jest.fn(), + } + + const mockAuthorizationMiddleware = { + verifySiteAdmin: jest.fn(), + } + + const router = new RepoManagementRouter({ + repoManagementService: (mockRepoManagementService as unknown) as RepoManagementService, + authorizationMiddleware: (mockAuthorizationMiddleware as unknown) as AuthorizationMiddleware, + }) + + const subrouter = express() + // We can use read route handler here because we don't need to lock the repo + subrouter.post("/resetRepo", attachReadRouteHandlerWrapper(router.resetRepo)) + + const app = generateRouter(subrouter) + + beforeEach(() => { + jest.clearAllMocks() + }) + + describe("resetRepo", () => { + it("should return 200 if repo was successfully reset", async () => { + mockRepoManagementService.resetRepo.mockReturnValueOnce( + okAsync(undefined) + ) + + const response = await request(app).post("/resetRepo").send({ + branchName: "branch-name", + commitSha: "commit-sha", + }) + + expect(response.status).toBe(200) + expect(mockRepoManagementService.resetRepo).toHaveBeenCalledTimes(1) + }) + + it("should return 400 if a BadRequestError is received", async () => { + mockRepoManagementService.resetRepo.mockReturnValueOnce( + errAsync(new BadRequestError("error")) + ) + + const response = await request(app).post("/resetRepo").send({ + branchName: "branch-name", + commitSha: "commit-sha", + }) + + expect(response.status).toBe(400) + expect(mockRepoManagementService.resetRepo).toHaveBeenCalledTimes(1) + }) + + it("should return 403 if a ForbiddenError is received", async () => { + mockRepoManagementService.resetRepo.mockReturnValueOnce( + errAsync(new ForbiddenError()) + ) + + const response = await request(app).post("/resetRepo").send({ + branchName: "branch-name", + commitSha: "commit-sha", + }) + + expect(response.status).toBe(403) + expect(mockRepoManagementService.resetRepo).toHaveBeenCalledTimes(1) + }) + + it("should return 500 if a GitFileSystemError is received", async () => { + mockRepoManagementService.resetRepo.mockReturnValueOnce( + errAsync(new GitFileSystemError("error")) + ) + + const response = await request(app).post("/resetRepo").send({ + branchName: "branch-name", + commitSha: "commit-sha", + }) + + expect(response.status).toBe(500) + expect(mockRepoManagementService.resetRepo).toHaveBeenCalledTimes(1) + }) + + it("should return 502 if a GitHubApiError error is received", async () => { + mockRepoManagementService.resetRepo.mockReturnValueOnce( + errAsync(new GitHubApiError("error")) + ) + + const response = await request(app).post("/resetRepo").send({ + branchName: "branch-name", + commitSha: "commit-sha", + }) + + expect(response.status).toBe(502) + expect(mockRepoManagementService.resetRepo).toHaveBeenCalledTimes(1) + }) + }) +}) diff --git a/src/routes/v2/authenticatedSites/index.js b/src/routes/v2/authenticatedSites/index.js index 5e3ec19e4..28a7d7bd6 100644 --- a/src/routes/v2/authenticatedSites/index.js +++ b/src/routes/v2/authenticatedSites/index.js @@ -15,6 +15,9 @@ const { } = require("@routes/v2/authenticatedSites/mediaCategories") const { MediaFilesRouter } = require("@routes/v2/authenticatedSites/mediaFiles") const { NavigationRouter } = require("@routes/v2/authenticatedSites/navigation") +const { + RepoManagementRouter, +} = require("@routes/v2/authenticatedSites/repoManagement") const { ResourceCategoriesRouter, } = require("@routes/v2/authenticatedSites/resourceCategories") @@ -92,6 +95,7 @@ const getAuthenticatedSitesSubrouter = ({ notificationOnEditHandler, sitesService, deploymentsService, + repoManagementService, }) => { const collectionYmlService = new CollectionYmlService({ gitHubService }) const homepagePageService = new HomepagePageService({ gitHubService }) @@ -196,6 +200,10 @@ const getAuthenticatedSitesSubrouter = ({ const navigationV2Router = new NavigationRouter({ navigationYmlService: navYmlService, }) + const repoManagementV2Router = new RepoManagementRouter({ + repoManagementService, + authorizationMiddleware, + }) const authenticatedSitesSubrouter = express.Router({ mergeParams: true }) @@ -236,6 +244,7 @@ const getAuthenticatedSitesSubrouter = ({ authenticatedSitesSubrouter.use("/contactUs", contactUsV2Router.getRouter()) authenticatedSitesSubrouter.use("/homepage", homepageV2Router.getRouter()) authenticatedSitesSubrouter.use("/settings", settingsV2Router.getRouter()) + authenticatedSitesSubrouter.use("/admin", repoManagementV2Router.getRouter()) authenticatedSitesSubrouter.use(notificationOnEditHandler.createNotification) return authenticatedSitesSubrouter diff --git a/src/routes/v2/authenticatedSites/repoManagement.ts b/src/routes/v2/authenticatedSites/repoManagement.ts new file mode 100644 index 000000000..7d2a4d539 --- /dev/null +++ b/src/routes/v2/authenticatedSites/repoManagement.ts @@ -0,0 +1,71 @@ +import autoBind from "auto-bind" +import express from "express" + +import { BadRequestError } from "@errors/BadRequestError" +import { ForbiddenError } from "@errors/ForbiddenError" + +import { AuthorizationMiddleware } from "@middleware/authorization" +import { attachWriteRouteHandlerWrapper } from "@middleware/routeHandler" + +import UserWithSiteSessionData from "@root/classes/UserWithSiteSessionData" +import GitFileSystemError from "@root/errors/GitFileSystemError" +import { attachSiteHandler } from "@root/middleware" +import type { RequestHandler } from "@root/types" +import RepoManagementService from "@services/admin/RepoManagementService" + +interface RepoManagementRouterProps { + repoManagementService: RepoManagementService + authorizationMiddleware: AuthorizationMiddleware +} + +export class RepoManagementRouter { + private readonly repoManagementService + + private readonly authorizationMiddleware + + constructor({ + repoManagementService, + authorizationMiddleware, + }: RepoManagementRouterProps) { + this.repoManagementService = repoManagementService + this.authorizationMiddleware = authorizationMiddleware + autoBind(this) + } + + resetRepo: RequestHandler< + never, + void | { message: string }, + { branchName: string; commitSha: string }, + unknown, + { userWithSiteSessionData: UserWithSiteSessionData } + > = async (req, res) => { + const { userWithSiteSessionData } = res.locals + const { branchName, commitSha } = req.body + + return this.repoManagementService + .resetRepo(userWithSiteSessionData, branchName, commitSha) + .map(() => res.status(200).send()) + .mapErr((error) => { + if (error instanceof BadRequestError) { + return res.status(400).json({ message: error.message }) + } + if (error instanceof ForbiddenError) { + return res.status(403).json({ message: error.message }) + } + if (error instanceof GitFileSystemError) { + return res.status(500).json({ message: error.message }) + } + return res.status(502).json({ message: error.message }) + }) + } + + getRouter() { + const router = express.Router({ mergeParams: true }) + router.use(attachSiteHandler) + router.use(this.authorizationMiddleware.verifySiteMember) + + router.post("/resetRepo", attachWriteRouteHandlerWrapper(this.resetRepo)) + + return router + } +} diff --git a/src/server.js b/src/server.js index 707d1b0b2..f6228ce49 100644 --- a/src/server.js +++ b/src/server.js @@ -76,6 +76,7 @@ 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 RepoManagementService from "./services/admin/RepoManagementService" import GitFileSystemService from "./services/db/GitFileSystemService" import RepoService from "./services/db/RepoService" import { PageService } from "./services/fileServices/MdPageServices/PageService" @@ -165,6 +166,9 @@ const gitHubService = new RepoService( isomerRepoAxiosInstance, gitFileSystemService ) +const repoManagementService = new RepoManagementService({ + repoService: gitHubService, +}) const configYmlService = new ConfigYmlService({ gitHubService }) const footerYmlService = new FooterYmlService({ gitHubService }) const collectionYmlService = new CollectionYmlService({ gitHubService }) @@ -338,6 +342,7 @@ const authenticatedSitesSubrouterV2 = getAuthenticatedSitesSubrouter({ notificationOnEditHandler, sitesService, deploymentsService, + repoManagementService, }) const sgidAuthRouter = new SgidAuthRouter({ usersService, diff --git a/src/services/admin/RepoManagementService.ts b/src/services/admin/RepoManagementService.ts new file mode 100644 index 000000000..6f830e16f --- /dev/null +++ b/src/services/admin/RepoManagementService.ts @@ -0,0 +1,53 @@ +import { ResultAsync, errAsync } from "neverthrow" + +import { BadRequestError } from "@errors/BadRequestError" +import { ForbiddenError } from "@errors/ForbiddenError" +import GitFileSystemError from "@errors/GitFileSystemError" +import GitHubApiError from "@errors/GitHubApiError" + +import UserWithSiteSessionData from "@root/classes/UserWithSiteSessionData" +import { ISOMER_E2E_TEST_REPOS } from "@root/constants" +import RepoService from "@services/db/RepoService" + +interface RepoManagementServiceProps { + repoService: RepoService +} + +class RepoManagementService { + private readonly repoService: RepoManagementServiceProps["repoService"] + + constructor({ repoService }: RepoManagementServiceProps) { + this.repoService = repoService + } + + resetRepo( + sessionData: UserWithSiteSessionData, + branchName: string, + commitSha: string + ): ResultAsync< + void, + ForbiddenError | BadRequestError | GitFileSystemError | GitHubApiError + > { + const { siteName } = sessionData + + if (!ISOMER_E2E_TEST_REPOS.includes(siteName)) { + return errAsync(new ForbiddenError(`${siteName} is not an e2e test repo`)) + } + + return ResultAsync.fromPromise( + this.repoService.updateRepoState(sessionData, { commitSha, branchName }), + (error) => { + if (error instanceof BadRequestError) { + return new BadRequestError(error.message) + } + if (error instanceof GitFileSystemError) { + return new GitFileSystemError(error.message) + } + + return new GitHubApiError(`Failed to reset repo to commit ${commitSha}`) + } + ) + } +} + +export default RepoManagementService diff --git a/src/services/admin/__tests__/RepoManagementService.spec.ts b/src/services/admin/__tests__/RepoManagementService.spec.ts new file mode 100644 index 000000000..c05abf5f5 --- /dev/null +++ b/src/services/admin/__tests__/RepoManagementService.spec.ts @@ -0,0 +1,60 @@ +import { ForbiddenError } from "@errors/ForbiddenError" + +import UserWithSiteSessionData from "@root/classes/UserWithSiteSessionData" +import { ISOMER_E2E_TEST_REPOS } from "@root/constants" +import _RepoManagementService from "@services/admin/RepoManagementService" +import RepoService from "@services/db/RepoService" + +const MockRepoService = { + updateRepoState: jest.fn(), +} + +const RepoManagementService = new _RepoManagementService({ + repoService: (MockRepoService as unknown) as RepoService, +}) + +describe("RepoManagementService", () => { + // Prevent inter-test pollution of mocks + afterEach(() => jest.clearAllMocks()) + + describe("resetRepo", () => { + it("should reset an e2e test repo successfully", async () => { + const mockSessionData = new UserWithSiteSessionData({ + githubId: "githubId", + accessToken: "accessToken", + isomerUserId: "isomerUserId", + email: "email", + siteName: ISOMER_E2E_TEST_REPOS[0], + }) + MockRepoService.updateRepoState.mockResolvedValueOnce(undefined) + + await RepoManagementService.resetRepo( + mockSessionData, + "branchName", + "commitSha" + ) + + expect(MockRepoService.updateRepoState).toHaveBeenCalledTimes(1) + }) + + it("should not reset a non-e2e test repo", async () => { + const mockSessionData = new UserWithSiteSessionData({ + githubId: "githubId", + accessToken: "accessToken", + isomerUserId: "isomerUserId", + email: "email", + siteName: "some-other-site", + }) + + const result = await RepoManagementService.resetRepo( + mockSessionData, + "branchName", + "commitSha" + ) + + expect(result.isErr()).toBe(true) + expect(result._unsafeUnwrapErr()).toBeInstanceOf(ForbiddenError) + expect(MockRepoService.updateRepoState).toHaveBeenCalledTimes(0) + }) + }) +}) diff --git a/src/services/middlewareServices/AuthenticationMiddlewareService.ts b/src/services/middlewareServices/AuthenticationMiddlewareService.ts index 708d8469d..0e0b930a6 100644 --- a/src/services/middlewareServices/AuthenticationMiddlewareService.ts +++ b/src/services/middlewareServices/AuthenticationMiddlewareService.ts @@ -28,6 +28,7 @@ export const E2E_EMAIL_TEST_SITE = { name: "e2e email test site", repo: "e2e-email-test-repo", } +export const E2E_NOT_GGS_TEST_REPO = "e2e-notggs-test-repo" const E2E_TEST_SECRET = config.get("cypress.e2eTestSecret") export const E2E_TEST_GH_TOKEN = config.get("cypress.e2eTestGithubToken") @@ -190,7 +191,8 @@ export default class AuthenticationMiddlewareService { (userType === E2E_USERS.Email.Admin || userType === E2E_USERS.Email.Collaborator) const isGithubE2eAccess = - repo === E2E_TEST_REPO && userType === "Github user" + (repo === E2E_TEST_REPO || repo === E2E_NOT_GGS_TEST_REPO) && + userType === "Github user" if (!isGithubE2eAccess && !isEmailE2eAccess) throw new AuthError( diff --git a/src/utils/mutex-utils.js b/src/utils/mutex-utils.js index bd7e75ad6..fa623c383 100644 --- a/src/utils/mutex-utils.js +++ b/src/utils/mutex-utils.js @@ -12,7 +12,11 @@ const NODE_ENV = config.get("env") const MUTEX_TABLE_NAME = config.get("mutexTableName") const IS_DEV = NODE_ENV === "dev" || NODE_ENV === "test" || NODE_ENV === "vapt" -const E2E_TEST_REPOS = ["e2e-email-test-repo", "e2e-test-repo"] +const E2E_TEST_REPOS = [ + "e2e-email-test-repo", + "e2e-test-repo", + "e2e-notggs-test-repo", +] const mockMutexObj = {} // Dynamodb constants From 12d6123011507bd5564877aea1fd95d531953a0a Mon Sep 17 00:00:00 2001 From: seaerchin <44049504+seaerchin@users.noreply.github.com> Date: Thu, 5 Oct 2023 11:36:11 +0800 Subject: [PATCH 5/8] docs(pr-template): add checkbox for ssm and 1pw for env var (#953) --- .github/pull_request_template.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 8dd123d43..2f206a207 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -50,6 +50,7 @@ Closes [insert issue #] **New environment variables**: - `env var` : env var details + - [ ] added env var to 1PW + SSM script (`fetch_ssm_parameters.sh`) **New scripts**: From 5b9b8884149cf255c72c7f49da8c9077ee688219 Mon Sep 17 00:00:00 2001 From: seaerchin <44049504+seaerchin@users.noreply.github.com> Date: Thu, 5 Oct 2023 11:36:34 +0800 Subject: [PATCH 6/8] feat(formsg): clone repo on webhook trigger from forms (#947) * feat(formsg): clone repo on webhook trigger from forms * ci(.env.test): add env var * fix(env-var): add to ssm script * refactor(formsg): shift cloning to separate router --------- Co-authored-by: seaerchin --- .env.test | 3 +- .../predeploy/06_fetch_ssm_parameters.sh | 143 +++++++++--------- src/config/config.ts | 7 + src/routes/formsgSiteClone.ts | 125 +++++++++++++++ src/routes/formsgSiteCreation.ts | 86 ++++++++++- src/server.js | 11 +- 6 files changed, 301 insertions(+), 74 deletions(-) create mode 100644 src/routes/formsgSiteClone.ts diff --git a/.env.test b/.env.test index 67018d209..05c0523ec 100644 --- a/.env.test +++ b/.env.test @@ -22,6 +22,7 @@ export SYSTEM_GITHUB_TOKEN="github_token" # FormSG keys export SITE_CREATE_FORM_KEY="site_form_key" export SITE_LAUNCH_FORM_KEY="site_launch_form_key" +export SITE_CLONE_FORM_KEY="site_clone_form_key" # Required to connect to DynamoDB export AWS_ACCESS_KEY_ID="abc123" @@ -81,4 +82,4 @@ export SGID_PRIVATE_KEY="private" export SGID_REDIRECT_URI="http://localhost:8081/v2/auth/sgid/auth-redirect" # GrowthBook -export GROWTHBOOK_CLIENT_KEY="some random key" \ No newline at end of file +export GROWTHBOOK_CLIENT_KEY="some random key" diff --git a/.platform/hooks/predeploy/06_fetch_ssm_parameters.sh b/.platform/hooks/predeploy/06_fetch_ssm_parameters.sh index 31cebd0b0..ec059234f 100644 --- a/.platform/hooks/predeploy/06_fetch_ssm_parameters.sh +++ b/.platform/hooks/predeploy/06_fetch_ssm_parameters.sh @@ -20,100 +20,101 @@ fi ENV_TYPE=$(/opt/elasticbeanstalk/bin/get-config environment -k SSM_PREFIX) TIMESTAMP=$(date '+%Y-%m-%d %H:%M:%S') -echo "Timestamp: $TIMESTAMP - ENV TYPE: $ENV_TYPE" > /tmp/ssm-type.txt +echo "Timestamp: $TIMESTAMP - ENV TYPE: $ENV_TYPE" >/tmp/ssm-type.txt # List of all env vars to fetch ENV_VARS=( - "AUTH_TOKEN_EXPIRY_DURATION_IN_MILLISECONDS" - "AWS_BACKEND_EB_ENV_NAME" - "AWS_REGION" - "CLIENT_ID" - "CLIENT_SECRET" - "CLOUDMERSIVE_API_KEY" - "COOKIE_DOMAIN" - "DB_ACQUIRE" - "DB_MAX_POOL" - "DB_MIN_POOL" - "DB_TIMEOUT" - "DB_URI" - "DD_AGENT_MAJOR_VERSION" - "DD_ENV" - "DD_LOGS_INJECTION" - "DD_SERVICE" - "DD_TAGS" - "DD_TRACE_STARTUP_LOGS" - "E2E_TEST_GH_TOKEN" - "E2E_TEST_REPO" - "E2E_TEST_SECRET" - "EFS_VOL_PATH" - "ENCRYPTION_SECRET" - "FF_DEPRECATE_SITE_QUEUES" - "FRONTEND_URL" - "GGS_EXPERIMENTAL_TRACKING_SITES" - "GITHUB_BUILD_ORG_NAME" - "GITHUB_BUILD_REPO_NAME" - "GITHUB_ORG_NAME" - "GROWTHBOOK_CLIENT_KEY" - "INCOMING_QUEUE_URL" - "ISOMERPAGES_REPO_PAGE_COUNT" - "JWT_SECRET" - "MAX_NUM_OTP_ATTEMPTS" - "MOCK_AMPLIFY_DOMAIN_ASSOCIATION_CALLS" - "MUTEX_TABLE_NAME" - "NETLIFY_ACCESS_TOKEN" - "NODE_ENV" - "OTP_EXPIRY" - "OTP_SECRET" - "OUTGOING_QUEUE_URL" - "POSTMAN_API_KEY" - "POSTMAN_SMS_CRED_NAME" - "REDIRECT_URI" - "SESSION_SECRET" - "SGID_CLIENT_ID" - "SGID_CLIENT_SECRET" - "SGID_PRIVATE_KEY" - "SGID_REDIRECT_URI" - "SITE_CREATE_FORM_KEY" - "SITE_LAUNCH_DYNAMO_DB_TABLE_NAME" - "SITE_LAUNCH_FORM_KEY" - "SITE_PASSWORD_SECRET_KEY" - "SSM_PREFIX" - "STEP_FUNCTIONS_ARN" - "SYSTEM_GITHUB_TOKEN" + "AUTH_TOKEN_EXPIRY_DURATION_IN_MILLISECONDS" + "AWS_BACKEND_EB_ENV_NAME" + "AWS_REGION" + "CLIENT_ID" + "CLIENT_SECRET" + "CLOUDMERSIVE_API_KEY" + "COOKIE_DOMAIN" + "DB_ACQUIRE" + "DB_MAX_POOL" + "DB_MIN_POOL" + "DB_TIMEOUT" + "DB_URI" + "DD_AGENT_MAJOR_VERSION" + "DD_ENV" + "DD_LOGS_INJECTION" + "DD_SERVICE" + "DD_TAGS" + "DD_TRACE_STARTUP_LOGS" + "E2E_TEST_GH_TOKEN" + "E2E_TEST_REPO" + "E2E_TEST_SECRET" + "EFS_VOL_PATH" + "ENCRYPTION_SECRET" + "FF_DEPRECATE_SITE_QUEUES" + "FRONTEND_URL" + "GGS_EXPERIMENTAL_TRACKING_SITES" + "GITHUB_BUILD_ORG_NAME" + "GITHUB_BUILD_REPO_NAME" + "GITHUB_ORG_NAME" + "GROWTHBOOK_CLIENT_KEY" + "INCOMING_QUEUE_URL" + "ISOMERPAGES_REPO_PAGE_COUNT" + "JWT_SECRET" + "MAX_NUM_OTP_ATTEMPTS" + "MOCK_AMPLIFY_DOMAIN_ASSOCIATION_CALLS" + "MUTEX_TABLE_NAME" + "NETLIFY_ACCESS_TOKEN" + "NODE_ENV" + "OTP_EXPIRY" + "OTP_SECRET" + "OUTGOING_QUEUE_URL" + "POSTMAN_API_KEY" + "POSTMAN_SMS_CRED_NAME" + "REDIRECT_URI" + "SESSION_SECRET" + "SGID_CLIENT_ID" + "SGID_CLIENT_SECRET" + "SGID_PRIVATE_KEY" + "SGID_REDIRECT_URI" + "SITE_CREATE_FORM_KEY" + "SITE_LAUNCH_DYNAMO_DB_TABLE_NAME" + "SITE_LAUNCH_FORM_KEY" + "SITE_CLONE_FORM_KEY" + "SITE_PASSWORD_SECRET_KEY" + "SSM_PREFIX" + "STEP_FUNCTIONS_ARN" + "SYSTEM_GITHUB_TOKEN" ) echo "Set AWS region" aws configure set default.region ap-southeast-1 -set +e # Do not exit if a command fails +set +e # Do not exit if a command fails for ENV_VAR in "${ENV_VARS[@]}"; do echo "Fetching ${ENV_VAR} from SSM" - + VALUE=$(aws ssm get-parameter --name "${ENV_TYPE}_${ENV_VAR}" --with-decryption --query "Parameter.Value" --output text 2>/dev/null) - STATUS=$? # Capture exit status of the aws ssm command - + STATUS=$? # Capture exit status of the aws ssm command + if [ $STATUS -ne 0 ]; then echo "Failed to fetch ${ENV_VAR}. Skipping." continue fi - - echo "${ENV_VAR}=${VALUE}" >> /tmp/isomer/.isomer.env + + echo "${ENV_VAR}=${VALUE}" >>/tmp/isomer/.isomer.env echo "Saved ${ENV_VAR}" done -set -e # Exit on command failure from this point onwards +set -e # Exit on command failure from this point onwards # Use flock to ensure that the EFS file is locked during the copy operation ( - flock -n 200 || exit 1 + flock -n 200 || exit 1 - # Copy the local file to EFS - echo "Copying local env file to EFS" - cp /tmp/isomer/.isomer.env /efs/isomer/.isomer.env + # Copy the local file to EFS + echo "Copying local env file to EFS" + cp /tmp/isomer/.isomer.env /efs/isomer/.isomer.env - # Ensure the file on EFS is owned by webapp so it has access - chown webapp:webapp /efs/isomer/.isomer.env + # Ensure the file on EFS is owned by webapp so it has access + chown webapp:webapp /efs/isomer/.isomer.env ) 200>/efs/isomer/.isomer.lock @@ -126,4 +127,4 @@ else echo "Couldn't acquire the lock. Another instance might be writing to the file." fi -echo "Operation completed." \ No newline at end of file +echo "Operation completed." diff --git a/src/config/config.ts b/src/config/config.ts index ec89cf26b..b36910f4a 100644 --- a/src/config/config.ts +++ b/src/config/config.ts @@ -283,6 +283,13 @@ const config = convict({ format: "required-string", default: "", }, + siteCloneFormKey: { + doc: "FormSG API key for site clone form", + env: "SITE_CLONE_FORM_KEY", + sensitive: true, + format: "required-string", + default: "", + }, }, postman: { apiKey: { diff --git a/src/routes/formsgSiteClone.ts b/src/routes/formsgSiteClone.ts new file mode 100644 index 000000000..67ec0225c --- /dev/null +++ b/src/routes/formsgSiteClone.ts @@ -0,0 +1,125 @@ +import { DecryptedContent } from "@opengovsg/formsg-sdk/dist/types" +import autoBind from "auto-bind" +import express, { RequestHandler } from "express" + +import { config } from "@config/config" + +import logger from "@logger/logger" + +import { getField } from "@utils/formsg-utils" + +import { ISOMER_ADMIN_EMAIL } from "@root/constants" +import { attachFormSGHandler } from "@root/middleware" +import GitFileSystemService from "@services/db/GitFileSystemService" +import UsersService from "@services/identity/UsersService" +import InfraService from "@services/infra/InfraService" +import { mailer } from "@services/utilServices/MailClient" + +const SITE_CLONE_FORM_KEY = config.get("formSg.siteCloneFormKey") + +export interface FormsgSiteCloneRouterProps { + usersService: UsersService + infraService: InfraService + gitFileSystemService: GitFileSystemService +} + +export class FormsgSiteCloneRouter { + private readonly gitFileSystemService: FormsgSiteCloneRouterProps["gitFileSystemService"] + + constructor({ gitFileSystemService }: FormsgSiteCloneRouterProps) { + this.gitFileSystemService = gitFileSystemService + // We need to bind all methods because we don't invoke them from the class directly + autoBind(this) + } + + cloneSiteToEfs: RequestHandler< + never, + Record, + { data: { submissionId: string } }, + never, + { submission: DecryptedContent } + > = async (req, res) => { + // 1. Extract arguments + const { submissionId } = req.body.data + const { responses } = res.locals.submission + const requesterEmail = getField(responses, "Email") + // NOTE: The field is required by our form so this cannot be empty or undefined + const githubRepoName = getField(responses, "Github Repo Name") as string + + if ( + !requesterEmail || + !githubRepoName || + !requesterEmail.endsWith("@open.gov.sg") + ) { + return this.sendCloneError( + ISOMER_ADMIN_EMAIL, + githubRepoName, + submissionId, + "Invalid email or missing github repo name detected for submission" + ) + } + + logger.info( + `${requesterEmail} requested for ${githubRepoName} to be cloned onto EFS` + ) + + this.gitFileSystemService + .clone(githubRepoName) + .map((path) => { + logger.info(`Cloned ${githubRepoName} to ${path}`) + this.sendCloneSuccess( + requesterEmail, + githubRepoName, + submissionId, + path + ) + }) + .mapErr((err) => { + logger.error( + `Cloning repo: ${githubRepoName} to EFS failed with error: ${JSON.stringify( + err + )}` + ) + this.sendCloneError( + requesterEmail, + githubRepoName, + submissionId, + err.message + ) + }) + } + + sendCloneSuccess = async ( + requesterEmail: string, + githubRepoName: string, + submissionId: string, + path: string + ) => { + const subject = `[Isomer] Clone site ${githubRepoName} SUCCESS` + const html = `

Isomer site ${githubRepoName} was cloned successfully to EFS path: ${path}. (Form submission id [${submissionId}])

` + await mailer.sendMail(requesterEmail, subject, html) + } + + async sendCloneError( + requesterEmail: string, + githubRepoName: string, + submissionId: string, + message: string + ) { + const subject = `[Isomer] Clone site ${githubRepoName} FAILURE` + const html = `

Isomer site ${githubRepoName} was not cloned successfully. Cloning failed with error: ${message} (Form submission id [${submissionId}])

` + await mailer.sendMail(requesterEmail, subject, html) + } + + getRouter() { + const router = express.Router({ mergeParams: true }) + + router.post( + "/clone-site", + attachFormSGHandler(SITE_CLONE_FORM_KEY), + this.cloneSiteToEfs + ) + + return router + } +} diff --git a/src/routes/formsgSiteCreation.ts b/src/routes/formsgSiteCreation.ts index cd859ba11..8d80c9e6c 100644 --- a/src/routes/formsgSiteCreation.ts +++ b/src/routes/formsgSiteCreation.ts @@ -11,10 +11,12 @@ import { BadRequestError } from "@errors/BadRequestError" import { getField } from "@utils/formsg-utils" import { attachFormSGHandler } from "@root/middleware" +import GitFileSystemService from "@services/db/GitFileSystemService" import UsersService from "@services/identity/UsersService" import InfraService from "@services/infra/InfraService" import { mailer } from "@services/utilServices/MailClient" +const SITE_CLONE_FORM_KEY = config.get("formSg.siteCloneFormKey") const SITE_CREATE_FORM_KEY = config.get("formSg.siteCreateFormKey") const REQUESTER_EMAIL_FIELD = "Government E-mail" const SITE_NAME_FIELD = "Site Name" @@ -25,6 +27,7 @@ const LOGIN_TYPE_FIELD = "Login Type" export interface FormsgRouterProps { usersService: UsersService infraService: InfraService + gitFileSystemService: GitFileSystemService } export class FormsgRouter { @@ -32,9 +35,16 @@ export class FormsgRouter { private readonly infraService: FormsgRouterProps["infraService"] - constructor({ usersService, infraService }: FormsgRouterProps) { + private readonly gitFileSystemService: FormsgRouterProps["gitFileSystemService"] + + constructor({ + usersService, + infraService, + gitFileSystemService, + }: FormsgRouterProps) { this.usersService = usersService this.infraService = infraService + this.gitFileSystemService = gitFileSystemService // We need to bind all methods because we don't invoke them from the class directly autoBind(this) } @@ -157,9 +167,83 @@ export class FormsgRouter { await mailer.sendMail(email, subject, html) } + cloneSiteToEfs: RequestHandler< + never, + Record, + { data: { submissionId: string } }, + never, + { submission: DecryptedContent } + > = async (req, res) => { + // 1. Extract arguments + const { submissionId } = req.body.data + const { responses } = res.locals.submission + // NOTE: This is validated by formsg to be of domain `@open.gov.sg`; + // hence, not revalidating here + const requesterEmail = getField(responses, "Email") as string + // NOTE: The field is required by our form so this cannot be empty or undefined + const githubRepoName = getField(responses, "Github Repo Name") as string + + logger.info( + `${requesterEmail} requested for ${githubRepoName} to be cloned onto EFS` + ) + + this.gitFileSystemService + .clone(githubRepoName) + .map((path) => { + logger.info(`Cloned ${githubRepoName} to ${path}`) + this.sendCloneSuccess( + requesterEmail, + githubRepoName, + submissionId, + path + ) + }) + .mapErr((err) => { + logger.error( + `Cloning repo: ${githubRepoName} to EFS failed with error: ${JSON.stringify( + err + )}` + ) + this.sendCloneError( + requesterEmail, + githubRepoName, + submissionId, + err.message + ) + }) + } + + sendCloneSuccess = async ( + requesterEmail: string, + githubRepoName: string, + submissionId: string, + path: string + ) => { + const subject = `[Isomer] Clone site ${githubRepoName} SUCCESS` + const html = `

Isomer site ${githubRepoName} was cloned successfully to EFS path: ${path}. (Form submission id [${submissionId}])

` + await mailer.sendMail(requesterEmail, subject, html) + } + + async sendCloneError( + requesterEmail: string, + githubRepoName: string, + submissionId: string, + message: string + ) { + const subject = `[Isomer] Clone site ${githubRepoName} FAILURE` + const html = `

Isomer site ${githubRepoName} was not cloned successfully. Cloning failed with error: ${message} (Form submission id [${submissionId}])

` + await mailer.sendMail(requesterEmail, subject, html) + } + getRouter() { const router = express.Router({ mergeParams: true }) + router.post( + "/clone-site", + attachFormSGHandler(SITE_CLONE_FORM_KEY), + this.cloneSiteToEfs + ) + router.post( "/create-site", attachFormSGHandler(SITE_CREATE_FORM_KEY), diff --git a/src/server.js b/src/server.js index f6228ce49..199a67bc3 100644 --- a/src/server.js +++ b/src/server.js @@ -148,6 +148,7 @@ const FRONTEND_URL = config.get("app.frontendUrl") // Import routes const { errorHandler } = require("@middleware/errorHandler") +const { FormsgSiteCloneRouter } = require("@routes/formsgSiteClone") const { FormsgRouter } = require("@routes/formsgSiteCreation") const { FormsgSiteLaunchRouter } = require("@routes/formsgSiteLaunch") const { AuthRouter } = require("@routes/v2/auth") @@ -356,11 +357,18 @@ const authV2Router = new AuthRouter({ statsMiddleware, sgidAuthRouter, }) -const formsgRouter = new FormsgRouter({ usersService, infraService }) +const formsgRouter = new FormsgRouter({ + usersService, + infraService, + gitFileSystemService, +}) const formsgSiteLaunchRouter = new FormsgSiteLaunchRouter({ usersService, infraService, }) +const formsgSiteCloneRouter = new FormsgSiteCloneRouter({ + gitFileSystemService, +}) const app = express() @@ -408,6 +416,7 @@ app.use("/v2/sites/:siteName", authenticatedSitesSubrouterV2) // FormSG Backend handler routes app.use("/formsg", formsgRouter.getRouter()) app.use("/formsg", formsgSiteLaunchRouter.getRouter()) +app.use("/formsg", formsgSiteCloneRouter.getRouter()) // catch unknown routes app.use((req, res, next) => { From f3649a267a42102b56d382099283af3221b21a78 Mon Sep 17 00:00:00 2001 From: seaerchin <44049504+seaerchin@users.noreply.github.com> Date: Thu, 5 Oct 2023 15:01:53 +0800 Subject: [PATCH 7/8] fix(clonesite): remove extra endpoint (#956) --- src/routes/formsgSiteCreation.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/routes/formsgSiteCreation.ts b/src/routes/formsgSiteCreation.ts index 8d80c9e6c..209d17c05 100644 --- a/src/routes/formsgSiteCreation.ts +++ b/src/routes/formsgSiteCreation.ts @@ -238,12 +238,6 @@ export class FormsgRouter { getRouter() { const router = express.Router({ mergeParams: true }) - router.post( - "/clone-site", - attachFormSGHandler(SITE_CLONE_FORM_KEY), - this.cloneSiteToEfs - ) - router.post( "/create-site", attachFormSGHandler(SITE_CREATE_FORM_KEY), From cecaaca4fa9375d1649e024296d0ff8bbad2e427 Mon Sep 17 00:00:00 2001 From: Alexander Lee Date: Thu, 5 Oct 2023 16:27:22 +0800 Subject: [PATCH 8/8] 0.45.0 --- CHANGELOG.md | 15 ++++++++++++++- package-lock.json | 4 ++-- package.json | 2 +- 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5cfcda28b..86f99f891 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,8 +4,21 @@ All notable changes to this project will be documented in this file. Dates are d Generated by [`auto-changelog`](https://github.com/CookPete/auto-changelog). +#### [v0.45.0](https://github.com/isomerpages/isomercms-backend/compare/v0.44.0...v0.45.0) + +- fix(clonesite): remove extra endpoint [`#956`](https://github.com/isomerpages/isomercms-backend/pull/956) +- feat(formsg): clone repo on webhook trigger from forms [`#947`](https://github.com/isomerpages/isomercms-backend/pull/947) +- docs(pr-template): add checkbox for ssm and 1pw for env var [`#953`](https://github.com/isomerpages/isomercms-backend/pull/953) +- feat: add new admin endpoint to reset repository [`#950`](https://github.com/isomerpages/isomercms-backend/pull/950) +- IS-621: fix issues for staging deploy node 18 [`#951`](https://github.com/isomerpages/isomercms-backend/pull/951) +- feat: add ability to update repo state for GGS [`#949`](https://github.com/isomerpages/isomercms-backend/pull/949) +- feat(template): add ffs as a manual check-in [`#933`](https://github.com/isomerpages/isomercms-backend/pull/933) +- Release/0.44.0 [re-merge] [`#946`](https://github.com/isomerpages/isomercms-backend/pull/946) + #### [v0.44.0](https://github.com/isomerpages/isomercms-backend/compare/v0.43.0...v0.44.0) +> 20 September 2023 + - IS-412: Move fetching SSM params to prebuild [`#943`](https://github.com/isomerpages/isomercms-backend/pull/943) - 0.43.0 [`#941`](https://github.com/isomerpages/isomercms-backend/pull/941) @@ -24,12 +37,12 @@ Generated by [`auto-changelog`](https://github.com/CookPete/auto-changelog). - fix(sl): add retries when creating indirection domain [`#935`](https://github.com/isomerpages/isomercms-backend/pull/935) - Fix/e2e mutex [`#934`](https://github.com/isomerpages/isomercms-backend/pull/934) - release(v0.41.0): merge to `develop` [`#929`](https://github.com/isomerpages/isomercms-backend/pull/929) -- Hotfix - Staging E2E, Tests [`#931`](https://github.com/isomerpages/isomercms-backend/pull/931) #### [v0.41.0](https://github.com/isomerpages/isomercms-backend/compare/v0.40.1...v0.41.0) > 30 August 2023 +- Hotfix - Staging E2E, Tests [`#931`](https://github.com/isomerpages/isomercms-backend/pull/931) - IS-310: Setup GrowthBook for BE [`#926`](https://github.com/isomerpages/isomercms-backend/pull/926) - fix: upgrade aws-sdk from 2.1416.0 to 2.1428.0 [`#921`](https://github.com/isomerpages/isomercms-backend/pull/921) - feat(sl): enhance site launch process to utilise DNS indirection layer [`#920`](https://github.com/isomerpages/isomercms-backend/pull/920) diff --git a/package-lock.json b/package-lock.json index f4ffbafac..c39101a98 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "isomercms", - "version": "0.44.0", + "version": "0.45.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "isomercms", - "version": "0.44.0", + "version": "0.45.0", "dependencies": { "@aws-sdk/client-amplify": "^3.370.0", "@aws-sdk/client-cloudwatch-logs": "^3.370.0", diff --git a/package.json b/package.json index ca42244cc..1dba945f3 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "isomercms", - "version": "0.44.0", + "version": "0.45.0", "private": true, "scripts": { "build": "tsc -p tsconfig.build.json",