From 613599e6afe991bb3ec433b464ed0d9657046fe9 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Sat, 1 Feb 2025 07:31:43 -0500 Subject: [PATCH 1/4] feat: encrypt token on disk using safe storage api Signed-off-by: Adam Setch --- src/main/main.ts | 11 +++++++++- src/renderer/__mocks__/electron.js | 4 ++++ src/renderer/context/App.tsx | 32 ++++++++++++++++++++++++++++-- src/renderer/utils/api/request.ts | 12 +++++++++-- src/renderer/utils/auth/utils.ts | 5 +++-- src/renderer/utils/comms.test.ts | 20 +++++++++++++++++++ src/renderer/utils/comms.ts | 14 +++++++++++++ 7 files changed, 91 insertions(+), 7 deletions(-) diff --git a/src/main/main.ts b/src/main/main.ts index 1382128df..b24bac6f4 100644 --- a/src/main/main.ts +++ b/src/main/main.ts @@ -1,4 +1,4 @@ -import { app, globalShortcut, ipcMain as ipc } from 'electron'; +import { app, globalShortcut, ipcMain as ipc, safeStorage } from 'electron'; import log from 'electron-log'; import { menubar } from 'menubar'; @@ -169,6 +169,15 @@ app.whenReady().then(async () => { }); }); +// Safe Storage +ipc.handle(namespacedEvent('safe-storage-encrypt'), (_, settings) => { + return safeStorage.encryptString(settings).toString('base64'); +}); + +ipc.handle(namespacedEvent('safe-storage-decrypt'), (_, settings) => { + return safeStorage.decryptString(Buffer.from(settings, 'base64')); +}); + // Handle gitify:// custom protocol URL events for OAuth 2.0 callback app.on('open-url', (event, url) => { event.preventDefault(); diff --git a/src/renderer/__mocks__/electron.js b/src/renderer/__mocks__/electron.js index 5187ed011..2a8783e58 100644 --- a/src/renderer/__mocks__/electron.js +++ b/src/renderer/__mocks__/electron.js @@ -43,6 +43,10 @@ module.exports = { return Promise.resolve('darwin'); case namespacedEvent('version'): return Promise.resolve('0.0.1'); + case namespacedEvent('safe-storage-encrypt'): + return Promise.resolve('encrypted'); + case namespacedEvent('safe-storage-decrypt'): + return Promise.resolve('decrypted'); default: return Promise.reject(new Error(`Unknown channel: ${channel}`)); } diff --git a/src/renderer/context/App.tsx b/src/renderer/context/App.tsx index 7f9aca897..c2065e6be 100644 --- a/src/renderer/context/App.tsx +++ b/src/renderer/context/App.tsx @@ -28,6 +28,7 @@ import { type Status, type SystemSettingsState, Theme, + type Token, } from '../types'; import type { Notification } from '../typesGitHub'; import { headNotifications } from '../utils/api/client'; @@ -44,6 +45,8 @@ import { removeAccount, } from '../utils/auth/utils'; import { + decryptValue, + encryptValue, setAlternateIdleIcon, setAutoLaunch, setKeyboardShortcut, @@ -281,20 +284,45 @@ export const AppProvider = ({ children }: { children: ReactNode }) => { if (existing.settings) { setKeyboardShortcut(existing.settings.keyboardShortcut); setAlternateIdleIcon(existing.settings.useAlternateIdleIcon); - setSettings({ ...defaultSettings, ...existing.settings }); + setSettings({ + ...defaultSettings, + ...Object.fromEntries( + Object.entries(existing.settings).filter( + ([key]) => key in defaultSettings, + ), + ), + }); webFrame.setZoomLevel( zoomPercentageToLevel(existing.settings.zoomPercentage), ); } if (existing.auth) { - setAuth({ ...defaultAuth, ...existing.auth }); + setAuth({ + ...defaultAuth, + ...Object.fromEntries( + Object.entries(existing.auth).filter(([key]) => key in defaultAuth), + ), + }); // Refresh account data on app start for (const account of existing.auth.accounts) { + /** + * Check if each account has an encrypted token. + * If not encrypt it and save it. + */ + try { + await decryptValue(account.token); + } catch (err) { + const encryptedToken = await encryptValue(account.token); + account.token = encryptedToken as Token; + } + await refreshAccount(account); } } + + // saveState({ auth: existing.auth, settings }); }, []); const fetchNotificationsWithAccounts = useCallback( diff --git a/src/renderer/utils/api/request.ts b/src/renderer/utils/api/request.ts index 39933e4fb..5616f18a3 100644 --- a/src/renderer/utils/api/request.ts +++ b/src/renderer/utils/api/request.ts @@ -4,8 +4,9 @@ import axios, { type Method, } from 'axios'; -import { logError } from '../../../shared/logger'; +import { logError, logWarn } from '../../../shared/logger'; import type { Link, Token } from '../../types'; +import { decryptValue } from '../comms'; import { getNextURLFromLinkHeader } from './utils'; /** @@ -44,8 +45,15 @@ export async function apiRequestAuth( data = {}, fetchAllRecords = false, ): AxiosPromise | null { + let apiToken = token; + try { + apiToken = (await decryptValue(token)) as Token; + } catch (err) { + logWarn('apiRequestAuth', 'Token is not yet encrypted'); + } + axios.defaults.headers.common.Accept = 'application/json'; - axios.defaults.headers.common.Authorization = `token ${token}`; + axios.defaults.headers.common.Authorization = `token ${apiToken}`; axios.defaults.headers.common['Content-Type'] = 'application/json'; axios.defaults.headers.common['Cache-Control'] = shouldRequestWithNoCache(url) ? 'no-cache' diff --git a/src/renderer/utils/auth/utils.ts b/src/renderer/utils/auth/utils.ts index 004974040..8a7651d8c 100644 --- a/src/renderer/utils/auth/utils.ts +++ b/src/renderer/utils/auth/utils.ts @@ -18,7 +18,7 @@ import type { import type { UserDetails } from '../../typesGitHub'; import { getAuthenticatedUser } from '../api/client'; import { apiRequest } from '../api/request'; -import { openExternalLink } from '../comms'; +import { encryptValue, openExternalLink } from '../comms'; import { Constants } from '../constants'; import { getPlatformFromHostname } from '../helpers'; import type { AuthMethod, AuthResponse, AuthTokenResponse } from './types'; @@ -109,12 +109,13 @@ export async function addAccount( hostname: Hostname, ): Promise { const accountList = auth.accounts; + const encryptedToken = await encryptValue(token); let newAccount = { hostname: hostname, method: method, platform: getPlatformFromHostname(hostname), - token: token, + token: encryptedToken, } as Account; newAccount = await refreshAccount(newAccount); diff --git a/src/renderer/utils/comms.test.ts b/src/renderer/utils/comms.test.ts index 4772cd47a..cc090e496 100644 --- a/src/renderer/utils/comms.test.ts +++ b/src/renderer/utils/comms.test.ts @@ -4,6 +4,8 @@ import { namespacedEvent } from '../../shared/events'; import { mockSettings } from '../__mocks__/state-mocks'; import type { Link } from '../types'; import { + decryptValue, + encryptValue, getAppVersion, hideWindow, openExternalLink, @@ -68,6 +70,24 @@ describe('renderer/utils/comms.ts', () => { expect(ipcRenderer.invoke).toHaveBeenCalledWith(namespacedEvent('version')); }); + it('should encrypt a value', async () => { + await encryptValue('value'); + expect(ipcRenderer.invoke).toHaveBeenCalledTimes(1); + expect(ipcRenderer.invoke).toHaveBeenCalledWith( + namespacedEvent('safe-storage-encrypt'), + 'value', + ); + }); + + it('should decrypt a value', async () => { + await decryptValue('value'); + expect(ipcRenderer.invoke).toHaveBeenCalledTimes(1); + expect(ipcRenderer.invoke).toHaveBeenCalledWith( + namespacedEvent('safe-storage-decrypt'), + 'value', + ); + }); + it('should quit the app', () => { quitApp(); expect(ipcRenderer.send).toHaveBeenCalledTimes(1); diff --git a/src/renderer/utils/comms.ts b/src/renderer/utils/comms.ts index 8bd8d16fb..2cba5de3a 100644 --- a/src/renderer/utils/comms.ts +++ b/src/renderer/utils/comms.ts @@ -24,6 +24,20 @@ export async function getAppVersion(): Promise { return await ipcRenderer.invoke(namespacedEvent('version')); } +export async function encryptValue(value: string): Promise { + return await ipcRenderer.invoke( + namespacedEvent('safe-storage-encrypt'), + value, + ); +} + +export async function decryptValue(value: string): Promise { + return await ipcRenderer.invoke( + namespacedEvent('safe-storage-decrypt'), + value, + ); +} + export function quitApp(): void { ipcRenderer.send(namespacedEvent('quit')); } From 1804bb03d5c12af23c52057f96cf725f9c7a6c07 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Sat, 1 Feb 2025 07:32:38 -0500 Subject: [PATCH 2/4] feat: encrypt token on disk using safe storage api Signed-off-by: Adam Setch --- package.json | 2 +- src/main/main.ts | 4 ++- src/renderer/context/App.test.tsx | 2 +- .../api/__snapshots__/client.test.ts.snap | 26 +++++++++---------- .../api/__snapshots__/request.test.ts.snap | 4 +-- src/renderer/utils/auth/utils.test.ts | 8 +++--- 6 files changed, 24 insertions(+), 22 deletions(-) diff --git a/package.json b/package.json index 9a3331765..84d9b39ba 100644 --- a/package.json +++ b/package.json @@ -83,7 +83,7 @@ "protocols": [ { "name": "Gitify", - "schemes": ["gitify"] + "schemes": ["gitify", "gitify-dev"] } ], "mac": { diff --git a/src/main/main.ts b/src/main/main.ts index b24bac6f4..7273b8147 100644 --- a/src/main/main.ts +++ b/src/main/main.ts @@ -38,7 +38,9 @@ const menuBuilder = new MenuBuilder(mb); const contextMenu = menuBuilder.buildMenu(); // Register your app as the handler for a custom protocol -app.setAsDefaultProtocolClient('gitify'); +const protocolName = + process.env.NODE_ENV === 'development' ? 'gitify-dev' : 'gitify'; +app.setAsDefaultProtocolClient(protocolName); if (isMacOS() || isWindows()) { /** diff --git a/src/renderer/context/App.test.tsx b/src/renderer/context/App.test.tsx index 798bf2808..2a5b00176 100644 --- a/src/renderer/context/App.test.tsx +++ b/src/renderer/context/App.test.tsx @@ -237,7 +237,7 @@ describe('renderer/context/App.tsx', () => { expect(apiRequestAuthMock).toHaveBeenCalledWith( 'https://api.github.com/user', 'GET', - '123-456', + 'encrypted', ); }); }); diff --git a/src/renderer/utils/api/__snapshots__/client.test.ts.snap b/src/renderer/utils/api/__snapshots__/client.test.ts.snap index 92d1f9c40..0157f3a70 100644 --- a/src/renderer/utils/api/__snapshots__/client.test.ts.snap +++ b/src/renderer/utils/api/__snapshots__/client.test.ts.snap @@ -3,7 +3,7 @@ exports[`renderer/utils/api/client.ts getAuthenticatedUser should fetch authenticated user - enterprise 1`] = ` { "Accept": "application/json", - "Authorization": "token token-123-456", + "Authorization": "token decrypted", "Cache-Control": "", "Content-Type": "application/json", } @@ -12,7 +12,7 @@ exports[`renderer/utils/api/client.ts getAuthenticatedUser should fetch authenti exports[`renderer/utils/api/client.ts getAuthenticatedUser should fetch authenticated user - github 1`] = ` { "Accept": "application/json", - "Authorization": "token token-123-456", + "Authorization": "token decrypted", "Cache-Control": "", "Content-Type": "application/json", } @@ -21,7 +21,7 @@ exports[`renderer/utils/api/client.ts getAuthenticatedUser should fetch authenti exports[`renderer/utils/api/client.ts headNotifications should fetch notifications head - enterprise 1`] = ` { "Accept": "application/json", - "Authorization": "token token-123-456", + "Authorization": "token decrypted", "Cache-Control": "no-cache", "Content-Type": "application/json", } @@ -30,7 +30,7 @@ exports[`renderer/utils/api/client.ts headNotifications should fetch notificatio exports[`renderer/utils/api/client.ts headNotifications should fetch notifications head - github 1`] = ` { "Accept": "application/json", - "Authorization": "token token-123-456", + "Authorization": "token decrypted", "Cache-Control": "no-cache", "Content-Type": "application/json", } @@ -39,7 +39,7 @@ exports[`renderer/utils/api/client.ts headNotifications should fetch notificatio exports[`renderer/utils/api/client.ts ignoreNotificationThreadSubscription should ignore notification thread subscription - enterprise 1`] = ` { "Accept": "application/json", - "Authorization": "token token-123-456", + "Authorization": "token decrypted", "Cache-Control": "", "Content-Type": "application/json", } @@ -48,7 +48,7 @@ exports[`renderer/utils/api/client.ts ignoreNotificationThreadSubscription shoul exports[`renderer/utils/api/client.ts ignoreNotificationThreadSubscription should ignore notification thread subscription - github 1`] = ` { "Accept": "application/json", - "Authorization": "token token-123-456", + "Authorization": "token decrypted", "Cache-Control": "", "Content-Type": "application/json", } @@ -57,7 +57,7 @@ exports[`renderer/utils/api/client.ts ignoreNotificationThreadSubscription shoul exports[`renderer/utils/api/client.ts listNotificationsForAuthenticatedUser should list notifications for user - github cloud - fetchAllNotifications false 1`] = ` { "Accept": "application/json", - "Authorization": "token token-123-456", + "Authorization": "token decrypted", "Cache-Control": "no-cache", "Content-Type": "application/json", } @@ -66,7 +66,7 @@ exports[`renderer/utils/api/client.ts listNotificationsForAuthenticatedUser shou exports[`renderer/utils/api/client.ts listNotificationsForAuthenticatedUser should list notifications for user - github cloud - fetchAllNotifications true 1`] = ` { "Accept": "application/json", - "Authorization": "token token-123-456", + "Authorization": "token decrypted", "Cache-Control": "no-cache", "Content-Type": "application/json", } @@ -75,7 +75,7 @@ exports[`renderer/utils/api/client.ts listNotificationsForAuthenticatedUser shou exports[`renderer/utils/api/client.ts listNotificationsForAuthenticatedUser should list notifications for user - github enterprise server 1`] = ` { "Accept": "application/json", - "Authorization": "token 1234568790", + "Authorization": "token decrypted", "Cache-Control": "no-cache", "Content-Type": "application/json", } @@ -84,7 +84,7 @@ exports[`renderer/utils/api/client.ts listNotificationsForAuthenticatedUser shou exports[`renderer/utils/api/client.ts markNotificationThreadAsDone should mark notification thread as done - enterprise 1`] = ` { "Accept": "application/json", - "Authorization": "token token-123-456", + "Authorization": "token decrypted", "Cache-Control": "", "Content-Type": "application/json", } @@ -93,7 +93,7 @@ exports[`renderer/utils/api/client.ts markNotificationThreadAsDone should mark n exports[`renderer/utils/api/client.ts markNotificationThreadAsDone should mark notification thread as done - github 1`] = ` { "Accept": "application/json", - "Authorization": "token token-123-456", + "Authorization": "token decrypted", "Cache-Control": "", "Content-Type": "application/json", } @@ -102,7 +102,7 @@ exports[`renderer/utils/api/client.ts markNotificationThreadAsDone should mark n exports[`renderer/utils/api/client.ts markNotificationThreadAsRead should mark notification thread as read - enterprise 1`] = ` { "Accept": "application/json", - "Authorization": "token token-123-456", + "Authorization": "token decrypted", "Cache-Control": "", "Content-Type": "application/json", } @@ -111,7 +111,7 @@ exports[`renderer/utils/api/client.ts markNotificationThreadAsRead should mark n exports[`renderer/utils/api/client.ts markNotificationThreadAsRead should mark notification thread as read - github 1`] = ` { "Accept": "application/json", - "Authorization": "token token-123-456", + "Authorization": "token decrypted", "Cache-Control": "", "Content-Type": "application/json", } diff --git a/src/renderer/utils/api/__snapshots__/request.test.ts.snap b/src/renderer/utils/api/__snapshots__/request.test.ts.snap index 1e7c21f34..4af54e921 100644 --- a/src/renderer/utils/api/__snapshots__/request.test.ts.snap +++ b/src/renderer/utils/api/__snapshots__/request.test.ts.snap @@ -3,7 +3,7 @@ exports[`apiRequestAuth should make an authenticated request with the correct parameters 1`] = ` { "Accept": "application/json", - "Authorization": "token yourAuthToken", + "Authorization": "token decrypted", "Cache-Control": "", "Content-Type": "application/json", } @@ -12,7 +12,7 @@ exports[`apiRequestAuth should make an authenticated request with the correct pa exports[`apiRequestAuth should make an authenticated request with the correct parameters and default data 1`] = ` { "Accept": "application/json", - "Authorization": "token yourAuthToken", + "Authorization": "token decrypted", "Cache-Control": "", "Content-Type": "application/json", } diff --git a/src/renderer/utils/auth/utils.test.ts b/src/renderer/utils/auth/utils.test.ts index 6999537a1..b17ed3c11 100644 --- a/src/renderer/utils/auth/utils.test.ts +++ b/src/renderer/utils/auth/utils.test.ts @@ -191,7 +191,7 @@ describe('renderer/utils/auth/utils.ts', () => { hostname: 'github.com' as Hostname, method: 'Personal Access Token', platform: 'GitHub Cloud', - token: '123-456' as Token, + token: 'encrypted' as Token, user: mockGitifyUser, version: 'latest', }, @@ -211,7 +211,7 @@ describe('renderer/utils/auth/utils.ts', () => { hostname: 'github.com' as Hostname, method: 'OAuth App', platform: 'GitHub Cloud', - token: '123-456' as Token, + token: 'encrypted' as Token, user: mockGitifyUser, version: 'latest', }, @@ -243,7 +243,7 @@ describe('renderer/utils/auth/utils.ts', () => { hostname: 'github.gitify.io' as Hostname, method: 'Personal Access Token', platform: 'GitHub Enterprise Server', - token: '123-456' as Token, + token: 'encrypted' as Token, user: mockGitifyUser, version: '3.0.0', }, @@ -263,7 +263,7 @@ describe('renderer/utils/auth/utils.ts', () => { hostname: 'github.gitify.io' as Hostname, method: 'OAuth App', platform: 'GitHub Enterprise Server', - token: '123-456' as Token, + token: 'encrypted' as Token, user: mockGitifyUser, version: '3.0.0', }, From 679f2ef7e81a32d376ab0f9b33f83429aaa7651d Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Sat, 1 Feb 2025 07:34:27 -0500 Subject: [PATCH 3/4] feat: encrypt token on disk using safe storage api Signed-off-by: Adam Setch --- package.json | 2 +- src/main/main.ts | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index 84d9b39ba..9a3331765 100644 --- a/package.json +++ b/package.json @@ -83,7 +83,7 @@ "protocols": [ { "name": "Gitify", - "schemes": ["gitify", "gitify-dev"] + "schemes": ["gitify"] } ], "mac": { diff --git a/src/main/main.ts b/src/main/main.ts index 7273b8147..b24bac6f4 100644 --- a/src/main/main.ts +++ b/src/main/main.ts @@ -38,9 +38,7 @@ const menuBuilder = new MenuBuilder(mb); const contextMenu = menuBuilder.buildMenu(); // Register your app as the handler for a custom protocol -const protocolName = - process.env.NODE_ENV === 'development' ? 'gitify-dev' : 'gitify'; -app.setAsDefaultProtocolClient(protocolName); +app.setAsDefaultProtocolClient('gitify'); if (isMacOS() || isWindows()) { /** From 220ccfa291e6801941f2854196b1ede78034ee94 Mon Sep 17 00:00:00 2001 From: Adam Setch Date: Sat, 1 Feb 2025 08:16:52 -0500 Subject: [PATCH 4/4] feat: encrypt token on disk using safe storage api Signed-off-by: Adam Setch --- src/renderer/context/App.tsx | 20 +++----------------- src/renderer/utils/api/request.ts | 1 + 2 files changed, 4 insertions(+), 17 deletions(-) diff --git a/src/renderer/context/App.tsx b/src/renderer/context/App.tsx index c2065e6be..68973bcd5 100644 --- a/src/renderer/context/App.tsx +++ b/src/renderer/context/App.tsx @@ -284,31 +284,19 @@ export const AppProvider = ({ children }: { children: ReactNode }) => { if (existing.settings) { setKeyboardShortcut(existing.settings.keyboardShortcut); setAlternateIdleIcon(existing.settings.useAlternateIdleIcon); - setSettings({ - ...defaultSettings, - ...Object.fromEntries( - Object.entries(existing.settings).filter( - ([key]) => key in defaultSettings, - ), - ), - }); + setSettings({ ...defaultSettings, ...existing.settings }); webFrame.setZoomLevel( zoomPercentageToLevel(existing.settings.zoomPercentage), ); } if (existing.auth) { - setAuth({ - ...defaultAuth, - ...Object.fromEntries( - Object.entries(existing.auth).filter(([key]) => key in defaultAuth), - ), - }); + setAuth({ ...defaultAuth, ...existing.auth }); // Refresh account data on app start for (const account of existing.auth.accounts) { /** - * Check if each account has an encrypted token. + * Check if the account is using an encrypted token. * If not encrypt it and save it. */ try { @@ -321,8 +309,6 @@ export const AppProvider = ({ children }: { children: ReactNode }) => { await refreshAccount(account); } } - - // saveState({ auth: existing.auth, settings }); }, []); const fetchNotificationsWithAccounts = useCallback( diff --git a/src/renderer/utils/api/request.ts b/src/renderer/utils/api/request.ts index 5616f18a3..bde669c14 100644 --- a/src/renderer/utils/api/request.ts +++ b/src/renderer/utils/api/request.ts @@ -46,6 +46,7 @@ export async function apiRequestAuth( fetchAllRecords = false, ): AxiosPromise | null { let apiToken = token; + // TODO - Remove this try-catch block in a future release try { apiToken = (await decryptValue(token)) as Token; } catch (err) {