From c310d9363355a389f58dc66eeb02d10d19ce4853 Mon Sep 17 00:00:00 2001 From: CosmoX Date: Sat, 29 Aug 2026 22:25:31 -0700 Subject: [PATCH] Fix token persistence when safeStorage is unavailable (#666) * Fix safeStorage token fallback * Keep cached tokens in their existing storage * Preserve token during fallback writes --- app/utilities/accessTokenStorage.js | 50 +++--- tests/utilities/accessTokenStorage.test.js | 168 ++++++++++++++++++--- wiki/configuration.md | 2 +- 3 files changed, 177 insertions(+), 43 deletions(-) diff --git a/app/utilities/accessTokenStorage.js b/app/utilities/accessTokenStorage.js index a0f5c3c6..2f8b248c 100644 --- a/app/utilities/accessTokenStorage.js +++ b/app/utilities/accessTokenStorage.js @@ -14,10 +14,6 @@ function logWarn (logger, message) { if (logger && typeof logger.warn === 'function') logger.warn(message) } -function logInfo (logger, message) { - if (logger && typeof logger.info === 'function') logger.info(message) -} - function normalizeStorageMode (mode) { return STORAGE_MODES.has(mode) ? mode : DEFAULT_STORAGE_MODE } @@ -88,6 +84,29 @@ function createAccessTokenStorage ({ return unavailableReason } + function logFileStorageFallback (unavailableReason) { + logWarn(logger, `[auth] Falling back to local file for cached access token: ${unavailableReason}`) + } + + function readFileTokenFallback (unavailableReason) { + logFileStorageFallback(unavailableReason) + return localStorage.get(LEGACY_TOKEN_KEY) + } + + function writeFileTokenFallback (token, unavailableReason) { + logFileStorageFallback(unavailableReason) + + const legacyWrite = localStorage.set(LEGACY_TOKEN_KEY, token) + if (!legacyWrite.status) return createResult(false, null, legacyWrite.error) + + const encryptedClear = localStorage.set(ENCRYPTED_TOKEN_KEY, null) + return createResult( + Boolean(encryptedClear.status), + token, + encryptedClear.error + ) + } + function clearEncryptedToken () { const encryptedClear = localStorage.set(ENCRYPTED_TOKEN_KEY, null) const legacyClear = localStorage.set(LEGACY_TOKEN_KEY, null) @@ -97,9 +116,6 @@ function createAccessTokenStorage ({ function writeEncryptedToken (token) { if (!hasTokenValue(token)) return clearEncryptedToken() - const unavailableReason = ensureEncryptedStorageAvailable() - if (unavailableReason) return createResult(false, null, new Error(unavailableReason)) - let encryptedToken try { encryptedToken = resolveSafeStorage().encryptString(token) @@ -139,28 +155,15 @@ function createAccessTokenStorage ({ } } - function migrateLegacyToken () { - const legacyToken = localStorage.get(LEGACY_TOKEN_KEY) - if (!legacyToken.status || !hasTokenValue(legacyToken.data)) { - return createResult(false, null, legacyToken.error) - } - - const encryptedWrite = writeEncryptedToken(legacyToken.data) - if (!encryptedWrite.status) return encryptedWrite - - logInfo(logger, '[auth] Migrated cached access token to encrypted storage') - return createResult(true, legacyToken.data) - } - function getEncryptedToken () { const encryptedToken = localStorage.get(ENCRYPTED_TOKEN_KEY) if (encryptedToken.status && encryptedToken.data) { const unavailableReason = ensureEncryptedStorageAvailable() - if (unavailableReason) return createResult(false, null, new Error(unavailableReason)) + if (unavailableReason) return readFileTokenFallback(unavailableReason) return readEncryptedTokenRecord(encryptedToken.data) } - return migrateLegacyToken() + return localStorage.get(LEGACY_TOKEN_KEY) } return { @@ -172,6 +175,9 @@ function createAccessTokenStorage ({ set (token) { if (!hasTokenValue(token)) return clearEncryptedToken() if (getMode() === 'file') return localStorage.set(LEGACY_TOKEN_KEY, token) + + const unavailableReason = ensureEncryptedStorageAvailable() + if (unavailableReason) return writeFileTokenFallback(token, unavailableReason) return writeEncryptedToken(token) } } diff --git a/tests/utilities/accessTokenStorage.test.js b/tests/utilities/accessTokenStorage.test.js index 9e6708ac..660e2574 100644 --- a/tests/utilities/accessTokenStorage.test.js +++ b/tests/utilities/accessTokenStorage.test.js @@ -1,4 +1,7 @@ +import { mkdtempSync, readFileSync, rmSync } from 'node:fs' import { createRequire } from 'node:module' +import { tmpdir } from 'node:os' +import { join } from 'node:path' import { describe, expect, it, vi } from 'vitest' const require = createRequire(import.meta.url) @@ -11,6 +14,7 @@ const { getConfiguredStorageMode, getEffectiveStorageMode } = require('../../app/utilities/accessTokenStorage') +const { createElectronLocalStorage } = require('../../app/utilities/electronLocalStorage') function createConf (values = {}) { return { @@ -51,7 +55,6 @@ function createSafeStorage (options = {}) { function createLogger () { return { - info: vi.fn(), warn: vi.fn() } } @@ -139,50 +142,49 @@ describe('access token storage', () => { expect(getSafeStorage).not.toHaveBeenCalled() }) - it('migrates a legacy plaintext cached token into encrypted storage', () => { + it('reads a legacy plaintext cached token without relocating it', () => { const localStorage = createMemoryStorage({ [LEGACY_TOKEN_KEY]: 'legacy-token' }) - const logger = createLogger() + const safeStorage = createSafeStorage() const accessTokenStorage = createAccessTokenStorage({ conf: createConf({ 'security:cachedAccessTokenStorage': 'encrypted' }), isDev: false, localStorage, - logger, - safeStorage: createSafeStorage() + safeStorage }) expect(accessTokenStorage.get()).toEqual({ status: true, data: 'legacy-token' }) - expect(localStorage.values[LEGACY_TOKEN_KEY]).toBeNull() - expect(localStorage.values[ENCRYPTED_TOKEN_KEY].provider).toBe(SAFE_STORAGE_PROVIDER) - expect(logger.info).toHaveBeenCalledWith('[auth] Migrated cached access token to encrypted storage') + expect(localStorage.values[LEGACY_TOKEN_KEY]).toBe('legacy-token') + expect(localStorage.values[ENCRYPTED_TOKEN_KEY]).toBeUndefined() + expect(safeStorage.encryptString).not.toHaveBeenCalled() }) - it('does not use legacy plaintext token when encrypted storage is unavailable', () => { + it('reads the file token without probing safeStorage when no encrypted token exists', () => { const localStorage = createMemoryStorage({ [LEGACY_TOKEN_KEY]: 'legacy-token' }) - const logger = createLogger() + const safeStorage = createSafeStorage({ available: false }) const accessTokenStorage = createAccessTokenStorage({ conf: createConf({ 'security:cachedAccessTokenStorage': 'encrypted' }), isDev: false, localStorage, - logger, - safeStorage: createSafeStorage({ available: false }) + safeStorage }) - expect(accessTokenStorage.get().status).toBe(false) + expect(accessTokenStorage.get()).toEqual({ + status: true, + data: 'legacy-token' + }) expect(localStorage.values[LEGACY_TOKEN_KEY]).toBe('legacy-token') expect(localStorage.values[ENCRYPTED_TOKEN_KEY]).toBeUndefined() - expect(logger.warn).toHaveBeenCalledWith( - '[auth] Encrypted cached access token storage unavailable: safeStorage encryption unavailable' - ) + expect(safeStorage.isEncryptionAvailable).not.toHaveBeenCalled() }) - it('does not cache tokens on Linux when safeStorage selects basic_text', () => { + it('falls back to the legacy file on Linux when safeStorage selects basic_text', () => { const localStorage = createMemoryStorage() const logger = createLogger() const accessTokenStorage = createAccessTokenStorage({ @@ -194,12 +196,138 @@ describe('access token storage', () => { safeStorage: createSafeStorage({ backend: 'basic_text' }) }) - expect(accessTokenStorage.set('token-1').status).toBe(false) - expect(localStorage.values[LEGACY_TOKEN_KEY]).toBeUndefined() - expect(localStorage.values[ENCRYPTED_TOKEN_KEY]).toBeUndefined() + expect(accessTokenStorage.set('token-1')).toEqual({ + status: true, + data: 'token-1' + }) + expect(localStorage.values[LEGACY_TOKEN_KEY]).toBe('token-1') + expect(localStorage.values[ENCRYPTED_TOKEN_KEY]).toBeNull() expect(logger.warn).toHaveBeenCalledWith( '[auth] Encrypted cached access token storage unavailable: Linux safeStorage selected the insecure basic_text backend' ) + expect(logger.warn).toHaveBeenCalledWith( + '[auth] Falling back to local file for cached access token: Linux safeStorage selected the insecure basic_text backend' + ) + }) + + it('replaces an unreadable encrypted token with the local file fallback', () => { + const localStorage = createMemoryStorage({ + [ENCRYPTED_TOKEN_KEY]: { + version: 1, + provider: SAFE_STORAGE_PROVIDER, + data: Buffer.from('encrypted:stale-token', 'utf8').toString('base64') + } + }) + const accessTokenStorage = createAccessTokenStorage({ + conf: createConf({ 'security:cachedAccessTokenStorage': 'encrypted' }), + isDev: false, + localStorage, + safeStorage: createSafeStorage({ available: false }) + }) + + expect(accessTokenStorage.get().status).toBe(false) + expect(accessTokenStorage.set('current-token')).toEqual({ + status: true, + data: 'current-token' + }) + expect(localStorage.values[ENCRYPTED_TOKEN_KEY]).toBeNull() + expect(accessTokenStorage.get()).toEqual({ + status: true, + data: 'current-token' + }) + }) + + it('preserves the encrypted token when the fallback file write fails', () => { + const encryptedRecord = { + version: 1, + provider: SAFE_STORAGE_PROVIDER, + data: Buffer.from('encrypted:stale-token', 'utf8').toString('base64') + } + const localStorage = createMemoryStorage({ + [ENCRYPTED_TOKEN_KEY]: encryptedRecord + }) + const writeError = new Error('fallback write failed') + localStorage.set.mockImplementation((key, value) => { + if (key === LEGACY_TOKEN_KEY) { + return { status: false, data: null, error: writeError } + } + localStorage.values[key] = value + return { status: true, data: value } + }) + const accessTokenStorage = createAccessTokenStorage({ + conf: createConf({ 'security:cachedAccessTokenStorage': 'encrypted' }), + isDev: false, + localStorage, + safeStorage: createSafeStorage({ available: false }) + }) + + expect(accessTokenStorage.set('current-token')).toEqual({ + status: false, + data: null, + error: writeError + }) + expect(localStorage.values[ENCRYPTED_TOKEN_KEY]).toEqual(encryptedRecord) + expect(localStorage.values[LEGACY_TOKEN_KEY]).toBeUndefined() + expect(localStorage.set).toHaveBeenCalledTimes(1) + expect(localStorage.set).toHaveBeenCalledWith(LEGACY_TOKEN_KEY, 'current-token') + }) + + it('relocates a fallback token only when the token is updated', () => { + const localStorage = createMemoryStorage() + const safeStorage = createSafeStorage({ available: false }) + const accessTokenStorage = createAccessTokenStorage({ + conf: createConf({ 'security:cachedAccessTokenStorage': 'encrypted' }), + isDev: false, + localStorage, + safeStorage + }) + + expect(accessTokenStorage.set('token-1').status).toBe(true) + safeStorage.isEncryptionAvailable.mockReturnValue(true) + + expect(accessTokenStorage.get()).toEqual({ + status: true, + data: 'token-1' + }) + expect(localStorage.values[LEGACY_TOKEN_KEY]).toBe('token-1') + expect(localStorage.values[ENCRYPTED_TOKEN_KEY]).toBeNull() + expect(safeStorage.encryptString).not.toHaveBeenCalled() + + expect(accessTokenStorage.set('token-2')).toEqual({ + status: true, + data: 'token-2' + }) + expect(localStorage.values[LEGACY_TOKEN_KEY]).toBeNull() + expect(localStorage.values[ENCRYPTED_TOKEN_KEY]).toEqual({ + version: 1, + provider: SAFE_STORAGE_PROVIDER, + data: Buffer.from('encrypted:token-2', 'utf8').toString('base64') + }) + }) + + it('persists the fallback token in the original local storage file across restarts', () => { + const userDataPath = mkdtempSync(join(tmpdir(), 'lepton-token-fallback-')) + + try { + const localStorage = createElectronLocalStorage({ + getUserDataPath: () => userDataPath + }) + const storageOptions = { + conf: createConf({ 'security:cachedAccessTokenStorage': 'encrypted' }), + isDev: false, + localStorage, + safeStorage: createSafeStorage({ available: false }) + } + + expect(createAccessTokenStorage(storageOptions).set('token-1').status).toBe(true) + expect(readFileSync(join(userDataPath, 'storage', 'token.json'), 'utf8')).toBe(JSON.stringify('token-1')) + expect(createAccessTokenStorage(storageOptions).get()).toEqual({ + status: true, + data: 'token-1' + }) + } finally { + rmSync(userDataPath, { recursive: true, force: true }) + } }) it('clears encrypted and legacy cached tokens on logout', () => { diff --git a/wiki/configuration.md b/wiki/configuration.md index 89880ee6..5c5a4d01 100644 --- a/wiki/configuration.md +++ b/wiki/configuration.md @@ -91,7 +91,7 @@ The file is not generated automatically. Create it if you want to override the d | | `boringAvatarVariant` | `beam` | Variant used when `avatar.type` is `boring`. | | `userPanel` | `hideProfilePhoto` | `false` | Hide the profile photo in the user panel. | | `logger` | `level` | `info` | Logging level. Use `info` for normal use or `debug` when collecting diagnostic logs. | -| `security` | `cachedAccessTokenStorage` | `auto` | Cached GitHub OAuth token storage. Supported values: `auto`, `encrypted`, `file`. `auto` uses file storage in development builds and Electron safeStorage in packaged builds. | +| `security` | `cachedAccessTokenStorage` | `auto` | Cached GitHub OAuth token storage. Supported values: `auto`, `encrypted`, `file`. `auto` uses file storage in development builds and Electron safeStorage in packaged builds. When safeStorage is unavailable, token updates fall back to the legacy plaintext `/storage/token.json` file. Cached tokens stay in the location where they were written until the token is updated again. | | `proxy` | `enable` | `false` | Route GitHub API requests and Electron sessions through a proxy. | | | `address` | `socks://localhost:1080` | Proxy address. Supports normal proxy URLs, Chromium proxy rule lists, and `pac+https://...` PAC URLs. | | `snippet` | `sorting` | `updated_at` | Snippet order. Supported values: `updated_at`, `created_at`, `description`. |