test: pin auth behavior before the multi-account refactor - #1417
Merged
Merged
Conversation
Stage-0 of #1297. The keyring backend — the default for real users — was never reached through a command: useAuthSetup and run-cli both pin APIFY_DISABLE_KEYRING=1, and the auth commands only ran under test:api. - Shared @napi-rs/keyring fake in test/__setup__/keyring-mock.ts, plus a useKeyringBackend() hook so one file can cover both backends - New test/local/commands/auth.test.ts stubs apify-client, so login, logout and auth token now run in test:local on both backends - credentials.test.ts: writeFileSync spy so the skipIfUnchanged tests can fail, 0o600 assertions, keyring write failures for setToken and setProxyPassword, the stale-credentials throw, resolveToken happy path - e2e auth token now compares stdout to the token, not to length > 0 Every new test was mutation-verified against the branch it covers. Closes #1387 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
From the second staff review: - getBackend() reading a persisted secretsBackend marker is what makes a keyring downgrade survive across processes; it had no test - every login in auth.test.ts used one identity, so the merge in getLoggedClient was never given a differing user. Stage-1 rewrites exactly that merge Also imports process in credentials.test.ts and notes the nesting constraint on useKeyringBackend. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
github-actions
Bot
added
t-tooling
l2ysho
marked this pull request as ready for review
September 10, 2026 12:40
1 task
DaveHanns
approved these changes
Sep 21, 2026
l2ysho
added a commit
that referenced
this pull request
Sep 22, 2026
Resolves conflicts in the three auth test files. #1417 was squash-merged into master, so master's copies are content-identical to this branch's ancestor; the branch versions supersede them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
l2ysho
added a commit
that referenced
this pull request
Sep 23, 2026
> [!NOTE] > **TL;DR** > - `APIFY_TOKEN` now has max priority if set, check comparison tables below for more info > - `resolveAuth()` replaces `getApifyTokenFromEnvOrAuthFile` and `resolveApifyToken` -> solving issues with rewriting auth.json when not needed > - `--token` available for `auth login` and `mcp install` Stacked on #1417 (Stage-0). Base branch is `claude/github-issue-1387-d39d52`, not `master`. Closes [#1418](#1418) and [#720](#720). ## Checklist - [x] Check `skills/apify/SKILL.md` after the implementation lands ## Behavior, before and after Run against two real accounts, **account-a** (logged in) and **account-b** (`$B`), with 1.10.0 side by side. ### `APIFY_TOKEN` no longer ignored | Command | Before | Now | |---|---|---| | `APIFY_TOKEN=$B apify actors ls` | lists **account-a**'s Actors | lists **account-b**'s Actors | | `APIFY_TOKEN=$B apify actors ls` *(logged out)* | `Error: You are not logged in` | lists **account-b**'s Actors | | `APIFY_TOKEN=$B apify info` | `username: account-a` | `username: account-b` + `token source: APIFY_TOKEN environment variable` | | `APIFY_TOKEN=$B apify auth token` | prints **account-a**'s token | prints **account-b**'s token | | `APIFY_TOKEN=$B apify actors info <name>` | resolves `account-a/<name>` → not found | resolves `account-b/<name>` → found | ### `apify run` handed the child the wrong identity | Command | Before | Now | |---|---|---| | `APIFY_TOKEN=$B apify run` | child gets **account-a**'s token, user id and proxy password | child gets **account-b**'s | This is the "Insufficient permissions for the Actor run" report, and why it started working after `apify logout`. ### Reads overwrote the stored login | Command | Before | Now | |---|---|---| | `APIFY_IS_AT_HOME=1 APIFY_TOKEN=$B apify actor charge result-item` | errors on the missing run id — **and `auth.json` now holds account-b**. Unsetting `APIFY_TOKEN` does not restore account-a | same error, `auth.json` untouched | ### A misconfigured `APIFY_TOKEN` fails instead of guessing | Command | Before | Now | |---|---|---| | `APIFY_TOKEN=undefined apify actors ls` | silently uses the stored login | `Error: APIFY_TOKEN is set to "undefined", which is not an API token.` exit 5 | | `APIFY_TOKEN= apify actors ls` | silently uses the stored login | unchanged — an unset variable is not worth a message | | `APIFY_TOKEN=<revoked> apify actors ls` | silently succeeds as **account-a** | `Error: The API token in APIFY_TOKEN was rejected.` | | `APIFY_TOKEN=$B apify login --token $A` | `Success: logged in as account-a`, while every later command ran as account-b | `Error:` exit 5. Same token stays silent — that is the CI idiom | | `APIFY_TOKEN=undefined apify login --token $A` | success, then every later command failed | warns and proceeds: login is the only way to fix the shell | | `apify login --token <bad>` | exit **0** — `login && push` ran on | exit **1** | ## What changed - One `resolveAuth()` in the new `src/lib/auth.ts`. `getApifyTokenFromEnvOrAuthFile` and `resolveApifyToken` are gone. No special case for the `actor` entrypoint: inside a platform run there is no stored login, so `APIFY_TOKEN` wins on its own. - Resolve is split from persist. `loginWithToken()` is the only credential writer, and it replaces the stored account rather than merging into it — including the keyring proxy password, which used to survive a re-login and leak the wrong account's credential into `apify run`'s child. - `resolveAuth` is single-flighted. It ran 2–4× per command, each an uncached OS keyring read. - One reader for `APIFY_TOKEN`, returning unset / invalid / token, so `login` and `logout` see a value every other command rejects. - Commands that address resources by `username` moved to `getCurrentUserInfo()`, which reads the resolved account rather than `auth.json`. - `--token` stays on `login` and `mcp install` only. No global flag — `APIFY_TOKEN` already does that job, and two mechanisms for one thing is worse. Closes #1418. Closes #720. ## Verification - `pnpm run test:local` — 621 passed, 4 skipped. - Same with `APIFY_TOKEN` set to a bogus value, which is deliberate: a host token must not decide which account the tests run as. - Lint, format, build, `update-docs` clean. - `pnpm run test:api` not run — no token in this environment. - The before/after table above was run by hand against two real accounts. ## Upgrade note A stale `APIFY_TOKEN` in a shell profile or CI job now decides which account commands run as. If it holds a placeholder, commands fail rather than falling back. Worth a release note. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Edyta <142720610+szaganek@users.noreply.github.com>
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
Stage-0 of #1297. Test-only, main reason is to have better coverage of auth flow before we start with #1297 .
Why
useAuthSetupandrun-cliboth pinAPIFY_DISABLE_KEYRING=1, so no test ever hit the keyring path.src/commands/auth/*ran only undertest:api/test:e2e, which needTEST_USER_TOKEN.getLoggedClientis a secondauth.jsonwriter, separate fromcredentials.ts. Its keyring branch — striptoken, stripproxy.password, dropproxywhen empty — had no test.What
test/__setup__/keyring-mock.ts— shared@napi-rs/keyringfake. ExposeskeyringStore,keyringFailures,keyringSetKeys.useKeyringBackend()inuseAuthSetup.ts— describe-scoped, so one file covers both backends. Throws if the file forgot the mock, rather than writing to the developer's real OS keyring.test/local/commands/auth.test.ts— 11 tests. Stubsapify-client, sologin/logout/auth tokenrun intest:localon both backends.credentials.test.ts— 23 → 42 tests:writeFileSyncspy (F1/F2 could not fail before),0o600, keyring write failures forsetTokenandsetProxyPassword, the stale-credentials throw,resolveTokenviagetApifyClientOptions, the persistedsecretsBackendmarker branch.auth tokencompares stdout to the token, not tolength > 0(F5).All 8 exit criteria in #1387 are met, plus F3 and the account-switch merge that Stage-1 rewrites.
Notes
resolveTokenhas anAPIFY_TOKENbranch. It does not —src/lib/utils.ts:132isexistingToken ?? (ensureMigrated(), getToken()). That variable is read inactor.ts,mcp/install.tsandactor/charge.tsinstead.auth.test.ts:116asserts that fields from the old account survive a re-login. That is current behavior, pinned deliberately, and flagged in a comment.Verification
test:local585 passed / 4 skipped. Lint, format, build, test typecheck clean.test:apinot run locally — no token here; theuseAuthSetup.tschange is one added export.🤖 Generated with Claude Code