From a7314655fd85e9972b585671f540e02d980661c8 Mon Sep 17 00:00:00 2001 From: Boris Gvozdev Date: Wed, 1 Jun 2022 10:54:50 +1000 Subject: [PATCH 01/11] ARC-1355: passing JWT as a cookie value to github/configure page --- src/jira/util/jwt.ts | 10 ++- src/middleware/jira-jwt-middleware.ts | 2 +- src/middleware/jirahost-middleware.ts | 110 +++++++++++++++++++++----- static/js/jira-configuration.js | 3 +- views/session.hbs | 7 +- 5 files changed, 108 insertions(+), 24 deletions(-) diff --git a/src/jira/util/jwt.ts b/src/jira/util/jwt.ts index a538ab8e64..818b7707f0 100644 --- a/src/jira/util/jwt.ts +++ b/src/jira/util/jwt.ts @@ -30,8 +30,6 @@ export enum TokenType { export function extractJwtFromRequest(req: Request): string | undefined { const tokenInQuery = req.query?.[JWT_PARAM]; - - // JWT appears in both parameter and body will result query hash being invalid. const tokenInBody = req.body?.[JWT_PARAM]; if (tokenInQuery && tokenInBody) { req.log.info("JWT token can only appear in either query parameter or request body."); @@ -39,7 +37,6 @@ export function extractJwtFromRequest(req: Request): string | undefined { } let token = tokenInQuery || tokenInBody; - // if there was no token in the query-string then fall back to checking the Authorization header const authHeader = req.headers?.[AUTH_HEADER]; if (authHeader?.startsWith("JWT ")) { if (token) { @@ -50,6 +47,13 @@ export function extractJwtFromRequest(req: Request): string | undefined { } } + if (!token) { + token = req.cookies?.[JWT_PARAM]; + if (token) { + req.log.info("JWT token found in cookies (last resort)"); + } + } + // JWT is missing in query and we don't have a valid body. if (!token) { req.log.info("JWT token is missing in the request"); diff --git a/src/middleware/jira-jwt-middleware.ts b/src/middleware/jira-jwt-middleware.ts index 6e71f5807a..54ae2af074 100644 --- a/src/middleware/jira-jwt-middleware.ts +++ b/src/middleware/jira-jwt-middleware.ts @@ -2,7 +2,7 @@ import { Installation } from "models/installation"; import { NextFunction, Request, Response } from "express"; import { sendError, TokenType, verifySymmetricJwtTokenMiddleware } from "../jira/util/jwt"; -const verifyJiraJwtMiddleware = (tokenType: TokenType) => async ( +export const verifyJiraJwtMiddleware = (tokenType: TokenType) => async ( req: Request, res: Response, next: NextFunction diff --git a/src/middleware/jirahost-middleware.ts b/src/middleware/jirahost-middleware.ts index 5b45be3a00..b762aecc5b 100644 --- a/src/middleware/jirahost-middleware.ts +++ b/src/middleware/jirahost-middleware.ts @@ -1,27 +1,101 @@ // setup route middlewares -import { NextFunction, Request, Response } from "express"; -import { postInstallUrl } from "routes/jira/jira-atlassian-connect-get"; +import {NextFunction, Request, Response} from "express"; +import {verifyJiraJwtMiddleware} from "middleware/jira-jwt-middleware"; +import {TokenType} from "~/src/jira/util/jwt"; +// import {postInstallUrl} from "routes/jira/jira-atlassian-connect-get"; + +const extractJiraHost = (req: Request): string | null => { + return (req.query.xdm_e as string) || req.body?.jiraHost || req.cookies.jiraHost; +} + +const isExtractedFromCookie = (req: Request): boolean => { + return !(req.query.xdm_e as string) && !req.body?.jiraHost && req.cookies.jiraHost; +} + +const detectJwtTokenType = (req: Request): TokenType => { + if (req.query.xdm_e) { + return TokenType.normal; + } + return TokenType.context; +} + +// +// Updates res.locals.jiraHost based on values in the request if JWT validation is OK. +// +// Q: Could I provide a fake jiraHost but a valid JWT, would it impersonate me as someone else? +// A: No. Each Jira has its own connect shared secret. When a fake jiraHost is provided, a wrong shared secret is +// fetched and JWT validation will fail. +// +// export const jirahostMiddleware = (req: Request, res: Response, next: NextFunction) => { - if (req.cookies.jiraHost) { - // Save jirahost to secure session - req.session.jiraHost = req.cookies.jiraHost; - // delete jirahost from cookies. + const requestJiraHost = extractJiraHost(req); + const takenFromCookies = isExtractedFromCookie(req); + + if (requestJiraHost) { + + res.locals.jiraHost = requestJiraHost; + + // JWT validation makes sure "res.locals.jiraHost" is legit, not the cookie value. To avoid + // any temptation to use it later, let's remove it straight away! res.clearCookie("jiraHost"); - } - if (req.path == postInstallUrl && req.method == "GET") { - // Only save xdm_e query when on the GET post install url (iframe url) - res.locals.jiraHost = req.query.xdm_e as string; - } else if ((req.path == postInstallUrl && req.method != "GET") || req.path == "/jira/sync") { - // Only save the jiraHost from the body for specific routes that use it - res.locals.jiraHost = req.body?.jiraHost; + const jwtTokenType = detectJwtTokenType(req); + verifyJiraJwtMiddleware(jwtTokenType)(req, res, () => { + + // Cannot hold and rely on cookies because the issued context JWTs are short-lived + // (enough to validate once but not enough for long-running routines) + if (takenFromCookies) { + req.session.jiraHost = res.locals.jiraHost; + } + // Cleaning up outside of "if" to unblock cookies if they were corrupted somehow + // on any other successful validation (e.g. when /jira/configuration is refreshed) + res.clearCookie("jwt"); + + next(); + }); } else { - // Save jiraHost from session for any other URLs - res.locals.jiraHost = req.session.jiraHost; + next(); } - req.addLogFields({ jiraHost: res.locals.jiraHost }); - - next(); + // + // + // let jwtTokenType: (TokenType | null) = TokenType.context; + // let fromCookies = false; + // + // if (req.path == postInstallUrl && req.method == "GET") { + // // Only save xdm_e query when on the GET post install url (iframe url) + // res.locals.jiraHost = req.query.xdm_e as string; + // jwtTokenType = TokenType.normal; + // } else if ((req.path == postInstallUrl && req.method != "GET") || req.path == "/jira/sync") { + // // Only save the jiraHost from the body for specific routes that use it + // res.locals.jiraHost = req.body?.jiraHost; + // } else if (req.cookies.jiraHost) { + // res.locals.jiraHost = req.cookies.jiraHost; + // // JWT validation makes sure "res.locals.jiraHost" is legit, not the cookie value. To avoid + // // any temptation to use it later, let's remove it straight away! + // res.clearCookie("jiraHost"); + // fromCookies = true; + // } else { + // res.locals.jiraHost = req.session.jiraHost; + // jwtTokenType = null; + // } + // + // req.addLogFields({ jiraHost: res.locals.jiraHost }); + // + // if (jwtTokenType) { + // verifyJiraJwtMiddleware(jwtTokenType)(req, res, () => { + // // Cannot hold and rely on cookies because the issued context JWTs are short-lived + // // (enough to validate once but not enough for long-running routines) + // if (fromCookies) { + // req.session.jiraHost = res.locals.jiraHost; + // } + // // Cleaning up outside of "if" to unblock cookies if they were corrupted somehow + // // on any other successful validation (e.g. when /jira/configuration is refreshed) + // res.clearCookie("jwt"); + // next(); + // }); + // } else { + // next(); + // } }; diff --git a/static/js/jira-configuration.js b/static/js/jira-configuration.js index 2d1d3fcdcd..24055fce93 100644 --- a/static/js/jira-configuration.js +++ b/static/js/jira-configuration.js @@ -16,9 +16,10 @@ function openChildWindow(url) { $(".add-organization-link").click(function(event) { event.preventDefault(); - window.AP.context.getToken(function() { + window.AP.context.getToken(function(token) { const child = openChildWindow("/session/github/configuration"); child.window.jiraHost = jiraHost; + child.window.jwt = token; }); }); diff --git a/views/session.hbs b/views/session.hbs index 6af601ae6f..5b26bd496c 100644 --- a/views/session.hbs +++ b/views/session.hbs @@ -46,8 +46,13 @@