Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 13 additions & 8 deletions src/main/handlers/storage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,18 +71,18 @@ describe('main/handlers/storage.ts', () => {
expect(result).toBe(Buffer.from('plain-token').toString('base64'));
});

it('decrypt handler returns the unwrapped plaintext string', async () => {
it('decrypt handler returns the unwrapped token without rotation', async () => {
registerStorageHandlers();
const decrypt = getHandler(EVENTS.SAFE_STORAGE_DECRYPT);

const ciphertext = Buffer.from('plain-token').toString('base64');
const result = await decrypt({}, ciphertext);

expect(typeof result).toBe('string');
expect(result).toBe('plain-token');
expect(result).toEqual({ token: 'plain-token' });
expect(safeStorage.encryptStringAsync).not.toHaveBeenCalled();
});

it('decrypt handler unwraps result when shouldReEncrypt is true', async () => {
it('decrypt handler re-encrypts and returns new ciphertext when shouldReEncrypt is true', async () => {
vi.mocked(safeStorage.decryptStringAsync).mockResolvedValueOnce({
shouldReEncrypt: true,
result: 'rotated-token',
Expand All @@ -92,8 +92,13 @@ describe('main/handlers/storage.ts', () => {

const result = await decrypt({}, 'irrelevant');

expect(typeof result).toBe('string');
expect(result).toBe('rotated-token');
expect(safeStorage.encryptStringAsync).toHaveBeenCalledWith(
'rotated-token',
);
expect(result).toEqual({
token: 'rotated-token',
reEncryptedToken: Buffer.from('rotated-token').toString('base64'),
});
});

it('encrypt → decrypt round-trip preserves the original string', async () => {
Expand All @@ -102,9 +107,9 @@ describe('main/handlers/storage.ts', () => {
const decrypt = getHandler(EVENTS.SAFE_STORAGE_DECRYPT);

const ciphertext = (await encrypt({}, 'round-trip-token')) as string;
const plaintext = await decrypt({}, ciphertext);
const result = (await decrypt({}, ciphertext)) as { token: string };

expect(plaintext).toBe('round-trip-token');
expect(result.token).toBe('round-trip-token');
});

it('decrypt handler rethrows and logs on safeStorage failure', async () => {
Expand Down
18 changes: 15 additions & 3 deletions src/main/handlers/storage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,26 @@ export function registerStorageHandlers(): void {
});

/**
* Decrypt a base64-encoded string using Electron's safeStorage and return the decrypted value.
* Decrypt a base64-encoded string using Electron's safeStorage and return the
* decrypted token. When the OS keychain has rotated keys during decryption,
* also re-encrypt the value with the current key and return the new
* ciphertext under `reEncryptedToken` so callers can persist it.
*/
handleMainEvent(EVENTS.SAFE_STORAGE_DECRYPT, async (_, value: string) => {
try {
const { result } = await safeStorage.decryptStringAsync(
const { result, shouldReEncrypt } = await safeStorage.decryptStringAsync(
Buffer.from(value, 'base64'),
);
return result;

if (!shouldReEncrypt) {
return { token: result };
}

const reEncrypted = await safeStorage.encryptStringAsync(result);
return {
token: result,
reEncryptedToken: reEncrypted.toString('base64'),
};
} catch (err) {
logError(
'main:safe-storage-decrypt',
Expand Down
2 changes: 1 addition & 1 deletion src/renderer/__helpers__/vitest.setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ window.gitify = {
},
twemojiDirectory: vi.fn().mockResolvedValue('/mock/images/assets'),
openExternalLink: vi.fn(),
decryptValue: vi.fn().mockResolvedValue('decrypted'),
decryptValue: vi.fn().mockResolvedValue({ token: 'decrypted' }),
encryptValue: vi.fn().mockResolvedValue('encrypted'),
platform: {
isLinux: vi.fn().mockReturnValue(false),
Expand Down
89 changes: 89 additions & 0 deletions src/renderer/context/App.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import * as authFlows from '../utils/auth/flows';
import * as authUtils from '../utils/auth/utils';
import * as storage from '../utils/core/storage';
import * as notifications from '../utils/notifications/notifications';
import * as comms from '../utils/system/comms';
import * as tray from '../utils/system/tray';
import { type AppContextState, AppProvider } from './App';
import { defaultSettings } from './defaults';
Expand Down Expand Up @@ -312,4 +313,92 @@ describe('renderer/context/App.tsx', () => {
);
});
});

describe('migrateAuthTokens (startup)', () => {
const refreshAccountSpy = vi
.spyOn(authUtils, 'refreshAccount')
.mockImplementation(async (account) => account);
const decryptValueSpy = vi.spyOn(comms, 'decryptValue');
const encryptValueSpy = vi.spyOn(comms, 'encryptValue');
const saveStateSpy = vi
.spyOn(storage, 'saveState')
.mockImplementation(vi.fn());
const loadStateSpy = vi.spyOn(storage, 'loadState');

beforeEach(() => {
vi.useRealTimers();
saveStateSpy.mockClear();
decryptValueSpy.mockReset();
encryptValueSpy.mockReset();
refreshAccountSpy.mockClear();
});

afterEach(() => {
vi.useFakeTimers();
});

it('persists rotated ciphertext when decryptValue reports a re-encryption', async () => {
loadStateSpy.mockReturnValue({
auth: { accounts: [mockGitHubCloudAccount] } as AuthState,
settings: mockSettings,
});
decryptValueSpy.mockResolvedValue({
token: 'plain-token',
reEncryptedToken: 'rotated-cipher',
});

await act(async () => {
renderWithProviders(<AppProvider>{null}</AppProvider>);
});

const persistCall = saveStateSpy.mock.calls.find(
([state]) =>
(state as { auth: AuthState }).auth.accounts[0]?.token ===
'rotated-cipher',
);
expect(persistCall).toBeDefined();
});

it('does not persist when decryptValue returns no rotated ciphertext', async () => {
loadStateSpy.mockReturnValue({
auth: { accounts: [mockGitHubCloudAccount] } as AuthState,
settings: mockSettings,
});
decryptValueSpy.mockResolvedValue({ token: 'plain-token' });

await act(async () => {
renderWithProviders(<AppProvider>{null}</AppProvider>);
});

const tokenChanged = saveStateSpy.mock.calls.some(
([state]) =>
(state as { auth: AuthState }).auth.accounts[0]?.token !==
mockGitHubCloudAccount.token,
);
expect(tokenChanged).toBe(false);
});

it('re-encrypts plaintext token (legacy migration) when decrypt throws', async () => {
loadStateSpy.mockReturnValue({
auth: { accounts: [mockGitHubCloudAccount] } as AuthState,
settings: mockSettings,
});
decryptValueSpy.mockRejectedValue(new Error('not encrypted'));
encryptValueSpy.mockResolvedValue('newly-encrypted');

await act(async () => {
renderWithProviders(<AppProvider>{null}</AppProvider>);
});

expect(encryptValueSpy).toHaveBeenCalledWith(
mockGitHubCloudAccount.token,
);
const persistCall = saveStateSpy.mock.calls.find(
([state]) =>
(state as { auth: AuthState }).auth.accounts[0]?.token ===
'newly-encrypted',
);
expect(persistCall).toBeDefined();
});
});
});
5 changes: 4 additions & 1 deletion src/renderer/context/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,10 @@ export const AppProvider = ({ children }: { children: ReactNode }) => {
const migratedAccounts = await Promise.all(
auth.accounts.map(async (account) => {
try {
await decryptValue(account.token);
const { reEncryptedToken } = await decryptValue(account.token);
if (reEncryptedToken) {
return { ...account, token: reEncryptedToken as Token };
}
return account;
} catch {
const encryptedToken = (await encryptValue(account.token)) as Token;
Expand Down
2 changes: 1 addition & 1 deletion src/renderer/utils/api/octokit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ describe('renderer/utils/api/octokit.ts', () => {
const mockDecryptValue = vi.spyOn(comms, 'decryptValue');

beforeEach(() => {
mockDecryptValue.mockResolvedValue('decrypted-token');
mockDecryptValue.mockResolvedValue({ token: 'decrypted-token' });
clearOctokitClientCache();
});

Expand Down
2 changes: 1 addition & 1 deletion src/renderer/utils/api/octokit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ export async function createOctokitClientUncached(
account: Account,
type: APIClientType,
): Promise<OctokitClient> {
const decryptedToken = await decryptValue(account.token);
const { token: decryptedToken } = await decryptValue(account.token);

const version = await getAppVersion();
const userAgent = `${APPLICATION.NAME}/${version}`;
Expand Down
2 changes: 1 addition & 1 deletion src/renderer/utils/system/comms.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ describe('renderer/utils/comms.ts', () => {

expect(window.gitify.decryptValue).toHaveBeenCalledTimes(1);
expect(window.gitify.decryptValue).toHaveBeenCalledWith('encrypted');
expect(value).toBe('decrypted');
expect(value).toEqual({ token: 'decrypted' });
});
});

Expand Down
11 changes: 9 additions & 2 deletions src/renderer/utils/system/comms.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import type { ISafeStorageDecryptResult } from '../../../shared/events';

import { defaultSettings } from '../../context/defaults';

import { type Link, OpenPreference } from '../../types';
Expand Down Expand Up @@ -49,10 +51,15 @@ export async function encryptValue(value: string): Promise<string> {
/**
* Decrypts a previously encrypted string using the native Electron decryption bridge.
*
* Resolves to the decrypted token. When the OS keychain rotated keys during
* decryption, `reEncryptedToken` is also set so callers can persist the new
* ciphertext alongside or in place of the original.
*
* @param value - The encrypted string to decrypt.
* @returns Promise resolving to the decrypted plaintext string.
*/
export async function decryptValue(value: string): Promise<string> {
export async function decryptValue(
value: string,
): Promise<ISafeStorageDecryptResult> {
return await window.gitify.decryptValue(value);
}

Expand Down
18 changes: 17 additions & 1 deletion src/shared/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,19 @@ export interface IOpenExternal {
activate: boolean;
}

/**
* Result of decrypting a value via Electron's safe storage.
*
* `reEncryptedToken` is set when the OS keychain rotated keys during decrypt
* (signaled by `safeStorage.decryptStringAsync`'s `shouldReEncrypt` flag).
* Callers that persist the original ciphertext should overwrite it with the
* new value so future sessions stay aligned with the active keychain key.
*/
export interface ISafeStorageDecryptResult {
token: string;
reEncryptedToken?: string;
}

/** Shape of a single event contract: a request payload and a response payload. */
type Contract = { request: unknown; response: unknown };

Expand Down Expand Up @@ -83,7 +96,10 @@ export type EventContracts = AssertEventCoverage<{
};
[EVENTS.UPDATE_AUTO_LAUNCH]: { request: IAutoLaunch; response: undefined };
[EVENTS.SAFE_STORAGE_ENCRYPT]: { request: string; response: string };
[EVENTS.SAFE_STORAGE_DECRYPT]: { request: string; response: string };
[EVENTS.SAFE_STORAGE_DECRYPT]: {
request: string;
response: ISafeStorageDecryptResult;
};
[EVENTS.NOTIFICATION_SOUND_PATH]: { request: undefined; response: string };
[EVENTS.OPEN_EXTERNAL]: { request: IOpenExternal; response: undefined };
[EVENTS.RESET_APP]: { request: undefined; response: undefined };
Expand Down
Loading