diff --git a/src/commands/auth/logout.ts b/src/commands/auth/logout.ts index 3ece55890..68f3b4768 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 { 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 { 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,10 @@ 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(); - await rimrafPromised(AUTH_FILE_PATH()); await updateUserId(null); diff --git a/src/lib/auth-file.ts b/src/lib/auth-file.ts new file mode 100644 index 000000000..9e7149c2d --- /dev/null +++ b/src/lib/auth-file.ts @@ -0,0 +1,308 @@ +import { existsSync, readFileSync, renameSync, rmSync, writeFileSync } from 'node:fs'; + +import { cryptoRandomObjectId } from '@apify/utilities'; + +import { AUTH_FILE_PATH } from './consts.js'; +import type { CredentialsBackend } from './credentials.js'; +import { ensureApifyDirectory } from './files.js'; +import { warning } from './outputs.js'; +import { cliDebugPrint } from './utils/cliDebugPrint.js'; + +export const AUTH_FILE_VERSION = 2; + +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; + /** These three are unread until the device flow lands, and reserved so it needs no migration. */ + authMethod: 'token'; + expiresAt: string | null; + hasRefreshToken: boolean; + /** Reserved: a keyring failure on one profile must not redirect another profile's reads. */ + secretsBackend?: CredentialsBackend; + loggedInAt: string | null; +} + +/** + * `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. + */ +export interface AuthFile { + version?: number; + activeProfile?: string; + profiles?: Record; + secretsBackend?: CredentialsBackend; + token?: string; + proxy?: { password?: string; [k: string]: unknown }; +} + +/** + * The flat shape written before profiles existed: one account spread across the top level, and no + * `version` field. Only the migration and the pre-migration read path see it. + */ +export interface LegacyAuthFile extends AuthFile { + id?: string; + username?: string; + organizationOwnerUserId?: string; +} + +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 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 { + const parsed: unknown = JSON.parse(readFileSync(AUTH_FILE_PATH(), 'utf-8')); + // A valid JSON string or array is as unusable as a parse error, and must not be rewritten. + if (typeof parsed !== 'object' || parsed === null || Array.isArray(parsed)) return null; + + return parsed as AuthFile; + } catch { + return null; + } +} + +export function readAuthFile(): AuthFile { + return parseAuthFile() ?? {}; +} + +export function writeAuthFile(data: AuthFile) { + atomicWriteJson(AUTH_FILE_PATH(), data); +} + +/** Temp file then rename: two CLI processes can run at once, and a torn file reads as logged out. */ +function atomicWriteJson(path: string, data: unknown) { + 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; + } +} + +function v1Profile(file: LegacyAuthFile): AuthProfile { + return { + ...(typeof file.username === 'string' ? { username: file.username } : {}), + name: null, + ...(typeof file.organizationOwnerUserId === 'string' + ? { organizationOwnerUserId: file.organizationOwnerUserId } + : {}), + authMethod: 'token', + expiresAt: null, + hasRefreshToken: false, + loggedInAt: null, + }; +} + +/** + * 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: 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. + 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; +} + +/** + * A snapshot of the pre-v2 file, so an upgrade is inspectable. Written once and never refreshed, + * which is why the secrets are left out: a rotated token copied here would outlive the account it + * belongs to. Nothing reads it. + */ +function backUpV1File(file: AuthFile) { + if (existsSync(AUTH_BACKUP_FILE_PATH())) return; + + const { token: _token, proxy: _proxy, ...withoutSecrets } = file; + atomicWriteJson(AUTH_BACKUP_FILE_PATH(), withoutSecrets); +} + +/** + * One entry per format bump, keyed by the version it upgrades from. A file at version N runs every + * step from N upwards, so moving data between shapes later means adding an entry here rather than + * reworking this module. The first shape carried no `version` field at all; it counts as 1. + */ +const MIGRATION_STEPS: Record AuthFile> = { + 1: toV2, +}; + +const FIRST_AUTH_FILE_VERSION = 1; + +async function migrateAuthFile(): 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; + if (Object.keys(file).length === 0) return; + + const from = typeof file.version === 'number' ? file.version : FIRST_AUTH_FILE_VERSION; + if (from >= AUTH_FILE_VERSION) return; + + // The backup captures the shape the user arrived with, before any step touches it. + if (from === FIRST_AUTH_FILE_VERSION) backUpV1File(file); + + let migrated = file; + for (let version = from; version < AUTH_FILE_VERSION; version++) { + const step = MIGRATION_STEPS[version]; + if (!step) throw new Error(`No migration step from auth file version ${version}.`); + + migrated = step(migrated); + } + + 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. + 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.`, + }); + } + })(); + + return migrationPromise; +} + +/** + * A file from a newer CLI is not something to guess at — migrating it backwards would drop + * whatever that version stores. + */ +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. It uses auth file version ${version} and this one reads ${AUTH_FILE_VERSION}. Upgrade the CLI, or run "apify logout" to discard them.`, + ); + } +} + +/** + * Runs after `ensureMigrated()`, which moves v1 secrets into the keyring. The two stay separate so + * a keyring failure and a shape failure cannot mask each other. + * + * Migrating never throws — it must not block a command. Throws only for a file a newer CLI wrote. + */ +export async function ensureAuthFileCurrent(): Promise { + await migrateAuthFile(); + assertSupportedAuthFileVersion(); +} + +/** + * Reads the pre-profile shape as well, and keeps doing so: `useRentalSunsetNotice` calls this + * without migrating first, to avoid a keychain prompt on commands that need no login. + */ +export function lookUpActiveProfile(): ActiveProfileLookup { + const file = readAuthFile(); + + if (file.version !== AUTH_FILE_VERSION) { + // Pre-migration, or a version this CLI does not know. Either way the only account it can + // name is the flat one, and a newer file has no top-level id to find. + const legacy = file as LegacyAuthFile; + return typeof legacy.id === 'string' ? { profile: { id: legacy.id, ...v1Profile(legacy) } } : {}; + } + + if (!file.activeProfile) return {}; + + const profile = file.profiles?.[file.activeProfile]; + if (!profile) return { missingProfile: file.activeProfile }; + + return { profile: { id: file.activeProfile, ...profile } }; +} + +export function getActiveProfile(): (AuthProfile & { id: string }) | undefined { + return lookUpActiveProfile().profile; +} + +/** + * 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. + */ +export function replaceStoredAccount(userId: string, profile: AuthProfile, secretsBackend: CredentialsBackend) { + assertSupportedAuthFileVersion(); + + // The snapshot described the account being replaced, and is never refreshed, so keeping it + // would leave one user's details on disk under another user's login. + rmSync(AUTH_BACKUP_FILE_PATH(), { force: true, maxRetries: 10, retryDelay: 100 }); + + 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() { + const file = readAuthFile(); + + // No version guard: refusing to discard a file this CLI cannot read leaves no way out. It goes + // whole rather than edited, which would leave something worse than either outcome. + if (file.version !== AUTH_FILE_VERSION) { + discardAuthFiles(); + return; + } + + const active = file.activeProfile; + if (active && file.profiles) delete file.profiles[active]; + delete file.activeProfile; + delete file.token; + delete file.proxy; + + if (Object.keys(file.profiles ?? {}).length === 0) { + discardAuthFiles(); + return; + } + + writeAuthFile(file); +} + +function discardAuthFiles() { + rmSync(AUTH_FILE_PATH(), { force: true, maxRetries: 10, retryDelay: 100 }); + rmSync(AUTH_BACKUP_FILE_PATH(), { force: true, maxRetries: 10, retryDelay: 100 }); +} diff --git a/src/lib/auth.ts b/src/lib/auth.ts index b2793e193..636dfb1cd 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 { ApifyApiError, ApifyClient, type ApifyClientOptions } from 'apify-client'; @@ -6,6 +6,7 @@ import { AxiosHeaders } from 'axios'; import { APIFY_ENV_VARS } from '@apify/consts'; +import { ensureAuthFileCurrent, replaceStoredAccount } from './auth-file.js'; import { APIFY_CLIENT_DEFAULT_HEADERS, AUTH_FILE_PATH, CommandExitCodes } from './consts.js'; import { deleteProxyPassword, @@ -14,9 +15,7 @@ import { getToken, setProxyPassword, setToken, - stripProxyPassword, } from './credentials.js'; -import { ensureApifyDirectory } from './files.js'; import { warning } from './outputs.js'; import type { AuthJSON } from './types.js'; import { cliDebugPrint } from './utils/cliDebugPrint.js'; @@ -96,6 +95,10 @@ export const resolveAuth = async (): Promise => { return { token: envToken.token, source: 'env' } as const; } + // 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(); + const storedToken = await getToken(); return storedToken ? ({ token: storedToken, source: 'stored' } as const) : undefined; })(); @@ -168,17 +171,28 @@ export async function loginWithToken( return null; } - const proxyPassword = userInfo.proxy?.password; + if (!userInfo.id) { + throw new Error('The Apify API returned no user ID for this token, so the login cannot be stored.'); + } - // Replaces the previous account rather than merging, so stale fields cannot linger. The spread - // is shallow, so stripping here also clears userInfo.proxy — read the password first. - const fileContents = { ...userInfo, secretsBackend: await getBackend() }; - stripProxyPassword(fileContents); + const proxyPassword = userInfo.proxy?.password; - ensureApifyDirectory(AUTH_FILE_PATH()); - writeFileSync(AUTH_FILE_PATH(), JSON.stringify(fileContents, null, '\t'), { mode: 0o600 }); + const { organizationOwnerUserId } = userInfo as { organizationOwnerUserId?: string }; + replaceStoredAccount( + userInfo.id, + { + username: userInfo.username, + name: null, + ...(organizationOwnerUserId ? { organizationOwnerUserId } : {}), + authMethod: 'token', + expiresAt: null, + hasRefreshToken: false, + loggedInAt: new Date().toISOString(), + }, + await getBackend(), + ); - // After the metadata file, which would clobber them on the file backend. `skipIfUnchanged` avoids a Keychain prompt. + // After the account, which drops the previous secrets. `skipIfUnchanged` avoids a Keychain prompt. await setToken(token, { skipIfUnchanged: true }); if (proxyPassword) { diff --git a/src/lib/credentials.ts b/src/lib/credentials.ts index 3758cd502..849ed51b5 100644 --- a/src/lib/credentials.ts +++ b/src/lib/credentials.ts @@ -1,8 +1,6 @@ -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 { AUTH_FILE_VERSION, readAuthFile, writeAuthFile } from './auth-file.js'; import { useCLIMetadata } from './hooks/useCLIMetadata.js'; import { cliDebugPrint } from './utils/cliDebugPrint.js'; @@ -22,13 +20,6 @@ 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; @@ -104,16 +95,6 @@ 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 {}; - } -} - /** * Remove the proxy password, keeping any sibling field like `groups` and dropping `proxy` * entirely when the secret was all it carried. @@ -125,11 +106,6 @@ export function stripProxyPassword(data: { proxy?: { password?: string } }) { if (Object.keys(data.proxy).length === 0) delete data.proxy; } -function writeAuthFile(data: StoredAuthFile) { - ensureApifyDirectory(AUTH_FILE_PATH()); - writeFileSync(AUTH_FILE_PATH(), JSON.stringify(data, null, '\t'), { mode: 0o600 }); -} - async function getKeyringEntry(account: string): Promise { const mod = await loadKeyringModule(); if (!mod) return null; @@ -272,6 +248,9 @@ export async function ensureMigrated(): Promise { migrationPromise = (async () => { try { const file = readAuthFile(); + // A file a newer CLI wrote is not ours to rewrite, and this runs before the shape + // migration reports it. + if (typeof file.version === 'number' && file.version > AUTH_FILE_VERSION) return; if (file.secretsBackend) return; if (!file.token && !file.proxy?.password) return; 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 0bdfea073..b1ce8e476 100644 --- a/src/lib/utils.ts +++ b/src/lib/utils.ts @@ -32,6 +32,7 @@ import { SOURCE_FILE_FORMATS, } from '@apify/consts'; +import { ensureAuthFileCurrent, lookUpActiveProfile } from './auth-file.js'; import { describeAuthFailure, getApifyClientOptionsForToken, resolveAuth, type ResolvedAuth } 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 { ensureMigrated, getProxyPassword, getToken } 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'; @@ -85,33 +86,38 @@ 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 ensureAuthFileCurrent(); - 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(); + + const result: AuthJSON = {}; + if (profile) { + result.id = profile.id; + if (profile.username) result.username = profile.username; + if (profile.organizationOwnerUserId) result.organizationOwnerUserId = profile.organizationOwnerUserId; } - if ((await getBackend()) === 'keyring') { - const token = await getToken(); - if (token) result.token = token; + const token = await getToken(); + if (token) result.token = token; - const proxyPassword = await getProxyPassword(); - if (proxyPassword) result.proxy = { ...result.proxy, password: proxyPassword }; - } + const proxyPassword = await getProxyPassword(); + if (proxyPassword) result.proxy = { password: proxyPassword }; - 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.'); + // Reported rather than swallowed: the commands that build `/` lookups would + // otherwise fail with a misleading "not found". + if (!profile) { + if (!result.token) return {}; + + 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.', + ); } return result; diff --git a/test/__setup__/auth-file.ts b/test/__setup__/auth-file.ts new file mode 100644 index 000000000..6e617345d --- /dev/null +++ b/test/__setup__/auth-file.ts @@ -0,0 +1,35 @@ +/** 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_PATH } from '../../src/lib/consts.js'; + +/** 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 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 e43d1a153..932e6d26b 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 { __resetAuthForTests } from '../../../src/lib/auth.js'; import { testRunCommand } from '../../../src/lib/command-framework/apify-command.js'; import { GLOBAL_CONFIGS_FOLDER } from '../../../src/lib/consts.js'; @@ -20,6 +21,7 @@ function resetAuthCaches() { __resetCredentialsForTests(); __resetUserInfoCacheForTests(); __resetAuthForTests(); + __resetAuthFileForTests(); } export interface UseAuthSetupOptions { diff --git a/test/api/commands/info.test.ts b/test/api/commands/info.test.ts index 5b037f66a..f034397f4 100644 --- a/test/api/commands/info.test.ts +++ b/test/api/commands/info.test.ts @@ -1,8 +1,6 @@ -import { readFileSync } from 'node:fs'; - import { InfoCommand } from '../../../src/commands/info.js'; import { testRunCommand } from '../../../src/lib/command-framework/apify-command.js'; -import { AUTH_FILE_PATH } from '../../../src/lib/consts.js'; +import { readActiveProfile } from '../../__setup__/auth-file.js'; import { safeLogin, useAuthSetup } from '../../__setup__/hooks/useAuthSetup.js'; import { useConsoleSpy } from '../../__setup__/hooks/useConsoleSpy.js'; @@ -21,12 +19,10 @@ describe('[api] apify info', () => { await safeLogin(); await testRunCommand(InfoCommand, {}); - const userInfoFromConfig = JSON.parse(readFileSync(AUTH_FILE_PATH(), 'utf8')); - const spy = logSpy(); expect(spy).toHaveBeenCalledTimes(3); - expect(spy.mock.calls[1][0]).to.include(userInfoFromConfig.id); + expect(spy.mock.calls[1][0]).to.include(readActiveProfile()!.id); expect(spy.mock.calls[2][0]).to.include('apify login'); }); }); diff --git a/test/api/commands/log_in_out.test.ts b/test/api/commands/log_in_out.test.ts index 5ae48d0ed..d86998f92 100644 --- a/test/api/commands/log_in_out.test.ts +++ b/test/api/commands/log_in_out.test.ts @@ -1,9 +1,11 @@ -import { existsSync, readFileSync } from 'node:fs'; +import { existsSync } from 'node:fs'; import axios from 'axios'; import { testRunCommand } from '../../../src/lib/command-framework/apify-command.js'; import { AUTH_FILE_PATH } from '../../../src/lib/consts.js'; +import { getToken } from '../../../src/lib/credentials.js'; +import { readActiveProfile } from '../../__setup__/auth-file.js'; import { TEST_USER_BAD_TOKEN, TEST_USER_TOKEN, testUserClient } from '../../__setup__/config.js'; import { safeLogin, useAuthSetup } from '../../__setup__/hooks/useAuthSetup.js'; import { useConsoleSpy } from '../../__setup__/hooks/useConsoleSpy.js'; @@ -31,31 +33,15 @@ describe('[api] apify login and logout', () => { it('should work with correct token', async () => { await safeLogin(TEST_USER_TOKEN); - const expectedUserInfo = Object.assign(await testUserClient.user('me').get(), { - token: TEST_USER_TOKEN, - }) as unknown as Record; - const userInfoFromConfig = JSON.parse(readFileSync(AUTH_FILE_PATH(), 'utf8')); + const expectedUserInfo = await testUserClient.user('me').get(); expect(lastErrorMessage()).to.include('Success:'); - // Omit currentBillingPeriod, It can change during tests - - const { - currentBillingPeriod: _1, - plan: _2, - createdAt: _3, - ...expectedUserInfoWithoutFloatFields - } = expectedUserInfo; - - const { - currentBillingPeriod: _4, - plan: _5, - createdAt: _6, - secretsBackend: _7, - ...userInfoFromConfigWithoutFloatFields - } = userInfoFromConfig; - - expect(expectedUserInfoWithoutFloatFields).to.eql(userInfoFromConfigWithoutFloatFields); + expect(readActiveProfile()).toMatchObject({ + id: expectedUserInfo.id, + username: expectedUserInfo.username, + }); + expect(await getToken()).to.eql(TEST_USER_TOKEN); await testRunCommand(LogoutCommand, {}); const isGlobalConfig = existsSync(AUTH_FILE_PATH()); @@ -83,29 +69,14 @@ describe('[api] apify login and logout', () => { expect(response.status).to.be.eql(200); - const expectedUserInfo = Object.assign(await testUserClient.user('me').get(), { - token: TEST_USER_TOKEN, - }) as unknown as Record; - const userInfoFromConfig = JSON.parse(readFileSync(AUTH_FILE_PATH(), 'utf8')); + const expectedUserInfo = await testUserClient.user('me').get(); expect(lastErrorMessage()).to.include('Success:'); - // Omit currentBillingPeriod, It can change during tests - - const { - currentBillingPeriod: _1, - plan: _2, - createdAt: _3, - ...expectedUserInfoWithoutFloatFields - } = expectedUserInfo; - const { - currentBillingPeriod: _4, - plan: _5, - createdAt: _6, - secretsBackend: _7, - ...userInfoFromConfigWithoutFloatFields - } = userInfoFromConfig; - - expect(expectedUserInfoWithoutFloatFields).to.eql(userInfoFromConfigWithoutFloatFields); + expect(readActiveProfile()).toMatchObject({ + id: expectedUserInfo.id, + username: expectedUserInfo.username, + }); + expect(await getToken()).to.eql(TEST_USER_TOKEN); }); }); diff --git a/test/local/commands/auth.test.ts b/test/local/commands/auth.test.ts index 02f24375f..bdcf3cc2f 100644 --- a/test/local/commands/auth.test.ts +++ b/test/local/commands/auth.test.ts @@ -1,9 +1,10 @@ -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 { 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 { @@ -31,7 +32,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', () => { @@ -41,14 +41,18 @@ 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, token: TOKEN, secretsBackend: 'file' }); + expect(readActiveProfile()).toEqual({ id: 'uid', username: 'me', - secretsBackend: 'file', + name: null, + authMethod: 'token', + expiresAt: null, + hasRefreshToken: false, + loggedInAt: expect.stringMatching(/^\d{4}-\d{2}-\d{2}T/), }); expect(lastErrorMessage()).toContain('You are logged in to Apify as me'); }); @@ -74,7 +78,7 @@ describe('auth commands', () => { expect(await getToken()).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(); @@ -82,9 +86,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', token: 'apify_api_other_token' }); + // 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' }); }); it('login with an invalid token stores nothing and fails the command', async () => { @@ -170,7 +175,7 @@ describe('auth commands', () => { expect(lastLogMessage()).toBe('apify_api_env_token'); expect(await getToken()).toBe(TOKEN); - expect(readAuthFile()).toMatchObject({ username: 'me' }); + expect(readActiveProfile()).toMatchObject({ username: 'me' }); }); }); @@ -184,17 +189,11 @@ describe('auth commands', () => { expect(keyringStore.get(KEYRING_PROXY_PASSWORD_KEY)).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 as an account with no proxy password forgets the previous one', async () => { diff --git a/test/local/commands/run.test.ts b/test/local/commands/run.test.ts index 75c1e141b..0811f67b4 100644 --- a/test/local/commands/run.test.ts +++ b/test/local/commands/run.test.ts @@ -4,7 +4,8 @@ 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 { getProxyPassword, getToken } from '../../../src/lib/credentials.js'; import { rimrafPromised } from '../../../src/lib/files.js'; import { getLocalDatasetPath, @@ -12,6 +13,7 @@ import { getLocalRequestQueuePath, getLocalStorageDir, } from '../../../src/lib/utils.js'; +import { readActiveProfile } from '../../__setup__/auth-file.js'; import { TEST_TIMEOUT } from '../../__setup__/consts.js'; import { safeLogin, useAuthSetup } from '../../__setup__/hooks/useAuthSetup.js'; import { useConsoleSpy } from '../../__setup__/hooks/useConsoleSpy.js'; @@ -123,11 +125,11 @@ 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')); - - 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); + // Read from disk, not through getLocalUserInfo: `run` sources these from that same + // function, so asserting against it would only prove it agrees with itself. + expect(localEnvVars[APIFY_ENV_VARS.PROXY_PASSWORD]).toStrictEqual(await getProxyPassword()); + expect(localEnvVars[APIFY_ENV_VARS.USER_ID]).toStrictEqual(readActiveProfile()!.id); + expect(localEnvVars[APIFY_ENV_VARS.TOKEN]).toStrictEqual(await getToken()); expect(localEnvVars.TEST_LOCAL).toStrictEqual(testEnvVars.TEST_LOCAL); }); @@ -164,11 +166,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')); - - 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[APIFY_ENV_VARS.PROXY_PASSWORD]).toStrictEqual(await getProxyPassword()); + expect(localEnvVars[APIFY_ENV_VARS.USER_ID]).toStrictEqual(readActiveProfile()!.id); + expect(localEnvVars[APIFY_ENV_VARS.TOKEN]).toStrictEqual(await getToken()); expect(localEnvVars.TEST_LOCAL).toStrictEqual(testEnvVars.TEST_LOCAL); const actOutputPath2 = joinPath(getLocalKeyValueStorePath(), 'owo.json'); @@ -204,11 +204,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')); - - 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[APIFY_ENV_VARS.PROXY_PASSWORD]).toStrictEqual(await getProxyPassword()); + expect(localEnvVars[APIFY_ENV_VARS.USER_ID]).toStrictEqual(readActiveProfile()!.id); + expect(localEnvVars[APIFY_ENV_VARS.TOKEN]).toStrictEqual(await getToken()); expect(localEnvVars.TEST_LOCAL).toStrictEqual(testEnvVars.TEST_LOCAL); const actOutputPath2 = joinPath(getLocalKeyValueStorePath(), 'two.json'); diff --git a/test/local/lib/auth-file.test.ts b/test/local/lib/auth-file.test.ts new file mode 100644 index 000000000..f3b17a90c --- /dev/null +++ b/test/local/lib/auth-file.test.ts @@ -0,0 +1,416 @@ +import { chmodSync, existsSync, mkdirSync, readFileSync, statSync, writeFileSync } from 'node:fs'; +import process from 'node:process'; + +import { + __resetAuthFileForTests, + AUTH_BACKUP_FILE_PATH, + type AuthProfile, + ensureAuthFileCurrent, + getActiveProfile, + lookUpActiveProfile, + removeActiveProfile, + replaceStoredAccount, +} 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 { 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, + keyringStore, + resetKeyringMock, +} from '../../__setup__/keyring-mock.js'; + +vi.mock('@napi-rs/keyring', () => import('../../__setup__/keyring-mock.js')); + +useAuthSetup(); +const { lastErrorMessage } = useConsoleSpy(); + +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, + loggedInAt: null, +}; + +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 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'); + }); + + 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, and back out again', async () => { + write(v1AuthFile({ secretsBackend: 'file', organizationOwnerUserId: 'owner-id' })); + + await ensureAuthFileCurrent(); + + expect(readActiveProfile()).toMatchObject({ organizationOwnerUserId: 'owner-id' }); + // `push` and the Console URL read it from here; the file alone is not enough. + await expect(getLocalUserInfo()).resolves.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(); + + // Without the reset the memoised promise short-circuits and the file is never re-read. + __resetAuthFileForTests(); + await ensureAuthFileCurrent(); + + expect(readAuthFile()).toEqual(migrated); + }); + + it('logout removes the backup along with the file', async () => { + write(v1AuthFile({ secretsBackend: 'file' })); + await ensureAuthFileCurrent(); + expect(existsSync(AUTH_BACKUP_FILE_PATH())).toBe(true); + + removeActiveProfile(); + + expect(existsSync(AUTH_FILE_PATH())).toBe(false); + expect(existsSync(AUTH_BACKUP_FILE_PATH())).toBe(false); + }); + + // Only the temp-file + rename repairs an existing file's mode; a direct write would leave it. + it.skipIf(process.platform === 'win32')('tightens a pre-existing 0644 auth.json to 0600', async () => { + write(v1AuthFile({ secretsBackend: 'file' })); + chmodSync(AUTH_FILE_PATH(), 0o644); + + await ensureAuthFileCurrent(); + + expect(statSync(AUTH_FILE_PATH()).mode & 0o777).toBe(0o600); + }); + + // Windows has no POSIX modes: Node reports 0o666 there and chmod only moves the read-only bit. + it.skipIf(process.platform === 'win32')( + 'writes the backup readable only by the owner, whatever mode the v1 file had', + async () => { + write(v1AuthFile({ secretsBackend: 'file' })); + chmodSync(AUTH_FILE_PATH(), 0o644); + + await ensureAuthFileCurrent(); + + expect(statSync(AUTH_BACKUP_FILE_PATH()).mode & 0o777).toBe(0o600); + }, + ); + + // The backup is never refreshed, so a token in it would outlive the account it belongs to. + it('keeps the secrets out of the backup', async () => { + write(v1AuthFile({ secretsBackend: 'file' })); + + await ensureAuthFileCurrent(); + + const backup = readBackup(); + expect(backup).not.toHaveProperty('token'); + expect(backup).not.toHaveProperty('proxy'); + expect(backup).toMatchObject({ id: 'uid', username: 'me', email: 'me@example.com' }); + }); + + // 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('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('keeps 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(); + + 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. + expect(readBackup()).toEqual({ secretsBackend: 'file' }); + await expect(getLocalUserInfo()).rejects.toThrow('Stale credentials found without user metadata'); + }); + }); + + 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('is logged out when the missing profile leaves no token behind either', async () => { + write({ version: 2, activeProfile: 'gone', profiles: {}, secretsBackend: 'file' }); + + await expect(getLocalUserInfo()).resolves.toEqual({}); + }); + }); + + 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(() => replaceStoredAccount('uid2', V2_PROFILE, 'file')).toThrow('written by a newer Apify CLI'); + expect(readAuthFile()).toEqual(newer); + }); + + // Logout is the only way out of this state, so it is the one command that must not refuse. + it('is discarded by a logout', () => { + write({ version: 3, activeProfile: 'uid', profiles: { uid: { username: 'me' } }, token: 'tok' }); + + removeActiveProfile(); + + expect(existsSync(AUTH_FILE_PATH())).toBe(false); + }); + + it('says how to get out of the state', async () => { + write({ version: 3, activeProfile: 'uid', profiles: {} }); + + await expect(ensureAuthFileCurrent()).rejects.toThrow('apify logout'); + }); + }); + + describe('replacing the stored account', () => { + it('drops the previous account and its secrets', () => { + write({ + version: 2, + activeProfile: 'old', + profiles: { old: { ...V2_PROFILE, username: 'old' } }, + secretsBackend: 'file', + token: 'apify_api_old', + proxy: { password: 'old_pw' }, + }); + + replaceStoredAccount('new', { ...V2_PROFILE, username: 'new' }, 'file'); + + const file = readAuthFile(); + expect(Object.keys(file.profiles!)).toEqual(['new']); + expect(file.activeProfile).toBe('new'); + // A leftover token beside the new account authenticates as the wrong user. + expect(file).not.toHaveProperty('token'); + expect(file).not.toHaveProperty('proxy'); + }); + + it('leaves no token when the caller never writes one', async () => { + write({ + version: 2, + activeProfile: 'old', + profiles: { old: { ...V2_PROFILE, username: 'old' } }, + secretsBackend: 'file', + token: 'apify_api_old', + }); + + 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(); + }); + }); + + describe('replacing the stored account', () => { + it('removes the snapshot of the account it replaced', async () => { + write(v1AuthFile({ secretsBackend: 'file' })); + await ensureAuthFileCurrent(); + expect(existsSync(AUTH_BACKUP_FILE_PATH())).toBe(true); + + replaceStoredAccount('other', { ...V2_PROFILE, username: 'other' }, 'file'); + + // It described the previous account and is never refreshed, so it must not survive. + expect(existsSync(AUTH_BACKUP_FILE_PATH())).toBe(false); + }); + }); + + describe('the command paths that trigger the migration', () => { + it('getLocalUserInfo() migrates the file it reads', async () => { + write(v1AuthFile({ secretsBackend: 'file' })); + + await expect(getLocalUserInfo()).resolves.toMatchObject({ id: 'uid', username: 'me' }); + + expect(readAuthFile().version).toBe(2); + }); + + it('resolving a token migrates the file it reads', async () => { + write(v1AuthFile({ secretsBackend: 'file' })); + + await expect(resolveAuth()).resolves.toMatchObject({ source: 'stored' }); + + expect(readAuthFile().version).toBe(2); + }); + + it('a file a newer CLI wrote stops a command rather than being read as v1', async () => { + write({ version: 3, activeProfile: 'uid', profiles: {}, secretsBackend: 'file', token: 'tok' }); + + await expect(getLocalUserInfo()).rejects.toThrow('written by a newer Apify CLI'); + await expect(resolveAuth()).rejects.toThrow('written by a newer Apify CLI'); + }); + + // A platform run never reads the stored file, so a newer one must not stop it. + it('a file a newer CLI wrote does not stop a command running on APIFY_TOKEN', async () => { + write({ version: 3, activeProfile: 'uid', profiles: {}, secretsBackend: 'file', token: 'stored' }); + vitest.stubEnv('APIFY_TOKEN', 'apify_api_from_env'); + + await expect(resolveAuth()).resolves.toEqual({ token: 'apify_api_from_env', source: 'env' }); + }); + }); + + 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(KEYRING_TOKEN_KEY, 'tok_kr'); + keyringStore.set(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(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 62c441b36..0e3f4baf7 100644 --- a/test/local/lib/auth.test.ts +++ b/test/local/lib/auth.test.ts @@ -1,4 +1,4 @@ -import { existsSync, readFileSync } from 'node:fs'; +import { existsSync } from 'node:fs'; import { ApifyApiError } from 'apify-client'; @@ -7,6 +7,7 @@ import { AUTH_FILE_PATH, CommandExitCodes } from '../../../src/lib/consts.js'; import { getProxyPassword, getToken, setToken } 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'; import { useAuthSetup } from '../../__setup__/hooks/useAuthSetup.js'; import { useConsoleSpy } from '../../__setup__/hooks/useConsoleSpy.js'; @@ -21,8 +22,6 @@ const { lastErrorMessage, logMessages } = useConsoleSpy(); const STORED = 'apify_api_stored'; const ENV = 'apify_api_env'; -const readAuthFile = () => JSON.parse(readFileSync(AUTH_FILE_PATH(), 'utf-8')); - // A real ApifyApiError, not a look-alike: describeAuthFailure narrows on the class, so a // hand-built error would let the 401/403 branch rot without failing a test. const apiError = (statusCode: number) => @@ -135,7 +134,7 @@ describe('auth', () => { expect(await getToken()).toBe(STORED); expect(await getProxyPassword()).toBe('pw'); - expect(readAuthFile()).toMatchObject({ id: 'uid', username: 'me' }); + expect(readActiveProfile()).toMatchObject({ id: 'uid', username: 'me' }); }); it('writes nothing when the API rejects the token', async () => { @@ -239,7 +238,7 @@ describe('auth', () => { await resolveAuth(); expect(await getToken()).toBe(STORED); - expect(readAuthFile()).toMatchObject({ username: 'me' }); + expect(readActiveProfile()).toMatchObject({ username: 'me' }); }); }); }); diff --git a/test/local/lib/credentials.test.ts b/test/local/lib/credentials.test.ts index 991cd5a46..bb46f5c63 100644 --- a/test/local/lib/credentials.test.ts +++ b/test/local/lib/credentials.test.ts @@ -4,6 +4,7 @@ import process from 'node:process'; import { cryptoRandomObjectId } from '@apify/utilities'; +import { __resetAuthFileForTests } 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 { @@ -35,7 +36,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 }); @@ -52,12 +54,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()', () => { @@ -134,7 +138,9 @@ describe('credentials', () => { it('writes auth.json with mode 0600', async () => { await setToken('tok_123'); - expect(writeFileSyncSpy).toHaveBeenCalledWith(AUTH_FILE_PATH(), expect.any(String), { mode: 0o600 }); + 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 () => { @@ -337,7 +343,7 @@ describe('credentials', () => { }); describe('getLocalUserInfo()', () => { - it('on file backend, preserves non-secret proxy fields', async () => { + it('on file backend, keeps the proxy password and drops the groups nothing reads', async () => { vitest.stubEnv('APIFY_DISABLE_KEYRING', '1'); writeAuthFile({ username: 'me', @@ -347,7 +353,7 @@ 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 () => { diff --git a/test/local/lib/rental-sunset-notice.test.ts b/test/local/lib/rental-sunset-notice.test.ts index 9a038adb0..c6372647a 100644 --- a/test/local/lib/rental-sunset-notice.test.ts +++ b/test/local/lib/rental-sunset-notice.test.ts @@ -27,7 +27,24 @@ 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, + loggedInAt: null, + }, + }, + token: 'apify_api_token', + }), + ); } interface StoredRentalSunset {