feat(auth): use system browser for GitHub SSO / OAuth authentication

config/webpack.config.renderer.base.ts

@@ -42,8 +42,8 @@ const configuration: webpack.Configuration = {
   plugins: [
     // Development Keys - See README.md
     new webpack.EnvironmentPlugin({
-      OAUTH_CLIENT_ID: '3fef4433a29c6ad8f22c',
-      OAUTH_CLIENT_SECRET: '9670de733096c15322183ff17ed0fc8704050379',
+      OAUTH_CLIENT_ID: 'Ov23liQIkFs5ehQLNzHF',
+      OAUTH_CLIENT_SECRET: '404b80632292e18419dbd2a6ed25976856e95255',
     }),
 
     // Extract CSS into a separate file

src/main/main.ts

@@ -4,6 +4,7 @@ import { menubar } from 'menubar';
 
 import { APPLICATION } from '../shared/constants';
 import { namespacedEvent } from '../shared/events';
+import { logError } from '../shared/logger';
 import { isMacOS, isWindows } from '../shared/platform';
 import { onFirstRunMaybe } from './first-run';
 import { TrayIcons } from './icons';
@@ -37,11 +38,14 @@ const mb = menubar({
 const menuBuilder = new MenuBuilder(mb);
 const contextMenu = menuBuilder.buildMenu();
 
-/**
- * Electron Auto Updater only supports macOS and Windows
- * https://github.com/electron/update-electron-app
- */
+// Register your app as the handler for a custom protocol
+app.setAsDefaultProtocolClient('gitify');
+
 if (isMacOS() || isWindows()) {
+  /**
+   * Electron Auto Updater only supports macOS and Windows
+   * https://github.com/electron/update-electron-app
+   */
   const updater = new Updater(mb, menuBuilder);
   updater.initialize();
 }
@@ -186,3 +190,28 @@ app.whenReady().then(async () => {
     app.setLoginItemSettings(settings);
   });
 });
+
+// Handle gitify:// custom protocol URL events for OAuth 2.0 callback
+app.on('open-url', (event, url) => {
+  event.preventDefault();
+
+  const link = new URL(url);
+
+  const type = link.hostname;
+  const code = link.searchParams.get('code');
+
+  if (code && (type === 'auth' || type === 'oauth')) {
+    mb.window.webContents.send(namespacedEvent('auth-code'), type, code);
+  }
+
+  const error = link.searchParams.get('error');
+  const errorDescription = link.searchParams.get('error_description');
+
+  if (error) {
+    logError(
+      'main:open-url',
+      `Error during OAuth 2.0 callback ${error}`,
+      new Error(errorDescription),
+    );
+  }
+});

src/renderer/utils/auth/types.ts

@@ -22,6 +22,7 @@ export interface LoginPersonalAccessTokenOptions {
 }
 
 export interface AuthResponse {
+  authType: AuthMethod;
   authCode: AuthCode;
   authOptions: LoginOAuthAppOptions;
 }

src/renderer/utils/auth/utils.test.ts

@@ -1,6 +1,6 @@
-import remote from '@electron/remote';
 import axios from 'axios';
 import type { AxiosPromise, AxiosResponse } from 'axios';
+import { ipcRenderer } from 'electron';
 import nock from 'nock';
 import {
   mockAuth,
@@ -12,67 +12,80 @@ import type {
   AuthCode,
   AuthState,
   ClientID,
+  ClientSecret,
   Hostname,
   Token,
 } from '../../types';
+import * as comms from '../../utils/comms';
 import * as apiRequests from '../api/request';
 import type { AuthMethod } from './types';
 import * as auth from './utils';
 import { getNewOAuthAppURL, getNewTokenURL } from './utils';
 
-const browserWindow = new remote.BrowserWindow();
-
 describe('renderer/utils/auth/utils.ts', () => {
   describe('authGitHub', () => {
-    const loadURLMock = jest.spyOn(browserWindow, 'loadURL');
+    const openExternalLinkMock = jest
+      .spyOn(comms, 'openExternalLink')
+      .mockImplementation();
 
     afterEach(() => {
       jest.clearAllMocks();
     });
 
-    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, 'https://github.com/?code=123-456');
+    it('should call authGitHub - auth flow', async () => {
+      const mockIpcRendererOn = (
+        jest.spyOn(ipcRenderer, 'on') as jest.Mock
+      ).mockImplementation((event, callback) => {
+        if (event === 'gitify:auth-code') {
+          callback(null, 'auth', '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(
+      expect(openExternalLinkMock).toHaveBeenCalledTimes(1);
+      expect(openExternalLinkMock).toHaveBeenCalledWith(
         'https://github.com/login/oauth/authorize?client_id=FAKE_CLIENT_ID_123&scope=read%3Auser%2Cnotifications%2Crepo',
       );
 
-      expect(browserWindow.destroy).toHaveBeenCalledTimes(1);
+      expect(mockIpcRendererOn).toHaveBeenCalledTimes(1);
+      expect(mockIpcRendererOn).toHaveBeenCalledWith(
+        'gitify:auth-code',
+        expect.any(Function),
+      );
+
+      expect(res.authType).toBe('GitHub App');
+      expect(res.authCode).toBe('123-456');
     });
 
-    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, 'https://www.github.com/?error=Oops');
+    it('should call authGitHub - oauth flow', async () => {
+      const mockIpcRendererOn = (
+        jest.spyOn(ipcRenderer, 'on') as jest.Mock
+      ).mockImplementation((event, callback) => {
+        if (event === 'gitify:auth-code') {
+          callback(null, 'oauth', '123-456');
         }
       });
 
-      await expect(async () => await auth.authGitHub()).rejects.toEqual(
-        "Oops! Something went wrong and we couldn't log you in using GitHub. Please try again.",
+      const res = await auth.authGitHub({
+        clientId: 'BYO_CLIENT_ID' as ClientID,
+        clientSecret: 'BYO_CLIENT_SECRET' as ClientSecret,
+        hostname: 'my.git.com' as Hostname,
+      });
+
+      expect(openExternalLinkMock).toHaveBeenCalledTimes(1);
+      expect(openExternalLinkMock).toHaveBeenCalledWith(
+        'https://my.git.com/login/oauth/authorize?client_id=BYO_CLIENT_ID&scope=read%3Auser%2Cnotifications%2Crepo',
       );
-      expect(loadURLMock).toHaveBeenCalledTimes(1);
+
+      expect(mockIpcRendererOn).toHaveBeenCalledTimes(1);
+      expect(mockIpcRendererOn).toHaveBeenCalledWith(
+        'gitify:auth-code',
+        expect.any(Function),
+      );
+
+      expect(res.authType).toBe('OAuth App');
+      expect(res.authCode).toBe('123-456');
     });
   });
 

src/renderer/utils/auth/utils.ts

@@ -1,8 +1,9 @@
-import { BrowserWindow } from '@electron/remote';
 import { format } from 'date-fns';
 import semver from 'semver';
 
+import { ipcRenderer } from 'electron';
 import { APPLICATION } from '../../../shared/constants';
+import { namespacedEvent } from '../../../shared/events';
 import { logError, logWarn } from '../../../shared/logger';
 import type {
   Account,
@@ -17,78 +18,34 @@ import type {
 import type { UserDetails } from '../../typesGitHub';
 import { getAuthenticatedUser } from '../api/client';
 import { apiRequest } from '../api/request';
+import { openExternalLink } from '../comms';
 import { Constants } from '../constants';
 import { getPlatformFromHostname } from '../helpers';
 import type { AuthMethod, AuthResponse, AuthTokenResponse } from './types';
 
-// TODO - Refactor our OAuth2 flow to use system browser and local app gitify://callback - see #485 #561 #654
 export function authGitHub(
   authOptions = Constants.DEFAULT_AUTH_OPTIONS,
 ): Promise<AuthResponse> {
-  return new Promise((resolve, reject) => {
-    // Build the OAuth consent page URL
-    const authWindow = new BrowserWindow({
-      width: 548,
-      height: 736,
-      show: true,
-    });
-
+  return new Promise((resolve) => {
     const authUrl = new URL(`https://${authOptions.hostname}`);
     authUrl.pathname = '/login/oauth/authorize';
     authUrl.searchParams.append('client_id', authOptions.clientId);
     authUrl.searchParams.append('scope', Constants.AUTH_SCOPE.toString());
 
-    const session = authWindow.webContents.session;
-    session.clearStorageData();
-
-    authWindow.loadURL(authUrl.toString());
-
-    const handleCallback = (url: Link) => {
-      const raw_code = /code=([^&]*)/.exec(url) || null;
-      const authCode =
-        raw_code && raw_code.length > 1 ? (raw_code[1] as AuthCode) : 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.',
-        );
-      }
+    openExternalLink(authUrl.toString() as Link);
+
+    const handleCallback = (authType: AuthMethod, authCode: AuthCode) => {
+      resolve({ authType, authCode, authOptions });
     };
 
-    // If "Done" button is pressed, hide "Loading"
-    authWindow.on('close', () => {
-      authWindow.destroy();
-    });
-
-    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}/.`,
-          );
-        }
+    ipcRenderer.on(
+      namespacedEvent('auth-code'),
+      (_, authType: 'auth' | 'oauth', authCode: AuthCode) => {
+        const type: AuthMethod =
+          authType === 'auth' ? 'GitHub App' : 'OAuth App';
+        handleCallback(type, authCode);
       },
     );
-
-    authWindow.webContents.on('will-redirect', (event, url) => {
-      event.preventDefault();
-      handleCallback(url as Link);
-    });
-
-    authWindow.webContents.on('will-navigate', (event, url) => {
-      event.preventDefault();
-      handleCallback(url as Link);
-    });
   });
 }
 
@@ -249,11 +206,11 @@ export function getNewOAuthAppURL(hostname: Hostname): Link {
   );
   newOAuthAppURL.searchParams.append(
     'oauth_application[url]',
-    'https://www.gitify.io',
+    'https://gitify.io',
   );
   newOAuthAppURL.searchParams.append(
     'oauth_application[callback_url]',
-    'https://www.gitify.io/callback',
+    'gitify://oauth',
   );
 
   return newOAuthAppURL.toString() as Link;