Skip to content

Commit

Permalink
FF Cleanup - github-scopes (#2346)
Browse files Browse the repository at this point in the history
- `github-scopes` FF cleanup
  • Loading branch information
krazziekay authored Aug 16, 2023
1 parent 0531ce3 commit dbf81ee
Show file tree
Hide file tree
Showing 8 changed files with 7 additions and 42 deletions.
1 change: 0 additions & 1 deletion src/config/feature-flags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand Down
4 changes: 1 addition & 3 deletions src/routes/github/github-oauth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand All @@ -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", () => {
Expand Down
4 changes: 1 addition & 3 deletions src/routes/github/github-oauth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand All @@ -32,8 +31,7 @@ const getRedirectUrl = async (res: Response, state: string) => {
if (res.locals?.gitHubAppConfig?.uuid) {
callbackPath = callbackPathServer.replace("<uuid>", 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}`;
Expand Down
20 changes: 2 additions & 18 deletions src/routes/github/github-router.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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"
Expand All @@ -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`);
Expand Down Expand Up @@ -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"
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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", () => {
Expand Down

0 comments on commit dbf81ee

Please sign in to comment.