diff --git a/src/commands/actor/charge.ts b/src/commands/actor/charge.ts index dc2bd9ebb..b12a422a3 100644 --- a/src/commands/actor/charge.ts +++ b/src/commands/actor/charge.ts @@ -1,6 +1,6 @@ import { APIFY_ENV_VARS } from '@apify/consts'; -import { getApifyTokenFromEnvOrAuthFile } from '../../lib/actor.js'; +import { getApifyToken } from '../../lib/actor.js'; import { ApifyCommand } from '../../lib/command-framework/apify-command.js'; import { Args } from '../../lib/command-framework/args.js'; import { Flags } from '../../lib/command-framework/flags.js'; @@ -86,7 +86,7 @@ export class ActorChargeCommand extends ApifyCommand return; } - const apifyToken = await getApifyTokenFromEnvOrAuthFile(); + const apifyToken = await getApifyToken(); const apifyClient = await getLoggedClient(apifyToken); if (!apifyClient) { throw new Error('Apify token is not set. Please set it using the environment variable APIFY_TOKEN.'); diff --git a/src/commands/auth/login.ts b/src/commands/auth/login.ts index 0f1fd0d58..756116d8e 100644 --- a/src/commands/auth/login.ts +++ b/src/commands/auth/login.ts @@ -32,7 +32,7 @@ const API_VERSION = 'v1'; const tryToLogin = async (token: string) => { const apiBaseUrl = getConsoleUrl().includes('localhost') ? LOCAL_API_BASE_URL : undefined; - const isUserLogged = await getLoggedClient(token, apiBaseUrl); + const isUserLogged = await getLoggedClient(token, apiBaseUrl, { persistCredentials: true }); const userInfo = await getLocalUserInfo(); if (isUserLogged) { diff --git a/src/commands/auth/token.ts b/src/commands/auth/token.ts index c2a307e88..4f962380a 100644 --- a/src/commands/auth/token.ts +++ b/src/commands/auth/token.ts @@ -1,6 +1,6 @@ import { ApifyCommand } from '../../lib/command-framework/apify-command.js'; import { simpleLog } from '../../lib/outputs.js'; -import { getLocalUserInfo, getLoggedClientOrThrow } from '../../lib/utils.js'; +import { getLoggedClientOrThrow, resolveToken } from '../../lib/utils.js'; export class AuthTokenCommand extends ApifyCommand { static override name = 'token' as const; @@ -9,7 +9,7 @@ export class AuthTokenCommand extends ApifyCommand { static override examples = [ { - description: 'Print the stored API token to stdout (use with care — it is a secret).', + description: 'Print the API token in use to stdout (use with care — it is a secret).', command: 'apify auth token', }, ]; @@ -18,10 +18,11 @@ export class AuthTokenCommand extends ApifyCommand { async run() { await getLoggedClientOrThrow(); - const userInfo = await getLocalUserInfo(); + // Must match what the other commands actually authenticate with, so APIFY_TOKEN wins over the stored login. + const token = await resolveToken(); - if (userInfo.token) { - simpleLog({ message: userInfo.token, stdout: true }); + if (token) { + simpleLog({ message: token, stdout: true }); } } } diff --git a/src/commands/run.ts b/src/commands/run.ts index 050e3e3c6..2e9f5c8a3 100644 --- a/src/commands/run.ts +++ b/src/commands/run.ts @@ -328,7 +328,8 @@ export class RunCommand extends ApifyCommand { if (proxy && proxy.password) localEnvVars[APIFY_ENV_VARS.PROXY_PASSWORD] = proxy.password; if (userId) localEnvVars[APIFY_ENV_VARS.USER_ID] = userId; - if (token) localEnvVars[APIFY_ENV_VARS.TOKEN] = token; + // Don't clobber an explicitly-set APIFY_TOKEN inherited from the environment — it must win over the stored login. + if (token && !process.env[APIFY_ENV_VARS.TOKEN]) localEnvVars[APIFY_ENV_VARS.TOKEN] = token; if (localConfig!.environmentVariables) { const updatedEnv = replaceSecretsValue(localConfig!.environmentVariables as Record, undefined, { allowMissing: this.flags.allowMissingSecrets, diff --git a/src/lib/actor.ts b/src/lib/actor.ts index f0823c2c7..950bd7c6e 100644 --- a/src/lib/actor.ts +++ b/src/lib/actor.ts @@ -9,7 +9,7 @@ import { ApifyClient } from 'apify-client'; import { ACTOR_ENV_VARS, APIFY_ENV_VARS, KEY_VALUE_STORE_KEYS, LOCAL_ACTOR_ENV_VARS } from '@apify/consts'; -import { getApifyClientOptions, getLocalStorageDir, getLocalUserInfo } from './utils.js'; +import { getApifyClientOptions, getLocalStorageDir, resolveToken } from './utils.js'; export const APIFY_STORAGE_TYPES = { KEY_VALUE_STORE: 'KEY_VALUE_STORE', @@ -18,23 +18,18 @@ export const APIFY_STORAGE_TYPES = { } as const; /** - * Returns Apify token from environment variable or local auth file. + * Returns the Apify token to use — the `APIFY_TOKEN` env var, else the token stored by `apify login`. * @returns Apify token */ -export const getApifyTokenFromEnvOrAuthFile = async () => { - const apifyToken = process.env[APIFY_ENV_VARS.TOKEN]; - if (apifyToken) { - return apifyToken; - } - - const localUserInfo = await getLocalUserInfo(); - if (!localUserInfo || !localUserInfo.token) { +export const getApifyToken = async () => { + const apifyToken = await resolveToken(); + if (!apifyToken) { throw new Error( 'Apify token is not set. Please set it using the environment variable APIFY_TOKEN or apify login command.', ); } - return localUserInfo.token; + return apifyToken; }; /** @@ -54,7 +49,7 @@ export const getApifyStorageClient = async ( ...options, }); } - const apifyToken = await getApifyTokenFromEnvOrAuthFile(); + const apifyToken = await getApifyToken(); return new ApifyClient({ ...(await getApifyClientOptions(apifyToken)), diff --git a/src/lib/utils.ts b/src/lib/utils.ts index 99d8784e8..d0a3482df 100644 --- a/src/lib/utils.ts +++ b/src/lib/utils.ts @@ -86,10 +86,19 @@ 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. + * + * auth.json only ever describes the account `apify login` stored, and `APIFY_TOKEN` exists to act + * as a different one (typically an organization), so the identity is read from the API whenever it + * is set. */ export const getLocalUserInfo = async (): Promise => { await ensureMigrated(); + const overrideToken = process.env[APIFY_ENV_VARS.TOKEN]; + if (overrideToken) { + return { ...(await fetchActiveUserInfo(overrideToken)), token: overrideToken }; + } + let result: AuthJSON = {}; try { const raw = await readFile(AUTH_FILE_PATH(), 'utf-8'); @@ -98,10 +107,10 @@ export const getLocalUserInfo = async (): Promise => { // auth.json may not exist yet (fresh keyring-only state); fall through } - if ((await getBackend()) === 'keyring') { - const token = await getToken(); - if (token) result.token = token; + const storedToken = await getToken(); + if (storedToken) result.token = storedToken; + if ((await getBackend()) === 'keyring') { const proxyPassword = await getProxyPassword(); if (proxyPassword) result.proxy = { ...result.proxy, password: proxyPassword }; } @@ -129,8 +138,16 @@ export async function getLoggedClientOrThrow() { return loggedClient; } -const resolveToken = async (existingToken?: string): Promise => { +/** + * Resolves the token to use, in order: explicitly passed token (e.g. `--token`) > + * `APIFY_TOKEN` env var > the token stored by `apify login`. + * + * The first two are one-time overrides and must never be written over the stored login — + * see the `persistCredentials` option of {@link getLoggedClient}. + */ +export const resolveToken = async (existingToken?: string): Promise => { if (existingToken) return existingToken; + if (process.env[APIFY_ENV_VARS.TOKEN]) return process.env[APIFY_ENV_VARS.TOKEN]; await ensureMigrated(); return getToken(); }; @@ -160,13 +177,41 @@ export const getApifyClientOptions = async (token?: string, apiBaseUrl?: string) }; }; +/** + * User info of the account the process authenticates as, keyed by the token it was fetched with. + * Seeded by {@link getLoggedClient} from the user info it fetches anyway, so commands that get a + * client first (nearly all of them) pay no extra API call. + */ +let activeUserInfo: { token: string; info: AuthJSON } | undefined; + +async function fetchActiveUserInfo(token: string): Promise { + if (activeUserInfo?.token !== token) { + const client = new ApifyClient(await getApifyClientOptions(token)); + try { + activeUserInfo = { token, info: await client.user('me').get() }; + } catch (err) { + cliDebugPrint('[fetchActiveUserInfo] error getting user info', { error: err }); + throw new Error(`The token in ${APIFY_ENV_VARS.TOKEN} was rejected by the Apify API. Is it still valid?`); + } + } + + return activeUserInfo.info; +} + /** * Gets instance of ApifyClient for token or for params from global auth file. * - * Refreshes the user metadata in auth.json each run. Secrets (token, proxy.password) only - * get written when their value actually changes — avoids macOS Keychain prompts on every command. + * Secrets (token, proxy.password) and user metadata are only written when `persistCredentials` + * is set — i.e. from `apify login`. Every other caller resolves a possibly-overridden token + * (`--token`, `APIFY_TOKEN`), and persisting that would silently replace the stored login. + * When writing, secrets only change on disk if their value differs — avoids macOS Keychain + * prompts on every command. */ -export async function getLoggedClient(token?: string, apiBaseUrl?: string) { +export async function getLoggedClient( + token?: string, + apiBaseUrl?: string, + { persistCredentials = false }: { persistCredentials?: boolean } = {}, +) { const resolvedToken = await resolveToken(token); const apifyClient = new ApifyClient(await getApifyClientOptions(resolvedToken, apiBaseUrl)); @@ -179,6 +224,14 @@ export async function getLoggedClient(token?: string, apiBaseUrl?: string) { return null; } + if (apifyClient.token) { + activeUserInfo = { token: apifyClient.token, info: userInfo }; + } + + if (!persistCredentials) { + return apifyClient; + } + if (apifyClient.token) { await setToken(apifyClient.token, { skipIfUnchanged: true }); } diff --git a/test/local/commands/run.test.ts b/test/local/commands/run.test.ts index bc4e26323..bd438ab92 100644 --- a/test/local/commands/run.test.ts +++ b/test/local/commands/run.test.ts @@ -216,6 +216,33 @@ describe('apify run', () => { expect(actOutput2).toStrictEqual('can play'); }); + // Regression: an inherited APIFY_TOKEN must reach the Actor unchanged, not be clobbered by the stored login token. + it(`[api] does not override an inherited ${APIFY_ENV_VARS.TOKEN} with the stored token`, async () => { + await safeLogin(); + + const auth = JSON.parse(readFileSync(AUTH_FILE_PATH(), 'utf8')); + const inheritedToken = `${auth.token}_inherited`; + vitest.stubEnv(APIFY_ENV_VARS.TOKEN, inheritedToken); + + const actCode = ` + import { Actor } from 'apify'; + + Actor.main(async () => { + await Actor.setValue('OUTPUT', process.env); + console.log('Done.'); + }); + `; + writeFileSync(joinPath('src/main.js'), actCode, { flag: 'w' }); + + await testRunCommand(RunCommand, {}); + + const actOutputPath = joinPath(getLocalKeyValueStorePath(), 'OUTPUT.json'); + const localEnvVars = JSON.parse(readFileSync(actOutputPath, 'utf8')); + + expect(localEnvVars[APIFY_ENV_VARS.TOKEN]).toStrictEqual(inheritedToken); + expect(localEnvVars[APIFY_ENV_VARS.TOKEN]).not.toStrictEqual(auth.token); + }); + it('run purge stores', async () => { const input = { myInput: 'value', diff --git a/test/local/lib/credentials.test.ts b/test/local/lib/credentials.test.ts index 4a0566ed8..0ee9bdc21 100644 --- a/test/local/lib/credentials.test.ts +++ b/test/local/lib/credentials.test.ts @@ -14,7 +14,27 @@ import { setProxyPassword, setToken, } from '../../../src/lib/credentials.js'; -import { getLocalUserInfo } from '../../../src/lib/utils.js'; +import { getApifyClientOptions, getLocalUserInfo, getLoggedClient } from '../../../src/lib/utils.js'; + +// Stubs out the `user('me').get()` round-trip so getLoggedClient() can be tested without the API. +vi.mock('apify-client', () => { + class ApifyClient { + token?: string; + constructor(options: { token?: string }) { + this.token = options.token; + } + user() { + return { + get: async () => ({ + id: `id_for_${this.token}`, + username: `user_for_${this.token}`, + proxy: { password: `pw_for_${this.token}` }, + }), + }; + } + } + return { ApifyClient }; +}); const keyringStore = new Map(); const keyringFailures = new Set(); @@ -49,6 +69,7 @@ const readAuthFile = () => JSON.parse(readFileSync(AUTH_FILE_PATH(), 'utf-8')); describe('credentials', () => { beforeEach(() => { vitest.stubEnv('__APIFY_INTERNAL_TEST_AUTH_PATH__', cryptoRandomObjectId(12)); + vitest.stubEnv('APIFY_TOKEN', undefined); keyringStore.clear(); keyringFailures.clear(); __resetCredentialsForTests(); @@ -277,5 +298,89 @@ describe('credentials', () => { expect(info.token).toBe('tok_kr'); expect(info.proxy?.password).toBe('pw_kr'); }); + + it('describes the APIFY_TOKEN account, not the stored login', async () => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', '1'); + writeAuthFile({ username: 'stored', id: 'stored_id', token: 'stored_tok', secretsBackend: 'file' }); + vitest.stubEnv('APIFY_TOKEN', 'env_tok_a'); + + const info = await getLocalUserInfo(); + expect(info).toMatchObject({ + username: 'user_for_env_tok_a', + id: 'id_for_env_tok_a', + token: 'env_tok_a', + }); + expect(info.proxy?.password).toBe('pw_for_env_tok_a'); + }); + + it('describes the APIFY_TOKEN account when there is no stored login at all', async () => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', '1'); + vitest.stubEnv('APIFY_TOKEN', 'env_tok_b'); + + expect(await getLocalUserInfo()).toMatchObject({ username: 'user_for_env_tok_b', id: 'id_for_env_tok_b' }); + }); + }); + + // Precedence: explicit token arg (e.g. --token) > APIFY_TOKEN env var > stored login token. + // Regression guard for the env var being ignored in favour of the stored token. + describe('token resolution precedence (getApifyClientOptions)', () => { + it('prefers the APIFY_TOKEN env var over the stored token', async () => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', '1'); + await setToken('stored_tok'); + vitest.stubEnv('APIFY_TOKEN', 'env_tok'); + const { token } = await getApifyClientOptions(); + expect(token).toBe('env_tok'); + }); + + it('prefers an explicitly passed token over the APIFY_TOKEN env var', async () => { + vitest.stubEnv('APIFY_TOKEN', 'env_tok'); + const { token } = await getApifyClientOptions('explicit_tok'); + expect(token).toBe('explicit_tok'); + }); + + it('falls back to the stored token when APIFY_TOKEN is not set', async () => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', '1'); + vitest.stubEnv('APIFY_TOKEN', ''); + await setToken('stored_tok'); + const { token } = await getApifyClientOptions(); + expect(token).toBe('stored_tok'); + }); + }); + + // A one-time token override (APIFY_TOKEN / --token) must never be written over the durable + // `apify login` credentials — only `apify login` itself persists. + describe('getLoggedClient() credential persistence', () => { + beforeEach(async () => { + vitest.stubEnv('APIFY_DISABLE_KEYRING', '1'); + await setToken('stored_tok'); + writeAuthFile({ ...readAuthFile(), username: 'stored_user', id: 'stored_id' }); + }); + + it('uses APIFY_TOKEN without overwriting the stored login', async () => { + vitest.stubEnv('APIFY_TOKEN', 'env_tok'); + + const client = await getLoggedClient(); + + expect(client?.token).toBe('env_tok'); + expect(await getToken()).toBe('stored_tok'); + expect(readAuthFile().username).toBe('stored_user'); + expect(readAuthFile().id).toBe('stored_id'); + }); + + it('uses an explicitly passed token without overwriting the stored login', async () => { + const client = await getLoggedClient('flag_tok'); + + expect(client?.token).toBe('flag_tok'); + expect(await getToken()).toBe('stored_tok'); + expect(readAuthFile().username).toBe('stored_user'); + }); + + it('persists the token and user metadata when persistCredentials is set (apify login)', async () => { + const client = await getLoggedClient('login_tok', undefined, { persistCredentials: true }); + + expect(client?.token).toBe('login_tok'); + expect(await getToken()).toBe('login_tok'); + expect(readAuthFile().username).toBe('user_for_login_tok'); + }); }); });