Remove use-installation-client-to-check-permission (#2557)

src/config/feature-flags.ts

@@ -22,7 +22,6 @@ export enum BooleanFlags {
 	ENABLE_GITHUB_SECURITY_IN_JIRA = "enable-github-security-in-jira",
 	DELETE_MESSAGE_ON_BACKFILL_WHEN_OTHERS_WORKING_ON_IT = "delete-message-on-backfill-when-others-working-on-it",
 	ENABLE_5KU_BACKFILL_PAGE = "enable-5ku-experience-backfill-page",
-	USE_INSTALLATION_CLIENT_CHECK_PERMISSION = "use-installation-client-to-check-permission",
 	USE_CUSTOM_ROOT_CA_BUNDLE = "use-custom-root-ca-bundle",
 	GENERATE_CORE_HEAP_DUMPS_ON_LOW_MEM = "generate-core-heap-dumps-on-low-mem"
 }

src/routes/github/configuration/github-configuration-get.ts

@@ -89,7 +89,6 @@ export const getInstallationsWithAdmin = async (
 		//  all orgs the user is a member of and cross reference with the installation org
 		const checkAdmin = isUserAdminOfOrganization(
 			gitHubUserClient,
-			jiraHost,
 			gitHubClient,
 			installation.account.login,
 			login,

src/routes/github/subscription/github-subscription-delete.ts

@@ -40,7 +40,6 @@ export const GithubSubscriptionDelete = async (req: Request, res: Response): Pro
 		// Only show the page if the logged in user is an admin of this installation
 		if (!await isUserAdminOfOrganization(
 			gitHubUserClient,
-			jiraHost,
 			gitHubInstallationClient ,
 			installation.account.login,
 			login,

src/services/subscription-installation-service.ts

@@ -31,7 +31,7 @@ export const hasAdminAccess = async (githubToken: string, jiraHost: string, gitH
 		const gitHubInstallationClient = await createInstallationClient(gitHubInstallationId, jiraHost, { trigger: "hasAdminAccess" }, logger, gitHubServerAppIdPk);
 
 		logger.info("Checking if the user is an admin");
-		return await isUserAdminOfOrganization(gitHubUserClient, jiraHost, gitHubInstallationClient, installation.account.login, login, installation.target_type, logger);
+		return await isUserAdminOfOrganization(gitHubUserClient, gitHubInstallationClient, installation.account.login, login, installation.target_type, logger);
 	} catch (err: unknown) {
 		logger.warn({ err }, "Error checking user access");
 		return false;

src/util/github-utils.test.ts

@@ -4,8 +4,6 @@ import { GitHubUserClient } from "~/src/github/client/github-user-client";
 import { GitHubInstallationClient } from "~/src/github/client/github-installation-client";
 import { InstallationId } from "~/src/github/client/installation-id";
 import { getLogger } from "config/logger";
-import { BooleanFlags, booleanFlag } from "config/feature-flags";
-import { when } from "jest-when";
 
 jest.mock("config/feature-flags");
 
@@ -25,7 +23,6 @@ describe("GitHub Utils", () => {
 
 			expect(await isUserAdminOfOrganization(
 				githubUserClient,
-				jiraHost,
 				gitHubInstallationClient,
 				"test-org",
 				"test-user",
@@ -41,7 +38,6 @@ describe("GitHub Utils", () => {
 
 			expect(await isUserAdminOfOrganization(
 				githubUserClient,
-				jiraHost,
 				gitHubInstallationClient,
 				"test-org",
 				"test-user",
@@ -53,7 +49,6 @@ describe("GitHub Utils", () => {
 		it("should return true if repo is owned by a given user", async () => {
 			expect(await isUserAdminOfOrganization(
 				githubUserClient,
-				jiraHost,
 				gitHubInstallationClient,
 				"test-user",
 				"test-user",
@@ -65,7 +60,6 @@ describe("GitHub Utils", () => {
 		it("should return false if repo is owned by another user", async () => {
 			expect(await isUserAdminOfOrganization(
 				githubUserClient,
-				jiraHost,
 				gitHubInstallationClient,
 				"different-user",
 				"test-user",
@@ -76,8 +70,6 @@ describe("GitHub Utils", () => {
 
 		it("should call app client to check permission, fail at non admin role", async () => {
 
-			when(booleanFlag).calledWith(BooleanFlags.USE_INSTALLATION_CLIENT_CHECK_PERMISSION, expect.anything()).mockResolvedValue(true);
-
 			githubNock.post("/app/installations/111/access_tokens").reply(200, { token: "token", expires_at: new Date().getTime() });
 
 			githubNock
@@ -86,7 +78,6 @@ describe("GitHub Utils", () => {
 
 			expect(await isUserAdminOfOrganization(
 				githubUserClient,
-				jiraHost,
 				gitHubInstallationClient,
 				"test-org",
 				"test-user",
@@ -97,8 +88,6 @@ describe("GitHub Utils", () => {
 
 		it("should call app client to check permission, fail at non-active state", async () => {
 
-			when(booleanFlag).calledWith(BooleanFlags.USE_INSTALLATION_CLIENT_CHECK_PERMISSION, expect.anything()).mockResolvedValue(true);
-
 			githubNock.post("/app/installations/111/access_tokens").reply(200, { token: "token", expires_at: new Date().getTime() });
 
 			githubNock
@@ -107,7 +96,6 @@ describe("GitHub Utils", () => {
 
 			expect(await isUserAdminOfOrganization(
 				githubUserClient,
-				jiraHost,
 				gitHubInstallationClient,
 				"test-org",
 				"test-user",
@@ -118,8 +106,6 @@ describe("GitHub Utils", () => {
 
 		it("should call app client to check permission, success when active and admin", async () => {
 
-			when(booleanFlag).calledWith(BooleanFlags.USE_INSTALLATION_CLIENT_CHECK_PERMISSION, expect.anything()).mockResolvedValue(true);
-
 			githubNock.post("/app/installations/111/access_tokens").reply(200, { token: "token", expires_at: new Date().getTime() });
 
 			githubNock
@@ -128,7 +114,6 @@ describe("GitHub Utils", () => {
 
 			expect(await isUserAdminOfOrganization(
 				githubUserClient,
-				jiraHost,
 				gitHubInstallationClient,
 				"test-org",
 				"test-user",

src/util/github-utils.ts

@@ -1,28 +1,25 @@
 import { GitHubUserClient } from "~/src/github/client/github-user-client";
 import { GitHubInstallationClient } from "~/src/github/client/github-installation-client";
 import Logger from "bunyan";
-import { booleanFlag, BooleanFlags } from "config/feature-flags";
 
-export const isUserAdminOfOrganization = async (githubClient: GitHubUserClient, jiraHost: string, gitHubInstallationClient: GitHubInstallationClient, orgName: string, username: string, orgType: string, logger: Logger): Promise<boolean> => {
+export const isUserAdminOfOrganization = async (githubUserClient: GitHubUserClient, gitHubInstallationClient: GitHubInstallationClient, orgName: string, username: string, orgType: string, logger: Logger): Promise<boolean> => {
 	if (orgType === "User") {
 		logger.info("isUserAdminOfOrganization: orgType is a user");
 		return orgName === username;
 	}
 
 	try {
-		if (await booleanFlag(BooleanFlags.USE_INSTALLATION_CLIENT_CHECK_PERMISSION, jiraHost)) {
-			logger.info("isUserAdminOfOrganization: orgType is an org, checking membership (app client)");
-			const { data: { state, role } } = await gitHubInstallationClient.getUserMembershipForOrg(username, orgName);
-			logger.info({ orgName, username, state, role }, `isUserAdminOfOrganization: User has role for org`);
-			return role === "admin" && state === "active";
-		}
+		logger.info("isUserAdminOfOrganization: orgType is an org, checking membership (app client)");
+		const { data: { state, role } } = await gitHubInstallationClient.getUserMembershipForOrg(username, orgName);
+		logger.info({ orgName, username, state, role }, `isUserAdminOfOrganization: User has role for org`);
+		return role === "admin" && state === "active";
 	} catch (err: unknown) {
-		logger.error({ err }, "Fail checking permission using GitHub App Client, fallback to user client");
+		logger.warn({ err }, "Fail checking permission using GitHub App Client, fallback to user client");
 	}
 
 	try {
-		logger.info("isUserAdminOfOrganization: orgType is an org, checking membership");
-		const { data: { role } } = await githubClient.getMembershipForOrg(orgName);
+		logger.info("isUserAdminOfOrganization: orgType is an org, checking membership (user client)");
+		const { data: { role } } = await githubUserClient.getMembershipForOrg(orgName);
 		logger.info({ orgName, username }, `isUserAdminOfOrganization: User has ${role} role for org`);
 		return role === "admin";
 	} catch (err: unknown) {