Keep refreshed token in memory when keychain write fails - #259
Open
onevcat wants to merge 5 commits into
Open
Conversation
When storing a new access token to the keychain fails (e.g. errSecNotAvailable while securityd is unavailable), the token is now kept in memory instead of being discarded, so the current session continues and the consumed refresh token is not sent again. The SDK retries the keychain writing when the app becomes active or protected data becomes available, and logs diagnostics (raw status, app state, protected data availability) on failure. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The synchronous overrides are treated as nonisolated by the XCTest SDK on CI's Xcode version, so calling main actor-isolated helpers from them fails to compile. Follow the async throws pattern used by RefreshTokenPipelineTests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
In Swift 4.2 mode try? does not flatten the resulting optional (pre SE-0230), so the expression had type AccessToken??. Use do/catch instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Token refresh and login no longer report a failure when the keychain write fails, so apps lost the ability to observe such failures. Post a public notification (with the underlying error in userInfo) on the initial write failure and on each failed retry, and expose it to Objective-C as well. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The retry runs on a background queue in production, which made the lifecycle-notification tests depend on dispatch timing and flaky on CI. Inject the retry executor so tests run it synchronously. Co-Authored-By: Claude Fable 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.
Background
Users reported intermittent
-25291(errSecNotAvailable) keychain errors when the SDK stores a refreshed access token. When this happens, the refreshed token is discarded and the old (already consumed) refresh token is sent on the next refresh, which can force a re-login, especially for channels with refresh token rotation enabled.Investigation showed
-25291means the keychain daemon (securityd) is unreachable or hits an internal error. It is environmental and transient: once the daemon recovers, the same write succeeds with the stored data intact.Changes
AccessTokenStore.setCurrentTokenno longer throws. When the keychain write fails, the token is kept in memory with a pending flag, so the current session continues with the new token and the consumed refresh token is not re-sent. The.LineSDKAccessTokenDidUpdatenotification is still posted..LineSDKAccessTokenDidFailToPersistnotification, posted when the initial write or a retry fails. The underlying error is inuserInfounderLineSDKNotificationKey.persistingError. Also exposed to Objective-C.removeCurrentAccessTokennow clears the in-memory token and the pending state even when the keychain does not contain the item, so a logout cannot be resurrected by a later retry.KeychainStoreto simulate keychain write failures, andAccessTokenStoreTestscovering the failure, retry, lifecycle-triggered retry, notifications, logout, and 401 auto-refresh scenarios.Behavior change
Previously, a keychain write failure surfaced as a failure result from
API.Auth.refreshAccessToken(and the login flow), even though the server-side operation had succeeded. Now these operations report success and the SDK keeps the token in memory. Apps that need visibility into storing failures (for example, for telemetry) should observe.LineSDKAccessTokenDidFailToPersist. This is documented in the CHANGELOG.Notes
If the app process terminates before a pending write succeeds, the old token is loaded on the next launch and a re-login may still be required for channels with refresh token rotation. This residual risk cannot be eliminated on the client side.
🤖 Generated with Claude Code