From 0efb55bf5dca4e76867dd7b7b0f1a5561632b6d7 Mon Sep 17 00:00:00 2001 From: Kayub Maharjan Date: Wed, 16 Aug 2023 11:51:21 +1000 Subject: [PATCH] - `github-scopes` FF cleanup --- src/config/feature-flags.ts | 1 - .../github-configuration-get.test.ts | 3 +-- .../github-configuration-router.test.ts | 11 ---------- src/routes/github/github-oauth.test.ts | 4 +--- src/routes/github/github-oauth.ts | 4 +--- src/routes/github/github-router.test.ts | 20 ++----------------- ...-subscription-deferred-install-get.test.ts | 3 +-- ...subscription-deferred-install-post.test.ts | 3 +-- 8 files changed, 7 insertions(+), 42 deletions(-) diff --git a/src/config/feature-flags.ts b/src/config/feature-flags.ts index c30db2d738..c2647ba881 100644 --- a/src/config/feature-flags.ts +++ b/src/config/feature-flags.ts @@ -34,7 +34,6 @@ export enum BooleanFlags { } export enum StringFlags { - GITHUB_SCOPES = "github-scopes", BLOCKED_INSTALLATIONS = "blocked-installations", LOG_LEVEL = "log-level", HEADERS_TO_ENCRYPT = "headers-to-encrypt", diff --git a/src/routes/github/configuration/github-configuration-get.test.ts b/src/routes/github/configuration/github-configuration-get.test.ts index ceb7ada289..6fbdfa67a2 100644 --- a/src/routes/github/configuration/github-configuration-get.test.ts +++ b/src/routes/github/configuration/github-configuration-get.test.ts @@ -6,7 +6,7 @@ import { Subscription } from "models/subscription"; import { DatabaseStateCreator } from "test/utils/database-state-creator"; import { Application } from "express"; import { getFrontendApp } from "~/src/app"; -import { booleanFlag, BooleanFlags, stringFlag, StringFlags } from "config/feature-flags"; +import { booleanFlag, BooleanFlags } from "config/feature-flags"; import { when } from "jest-when"; import { envVars } from "config/env"; import { GitHubServerApp } from "models/github-server-app"; @@ -25,7 +25,6 @@ describe("github-configuration-get", () => { app = getFrontendApp(); when(booleanFlag).calledWith(BooleanFlags.ENABLE_SUBSCRIPTION_DEFERRED_INSTALL, installation.jiraHost).mockResolvedValue(true); - when(stringFlag).calledWith(StringFlags.GITHUB_SCOPES, expect.anything(), expect.anything()).mockResolvedValue("user,repo"); }); describe("cloud", () => { diff --git a/src/routes/github/configuration/github-configuration-router.test.ts b/src/routes/github/configuration/github-configuration-router.test.ts index c655c67759..8366024af3 100644 --- a/src/routes/github/configuration/github-configuration-router.test.ts +++ b/src/routes/github/configuration/github-configuration-router.test.ts @@ -7,10 +7,7 @@ import { Application } from "express"; import { generateSignedSessionCookieHeader } from "test/utils/cookies"; import { ViewerRepositoryCountQuery } from "~/src/github/client/github-queries"; import installationResponse from "fixtures/jira-configuration/single-installation.json"; -import { when } from "jest-when"; -import { stringFlag, StringFlags } from "config/feature-flags"; -const DEFAULT_SCOPES = "user,repo"; jest.mock("config/feature-flags"); @@ -44,10 +41,6 @@ describe("Github Configuration", () => { describe("Github Token Validation", () => { it("should return redirect to github oauth flow for GET request if token is missing", async () => { - when(stringFlag) - .calledWith(StringFlags.GITHUB_SCOPES, expect.anything(), jiraHost) - .mockResolvedValue(DEFAULT_SCOPES); - return supertest(frontendApp) .get("/github/configuration") .set( @@ -67,10 +60,6 @@ describe("Github Configuration", () => { .get("/") .matchHeader("Authorization", /^Bearer .+$/) .reply(403); - when(stringFlag) - .calledWith(StringFlags.GITHUB_SCOPES, expect.anything(), jiraHost) - .mockResolvedValue(DEFAULT_SCOPES); - return supertest(frontendApp) .get("/github/configuration") .set( diff --git a/src/routes/github/github-oauth.test.ts b/src/routes/github/github-oauth.test.ts index 7cfc121ab7..c9de8c1a97 100644 --- a/src/routes/github/github-oauth.test.ts +++ b/src/routes/github/github-oauth.test.ts @@ -12,7 +12,7 @@ import { generateSignedSessionCookieHeader, parseCookiesAndSession } from "test/utils/cookies"; -import { booleanFlag, BooleanFlags, stringFlag, StringFlags } from "config/feature-flags"; +import { booleanFlag, BooleanFlags } from "config/feature-flags"; import { when } from "jest-when"; import { Installation } from "models/installation"; @@ -22,8 +22,6 @@ describe("github-oauth", () => { let installation: Installation; beforeEach(async () => { installation = (await new DatabaseStateCreator().create()).installation; - - when(stringFlag).calledWith(StringFlags.GITHUB_SCOPES, expect.anything(), expect.anything()).mockResolvedValue("user,repo"); }); describe("GithubOAuthCallbackGet", () => { diff --git a/src/routes/github/github-oauth.ts b/src/routes/github/github-oauth.ts index 650191971c..88b98b1e3c 100644 --- a/src/routes/github/github-oauth.ts +++ b/src/routes/github/github-oauth.ts @@ -8,7 +8,6 @@ import { createHashWithSharedSecret } from "utils/encryption"; import { GitHubServerApp } from "models/github-server-app"; import { GitHubAppConfig } from "~/src/sqs/sqs.types"; import Logger from "bunyan"; -import { stringFlag, StringFlags } from "config/feature-flags"; import * as querystring from "querystring"; import { Installation } from "models/installation"; @@ -32,8 +31,7 @@ const getRedirectUrl = async (res: Response, state: string) => { if (res.locals?.gitHubAppConfig?.uuid) { callbackPath = callbackPathServer.replace("", res.locals.gitHubAppConfig.uuid); } - const definedScopes = await stringFlag(StringFlags.GITHUB_SCOPES, "user,repo", res.locals.jiraHost); - const scopes = definedScopes.split(","); + const scopes = ["user", "repo"]; const { hostname, clientId } = res.locals.gitHubAppConfig; const callbackURI = `${appUrl}${callbackPath}`; diff --git a/src/routes/github/github-router.test.ts b/src/routes/github/github-router.test.ts index 041135433c..0974094b6c 100644 --- a/src/routes/github/github-router.test.ts +++ b/src/routes/github/github-router.test.ts @@ -12,8 +12,6 @@ import { generateSignedSessionCookieHeader, parseCookiesAndSession } from "test/utils/cookies"; -import { when } from "jest-when"; -import { stringFlag, StringFlags } from "config/feature-flags"; import * as cookie from "cookie"; jest.mock("./configuration/github-configuration-get"); @@ -23,8 +21,6 @@ const VALID_TOKEN = "valid-token"; const GITHUB_SERVER_APP_UUID: string = v4uuid(); const GITHUB_SERVER_APP_ID = Math.floor(Math.random() * 10000); const GITHUB_SERVER_CLIENT_ID = "client-id"; -const DEFAULT_SCOPES = "user,repo"; -const TESTING_SCOPES = "scope1,scope2"; const prepareGitHubServerAppInDB = async (jiraInstallaionId: number) => { const existed = await GitHubServerApp.findForUuid(GITHUB_SERVER_APP_UUID); @@ -66,12 +62,6 @@ const mockConfigurationGetProceed = ()=>{ }; describe("GitHub router", () => { - beforeEach(() => { - when(stringFlag) - .calledWith(StringFlags.GITHUB_SCOPES, expect.anything(), jiraHost) - .mockResolvedValue(DEFAULT_SCOPES); - }); - describe("Common route utilities", () => { describe("Cloud scenario", () => { let app: Application; @@ -103,9 +93,6 @@ describe("GitHub router", () => { }); }); it("testing the redirect URL in GithubOAuthLoginGet middleware when FF is on", async () => { - when(stringFlag) - .calledWith(StringFlags.GITHUB_SCOPES, expect.anything(), jiraHost) - .mockResolvedValue(TESTING_SCOPES); const response = await supertest(app) .get(`/github/configuration`) .set("x-forwarded-proto", "https") // otherwise cookies won't be returned cause they are "secure" @@ -124,7 +111,7 @@ describe("GitHub router", () => { const resultUrl = response.headers.location; const redirectUrl = `${envVars.APP_URL}/github/callback`; - const expectedUrl = `https://github.com/login/oauth/authorize?client_id=${envVars.GITHUB_CLIENT_ID}&scope=scope1%20scope2&redirect_uri=${encodeURIComponent(redirectUrl)}&state=${oauthStateKey}`; + const expectedUrl = `https://github.com/login/oauth/authorize?client_id=${envVars.GITHUB_CLIENT_ID}&scope=user%20repo&redirect_uri=${encodeURIComponent(redirectUrl)}&state=${oauthStateKey}`; expect(resultUrl).toEqual(expectedUrl); expect(oauthState["postLoginRedirectUrl"]).toStrictEqual(`/github/configuration`); @@ -242,9 +229,6 @@ describe("GitHub router", () => { }); it("testing the redirect URL in GithubOAuthLoginGet middleware when FF is on", async () => { - when(stringFlag) - .calledWith(StringFlags.GITHUB_SCOPES, expect.anything(), jiraHost) - .mockResolvedValue(TESTING_SCOPES); const response = await supertest(app) .get(`/github/${GITHUB_SERVER_APP_UUID}/configuration`) .set("x-forwarded-proto", "https") // otherwise cookies won't be returned cause they are "secure" @@ -260,7 +244,7 @@ describe("GitHub router", () => { const resultUrl = response.headers.location; const resultUrlWithoutState = resultUrl.split("&state")[0];// Ignoring state here cause state is different everytime const redirectUrl = `${envVars.APP_URL}/github/${GITHUB_SERVER_APP_UUID}/callback`; - const expectedUrlWithoutState = `${gheUrl}/login/oauth/authorize?client_id=${GITHUB_SERVER_CLIENT_ID}&scope=scope1%20scope2&redirect_uri=${encodeURIComponent(redirectUrl)}`; + const expectedUrlWithoutState = `${gheUrl}/login/oauth/authorize?client_id=${GITHUB_SERVER_CLIENT_ID}&scope=user%20repo&redirect_uri=${encodeURIComponent(redirectUrl)}`; expect(resultUrlWithoutState).toEqual(expectedUrlWithoutState); const { session } = parseCookiesAndSession(response); diff --git a/src/routes/github/subscription-deferred-install/github-subscription-deferred-install-get.test.ts b/src/routes/github/subscription-deferred-install/github-subscription-deferred-install-get.test.ts index 5670320b0e..4d88ec2b9a 100644 --- a/src/routes/github/subscription-deferred-install/github-subscription-deferred-install-get.test.ts +++ b/src/routes/github/subscription-deferred-install/github-subscription-deferred-install-get.test.ts @@ -8,7 +8,7 @@ import { } from "services/subscription-deferred-install-service"; import { Subscription } from "models/subscription"; import supertest from "supertest"; -import { booleanFlag, BooleanFlags, stringFlag, StringFlags } from "config/feature-flags"; +import { booleanFlag, BooleanFlags } from "config/feature-flags"; import { when } from "jest-when"; import { generateSignedSessionCookieHeader } from "test/utils/cookies"; import { hasAdminAccess } from "services/subscription-installation-service"; @@ -36,7 +36,6 @@ describe("github-subscription-deferred-install-get", () => { }; when(booleanFlag).calledWith(BooleanFlags.ENABLE_SUBSCRIPTION_DEFERRED_INSTALL, installation.jiraHost).mockResolvedValue(true); - when(stringFlag).calledWith(StringFlags.GITHUB_SCOPES, expect.anything(), expect.anything()).mockResolvedValue("user,repo"); }); describe("cloud", () => { diff --git a/src/routes/github/subscription-deferred-install/github-subscription-deferred-install-post.test.ts b/src/routes/github/subscription-deferred-install/github-subscription-deferred-install-post.test.ts index cf0c09b280..4e84def038 100644 --- a/src/routes/github/subscription-deferred-install/github-subscription-deferred-install-post.test.ts +++ b/src/routes/github/subscription-deferred-install/github-subscription-deferred-install-post.test.ts @@ -9,7 +9,7 @@ import { } from "services/subscription-deferred-install-service"; import { Subscription } from "models/subscription"; import supertest from "supertest"; -import { booleanFlag, BooleanFlags, stringFlag, StringFlags } from "config/feature-flags"; +import { booleanFlag, BooleanFlags } from "config/feature-flags"; import { when } from "jest-when"; import { generateSignedSessionCookieHeader } from "test/utils/cookies"; import { verifyAdminPermsAndFinishInstallation } from "services/subscription-installation-service"; @@ -37,7 +37,6 @@ describe("github-subscription-deferred-install-post", () => { }; when(booleanFlag).calledWith(BooleanFlags.ENABLE_SUBSCRIPTION_DEFERRED_INSTALL, installation.jiraHost).mockResolvedValue(true); - when(stringFlag).calledWith(StringFlags.GITHUB_SCOPES, expect.anything(), expect.anything()).mockResolvedValue("user,repo"); }); describe("cloud", () => {