From 27642d79e0155c3b984fcc44b1e8f520dae077b0 Mon Sep 17 00:00:00 2001 From: Boris Gvozdev Date: Mon, 6 Nov 2023 14:30:33 +1100 Subject: [PATCH] NONE: remove FF jira-admin-check --- src/config/feature-flags.ts | 1 - .../jira-admin-permission-middleware.test.ts | 48 ------------------- .../jira-admin-permission-middleware.ts | 6 +-- .../jira-admin/jira-admin-check.test.ts | 5 -- .../middleware/jira-admin/jira-admin-check.ts | 7 +-- src/routes/github/github-oauth.test.ts | 6 --- .../jira/jira-connected-repos-get.test.ts | 6 --- 7 files changed, 2 insertions(+), 77 deletions(-) diff --git a/src/config/feature-flags.ts b/src/config/feature-flags.ts index d0a153d71a..ad009ac9ea 100644 --- a/src/config/feature-flags.ts +++ b/src/config/feature-flags.ts @@ -15,7 +15,6 @@ export enum BooleanFlags { MAINTENANCE_MODE = "maintenance-mode", VERBOSE_LOGGING = "verbose-logging", SEND_PR_COMMENTS_TO_JIRA = "send-pr-comments-to-jira_zy5ib", - JIRA_ADMIN_CHECK = "jira-admin-check", REMOVE_STALE_MESSAGES = "remove-stale-messages", USE_DYNAMODB_FOR_DEPLOYMENT_WEBHOOK = "use-dynamodb-for-deployment-webhook", USE_DYNAMODB_FOR_DEPLOYMENT_BACKFILL = "use-dynamodb-for-deployment-backfill", diff --git a/src/middleware/jira-admin-permission-middleware.test.ts b/src/middleware/jira-admin-permission-middleware.test.ts index 29543f8edb..caa6b1ac7c 100644 --- a/src/middleware/jira-admin-permission-middleware.test.ts +++ b/src/middleware/jira-admin-permission-middleware.test.ts @@ -4,10 +4,6 @@ import { fetchAndSaveUserJiraAdminStatus, jiraAdminPermissionsMiddleware } from "middleware/jira-admin-permission-middleware"; -import { booleanFlag, BooleanFlags } from "config/feature-flags"; -import { when } from "jest-when"; - -jest.mock("config/feature-flags"); describe("jiraAdminPermissionsMiddleware", () => { let mockRequest; @@ -24,10 +20,6 @@ describe("jiraAdminPermissionsMiddleware", () => { send: jest.fn() }; mockNext = jest.fn(); - - when(booleanFlag).calledWith( - BooleanFlags.JIRA_ADMIN_CHECK - ).mockResolvedValue(true); }); test("should return 403 Forbidden if session is undefined", async () => { @@ -48,46 +40,6 @@ describe("jiraAdminPermissionsMiddleware", () => { }); }); -// Delete this describe block during flag clean up -describe("jiraAdminPermissionsMiddleware - feature flag off", () => { - let mockRequest; - let mockResponse; - let mockNext; - - beforeEach(() => { - mockRequest = { - session: {}, - log: { info: jest.fn() } - }; - mockResponse = { - status: jest.fn().mockReturnThis(), - send: jest.fn() - }; - mockNext = jest.fn(); - - when(booleanFlag).calledWith( - BooleanFlags.JIRA_ADMIN_CHECK - ).mockResolvedValue(false); - }); - - test("should return 403 Forbidden if session is undefined", async () => { - delete mockRequest.session.isJiraAdmin; - await jiraAdminPermissionsMiddleware(mockRequest, mockResponse, mockNext); - expect(mockNext).toHaveBeenCalled(); - }); - - test("should return 403 Forbidden if hasAdminPermissions is false", async () => { - mockRequest.session.isJiraAdmin = false; - await jiraAdminPermissionsMiddleware(mockRequest, mockResponse, mockNext); - expect(mockNext).toHaveBeenCalled(); - }); - - test("should call next() if user has Jira admin permissions", async () => { - mockRequest.session.isJiraAdmin = true; - await jiraAdminPermissionsMiddleware(mockRequest, mockResponse, mockNext); - expect(mockNext).toHaveBeenCalled(); - }); -}); describe("fetchAndSaveUserJiraAdminStatus", () => { const mockRequest = { diff --git a/src/middleware/jira-admin-permission-middleware.ts b/src/middleware/jira-admin-permission-middleware.ts index 66e99b1f19..88f7baf5c9 100644 --- a/src/middleware/jira-admin-permission-middleware.ts +++ b/src/middleware/jira-admin-permission-middleware.ts @@ -1,7 +1,6 @@ import { NextFunction, Request, Response } from "express"; import { Installation } from "models/installation"; import { JiraClient } from "models/jira-client"; -import { booleanFlag, BooleanFlags } from "config/feature-flags"; export const fetchAndSaveUserJiraAdminStatus = async (req: Request, claims: { sub?: string;}, installation: Installation): Promise => { const ADMIN_PERMISSION = "ADMINISTER"; @@ -27,10 +26,7 @@ export const fetchAndSaveUserJiraAdminStatus = async (req: Request, claims: { su } }; -export const jiraAdminPermissionsMiddleware = async (req: Request, res: Response, next: NextFunction): Promise => { - if (!(await booleanFlag(BooleanFlags.JIRA_ADMIN_CHECK))) { - return next(); - } +export const jiraAdminPermissionsMiddleware = (req: Request, res: Response, next: NextFunction): void | Response => { const { isJiraAdmin } = req.session; if (isJiraAdmin === undefined) { diff --git a/src/rest/middleware/jira-admin/jira-admin-check.test.ts b/src/rest/middleware/jira-admin/jira-admin-check.test.ts index add780839d..e1ef917a0a 100644 --- a/src/rest/middleware/jira-admin/jira-admin-check.test.ts +++ b/src/rest/middleware/jira-admin/jira-admin-check.test.ts @@ -2,11 +2,8 @@ import { encodeSymmetric } from "atlassian-jwt"; import { Application } from "express"; import supertest from "supertest"; import { Installation } from "models/installation"; -import { booleanFlag, BooleanFlags } from "config/feature-flags"; -import { when } from "jest-when"; import { getFrontendApp } from "~/src/app"; -jest.mock("config/feature-flags"); jest.mock("models/jira-client"); const testSharedSecret = "test-secret"; @@ -19,8 +16,6 @@ describe("Jira Admin Check", () => { beforeEach(async () => { - when(booleanFlag).calledWith(BooleanFlags.JIRA_ADMIN_CHECK, jiraHost).mockResolvedValue(true); - app = getFrontendApp(); await Installation.install({ diff --git a/src/rest/middleware/jira-admin/jira-admin-check.ts b/src/rest/middleware/jira-admin/jira-admin-check.ts index a083dcac76..7eaaba6ae8 100644 --- a/src/rest/middleware/jira-admin/jira-admin-check.ts +++ b/src/rest/middleware/jira-admin/jira-admin-check.ts @@ -1,6 +1,5 @@ import { NextFunction, Request, Response } from "express"; import { JiraClient } from "models/jira-client"; -import { booleanFlag, BooleanFlags } from "config/feature-flags"; import { InsufficientPermissionError, InvalidTokenError } from "config/errors"; import { errorWrapper } from "../../helper"; import { BaseLocals } from "../../routes"; @@ -8,11 +7,7 @@ import { BaseLocals } from "../../routes"; const ADMIN_PERMISSION = "ADMINISTER"; export const JiraAdminEnforceMiddleware = errorWrapper("jiraAdminEnforceMiddleware", async (req: Request, res: Response, next: NextFunction): Promise => { - const { accountId, installation, jiraHost } = res.locals; - - if (!(await booleanFlag(BooleanFlags.JIRA_ADMIN_CHECK, jiraHost))) { - return next(); - } + const { accountId, installation } = res.locals; if (!accountId) { throw new InvalidTokenError("Missing userAccountId"); diff --git a/src/routes/github/github-oauth.test.ts b/src/routes/github/github-oauth.test.ts index 57a36dd466..9cdc7ccdc9 100644 --- a/src/routes/github/github-oauth.test.ts +++ b/src/routes/github/github-oauth.test.ts @@ -12,12 +12,8 @@ import { generateSignedSessionCookieHeader, parseCookiesAndSession } from "test/utils/cookies"; -import { booleanFlag, BooleanFlags } from "config/feature-flags"; -import { when } from "jest-when"; import { Installation } from "models/installation"; -jest.mock("config/feature-flags"); - describe("github-oauth", () => { let installation: Installation; beforeEach(async () => { @@ -175,8 +171,6 @@ describe("github-oauth", () => { }); it("must work only for Jira admins", async () => { - when(booleanFlag).calledWith(BooleanFlags.JIRA_ADMIN_CHECK).mockResolvedValue(true); - const res = await supertest(getFrontendApp()) .get("/github/login?blah=true") .set( diff --git a/src/routes/jira/jira-connected-repos-get.test.ts b/src/routes/jira/jira-connected-repos-get.test.ts index 35ba654d90..7812fb2b5a 100644 --- a/src/routes/jira/jira-connected-repos-get.test.ts +++ b/src/routes/jira/jira-connected-repos-get.test.ts @@ -5,12 +5,8 @@ import { getLogger } from "config/logger"; import { Subscription } from "models/subscription"; import { DatabaseStateCreator } from "test/utils/database-state-creator"; import supertest from "supertest"; -import { booleanFlag, BooleanFlags } from "config/feature-flags"; -import { when } from "jest-when"; import { RepoSyncState } from "models/reposyncstate"; -jest.mock("config/feature-flags"); - describe("jira-connected-repos-get", () => { let app; @@ -35,8 +31,6 @@ describe("jira-connected-repos-get", () => { installation = result.installation; subscription = result.subscription; repoSyncState = result.repoSyncState!; - - when(booleanFlag).calledWith(BooleanFlags.JIRA_ADMIN_CHECK).mockResolvedValue(true); }); it("should return 403 when not an admin", async () => {