diff --git a/src/commands/auth/logout.ts b/src/commands/auth/logout.ts index 68f3b4768..e7593068e 100644 --- a/src/commands/auth/logout.ts +++ b/src/commands/auth/logout.ts @@ -1,12 +1,14 @@ +import process from 'node:process'; + import { APIFY_ENV_VARS } from '@apify/consts'; -import { removeActiveProfile } from '../../lib/auth-file.js'; +import { getActiveProfileId, removeActiveProfile } from '../../lib/auth-file.js'; import { invalidEnvTokenMessage, readEnvToken } from '../../lib/auth.js'; import { ApifyCommand } from '../../lib/command-framework/apify-command.js'; -import { AUTH_FILE_PATH } from '../../lib/consts.js'; +import { AUTH_FILE_PATH, CommandExitCodes } from '../../lib/consts.js'; import { clearKeyringSecrets } from '../../lib/credentials.js'; import { updateUserId } from '../../lib/hooks/telemetry/useTelemetryState.js'; -import { success, warning } from '../../lib/outputs.js'; +import { error, success, warning } from '../../lib/outputs.js'; import { tildify } from '../../lib/utils.js'; export class AuthLogoutCommand extends ApifyCommand { @@ -28,10 +30,28 @@ export class AuthLogoutCommand extends ApifyCommand { static override docsUrl = 'https://docs.apify.com/cli/docs/reference#apify-logout'; async run() { - // The file goes first: it is the step that can refuse, and refusing before the keyring is - // cleared leaves a logged-in state rather than half a logout. - removeActiveProfile(); - await clearKeyringSecrets(); + // Read before either step runs: once the profile is gone, nothing names the keyring entries it owns. + const activeProfileId = getActiveProfileId(); + + // Both steps are attempted even when the first one fails, so neither the secrets nor the + // profile are left behind just because the other could not be removed. + const keyringError = await clearKeyringSecrets(activeProfileId).then( + () => null, + (err: unknown) => err, + ); + + let profileError: unknown = null; + try { + removeActiveProfile(); + } catch (err) { + profileError = err; + } + + if (keyringError || profileError) { + error({ message: partialLogoutMessage(activeProfileId, keyringError, profileError) }); + process.exitCode = CommandExitCodes.RunFailed; + return; + } await updateUserId(null); @@ -47,3 +67,21 @@ export class AuthLogoutCommand extends ApifyCommand { } } } + +function reasonOf(err: unknown) { + return err instanceof Error ? err.message : String(err); +} + +function partialLogoutMessage(activeProfileId: string | undefined, keyringError: unknown, profileError: unknown) { + const keyringPart = keyringError + ? `Your secrets are still in the OS keyring${activeProfileId ? ` under the account ${activeProfileId}` : ''}; delete them with your OS keyring app.` + : 'Your secrets were removed from the OS keyring.'; + + const profilePart = profileError + ? `Your account is still in ${AUTH_FILE_PATH()}; delete that file to finish logging out.` + : `Your account was removed from ${AUTH_FILE_PATH()}.`; + + const reasons = [keyringError, profileError].filter(Boolean).map(reasonOf).join(' '); + + return `Logout did not finish. ${keyringPart} ${profilePart} ${reasons}`; +} diff --git a/src/lib/auth-file.ts b/src/lib/auth-file.ts index 9e7149c2d..88bc59d92 100644 --- a/src/lib/auth-file.ts +++ b/src/lib/auth-file.ts @@ -3,7 +3,7 @@ import { existsSync, readFileSync, renameSync, rmSync, writeFileSync } from 'nod import { cryptoRandomObjectId } from '@apify/utilities'; import { AUTH_FILE_PATH } from './consts.js'; -import type { CredentialsBackend } from './credentials.js'; +import type { CredentialsBackend, SecretKind } from './credentials.js'; import { ensureApifyDirectory } from './files.js'; import { warning } from './outputs.js'; import { cliDebugPrint } from './utils/cliDebugPrint.js'; @@ -26,14 +26,21 @@ export interface AuthProfile { authMethod: 'token'; expiresAt: string | null; hasRefreshToken: boolean; - /** Reserved: a keyring failure on one profile must not redirect another profile's reads. */ + /** + * Where this profile's secrets live, when that differs from the file-level `secretsBackend`. + * Written only when a keyring write for this profile fails, so one profile falling back to the + * file cannot silently redirect another profile's reads to a place its secrets are not. + */ secretsBackend?: CredentialsBackend; loggedInAt: string | null; + /** File backend only. The keyring backend keeps these in the OS store instead. */ + token?: string; + proxy?: { password?: string }; } /** - * `auth.json` as this CLI writes it. `token` and `proxy` are the file backend's secret storage; - * they stay outside the profiles until each profile gets its own keys. + * `auth.json` as this CLI writes it. Top-level `token` and `proxy` are where the file backend kept + * secrets before they were keyed per profile; `ensureSecretsKeyed()` moves them into the profile. */ export interface AuthFile { version?: number; @@ -127,8 +134,8 @@ function v1Profile(file: LegacyAuthFile): AuthProfile { function toV2(file: LegacyAuthFile): AuthFile { const migrated: AuthFile = { version: AUTH_FILE_VERSION, profiles: {} }; - // A v1 file with a token but no ID has no key to store the profile under. Keep the secrets so - // the next command reports stale credentials instead of a silent logged-out state. + // A v1 file with a token but no ID has no key to store the profile under. The secrets are + // carried over here and dropped by `ensureSecretsKeyed()`, which is what forces the re-login. if (typeof file.id === 'string') { migrated.activeProfile = file.id; migrated.profiles![file.id] = v1Profile(file); @@ -190,11 +197,10 @@ async function migrateAuthFile(): Promise { writeAuthFile(migrated); } catch (err) { - // The readers understand the old shape, so nothing is broken and the next command tries - // again. Still said out loud, because failing on every run should not be invisible. + // Not rethrown: the migration must not abort the command, which fails at the auth step. cliDebugPrint('auth-file', 'auth file migration failed', err); warning({ - message: `Your login still works, but ${AUTH_FILE_PATH()} could not be updated to the current format. Set APIFY_CLI_DEBUG=1 to see why.`, + message: `Your stored login cannot be read until ${AUTH_FILE_PATH()} is updated to the current format, and the update failed. Make the directory it is in writable, then run the command again. Set APIFY_CLI_DEBUG=1 to see why.`, }); } })(); @@ -254,10 +260,90 @@ export function getActiveProfile(): (AuthProfile & { id: string }) | undefined { } /** - * Replaces the file with this one account. A second profile would name an account that cannot - * authenticate until each has its own secret, and dropping the old secrets is what keeps the write - * safe: the caller writes the new token next, so a failure there leaves nobody logged in rather - * than the old token beside the new name. Additive login is #1386. + * The user ID every secret is keyed by. Taken from `activeProfile` rather than from the profile + * object, so a file whose `activeProfile` names a missing profile still resolves its secrets and + * reports the dangling profile instead of looking logged out. + */ +export function getActiveProfileId(): string | undefined { + const file = readAuthFile(); + + if (file.version !== AUTH_FILE_VERSION) { + const legacy = file as LegacyAuthFile; + return typeof legacy.id === 'string' ? legacy.id : undefined; + } + + return file.activeProfile; +} + +/** The file backend's stored secret, or `undefined` when the profile does not hold one. */ +export function readProfileSecret(userId: string, kind: SecretKind): string | undefined { + const profile = readAuthFile().profiles?.[userId]; + if (!profile) return undefined; + + return kind === 'token' ? profile.token : profile.proxy?.password; +} + +/** Where this profile's secrets live, or `undefined` when it follows the file-level default. */ +export function readProfileBackend(userId: string): CredentialsBackend | undefined { + return readAuthFile().profiles?.[userId]?.secretsBackend; +} + +/** + * Stores a file-backend secret on the profile. A missing profile is left alone: inventing one + * would fabricate the account metadata the CLI reads. + */ +export function writeProfileSecret(userId: string, kind: SecretKind, value: string) { + updateProfile(userId, (profile) => setProfileSecret(profile, kind, value)); +} + +/** + * Stores the secret and records that this profile reads from the file from now on, in one write. + * Called when a keyring write for this profile failed: splitting the two would leave a window + * where the profile looks logged out, or where it still points at a keyring entry that is not there. + */ +export function moveProfileSecretToFile(userId: string, kind: SecretKind, value: string) { + updateProfile(userId, (profile) => { + setProfileSecret(profile, kind, value); + profile.secretsBackend = 'file'; + }); +} + +function setProfileSecret(profile: AuthProfile, kind: SecretKind, value: string) { + if (kind === 'token') { + profile.token = value; + } else { + profile.proxy = { ...profile.proxy, password: value }; + } +} + +/** Forgets one of a profile's file-backend secrets. */ +export function deleteProfileSecret(userId: string, kind: SecretKind) { + if (readProfileSecret(userId, kind) === undefined) return; + + updateProfile(userId, (profile) => { + if (kind === 'token') { + delete profile.token; + } else { + // The profile's proxy object carries nothing but the password. + delete profile.proxy; + } + }); +} + +function updateProfile(userId: string, edit: (profile: AuthProfile) => void) { + const file = readAuthFile(); + const profile = file.profiles?.[userId]; + if (!profile) return; + + edit(profile); + writeAuthFile(file); +} + +/** + * Replaces the file with this one account, dropping any previous profile and its secrets. Nothing + * puts a second profile there yet; additive login is #1386. Dropping the old secrets is what keeps + * the write safe: the caller writes the new token next, so a failure there leaves nobody logged in + * rather than the old token beside the new name. */ export function replaceStoredAccount(userId: string, profile: AuthProfile, secretsBackend: CredentialsBackend) { assertSupportedAuthFileVersion(); diff --git a/src/lib/auth.ts b/src/lib/auth.ts index 636dfb1cd..7085797e3 100644 --- a/src/lib/auth.ts +++ b/src/lib/auth.ts @@ -6,15 +6,16 @@ import { AxiosHeaders } from 'axios'; import { APIFY_ENV_VARS } from '@apify/consts'; -import { ensureAuthFileCurrent, replaceStoredAccount } from './auth-file.js'; +import { ensureAuthFileCurrent, getActiveProfileId, replaceStoredAccount } from './auth-file.js'; import { APIFY_CLIENT_DEFAULT_HEADERS, AUTH_FILE_PATH, CommandExitCodes } from './consts.js'; import { - deleteProxyPassword, + clearKeyringSecrets, + deleteSecret, ensureMigrated, + ensureSecretsKeyed, getBackend, - getToken, - setProxyPassword, - setToken, + getSecret, + setSecret, } from './credentials.js'; import { warning } from './outputs.js'; import type { AuthJSON } from './types.js'; @@ -98,8 +99,10 @@ export const resolveAuth = async (): Promise => { // Only now, because the stored file is not this command's credential when APIFY_TOKEN is // set. A file a newer CLI wrote would otherwise stop a platform run that never reads it. await ensureAuthFileCurrent(); + await ensureSecretsKeyed(); - const storedToken = await getToken(); + const userId = getActiveProfileId(); + const storedToken = userId ? await getSecret(userId, 'token') : undefined; return storedToken ? ({ token: storedToken, source: 'stored' } as const) : undefined; })(); @@ -177,6 +180,8 @@ export async function loginWithToken( const proxyPassword = userInfo.proxy?.password; + const previousUserId = getActiveProfileId(); + const { organizationOwnerUserId } = userInfo as { organizationOwnerUserId?: string }; replaceStoredAccount( userInfo.id, @@ -192,13 +197,18 @@ export async function loginWithToken( await getBackend(), ); + // Only once the switch is on disk: a failed write leaves auth.json naming the previous account, whose entries nothing else can find. + if (previousUserId && previousUserId !== userInfo.id) { + await clearKeyringSecrets(previousUserId); + } + // After the account, which drops the previous secrets. `skipIfUnchanged` avoids a Keychain prompt. - await setToken(token, { skipIfUnchanged: true }); + await setSecret(userInfo.id, 'token', token, { skipIfUnchanged: true }); if (proxyPassword) { - await setProxyPassword(proxyPassword, { skipIfUnchanged: true }); + await setSecret(userInfo.id, 'proxy-password', proxyPassword, { skipIfUnchanged: true }); } else { - await deleteProxyPassword(); + await deleteSecret(userInfo.id, 'proxy-password'); } return { client: apifyClient, userInfo }; diff --git a/src/lib/credentials.ts b/src/lib/credentials.ts index 849ed51b5..38d495f43 100644 --- a/src/lib/credentials.ts +++ b/src/lib/credentials.ts @@ -1,15 +1,50 @@ import process from 'node:process'; -import { AUTH_FILE_VERSION, readAuthFile, writeAuthFile } from './auth-file.js'; +import type { AuthFile } from './auth-file.js'; +import { + AUTH_FILE_VERSION, + deleteProfileSecret, + moveProfileSecretToFile, + readAuthFile, + readProfileBackend, + readProfileSecret, + writeAuthFile, + writeProfileSecret, +} from './auth-file.js'; import { useCLIMetadata } from './hooks/useCLIMetadata.js'; import { cliDebugPrint } from './utils/cliDebugPrint.js'; +/** + * Base service name. The per-kind services hang off it; the bare name is where secrets sat before + * they were keyed by user. + */ const KEYRING_SERVICE = 'com.apify.cli'; -const TOKEN_ACCOUNT = 'token'; -const PROXY_PASSWORD_ACCOUNT = 'proxy-password'; export type CredentialsBackend = 'keyring' | 'file'; +export type SecretKind = 'token' | 'proxy-password'; + +const SECRET_KINDS: readonly SecretKind[] = ['token', 'proxy-password']; + +interface KeyringKey { + service: string; + account: string; +} + +/** + * One keyring service per kind, with the user ID as the account. A composite account name + * (`token:` under a single service) would depend on `:` being legal in an account name on + * macOS Keychain, libsecret and Windows Credential Manager, and it reads worse in keyring UIs. + */ +function keyringKey(userId: string, kind: SecretKind): KeyringKey { + return { service: `${KEYRING_SERVICE}.${kind}`, account: userId }; +} + +/** Where a secret sat before it was keyed by user: one service, the kind as the account. */ +function legacyKeyringKey(kind: SecretKind): KeyringKey { + return { service: KEYRING_SERVICE, account: kind }; +} + interface KeyringEntry { getPassword(): string | null; setPassword(password: string): void; @@ -23,12 +58,14 @@ interface KeyringModule { let cachedKeyringModule: KeyringModule | null | undefined; let backendPromise: Promise | undefined; let migrationPromise: Promise | undefined; +let keyingPromise: Promise | undefined; /** Test-only: clear cached module/backend/migration so each test starts fresh. */ export function __resetCredentialsForTests() { cachedKeyringModule = undefined; backendPromise = undefined; migrationPromise = undefined; + keyingPromise = undefined; } async function loadKeyringModule(): Promise { @@ -70,9 +107,11 @@ async function importKeyringModule(): Promise { * Single-flight via a promise so concurrent callers share the same lookup. * Order: APIFY_DISABLE_KEYRING env override -> persisted marker in auth.json -> module load. * + * This is the default every profile follows; a profile whose keyring write failed records its own + * `secretsBackend` and reads through {@link backendFor} instead. + * * No write-probe runs here: on macOS that would pop a keychain prompt before the user has - * authorized one. The first real write is the probe — failure is caught and downgraded - * via `downgradeBackendToFile()`, persisting the file marker so future runs skip the keyring. + * authorized one. The first real write is the probe, and a failure falls back to the file. */ export async function getBackend(): Promise { if (backendPromise) return backendPromise; @@ -88,8 +127,9 @@ export async function getBackend(): Promise { } /** - * Called when a keyring write fails at runtime. Flips the cached backend so subsequent - * reads/writes use the file path immediately, without waiting for the marker on disk. + * Called when a keyring write fails before any profile exists, so there is nothing to record the + * fallback on but the file itself. Flips the cached backend so subsequent reads and writes use the + * file path immediately, without waiting for the marker on disk. */ function downgradeBackendToFile() { backendPromise = Promise.resolve('file'); @@ -106,131 +146,113 @@ export function stripProxyPassword(data: { proxy?: { password?: string } }) { if (Object.keys(data.proxy).length === 0) delete data.proxy; } -async function getKeyringEntry(account: string): Promise { +async function getKeyringEntry({ service, account }: KeyringKey): Promise { const mod = await loadKeyringModule(); if (!mod) return null; - return new mod.Entry(KEYRING_SERVICE, account); + return new mod.Entry(service, account); } -async function readKeyring(account: string): Promise { +async function readKeyring(key: KeyringKey): Promise { try { - const entry = await getKeyringEntry(account); + const entry = await getKeyringEntry(key); if (!entry) return undefined; return entry.getPassword() ?? undefined; } catch (err) { - cliDebugPrint('credentials', `failed to read ${account} from keyring`, err); + cliDebugPrint('credentials', `failed to read ${key.service}/${key.account} from keyring`, err); return undefined; } } -async function writeKeyring(account: string, value: string): Promise { - const entry = await getKeyringEntry(account); +async function writeKeyring(key: KeyringKey, value: string): Promise { + const entry = await getKeyringEntry(key); if (!entry) { throw new Error('OS keyring is not available.'); } entry.setPassword(value); } -async function deleteKeyring(account: string): Promise { +async function deleteKeyring(key: KeyringKey): Promise { try { - const entry = await getKeyringEntry(account); + const entry = await getKeyringEntry(key); if (!entry) return; entry.deletePassword(); } catch (err) { - cliDebugPrint('credentials', `failed to delete ${account} from keyring`, err); + cliDebugPrint('credentials', `failed to delete ${key.service}/${key.account} from keyring`, err); } } -export async function getToken(): Promise { - const backend = await getBackend(); - if (backend === 'keyring') return readKeyring(TOKEN_ACCOUNT); - return readAuthFile().token; +/** + * Where one account's secrets live. A profile that fell back to the file after a keyring failure + * says so itself; every other profile follows the file-level choice. + */ +async function backendFor(userId: string): Promise { + return readProfileBackend(userId) ?? (await getBackend()); } -export async function getProxyPassword(): Promise { - const backend = await getBackend(); - if (backend === 'keyring') return readKeyring(PROXY_PASSWORD_ACCOUNT); - return readAuthFile().proxy?.password; +/** One account's secret of the given kind, from whichever backend holds it. */ +export async function getSecret(userId: string, kind: SecretKind): Promise { + if ((await backendFor(userId)) === 'keyring') return readKeyring(keyringKey(userId, kind)); + return readProfileSecret(userId, kind); } /** - * Persist token. When `skipIfUnchanged` is true and the stored value already matches, - * the write is skipped. This avoids macOS Keychain prompts on every command. + * Persist one account's secret. 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; - } +export async function setSecret( + userId: string, + kind: SecretKind, + value: string, + opts: { skipIfUnchanged?: boolean } = {}, +): Promise { + const backend = await backendFor(userId); + if (opts.skipIfUnchanged && (await getSecret(userId, kind)) === value) return; if (backend === 'keyring') { try { - await writeKeyring(TOKEN_ACCOUNT, token); + await writeKeyring(keyringKey(userId, kind), value); return; } catch (err) { + // Recorded on the profile rather than on the file, so an account whose secrets are in + // the keyring is not redirected to a file that does not hold them. cliDebugPrint('credentials', 'keyring write failed; falling back to file', err); - downgradeBackendToFile(); - } - } - - 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') { - try { - await writeKeyring(PROXY_PASSWORD_ACCOUNT, password); + moveProfileSecretToFile(userId, kind, value); return; - } catch (err) { - cliDebugPrint('credentials', 'keyring write failed; falling back to file', err); - downgradeBackendToFile(); } } - const data = readAuthFile(); - data.proxy = { ...data.proxy, password }; - data.secretsBackend = 'file'; - writeAuthFile(data); + writeProfileSecret(userId, kind, value); } /** - * Forget the stored proxy password. Called when an account has none, so the previous account's - * does not survive a re-login — the keyring outlives the auth.json rewrite that replaces - * everything else. + * Forget one of an account's secrets. Called for a proxy password when the account has none, so + * the previous account's does not survive a re-login — the keyring outlives the auth.json rewrite + * that replaces everything else. */ -export async function deleteProxyPassword(): Promise { - if ((await getBackend()) === 'keyring') { - await deleteKeyring(PROXY_PASSWORD_ACCOUNT); +export async function deleteSecret(userId: string, kind: SecretKind): Promise { + if ((await backendFor(userId)) === 'keyring') { + await deleteKeyring(keyringKey(userId, kind)); return; } - const data = readAuthFile(); - if (!data.proxy?.password) return; - - stripProxyPassword(data); - writeAuthFile(data); + deleteProfileSecret(userId, kind); } /** - * Remove the token and proxy-password entries from the OS keyring. Always attempts the - * keyring deletes 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. Plaintext secrets in `auth.json` are the caller's - * responsibility (e.g. `logout` removes the whole file). + * Remove one profile's keyring entries, plus the fixed-name entries used before secrets were keyed + * by user. Always attempts the keyring deletes 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. + * + * The keyring has no listing API, so `auth.json` is the only index of what it holds. Call this + * before the profile leaves the file, or its entries become unreachable. Secrets stored in + * `auth.json` itself go with the profile that holds them. */ -export async function clearKeyringSecrets(): Promise { - await deleteKeyring(TOKEN_ACCOUNT); - await deleteKeyring(PROXY_PASSWORD_ACCOUNT); +export async function clearKeyringSecrets(userId?: string): Promise { + for (const kind of SECRET_KINDS) { + if (userId) await deleteKeyring(keyringKey(userId, kind)); + await deleteKeyring(legacyKeyringKey(kind)); + } } /** @@ -262,8 +284,10 @@ export async function ensureMigrated(): Promise { } try { - if (file.token) await writeKeyring(TOKEN_ACCOUNT, file.token); - if (file.proxy?.password) await writeKeyring(PROXY_PASSWORD_ACCOUNT, file.proxy.password); + if (file.token) await writeKeyring(legacyKeyringKey('token'), file.token); + if (file.proxy?.password) { + await writeKeyring(legacyKeyringKey('proxy-password'), file.proxy.password); + } } catch (err) { cliDebugPrint('credentials', 'keyring write failed during migration; falling back to file', err); downgradeBackendToFile(); @@ -282,3 +306,100 @@ export async function ensureMigrated(): Promise { })(); return migrationPromise; } + +/** + * Drops secrets there is no user ID to file under. That state already required a re-login — the + * CLI has no account to attach the token to — so nothing reachable is lost. + */ +async function dropUnkeyedSecrets(file: AuthFile): Promise { + for (const kind of SECRET_KINDS) await deleteKeyring(legacyKeyringKey(kind)); + + if (file.token === undefined && file.proxy === undefined) return; + + delete file.token; + delete file.proxy; + writeAuthFile(file); +} + +/** + * Write the new entry, verify it reads back, then delete the old one. The reverse order loses the + * secret when the delete succeeds and the write does not. + */ +async function keyKeyringSecrets(userId: string): Promise { + for (const kind of SECRET_KINDS) { + const legacy = legacyKeyringKey(kind); + const value = await readKeyring(legacy); + if (value === undefined) continue; + + // A failure earlier in this loop moved this profile to the file, so the secrets after it + // belong there too rather than under a keyring name nothing will read. + if ((await backendFor(userId)) === 'keyring') { + const target = keyringKey(userId, kind); + + try { + await writeKeyring(target, value); + if ((await readKeyring(target)) === value) await deleteKeyring(legacy); + continue; + } catch (err) { + cliDebugPrint('credentials', 'keyring write failed while keying secrets by user', err); + } + } + + moveProfileSecretToFile(userId, kind, value); + if (readProfileSecret(userId, kind) === value) await deleteKeyring(legacy); + } +} + +/** One atomic write moves the secrets into the profile and clears the top level. */ +function keyFileSecrets(userId: string, file: AuthFile): void { + const profile = file.profiles?.[userId]; + if (!profile) return; + + const { token } = file; + const proxyPassword = file.proxy?.password; + if (token === undefined && proxyPassword === undefined) return; + + if (token !== undefined) profile.token = token; + if (proxyPassword !== undefined) profile.proxy = { password: proxyPassword }; + + // The file-level marker already says `file`: nothing else puts secrets at the top level. + delete file.token; + delete file.proxy; + writeAuthFile(file); +} + +/** + * Moves secrets off the fixed names they shared onto keys that carry the user ID, so a second + * account cannot overwrite the first one's token. + * + * Runs after `ensureAuthFileCurrent()` — the user ID comes from the v2 file. A v2 file whose + * secrets still sit under the old names is a supported state: every user is in it between the two + * releases, and the two migrations stay independent. + * + * Idempotent, single-flight, and it never throws — a migration failure must not block a command. + */ +export async function ensureSecretsKeyed(): Promise { + keyingPromise ??= (async () => { + try { + const file = readAuthFile(); + if (file.version !== AUTH_FILE_VERSION) return; + + const userId = file.activeProfile; + if (!userId) { + await dropUnkeyedSecrets(file); + return; + } + + if ((await backendFor(userId)) === 'keyring') { + await keyKeyringSecrets(userId); + return; + } + + keyFileSecrets(userId, file); + } catch (err) { + cliDebugPrint('credentials', 'keying secrets by user failed', err); + } + })(); + + return keyingPromise; +} diff --git a/src/lib/utils.ts b/src/lib/utils.ts index b1ce8e476..08bcb94a1 100644 --- a/src/lib/utils.ts +++ b/src/lib/utils.ts @@ -42,7 +42,7 @@ import { MINIMUM_SUPPORTED_PYTHON_VERSION, SUPPORTED_NODEJS_VERSION, } from './consts.js'; -import { ensureMigrated, getProxyPassword, getToken } from './credentials.js'; +import { ensureMigrated, ensureSecretsKeyed, getSecret } from './credentials.js'; import { deleteFile, ensureFolderExistsSync, rimrafPromised } from './files.js'; import { useCLIMetadata } from './hooks/useCLIMetadata.js'; import { inputFileRegExp, TEMP_INPUT_KEY_PREFIX } from './input-key.js'; @@ -92,34 +92,30 @@ export const getLocalRequestQueuePath = (storeId?: string) => { export const getLocalUserInfo = async (): Promise => { await ensureMigrated(); await ensureAuthFileCurrent(); + await ensureSecretsKeyed(); const { profile, missingProfile } = lookUpActiveProfile(); - const result: AuthJSON = {}; - if (profile) { - result.id = profile.id; - if (profile.username) result.username = profile.username; - if (profile.organizationOwnerUserId) result.organizationOwnerUserId = profile.organizationOwnerUserId; - } - - const token = await getToken(); - if (token) result.token = token; - - const proxyPassword = await getProxyPassword(); - if (proxyPassword) result.proxy = { password: proxyPassword }; - // Reported rather than swallowed: the commands that build `/` lookups would // otherwise fail with a misleading "not found". - if (!profile) { - if (!result.token) return {}; - + if (missingProfile) { throw new Error( - missingProfile - ? `Your active profile "${missingProfile}" is missing from ${AUTH_FILE_PATH()}. Run "apify login" to log in again.` - : 'Stale credentials found without user metadata. Run "apify login" again.', + `Your active profile "${missingProfile}" is missing from ${AUTH_FILE_PATH()}. Run "apify login" to log in again.`, ); } + if (!profile) return {}; + + const result: AuthJSON = { id: profile.id }; + if (profile.username) result.username = profile.username; + if (profile.organizationOwnerUserId) result.organizationOwnerUserId = profile.organizationOwnerUserId; + + const token = await getSecret(profile.id, 'token'); + if (token) result.token = token; + + const proxyPassword = await getSecret(profile.id, 'proxy-password'); + if (proxyPassword) result.proxy = { password: proxyPassword }; + return result; }; diff --git a/test/__setup__/auth-file.ts b/test/__setup__/auth-file.ts index 6e617345d..26588be38 100644 --- a/test/__setup__/auth-file.ts +++ b/test/__setup__/auth-file.ts @@ -3,8 +3,12 @@ import { readFileSync } from 'node:fs'; import type { AuthFile, AuthProfile } from '../../src/lib/auth-file.js'; +import { AUTH_FILE_VERSION } from '../../src/lib/auth-file.js'; import { AUTH_FILE_PATH } from '../../src/lib/consts.js'; +/** The user ID the fixtures below key their single profile by. */ +export const TEST_USER_ID = 'uid'; + /** The raw file, for assertions about the version, the backend marker, or where secrets landed. */ export function readAuthFile(): AuthFile { return JSON.parse(readFileSync(AUTH_FILE_PATH(), 'utf-8')) as AuthFile; @@ -19,6 +23,26 @@ export function readActiveProfile(): (AuthProfile & { id: string }) | undefined return profile ? { id: activeProfile, ...profile } : undefined; } +/** A v2 `auth.json` holding one profile, the shape a login writes. */ +export function v2AuthFile(profile: Partial = {}, rest: Partial = {}): AuthFile { + return { + version: AUTH_FILE_VERSION, + activeProfile: TEST_USER_ID, + profiles: { + [TEST_USER_ID]: { + username: 'me', + name: null, + authMethod: 'token', + expiresAt: null, + hasRefreshToken: false, + loggedInAt: null, + ...profile, + }, + }, + ...rest, + }; +} + /** A v1 `auth.json`, the shape every CLI before the profile migration wrote. */ export function v1AuthFile(overrides: Record = {}) { return { diff --git a/test/__setup__/keyring-mock.ts b/test/__setup__/keyring-mock.ts index 902896793..6bfdf53cb 100644 --- a/test/__setup__/keyring-mock.ts +++ b/test/__setup__/keyring-mock.ts @@ -3,8 +3,16 @@ * `vi.mock('@napi-rs/keyring', () => import('/keyring-mock.js'))`. */ -export const KEYRING_TOKEN_KEY = 'com.apify.cli:token'; -export const KEYRING_PROXY_PASSWORD_KEY = 'com.apify.cli:proxy-password'; +/** The fixed names secrets shared before they were keyed by user. */ +export const LEGACY_KEYRING_TOKEN_KEY = 'com.apify.cli:token'; +export const LEGACY_KEYRING_PROXY_PASSWORD_KEY = 'com.apify.cli:proxy-password'; + +/** + * One service per kind, the user ID as the account. Spelled out here rather than imported so the + * test fails when the production key scheme changes without anyone meaning to change it. + */ +export const keyringTokenKey = (userId: string) => `com.apify.cli.token:${userId}`; +export const keyringProxyPasswordKey = (userId: string) => `com.apify.cli.proxy-password:${userId}`; export const keyringStore = new Map(); diff --git a/test/local/commands/auth.test.ts b/test/local/commands/auth.test.ts index bdcf3cc2f..e678ba951 100644 --- a/test/local/commands/auth.test.ts +++ b/test/local/commands/auth.test.ts @@ -1,20 +1,23 @@ -import { existsSync, statSync } from 'node:fs'; +import { chmodSync, existsSync, statSync } from 'node:fs'; import process from 'node:process'; -import { AUTH_FILE_PATH, CommandExitCodes } from '../../../src/lib/consts.js'; -import { getToken } from '../../../src/lib/credentials.js'; +import { AUTH_FILE_PATH, CommandExitCodes, GLOBAL_CONFIGS_FOLDER } from '../../../src/lib/consts.js'; +import { getSecret } from '../../../src/lib/credentials.js'; import { clientState, resetApifyClientMock } from '../../__setup__/apify-client-mock.js'; import { readActiveProfile, readAuthFile } from '../../__setup__/auth-file.js'; import { useAuthSetup, useKeyringBackend } from '../../__setup__/hooks/useAuthSetup.js'; import { useConsoleSpy } from '../../__setup__/hooks/useConsoleSpy.js'; import { - KEYRING_PROXY_PASSWORD_KEY, - KEYRING_TOKEN_KEY, + keyringProxyPasswordKey, keyringSetKeys, keyringStore, + keyringTokenKey, resetKeyringMock, } from '../../__setup__/keyring-mock.js'; +const TOKEN_KEY = keyringTokenKey('uid'); +const PROXY_PASSWORD_KEY = keyringProxyPasswordKey('uid'); + vi.mock('@napi-rs/keyring', () => import('../../__setup__/keyring-mock.js')); vi.mock('apify-client', async (importOriginal) => ({ @@ -44,7 +47,8 @@ describe('auth commands', () => { it('login stores the token and one profile keyed by user ID', async () => { await login(); - expect(readAuthFile()).toMatchObject({ version: 2, token: TOKEN, secretsBackend: 'file' }); + expect(readAuthFile()).toMatchObject({ version: 2, secretsBackend: 'file' }); + expect(readAuthFile().token).toBeUndefined(); expect(readActiveProfile()).toEqual({ id: 'uid', username: 'me', @@ -53,6 +57,8 @@ describe('auth commands', () => { expiresAt: null, hasRefreshToken: false, loggedInAt: expect.stringMatching(/^\d{4}-\d{2}-\d{2}T/), + token: TOKEN, + proxy: { password: 'pw' }, }); expect(lastErrorMessage()).toContain('You are logged in to Apify as me'); }); @@ -75,7 +81,7 @@ describe('auth commands', () => { await testRunCommand(AuthLogoutCommand, {}); expect(existsSync(AUTH_FILE_PATH())).toBe(false); - expect(await getToken()).toBeUndefined(); + expect(await getSecret('uid', 'token')).toBeUndefined(); }); it('logging in as another account replaces the stored profile', async () => { @@ -86,10 +92,10 @@ describe('auth commands', () => { await login('apify_api_other_token'); const authFile = readAuthFile(); - expect(authFile).toMatchObject({ activeProfile: 'uid2', token: 'apify_api_other_token' }); + expect(authFile).toMatchObject({ activeProfile: 'uid2' }); // Additive login is a later stage; until then the old profile must not linger. expect(Object.keys(authFile.profiles!)).toEqual(['uid2']); - expect(readActiveProfile()).toMatchObject({ username: 'other' }); + expect(readActiveProfile()).toMatchObject({ username: 'other', token: 'apify_api_other_token' }); }); it('login with an invalid token stores nothing and fails the command', async () => { @@ -119,7 +125,7 @@ describe('auth commands', () => { await login(); - expect(await getToken()).toBe(TOKEN); + expect(await getSecret('uid', 'token')).toBe(TOKEN); expect(lastErrorMessage()).toContain('You are logged in to Apify as me'); }); @@ -128,7 +134,7 @@ describe('auth commands', () => { await login(); - expect(await getToken()).toBe(TOKEN); + expect(await getSecret('uid', 'token')).toBe(TOKEN); expect(lastErrorMessage()).toContain('You are logged in to Apify as me'); }); @@ -174,7 +180,7 @@ describe('auth commands', () => { await testRunCommand(AuthTokenCommand, {}); expect(lastLogMessage()).toBe('apify_api_env_token'); - expect(await getToken()).toBe(TOKEN); + expect(await getSecret('uid', 'token')).toBe(TOKEN); expect(readActiveProfile()).toMatchObject({ username: 'me' }); }); }); @@ -185,8 +191,8 @@ describe('auth commands', () => { it('login stores the secrets in the keyring and keeps them out of auth.json', async () => { await login(); - expect(keyringStore.get(KEYRING_TOKEN_KEY)).toBe(TOKEN); - expect(keyringStore.get(KEYRING_PROXY_PASSWORD_KEY)).toBe('pw'); + expect(keyringStore.get(TOKEN_KEY)).toBe(TOKEN); + expect(keyringStore.get(PROXY_PASSWORD_KEY)).toBe('pw'); const authFile = readAuthFile(); expect(authFile).toMatchObject({ version: 2, secretsBackend: 'keyring' }); @@ -198,21 +204,43 @@ describe('auth commands', () => { it('logging in as an account with no proxy password forgets the previous one', async () => { await login(); - expect(keyringStore.get(KEYRING_PROXY_PASSWORD_KEY)).toBe('pw'); + expect(keyringStore.get(PROXY_PASSWORD_KEY)).toBe('pw'); clientState.user = { id: 'uid2', username: 'other' }; await login('apify_api_other_token'); // The keyring outlives the auth.json rewrite, so without an explicit delete the child // Actor would run with the previous account's proxy credential. - expect(keyringStore.has(KEYRING_PROXY_PASSWORD_KEY)).toBe(false); + expect(keyringStore.has(PROXY_PASSWORD_KEY)).toBe(false); + expect(keyringStore.has(keyringProxyPasswordKey('uid2'))).toBe(false); + }); + + it('switching accounts clears the outgoing account entries', async () => { + await login(); + expect(keyringStore.get(TOKEN_KEY)).toBe(TOKEN); + + clientState.user = { id: 'uid2', username: 'other', proxy: { password: 'pw2' } }; + await login('apify_api_other_token'); + + // auth.json no longer names uid, and the keyring has no listing API, so anything left + // under its key would be unreachable for good. + expect(keyringStore.get(TOKEN_KEY)).toBeUndefined(); + expect(keyringStore.get(keyringTokenKey('uid2'))).toBe('apify_api_other_token'); + }); + + it('logging in again as the same account keeps its entries', async () => { + await login(); + await login(); + + expect(keyringStore.get(TOKEN_KEY)).toBe(TOKEN); + expect(keyringStore.get(PROXY_PASSWORD_KEY)).toBe('pw'); }); it('logging in twice with the same token writes the keyring once', async () => { await login(); await login(); - expect(keyringSetKeys.filter((key) => key === KEYRING_TOKEN_KEY)).toHaveLength(1); + expect(keyringSetKeys.filter((key) => key === TOKEN_KEY)).toHaveLength(1); }); it('auth token prints the token from the keyring', async () => { @@ -229,5 +257,50 @@ describe('auth commands', () => { expect(keyringStore.size).toBe(0); expect(existsSync(AUTH_FILE_PATH())).toBe(false); }); + + // Clearing the keyring before the switch is written left both accounts unreachable: the + // keyring has no listing API, so auth.json is the only index of what it holds. + it.skipIf(process.platform === 'win32')( + 'a switch that cannot be written keeps the outgoing account entries', + async () => { + await login(); + expect(keyringStore.get(TOKEN_KEY)).toBe(TOKEN); + + clientState.user = { id: 'uid2', username: 'other' }; + chmodSync(GLOBAL_CONFIGS_FOLDER(), 0o500); + + try { + await login('apify_api_other_token'); + + expect(readActiveProfile()).toMatchObject({ id: 'uid' }); + expect(keyringStore.get(TOKEN_KEY)).toBe(TOKEN); + expect(keyringStore.get(PROXY_PASSWORD_KEY)).toBe('pw'); + } finally { + chmodSync(GLOBAL_CONFIGS_FOLDER(), 0o700); + process.exitCode = 0; + } + }, + ); + + // Exiting 0 with a success line told the user they were logged out while auth.json still + // held the account the keyring entries were just deleted for. + it.skipIf(process.platform === 'win32')('logout says so when the profile cannot be removed', async () => { + await login(); + chmodSync(GLOBAL_CONFIGS_FOLDER(), 0o500); + + try { + await testRunCommand(AuthLogoutCommand, {}); + + expect(keyringStore.size).toBe(0); + expect(existsSync(AUTH_FILE_PATH())).toBe(true); + expect(lastErrorMessage()).toContain('Logout did not finish'); + expect(lastErrorMessage()).toContain('Your secrets were removed from the OS keyring.'); + expect(lastErrorMessage()).toContain(`Your account is still in ${AUTH_FILE_PATH()}`); + expect(process.exitCode).toBe(CommandExitCodes.RunFailed); + } finally { + chmodSync(GLOBAL_CONFIGS_FOLDER(), 0o700); + process.exitCode = 0; + } + }); }); }); diff --git a/test/local/lib/auth-file.test.ts b/test/local/lib/auth-file.test.ts index f3b17a90c..ec1029357 100644 --- a/test/local/lib/auth-file.test.ts +++ b/test/local/lib/auth-file.test.ts @@ -13,14 +13,14 @@ import { } from '../../../src/lib/auth-file.js'; import { resolveAuth } from '../../../src/lib/auth.js'; import { AUTH_FILE_PATH, GLOBAL_CONFIGS_FOLDER } from '../../../src/lib/consts.js'; -import { ensureMigrated, getProxyPassword, getToken } from '../../../src/lib/credentials.js'; +import { ensureMigrated, ensureSecretsKeyed, getSecret } from '../../../src/lib/credentials.js'; import { getLocalUserInfo } from '../../../src/lib/utils.js'; import { readActiveProfile, readAuthFile, v1AuthFile } from '../../__setup__/auth-file.js'; import { useAuthSetup, useKeyringBackend } from '../../__setup__/hooks/useAuthSetup.js'; import { useConsoleSpy } from '../../__setup__/hooks/useConsoleSpy.js'; import { - KEYRING_PROXY_PASSWORD_KEY, - KEYRING_TOKEN_KEY, + LEGACY_KEYRING_PROXY_PASSWORD_KEY, + LEGACY_KEYRING_TOKEN_KEY, keyringStore, resetKeyringMock, } from '../../__setup__/keyring-mock.js'; @@ -77,9 +77,12 @@ describe('auth.json v2', () => { await ensureAuthFileCurrent(); - expect(readAuthFile()).toMatchObject({ version: 2, secretsBackend: 'file', token: 'apify_api_v1_token' }); - expect(await getToken()).toBe('apify_api_v1_token'); - expect(await getProxyPassword()).toBe('pw'); + await ensureSecretsKeyed(); + + expect(readAuthFile()).toMatchObject({ version: 2, secretsBackend: 'file' }); + expect(readActiveProfile()).toMatchObject({ token: 'apify_api_v1_token', proxy: { password: 'pw' } }); + expect(await getSecret('uid', 'token')).toBe('apify_api_v1_token'); + expect(await getSecret('uid', 'proxy-password')).toBe('pw'); }); it('drops the fields nothing in the CLI reads', async () => { @@ -178,19 +181,26 @@ describe('auth.json v2', () => { }); // The failure path had no cover: the whole migration sits in one try/catch. - it.skipIf(process.platform === 'win32')('says so when it cannot write, and still logs you in', async () => { - write(v1AuthFile({ secretsBackend: 'file' })); - chmodSync(GLOBAL_CONFIGS_FOLDER(), 0o500); - - try { - // The old shape still reads, so the command that triggered this keeps working. - await expect(getLocalUserInfo()).resolves.toMatchObject({ id: 'uid', username: 'me' }); - expect(lastErrorMessage()).toContain('Your login still works'); - expect(readAuthFile().version).toBeUndefined(); - } finally { - chmodSync(GLOBAL_CONFIGS_FOLDER(), 0o700); - } - }); + it.skipIf(process.platform === 'win32')( + 'says the stored login cannot be read when it cannot write, and hands back no token', + async () => { + write(v1AuthFile({ secretsBackend: 'file' })); + chmodSync(GLOBAL_CONFIGS_FOLDER(), 0o500); + + try { + const info = await getLocalUserInfo(); + + expect(info).toMatchObject({ id: 'uid', username: 'me' }); + expect(info).not.toHaveProperty('token'); + expect(lastErrorMessage()).toContain('Your stored login cannot be read'); + // The write goes through a temp file and a rename, so the directory is what must be writable. + expect(lastErrorMessage()).toContain('Make the directory it is in writable'); + expect(readAuthFile().version).toBeUndefined(); + } finally { + chmodSync(GLOBAL_CONFIGS_FOLDER(), 0o700); + } + }, + ); it('does nothing when there is no file', async () => { await ensureAuthFileCurrent(); @@ -208,20 +218,16 @@ describe('auth.json v2', () => { expect(existsSync(AUTH_BACKUP_FILE_PATH())).toBe(false); }); - it('keeps the secrets of a v1 file that has no user ID, so the next command asks for a re-login', async () => { + it('drops the secrets of a v1 file that has no user ID, so the next command asks for a re-login', async () => { write({ token: 'apify_api_v1_token', secretsBackend: 'file' }); await ensureAuthFileCurrent(); + await ensureSecretsKeyed(); - expect(readAuthFile()).toEqual({ - version: 2, - profiles: {}, - secretsBackend: 'file', - token: 'apify_api_v1_token', - }); - // The token stays in auth.json, where the re-login prompt can see it, not in the backup. + // That state already needed a re-login: there is no account to attach the token to. + expect(readAuthFile()).toEqual({ version: 2, profiles: {}, secretsBackend: 'file' }); expect(readBackup()).toEqual({ secretsBackend: 'file' }); - await expect(getLocalUserInfo()).rejects.toThrow('Stale credentials found without user metadata'); + await expect(getLocalUserInfo()).resolves.toEqual({}); }); }); @@ -250,10 +256,10 @@ describe('auth.json v2', () => { await expect(getLocalUserInfo()).rejects.toThrow('Your active profile "gone" is missing'); }); - it('is logged out when the missing profile leaves no token behind either', async () => { + it('names the missing profile even when no secret is left behind', async () => { write({ version: 2, activeProfile: 'gone', profiles: {}, secretsBackend: 'file' }); - await expect(getLocalUserInfo()).resolves.toEqual({}); + await expect(getLocalUserInfo()).rejects.toThrow('Your active profile "gone" is missing'); }); }); @@ -321,7 +327,7 @@ describe('auth.json v2', () => { replaceStoredAccount('new', { ...V2_PROFILE, username: 'new' }, 'file'); // Logged out, rather than logged in as the account that just went away. - await expect(getToken()).resolves.toBeUndefined(); + await expect(getSecret('new', 'token')).resolves.toBeUndefined(); }); }); @@ -376,8 +382,8 @@ describe('auth.json v2', () => { // State B in the wild: secrets already in the keyring, auth.json holding only metadata. it('migrates state B without touching the keyring', async () => { - keyringStore.set(KEYRING_TOKEN_KEY, 'tok_kr'); - keyringStore.set(KEYRING_PROXY_PASSWORD_KEY, 'pw_kr'); + keyringStore.set(LEGACY_KEYRING_TOKEN_KEY, 'tok_kr'); + keyringStore.set(LEGACY_KEYRING_PROXY_PASSWORD_KEY, 'pw_kr'); write({ id: 'uid', username: 'me', email: 'me@example.com', secretsBackend: 'keyring' }); await ensureMigrated(); @@ -404,7 +410,7 @@ describe('auth.json v2', () => { await ensureMigrated(); await ensureAuthFileCurrent(); - expect(keyringStore.get(KEYRING_TOKEN_KEY)).toBe('apify_api_v1_token'); + expect(keyringStore.get(LEGACY_KEYRING_TOKEN_KEY)).toBe('apify_api_v1_token'); expect(readAuthFile()).toEqual({ version: 2, activeProfile: 'uid', diff --git a/test/local/lib/auth.test.ts b/test/local/lib/auth.test.ts index 0e3f4baf7..0ce3d90f1 100644 --- a/test/local/lib/auth.test.ts +++ b/test/local/lib/auth.test.ts @@ -4,7 +4,7 @@ import { ApifyApiError } from 'apify-client'; import { loginWithToken, resolveAuth } from '../../../src/lib/auth.js'; import { AUTH_FILE_PATH, CommandExitCodes } from '../../../src/lib/consts.js'; -import { getProxyPassword, getToken, setToken } from '../../../src/lib/credentials.js'; +import { getSecret } from '../../../src/lib/credentials.js'; import { getCurrentUserInfo, getLoggedClientOrThrow } from '../../../src/lib/utils.js'; import { clientState, resetApifyClientMock } from '../../__setup__/apify-client-mock.js'; import { readActiveProfile } from '../../__setup__/auth-file.js'; @@ -132,8 +132,8 @@ describe('auth', () => { it('saves the token, the proxy password and the account metadata', async () => { await loginWithToken(STORED); - expect(await getToken()).toBe(STORED); - expect(await getProxyPassword()).toBe('pw'); + expect(await getSecret('uid', 'token')).toBe(STORED); + expect(await getSecret('uid', 'proxy-password')).toBe('pw'); expect(readActiveProfile()).toMatchObject({ id: 'uid', username: 'me' }); }); @@ -149,7 +149,7 @@ describe('auth', () => { await loginWithToken(STORED); - expect(await getToken()).toBe(STORED); + expect(await getSecret('uid', 'token')).toBe(STORED); }); }); @@ -237,7 +237,7 @@ describe('auth', () => { await resolveAuth(); - expect(await getToken()).toBe(STORED); + expect(await getSecret('uid', 'token')).toBe(STORED); expect(readActiveProfile()).toMatchObject({ username: 'me' }); }); }); diff --git a/test/local/lib/credentials.test.ts b/test/local/lib/credentials.test.ts index bb46f5c63..a620d2a92 100644 --- a/test/local/lib/credentials.test.ts +++ b/test/local/lib/credentials.test.ts @@ -10,20 +10,23 @@ import { AUTH_FILE_PATH, GLOBAL_CONFIGS_FOLDER } from '../../../src/lib/consts.j import { __resetCredentialsForTests, clearKeyringSecrets, + deleteSecret, ensureMigrated, + ensureSecretsKeyed, getBackend, - getProxyPassword, - getToken, - setProxyPassword, - setToken, + getSecret, + setSecret, } from '../../../src/lib/credentials.js'; import { getLocalUserInfo } from '../../../src/lib/utils.js'; +import { TEST_USER_ID, v2AuthFile } from '../../__setup__/auth-file.js'; import { - KEYRING_PROXY_PASSWORD_KEY, - KEYRING_TOKEN_KEY, + LEGACY_KEYRING_PROXY_PASSWORD_KEY, + LEGACY_KEYRING_TOKEN_KEY, keyringFailures, + keyringProxyPasswordKey, keyringSetKeys, keyringStore, + keyringTokenKey, resetKeyringMock, } from '../../__setup__/keyring-mock.js'; @@ -46,6 +49,14 @@ const writeAuthFile = (data: Record) => { const readAuthFile = () => JSON.parse(readFileSync(AUTH_FILE_PATH(), 'utf-8')); +const readProfile = () => readAuthFile().profiles[TEST_USER_ID]; + +const writeV2AuthFile = (...args: Parameters) => + writeAuthFile(v2AuthFile(...args) as Record); + +const TOKEN_KEY = keyringTokenKey(TEST_USER_ID); +const PROXY_PASSWORD_KEY = keyringProxyPasswordKey(TEST_USER_ID); + describe('credentials', () => { beforeEach(() => { vitest.stubEnv('__APIFY_INTERNAL_TEST_AUTH_PATH__', cryptoRandomObjectId(12)); @@ -92,59 +103,81 @@ describe('credentials', () => { describe('file backend', () => { beforeEach(() => { vitest.stubEnv('APIFY_DISABLE_KEYRING', '1'); + writeV2AuthFile({}, { secretsBackend: 'file' }); + writeFileSyncSpy.mockClear(); }); - 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 token through the profile', async () => { + await setSecret(TEST_USER_ID, 'token', 'tok_123'); + expect(await getSecret(TEST_USER_ID, 'token')).toBe('tok_123'); + expect(readProfile().token).toBe('tok_123'); + expect(readAuthFile().token).toBeUndefined(); + // The profile follows the file-level choice, so it records no backend of its own. + expect(readProfile().secretsBackend).toBeUndefined(); + }); + + it('round-trips the proxy password through the profile', async () => { + await setSecret(TEST_USER_ID, 'proxy-password', 'pw_abc'); + expect(await getSecret(TEST_USER_ID, 'proxy-password')).toBe('pw_abc'); + expect(readProfile().proxy).toEqual({ password: 'pw_abc' }); + }); + + it('deleteSecret() forgets the proxy password and leaves the token alone', async () => { + await setSecret(TEST_USER_ID, 'token', 'tok_123'); + await setSecret(TEST_USER_ID, 'proxy-password', 'pw_abc'); + + await deleteSecret(TEST_USER_ID, 'proxy-password'); + + expect(readProfile().proxy).toBeUndefined(); + expect(readProfile().token).toBe('tok_123'); }); - 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('leaves another profile alone', async () => { + const file = v2AuthFile(); + file.profiles!.other = { ...file.profiles![TEST_USER_ID], token: 'tok_other' }; + writeAuthFile(file as Record); + + await setSecret(TEST_USER_ID, 'token', 'tok_123'); + expect(readAuthFile().profiles.other.token).toBe('tok_other'); }); - it('preserves other proxy fields when only the password changes', async () => { - writeAuthFile({ proxy: { password: 'old', groups: [{ name: 'g' }] } } as never); - await setProxyPassword('new'); - expect(readAuthFile().proxy).toEqual({ password: 'new', groups: [{ name: 'g' }] }); + it('does nothing when the profile is not in the file', async () => { + writeAuthFile({ version: 2, activeProfile: 'gone', profiles: {} }); + await setSecret('gone', 'token', 'tok_123'); + expect(await getSecret('gone', 'token')).toBeUndefined(); }); it('skipIfUnchanged skips the write when the stored token matches', async () => { - await setToken('tok_123'); + await setSecret(TEST_USER_ID, 'token', 'tok_123'); writeFileSyncSpy.mockClear(); - await setToken('tok_123', { skipIfUnchanged: true }); + await setSecret(TEST_USER_ID, 'token', 'tok_123', { skipIfUnchanged: true }); expect(authFileWrites()).toHaveLength(0); }); it('skipIfUnchanged skips the write when the stored proxy password matches', async () => { - await setProxyPassword('pw_abc'); + await setSecret(TEST_USER_ID, 'proxy-password', 'pw_abc'); writeFileSyncSpy.mockClear(); - await setProxyPassword('pw_abc', { skipIfUnchanged: true }); + await setSecret(TEST_USER_ID, 'proxy-password', 'pw_abc', { skipIfUnchanged: true }); expect(authFileWrites()).toHaveLength(0); }); it('skipIfUnchanged still writes when the value differs', async () => { - await setToken('tok_123'); + await setSecret(TEST_USER_ID, 'token', 'tok_123'); writeFileSyncSpy.mockClear(); - await setToken('tok_456', { skipIfUnchanged: true }); + await setSecret(TEST_USER_ID, 'token', 'tok_456', { skipIfUnchanged: true }); expect(authFileWrites()).toHaveLength(1); - expect(await getToken()).toBe('tok_456'); + expect(await getSecret(TEST_USER_ID, 'token')).toBe('tok_456'); }); it('writes auth.json with mode 0600', async () => { - await setToken('tok_123'); + await setSecret(TEST_USER_ID, 'token', 'tok_123'); expect(writeFileSyncSpy).toHaveBeenCalledWith(expect.stringContaining(AUTH_FILE_PATH()), expect.any(String), { mode: 0o600, }); }); it.skipIf(process.platform === 'win32')('creates auth.json readable only by the owner', async () => { - await setToken('tok_123'); + await setSecret(TEST_USER_ID, 'token', 'tok_123'); expect(statSync(AUTH_FILE_PATH()).mode & 0o777).toBe(0o600); }); }); @@ -154,83 +187,140 @@ describe('credentials', () => { 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(KEYRING_TOKEN_KEY)).toBe('tok_123'); + it('keys the token by user ID and keeps it out of auth.json', async () => { + await setSecret(TEST_USER_ID, 'token', 'tok_123'); + expect(await getSecret(TEST_USER_ID, 'token')).toBe('tok_123'); + expect(keyringStore.get(TOKEN_KEY)).toBe('tok_123'); + expect(keyringStore.get(LEGACY_KEYRING_TOKEN_KEY)).toBeUndefined(); 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(KEYRING_PROXY_PASSWORD_KEY)).toBe('pw_abc'); + it('keys the proxy password by user ID and keeps it out of auth.json', async () => { + await setSecret(TEST_USER_ID, 'proxy-password', 'pw_abc'); + expect(await getSecret(TEST_USER_ID, 'proxy-password')).toBe('pw_abc'); + expect(keyringStore.get(PROXY_PASSWORD_KEY)).toBe('pw_abc'); expect(existsSync(AUTH_FILE_PATH())).toBe(false); }); - it('clearKeyringSecrets() removes the token and proxy entries from the keyring', async () => { - await setToken('tok_123'); - await setProxyPassword('pw_abc'); - await clearKeyringSecrets(); - expect(await getToken()).toBeUndefined(); - expect(await getProxyPassword()).toBeUndefined(); + it('gives two accounts their own entries', async () => { + await setSecret(TEST_USER_ID, 'token', 'tok_123'); + await setSecret('other', 'token', 'tok_other'); + expect(keyringStore.get(TOKEN_KEY)).toBe('tok_123'); + expect(keyringStore.get(keyringTokenKey('other'))).toBe('tok_other'); + }); + + it('deleteSecret() removes only that account and kind', async () => { + await setSecret(TEST_USER_ID, 'proxy-password', 'pw_abc'); + await setSecret('other', 'proxy-password', 'pw_other'); + + await deleteSecret(TEST_USER_ID, 'proxy-password'); + + expect(keyringStore.get(PROXY_PASSWORD_KEY)).toBeUndefined(); + expect(keyringStore.get(keyringProxyPasswordKey('other'))).toBe('pw_other'); }); it('skipIfUnchanged skips the keyring write when the stored token matches', async () => { - await setToken('tok_123'); - await setToken('tok_123', { skipIfUnchanged: true }); - expect(keyringSetKeys.filter((key) => key === KEYRING_TOKEN_KEY)).toHaveLength(1); + await setSecret(TEST_USER_ID, 'token', 'tok_123'); + await setSecret(TEST_USER_ID, 'token', 'tok_123', { skipIfUnchanged: true }); + expect(keyringSetKeys.filter((key) => key === TOKEN_KEY)).toHaveLength(1); expect(authFileWrites()).toHaveLength(0); }); it('skipIfUnchanged skips the keyring write when the stored proxy password matches', async () => { - await setProxyPassword('pw_abc'); - await setProxyPassword('pw_abc', { skipIfUnchanged: true }); - expect(keyringSetKeys.filter((key) => key === KEYRING_PROXY_PASSWORD_KEY)).toHaveLength(1); + await setSecret(TEST_USER_ID, 'proxy-password', 'pw_abc'); + await setSecret(TEST_USER_ID, 'proxy-password', 'pw_abc', { skipIfUnchanged: true }); + expect(keyringSetKeys.filter((key) => key === PROXY_PASSWORD_KEY)).toHaveLength(1); expect(authFileWrites()).toHaveLength(0); }); - it('falls back to auth.json when the keyring token write fails', async () => { - keyringFailures.add(KEYRING_TOKEN_KEY); - await setToken('tok_123'); + it('falls back to the profile when the keyring token write fails', async () => { + writeV2AuthFile({}, { secretsBackend: 'keyring' }); + keyringFailures.add(TOKEN_KEY); + await setSecret(TEST_USER_ID, 'token', 'tok_123'); - expect(keyringStore.get(KEYRING_TOKEN_KEY)).toBeUndefined(); - expect(readAuthFile()).toEqual({ token: 'tok_123', secretsBackend: 'file' }); - expect(await getBackend()).toBe('file'); - expect(await getToken()).toBe('tok_123'); + expect(keyringStore.get(TOKEN_KEY)).toBeUndefined(); + expect(readProfile().token).toBe('tok_123'); + // Recorded on the profile. The file-level choice is left alone, so it still describes + // every account whose secrets did reach the keyring. + expect(readProfile().secretsBackend).toBe('file'); + expect(readAuthFile().secretsBackend).toBe('keyring'); + expect(await getBackend()).toBe('keyring'); + expect(await getSecret(TEST_USER_ID, 'token')).toBe('tok_123'); }); it('keeps using auth.json for later writes after a keyring failure', async () => { - keyringFailures.add(KEYRING_TOKEN_KEY); - await setToken('tok_123'); + writeV2AuthFile({}, { secretsBackend: 'keyring' }); + keyringFailures.add(TOKEN_KEY); + await setSecret(TEST_USER_ID, 'token', 'tok_123'); + + await setSecret(TEST_USER_ID, 'proxy-password', 'pw_abc'); + expect(keyringStore.get(PROXY_PASSWORD_KEY)).toBeUndefined(); + expect(readProfile().proxy).toEqual({ password: 'pw_abc' }); + }); - await setProxyPassword('pw_abc'); - expect(keyringStore.get(KEYRING_PROXY_PASSWORD_KEY)).toBeUndefined(); - expect(readAuthFile().proxy).toEqual({ password: 'pw_abc' }); + it('leaves another profile on the keyring after one profile falls back', async () => { + const file = v2AuthFile({}, { secretsBackend: 'keyring' }); + file.profiles!.other = { ...file.profiles![TEST_USER_ID] }; + writeAuthFile(file as Record); + keyringFailures.add(TOKEN_KEY); + + await setSecret(TEST_USER_ID, 'token', 'tok_123'); + await setSecret('other', 'token', 'tok_other'); + + expect(readAuthFile().profiles.other.secretsBackend).toBeUndefined(); + expect(keyringStore.get(keyringTokenKey('other'))).toBe('tok_other'); + expect(await getSecret('other', 'token')).toBe('tok_other'); + expect(await getSecret(TEST_USER_ID, 'token')).toBe('tok_123'); }); - it('falls back to auth.json when the keyring proxy password write fails', async () => { - keyringFailures.add(KEYRING_PROXY_PASSWORD_KEY); - await setProxyPassword('pw_abc'); + it('falls back to the profile when the keyring proxy password write fails', async () => { + writeV2AuthFile({}, { secretsBackend: 'keyring' }); + keyringFailures.add(PROXY_PASSWORD_KEY); + await setSecret(TEST_USER_ID, 'proxy-password', 'pw_abc'); - expect(keyringStore.get(KEYRING_PROXY_PASSWORD_KEY)).toBeUndefined(); - expect(readAuthFile()).toEqual({ proxy: { password: 'pw_abc' }, secretsBackend: 'file' }); - expect(await getProxyPassword()).toBe('pw_abc'); + expect(keyringStore.get(PROXY_PASSWORD_KEY)).toBeUndefined(); + expect(readProfile().proxy).toEqual({ password: 'pw_abc' }); + expect(readProfile().secretsBackend).toBe('file'); + expect(await getSecret(TEST_USER_ID, 'proxy-password')).toBe('pw_abc'); }); }); describe('clearKeyringSecrets()', () => { - it('clears the keyring token entry even when APIFY_DISABLE_KEYRING=1 is set at logout time', async () => { + beforeEach(() => { vitest.stubEnv('APIFY_DISABLE_KEYRING', ''); - await setToken('tok_123'); - expect(keyringStore.get(KEYRING_TOKEN_KEY)).toBe('tok_123'); + }); + + it('removes the profile entries and the fixed-name ones left from before', async () => { + keyringStore.set(LEGACY_KEYRING_TOKEN_KEY, 'tok_old'); + keyringStore.set(LEGACY_KEYRING_PROXY_PASSWORD_KEY, 'pw_old'); + await setSecret(TEST_USER_ID, 'token', 'tok_123'); + await setSecret(TEST_USER_ID, 'proxy-password', 'pw_abc'); + + await clearKeyringSecrets(TEST_USER_ID); + + expect(keyringStore.size).toBe(0); + }); + + it('leaves other profiles alone', async () => { + await setSecret(TEST_USER_ID, 'token', 'tok_123'); + await setSecret('other', 'token', 'tok_other'); + + await clearKeyringSecrets(TEST_USER_ID); + + expect(keyringStore.get(TOKEN_KEY)).toBeUndefined(); + expect(keyringStore.get(keyringTokenKey('other'))).toBe('tok_other'); + }); + + it('clears the keyring entries even when APIFY_DISABLE_KEYRING=1 is set at logout time', async () => { + await setSecret(TEST_USER_ID, 'token', 'tok_123'); + expect(keyringStore.get(TOKEN_KEY)).toBe('tok_123'); __resetCredentialsForTests(); vitest.stubEnv('APIFY_DISABLE_KEYRING', '1'); expect(await getBackend()).toBe('file'); - await clearKeyringSecrets(); - expect(keyringStore.get(KEYRING_TOKEN_KEY)).toBeUndefined(); + await clearKeyringSecrets(TEST_USER_ID); + expect(keyringStore.get(TOKEN_KEY)).toBeUndefined(); }); }); @@ -254,7 +344,7 @@ describe('credentials', () => { vitest.stubEnv('APIFY_DISABLE_KEYRING', ''); writeAuthFile({ token: 'tok', proxy: { password: 'pw' }, secretsBackend: 'keyring' }); await ensureMigrated(); - expect(keyringStore.get(KEYRING_TOKEN_KEY)).toBeUndefined(); + expect(keyringStore.get(LEGACY_KEYRING_TOKEN_KEY)).toBeUndefined(); expect(readAuthFile()).toEqual({ token: 'tok', proxy: { password: 'pw' }, secretsBackend: 'keyring' }); }); @@ -278,8 +368,8 @@ describe('credentials', () => { vitest.stubEnv('APIFY_DISABLE_KEYRING', ''); writeAuthFile({ token: 'tok', proxy: { password: 'pw' }, username: 'u' }); await ensureMigrated(); - expect(keyringStore.get(KEYRING_TOKEN_KEY)).toBe('tok'); - expect(keyringStore.get(KEYRING_PROXY_PASSWORD_KEY)).toBe('pw'); + expect(keyringStore.get(LEGACY_KEYRING_TOKEN_KEY)).toBe('tok'); + expect(keyringStore.get(LEGACY_KEYRING_PROXY_PASSWORD_KEY)).toBe('pw'); const file = readAuthFile(); expect(file.token).toBeUndefined(); expect(file.proxy).toBeUndefined(); @@ -291,7 +381,7 @@ describe('credentials', () => { vitest.stubEnv('APIFY_DISABLE_KEYRING', ''); writeAuthFile({ token: 'tok', proxy: { password: 'pw', groups: [{ name: 'g' }] }, username: 'u' }); await ensureMigrated(); - expect(keyringStore.get(KEYRING_PROXY_PASSWORD_KEY)).toBe('pw'); + expect(keyringStore.get(LEGACY_KEYRING_PROXY_PASSWORD_KEY)).toBe('pw'); const file = readAuthFile(); expect(file.proxy).toEqual({ groups: [{ name: 'g' }] }); expect(file.secretsBackend).toBe('keyring'); @@ -301,7 +391,7 @@ describe('credentials', () => { vitest.stubEnv('APIFY_DISABLE_KEYRING', ''); writeAuthFile({ proxy: { password: 'pw' }, username: 'u' }); await ensureMigrated(); - expect(keyringStore.get(KEYRING_PROXY_PASSWORD_KEY)).toBe('pw'); + expect(keyringStore.get(LEGACY_KEYRING_PROXY_PASSWORD_KEY)).toBe('pw'); const file = readAuthFile(); expect(file.proxy).toBeUndefined(); expect(file.username).toBe('u'); @@ -319,7 +409,7 @@ describe('credentials', () => { it('falls back to file backend when the proxy keyring write fails after token succeeds', async () => { vitest.stubEnv('APIFY_DISABLE_KEYRING', ''); - keyringFailures.add(KEYRING_PROXY_PASSWORD_KEY); + keyringFailures.add(LEGACY_KEYRING_PROXY_PASSWORD_KEY); writeAuthFile({ token: 'tok', proxy: { password: 'pw' }, username: 'u' }); await ensureMigrated(); const file = readAuthFile(); @@ -342,7 +432,114 @@ describe('credentials', () => { }); }); + describe('ensureSecretsKeyed()', () => { + it('moves keyring entries off the fixed names onto the user ID', async () => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', ''); + writeV2AuthFile({}, { secretsBackend: 'keyring' }); + keyringStore.set(LEGACY_KEYRING_TOKEN_KEY, 'tok'); + keyringStore.set(LEGACY_KEYRING_PROXY_PASSWORD_KEY, 'pw'); + + await ensureSecretsKeyed(); + + expect(keyringStore.get(TOKEN_KEY)).toBe('tok'); + expect(keyringStore.get(PROXY_PASSWORD_KEY)).toBe('pw'); + expect(keyringStore.get(LEGACY_KEYRING_TOKEN_KEY)).toBeUndefined(); + expect(keyringStore.get(LEGACY_KEYRING_PROXY_PASSWORD_KEY)).toBeUndefined(); + }); + + it('moves top-level file secrets into the profile', async () => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', '1'); + writeV2AuthFile({}, { secretsBackend: 'file', token: 'tok', proxy: { password: 'pw' } }); + + await ensureSecretsKeyed(); + + expect(readProfile()).toMatchObject({ token: 'tok', proxy: { password: 'pw' } }); + const file = readAuthFile(); + expect(file.token).toBeUndefined(); + expect(file.proxy).toBeUndefined(); + expect(file.secretsBackend).toBe('file'); + }); + + it('drops secrets it has no user ID to file under', async () => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', ''); + writeAuthFile({ version: 2, profiles: {}, secretsBackend: 'keyring', token: 'tok' }); + keyringStore.set(LEGACY_KEYRING_TOKEN_KEY, 'tok_kr'); + + await ensureSecretsKeyed(); + + expect(readAuthFile().token).toBeUndefined(); + expect(keyringStore.get(LEGACY_KEYRING_TOKEN_KEY)).toBeUndefined(); + }); + + it('drops the legacy entries even under APIFY_DISABLE_KEYRING=1', async () => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', '1'); + writeAuthFile({ version: 2, profiles: {}, secretsBackend: 'keyring' }); + keyringStore.set(LEGACY_KEYRING_TOKEN_KEY, 'tok_kr'); + + await ensureSecretsKeyed(); + + expect(keyringStore.get(LEGACY_KEYRING_TOKEN_KEY)).toBeUndefined(); + }); + + it('is a no-op on a file whose secrets are already keyed', async () => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', '1'); + writeV2AuthFile({ token: 'tok' }, { secretsBackend: 'file' }); + writeFileSyncSpy.mockClear(); + + await ensureSecretsKeyed(); + + expect(authFileWrites()).toHaveLength(0); + expect(readProfile().token).toBe('tok'); + }); + + it('is a no-op on a file the shape migration has not reached', async () => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', '1'); + writeAuthFile({ id: 'uid', token: 'tok' }); + writeFileSyncSpy.mockClear(); + + await ensureSecretsKeyed(); + + expect(authFileWrites()).toHaveLength(0); + expect(readAuthFile().token).toBe('tok'); + }); + + it('moves the profile to the file when the keyring write fails mid-migration', async () => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', ''); + writeV2AuthFile({}, { secretsBackend: 'keyring' }); + keyringStore.set(LEGACY_KEYRING_TOKEN_KEY, 'tok'); + keyringStore.set(LEGACY_KEYRING_PROXY_PASSWORD_KEY, 'pw'); + keyringFailures.add(TOKEN_KEY); + + await ensureSecretsKeyed(); + + // Both secrets land in the file: the fallback holds for the rest of the loop. + expect(readProfile()).toMatchObject({ token: 'tok', proxy: { password: 'pw' }, secretsBackend: 'file' }); + expect(readAuthFile().secretsBackend).toBe('keyring'); + expect(keyringStore.size).toBe(0); + }); + + it('is memoized within a process', async () => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', '1'); + writeV2AuthFile({}, { secretsBackend: 'file', token: 'tok' }); + await ensureSecretsKeyed(); + expect(readProfile().token).toBe('tok'); + + writeV2AuthFile({}, { secretsBackend: 'file', token: 'tok2' }); + await ensureSecretsKeyed(); + expect(readAuthFile().token).toBe('tok2'); + }); + }); + describe('getLocalUserInfo()', () => { + it('on file backend, reads the token and proxy password from the profile', async () => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', '1'); + writeV2AuthFile({ token: 'tok', proxy: { password: 'pw' } }, { secretsBackend: 'file' }); + + const info = await getLocalUserInfo(); + expect(info.token).toBe('tok'); + expect(info.proxy).toEqual({ password: 'pw' }); + }); + it('on file backend, keeps the proxy password and drops the groups nothing reads', async () => { vitest.stubEnv('APIFY_DISABLE_KEYRING', '1'); writeAuthFile({ @@ -358,8 +555,8 @@ describe('credentials', () => { it('on keyring backend, overlays token and proxy password from keyring', async () => { vitest.stubEnv('APIFY_DISABLE_KEYRING', ''); - keyringStore.set(KEYRING_TOKEN_KEY, 'tok_kr'); - keyringStore.set(KEYRING_PROXY_PASSWORD_KEY, 'pw_kr'); + keyringStore.set(LEGACY_KEYRING_TOKEN_KEY, 'tok_kr'); + keyringStore.set(LEGACY_KEYRING_PROXY_PASSWORD_KEY, 'pw_kr'); writeAuthFile({ username: 'me', id: 'uid', secretsBackend: 'keyring' }); const info = await getLocalUserInfo(); expect(info.token).toBe('tok_kr'); @@ -371,16 +568,23 @@ describe('credentials', () => { expect(await getLocalUserInfo()).toEqual({}); }); - it('on file backend, throws when a token is stored without user metadata', async () => { + it('on file backend, reports logged out for a token stored without user metadata', async () => { vitest.stubEnv('APIFY_DISABLE_KEYRING', '1'); writeAuthFile({ token: 'tok', secretsBackend: 'file' }); - await expect(getLocalUserInfo()).rejects.toThrow('Stale credentials found without user metadata'); + + expect(await getLocalUserInfo()).toEqual({}); + // The secret is dropped rather than left unreachable, so the next command asks for a login. + expect(readAuthFile().token).toBeUndefined(); }); - it('on keyring backend, throws when the keyring holds a token but auth.json is gone', async () => { + it('on keyring backend, reports logged out when the keyring holds a token but auth.json is gone', async () => { vitest.stubEnv('APIFY_DISABLE_KEYRING', ''); - keyringStore.set(KEYRING_TOKEN_KEY, 'tok_kr'); - await expect(getLocalUserInfo()).rejects.toThrow('Stale credentials found without user metadata'); + keyringStore.set(LEGACY_KEYRING_TOKEN_KEY, 'tok_kr'); + + expect(await getLocalUserInfo()).toEqual({}); + // auth.json is the only index of the keyring, so a hand-deleted file strands the entry. + // Reaching for it on a machine with no account would touch the keyring on every command. + expect(keyringStore.get(LEGACY_KEYRING_TOKEN_KEY)).toBe('tok_kr'); }); }); });