feat: key secrets by user ID on both backends

src/commands/auth/login.ts

@@ -13,7 +13,7 @@ import { ApifyCommand } from '../../lib/command-framework/apify-command.js';
 import { Flags } from '../../lib/command-framework/flags.js';
 import { getConsoleIntegrationsUrl, getConsoleUrl } from '../../lib/console-url.js';
 import { AUTH_FILE_PATH, CommandExitCodes } from '../../lib/consts.js';
-import { getBackend } from '../../lib/credentials.js';
+import { backendFor } from '../../lib/credentials.js';
 import { updateUserId } from '../../lib/hooks/telemetry/useTelemetryState.js';
 import { useMaskedInput } from '../../lib/hooks/user-confirmations/useMaskedInput.js';
 import { useSelectFromList } from '../../lib/hooks/user-confirmations/useSelectFromList.js';
@@ -36,7 +36,7 @@ const tryToLogin = async (token: string) => {
 		const { userInfo } = result;
 		await updateUserId(userInfo.id!);
 
-		const backend = await getBackend();
+		const backend = await backendFor(userInfo.id!);
 		let tokenLocation: string;
 		if (backend === 'keyring') {
 			tokenLocation = 'your OS keyring';

src/commands/auth/logout.ts

@@ -1,12 +1,19 @@
+import process from 'node:process';
+
 import { APIFY_ENV_VARS } from '@apify/consts';
 
-import { removeActiveProfile } from '../../lib/auth-file.js';
+import { getActiveProfileId, removeActiveProfile } from '../../lib/auth-file.js';
 import { invalidEnvTokenMessage, readEnvToken } from '../../lib/auth.js';
 import { ApifyCommand } from '../../lib/command-framework/apify-command.js';
-import { AUTH_FILE_PATH } from '../../lib/consts.js';
-import { clearKeyringSecrets } from '../../lib/credentials.js';
+import { AUTH_FILE_PATH, CommandExitCodes } from '../../lib/consts.js';
+import {
+	clearKeyringSecrets,
+	describeLeftovers,
+	type KeyringLeftover,
+	leftoverReasons,
+} from '../../lib/credentials.js';
 import { updateUserId } from '../../lib/hooks/telemetry/useTelemetryState.js';
-import { success, warning } from '../../lib/outputs.js';
+import { error, success, warning } from '../../lib/outputs.js';
 import { tildify } from '../../lib/utils.js';
 
 export class AuthLogoutCommand extends ApifyCommand<typeof AuthLogoutCommand> {
@@ -28,13 +35,31 @@ export class AuthLogoutCommand extends ApifyCommand<typeof AuthLogoutCommand> {
 	static override docsUrl = 'https://docs.apify.com/cli/docs/reference#apify-logout';
 
 	async run() {
-		await clearKeyringSecrets();
-		removeActiveProfile();
+		// Read before either step runs: once the profile is gone, nothing names the keyring entries it owns.
+		const activeProfileId = getActiveProfileId();
+
+		// Both steps are attempted even when the first one fails, so neither the secrets nor the
+		// profile are left behind just because the other could not be removed.
+		const leftovers = await clearKeyringSecrets(activeProfileId);
+
+		let profileError: unknown = null;
+		try {
+			removeActiveProfile();
+		} catch (err) {
+			profileError = err;
+		}
 
-		await updateUserId(null);
+		// The account is off disk whenever the profile step succeeded, so the telemetry ID goes too.
+		if (!profileError) await updateUserId(null);
 
-		success({ message: 'You are logged out from your Apify account.' });
+		if (leftovers.length || profileError) {
+			error({ message: partialLogoutMessage(leftovers, profileError) });
+			process.exitCode = CommandExitCodes.RunFailed;
+		} else {
+			success({ message: 'You are logged out from your Apify account.' });
+		}
 
+		// Said either way: a half-finished logout is when this matters most.
 		const envToken = readEnvToken();
 		if (envToken.kind === 'token') {
 			warning({
@@ -45,3 +70,21 @@ export class AuthLogoutCommand extends ApifyCommand<typeof AuthLogoutCommand> {
 		}
 	}
 }
+
+function reasonOf(err: unknown) {
+	return err instanceof Error ? err.message : String(err);
+}
+
+function partialLogoutMessage(leftovers: KeyringLeftover[], profileError: unknown) {
+	const keyringPart = leftovers.length
+		? `Your secrets are still in the OS keyring at ${describeLeftovers(leftovers)}; delete them with your OS keyring app.`
+		: 'Your secrets were removed from the OS keyring.';
+
+	const profilePart = profileError
+		? `Your account is still in ${tildify(AUTH_FILE_PATH())}; delete that file to finish logging out.`
+		: `Your account was removed from ${tildify(AUTH_FILE_PATH())}.`;
+
+	const reasons = [leftoverReasons(leftovers), profileError ? reasonOf(profileError) : ''].filter(Boolean).join(' ');
+
+	return `Logout did not finish. ${keyringPart} ${profilePart} The reason was: ${reasons}`;
+}

src/lib/auth-file.ts

@@ -3,7 +3,7 @@ import { existsSync, readFileSync, renameSync, rmSync, writeFileSync } from 'nod
 import { cryptoRandomObjectId } from '@apify/utilities';
 
 import { AUTH_FILE_PATH } from './consts.js';
-import type { CredentialsBackend } from './credentials.js';
+import type { SecretKind } from './credentials.js';
 import { ensureApifyDirectory } from './files.js';
 import { warning } from './outputs.js';
 import { cliDebugPrint } from './utils/cliDebugPrint.js';
@@ -24,20 +24,23 @@ export interface AuthProfile {
 	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;
+	/**
+	 * Set only when the keyring is disabled, unavailable, or refused the write. A token here is
+	 * the record of where this profile's secrets live: no marker says so separately.
+	 */
+	token?: string;
+	proxy?: { password?: string };
 }
 
 /**
- * `auth.json` as this CLI writes it. `token` and `proxy` are the file backend's secret storage;
- * they stay outside the profiles until each profile gets its own keys.
+ * `auth.json` as this CLI writes it. Top-level `token` and `proxy` are where the file backend kept
+ * secrets before they were keyed per profile; `ensureSecretsKeyed()` moves them into the profile.
  */
 export interface AuthFile {
 	version?: number;
 	activeProfile?: string;
 	profiles?: Record<string, AuthProfile>;
-	secretsBackend?: CredentialsBackend;
 	token?: string;
 	proxy?: { password?: string; [k: string]: unknown };
 }
@@ -125,14 +128,13 @@ function v1Profile(file: LegacyAuthFile): AuthProfile {
 function toV2(file: LegacyAuthFile): AuthFile {
 	const migrated: AuthFile = { version: AUTH_FILE_VERSION, profiles: {} };
 
-	// A v1 file with a token but no ID has no key to store the profile under. Keep the secrets so
-	// the next command reports stale credentials instead of a silent logged-out state.
+	// A v1 file with a token but no ID has no key to store the profile under. The secrets are
+	// carried over here and dropped by `ensureSecretsKeyed()`, which is what forces the re-login.
 	if (typeof file.id === 'string') {
 		migrated.activeProfile = file.id;
 		migrated.profiles![file.id] = v1Profile(file);
 	}
 
-	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 };
 
@@ -173,11 +175,10 @@ async function migrateAuthFile(): Promise<void> {
 
 			writeAuthFile(migrated);
 		} catch (err) {
-			// The readers understand the old shape, so nothing is broken and the next command tries
-			// again. Still said out loud, because failing on every run should not be invisible.
+			// Not rethrown: the migration must not abort the command, which fails at the auth step.
 			cliDebugPrint('auth-file', 'auth file migration failed', err);
 			warning({
-				message: `Your login still works, but ${AUTH_FILE_PATH()} could not be updated to the current format. Set APIFY_CLI_DEBUG=1 to see why.`,
+				message: `Your stored login cannot be read until ${AUTH_FILE_PATH()} is updated to the current format, and the update failed. Make the directory it is in writable, then run the command again. Set APIFY_CLI_DEBUG=1 to see why.`,
 			});
 		}
 	})();
@@ -237,19 +238,92 @@ export function getActiveProfile(): (AuthProfile & { id: string }) | undefined {
 }
 
 /**
- * Replaces the file with this one account. A second profile would name an account that cannot
- * authenticate until each has its own secret, and dropping the old secrets is what keeps the write
- * safe: the caller writes the new token next, so a failure there leaves nobody logged in rather
- * than the old token beside the new name. Additive login is #1386.
+ * The user ID every secret is keyed by. Taken from `activeProfile` rather than from the profile
+ * object, so a file whose `activeProfile` names a missing profile still resolves its secrets and
+ * reports the dangling profile instead of looking logged out.
+ */
+export function getActiveProfileId(): string | undefined {
+	const file = readAuthFile();
+
+	if (file.version !== AUTH_FILE_VERSION) {
+		const legacy = file as LegacyAuthFile;
+		return typeof legacy.id === 'string' ? legacy.id : undefined;
+	}
+
+	return file.activeProfile;
+}
+
+/** The file backend's stored secret, or `undefined` when the profile does not hold one. */
+export function readProfileSecret(userId: string, kind: SecretKind): string | undefined {
+	const profile = readAuthFile().profiles?.[userId];
+	if (!profile) return undefined;
+
+	return kind === 'token' ? profile.token : profile.proxy?.password;
+}
+
+/**
+ * Stores a file-backend secret on the profile. A missing profile is left alone: inventing one
+ * would fabricate the account metadata the CLI reads.
+ */
+export function writeProfileSecret(userId: string, kind: SecretKind, value: string) {
+	updateProfile(userId, (profile) => setProfileSecret(profile, kind, value));
+}
+
+/** Forgets every file-backend secret of a profile, once its token is in the keyring. */
+export function clearProfileFileSecrets(userId: string) {
+	const profile = readAuthFile().profiles?.[userId];
+	if (profile?.token === undefined && profile?.proxy === undefined) return;
+
+	updateProfile(userId, (edited) => {
+		delete edited.token;
+		delete edited.proxy;
+	});
+}
+
+function setProfileSecret(profile: AuthProfile, kind: SecretKind, value: string) {
+	if (kind === 'token') {
+		profile.token = value;
+	} else {
+		profile.proxy = { ...profile.proxy, password: value };
+	}
+}
+
+/** Forgets one of a profile's file-backend secrets. */
+export function deleteProfileSecret(userId: string, kind: SecretKind) {
+	if (readProfileSecret(userId, kind) === undefined) return;
+
+	updateProfile(userId, (profile) => {
+		if (kind === 'token') {
+			delete profile.token;
+		} else {
+			// The profile's proxy object carries nothing but the password.
+			delete profile.proxy;
+		}
+	});
+}
+
+function updateProfile(userId: string, edit: (profile: AuthProfile) => void) {
+	const file = readAuthFile();
+	const profile = file.profiles?.[userId];
+	if (!profile) return;
+
+	edit(profile);
+	writeAuthFile(file);
+}
+
+/**
+ * Replaces the file with this one account, dropping any previous profile and its secrets. Nothing
+ * puts a second profile there yet; additive login is #1386. Dropping the old secrets is what keeps
+ * the write safe: the caller writes the new token next, so a failure there leaves nobody logged in
+ * rather than the old token beside the new name.
  */
-export function replaceStoredAccount(userId: string, profile: AuthProfile, secretsBackend: CredentialsBackend) {
+export function replaceStoredAccount(userId: string, profile: AuthProfile) {
 	assertSupportedAuthFileVersion();
 
 	writeAuthFile({
 		version: AUTH_FILE_VERSION,
 		activeProfile: userId,
 		profiles: { [userId]: profile },
-		secretsBackend,
 	});
 }
 

src/lib/auth.ts

@@ -6,15 +6,16 @@ import { AxiosHeaders } from 'axios';
 
 import { APIFY_ENV_VARS } from '@apify/consts';
 
-import { ensureAuthFileCurrent, replaceStoredAccount } from './auth-file.js';
+import { getActiveProfileId, replaceStoredAccount } from './auth-file.js';
 import { APIFY_CLIENT_DEFAULT_HEADERS, AUTH_FILE_PATH, CommandExitCodes } from './consts.js';
 import {
-	deleteProxyPassword,
+	clearKeyringSecrets,
+	deleteSecret,
+	describeLeftovers,
+	ensureCredentialsCurrent,
 	ensureMigrated,
-	getBackend,
-	getToken,
-	setProxyPassword,
-	setToken,
+	getSecret,
+	setSecret,
 } from './credentials.js';
 import { warning } from './outputs.js';
 import type { AuthJSON } from './types.js';
@@ -69,7 +70,7 @@ export function __resetAuthForTests() {
  * The single token resolver. Order: `APIFY_TOKEN` -> stored login. Inside a platform run there is
  * no stored login, so `APIFY_TOKEN` wins without a special case for the `actor` entrypoint.
  *
- * Single-flighted like {@link getBackend}, because several callers resolve per command and reading
+ * Single-flighted like `getBackend()`, because several callers resolve per command and reading
  * the stored token is an uncached OS keyring hit.
  *
  * Read-only by contract, apart from the one-shot migration of an existing plaintext auth.json.
@@ -97,9 +98,10 @@ export const resolveAuth = async (): Promise<ResolvedAuth | undefined> => {
 
 		// Only now, because the stored file is not this command's credential when APIFY_TOKEN is
 		// set. A file a newer CLI wrote would otherwise stop a platform run that never reads it.
-		await ensureAuthFileCurrent();
+		await ensureCredentialsCurrent();
 
-		const storedToken = await getToken();
+		const userId = getActiveProfileId();
+		const storedToken = userId ? await getSecret(userId, 'token') : undefined;
 		return storedToken ? ({ token: storedToken, source: 'stored' } as const) : undefined;
 	})();
 
@@ -177,28 +179,41 @@ export async function loginWithToken(
 
 	const proxyPassword = userInfo.proxy?.password;
 
+	const previousUserId = getActiveProfileId();
+
 	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(),
-	);
+	replaceStoredAccount(userInfo.id, {
+		username: userInfo.username,
+		name: null,
+		...(organizationOwnerUserId ? { organizationOwnerUserId } : {}),
+		authMethod: 'token',
+		expiresAt: null,
+		hasRefreshToken: false,
+		loggedInAt: new Date().toISOString(),
+	});
+
+	// Only once the switch is on disk: a failed write leaves auth.json naming the previous account,
+	// whose entries nothing else can find. The fixed names go every time, stale by then either way.
+	const staleUserId = previousUserId === userInfo.id ? undefined : previousUserId;
+	const leftovers = await clearKeyringSecrets(staleUserId);
 
 	// After the account, which drops the previous secrets. `skipIfUnchanged` avoids a Keychain prompt.
-	await setToken(token, { skipIfUnchanged: true });
+	await setSecret(userInfo.id, 'token', token, { skipIfUnchanged: true });
 
 	if (proxyPassword) {
-		await setProxyPassword(proxyPassword, { skipIfUnchanged: true });
+		await setSecret(userInfo.id, 'proxy-password', proxyPassword, { skipIfUnchanged: true });
 	} else {
-		await deleteProxyPassword();
+		// A refused delete leaves the revoked password where every read looks first.
+		const leftover = await deleteSecret(userInfo.id, 'proxy-password');
+		if (leftover) leftovers.push(leftover);
+	}
+
+	if (leftovers.length) {
+		warning({
+			message:
+				`Secrets this login could not remove are still in the OS keyring at ` +
+				`${describeLeftovers(leftovers)}; delete them with your OS keyring app.`,
+		});
 	}
 
 	return { client: apifyClient, userInfo };