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] 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