ARC-1810 remove decrypted ghe app secrets from res.locals (atlassian#…

…1730)

* ARC-1810 remove decrypted ghe app secrets from res.locals

src/middleware/github-server-app-middleware.test.ts

@@ -100,20 +100,7 @@ describe("github-server-app-middleware", () => {
 			webhookSecret: "encrypted:mywebhooksecret",
 			privateKey: "encrypted:myprivatekey",
 			installationId: JIRA_INSTALLATION_ID,
-			decrypt: async (s: any) => s,
-
-			getDecryptedGitHubClientSecret: () => {
-				return Promise.resolve("myghsecret");
-			},
-
-			getDecryptedPrivateKey: () => {
-				return Promise.resolve("myprivatekey");
-			},
-
-			getDecryptedWebhookSecret: () => {
-				return Promise.resolve("mywebhooksecret");
-			}
-
+			decrypt: async (s: any) => s
 		};
 
 		installation = {
@@ -139,10 +126,7 @@ describe("github-server-app-middleware", () => {
 			appId: GIT_HUB_SERVER_APP_APP_ID,
 			uuid: UUID,
 			clientId: "lvl.1234",
-			gitHubClientSecret: "myghsecret",
-			hostname: "http://myinternalserver.com",
-			privateKey: "myprivatekey",
-			webhookSecret: "mywebhooksecret"
+			hostname: "http://myinternalserver.com"
 		});
 	});
 });

src/middleware/github-server-app-middleware.ts

@@ -2,7 +2,6 @@ import { NextFunction, Request, Response } from "express";
 import { GitHubServerApp } from "models/github-server-app";
 import { Installation } from "models/installation";
 import { envVars } from "config/env";
-import { keyLocator } from "../github/client/key-locator";
 import { GITHUB_CLOUD_BASEURL } from "utils/get-github-client-config";
 
 export const GithubServerAppMiddleware = async (req: Request, res: Response, next: NextFunction): Promise<void> => {
@@ -43,10 +42,7 @@ export const GithubServerAppMiddleware = async (req: Request, res: Response, nex
 			appId: gitHubServerApp.appId,
 			uuid: gitHubServerApp.uuid,
 			hostname: gitHubServerApp.gitHubBaseUrl,
-			clientId: gitHubServerApp.gitHubClientId,
-			gitHubClientSecret: await gitHubServerApp.getDecryptedGitHubClientSecret(),
-			webhookSecret: await gitHubServerApp.getDecryptedWebhookSecret(),
-			privateKey: await gitHubServerApp.getDecryptedPrivateKey()
+			clientId: gitHubServerApp.gitHubClientId
 		};
 	} else {
 		req.log.info("Defining GitHub app config for GitHub Cloud.");
@@ -55,10 +51,7 @@ export const GithubServerAppMiddleware = async (req: Request, res: Response, nex
 			appId: envVars.APP_ID,
 			uuid: undefined, //undefined for cloud
 			hostname: GITHUB_CLOUD_BASEURL,
-			clientId: envVars.GITHUB_CLIENT_ID,
-			gitHubClientSecret: envVars.GITHUB_CLIENT_SECRET,
-			webhookSecret: envVars.WEBHOOK_SECRET,
-			privateKey: await keyLocator(undefined)
+			clientId: envVars.GITHUB_CLIENT_ID
 		};
 	}
 

src/models/github-server-app.test.ts

@@ -23,6 +23,37 @@ describe("GitHubServerApp", () => {
 		expect(savedGitHubServerApp?.gitHubAppName).toEqual("My GitHub Server App");
 	});
 
+	describe("GHES function", () => {
+		it("should NOT update missing columns if it is not specified in the request body", async () => {
+
+			await GitHubServerApp.install(payload);
+
+			const payLoadWithoutSomeColumns = {
+				uuid: uuid,
+				appId: 123,
+				privateKey: undefined //some specified as with undefined
+				//some specified as missing the key ( so undefined as well )
+			};
+
+			await GitHubServerApp.updateGitHubAppByUUID(payLoadWithoutSomeColumns);
+
+			const app = await GitHubServerApp.findForUuid(uuid);
+
+			expect(app).toBeDefined();
+			expect(app).toEqual(expect.objectContaining({
+				uuid: uuid,
+				appId: 123,
+				gitHubAppName: "My GitHub Server App",
+				gitHubBaseUrl: "http://myinternalserver.com",
+				gitHubClientId: "lvl.1234",
+				gitHubClientSecret: "encrypted:myghsecret",
+				webhookSecret: "encrypted:mywebhooksecret",
+				privateKey: "encrypted:myprivatekey",
+				installationId: 10
+			}));
+		});
+	});
+
 	describe("cryptor", () => {
 		//--------- helpers
 		const defaults = (uuid: string, surfix?: string) => ({
@@ -292,21 +323,21 @@ describe("GitHubServerApp", () => {
 	it("getDecryptedPrivateKey should return decrypted value", async () => {
 		await GitHubServerApp.install(payload);
 		const savedGitHubServerApp = await GitHubServerApp.findForUuid(uuid);
-		expect(await savedGitHubServerApp!.privateKey).toEqual("encrypted:myprivatekey");
+		expect(savedGitHubServerApp!.privateKey).toEqual("encrypted:myprivatekey");
 		expect(await savedGitHubServerApp!.getDecryptedPrivateKey()).toEqual("myprivatekey");
 	});
 
 	it("getDecryptedGitHubClientSecret should return decrypted value", async () => {
 		await GitHubServerApp.install(payload);
 		const savedGitHubServerApp = await GitHubServerApp.findForUuid(uuid);
-		expect(await savedGitHubServerApp!.gitHubClientSecret).toEqual("encrypted:myghsecret");
+		expect(savedGitHubServerApp!.gitHubClientSecret).toEqual("encrypted:myghsecret");
 		expect(await savedGitHubServerApp!.getDecryptedGitHubClientSecret()).toEqual("myghsecret");
 	});
 
 	it("getDecryptedWebhookSecret should return decrypted value", async () => {
 		await GitHubServerApp.install(payload);
 		const savedGitHubServerApp = await GitHubServerApp.findForUuid(uuid);
-		expect(await savedGitHubServerApp!.webhookSecret).toEqual("encrypted:mywebhooksecret");
+		expect(savedGitHubServerApp!.webhookSecret).toEqual("encrypted:mywebhooksecret");
 		expect(await savedGitHubServerApp!.getDecryptedWebhookSecret()).toEqual("mywebhooksecret");
 	});
 });

src/models/github-server-app.ts

@@ -23,6 +23,19 @@ export interface GitHubServerAppPayload {
 	installationId: number;
 }
 
+export interface GitHubServerAppUpdatePayload {
+	uuid: string;
+	appId: number;
+	gitHubBaseUrl?: string;
+	gitHubClientId?: string;
+	gitHubClientSecret?: string;
+	webhookSecret?: string;
+	privateKey?: string;
+	gitHubAppName?: string;
+	installationId?: number;
+}
+
+
 export class GitHubServerApp extends EncryptedModel {
 	id: number;
 	uuid: string;
@@ -157,7 +170,7 @@ export class GitHubServerApp extends EncryptedModel {
 		});
 	}
 
-	static async updateGitHubAppByUUID(payload: GitHubServerAppPayload): Promise<void> {
+	static async updateGitHubAppByUUID(payload: GitHubServerAppUpdatePayload): Promise<void> {
 		const {
 			uuid,
 			appId,

src/routes/github/github-oauth-router.ts

@@ -8,6 +8,7 @@ import { Errors } from "config/errors";
 import { getGitHubApiUrl, createAnonymousClientByGitHubAppId } from "~/src/util/get-github-client-config";
 import { createHashWithSharedSecret } from "utils/encryption";
 import { BooleanFlags, booleanFlag } from "config/feature-flags";
+import { GitHubServerApp } from "models/github-server-app";
 
 const logger = getLogger("github-oauth");
 const appUrl = envVars.APP_URL;
@@ -79,10 +80,13 @@ const GithubOAuthCallbackGet = async (req: Request, res: Response, next: NextFun
 	if (!code) return next("Missing OAuth Code");
 
 	const { jiraHost, gitHubAppConfig } = res.locals;
-	const { hostname, clientId, gitHubClientSecret, uuid } = gitHubAppConfig;
+	const { hostname, clientId, uuid } = gitHubAppConfig;
 	req.log.info({ jiraHost }, "Jira Host attempting to auth with GitHub");
 	req.log.debug(`extracted jiraHost from redirect url: ${jiraHost}`);
 
+	const gitHubClientSecret = await getCloudOrGHESAppClientSecret(gitHubAppConfig);
+	if (!gitHubClientSecret) return next("Missing GitHubApp client secret from uuid");
+
 	logger.info(`${createHashWithSharedSecret(gitHubClientSecret)} is used`);
 
 
@@ -182,6 +186,18 @@ export const GithubAuthMiddleware = async (req: Request, res: Response, next: Ne
 	}
 };
 
+const getCloudOrGHESAppClientSecret = async (gitHubAppConfig) => {
+
+	if (!gitHubAppConfig.gitHubAppId) {
+		return envVars.GITHUB_CLIENT_SECRET;
+	}
+
+	const ghesApp = await GitHubServerApp.findForUuid(gitHubAppConfig.uuid);
+	if (!ghesApp) return undefined;
+
+	return ghesApp.getDecryptedGitHubClientSecret();
+};
+
 // IMPORTANT: We need to keep the login/callback/middleware functions
 // in the same file as they reference each other
 export const GithubOAuthRouter = Router();

src/routes/github/github-router.test.ts

@@ -115,9 +115,7 @@ describe("GitHub router", () => {
 							githubToken: VALID_TOKEN,
 							jiraHost,
 							gitHubAppConfig: expect.objectContaining({
-								appId: envVars.APP_ID,
-								gitHubClientSecret: envVars.GITHUB_CLIENT_SECRET,
-								webhookSecret: envVars.WEBHOOK_SECRET
+								appId: envVars.APP_ID
 							})
 						})
 					}),
@@ -131,9 +129,7 @@ describe("GitHub router", () => {
 						githubToken: VALID_TOKEN,
 						jiraHost,
 						gitHubAppConfig: expect.objectContaining({
-							appId: envVars.APP_ID,
-							gitHubClientSecret: envVars.GITHUB_CLIENT_SECRET,
-							webhookSecret: envVars.WEBHOOK_SECRET
+							appId: envVars.APP_ID
 						})
 					}));
 			});
@@ -191,10 +187,7 @@ describe("GitHub router", () => {
 							gitHubAppId: gitHubAppId,
 							gitHubAppConfig: expect.objectContaining({
 								appId: GITHUB_SERVER_APP_ID,
-								uuid: GITHUB_SERVER_APP_UUID,
-								gitHubClientSecret: "gitHubClientSecret",
-								webhookSecret: "webhookSecret",
-								privateKey: "privateKey"
+								uuid: GITHUB_SERVER_APP_UUID
 							})
 						})
 					}),

src/routes/jira/connect/enterprise/app/jira-connect-enterprise-app-put.test.ts

@@ -67,6 +67,46 @@ describe("PUT /jira/connect/enterprise/app/:uuid", () => {
 			})
 			.send(payload)
 			.expect(202);
+
+	});
+
+	it("should use existing privateKey if new privateKey is not passed in as body", async () => {
+
+		let existingApp = await GitHubServerApp.create({
+			uuid,
+			appId: 1,
+			gitHubAppName: "my awesome app",
+			gitHubBaseUrl: "http://myinternalinstance.com",
+			gitHubClientId: "lvl.1n23j12389wndd",
+			gitHubClientSecret: "secret",
+			webhookSecret: "anothersecret",
+			privateKey: "privatekey",
+			installationId: installation.id
+		});
+
+		const payload ={
+			gitHubAppName: "my-app",
+			webhookSecret: `secret`,
+			appId: "1",
+			gitHubClientId: "Iv1.msdnf2893rwhdbf",
+			gitHubClientSecret: "secret",
+			uuid,
+			gitHubBaseUrl: "http://testserver.com",
+			//privateKey: "new private key not passed in",
+			jiraHost
+		};
+
+		await supertest(app)
+			.put(`/jira/connect/enterprise/app/${uuid}`)
+			.query({
+				jwt
+			})
+			.send(payload)
+			.expect(202);
+
+		existingApp = await GitHubServerApp.findByPk(existingApp.id);
+
+		expect(await existingApp.getDecryptedPrivateKey()).toBe("privatekey");
 	});
 
 	it("should return 202 when correct uuid and installation id are passed, with partial data", async () => {

src/routes/jira/connect/enterprise/app/jira-connect-enterprise-app-put.ts

@@ -17,7 +17,7 @@ export const JiraConnectEnterpriseAppPut = async (
 
 		const updatedAppPayload = { ...req.body };
 		if (!updatedAppPayload.privateKey) {
-			updatedAppPayload.privateKey = verifiedApp.privateKey; // will be encrypted on save
+			updatedAppPayload.privateKey = undefined;
 		}
 
 		await GitHubServerApp.updateGitHubAppByUUID(req.body);

src/routes/jira/connect/enterprise/app/jira-connect-enterprise-app-router.test.ts

@@ -52,8 +52,7 @@ describe("JiraConnectEnterpriseAppRouter", () => {
 				.expect(200);
 			expect(capturedGHEAppConfig).toEqual(expect.objectContaining({
 				uuid: GHE_APP_UUID,
-				appId: 1,
-				privateKey: "private-key"
+				appId: 1
 			}));
 		});
 		it("should throw error for invalid uuid", async () => {

src/sqs/sqs.types.ts

@@ -170,9 +170,6 @@ export type GitHubAppConfig = {
 	gitHubBaseUrl: string, // GITHUB_CLOUD_BASEURL for cloud
 	gitHubApiUrl: string,
 	uuid: string | undefined,
-	//gitHubClientSecret: string,
-	//webhookSecret: string,
-	//privateKey: string
 }
 
 //refer from https://docs.github.com/en/rest/repos/repos#get-a-repository