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..f74625c3f --- /dev/null +++ b/src/lib/auth-file.ts @@ -0,0 +1,247 @@ +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 } from './credentials.js'; +import { ensureApifyDirectory } from './files.js'; +import { cliDebugPrint } from './utils/cliDebugPrint.js'; + +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; +} + +/** + * `auth.json` as it sits on disk. `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 }; + [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. 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; +} + +/** 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. + */ +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; +} + +/** + * 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 b2793e193..6dd005e7b 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, setActiveProfile } 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'; @@ -81,6 +80,7 @@ export function __resetAuthForTests() { export const resolveAuth = async (): Promise => { authPromise ??= (async () => { await ensureMigrated(); + await ensureAuthFileCurrent(); const envToken = readEnvToken(); if (envToken.kind === 'invalid') { @@ -168,15 +168,27 @@ 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 }); + // 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(), + ); // After the metadata file, which would clobber them on the file backend. `skipIfUnchanged` avoids a Keychain prompt. await setToken(token, { skipIfUnchanged: true }); diff --git a/src/lib/credentials.ts b/src/lib/credentials.ts index 3758cd502..c6e5ff1d6 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 { 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; 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 4a35f730e..b76561836 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'; @@ -84,33 +85,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.'); + // A token with no profile behind it is 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/local/commands/auth.test.ts b/test/local/commands/auth.test.ts index 9daaff2a3..b83c92797 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,17 @@ 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, }); expect(lastErrorMessage()).toContain('You are logged in to Apify as me'); }); @@ -74,7 +77,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 +85,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 +174,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 +188,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..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..8afc82c81 --- /dev/null +++ b/test/local/lib/auth-file.test.ts @@ -0,0 +1,255 @@ +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 { 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 { + 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 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 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', 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('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', + }); + expect(readBackup()).toEqual({ token: 'apify_api_v1_token', 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(() => 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(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..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 {