diff --git a/src/commands/auth/logout.ts b/src/commands/auth/logout.ts index 936403e80..85c2beb14 100644 --- a/src/commands/auth/logout.ts +++ b/src/commands/auth/logout.ts @@ -1,10 +1,10 @@ import { APIFY_ENV_VARS } from '@apify/consts'; +import { assertSupportedAuthFileVersion, getActiveProfileId, removeActiveProfile } from '../../lib/auth-file.js'; import { getEnvToken } from '../../lib/auth.js'; import { ApifyCommand } from '../../lib/command-framework/apify-command.js'; import { AUTH_FILE_PATH } from '../../lib/consts.js'; import { clearKeyringSecrets } from '../../lib/credentials.js'; -import { rimrafPromised } from '../../lib/files.js'; import { updateUserId } from '../../lib/hooks/telemetry/useTelemetryState.js'; import { success, warning } from '../../lib/outputs.js'; import { tildify } from '../../lib/utils.js'; @@ -28,8 +28,12 @@ export class AuthLogoutCommand extends ApifyCommand { static override docsUrl = 'https://docs.apify.com/cli/docs/reference#apify-logout'; async run() { - await clearKeyringSecrets(); - await rimrafPromised(AUTH_FILE_PATH()); + // The refusal comes first, so a file this CLI must not touch leaves a logged-in state + // rather than half a logout. The keyring goes next: `auth.json` is the only index of what + // the keyring holds, so removing the profile first would strand its entries. + assertSupportedAuthFileVersion(); + await clearKeyringSecrets(getActiveProfileId()); + removeActiveProfile(); await updateUserId(null); diff --git a/src/lib/auth-file.ts b/src/lib/auth-file.ts new file mode 100644 index 000000000..fa7ea5ba3 --- /dev/null +++ b/src/lib/auth-file.ts @@ -0,0 +1,296 @@ +import { copyFileSync, existsSync, readFileSync, renameSync, rmSync, writeFileSync } from 'node:fs'; + +import { cryptoRandomObjectId } from '@apify/utilities'; + +import { AUTH_FILE_PATH } from './consts.js'; +import type { CredentialsBackend, SecretKind } from './credentials.js'; +import { ensureApifyDirectory } from './files.js'; +import { cliDebugPrint } from './utils/cliDebugPrint.js'; + +export const AUTH_FILE_VERSION = 2; + +/** The way back to a CLI that only reads the v1 shape. */ +export const AUTH_BACKUP_FILE_PATH = () => `${AUTH_FILE_PATH()}.v1.bak`; + +/** + * One account. Keyed by user ID in {@link AuthFile.profiles}, so renaming a profile can never + * orphan the secret that key names. + */ +export interface AuthProfile { + username?: string; + /** Human label for `--profile `. Unused until profiles get names. */ + name: string | null; + /** Set means the profile is an organization rather than a personal account. */ + organizationOwnerUserId?: string; + /** How the token was obtained. Unused until the device flow lands. */ + authMethod: 'token'; + /** When the access token expires. Unused until the device flow lands. */ + expiresAt: string | null; + /** Whether a refresh token came with the access token. Unused until the device flow lands. */ + hasRefreshToken: boolean; + /** File backend only. The keyring backend keeps this in the OS store instead. */ + token?: string; + /** File backend only. The keyring backend keeps this in the OS store instead. */ + proxy?: { password?: string }; +} + +/** + * `auth.json` as it sits on disk. 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; + activeProfile?: string; + profiles?: Record; + secretsBackend?: CredentialsBackend; + token?: string; + proxy?: { password?: string; [k: string]: unknown }; + [k: string]: unknown; +} + +export interface ActiveProfileLookup { + profile?: AuthProfile & { id: string }; + /** Set when `activeProfile` names a profile the file does not contain. */ + missingProfile?: string; +} + +let migrationPromise: Promise | undefined; + +/** Test-only: let each test run the v2 migration again. */ +export function __resetAuthFileForTests() { + migrationPromise = undefined; +} + +/** `null` tells a corrupt file from an absent one, which the migration must not overwrite. */ +function parseAuthFile(): AuthFile | null { + if (!existsSync(AUTH_FILE_PATH())) return {}; + + try { + return JSON.parse(readFileSync(AUTH_FILE_PATH(), 'utf-8')) as AuthFile; + } catch { + return null; + } +} + +/** The parsed file, or an empty object when it is missing or unreadable. */ +export function readAuthFile(): AuthFile { + return parseAuthFile() ?? {}; +} + +/** + * Atomic write: a temp file next to the target, then a rename. Two CLI processes can run at once, + * and a half-written auth.json reads as logged out. + */ +export function writeAuthFile(data: AuthFile) { + const path = AUTH_FILE_PATH(); + ensureApifyDirectory(path); + + const tempPath = `${path}.tmp-${cryptoRandomObjectId(8)}`; + + try { + writeFileSync(tempPath, JSON.stringify(data, null, '\t'), { mode: 0o600 }); + renameSync(tempPath, path); + } catch (err) { + rmSync(tempPath, { force: true }); + throw err; + } +} + +/** The one account a v1 file described, as a profile. */ +function v1Profile(file: AuthFile): AuthProfile { + return { + ...(typeof file.username === 'string' ? { username: file.username } : {}), + name: null, + ...(typeof file.organizationOwnerUserId === 'string' + ? { organizationOwnerUserId: file.organizationOwnerUserId } + : {}), + authMethod: 'token', + expiresAt: null, + hasRefreshToken: false, + }; +} + +/** + * A v1 file described one account, so everything in it belongs to one profile. `email`, `plan`, + * `effectivePlatformFeatures`, `isPaying`, `createdAt` and `proxy.groups` are dropped — nothing in + * the CLI reads them. + */ +function toV2(file: AuthFile): 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. 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); + } + + if (file.secretsBackend) migrated.secretsBackend = file.secretsBackend; + if (typeof file.token === 'string') migrated.token = file.token; + if (typeof file.proxy?.password === 'string') migrated.proxy = { password: file.proxy.password }; + + return migrated; +} + +/** Never overwrites an existing backup: the first one is the file the user started with. */ +function backUpV1File() { + if (existsSync(AUTH_BACKUP_FILE_PATH())) return; + copyFileSync(AUTH_FILE_PATH(), AUTH_BACKUP_FILE_PATH()); +} + +async function migrateToV2(): Promise { + migrationPromise ??= (async () => { + try { + const file = parseAuthFile(); + + // A corrupt file is left alone: readers already treat it as logged out, and rewriting + // it would destroy what the user could still recover by hand. + if (!file) return; + // A numbered version is either already current or from another CLI; either way there + // is nothing to migrate. `assertSupportedAuthFileVersion` reports a newer one. + if (typeof file.version === 'number') return; + if (Object.keys(file).length === 0) return; + + backUpV1File(); + writeAuthFile(toV2(file)); + } catch (err) { + cliDebugPrint('auth-file', 'migration to v2 failed', err); + } + })(); + + return migrationPromise; +} + +/** + * A file from a newer CLI is not something to guess at — migrating it backwards would drop + * whatever that version stores. + */ +export function assertSupportedAuthFileVersion() { + const { version } = readAuthFile(); + + if (typeof version === 'number' && version > AUTH_FILE_VERSION) { + throw new Error( + `Your credentials in ${AUTH_FILE_PATH()} were written by a newer Apify CLI (auth file version ${version}, this one reads ${AUTH_FILE_VERSION}). Upgrade the CLI to use them.`, + ); + } +} + +/** + * Brings `auth.json` to the v2 profile shape and refuses a file a newer CLI wrote. Runs after + * `ensureMigrated()`, which moves v1 secrets into the keyring; the two steps stay separate so a + * keyring failure and a shape failure cannot mask each other. + * + * The migration itself is idempotent, single-flight and never throws — it must not block a command. + */ +export async function ensureAuthFileCurrent(): Promise { + await migrateToV2(); + assertSupportedAuthFileVersion(); +} + +/** + * The active profile with its user ID. Reads a v1 file too, so a command that runs before the + * migration still finds the account. + */ +export function lookUpActiveProfile(): ActiveProfileLookup { + const file = readAuthFile(); + + if (file.version !== AUTH_FILE_VERSION) { + return typeof file.id === 'string' ? { profile: { id: file.id, ...v1Profile(file) } } : {}; + } + + if (!file.activeProfile) return {}; + + const profile = file.profiles?.[file.activeProfile]; + if (!profile) return { missingProfile: file.activeProfile }; + + return { profile: { id: file.activeProfile, ...profile } }; +} + +/** The active profile, or `undefined` when nothing usable is stored. */ +export function getActiveProfile(): (AuthProfile & { id: string }) | undefined { + return lookUpActiveProfile().profile; +} + +/** + * 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) { + return typeof file.id === 'string' ? file.id : undefined; + } + + return file.activeProfile; +} + +/** Where the file backend keeps a secret inside a profile. */ +function profileSecret(profile: AuthProfile, kind: SecretKind): string | undefined { + return kind === 'token' ? profile.token : profile.proxy?.password; +} + +/** 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]; + return profile ? profileSecret(profile, kind) : undefined; +} + +/** + * Stores a file-backend secret on the profile and marks the file as the secrets backend. 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) { + const file = readAuthFile(); + const profile = file.profiles?.[userId]; + if (!profile) return; + + if (kind === 'token') { + profile.token = value; + } else { + profile.proxy = { ...profile.proxy, password: value }; + } + + file.secretsBackend = 'file'; + writeAuthFile(file); +} + +/** + * Stores one account and makes it active, replacing whatever was there. Nothing puts a second + * profile in the file yet, so `apify login` owns all of it. + */ +export function setActiveProfile(userId: string, profile: AuthProfile, secretsBackend: CredentialsBackend) { + assertSupportedAuthFileVersion(); + + writeAuthFile({ + version: AUTH_FILE_VERSION, + activeProfile: userId, + profiles: { [userId]: profile }, + secretsBackend, + }); +} + +/** + * Drops the active profile together with the secrets stored beside it. The file and the v1 backup + * go away once no profile is left, so logging out leaves no token on disk. + */ +export function removeActiveProfile() { + assertSupportedAuthFileVersion(); + + const file = readAuthFile(); + const active = file.version === AUTH_FILE_VERSION ? file.activeProfile : undefined; + + if (active && file.profiles) delete file.profiles[active]; + delete file.activeProfile; + delete file.token; + delete file.proxy; + + if (Object.keys(file.profiles ?? {}).length === 0) { + rmSync(AUTH_FILE_PATH(), { force: true }); + rmSync(AUTH_BACKUP_FILE_PATH(), { force: true }); + return; + } + + writeAuthFile(file); +} diff --git a/src/lib/auth.ts b/src/lib/auth.ts index 3923ad938..467868038 100644 --- a/src/lib/auth.ts +++ b/src/lib/auth.ts @@ -1,4 +1,4 @@ -import { existsSync, writeFileSync } from 'node:fs'; +import { existsSync } from 'node:fs'; import process from 'node:process'; import { ApifyClient, type ApifyClientOptions } from 'apify-client'; @@ -6,9 +6,9 @@ import { AxiosHeaders } from 'axios'; import { APIFY_ENV_VARS } from '@apify/consts'; +import { getActiveProfileId, setActiveProfile } from './auth-file.js'; import { APIFY_CLIENT_DEFAULT_HEADERS, AUTH_FILE_PATH } from './consts.js'; -import { ensureMigrated, getBackend, getToken, setProxyPassword, setToken } from './credentials.js'; -import { ensureApifyDirectory } from './files.js'; +import { clearKeyringSecrets, ensureCredentialsCurrent, getBackend, getSecret, setSecret } from './credentials.js'; import { warning } from './outputs.js'; import { cliDebugPrint } from './utils/cliDebugPrint.js'; @@ -85,9 +85,10 @@ export const resolveAuth = async (explicitToken?: string): Promise = { ...userInfo, secretsBackend: await getBackend() }; - delete fileContents.token; - if (fileContents.proxy && typeof fileContents.proxy === 'object') { - const { password: _password, ...rest } = fileContents.proxy as { password?: string }; - if (Object.keys(rest).length > 0) { - fileContents.proxy = rest; - } else { - delete fileContents.proxy; - } + if (!userInfo.id) { + throw new Error('The Apify API returned no user ID for this token, so the login cannot be stored.'); + } + + // `auth.json` is the only index of what the keyring holds, so the outgoing account's entries + // have to go before its ID leaves the file. + const previousUserId = getActiveProfileId(); + if (previousUserId && previousUserId !== userInfo.id) { + await clearKeyringSecrets(previousUserId); } - ensureApifyDirectory(AUTH_FILE_PATH()); - writeFileSync(AUTH_FILE_PATH(), JSON.stringify(fileContents, null, '\t'), { mode: 0o600 }); + // The profile is keyed by user ID, and it replaces whatever was stored rather than merging + // into it, so fields the new account does not have cannot linger from the old one. + const { organizationOwnerUserId } = userInfo as { organizationOwnerUserId?: string }; + setActiveProfile( + userInfo.id, + { + username: userInfo.username, + name: null, + ...(organizationOwnerUserId ? { organizationOwnerUserId } : {}), + authMethod: 'token', + expiresAt: null, + hasRefreshToken: false, + }, + await getBackend(), + ); // Written after the metadata file, which would otherwise clobber them on the file backend. // `skipIfUnchanged` avoids a macOS Keychain prompt when the value already matches. - await setToken(token, { skipIfUnchanged: true }); + await setSecret(userInfo.id, 'token', token, { skipIfUnchanged: true }); const proxyPassword = userInfo.proxy?.password; if (proxyPassword) { - await setProxyPassword(proxyPassword, { skipIfUnchanged: true }); + await setSecret(userInfo.id, 'proxy-password', proxyPassword, { skipIfUnchanged: true }); } return apifyClient; diff --git a/src/lib/credentials.ts b/src/lib/credentials.ts index f4a8e2d1d..6d673a86e 100644 --- a/src/lib/credentials.ts +++ b/src/lib/credentials.ts @@ -1,17 +1,43 @@ -import { existsSync, readFileSync, writeFileSync } from 'node:fs'; import process from 'node:process'; -import { AUTH_FILE_PATH } from './consts.js'; -import { ensureApifyDirectory } from './files.js'; +import type { AuthFile } from './auth-file.js'; +import { + AUTH_FILE_VERSION, + ensureAuthFileCurrent, + readAuthFile, + 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']; + +/** + * 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) { + 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) { + return { service: KEYRING_SERVICE, account: kind }; +} + interface KeyringEntry { getPassword(): string | null; setPassword(password: string): void; @@ -22,22 +48,17 @@ interface KeyringModule { Entry: new (service: string, account: string) => KeyringEntry; } -interface StoredAuthFile { - token?: string; - proxy?: { password?: string; [k: string]: unknown }; - secretsBackend?: CredentialsBackend; - [k: string]: unknown; -} - 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 { @@ -104,82 +125,69 @@ function downgradeBackendToFile() { backendPromise = Promise.resolve('file'); } -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 }); +interface KeyringKey { + service: string; + account: string; } -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 { +/** One account's secret of the given kind, from whichever backend holds it. */ +export async function getSecret(userId: string, kind: SecretKind): 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; + if (backend === '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 { +export async function setSecret( + userId: string, + kind: SecretKind, + value: 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 (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) { cliDebugPrint('credentials', 'keyring write failed; falling back to file', err); @@ -187,45 +195,24 @@ export async function setToken(token: string, opts: { skipIfUnchanged?: boolean } } - 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); - 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); } /** - * 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)); + } } /** @@ -254,8 +241,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(); @@ -279,3 +268,118 @@ export async function ensureMigrated(): Promise { })(); return migrationPromise; } + +/** + * Drops secrets there is no user ID to file under. `auth.json.v1.bak` still holds whatever the v1 + * file had, so the way back is a re-login rather than a lost account. + * + * The keyring deletes run whatever the current backend is, for the same reason + * {@link clearKeyringSecrets} does them: toggling `APIFY_DISABLE_KEYRING=1` would otherwise leave + * entries behind that nothing records and no command can reach. + */ +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 downgrades the backend for the rest of the process, so + // the secrets after it belong in the file rather than under a name nothing will read. + if ((await getBackend()) === '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); + downgradeBackendToFile(); + } + } + + writeProfileSecret(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 = { ...profile.proxy, password: proxyPassword }; + + delete file.token; + delete file.proxy; + file.secretsBackend = 'file'; + 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 backend = await getBackend(); + const userId = file.activeProfile; + + if (!userId) { + await dropUnkeyedSecrets(file); + return; + } + + if (backend === 'keyring') { + await keyKeyringSecrets(userId); + return; + } + + keyFileSecrets(userId, file); + } catch (err) { + cliDebugPrint('credentials', 'keying secrets by user failed', err); + } + })(); + + return keyingPromise; +} + +/** + * Every storage migration, in the one order that works: v1 secrets out of `auth.json`, then the v2 + * profile shape, then secrets keyed by user ID — the last step needs the user ID the second writes. + * Throws only for a file a newer CLI wrote. + */ +export async function ensureCredentialsCurrent(): Promise { + await ensureMigrated(); + await ensureAuthFileCurrent(); + await ensureSecretsKeyed(); +} diff --git a/src/lib/hooks/useCLIMetadata.ts b/src/lib/hooks/useCLIMetadata.ts index a90a8c30d..754232a12 100644 --- a/src/lib/hooks/useCLIMetadata.ts +++ b/src/lib/hooks/useCLIMetadata.ts @@ -7,8 +7,8 @@ export const DEVELOPMENT_VERSION_MARKER = '0.0.0'; export const DEVELOPMENT_HASH_MARKER = '0000000'; // These values are replaced with the actual values when building the CLI -const CLI_VERSION = DEVELOPMENT_VERSION_MARKER; -const CLI_HASH = DEVELOPMENT_HASH_MARKER; +const CLI_VERSION = '1.10.1'; +const CLI_HASH = '1e7e56cb213537bb4bd5e7878e574191d732e106'; export type InstallMethod = 'npm' | 'pnpm' | 'homebrew' | 'volta' | 'bundle' | 'bun'; diff --git a/src/lib/hooks/useRentalSunsetNotice.ts b/src/lib/hooks/useRentalSunsetNotice.ts index 9d2467b74..57d3dea41 100644 --- a/src/lib/hooks/useRentalSunsetNotice.ts +++ b/src/lib/hooks/useRentalSunsetNotice.ts @@ -1,19 +1,17 @@ -import { readFile } from 'node:fs/promises'; import process from 'node:process'; import axios from 'axios'; import chalk from 'chalk'; import { isCI } from 'ci-info'; +import { getActiveProfile } from '../auth-file.js'; import { APIFY_CLIENT_DEFAULT_HEADERS, - AUTH_FILE_PATH, CHECK_RENTAL_ACTORS_EVERY_MILLIS, RENTAL_SUNSET_NOTICE_EVERY_MILLIS, RENTAL_SUNSET_NOTICE_UNTIL, } from '../consts.js'; import { simpleLog, warning } from '../outputs.js'; -import type { AuthJSON } from '../types.js'; import { cliDebugPrint } from '../utils/cliDebugPrint.js'; import { useCLIMetadata } from './useCLIMetadata.js'; import { type LatestState, updateLocalState, useLocalState } from './useLocalState.js'; @@ -92,18 +90,12 @@ export function renderRentalSunsetNotice(rentalActorCount: number) { } /** - * Reads the logged in username straight from auth.json instead of going through `getLocalUserInfo`, - * which resolves the token from the OS keyring and would trigger a keychain prompt on commands that - * do not need authentication at all. + * Reads the username straight out of auth.json instead of going through `getLocalUserInfo`, which + * resolves the token from the OS keyring and would trigger a keychain prompt on commands that do + * not need authentication at all. */ -async function getLocalUsername() { - try { - const raw = await readFile(AUTH_FILE_PATH(), 'utf-8'); - - return (JSON.parse(raw) as AuthJSON).username; - } catch { - return undefined; - } +function getLocalUsername() { + return getActiveProfile()?.username; } /** @@ -207,7 +199,7 @@ export async function useRentalSunsetNotice() { return; } - const username = await getLocalUsername(); + const username = getLocalUsername(); if (!username) { cliDebugPrint('useRentalSunsetNotice', 'Not logged in, skipping the check'); diff --git a/src/lib/types.ts b/src/lib/types.ts index f639688ed..cbb83cd7d 100644 --- a/src/lib/types.ts +++ b/src/lib/types.ts @@ -4,7 +4,6 @@ export interface AuthJSON { token?: string; id?: string; username?: string; - email?: string; proxy?: { password: string; }; diff --git a/src/lib/utils.ts b/src/lib/utils.ts index 4dc680231..dbe0a70e7 100644 --- a/src/lib/utils.ts +++ b/src/lib/utils.ts @@ -32,6 +32,7 @@ import { SOURCE_FILE_FORMATS, } from '@apify/consts'; +import { lookUpActiveProfile } from './auth-file.js'; import { describeAuthFailure, getApifyClientOptions, resolveAuth } from './auth.js'; import { AUTH_FILE_PATH, @@ -41,7 +42,7 @@ import { MINIMUM_SUPPORTED_PYTHON_VERSION, SUPPORTED_NODEJS_VERSION, } from './consts.js'; -import { ensureMigrated, getBackend, getProxyPassword, getToken } from './credentials.js'; +import { ensureCredentialsCurrent, 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'; @@ -84,34 +85,33 @@ export const getLocalRequestQueuePath = (storeId?: string) => { }; /** - * 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. + * The active profile in the flat shape the CLI consumes, or an empty object when nothing is + * stored. Secrets come from whichever backend holds them; the metadata comes from auth.json. */ export const getLocalUserInfo = async (): Promise => { - await ensureMigrated(); + await ensureCredentialsCurrent(); - let result: AuthJSON = {}; - try { - const raw = await readFile(AUTH_FILE_PATH(), 'utf-8'); - result = JSON.parse(raw) as AuthJSON; - } catch { - // auth.json may not exist yet (fresh keyring-only state); fall through + const { profile, missingProfile } = lookUpActiveProfile(); + + // A profile the file does not hold is reported rather than swallowed: the commands that build + // `/` lookups would otherwise fail with a misleading "not found". + if (missingProfile) { + throw new Error( + `Your active profile "${missingProfile}" is missing from ${AUTH_FILE_PATH()}. Run "apify login" to log in again.`, + ); } - if ((await getBackend()) === 'keyring') { - const token = await getToken(); - if (token) result.token = token; + if (!profile) return {}; - const proxyPassword = await getProxyPassword(); - if (proxyPassword) result.proxy = { ...result.proxy, password: proxyPassword }; - } + const result: AuthJSON = { id: profile.id }; + if (profile.username) result.username = profile.username; + if (profile.organizationOwnerUserId) result.organizationOwnerUserId = profile.organizationOwnerUserId; - const hasUserMetadata = !!(result.username || result.id); - const isComplete = hasUserMetadata || !!result.token; - if (!isComplete) return {}; - if (!hasUserMetadata) { - throw new Error('Stale credentials found without user metadata. Please run "apify login" again.'); - } + 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 new file mode 100644 index 000000000..2c8dea411 --- /dev/null +++ b/test/__setup__/auth-file.ts @@ -0,0 +1,58 @@ +/** Reading `auth.json` in tests, so no test has to know the profile shape by hand. */ + +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; +} + +/** The active profile with its user ID, read straight off disk rather than through the CLI. */ +export function readActiveProfile(): (AuthProfile & { id: string }) | undefined { + const { activeProfile, profiles } = readAuthFile(); + if (!activeProfile) return undefined; + + const profile = profiles?.[activeProfile]; + 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, + ...profile, + }, + }, + ...rest, + }; +} + +/** A v1 `auth.json`, the shape every CLI before the profile migration wrote. */ +export function v1AuthFile(overrides: Record = {}) { + return { + id: 'uid', + username: 'me', + email: 'me@example.com', + token: 'apify_api_v1_token', + proxy: { password: 'pw', groups: [{ name: 'g' }] }, + plan: { id: 'FREE' }, + isPaying: false, + createdAt: '2021-03-27T22:27:56.809Z', + ...overrides, + }; +} diff --git a/test/__setup__/hooks/useAuthSetup.ts b/test/__setup__/hooks/useAuthSetup.ts index 5fefdbe00..a375c313b 100644 --- a/test/__setup__/hooks/useAuthSetup.ts +++ b/test/__setup__/hooks/useAuthSetup.ts @@ -6,6 +6,7 @@ import { isCI } from 'ci-info'; import { cryptoRandomObjectId } from '@apify/utilities'; import { LoginCommand } from '../../../src/commands/login.js'; +import { __resetAuthFileForTests } from '../../../src/lib/auth-file.js'; import { __resetAuthNoticesForTests } from '../../../src/lib/auth.js'; import { testRunCommand } from '../../../src/lib/command-framework/apify-command.js'; import { GLOBAL_CONFIGS_FOLDER } from '../../../src/lib/consts.js'; @@ -49,6 +50,7 @@ export function useAuthSetup({ cleanup = true, perTest = true }: UseAuthSetupOpt __resetCredentialsForTests(); __resetUserInfoCacheForTests(); __resetAuthNoticesForTests(); + __resetAuthFileForTests(); }); after(async () => { @@ -59,6 +61,7 @@ export function useAuthSetup({ cleanup = true, perTest = true }: UseAuthSetupOpt __resetCredentialsForTests(); __resetUserInfoCacheForTests(); __resetAuthNoticesForTests(); + __resetAuthFileForTests(); vitest.unstubAllEnvs(); }); } @@ -80,6 +83,7 @@ export function useKeyringBackend() { __resetCredentialsForTests(); __resetUserInfoCacheForTests(); __resetAuthNoticesForTests(); + __resetAuthFileForTests(); }); } 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 ecf6b4d51..80c0c9299 100644 --- a/test/local/commands/auth.test.ts +++ b/test/local/commands/auth.test.ts @@ -1,15 +1,16 @@ -import { existsSync, readFileSync, statSync } from 'node:fs'; +import { 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 { getSecret } from '../../../src/lib/credentials.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'; @@ -56,7 +57,6 @@ const { testRunCommand } = await import('../../../src/lib/command-framework/apif const TOKEN = 'apify_api_test_token'; -const readAuthFile = () => JSON.parse(readFileSync(AUTH_FILE_PATH(), 'utf-8')); const login = (token = TOKEN) => testRunCommand(AuthLoginCommand, { flags_token: token }); describe('auth commands', () => { @@ -71,14 +71,19 @@ describe('auth commands', () => { }); describe('file backend', () => { - it('login stores the token and user metadata in auth.json', async () => { + it('login stores the token and one profile keyed by user ID', async () => { await login(); - expect(readAuthFile()).toMatchObject({ - token: TOKEN, + expect(readAuthFile()).toMatchObject({ version: 2, secretsBackend: 'file' }); + expect(readActiveProfile()).toEqual({ id: 'uid', username: 'me', - secretsBackend: 'file', + name: null, + authMethod: 'token', + expiresAt: null, + hasRefreshToken: false, + token: TOKEN, + proxy: { password: 'pw' }, }); expect(lastErrorMessage()).toContain('You are logged in to Apify as me'); }); @@ -101,10 +106,10 @@ 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 metadata', async () => { + it('logging in as another account replaces the stored profile', async () => { clientState.user = { id: 'uid', username: 'me', email: 'me@example.com' }; await login(); @@ -112,9 +117,10 @@ describe('auth commands', () => { await login('apify_api_other_token'); const authFile = readAuthFile(); - expect(authFile).toMatchObject({ token: 'apify_api_other_token', id: 'uid2', username: 'other' }); - // The new account has no email, so the old one must not linger. - expect(authFile.email).toBeUndefined(); + 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', token: 'apify_api_other_token' }); }); it('login with an invalid token stores nothing and fails the command', async () => { @@ -132,7 +138,7 @@ describe('auth commands', () => { vitest.stubEnv('APIFY_TOKEN', 'apify_api_env_token'); await login(); - expect(await getToken()).toBe(TOKEN); + expect(await getSecret('uid', 'token')).toBe(TOKEN); expect(lastErrorMessage()).toContain('APIFY_TOKEN is set, so other commands keep using that token'); }); @@ -176,8 +182,8 @@ describe('auth commands', () => { await testRunCommand(AuthTokenCommand, {}); expect(lastLogMessage()).toBe('apify_api_env_token'); - expect(await getToken()).toBe(TOKEN); - expect(readAuthFile()).toMatchObject({ username: 'me' }); + expect(await getSecret('uid', 'token')).toBe(TOKEN); + expect(readActiveProfile()).toMatchObject({ username: 'me' }); }); }); @@ -187,28 +193,22 @@ 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(keyringTokenKey('uid'))).toBe(TOKEN); + expect(keyringStore.get(keyringProxyPasswordKey('uid'))).toBe('pw'); const authFile = readAuthFile(); - expect(authFile).toMatchObject({ id: 'uid', username: 'me', secretsBackend: 'keyring' }); + expect(authFile).toMatchObject({ version: 2, secretsBackend: 'keyring' }); expect(authFile.token).toBeUndefined(); - expect(authFile.proxy).toEqual({ groups: [{ name: 'g' }] }); - }); - - it('login drops the proxy object from auth.json when it only held the password', async () => { - clientState.user.proxy = { password: 'pw' }; - await login(); - - expect(readAuthFile()).not.toHaveProperty('proxy'); - expect(keyringStore.get(KEYRING_PROXY_PASSWORD_KEY)).toBe('pw'); + // Proxy groups are not a secret, but nothing reads them either. + expect(authFile).not.toHaveProperty('proxy'); + expect(readActiveProfile()).toMatchObject({ id: 'uid', username: 'me' }); }); 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 === keyringTokenKey('uid'))).toHaveLength(1); }); it('auth token prints the token from the keyring', async () => { @@ -218,6 +218,28 @@ describe('auth commands', () => { expect(lastLogMessage()).toBe(TOKEN); }); + it('logging in as another account clears the outgoing account keyring entries', async () => { + clientState.user = { id: 'uid', username: 'me', proxy: { password: 'pw' } }; + await login(); + + clientState.user = { id: 'uid2', username: 'other', proxy: { password: 'pw2' } }; + await login('apify_api_other_token'); + + // auth.json no longer names `uid`, so entries left behind would be unreachable forever. + expect(keyringStore.get(keyringTokenKey('uid'))).toBeUndefined(); + expect(keyringStore.get(keyringProxyPasswordKey('uid'))).toBeUndefined(); + expect(keyringStore.get(keyringTokenKey('uid2'))).toBe('apify_api_other_token'); + expect(keyringStore.get(keyringProxyPasswordKey('uid2'))).toBe('pw2'); + }); + + it('logging in as the same account keeps its keyring entries', async () => { + await login(); + await login(); + + expect(keyringStore.get(keyringTokenKey('uid'))).toBe(TOKEN); + expect(keyringStore.get(keyringProxyPasswordKey('uid'))).toBe('pw'); + }); + it('logout clears the keyring and removes auth.json', async () => { await login(); await testRunCommand(AuthLogoutCommand, {}); diff --git a/test/local/commands/run.test.ts b/test/local/commands/run.test.ts index 75c1e141b..edb48ded2 100644 --- a/test/local/commands/run.test.ts +++ b/test/local/commands/run.test.ts @@ -4,13 +4,14 @@ import { dirname } from 'node:path'; import { ACTOR_ENV_VARS, APIFY_ENV_VARS } from '@apify/consts'; import { testRunCommand } from '../../../src/lib/command-framework/apify-command.js'; -import { AUTH_FILE_PATH, EMPTY_LOCAL_CONFIG, LOCAL_CONFIG_PATH } from '../../../src/lib/consts.js'; +import { EMPTY_LOCAL_CONFIG, LOCAL_CONFIG_PATH } from '../../../src/lib/consts.js'; import { rimrafPromised } from '../../../src/lib/files.js'; import { getLocalDatasetPath, getLocalKeyValueStorePath, getLocalRequestQueuePath, getLocalStorageDir, + getLocalUserInfo, } from '../../../src/lib/utils.js'; import { TEST_TIMEOUT } from '../../__setup__/consts.js'; import { safeLogin, useAuthSetup } from '../../__setup__/hooks/useAuthSetup.js'; @@ -123,9 +124,9 @@ describe('apify run', () => { const actOutputPath = joinPath(getLocalKeyValueStorePath(), 'OUTPUT.json'); const localEnvVars = JSON.parse(readFileSync(actOutputPath, 'utf8')); - const auth = JSON.parse(readFileSync(AUTH_FILE_PATH(), 'utf8')); + const auth = await getLocalUserInfo(); - expect(localEnvVars[APIFY_ENV_VARS.PROXY_PASSWORD]).toStrictEqual(auth.proxy.password); + expect(localEnvVars[APIFY_ENV_VARS.PROXY_PASSWORD]).toStrictEqual(auth.proxy!.password); expect(localEnvVars[APIFY_ENV_VARS.USER_ID]).toStrictEqual(auth.id); expect(localEnvVars[APIFY_ENV_VARS.TOKEN]).toStrictEqual(auth.token); expect(localEnvVars.TEST_LOCAL).toStrictEqual(testEnvVars.TEST_LOCAL); @@ -164,9 +165,9 @@ describe('apify run', () => { const actOutputPath = joinPath(getLocalKeyValueStorePath(), 'OUTPUT.json'); const localEnvVars = JSON.parse(readFileSync(actOutputPath, 'utf8')); - const auth = JSON.parse(readFileSync(AUTH_FILE_PATH(), 'utf8')); + const auth = await getLocalUserInfo(); - expect(localEnvVars[APIFY_ENV_VARS.PROXY_PASSWORD]).toStrictEqual(auth.proxy.password); + expect(localEnvVars[APIFY_ENV_VARS.PROXY_PASSWORD]).toStrictEqual(auth.proxy!.password); expect(localEnvVars[APIFY_ENV_VARS.USER_ID]).toStrictEqual(auth.id); expect(localEnvVars[APIFY_ENV_VARS.TOKEN]).toStrictEqual(auth.token); expect(localEnvVars.TEST_LOCAL).toStrictEqual(testEnvVars.TEST_LOCAL); @@ -204,9 +205,9 @@ describe('apify run', () => { const actOutputPath = joinPath(getLocalKeyValueStorePath(), 'OUTPUT.json'); const localEnvVars = JSON.parse(readFileSync(actOutputPath, 'utf8')); - const auth = JSON.parse(readFileSync(AUTH_FILE_PATH(), 'utf8')); + const auth = await getLocalUserInfo(); - expect(localEnvVars[APIFY_ENV_VARS.PROXY_PASSWORD]).toStrictEqual(auth.proxy.password); + expect(localEnvVars[APIFY_ENV_VARS.PROXY_PASSWORD]).toStrictEqual(auth.proxy!.password); expect(localEnvVars[APIFY_ENV_VARS.USER_ID]).toStrictEqual(auth.id); expect(localEnvVars[APIFY_ENV_VARS.TOKEN]).toStrictEqual(auth.token); expect(localEnvVars.TEST_LOCAL).toStrictEqual(testEnvVars.TEST_LOCAL); diff --git a/test/local/lib/auth-file.test.ts b/test/local/lib/auth-file.test.ts new file mode 100644 index 000000000..dff7f9d2a --- /dev/null +++ b/test/local/lib/auth-file.test.ts @@ -0,0 +1,252 @@ +import { existsSync, mkdirSync, readFileSync, writeFileSync } from 'node:fs'; + +import { + __resetAuthFileForTests, + AUTH_BACKUP_FILE_PATH, + type AuthProfile, + ensureAuthFileCurrent, + getActiveProfile, + lookUpActiveProfile, + removeActiveProfile, + setActiveProfile, +} from '../../../src/lib/auth-file.js'; +import { AUTH_FILE_PATH, GLOBAL_CONFIGS_FOLDER } from '../../../src/lib/consts.js'; +import { ensureCredentialsCurrent, ensureMigrated, 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 { + LEGACY_KEYRING_PROXY_PASSWORD_KEY, + LEGACY_KEYRING_TOKEN_KEY, + keyringStore, + resetKeyringMock, +} from '../../__setup__/keyring-mock.js'; + +vi.mock('@napi-rs/keyring', () => import('../../__setup__/keyring-mock.js')); + +useAuthSetup(); + +const write = (contents: unknown) => { + mkdirSync(GLOBAL_CONFIGS_FOLDER(), { recursive: true }); + writeFileSync(AUTH_FILE_PATH(), typeof contents === 'string' ? contents : JSON.stringify(contents)); +}; + +const readBackup = () => JSON.parse(readFileSync(AUTH_BACKUP_FILE_PATH(), 'utf-8')); + +const V2_PROFILE: AuthProfile = { + username: 'me', + name: null, + authMethod: 'token', + expiresAt: null, + hasRefreshToken: false, +}; + +const V1_PROFILE = { id: 'uid', ...V2_PROFILE }; + +describe('auth.json v2', () => { + beforeEach(() => { + resetKeyringMock(); + }); + + describe('migration', () => { + // State A in the wild: plaintext secrets and no backend marker, written before the keyring. + it('migrates state A, after ensureMigrated() has stamped the marker', async () => { + write(v1AuthFile()); + + await ensureMigrated(); + await ensureAuthFileCurrent(); + + expect(readAuthFile()).toEqual({ + version: 2, + activeProfile: 'uid', + profiles: { uid: V2_PROFILE }, + secretsBackend: 'file', + token: 'apify_api_v1_token', + proxy: { password: 'pw' }, + }); + }); + + // State C: plaintext secrets with the file marker already on them. + it('migrates state C and keeps the secrets in the file', async () => { + write(v1AuthFile({ secretsBackend: 'file' })); + + await ensureCredentialsCurrent(); + + 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 () => { + write(v1AuthFile({ secretsBackend: 'file' })); + + await ensureAuthFileCurrent(); + + const file = readAuthFile(); + for (const key of ['email', 'plan', 'isPaying', 'createdAt', 'id', 'username']) { + expect(file).not.toHaveProperty(key); + } + expect(file.proxy).toEqual({ password: 'pw' }); + }); + + it('carries organizationOwnerUserId into the profile', async () => { + write(v1AuthFile({ secretsBackend: 'file', organizationOwnerUserId: 'owner-id' })); + + await ensureAuthFileCurrent(); + + expect(readActiveProfile()).toMatchObject({ organizationOwnerUserId: 'owner-id' }); + }); + + it('backs the v1 file up and never overwrites the backup', async () => { + write(v1AuthFile({ secretsBackend: 'file' })); + + await ensureAuthFileCurrent(); + expect(readBackup()).toMatchObject({ id: 'uid', email: 'me@example.com' }); + + // A later process migrating another v1 file must leave the first backup alone. + write(v1AuthFile({ secretsBackend: 'file', username: 'someone-else' })); + __resetAuthFileForTests(); + await ensureAuthFileCurrent(); + + expect(readActiveProfile()).toMatchObject({ username: 'someone-else' }); + expect(readBackup()).toMatchObject({ username: 'me' }); + }); + + it('is a no-op on a file that is already v2', async () => { + write(v1AuthFile({ secretsBackend: 'file' })); + await ensureAuthFileCurrent(); + const migrated = readAuthFile(); + + await ensureAuthFileCurrent(); + + expect(readAuthFile()).toEqual(migrated); + }); + + it('does nothing when there is no file', async () => { + await ensureAuthFileCurrent(); + + expect(existsSync(AUTH_FILE_PATH())).toBe(false); + expect(existsSync(AUTH_BACKUP_FILE_PATH())).toBe(false); + }); + + it('leaves a corrupt file alone rather than rewriting it', async () => { + write('{ not json'); + + await ensureAuthFileCurrent(); + + expect(readFileSync(AUTH_FILE_PATH(), 'utf-8')).toBe('{ not json'); + expect(existsSync(AUTH_BACKUP_FILE_PATH())).toBe(false); + }); + + 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 ensureCredentialsCurrent(); + + expect(readAuthFile()).toEqual({ version: 2, profiles: {}, secretsBackend: 'file' }); + // The backup keeps the plaintext token, so the way back is a re-login, not a lost account. + expect(readBackup()).toEqual({ token: 'apify_api_v1_token', secretsBackend: 'file' }); + await expect(getLocalUserInfo()).resolves.toEqual({}); + }); + }); + + describe('reading the active profile', () => { + it('reads a v1 file that has not been migrated yet', () => { + write(v1AuthFile()); + + expect(getActiveProfile()).toEqual(V1_PROFILE); + }); + + it('returns nothing when no profile is stored', () => { + write({ version: 2, profiles: {} }); + + expect(lookUpActiveProfile()).toEqual({}); + }); + + it('names the profile activeProfile points at when the file does not contain it', () => { + write({ version: 2, activeProfile: 'gone', profiles: {} }); + + expect(lookUpActiveProfile()).toEqual({ missingProfile: 'gone' }); + }); + + it('names the missing profile rather than reporting a silent logged-out state', async () => { + write({ version: 2, activeProfile: 'gone', profiles: {}, secretsBackend: 'file', token: 'tok' }); + + await expect(getLocalUserInfo()).rejects.toThrow('Your active profile "gone" is missing'); + }); + + it('names the missing profile even when no secret is left behind', async () => { + write({ version: 2, activeProfile: 'gone', profiles: {}, secretsBackend: 'file' }); + + await expect(getLocalUserInfo()).rejects.toThrow('Your active profile "gone" is missing'); + }); + }); + + describe('a file a newer CLI wrote', () => { + it('is refused rather than migrated backwards', async () => { + write({ version: 3, activeProfile: 'uid', profiles: {} }); + + await expect(ensureAuthFileCurrent()).rejects.toThrow('written by a newer Apify CLI'); + }); + + it('is not replaced by a login', () => { + const newer = { version: 3, activeProfile: 'uid', profiles: { uid: { username: 'me' } } }; + write(newer); + + expect(() => setActiveProfile('uid2', V2_PROFILE, 'file')).toThrow('written by a newer Apify CLI'); + expect(readAuthFile()).toEqual(newer); + }); + + it('is not touched by a logout', () => { + const newer = { version: 3, activeProfile: 'uid', profiles: { uid: { username: 'me' } }, token: 'tok' }; + write(newer); + + expect(() => removeActiveProfile()).toThrow('written by a newer Apify CLI'); + expect(readAuthFile()).toEqual(newer); + }); + }); + + describe('keyring backend', () => { + useKeyringBackend(); + + // 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(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(); + await ensureAuthFileCurrent(); + + expect(readAuthFile()).toEqual({ + version: 2, + activeProfile: 'uid', + profiles: { uid: V2_PROFILE }, + secretsBackend: 'keyring', + }); + expect(await getLocalUserInfo()).toEqual({ + id: 'uid', + username: 'me', + token: 'tok_kr', + proxy: { password: 'pw_kr' }, + }); + }); + + // State A on a machine where the keyring works: ensureMigrated() moves the secrets first. + it('migrates state A to the keyring and then to v2', async () => { + write(v1AuthFile()); + + await ensureMigrated(); + await ensureAuthFileCurrent(); + + expect(keyringStore.get(LEGACY_KEYRING_TOKEN_KEY)).toBe('apify_api_v1_token'); + expect(readAuthFile()).toEqual({ + version: 2, + activeProfile: 'uid', + profiles: { uid: V2_PROFILE }, + secretsBackend: 'keyring', + }); + }); + }); +}); diff --git a/test/local/lib/auth.test.ts b/test/local/lib/auth.test.ts index 2d4da6e4c..d99ade2ea 100644 --- a/test/local/lib/auth.test.ts +++ b/test/local/lib/auth.test.ts @@ -1,9 +1,10 @@ -import { existsSync, readFileSync } from 'node:fs'; +import { existsSync } from 'node:fs'; import { loginWithToken, resolveAuth } from '../../../src/lib/auth.js'; import { AUTH_FILE_PATH } from '../../../src/lib/consts.js'; -import { getProxyPassword, getToken, setToken } from '../../../src/lib/credentials.js'; +import { getSecret } from '../../../src/lib/credentials.js'; import { getLoggedClientOrThrow } from '../../../src/lib/utils.js'; +import { readActiveProfile } from '../../__setup__/auth-file.js'; import { useAuthSetup } from '../../__setup__/hooks/useAuthSetup.js'; import { useConsoleSpy } from '../../__setup__/hooks/useConsoleSpy.js'; @@ -46,8 +47,6 @@ const STORED = 'apify_api_stored'; const ENV = 'apify_api_env'; const FLAG = 'apify_api_flag'; -const readAuthFile = () => JSON.parse(readFileSync(AUTH_FILE_PATH(), 'utf-8')); - describe('auth', () => { beforeEach(() => { clientState.fail = false; @@ -145,9 +144,9 @@ 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(readAuthFile()).toMatchObject({ id: 'uid', username: 'me' }); + expect(await getSecret('uid', 'token')).toBe(STORED); + expect(await getSecret('uid', 'proxy-password')).toBe('pw'); + expect(readActiveProfile()).toMatchObject({ id: 'uid', username: 'me' }); }); it('writes nothing when the API rejects the token', async () => { @@ -162,7 +161,7 @@ describe('auth', () => { await loginWithToken(STORED); - expect(await getToken()).toBe(STORED); + expect(await getSecret('uid', 'token')).toBe(STORED); }); }); @@ -209,16 +208,16 @@ describe('auth', () => { await resolveAuth(); - expect(await getToken()).toBe(STORED); - expect(readAuthFile()).toMatchObject({ username: 'me' }); + expect(await getSecret('uid', 'token')).toBe(STORED); + expect(readActiveProfile()).toMatchObject({ username: 'me' }); }); it('resolving a token the command was given leaves the stored login untouched', async () => { - await setToken(STORED); + await loginWithToken(STORED); await resolveAuth(FLAG); - expect(await getToken()).toBe(STORED); + expect(await getSecret('uid', 'token')).toBe(STORED); }); }); }); diff --git a/test/local/lib/credentials.test.ts b/test/local/lib/credentials.test.ts index 15c0b8c16..d0c2c4342 100644 --- a/test/local/lib/credentials.test.ts +++ b/test/local/lib/credentials.test.ts @@ -4,25 +4,28 @@ import process from 'node:process'; import { cryptoRandomObjectId } from '@apify/utilities'; +import { __resetAuthFileForTests } from '../../../src/lib/auth-file.js'; import { getApifyClientOptions } from '../../../src/lib/auth.js'; import { AUTH_FILE_PATH, GLOBAL_CONFIGS_FOLDER } from '../../../src/lib/consts.js'; import { __resetCredentialsForTests, clearKeyringSecrets, 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'; @@ -35,7 +38,8 @@ vi.mock('node:fs', async (importOriginal) => { }); const writeFileSyncSpy = vi.mocked(writeFileSync); -const authFileWrites = () => writeFileSyncSpy.mock.calls.filter((call) => call[0] === AUTH_FILE_PATH()); +// auth.json is written through a temp file and a rename, so the spied path carries a suffix. +const authFileWrites = () => writeFileSyncSpy.mock.calls.filter((call) => String(call[0]).startsWith(AUTH_FILE_PATH())); const writeAuthFile = (data: Record) => { mkdirSync(GLOBAL_CONFIGS_FOLDER(), { recursive: true }); @@ -44,6 +48,11 @@ const writeAuthFile = (data: Record) => { const readAuthFile = () => JSON.parse(readFileSync(AUTH_FILE_PATH(), 'utf-8')); +const readProfile = () => readAuthFile().profiles[TEST_USER_ID]; + +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)); @@ -52,12 +61,14 @@ describe('credentials', () => { resetKeyringMock(); writeFileSyncSpy.mockClear(); __resetCredentialsForTests(); + __resetAuthFileForTests(); }); afterEach(async () => { await rm(GLOBAL_CONFIGS_FOLDER(), { recursive: true, force: true }); vitest.unstubAllEnvs(); __resetCredentialsForTests(); + __resetAuthFileForTests(); }); describe('getBackend()', () => { @@ -88,57 +99,70 @@ describe('credentials', () => { describe('file backend', () => { beforeEach(() => { vitest.stubEnv('APIFY_DISABLE_KEYRING', '1'); + writeAuthFile(v2AuthFile() as Record); + 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(); + expect(readAuthFile().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('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('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('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('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'); - expect(writeFileSyncSpy).toHaveBeenCalledWith(AUTH_FILE_PATH(), expect.any(String), { mode: 0o600 }); + 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); }); }); @@ -148,83 +172,111 @@ 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('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 () => { + writeAuthFile(v2AuthFile() as Record); + 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(keyringStore.get(TOKEN_KEY)).toBeUndefined(); + expect(readProfile().token).toBe('tok_123'); + expect(readAuthFile().secretsBackend).toBe('file'); expect(await getBackend()).toBe('file'); - expect(await getToken()).toBe('tok_123'); + 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'); + writeAuthFile(v2AuthFile() as Record); + keyringFailures.add(TOKEN_KEY); + await setSecret(TEST_USER_ID, 'token', 'tok_123'); - await setProxyPassword('pw_abc'); - expect(keyringStore.get(KEYRING_PROXY_PASSWORD_KEY)).toBeUndefined(); - expect(readAuthFile().proxy).toEqual({ password: 'pw_abc' }); + await setSecret(TEST_USER_ID, 'proxy-password', 'pw_abc'); + expect(keyringStore.get(PROXY_PASSWORD_KEY)).toBeUndefined(); + expect(readProfile().proxy).toEqual({ password: 'pw_abc' }); }); - 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 () => { + writeAuthFile(v2AuthFile() as Record); + 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(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(); }); }); @@ -240,7 +292,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' }); }); @@ -264,8 +316,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(); @@ -277,7 +329,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'); @@ -287,7 +339,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'); @@ -305,7 +357,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(); @@ -328,8 +380,120 @@ describe('credentials', () => { }); }); + describe('ensureSecretsKeyed()', () => { + it('moves keyring entries off the fixed names onto the user ID', async () => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', ''); + writeAuthFile(v2AuthFile({}, { secretsBackend: 'keyring' }) as Record); + 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'); + writeAuthFile( + v2AuthFile({}, { secretsBackend: 'file', token: 'tok', proxy: { password: 'pw' } }) as Record, + ); + + 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 keyring entries even when APIFY_DISABLE_KEYRING=1 is set', async () => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', '1'); + writeAuthFile({ version: 2, profiles: {}, secretsBackend: 'keyring' }); + keyringStore.set(LEGACY_KEYRING_TOKEN_KEY, 'tok'); + keyringStore.set(LEGACY_KEYRING_PROXY_PASSWORD_KEY, 'pw'); + + await ensureSecretsKeyed(); + + expect(keyringStore.size).toBe(0); + }); + + it('is a no-op on a file whose secrets are already keyed', async () => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', '1'); + writeAuthFile(v2AuthFile({ token: 'tok' }, { secretsBackend: 'file' }) as Record); + 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('downgrades to the file backend when the keyring write fails mid-migration', async () => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', ''); + writeAuthFile(v2AuthFile({}, { secretsBackend: 'keyring' }) as Record); + 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 downgrade holds for the rest of the loop. + expect(readProfile()).toMatchObject({ token: 'tok', proxy: { password: 'pw' } }); + expect(readAuthFile().secretsBackend).toBe('file'); + expect(await getBackend()).toBe('file'); + expect(keyringStore.size).toBe(0); + }); + + it('is memoized within a process', async () => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', '1'); + writeAuthFile(v2AuthFile({}, { secretsBackend: 'file', token: 'tok' }) as Record); + await ensureSecretsKeyed(); + expect(readProfile().token).toBe('tok'); + + writeAuthFile(v2AuthFile({}, { secretsBackend: 'file', token: 'tok2' }) as Record); + await ensureSecretsKeyed(); + expect(readAuthFile().token).toBe('tok2'); + }); + }); + describe('getLocalUserInfo()', () => { - it('on file backend, preserves non-secret proxy fields', async () => { + it('on file backend, reads the token and proxy password from the profile', async () => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', '1'); + writeAuthFile( + v2AuthFile({ token: 'tok', proxy: { password: 'pw' } }, { secretsBackend: 'file' }) as Record, + ); + 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({ username: 'me', @@ -339,13 +503,13 @@ describe('credentials', () => { secretsBackend: 'file', }); const info = await getLocalUserInfo(); - expect(info.proxy).toEqual({ password: 'pw', groups: [{ name: 'g' }] }); + expect(info.proxy).toEqual({ password: 'pw' }); }); 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'); @@ -357,31 +521,39 @@ 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'); }); }); describe('getApifyClientOptions()', () => { beforeEach(() => { vitest.stubEnv('APIFY_DISABLE_KEYRING', '1'); + writeAuthFile(v2AuthFile() as Record); }); it('resolves the stored token when nothing overrides it', async () => { - await setToken('tok_stored'); + await setSecret(TEST_USER_ID, 'token', 'tok_stored'); expect((await getApifyClientOptions()).token).toBe('tok_stored'); }); it('prefers an explicitly passed token over the stored one', async () => { - await setToken('tok_stored'); + await setSecret(TEST_USER_ID, 'token', 'tok_stored'); expect((await getApifyClientOptions('tok_explicit')).token).toBe('tok_explicit'); }); diff --git a/test/local/lib/rental-sunset-notice.test.ts b/test/local/lib/rental-sunset-notice.test.ts index 9a038adb0..947503ac3 100644 --- a/test/local/lib/rental-sunset-notice.test.ts +++ b/test/local/lib/rental-sunset-notice.test.ts @@ -27,7 +27,17 @@ async function writeAuthFile(username: string | undefined) { const path = AUTH_FILE_PATH(); await mkdir(dirname(path), { recursive: true }); - await writeFile(path, JSON.stringify({ id: 'user-id', username, token: 'apify_api_token' })); + await writeFile( + path, + JSON.stringify({ + version: 2, + activeProfile: 'user-id', + profiles: { + 'user-id': { username, name: null, authMethod: 'token', expiresAt: null, hasRefreshToken: false }, + }, + token: 'apify_api_token', + }), + ); } interface StoredRentalSunset {