fix: macOS Keychain token caching bugs from #1#3
Closed
salmonumbrella wants to merge 5 commits intosteipete:mainfrom
Closed
fix: macOS Keychain token caching bugs from #1#3salmonumbrella wants to merge 5 commits intosteipete:mainfrom
salmonumbrella wants to merge 5 commits intosteipete:mainfrom
Conversation
…tence
Without KeychainTrustApplication: true, tokens are saved with an empty
TrustedApplications list ([]string{}), which prevents ANY application
from reading them back - including eightctl itself.
This caused the symptom where "keyring saved token" would succeed but
subsequent "keyring get" calls would fail with "item not found".
🤖 Generated with [Claude Code](https://claude.ai/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Authenticate() previously always hit the server, bypassing the token cache entirely. This caused unnecessary API calls and rate limiting, especially for commands like `whoami` that explicitly call Authenticate(). Now Authenticate() checks: 1. In-memory token (if still valid) 2. Cached token from keychain 3. Only then authenticates with the server This mirrors the behavior of ensureToken() but is needed because some commands call Authenticate() directly. 🤖 Generated with [Claude Code](https://claude.ai/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The macOS Keychain backend has issues with adhoc-signed Go binaries: Set() succeeds but Get() cannot retrieve items due to code signature and ACL problems. This causes tokens to appear "saved" but never load. Switch to FileBackend exclusively, which stores encrypted tokens in ~/.config/eightctl/keyring/ using JOSE encryption. Also add singleton pattern for the keyring connection to reduce file system operations and avoid potential concurrent access issues. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Changes: - requireAuthFields: Check credentials first before cache lookup, avoiding unnecessary Client creation when email/password are set - requireAuthFields: Construct Identity directly with same defaults as Client.New() to ensure cache key consistency - do(): Add debug logging when API returns 401 to help diagnose authentication issues The 401 logging revealed that the temperature API rejects tokens from legacy login, which is a separate issue to investigate. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The OAuth token endpoint was failing with 400 error because: 1. client_id was hardcoded as "sleep-client" instead of using c.ClientID 2. client_secret was empty instead of using c.ClientSecret Also removed explicit Accept-Encoding: gzip headers - Go's http.Transport handles gzip decompression automatically when headers are not manually set. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
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
1 participant
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.
Summary
Fixes the token caching bugs mentioned in #1:
Multiple issues were identified and fixed:
1.
KeychainTrustApplicationnot enabledWithout
KeychainTrustApplication: true, the keyring library saves tokens with an emptyTrustedApplicationslist, which means no application can read them back - including eightctl itself.2.
Authenticate()bypasses token cacheCommands like
whoamicallAuthenticate()directly, which bypassed the cache entirely and always hit the server. This caused unnecessary API calls and 429 rate limiting.3. Keyring backend issues on macOS
The
keyringlibrary's auto-detection was unreliable. Switched to explicitly useFileBackendwith a singleton pattern to ensure consistent token storage in~/.config/eightctl/keyring/.4. OAuth token endpoint broken (NEW)
The
authTokenEndpointwas hardcoded to use wrong credentials:client_id: "sleep-client"instead of the actual client IDclient_secret: ""instead of the actual secretThis caused 400 errors and fallback to legacy login, which then failed on temperature API calls with 401.
5. Gzip response handling (NEW)
Explicit
Accept-Encoding: gzipheaders were causing issues because Go'shttp.Transporthandles this automatically. Removed the headers to let Go handle compression transparently.Changes
KeychainTrustApplication: truein keyring configAuthenticate()check in-memory and cached tokens before hitting the serverrequireAuthFieldsto avoid creating temporary Client objectsc.ClientIDandc.ClientSecretAccept-Encoding: gzipheadersNotes
timezone: "local"in their config should change it to an IANA timezone (e.g.,America/Los_Angeles) astime.Local.String()returns"Local"which the API rejectsTest plan
eightctl whoamiuses cached token on subsequent runseightctl statusworks (previously failed with 401)eightctl sleep dayworks with proper timezone configeightctl device inforeturns full device details🤖 Generated with Claude Code