feat(auth): use os keyring for secure credential storage#1148
Draft
l2ysho wants to merge 10 commits into
Draft
Conversation
Store sensitive API tokens and proxy passwords in OS keyring for improved security, falling back to file.
getApifyClientOptions became async but several test sites still passed the returned Promise directly to `new ApifyClient(...)`, leaving the client with undefined token/baseUrl. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Persist the backend marker in auth.json on first login and honor it on subsequent runs so we don't re-probe the OS keyring on every CLI invocation. Also drop the write/delete side of the probe — getPassword on a non-existent entry is enough to detect an unavailable backend and avoids unnecessary Keychain access. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The OS keyring is machine-global, so the unique __APIFY_INTERNAL_TEST_AUTH_PATH__ per test only isolates auth.json, not the keyring. After one test logged in, the leaked token made later tests see getToken() return a value with no matching username/id and throw "Corrupted local user info". useAuthSetup already pins in-process tests to the file backend; do the same for the dist subprocesses runCli spawns. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previously, clearSecrets() was a no-op when the active backend was 'file', so a user who logged in normally and later set APIFY_DISABLE_KEYRING=1 before running logout would leave their token in the OS keyring with no in-CLI way to remove it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…in message When the user explicitly opts out via APIFY_DISABLE_KEYRING=1, calling the keyring "unavailable" and telling them to set the var they already set is misleading. Split the file-backend branch into two: env-disabled vs probe failure. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… keyring Scoping the keyring change down: stripping the entire proxy object from the userInfo write in getLoggedClient also dropped proxy.groups, which breaks the log_in_out API test that compares auth.json to the API user response. Leave proxy in the file as it was before and exclude the internal secretsBackend marker from the test comparison.
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
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.
Store sensitive API tokens and proxy passwords in OS keyring for improved security, falling back to file.