diff --git a/web-app/package-lock.json b/web-app/package-lock.json index 7cfe3b5..271a7aa 100644 --- a/web-app/package-lock.json +++ b/web-app/package-lock.json @@ -1410,6 +1410,12 @@ "@types/node": "*" } }, + "@types/crypto-js": { + "version": "4.1.1", + "resolved": "https://registry.npmjs.org/@types/crypto-js/-/crypto-js-4.1.1.tgz", + "integrity": "sha512-BG7fQKZ689HIoc5h+6D2Dgq1fABRa0RbBWKBd9SP/MVRVXROflpm5fhwyATX5duFmbStzyzyycPB8qUYKDH3NA==", + "dev": true + }, "@types/express": { "version": "4.17.13", "resolved": "https://registry.npmjs.org/@types/express/-/express-4.17.13.tgz", @@ -2629,6 +2635,11 @@ "which": "^2.0.1" } }, + "crypto-js": { + "version": "4.1.1", + "resolved": "https://registry.npmjs.org/crypto-js/-/crypto-js-4.1.1.tgz", + "integrity": "sha512-o2JlM7ydqd3Qk9CA0L4NL6mTzU2sdx96a+oOfPu8Mkl/PK51vSyoi8/rQ8NknZtk44vq15lmhAj9CIAGwgeWKw==" + }, "crypto-random-string": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/crypto-random-string/-/crypto-random-string-2.0.0.tgz", diff --git a/web-app/package.json b/web-app/package.json index 53ad302..73c53e4 100644 --- a/web-app/package.json +++ b/web-app/package.json @@ -25,11 +25,13 @@ "@octokit/plugin-paginate-rest": "^2.17.0", "@octokit/plugin-rest-endpoint-methods": "^5.13.0", "@probot/octokit-plugin-config": "^1.1.2", + "crypto-js": "^4.1.1", "probot": "^11.0.1", "query-string": "^7.0.1", "yaml": "^1.10.2" }, "devDependencies": { + "@types/crypto-js": "^4.1.1", "@types/jest": "^26.0.19", "@types/node": "^14.14.19", "@typescript-eslint/eslint-plugin": "^5.5.0", diff --git a/web-app/src/eventHandlers/pullRequestEventHandler.ts b/web-app/src/eventHandlers/pullRequestEventHandler.ts index f92e9d9..8bcacb9 100644 --- a/web-app/src/eventHandlers/pullRequestEventHandler.ts +++ b/web-app/src/eventHandlers/pullRequestEventHandler.ts @@ -50,20 +50,23 @@ export class PullRequestEventHandler { const appGitHubService = GitHubService.buildForApp(context.octokit as unknown as OctokitPlus, logger, appConfig); const payload = context.payload; + + const pullRepo = payload.repository; + const pullRequest = payload.pull_request; const pullInfo: PullInfo = { - owner: payload.repository.owner.login, - repo: payload.repository.name, - repoName: payload.repository.name, - pull_number: payload.pull_request.number, + owner: pullRepo.owner.login, + repo: pullRepo.name, + repoName: pullRepo.name, + pull_number: pullRequest.number, }; - const isDefaultBranch = payload.pull_request.base.ref === payload.repository.default_branch; + const isDefaultBranch = pullRequest.base.ref === pullRepo.default_branch; if (!isDefaultBranch) { logger.info("The PR is not targeting the default branch, will not post anything"); return; } - if (payload.pull_request.draft) { + if (pullRequest.draft) { logger.info("This is a draft PR. Exiting."); return; } @@ -100,7 +103,7 @@ export class PullRequestEventHandler { const shouldCreateDiscussionForFile = this.shouldCreateDiscussionForFile(appConfig.appSettings, filepath); if (shouldCreateDiscussionForFile) { try { - const fileref = payload.pull_request.head.ref; + const fileref = pullRequest.head.ref; // Parse the markdown to get the discussion metadata and details const parsedMarkdown = await this.getParsedMarkdownDiscussion(appGitHubService, logger, { filepath: filepath, @@ -108,7 +111,7 @@ export class PullRequestEventHandler { fileref: fileref, }); - const authorLogin = payload.pull_request.user.login; + const authorLogin = pullRequest.user.login; if (!parsedMarkdown.repo && !parsedMarkdown.team) { throw new Error("Markdown is missing a repo or team to post the discussion to"); @@ -127,16 +130,16 @@ export class PullRequestEventHandler { await this._tokenService.deleteRefreshToken(authorLogin); } if (!userRefreshToken || isNonProd) { - const fullAuthUrl = `${appConfig.base_url}${appConfig.auth_url}`; - commentBody += `- @${authorLogin} must [authenticate](${fullAuthUrl}) before merging this PR\n`; + const fullAuthUrl = `${appConfig.base_url}${appConfig.auth_url}?pull_url=${pullRequest.html_url}`; + commentBody += `- @${authorLogin}: you must [authorize the app](${fullAuthUrl}) before merging this pull request so the discussion can be created as you. This is not required every time.\n`; } commentBody += - "- Do not use relative links to files in your repo. Instead, use full URLs and for media drag/drop or paste the file into the markdown. The link generated for media should contain `https://user-images.githubusercontent.com`\n"; + "- Do not use relative links to files in your repo. Instead, use full URLs and for media drag/drop or paste the file into the markdown. The link generated for media should contain `https://user-images.githubusercontent.com`.\n"; if (!mostRecentBotCommentForFile) { await appGitHubService.createPullRequestComment({ ...pullInfo, - commit_id: payload.pull_request.head.sha, + commit_id: pullRequest.head.sha, start_line: 1, end_line: parsedMarkdown.headerEndLine, body: commentBody, @@ -177,7 +180,7 @@ export class PullRequestEventHandler { } else { await appGitHubService.createPullRequestComment({ ...pullInfo, - commit_id: payload.pull_request.head.sha, + commit_id: pullRequest.head.sha, start_line: 1, end_line: 1, body: errorMessage, @@ -198,14 +201,16 @@ export class PullRequestEventHandler { const appGitHubService = GitHubService.buildForApp(context.octokit as unknown as OctokitPlus, logger, appConfig); const payload = context.payload; + const pullRepo = payload.repository; + const pullRequest = payload.pull_request; const pullInfo: PullInfo = { - owner: payload.repository.owner.login, - repo: payload.repository.name, - repoName: payload.repository.name, - pull_number: payload.pull_request.number, + owner: pullRepo.owner.login, + repo: pullRepo.name, + repoName: pullRepo.name, + pull_number: pullRequest.number, }; - const isDefaultBranch = payload.pull_request.base.ref === payload.repository.default_branch; + const isDefaultBranch = pullRequest.base.ref === pullRepo.default_branch; if (!isDefaultBranch) { logger.info("The PR is not targeting the default branch, will not post anything"); @@ -215,7 +220,7 @@ export class PullRequestEventHandler { // Get pull request comments const app = await appGitHubService.getAuthenticatedApp(); const appLogin = `${app.slug}[bot]`; - const optionalPullRequestFooterMarkdown = payload.repository.private ? "" : `from a pull request `; + const optionalPullRequestFooterMarkdown = pullRepo.private ? "" : `from a pull request `; const postFooter = `\n\n
This discussion was created ${optionalPullRequestFooterMarkdown}using ${app.name}.\n`; const pullRequestComments = await appGitHubService.getPullRequestComments({ @@ -233,7 +238,7 @@ export class PullRequestEventHandler { } // Assume the pull request author is the intended post author - const authorLogin = payload.pull_request.user.login; + const authorLogin = pullRequest.user.login; const authorToken = await this._tokenService.refreshUserToken(authorLogin); // For each file to post diff --git a/web-app/src/services/authService.ts b/web-app/src/services/authService.ts index 6bbca0f..11998a1 100644 --- a/web-app/src/services/authService.ts +++ b/web-app/src/services/authService.ts @@ -2,6 +2,7 @@ import { exchangeWebFlowCode, GitHubAppAuthenticationWithRefreshToken } from "@o import { AppConfig } from "../models/appConfig"; import * as queryString from "query-string"; import { DeprecatedLogger } from "probot/lib/types"; +import CryptoJS from "crypto-js"; export class AuthService { private _appConfig: AppConfig; @@ -20,9 +21,17 @@ export class AuthService { const protocol = req.headers["x-forwarded-proto"] || req.protocol; const host = req.headers["x-forwarded-host"] || req.get("host"); + // Encrypt + const state = { + pull_url: req.query.pull_url, + github_client_id: this._appConfig.github_client_id, + }; + const encryptedState = CryptoJS.AES.encrypt(JSON.stringify(state), this._appConfig.github_client_secret).toString(); + const params = queryString.stringify({ client_id: this._appConfig.github_client_id, redirect_uri: `${protocol}://${host}${this._appConfig.github_callback_url}`, + state: encryptedState, }); const url = `https://github.com/login/oauth/authorize?${params}`; diff --git a/web-app/src/services/routerService.ts b/web-app/src/services/routerService.ts index eed9536..66eea96 100644 --- a/web-app/src/services/routerService.ts +++ b/web-app/src/services/routerService.ts @@ -3,6 +3,7 @@ import { Router } from "express"; import { AppConfig } from "../models/appConfig"; import { TokenService } from "./tokenService"; import { AuthService } from "./authService"; +import CryptoJS from "crypto-js"; export class RouterService { private _router: Router; @@ -10,6 +11,8 @@ export class RouterService { private _appConfig: AppConfig; private _tokenService: TokenService; private _authService: AuthService; + static readonly SECONDS_TO_REDIRECT = 6; + static readonly DEFAULT_REDIRECT = "https://github.com/philip-gai/announcement-drafter"; private constructor( logger: DeprecatedLogger, @@ -47,16 +50,63 @@ export class RouterService { } public addOAuthCallbackRoute(): this { - this._router.get(this._appConfig.github_callback_url, async (req, res) => { + this._router.get(this._appConfig.github_callback_url, async (req, res): Promise => { this._logger.info(`Received OAuth callback...`); - const code = (req.query.code as string) || "Bad code"; - this._logger.debug(`Code: ${code}`); + const code = req.query.code as string; + if (!code) { + res.status(400).send("No code"); + return; + } + + const encryptedState = req.query.state as string; + if (!encryptedState) { + res.status(400).send("No state"); + return; + } + + // Redirect to the DEFAULT_REDIRECT if no pull request url is found + let redirectUrl = RouterService.DEFAULT_REDIRECT; + try { + // Decrypt the state and validate it + const bytes = CryptoJS.AES.decrypt(encryptedState, this._appConfig.github_client_secret); + + const state = JSON.parse(bytes.toString(CryptoJS.enc.Utf8)); + if (state.github_client_id !== this._appConfig.github_client_id) { + res.status(400).send("Invalid state"); + return; + } + // Get the pull request url from the state for redirection + if (state.pull_url) { + redirectUrl = state.pull_url; + } + } catch (error) { + res.status(400).send("Unable to parse state"); + return; + } const { token, refreshToken, refreshTokenExpiresAt } = await this._authService.authenticateUser(code); await this._tokenService.upsertRefreshToken(token, refreshToken, refreshTokenExpiresAt); - res.redirect("/"); + // Display authentication success message and redirect after SECONDS_TO_REDIRECT seconds + const redirectLocationText = redirectUrl !== RouterService.DEFAULT_REDIRECT ? "pull request" : "Announcement Drafter repository"; + res.setHeader("Content-Type", "text/html"); + res.status(200).send(` + + + announcement-drafter | authorization + + +

Success! Now Announcement Drafter can create discussions for you 🚀

+

Sending you back to the ${redirectLocationText} in just a few seconds...

+ + + + `); }); return this; }