feat: key secrets by user ID on both backends

src/commands/auth/logout.ts

@@ -1,12 +1,14 @@
+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 { AUTH_FILE_PATH, CommandExitCodes } from '../../lib/consts.js';
 import { clearKeyringSecrets } 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,10 +30,28 @@ export class AuthLogoutCommand extends ApifyCommand<typeof AuthLogoutCommand> {
 	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();
+		// 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 keyringError = await clearKeyringSecrets(activeProfileId).then(
+			() => null,
+			(err: unknown) => err,
+		);
+
+		let profileError: unknown = null;
+		try {
+			removeActiveProfile();
+		} catch (err) {
+			profileError = err;
+		}
+
+		if (keyringError || profileError) {
+			error({ message: partialLogoutMessage(activeProfileId, keyringError, profileError) });
+			process.exitCode = CommandExitCodes.RunFailed;
+			return;
+		}
 
 		await updateUserId(null);
 
@@ -47,3 +67,21 @@ export class AuthLogoutCommand extends ApifyCommand<typeof AuthLogoutCommand> {
 		}
 	}
 }
+
+function reasonOf(err: unknown) {
+	return err instanceof Error ? err.message : String(err);
+}
+
+function partialLogoutMessage(activeProfileId: string | undefined, keyringError: unknown, profileError: unknown) {
+	const keyringPart = keyringError
+		? `Your secrets are still in the OS keyring${activeProfileId ? ` under the account ${activeProfileId}` : ''}; delete them with your OS keyring app.`
+		: 'Your secrets were removed from the OS keyring.';
+
+	const profilePart = profileError
+		? `Your account is still in ${AUTH_FILE_PATH()}; delete that file to finish logging out.`
+		: `Your account was removed from ${AUTH_FILE_PATH()}.`;
+
+	const reasons = [keyringError, profileError].filter(Boolean).map(reasonOf).join(' ');
+
+	return `Logout did not finish. ${keyringPart} ${profilePart} ${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 { CredentialsBackend, SecretKind } from './credentials.js';
 import { ensureApifyDirectory } from './files.js';
 import { warning } from './outputs.js';
 import { cliDebugPrint } from './utils/cliDebugPrint.js';
@@ -26,14 +26,21 @@ export interface AuthProfile {
 	authMethod: 'token';
 	expiresAt: string | null;
 	hasRefreshToken: boolean;
-	/** Reserved: a keyring failure on one profile must not redirect another profile's reads. */
+	/**
+	 * Where this profile's secrets live, when that differs from the file-level `secretsBackend`.
+	 * Written only when a keyring write for this profile fails, so one profile falling back to the
+	 * file cannot silently redirect another profile's reads to a place its secrets are not.
+	 */
 	secretsBackend?: CredentialsBackend;
 	loggedInAt: string | null;
+	/** File backend only. The keyring backend keeps these in the OS store instead. */
+	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;
@@ -127,8 +134,8 @@ 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);
@@ -190,11 +197,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.`,
 			});
 		}
 	})();
@@ -254,10 +260,90 @@ 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;
+}
+
+/** Where this profile's secrets live, or `undefined` when it follows the file-level default. */
+export function readProfileBackend(userId: string): CredentialsBackend | undefined {
+	return readAuthFile().profiles?.[userId]?.secretsBackend;
+}
+
+/**
+ * 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));
+}
+
+/**
+ * Stores the secret and records that this profile reads from the file from now on, in one write.
+ * Called when a keyring write for this profile failed: splitting the two would leave a window
+ * where the profile looks logged out, or where it still points at a keyring entry that is not there.
+ */
+export function moveProfileSecretToFile(userId: string, kind: SecretKind, value: string) {
+	updateProfile(userId, (profile) => {
+		setProfileSecret(profile, kind, value);
+		profile.secretsBackend = 'file';
+	});
+}
+
+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) {
 	assertSupportedAuthFileVersion();

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 { ensureAuthFileCurrent, getActiveProfileId, replaceStoredAccount } from './auth-file.js';
 import { APIFY_CLIENT_DEFAULT_HEADERS, AUTH_FILE_PATH, CommandExitCodes } from './consts.js';
 import {
-	deleteProxyPassword,
+	clearKeyringSecrets,
+	deleteSecret,
 	ensureMigrated,
+	ensureSecretsKeyed,
 	getBackend,
-	getToken,
-	setProxyPassword,
-	setToken,
+	getSecret,
+	setSecret,
 } from './credentials.js';
 import { warning } from './outputs.js';
 import type { AuthJSON } from './types.js';
@@ -98,8 +99,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 ensureSecretsKeyed();
 
-		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,6 +180,8 @@ export async function loginWithToken(
 
 	const proxyPassword = userInfo.proxy?.password;
 
+	const previousUserId = getActiveProfileId();
+
 	const { organizationOwnerUserId } = userInfo as { organizationOwnerUserId?: string };
 	replaceStoredAccount(
 		userInfo.id,
@@ -192,13 +197,18 @@ export async function loginWithToken(
 		await getBackend(),
 	);
 
+	// Only once the switch is on disk: a failed write leaves auth.json naming the previous account, whose entries nothing else can find.
+	if (previousUserId && previousUserId !== userInfo.id) {
+		await clearKeyringSecrets(previousUserId);
+	}
+
 	// 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();
+		await deleteSecret(userInfo.id, 'proxy-password');
 	}
 
 	return { client: apifyClient, userInfo };