diff --git a/package.json b/package.json index 899b8b644..a1f4b84e8 100644 --- a/package.json +++ b/package.json @@ -76,6 +76,7 @@ "@inquirer/input": "^5.0.10", "@inquirer/password": "^5.0.10", "@inquirer/select": "^5.1.2", + "@napi-rs/keyring": "^1.3.0", "@root/walk": "~1.1.0", "@sapphire/duration": "^1.2.0", "@sapphire/result": "^2.8.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index a5834240e..c1a30695a 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -44,6 +44,9 @@ importers: '@inquirer/select': specifier: ^5.1.2 version: 5.1.2(@types/node@24.12.0) + '@napi-rs/keyring': + specifier: ^1.3.0 + version: 1.3.0 '@root/walk': specifier: ~1.1.0 version: 1.1.0 @@ -2477,6 +2480,87 @@ packages: '@module-federation/webpack-bundler-runtime@0.22.0': resolution: {integrity: sha512-aM8gCqXu+/4wBmJtVeMeeMN5guw3chf+2i6HajKtQv7SJfxV/f4IyNQJUeUQu9HfiAZHjqtMV5Lvq/Lvh8LdyA==} + '@napi-rs/keyring-darwin-arm64@1.3.0': + resolution: {integrity: sha512-pl76hJvdYUBn6I24bXiOBMA9nbDapo3I5B+f3OorjDU4dUMSypXeKbOVehJe8fhgTiH24flMyTS3aAIy43xegQ==} + engines: {node: '>= 10'} + cpu: [arm64] + os: [darwin] + + '@napi-rs/keyring-darwin-x64@1.3.0': + resolution: {integrity: sha512-YcJtEV5LA3cvA4z3BurgxH5IhTsW1JfIvcAAcqcecwk06Si9F9NqkxbZVIfDwQ8oRHgaBmT3zZJnLAotCrVahw==} + engines: {node: '>= 10'} + cpu: [x64] + os: [darwin] + + '@napi-rs/keyring-freebsd-x64@1.3.0': + resolution: {integrity: sha512-vlLf31TGhfRAaxLDBhg8b89ss0HHD/lyNmL5F3UjSaz5CUXElsJmKYq9fqA/B+cZKUEUcLHHGhF0I/CqcFdaVw==} + engines: {node: '>= 10'} + cpu: [x64] + os: [freebsd] + + '@napi-rs/keyring-linux-arm-gnueabihf@1.3.0': + resolution: {integrity: sha512-KiWdMMu/Inz/bHHIAGrnF7r54FZDYXuHO6UFF/rhIrshUsxbMG1Rl9lEymNtqqsVo927G0VYcb02FzWQ3iBQRQ==} + engines: {node: '>= 10'} + cpu: [arm] + os: [linux] + + '@napi-rs/keyring-linux-arm64-gnu@1.3.0': + resolution: {integrity: sha512-eyKGpY40lm9Jvs1aD294XRH4y7+TlJM0YVAryZeXA6TX0mb4gMkxVXwSQv7MCwgah7raeUd0dKUb4BPAYIgcMg==} + engines: {node: '>= 10'} + cpu: [arm64] + os: [linux] + libc: [glibc] + + '@napi-rs/keyring-linux-arm64-musl@1.3.0': + resolution: {integrity: sha512-iIK6JWHXAJqDrEyLY3TmswwloVyt2vj+04TZnew+uSJ9gnDO8EwRbp3/iw3LpWaXiDO7VomGO6y8I0Id8uBZSw==} + engines: {node: '>= 10'} + cpu: [arm64] + os: [linux] + libc: [musl] + + '@napi-rs/keyring-linux-riscv64-gnu@1.3.0': + resolution: {integrity: sha512-/PGqrwn6EwgtK6vccASSXJRfOSP4vN1F4ASsIQ+7MdrK6hNvAJ1FZPrIuD5gGGdxezo3F++To2Wq7DbuGIeuNQ==} + engines: {node: '>= 10'} + cpu: [riscv64] + os: [linux] + libc: [glibc] + + '@napi-rs/keyring-linux-x64-gnu@1.3.0': + resolution: {integrity: sha512-2PDK1WKWTu9lBGq9VvNEkSlQD3O7YwVpmnyN2M3cy4v7NJ/8gDMd9GXv3G+FVXN13uhp4gnnPBS+ScefmEeD2A==} + engines: {node: '>= 10'} + cpu: [x64] + os: [linux] + libc: [glibc] + + '@napi-rs/keyring-linux-x64-musl@1.3.0': + resolution: {integrity: sha512-oJ2HkX8YUo46QBkn0pG+HuIKQNqr523q6vBobCn+P95s4C4K6/kLBqHY/1bg5J4ap31DzsznhnFKcfBNBsjCnw==} + engines: {node: '>= 10'} + cpu: [x64] + os: [linux] + libc: [musl] + + '@napi-rs/keyring-win32-arm64-msvc@1.3.0': + resolution: {integrity: sha512-tOd3c/uAaeoE4ycVlmAdSvygz0Zt3zdca6Y7gokBeIbaRDWpjDIUOpU3MvML59XAaqyuKGsVVu0F/DZb1lHPmw==} + engines: {node: '>= 10'} + cpu: [arm64] + os: [win32] + + '@napi-rs/keyring-win32-ia32-msvc@1.3.0': + resolution: {integrity: sha512-sPSqeAFZMGqP1R++M2JTza7GQJJ/TpCo6JU6Vcd4jnebvOaEDs9b7eipakU1PJdSvhpC2yXMCNRk9gXfrhuwHQ==} + engines: {node: '>= 10'} + cpu: [ia32] + os: [win32] + + '@napi-rs/keyring-win32-x64-msvc@1.3.0': + resolution: {integrity: sha512-4DnCWXwDc0HRKwyRlG5y0VhKZW2tNRQfKKfyj6IX/KWfDNyq9hn4n+GL1auyDcOO/v8PwnhmYo2+rOOqCkvvOg==} + engines: {node: '>= 10'} + cpu: [x64] + os: [win32] + + '@napi-rs/keyring@1.3.0': + resolution: {integrity: sha512-WrOw/bcXm0f9qHkumlT1QlArXSTWqaY9sunsDpOk+yCCorCKMxvWT/a3xko4EYHVdeZoh00yI2TydXn6eyICDA==} + engines: {node: '>= 10'} + '@napi-rs/wasm-runtime@1.0.7': resolution: {integrity: sha512-SeDnOO0Tk7Okiq6DbXmmBODgOAb9dp9gjlphokTUxmt8U3liIP1ZsozBahH69j/RJv+Rfs6IwUKHTgQYJ/HBAw==} @@ -12213,6 +12297,57 @@ snapshots: '@module-federation/runtime': 0.22.0 '@module-federation/sdk': 0.22.0 + '@napi-rs/keyring-darwin-arm64@1.3.0': + optional: true + + '@napi-rs/keyring-darwin-x64@1.3.0': + optional: true + + '@napi-rs/keyring-freebsd-x64@1.3.0': + optional: true + + '@napi-rs/keyring-linux-arm-gnueabihf@1.3.0': + optional: true + + '@napi-rs/keyring-linux-arm64-gnu@1.3.0': + optional: true + + '@napi-rs/keyring-linux-arm64-musl@1.3.0': + optional: true + + '@napi-rs/keyring-linux-riscv64-gnu@1.3.0': + optional: true + + '@napi-rs/keyring-linux-x64-gnu@1.3.0': + optional: true + + '@napi-rs/keyring-linux-x64-musl@1.3.0': + optional: true + + '@napi-rs/keyring-win32-arm64-msvc@1.3.0': + optional: true + + '@napi-rs/keyring-win32-ia32-msvc@1.3.0': + optional: true + + '@napi-rs/keyring-win32-x64-msvc@1.3.0': + optional: true + + '@napi-rs/keyring@1.3.0': + optionalDependencies: + '@napi-rs/keyring-darwin-arm64': 1.3.0 + '@napi-rs/keyring-darwin-x64': 1.3.0 + '@napi-rs/keyring-freebsd-x64': 1.3.0 + '@napi-rs/keyring-linux-arm-gnueabihf': 1.3.0 + '@napi-rs/keyring-linux-arm64-gnu': 1.3.0 + '@napi-rs/keyring-linux-arm64-musl': 1.3.0 + '@napi-rs/keyring-linux-riscv64-gnu': 1.3.0 + '@napi-rs/keyring-linux-x64-gnu': 1.3.0 + '@napi-rs/keyring-linux-x64-musl': 1.3.0 + '@napi-rs/keyring-win32-arm64-msvc': 1.3.0 + '@napi-rs/keyring-win32-ia32-msvc': 1.3.0 + '@napi-rs/keyring-win32-x64-msvc': 1.3.0 + '@napi-rs/wasm-runtime@1.0.7': dependencies: '@emnapi/core': 1.10.0 diff --git a/src/commands/actors/search.ts b/src/commands/actors/search.ts index d1a576e18..8f0bd94ed 100644 --- a/src/commands/actors/search.ts +++ b/src/commands/actors/search.ts @@ -96,7 +96,7 @@ export class ActorsSearchCommand extends ApifyCommand { if (isUserLogged) { await updateUserId(userInfo.id!); + const backend = await getBackend(); + let tokenLocation: string; + if (backend === 'keyring') { + tokenLocation = 'your OS keyring'; + } else if (process.env.APIFY_DISABLE_KEYRING === '1') { + tokenLocation = `${AUTH_FILE_PATH()} (OS keyring disabled via APIFY_DISABLE_KEYRING)`; + } else { + tokenLocation = `${AUTH_FILE_PATH()} (OS keyring unavailable; set APIFY_DISABLE_KEYRING=1 to silence)`; + } success({ - message: `You are logged in to Apify as ${userInfo.username || userInfo.id}. ${chalk.gray(`Your token is stored at ${AUTH_FILE_PATH()}.`)}`, + message: `You are logged in to Apify as ${userInfo.username || userInfo.id}. ${chalk.gray(`Your token is stored in ${tokenLocation}.`)}`, }); } else { error({ diff --git a/src/commands/auth/logout.ts b/src/commands/auth/logout.ts index a324e9878..19029efbd 100644 --- a/src/commands/auth/logout.ts +++ b/src/commands/auth/logout.ts @@ -1,5 +1,6 @@ import { ApifyCommand } from '../../lib/command-framework/apify-command.js'; import { AUTH_FILE_PATH } from '../../lib/consts.js'; +import { clearSecrets } from '../../lib/credentials.js'; import { rimrafPromised } from '../../lib/files.js'; import { updateUserId } from '../../lib/hooks/telemetry/useTelemetryState.js'; import { success } from '../../lib/outputs.js'; @@ -24,6 +25,7 @@ export class AuthLogoutCommand extends ApifyCommand { static override docsUrl = 'https://docs.apify.com/cli/docs/reference#apify-logout'; async run() { + await clearSecrets(); await rimrafPromised(AUTH_FILE_PATH()); await updateUserId(null); diff --git a/src/lib/actor.ts b/src/lib/actor.ts index c896fd638..f0823c2c7 100644 --- a/src/lib/actor.ts +++ b/src/lib/actor.ts @@ -57,7 +57,7 @@ export const getApifyStorageClient = async ( const apifyToken = await getApifyTokenFromEnvOrAuthFile(); return new ApifyClient({ - ...getApifyClientOptions(apifyToken), + ...(await getApifyClientOptions(apifyToken)), ...options, }); }; diff --git a/src/lib/credentials.ts b/src/lib/credentials.ts new file mode 100644 index 000000000..df95be6f6 --- /dev/null +++ b/src/lib/credentials.ts @@ -0,0 +1,250 @@ +import { existsSync, readFileSync, writeFileSync } from 'node:fs'; +import process from 'node:process'; + +import { AUTH_FILE_PATH } from './consts.js'; +import { ensureApifyDirectory } from './utils.js'; +import { cliDebugPrint } from './utils/cliDebugPrint.js'; + +const KEYRING_SERVICE = 'com.apify.cli'; +const TOKEN_ACCOUNT = 'token'; +const PROXY_PASSWORD_ACCOUNT = 'proxy-password'; +const PROBE_ACCOUNT = '__probe__'; + +export type CredentialsBackend = 'keyring' | 'file'; + +interface KeyringEntry { + getPassword(): string | null; + setPassword(password: string): void; + deletePassword(): boolean; +} + +interface KeyringModule { + Entry: new (service: string, account: string) => KeyringEntry; +} + +interface StoredAuthFile { + token?: string; + proxy?: { password: string }; + secretsBackend?: CredentialsBackend; + [k: string]: unknown; +} + +let cachedKeyringModule: KeyringModule | null | undefined; +let backendPromise: Promise | undefined; +let migrationPromise: Promise | undefined; + +/** Test-only: clear cached module/backend/migration so each test starts fresh. */ +export function __resetCredentialsForTests() { + cachedKeyringModule = undefined; + backendPromise = undefined; + migrationPromise = undefined; +} + +async function loadKeyringModule(): Promise { + if (cachedKeyringModule !== undefined) return cachedKeyringModule; + try { + // Indirect specifier so tsc doesn't try to resolve the module at compile time. + const specifier = '@napi-rs/keyring'; + cachedKeyringModule = (await import(specifier)) as KeyringModule; + } catch (err) { + cliDebugPrint('credentials', 'failed to load @napi-rs/keyring', err); + cachedKeyringModule = null; + } + return cachedKeyringModule; +} + +/** + * Full write/read-back/delete round-trip on a unique random account. + * A read-only probe would pass on Secret Service backends that reject writes + * (e.g. read-only sessions), giving us a false positive at login time. + */ +function probeKeyring(mod: KeyringModule): boolean { + const probeAccount = `${PROBE_ACCOUNT}_${Date.now()}_${Math.random().toString(36).slice(2)}`; + const probeValue = `probe-${Date.now()}`; + try { + const entry = new mod.Entry(KEYRING_SERVICE, probeAccount); + entry.setPassword(probeValue); + const readBack = entry.getPassword(); + entry.deletePassword(); + return readBack === probeValue; + } catch (err) { + cliDebugPrint('credentials', 'keyring probe failed', err); + return false; + } +} + +/** + * Picks a backend the first time it's called and caches the result for the rest of the process. + * Single-flight via a promise so concurrent callers share the same probe. + * Order: APIFY_DISABLE_KEYRING env override -> persisted marker in auth.json -> module load -> probe. + */ +export async function getBackend(): Promise { + if (backendPromise) return backendPromise; + backendPromise = (async (): Promise => { + if (process.env.APIFY_DISABLE_KEYRING === '1') return 'file'; + + const marker = readAuthFile().secretsBackend; + if (marker === 'file') return 'file'; + if (marker === 'keyring') { + const mod = await loadKeyringModule(); + return mod ? 'keyring' : 'file'; + } + + const mod = await loadKeyringModule(); + if (!mod) return 'file'; + return probeKeyring(mod) ? 'keyring' : 'file'; + })(); + return backendPromise; +} + +function readAuthFile(): StoredAuthFile { + if (!existsSync(AUTH_FILE_PATH())) return {}; + try { + const raw = readFileSync(AUTH_FILE_PATH(), 'utf-8'); + return JSON.parse(raw) as StoredAuthFile; + } catch { + return {}; + } +} + +function writeAuthFile(data: StoredAuthFile) { + ensureApifyDirectory(AUTH_FILE_PATH()); + writeFileSync(AUTH_FILE_PATH(), JSON.stringify(data, null, '\t'), { mode: 0o600 }); +} + +async function getKeyringEntry(account: string): Promise { + const mod = await loadKeyringModule(); + if (!mod) return null; + return new mod.Entry(KEYRING_SERVICE, account); +} + +async function readKeyring(account: string): Promise { + try { + const entry = await getKeyringEntry(account); + if (!entry) return undefined; + return entry.getPassword() ?? undefined; + } catch (err) { + cliDebugPrint('credentials', `failed to read ${account} from keyring`, err); + return undefined; + } +} + +async function writeKeyring(account: string, value: string): Promise { + const entry = await getKeyringEntry(account); + if (!entry) { + throw new Error('OS keyring is not available.'); + } + entry.setPassword(value); +} + +async function deleteKeyring(account: string): Promise { + try { + const entry = await getKeyringEntry(account); + if (!entry) return; + entry.deletePassword(); + } catch (err) { + cliDebugPrint('credentials', `failed to delete ${account} from keyring`, err); + } +} + +export async function getToken(): Promise { + const backend = await getBackend(); + if (backend === 'keyring') return readKeyring(TOKEN_ACCOUNT); + return readAuthFile().token; +} + +export async function getProxyPassword(): Promise { + const backend = await getBackend(); + if (backend === 'keyring') return readKeyring(PROXY_PASSWORD_ACCOUNT); + return readAuthFile().proxy?.password; +} + +/** + * Persist token. When `skipIfUnchanged` is true and the stored value already matches, + * the write is skipped. This avoids macOS Keychain prompts on every command. + */ +export async function setToken(token: string, opts: { skipIfUnchanged?: boolean } = {}): Promise { + const backend = await getBackend(); + if (opts.skipIfUnchanged) { + const existing = backend === 'keyring' ? await readKeyring(TOKEN_ACCOUNT) : readAuthFile().token; + if (existing === token) return; + } + + if (backend === 'keyring') { + await writeKeyring(TOKEN_ACCOUNT, token); + return; + } + + const data = readAuthFile(); + data.token = token; + data.secretsBackend = 'file'; + writeAuthFile(data); +} + +export async function setProxyPassword(password: string, opts: { skipIfUnchanged?: boolean } = {}): Promise { + const backend = await getBackend(); + if (opts.skipIfUnchanged) { + const existing = backend === 'keyring' ? await readKeyring(PROXY_PASSWORD_ACCOUNT) : readAuthFile().proxy?.password; + if (existing === password) return; + } + + if (backend === 'keyring') { + await writeKeyring(PROXY_PASSWORD_ACCOUNT, password); + return; + } + + const data = readAuthFile(); + data.proxy = { password }; + data.secretsBackend = 'file'; + writeAuthFile(data); +} + +/** + * Remove all stored secrets. Always attempts to clear the keyring even when the + * current backend is `file`, so toggling `APIFY_DISABLE_KEYRING=1` between login + * and logout does not orphan entries the user has no in-CLI way to discover. + */ +export async function clearSecrets(): Promise { + await deleteKeyring(TOKEN_ACCOUNT); + await deleteKeyring(PROXY_PASSWORD_ACCOUNT); +} + +/** + * One-shot, idempotent migration of legacy plaintext auth.json to the keyring. + * + * Both the API token and the proxy password are moved into the keyring on the keyring backend. + * + * - `secretsBackend` marker in auth.json makes re-entry a no-op. + * - On `file` backend the marker is written but secrets stay in auth.json. + * - On `keyring` backend the token and proxy password are moved out of auth.json. + * - Wrapped in try/catch so a migration failure never blocks the CLI. + */ +export async function ensureMigrated(): Promise { + if (migrationPromise) return migrationPromise; + migrationPromise = (async () => { + try { + const file = readAuthFile(); + if (file.secretsBackend) return; + if (!file.token) return; + + const backend = await getBackend(); + if (backend === 'file') { + file.secretsBackend = 'file'; + writeAuthFile(file); + return; + } + + await writeKeyring(TOKEN_ACCOUNT, file.token); + delete file.token; + if (file.proxy?.password) { + await writeKeyring(PROXY_PASSWORD_ACCOUNT, file.proxy.password); + delete file.proxy; + } + file.secretsBackend = 'keyring'; + writeAuthFile(file); + } catch (err) { + cliDebugPrint('credentials', 'migration failed', err); + } + })(); + return migrationPromise; +} diff --git a/src/lib/utils.ts b/src/lib/utils.ts index 852d1b242..06f35ea4e 100644 --- a/src/lib/utils.ts +++ b/src/lib/utils.ts @@ -40,11 +40,11 @@ import { AUTH_FILE_PATH, CommandExitCodes, DEFAULT_LOCAL_STORAGE_DIR, - GLOBAL_CONFIGS_FOLDER, LOCAL_CONFIG_PATH, MINIMUM_SUPPORTED_PYTHON_VERSION, SUPPORTED_NODEJS_VERSION, } from './consts.js'; +import { ensureMigrated, getBackend, getProxyPassword, getToken, setProxyPassword, setToken } from './credentials.js'; import { deleteFile, ensureFolderExistsSync, rimrafPromised } from './files.js'; import { inputFileRegExp, TEMP_INPUT_KEY_PREFIX } from './input-key.js'; import type { AuthJSON } from './types.js'; @@ -101,17 +101,29 @@ export const getLocalRequestQueuePath = (storeId?: string) => { }; /** - * Returns object from auth file or empty object. + * Returns object from auth file or empty object. Secrets (token, proxy password) are + * pulled from the keyring when that backend is active; user metadata lives in auth.json. */ export const getLocalUserInfo = async (): Promise => { + await ensureMigrated(); + let result: AuthJSON = {}; try { const raw = await readFile(AUTH_FILE_PATH(), 'utf-8'); result = JSON.parse(raw) as AuthJSON; } catch { - return {}; + // auth.json may not exist yet (fresh keyring-only state); fall through } + const token = await getToken(); + if (token) result.token = token; + + const proxyPassword = await getProxyPassword(); + if (proxyPassword) result.proxy = { password: proxyPassword }; + + const hasSomething = result.username || result.id || result.token; + if (!hasSomething) return {}; + if (!result.username && !result.id) { throw new Error('Corrupted local user info was found. Please run "apify login" to fix it.'); } @@ -132,13 +144,10 @@ export async function getLoggedClientOrThrow() { return loggedClient; } -const getTokenWithAuthFileFallback = (existingToken?: string) => { - if (!existingToken && existsSync(GLOBAL_CONFIGS_FOLDER()) && existsSync(AUTH_FILE_PATH())) { - const raw = readFileSync(AUTH_FILE_PATH(), 'utf-8'); - return JSON.parse(raw).token; - } - - return existingToken; +const resolveToken = async (existingToken?: string): Promise => { + if (existingToken) return existingToken; + await ensureMigrated(); + return getToken(); }; type CJSAxiosHeaders = import('axios', { with: { 'resolution-mode': 'require' } }).AxiosRequestConfig['headers']; @@ -146,8 +155,8 @@ type CJSAxiosHeaders = import('axios', { with: { 'resolution-mode': 'require' } /** * Returns options for ApifyClient */ -export const getApifyClientOptions = (token?: string, apiBaseUrl?: string): ApifyClientOptions => { - token = getTokenWithAuthFileFallback(token); +export const getApifyClientOptions = async (token?: string, apiBaseUrl?: string): Promise => { + token = await resolveToken(token); return { token, @@ -168,13 +177,14 @@ export const getApifyClientOptions = (token?: string, apiBaseUrl?: string): Apif /** * Gets instance of ApifyClient for token or for params from global auth file. - * NOTE: It refreshes global auth file each run - * @param [token] + * + * Refreshes the user metadata in auth.json each run. Secrets (token, proxy.password) only + * get written when their value actually changes — avoids macOS Keychain prompts on every command. */ export async function getLoggedClient(token?: string, apiBaseUrl?: string) { - token = getTokenWithAuthFileFallback(token); + token = await resolveToken(token); - const apifyClient = new ApifyClient(getApifyClientOptions(token, apiBaseUrl)); + const apifyClient = new ApifyClient(await getApifyClientOptions(token, apiBaseUrl)); let userInfo; try { @@ -184,10 +194,30 @@ export async function getLoggedClient(token?: string, apiBaseUrl?: string) { return null; } - // Always refresh Auth file - ensureApifyDirectory(AUTH_FILE_PATH()); + if (apifyClient.token) { + await setToken(apifyClient.token, { skipIfUnchanged: true }); + } + + const proxyPassword = (userInfo as { proxy?: { password?: string } } | undefined)?.proxy?.password; + if (proxyPassword) { + await setProxyPassword(proxyPassword, { skipIfUnchanged: true }); + } - writeFileSync(AUTH_FILE_PATH(), JSON.stringify({ token: apifyClient.token, ...userInfo }, null, '\t')); + ensureApifyDirectory(AUTH_FILE_PATH()); + const existingFile = (() => { + try { + return JSON.parse(readFileSync(AUTH_FILE_PATH(), 'utf-8')) as Record; + } catch { + return {}; + } + })(); + const backend = await getBackend(); + const fileContents: Record = { ...existingFile, ...userInfo, secretsBackend: backend }; + if (backend === 'keyring') { + delete fileContents.token; + delete fileContents.proxy; + } + writeFileSync(AUTH_FILE_PATH(), JSON.stringify(fileContents, null, '\t'), { mode: 0o600 }); return apifyClient; } diff --git a/test/__setup__/config.ts b/test/__setup__/config.ts index b2435f994..0c14fdf3a 100644 --- a/test/__setup__/config.ts +++ b/test/__setup__/config.ts @@ -13,9 +13,9 @@ if (!ENV_TEST_USER_TOKEN) { throw Error('You must configure "TEST_USER_TOKEN" environment variable to run tests!'); } -export const testUserClient = new ApifyClient(getApifyClientOptions(ENV_TEST_USER_TOKEN)); +export const testUserClient = new ApifyClient(await getApifyClientOptions(ENV_TEST_USER_TOKEN)); -export const badUserClient = new ApifyClient(getApifyClientOptions(TEST_USER_BAD_TOKEN)); +export const badUserClient = new ApifyClient(await getApifyClientOptions(TEST_USER_BAD_TOKEN)); export const TEST_USER_TOKEN = ENV_TEST_USER_TOKEN; diff --git a/test/__setup__/hooks/useAuthSetup.ts b/test/__setup__/hooks/useAuthSetup.ts index 841d54d07..78a55b32d 100644 --- a/test/__setup__/hooks/useAuthSetup.ts +++ b/test/__setup__/hooks/useAuthSetup.ts @@ -8,6 +8,7 @@ import { cryptoRandomObjectId } from '@apify/utilities'; import { LoginCommand } from '../../../src/commands/login.js'; import { testRunCommand } from '../../../src/lib/command-framework/apify-command.js'; import { GLOBAL_CONFIGS_FOLDER } from '../../../src/lib/consts.js'; +import { __resetCredentialsForTests } from '../../../src/lib/credentials.js'; import { getLocalUserInfo } from '../../../src/lib/utils.js'; export interface UseAuthSetupOptions { @@ -39,6 +40,10 @@ export function useAuthSetup({ cleanup = true, perTest = true }: UseAuthSetupOpt before(() => { vitest.stubEnv(envVariable, envValue()); + // Tests pin to the file backend so they don't touch the real OS keyring. + // Unit tests for credentials.ts override this explicitly. + vitest.stubEnv('APIFY_DISABLE_KEYRING', '1'); + __resetCredentialsForTests(); }); after(async () => { @@ -46,6 +51,7 @@ export function useAuthSetup({ cleanup = true, perTest = true }: UseAuthSetupOpt await rm(GLOBAL_CONFIGS_FOLDER(), { recursive: true, force: true }); } + __resetCredentialsForTests(); vitest.unstubAllEnvs(); }); } diff --git a/test/api/commands/log_in_out.test.ts b/test/api/commands/log_in_out.test.ts index 6987d4cfc..5ae48d0ed 100644 --- a/test/api/commands/log_in_out.test.ts +++ b/test/api/commands/log_in_out.test.ts @@ -51,6 +51,7 @@ describe('[api] apify login and logout', () => { currentBillingPeriod: _4, plan: _5, createdAt: _6, + secretsBackend: _7, ...userInfoFromConfigWithoutFloatFields } = userInfoFromConfig; @@ -101,6 +102,7 @@ describe('[api] apify login and logout', () => { currentBillingPeriod: _4, plan: _5, createdAt: _6, + secretsBackend: _7, ...userInfoFromConfigWithoutFloatFields } = userInfoFromConfig; diff --git a/test/e2e/__helpers__/run-cli.ts b/test/e2e/__helpers__/run-cli.ts index 8d1d05dca..fcaff98e6 100644 --- a/test/e2e/__helpers__/run-cli.ts +++ b/test/e2e/__helpers__/run-cli.ts @@ -45,6 +45,8 @@ export async function runCli( env: { APIFY_CLI_DISABLE_TELEMETRY: '1', APIFY_CLI_SKIP_UPDATE_CHECK: '1', + // Pin the file backend so e2e subprocesses don't share the host's OS keyring across tests. + APIFY_DISABLE_KEYRING: '1', ...options.env, }, }); diff --git a/test/e2e/commands/builds/lifecycle.test.ts b/test/e2e/commands/builds/lifecycle.test.ts index b54f65bdc..4697ec1ef 100644 --- a/test/e2e/commands/builds/lifecycle.test.ts +++ b/test/e2e/commands/builds/lifecycle.test.ts @@ -25,7 +25,7 @@ describe('[e2e][api] builds namespace', () => { throw new Error(`Failed to login:\n${loginResult.stderr}`); } - client = new ApifyClient(getApifyClientOptions(token)); + client = new ApifyClient(await getApifyClientOptions(token)); actor = await createTestActor('e2e-builds'); diff --git a/test/e2e/commands/datasets/lifecycle.test.ts b/test/e2e/commands/datasets/lifecycle.test.ts index 838a67bc0..b0b2deecf 100644 --- a/test/e2e/commands/datasets/lifecycle.test.ts +++ b/test/e2e/commands/datasets/lifecycle.test.ts @@ -24,7 +24,7 @@ describe('[e2e][api] datasets namespace', () => { throw new Error(`Failed to login:\n${loginResult.stderr}`); } - client = new ApifyClient(getApifyClientOptions(token)); + client = new ApifyClient(await getApifyClientOptions(token)); }); afterAll(async () => { diff --git a/test/e2e/commands/key-value-stores/lifecycle.test.ts b/test/e2e/commands/key-value-stores/lifecycle.test.ts index 191e76062..f01399a4b 100644 --- a/test/e2e/commands/key-value-stores/lifecycle.test.ts +++ b/test/e2e/commands/key-value-stores/lifecycle.test.ts @@ -24,7 +24,7 @@ describe('[e2e][api] key-value-stores namespace', () => { throw new Error(`Failed to login:\n${loginResult.stderr}`); } - client = new ApifyClient(getApifyClientOptions(token)); + client = new ApifyClient(await getApifyClientOptions(token)); }); afterAll(async () => { diff --git a/test/e2e/commands/runs/lifecycle.test.ts b/test/e2e/commands/runs/lifecycle.test.ts index 9c40111aa..da294a83f 100644 --- a/test/e2e/commands/runs/lifecycle.test.ts +++ b/test/e2e/commands/runs/lifecycle.test.ts @@ -30,7 +30,7 @@ describe('[e2e][api] runs lifecycle', () => { throw new Error(`Failed to login:\n${loginResult.stderr}`); } - client = new ApifyClient(getApifyClientOptions(token)); + client = new ApifyClient(await getApifyClientOptions(token)); const me = await client.user('me').get(); actor = await createTestActor('e2e-runs'); diff --git a/test/local/lib/credentials.test.ts b/test/local/lib/credentials.test.ts new file mode 100644 index 000000000..6a9d7c0d1 --- /dev/null +++ b/test/local/lib/credentials.test.ts @@ -0,0 +1,204 @@ +import { existsSync, mkdirSync, readFileSync, writeFileSync } from 'node:fs'; +import { rm } from 'node:fs/promises'; + +import { cryptoRandomObjectId } from '@apify/utilities'; + +import { AUTH_FILE_PATH, GLOBAL_CONFIGS_FOLDER } from '../../../src/lib/consts.js'; +import { + __resetCredentialsForTests, + clearSecrets, + ensureMigrated, + getBackend, + getProxyPassword, + getToken, + setProxyPassword, + setToken, +} from '../../../src/lib/credentials.js'; + +const keyringStore = new Map(); + +vi.mock('@napi-rs/keyring', () => { + class Entry { + private key: string; + constructor(service: string, account: string) { + this.key = `${service}:${account}`; + } + getPassword(): string | null { + return keyringStore.get(this.key) ?? null; + } + setPassword(password: string): void { + keyringStore.set(this.key, password); + } + deletePassword(): boolean { + return keyringStore.delete(this.key); + } + } + return { Entry }; +}); + +const writeAuthFile = (data: Record) => { + mkdirSync(GLOBAL_CONFIGS_FOLDER(), { recursive: true }); + writeFileSync(AUTH_FILE_PATH(), JSON.stringify(data)); +}; + +const readAuthFile = () => JSON.parse(readFileSync(AUTH_FILE_PATH(), 'utf-8')); + +describe('credentials', () => { + beforeEach(() => { + vitest.stubEnv('__APIFY_INTERNAL_TEST_AUTH_PATH__', cryptoRandomObjectId(12)); + keyringStore.clear(); + __resetCredentialsForTests(); + }); + + afterEach(async () => { + await rm(GLOBAL_CONFIGS_FOLDER(), { recursive: true, force: true }); + vitest.unstubAllEnvs(); + __resetCredentialsForTests(); + }); + + describe('getBackend()', () => { + it('returns "file" when APIFY_DISABLE_KEYRING=1', async () => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', '1'); + expect(await getBackend()).toBe('file'); + }); + + it('returns "keyring" when the keyring probe succeeds', async () => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', ''); + expect(await getBackend()).toBe('keyring'); + }); + + it('caches the backend choice for the rest of the process', async () => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', '1'); + expect(await getBackend()).toBe('file'); + vitest.stubEnv('APIFY_DISABLE_KEYRING', ''); + expect(await getBackend()).toBe('file'); + }); + }); + + describe('file backend', () => { + beforeEach(() => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', '1'); + }); + + it('round-trips the token through auth.json', async () => { + await setToken('tok_123'); + expect(await getToken()).toBe('tok_123'); + const file = readAuthFile(); + expect(file.token).toBe('tok_123'); + expect(file.secretsBackend).toBe('file'); + }); + + it('round-trips the proxy password through auth.json', async () => { + await setProxyPassword('pw_abc'); + expect(await getProxyPassword()).toBe('pw_abc'); + expect(readAuthFile().proxy).toEqual({ password: 'pw_abc' }); + }); + + it('skipIfUnchanged is a no-op when the stored value matches', async () => { + await setToken('tok_123'); + const before = readFileSync(AUTH_FILE_PATH(), 'utf-8'); + await setToken('tok_123', { skipIfUnchanged: true }); + const after = readFileSync(AUTH_FILE_PATH(), 'utf-8'); + expect(after).toBe(before); + }); + + it('skipIfUnchanged still writes when the value differs', async () => { + await setToken('tok_123'); + await setToken('tok_456', { skipIfUnchanged: true }); + expect(await getToken()).toBe('tok_456'); + }); + }); + + describe('keyring backend', () => { + beforeEach(() => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', ''); + }); + + it('round-trips the token through the keyring and keeps it out of auth.json', async () => { + await setToken('tok_123'); + expect(await getToken()).toBe('tok_123'); + expect(keyringStore.get('com.apify.cli:token')).toBe('tok_123'); + expect(existsSync(AUTH_FILE_PATH())).toBe(false); + }); + + it('round-trips the proxy password through the keyring and keeps it out of auth.json', async () => { + await setProxyPassword('pw_abc'); + expect(await getProxyPassword()).toBe('pw_abc'); + expect(keyringStore.get('com.apify.cli:proxy-password')).toBe('pw_abc'); + expect(existsSync(AUTH_FILE_PATH())).toBe(false); + }); + + it('clearSecrets() removes the token and proxy entries from the keyring', async () => { + await setToken('tok_123'); + await setProxyPassword('pw_abc'); + await clearSecrets(); + expect(await getToken()).toBeUndefined(); + expect(await getProxyPassword()).toBeUndefined(); + }); + }); + + describe('clearSecrets()', () => { + it('clears the keyring token entry even when APIFY_DISABLE_KEYRING=1 is set at logout time', async () => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', ''); + await setToken('tok_123'); + expect(keyringStore.get('com.apify.cli:token')).toBe('tok_123'); + + __resetCredentialsForTests(); + vitest.stubEnv('APIFY_DISABLE_KEYRING', '1'); + expect(await getBackend()).toBe('file'); + + await clearSecrets(); + expect(keyringStore.get('com.apify.cli:token')).toBeUndefined(); + }); + }); + + describe('ensureMigrated()', () => { + it('is a no-op when secretsBackend marker is already set', async () => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', '1'); + writeAuthFile({ token: 'tok', secretsBackend: 'file' }); + await ensureMigrated(); + expect(readAuthFile().token).toBe('tok'); + }); + + it('is a no-op when there are no secrets to migrate', async () => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', '1'); + await ensureMigrated(); + expect(existsSync(AUTH_FILE_PATH())).toBe(false); + }); + + it('on the file backend, stamps the marker without moving data', async () => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', '1'); + writeAuthFile({ token: 'tok', username: 'u' }); + await ensureMigrated(); + const file = readAuthFile(); + expect(file.token).toBe('tok'); + expect(file.username).toBe('u'); + expect(file.secretsBackend).toBe('file'); + }); + + it('on the keyring backend, moves the token and proxy password out of auth.json', async () => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', ''); + writeAuthFile({ token: 'tok', proxy: { password: 'pw' }, username: 'u' }); + await ensureMigrated(); + expect(keyringStore.get('com.apify.cli:token')).toBe('tok'); + expect(keyringStore.get('com.apify.cli:proxy-password')).toBe('pw'); + const file = readAuthFile(); + expect(file.token).toBeUndefined(); + expect(file.proxy).toBeUndefined(); + expect(file.username).toBe('u'); + expect(file.secretsBackend).toBe('keyring'); + }); + + it('is memoized within a process', async () => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', '1'); + writeAuthFile({ token: 'tok' }); + await ensureMigrated(); + expect(readAuthFile().secretsBackend).toBe('file'); + + // Overwrite the marker and call again — the memoized promise should short-circuit. + writeAuthFile({ token: 'tok2' }); + await ensureMigrated(); + expect(readAuthFile().secretsBackend).toBeUndefined(); + }); + }); +});