From 29fd59c07341bb8b9216b02ae4bdc9594f3c3d46 Mon Sep 17 00:00:00 2001 From: Brendan Mulholland Date: Wed, 4 Oct 2023 11:00:22 +0200 Subject: [PATCH] fix(auth): Support 2FA via browser window auth --- main.js | 9 ++-- package.json | 4 ++ src/utils/auth.test.ts | 108 ++++++++++++++++++++--------------------- src/utils/auth.ts | 84 +++++++++----------------------- 4 files changed, 85 insertions(+), 120 deletions(-) diff --git a/main.js b/main.js index 56ffc84ce..22de0adc0 100644 --- a/main.js +++ b/main.js @@ -1,11 +1,10 @@ +const { handleAuthCallback } = require('src/utils/auth') const { ipcMain, app, nativeTheme } = require('electron'); const { menubar } = require('menubar'); const { autoUpdater } = require('electron-updater'); const { onFirstRunMaybe } = require('./first-run'); const path = require('path'); -require('@electron/remote/main').initialize() - app.setAppUserModelId('com.electron.gitify'); const iconIdle = path.join( @@ -23,7 +22,6 @@ const browserWindowOpts = { minHeight: 400, resizable: false, webPreferences: { - enableRemoteModule: true, overlayScrollbars: true, nodeIntegration: true, contextIsolation: false, @@ -90,6 +88,11 @@ menubarApp.on('ready', () => { app.setLoginItemSettings(settings); }); + app.on('open-url', function(event, url) { + event.preventDefault(); + handleAuthCallback(url); + }); + menubarApp.window.webContents.on('devtools-opened', () => { menubarApp.window.setSize(800, 600); menubarApp.window.center(); diff --git a/package.json b/package.json index abbd6b7e1..0ebef9950 100644 --- a/package.json +++ b/package.json @@ -59,6 +59,10 @@ "main.js", "first-run.js" ], + "protocols": { + "name": "Gitify", + "schemes": ["gitify"] + }, "mac": { "category": "public.app-category.developer-tools", "icon": "assets/images/app-icon.icns", diff --git a/src/utils/auth.test.ts b/src/utils/auth.test.ts index 95d558881..ba8c37e86 100644 --- a/src/utils/auth.test.ts +++ b/src/utils/auth.test.ts @@ -1,65 +1,63 @@ import { AxiosPromise, AxiosResponse } from 'axios'; -import remote from '@electron/remote'; -const browserWindow = new remote.BrowserWindow(); - import * as auth from './auth'; import * as apiRequests from './api-requests'; import { AuthState } from '../types'; describe('utils/auth.tsx', () => { - describe('authGitHub', () => { - const loadURLMock = jest.spyOn(browserWindow, 'loadURL'); - - beforeEach(() => { - loadURLMock.mockReset(); - }); - - it('should call authGithub - success', async () => { - // Casting to jest.Mock avoids Typescript errors, where the spy is expected to match all the original - // function's typing. I might fix all that if the return type of this was actually used, or if I was - // writing this test for a new feature. Since I'm just upgrading Jest, jest.Mock is a nice escape hatch - ( - jest.spyOn(browserWindow.webContents, 'on') as jest.Mock - ).mockImplementation((event, callback): void => { - if (event === 'will-redirect') { - const event = new Event('will-redirect'); - callback(event, 'http://github.com/?code=123-456'); - } - }); - - const res = await auth.authGitHub(); - - expect(res.authCode).toBe('123-456'); - - expect( - browserWindow.webContents.session.clearStorageData, - ).toHaveBeenCalledTimes(1); - - expect(loadURLMock).toHaveBeenCalledTimes(1); - expect(loadURLMock).toHaveBeenCalledWith( - 'https://github.com/login/oauth/authorize?client_id=FAKE_CLIENT_ID_123&scope=read:user,notifications,repo', - ); - - expect(browserWindow.destroy).toHaveBeenCalledTimes(1); - }); - - it('should call authGithub - failure', async () => { - ( - jest.spyOn(browserWindow.webContents, 'on') as jest.Mock - ).mockImplementation((event, callback): void => { - if (event === 'will-redirect') { - const event = new Event('will-redirect'); - callback(event, 'http://www.github.com/?error=Oops'); - } - }); - - await expect(async () => await auth.authGitHub()).rejects.toEqual( - "Oops! Something went wrong and we couldn't log you in using Github. Please try again.", - ); - expect(loadURLMock).toHaveBeenCalledTimes(1); - }); - }); + // TODO: how to test? + // describe('authGitHub', () => { + // const loadURLMock = jest.spyOn(browserWindow, 'loadURL'); + // + // beforeEach(() => { + // loadURLMock.mockReset(); + // }); + // + // it('should call authGithub - success', async () => { + // // Casting to jest.Mock avoids Typescript errors, where the spy is expected to match all the original + // // function's typing. I might fix all that if the return type of this was actually used, or if I was + // // writing this test for a new feature. Since I'm just upgrading Jest, jest.Mock is a nice escape hatch + // ( + // jest.spyOn(browserWindow.webContents, 'on') as jest.Mock + // ).mockImplementation((event, callback): void => { + // if (event === 'will-redirect') { + // const event = new Event('will-redirect'); + // callback(event, 'http://github.com/?code=123-456'); + // } + // }); + // + // const res = await auth.authGitHub(); + // + // expect(res.authCode).toBe('123-456'); + // + // expect( + // browserWindow.webContents.session.clearStorageData, + // ).toHaveBeenCalledTimes(1); + // + // expect(loadURLMock).toHaveBeenCalledTimes(1); + // expect(loadURLMock).toHaveBeenCalledWith( + // 'https://github.com/login/oauth/authorize?client_id=FAKE_CLIENT_ID_123&scope=read:user,notifications,repo', + // ); + // + // expect(browserWindow.destroy).toHaveBeenCalledTimes(1); + // }); + // + // it('should call authGithub - failure', async () => { + // ( + // jest.spyOn(browserWindow.webContents, 'on') as jest.Mock + // ).mockImplementation((event, callback): void => { + // if (event === 'will-redirect') { + // const event = new Event('will-redirect'); + // callback(event, 'http://www.github.com/?error=Oops'); + // } + // }); + // + // await expect(async () => await auth.authGitHub()).rejects.toEqual( + // "Oops! Something went wrong and we couldn't log you in using Github. Please try again.", + // ); + // expect(loadURLMock).toHaveBeenCalledTimes(1); + // }); + // }); describe('getToken', () => { const authCode = '123-456'; diff --git a/src/utils/auth.ts b/src/utils/auth.ts index 96d83242f..f9d09aea7 100644 --- a/src/utils/auth.ts +++ b/src/utils/auth.ts @@ -1,5 +1,4 @@ -import { BrowserWindow } from '@electron/remote'; - +import { shell } from 'electron'; import { generateGitHubAPIUrl } from './helpers'; import { apiRequest, apiRequestAuth } from '../utils/api-requests'; import { AuthResponse, AuthState, AuthTokenResponse } from '../types'; @@ -9,68 +8,29 @@ import { User } from '../typesGithub'; export const authGitHub = ( authOptions = Constants.DEFAULT_AUTH_OPTIONS, ): Promise => { - return new Promise((resolve, reject) => { - // Build the OAuth consent page URL - const authWindow = new BrowserWindow({ - width: 548, - height: 736, - show: true, - }); - - const githubUrl = `https://${authOptions.hostname}/login/oauth/authorize`; - const authUrl = `${githubUrl}?client_id=${authOptions.clientId}&scope=${Constants.AUTH_SCOPE}`; - - const session = authWindow.webContents.session; - session.clearStorageData(); - - authWindow.loadURL(authUrl); - - const handleCallback = (url: string) => { - const raw_code = /code=([^&]*)/.exec(url) || null; - const authCode = raw_code && raw_code.length > 1 ? raw_code[1] : null; - const error = /\?error=(.+)$/.exec(url); - if (authCode || error) { - // Close the browser if code found or error - authWindow.destroy(); - } - // If there is a code, proceed to get token from github - if (authCode) { - resolve({ authCode, authOptions }); - } else if (error) { - reject( - "Oops! Something went wrong and we couldn't " + - 'log you in using Github. Please try again.', - ); - } - }; - - // If "Done" button is pressed, hide "Loading" - authWindow.on('close', () => { - authWindow.destroy(); - }); + const callbackUrl = encodeURIComponent("gitify://oauth-callback") - authWindow.webContents.on( - 'did-fail-load', - (event, errorCode, errorDescription, validatedURL) => { - if (validatedURL.includes(authOptions.hostname)) { - authWindow.destroy(); - reject( - `Invalid Hostname. Could not load https://${authOptions.hostname}/.`, - ); - } - }, - ); + const githubUrl = `https://${authOptions.hostname}/login/oauth/authorize`; + const authUrl = `${githubUrl}?client_id=${authOptions.clientId}&scope=${Constants.AUTH_SCOPE}&redirect_uri=${callbackUrl}`; - authWindow.webContents.on('will-redirect', (event, url) => { - event.preventDefault(); - handleCallback(url); - }); + shell.openExternal(authUrl); +}; - authWindow.webContents.on('will-navigate', (event, url) => { - event.preventDefault(); - handleCallback(url); - }); - }); +export const handleAuthCallback = (url: string) => { + const raw_code = /code=([^&]*)/.exec(url) || null; + const authCode = raw_code && raw_code.length > 1 ? raw_code[1] : null; + const error = /\?error=(.+)$/.exec(url); + + // If there is a code, proceed to get token from github + if (authCode) { + const { token } = await getToken(authCode); + } else if (error) { + // TODO: Error handling + // reject( + // "Oops! Something went wrong and we couldn't " + + // 'log you in using Github. Please try again.', + // ); + } }; export const getUserData = async ( @@ -90,7 +50,7 @@ export const getUserData = async ( }; }; -export const getToken = async ( +const getToken = async ( authCode: string, authOptions = Constants.DEFAULT_AUTH_OPTIONS, ): Promise => {