Skip to content

Commit 2e515d0

Browse files
setchysetchy
authored
feat: encrypt tokens using safe storage api (#1800)
* feat: encrypt token on disk using safe storage api Signed-off-by: Adam Setch <adam.setch@outlook.com> * feat: encrypt token on disk using safe storage api Signed-off-by: Adam Setch <adam.setch@outlook.com> * feat: encrypt token on disk using safe storage api Signed-off-by: Adam Setch <adam.setch@outlook.com> * feat: encrypt token on disk using safe storage api Signed-off-by: Adam Setch <adam.setch@outlook.com> --------- Signed-off-by: Adam Setch <adam.setch@outlook.com>
1 parent efe938b commit 2e515d0

File tree

11 files changed

+96
-25
lines changed

11 files changed

+96
-25
lines changed

‎src/main/main.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { app, globalShortcut, ipcMain as ipc } from 'electron';
1+
import { app, globalShortcut, ipcMain as ipc, safeStorage } from 'electron';
22
import log from 'electron-log';
33
import { menubar } from 'menubar';
44

@@ -171,6 +171,15 @@ app.whenReady().then(async () => {
171171
});
172172
});
173173

174+
// Safe Storage
175+
ipc.handle(namespacedEvent('safe-storage-encrypt'), (_, settings) => {
176+
return safeStorage.encryptString(settings).toString('base64');
177+
});
178+
179+
ipc.handle(namespacedEvent('safe-storage-decrypt'), (_, settings) => {
180+
return safeStorage.decryptString(Buffer.from(settings, 'base64'));
181+
});
182+
174183
// Handle gitify:// custom protocol URL events for OAuth 2.0 callback
175184
app.on('open-url', (event, url) => {
176185
event.preventDefault();

‎src/renderer/__mocks__/electron.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,10 @@ module.exports = {
4343
return Promise.resolve('darwin');
4444
case namespacedEvent('version'):
4545
return Promise.resolve('0.0.1');
46+
case namespacedEvent('safe-storage-encrypt'):
47+
return Promise.resolve('encrypted');
48+
case namespacedEvent('safe-storage-decrypt'):
49+
return Promise.resolve('decrypted');
4650
default:
4751
return Promise.reject(new Error(`Unknown channel: ${channel}`));
4852
}

‎src/renderer/context/App.test.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,7 @@ describe('renderer/context/App.tsx', () => {
237237
expect(apiRequestAuthMock).toHaveBeenCalledWith(
238238
'https://api.github.com/user',
239239
'GET',
240-
'123-456',
240+
'encrypted',
241241
);
242242
});
243243
});

‎src/renderer/context/App.tsx‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import {
2828
type Status,
2929
type SystemSettingsState,
3030
Theme,
31+
type Token,
3132
} from '../types';
3233
import type { Notification } from '../typesGitHub';
3334
import { headNotifications } from '../utils/api/client';
@@ -44,6 +45,8 @@ import {
4445
removeAccount,
4546
} from '../utils/auth/utils';
4647
import {
48+
decryptValue,
49+
encryptValue,
4750
setAlternateIdleIcon,
4851
setAutoLaunch,
4952
setKeyboardShortcut,
@@ -292,6 +295,17 @@ export const AppProvider = ({ children }: { children: ReactNode }) => {
292295

293296
// Refresh account data on app start
294297
for (const account of existing.auth.accounts) {
298+
/**
299+
* Check if the account is using an encrypted token.
300+
* If not encrypt it and save it.
301+
*/
302+
try {
303+
await decryptValue(account.token);
304+
} catch (err) {
305+
const encryptedToken = await encryptValue(account.token);
306+
account.token = encryptedToken as Token;
307+
}
308+
295309
await refreshAccount(account);
296310
}
297311
}

‎src/renderer/utils/api/__snapshots__/client.test.ts.snap‎

Lines changed: 13 additions & 13 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/renderer/utils/api/__snapshots__/request.test.ts.snap‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/renderer/utils/api/request.ts‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,9 @@ import axios, {
44
type Method,
55
} from 'axios';
66

7-
import { logError } from '../../../shared/logger';
7+
import { logError, logWarn } from '../../../shared/logger';
88
import type { Link, Token } from '../../types';
9+
import { decryptValue } from '../comms';
910
import { getNextURLFromLinkHeader } from './utils';
1011

1112
/**
@@ -44,8 +45,16 @@ export async function apiRequestAuth(
4445
data = {},
4546
fetchAllRecords = false,
4647
): AxiosPromise | null {
48+
let apiToken = token;
49+
// TODO - Remove this try-catch block in a future release
50+
try {
51+
apiToken = (await decryptValue(token)) as Token;
52+
} catch (err) {
53+
logWarn('apiRequestAuth', 'Token is not yet encrypted');
54+
}
55+
4756
axios.defaults.headers.common.Accept = 'application/json';
48-
axios.defaults.headers.common.Authorization = `token ${token}`;
57+
axios.defaults.headers.common.Authorization = `token ${apiToken}`;
4958
axios.defaults.headers.common['Content-Type'] = 'application/json';
5059
axios.defaults.headers.common['Cache-Control'] = shouldRequestWithNoCache(url)
5160
? 'no-cache'

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ describe('renderer/utils/auth/utils.ts', () => {
191191
hostname: 'github.com' as Hostname,
192192
method: 'Personal Access Token',
193193
platform: 'GitHub Cloud',
194-
token: '123-456' as Token,
194+
token: 'encrypted' as Token,
195195
user: mockGitifyUser,
196196
version: 'latest',
197197
},
@@ -211,7 +211,7 @@ describe('renderer/utils/auth/utils.ts', () => {
211211
hostname: 'github.com' as Hostname,
212212
method: 'OAuth App',
213213
platform: 'GitHub Cloud',
214-
token: '123-456' as Token,
214+
token: 'encrypted' as Token,
215215
user: mockGitifyUser,
216216
version: 'latest',
217217
},
@@ -243,7 +243,7 @@ describe('renderer/utils/auth/utils.ts', () => {
243243
hostname: 'github.gitify.io' as Hostname,
244244
method: 'Personal Access Token',
245245
platform: 'GitHub Enterprise Server',
246-
token: '123-456' as Token,
246+
token: 'encrypted' as Token,
247247
user: mockGitifyUser,
248248
version: '3.0.0',
249249
},
@@ -263,7 +263,7 @@ describe('renderer/utils/auth/utils.ts', () => {
263263
hostname: 'github.gitify.io' as Hostname,
264264
method: 'OAuth App',
265265
platform: 'GitHub Enterprise Server',
266-
token: '123-456' as Token,
266+
token: 'encrypted' as Token,
267267
user: mockGitifyUser,
268268
version: '3.0.0',
269269
},

‎src/renderer/utils/auth/utils.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import type {
1818
import type { UserDetails } from '../../typesGitHub';
1919
import { getAuthenticatedUser } from '../api/client';
2020
import { apiRequest } from '../api/request';
21-
import { openExternalLink } from '../comms';
21+
import { encryptValue, openExternalLink } from '../comms';
2222
import { Constants } from '../constants';
2323
import { getPlatformFromHostname } from '../helpers';
2424
import type { AuthMethod, AuthResponse, AuthTokenResponse } from './types';
@@ -109,12 +109,13 @@ export async function addAccount(
109109
hostname: Hostname,
110110
): Promise<AuthState> {
111111
const accountList = auth.accounts;
112+
const encryptedToken = await encryptValue(token);
112113

113114
let newAccount = {
114115
hostname: hostname,
115116
method: method,
116117
platform: getPlatformFromHostname(hostname),
117-
token: token,
118+
token: encryptedToken,
118119
} as Account;
119120

120121
newAccount = await refreshAccount(newAccount);

‎src/renderer/utils/comms.test.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import { namespacedEvent } from '../../shared/events';
44
import { mockSettings } from '../__mocks__/state-mocks';
55
import type { Link } from '../types';
66
import {
7+
decryptValue,
8+
encryptValue,
79
getAppVersion,
810
hideWindow,
911
openExternalLink,
@@ -68,6 +70,24 @@ describe('renderer/utils/comms.ts', () => {
6870
expect(ipcRenderer.invoke).toHaveBeenCalledWith(namespacedEvent('version'));
6971
});
7072

73+
it('should encrypt a value', async () => {
74+
await encryptValue('value');
75+
expect(ipcRenderer.invoke).toHaveBeenCalledTimes(1);
76+
expect(ipcRenderer.invoke).toHaveBeenCalledWith(
77+
namespacedEvent('safe-storage-encrypt'),
78+
'value',
79+
);
80+
});
81+
82+
it('should decrypt a value', async () => {
83+
await decryptValue('value');
84+
expect(ipcRenderer.invoke).toHaveBeenCalledTimes(1);
85+
expect(ipcRenderer.invoke).toHaveBeenCalledWith(
86+
namespacedEvent('safe-storage-decrypt'),
87+
'value',
88+
);
89+
});
90+
7191
it('should quit the app', () => {
7292
quitApp();
7393
expect(ipcRenderer.send).toHaveBeenCalledTimes(1);

0 commit comments

Comments
 (0)