From 468d4605b0e8680bc87898ed40f91e5e84f91805 Mon Sep 17 00:00:00 2001 From: Gary Xue Date: Tue, 8 Aug 2023 14:42:55 +1000 Subject: [PATCH 1/2] NONE open new tab instead of popup --- spa/src/pages/ConfigSteps/index.tsx | 2 +- spa/src/services/app-manager/index.ts | 2 +- spa/src/services/oauth-manager/index.ts | 2 +- spa/src/utils/index.ts | 22 ++-------------------- 4 files changed, 5 insertions(+), 23 deletions(-) diff --git a/spa/src/pages/ConfigSteps/index.tsx b/spa/src/pages/ConfigSteps/index.tsx index bc86467e8e..6f673b5e37 100644 --- a/spa/src/pages/ConfigSteps/index.tsx +++ b/spa/src/pages/ConfigSteps/index.tsx @@ -182,7 +182,7 @@ const ConfigSteps = () => { }; const logout = () => { - popup("https://github.com/logout", { width: 400, height: 600 }); + popup("https://github.com/logout"); clearGitHubToken(); analyticsClient.sendUIEvent({ actionSubject: "switchGitHubAccount", action: "clicked" }); }; diff --git a/spa/src/services/app-manager/index.ts b/spa/src/services/app-manager/index.ts index bf973a6240..ec62c81d01 100644 --- a/spa/src/services/app-manager/index.ts +++ b/spa/src/services/app-manager/index.ts @@ -49,7 +49,7 @@ async function installNewApp(callbacks: { }; window.addEventListener("message", handler); - const winInstall = popup(app.data.appInstallationUrl, { width: 1024, height: 760 }); + const winInstall = popup(app.data.appInstallationUrl); // Still need below interval for window close // As user might not finish the app install flow, there's no guarantee that above message diff --git a/spa/src/services/oauth-manager/index.ts b/spa/src/services/oauth-manager/index.ts index 083950b71a..93f4966602 100644 --- a/spa/src/services/oauth-manager/index.ts +++ b/spa/src/services/oauth-manager/index.ts @@ -26,7 +26,7 @@ async function authenticateInGitHub(onWinClosed: () => void): Promise { const res = await Api.auth.generateOAuthUrl(); if (res.data.redirectUrl && res.data.state) { window.localStorage.setItem(STATE_KEY, res.data.state); - const win = popup(res.data.redirectUrl, { width: 400, height: 600 }); + const win = popup(res.data.redirectUrl); if (win) { const winCloseCheckHandler = setInterval(() => { if (win.closed) { diff --git a/spa/src/utils/index.ts b/spa/src/utils/index.ts index 7b8e441c2a..c7c848a40e 100644 --- a/spa/src/utils/index.ts +++ b/spa/src/utils/index.ts @@ -6,8 +6,8 @@ export const getJiraJWT = (): Promise => new Promise(resolve => { }); }); -export function popup (url: string, opts: { width: number, height: number }) { - return window.open(url, "_blank", `popup,${popup_params(opts.width, opts.height)}`); +export function popup (url: string) { + return window.open(url, "_blank"); } export function reportError(err: unknown) { @@ -17,21 +17,3 @@ export function reportError(err: unknown) { //do nothing } } - -//https://stackoverflow.com/a/4682246 -function popup_params(width: number, height: number) { - try { - const a = typeof window.screenX != "undefined" ? window.screenX : window.screenLeft; - const i = typeof window.screenY != "undefined" ? window.screenY : window.screenTop; - const g = typeof window.outerWidth!="undefined" ? window.outerWidth : document.documentElement.clientWidth; - const f = typeof window.outerHeight != "undefined" ? window.outerHeight: (document.documentElement.clientHeight - 22); - const h = (a < 0) ? window.screen.width + a : a; - const left = h + ((g - width) / 2); - const top = i + ((f-height) / 2.5); - return "width=" + width + ",height=" + height + ",left=" + left + ",top=" + top + ",scrollbars=1"; - } catch (e) { - reportError(e); - return ""; - } -} - From b46a7b7be2d96e362d75ed2f2cebf0e54efa8721 Mon Sep 17 00:00:00 2001 From: Gary Xue Date: Tue, 8 Aug 2023 14:50:19 +1000 Subject: [PATCH 2/2] remove usage of local storage for checking state in spa exp --- spa/src/services/oauth-manager/index.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/spa/src/services/oauth-manager/index.ts b/spa/src/services/oauth-manager/index.ts index 93f4966602..8813196f67 100644 --- a/spa/src/services/oauth-manager/index.ts +++ b/spa/src/services/oauth-manager/index.ts @@ -2,11 +2,11 @@ import Api from "../../api"; import { AxiosError } from "axios"; import { popup, reportError } from "../../utils"; -const STATE_KEY = "oauth-localStorage-state"; - let username: string | undefined; let email: string | undefined; +let oauthState: string | undefined; + async function checkValidity(): Promise { if (!Api.token.hasGitHubToken()) return false; @@ -25,7 +25,7 @@ async function checkValidity(): Promise { async function authenticateInGitHub(onWinClosed: () => void): Promise { const res = await Api.auth.generateOAuthUrl(); if (res.data.redirectUrl && res.data.state) { - window.localStorage.setItem(STATE_KEY, res.data.state); + oauthState = res.data.state; const win = popup(res.data.redirectUrl); if (win) { const winCloseCheckHandler = setInterval(() => { @@ -39,10 +39,12 @@ async function authenticateInGitHub(onWinClosed: () => void): Promise { } async function finishOAuthFlow(code: string, state: string): Promise { + if (!code && !state) return false; - const prevState = window.localStorage.getItem(STATE_KEY); - window.localStorage.removeItem(STATE_KEY); + const prevState = oauthState; + oauthState = undefined; + if (state !== prevState) return false; try {