diff --git a/src/middleware/github-server-app-middleware.test.ts b/src/middleware/github-server-app-middleware.test.ts index 57446c6c1e..91c584aeac 100644 --- a/src/middleware/github-server-app-middleware.test.ts +++ b/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" }); }); }); diff --git a/src/middleware/github-server-app-middleware.ts b/src/middleware/github-server-app-middleware.ts index 9d2c27180d..ebbf8efe93 100644 --- a/src/middleware/github-server-app-middleware.ts +++ b/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 => { @@ -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 }; } diff --git a/src/models/github-server-app.test.ts b/src/models/github-server-app.test.ts index 96b1215f8f..b290e343b9 100644 --- a/src/models/github-server-app.test.ts +++ b/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"); }); }); diff --git a/src/models/github-server-app.ts b/src/models/github-server-app.ts index 1aaf0b8f85..ce09790e9f 100644 --- a/src/models/github-server-app.ts +++ b/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 { + static async updateGitHubAppByUUID(payload: GitHubServerAppUpdatePayload): Promise { const { uuid, appId, diff --git a/src/routes/github/github-oauth-router.ts b/src/routes/github/github-oauth-router.ts index dcc7539b6f..c8bfe72097 100644 --- a/src/routes/github/github-oauth-router.ts +++ b/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(); diff --git a/src/routes/github/github-router.test.ts b/src/routes/github/github-router.test.ts index a86bf8954c..aacddbb8e2 100644 --- a/src/routes/github/github-router.test.ts +++ b/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 }) }) }), diff --git a/src/routes/jira/connect/enterprise/app/jira-connect-enterprise-app-put.test.ts b/src/routes/jira/connect/enterprise/app/jira-connect-enterprise-app-put.test.ts index 33d4ff51a3..f0cbf6879e 100644 --- a/src/routes/jira/connect/enterprise/app/jira-connect-enterprise-app-put.test.ts +++ b/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 () => { diff --git a/src/routes/jira/connect/enterprise/app/jira-connect-enterprise-app-put.ts b/src/routes/jira/connect/enterprise/app/jira-connect-enterprise-app-put.ts index 67836e6208..17d176c32a 100644 --- a/src/routes/jira/connect/enterprise/app/jira-connect-enterprise-app-put.ts +++ b/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); diff --git a/src/routes/jira/connect/enterprise/app/jira-connect-enterprise-app-router.test.ts b/src/routes/jira/connect/enterprise/app/jira-connect-enterprise-app-router.test.ts index c84a82f0d8..9607082c6f 100644 --- a/src/routes/jira/connect/enterprise/app/jira-connect-enterprise-app-router.test.ts +++ b/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 () => { diff --git a/src/sqs/sqs.types.ts b/src/sqs/sqs.types.ts index c377531b23..af6f058974 100644 --- a/src/sqs/sqs.types.ts +++ b/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