fix(auth): defer keychain-sharing reconfigure during in-flight sign-in - #4231
Merged
Conversation
When AWSCognitoAuthPlugin is configured with a shared keychain access
group, every fetchAuthSession unconditionally sent a .reconfigure event
to the auth state machine. If a fetchAuthSession landed between a
sign-in step that returned .confirmSignInWithCustomChallenge and the
caller's confirmSignIn, the reconfigure tore down the .signingIn
substate and the next confirmSignIn threw
AuthError.invalidState("User is not attempting signIn operation").
Replace the unconditional reconfigure with a reconcile that:
- Reads the shared keychain credentials, compares against the local
state machine's authZ credentials, and skips reconfigure when they
match — eliminating wasted reconfigures on every fetch.
- Defers reconfigure during locally-originated in-flight flows
(.signingIn, .signingOut, .deletingUser, .federatingToIdentityPool,
.clearingFederation) when the keychain has no signed-in credentials.
- Adopts a sibling app's sign-in by reconfiguring even during
.signingIn when the remote keychain has user-pool tokens. Relax the
AWSAuthConfirmSignInTask top-level guard to accept .signedIn and
return .done so the pending confirmSignIn resolves cleanly.
Fixes #4224.
Fold the .signedIn early-return into the same switch that handles .signingIn dispatch. The listener loop already returns .done on .signedIn(.sessionEstablished), so the early return was redundant — the only thing the .signedIn branch needs to do is skip the event dispatch that's specific to the .signingIn path.
…CreateEvent and clarify keychain/state-machine fetch helper names Move the .signedIn early-return for shared-keychain adoption into analyzeCurrentStateAndCreateEvent so execute() has a single guard + single dispatch. Rename fetchRemoteCredentials and the local-state snapshot helper to fetchCredentialsFromKeychain and fetchCredentialsFromStateMachine to make the source-of-truth pairing explicit at the call site.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4231 +/- ##
==========================================
+ Coverage 66.72% 66.78% +0.05%
==========================================
Files 1151 1151
Lines 43817 43892 +75
==========================================
+ Hits 29239 29312 +73
- Misses 14578 14580 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
thisisabhash
approved these changes
Jun 9, 2026
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
2 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.
Issue
Fixes #4224
Description
When
AWSCognitoAuthPluginis configured with a shared keychain access group, every call toAmplify.Auth.fetchAuthSessionunconditionally sent a.reconfigureevent to the auth state machine. If the call landed between a sign-in step that returned.confirmSignInWithCustomChallengeand the caller'sconfirmSignIn, the reconfigure tore down the in-flight.signingInsubstate and the subsequentconfirmSignInthrew:The user could not complete sign-in until the entire flow was restarted. The repro is most visible with Custom Auth flows (which routinely park
.signingInfor tens of seconds while waiting for human approval, push delivery, or third-party verification), but it affects any in-flight sign-in flow.Why was the reconfigure there?
The keychain-sharing feature (#3947) needs sibling-app keychain mutations to be picked up without an app relaunch. The original implementation answered "did anything change?" with a coarse proxy:
accessGroup != nil. That predicate istruefor the lifetime of the process, so reconfigure fires on everyfetchAuthSessionregardless of whether the keychain actually changed — and regardless of whether the local state machine has work in flight.Fix
Replace the unconditional reconfigure with a reconcile that:
.signingIn— would destroy the user's pendingconfirmSignInflow.signingOut,.deletingUser,.federatingToIdentityPool,.clearingFederation— would skip in-flight side effects (token revoke, HostedUI session clearing, credential clearing, Hub events)The local flow runs to completion; the next
fetchAuthSessionreconciles with whatever is in the keychain at that point..signingInbecause the new credentials are strictly more useful than the in-flight challenge. To make the adoption succeed end-to-end,AWSAuthConfirmSignInTasknow accepts.signedInat its top-level guard and returnsAuthSignInResult(nextStep: .done)instead of throwinginvalidState.Decision matrix
AuthenticationState.signedIn/.signedOut/.error/ etc..signedIn/.signedOut/.error/ etc..signingIn.signingInnoCredentials.signingOut/.deletingUser/.federatingToIdentityPool/.clearingFederationFiles changed
AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Task/AWSAuthFetchSessionTask.swift— replaceforceReconfigureboolean withisKeychainSharingEnabled+reconcileWithSharedKeychainIfNeeded()decision logic.AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Task/AWSAuthConfirmSignInTask.swift— accept.signedInat top-level guard, return.doneso adoption resolves cleanly.AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/ClientBehavior/AWSCognitoAuthPlugin+ClientBehavior.swift— renameforceReconfigure→isKeychainSharingEnabledat the call site.AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/CredentialStorage/AmplifyCredentials.swift— addhasUserPoolTokenshelper for the adopt predicate.Tests
New
AWSAuthFetchSessionTaskKeychainSharingTestscovers:.signingIn+ remote signed-out → no reconfigure (defer; the regression scenario from Auth.fetchSession triggers state machine reconfigure that interrupts in-flight Custom Auth sign-in when keychain sharing is enabled #4224).signingIn+ remote signed-in → reconfigure runs (adopt sibling sign-in).signedIn+ matching remote → no reconfigure (eliminates wasted work).signedIn+ remote signed-out → reconfigure runs (sibling sign-out picked up).signingOut+ mismatched remote → no reconfigure (sign-out side effects must run)testConfirmSignInReturnsDoneWhenStateIsAlreadySignedIncovers the relaxedconfirmSignInguard producing.doneafter adoption.All existing
AWSCognitoAuthPluginUnitTestscontinue to pass.General Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.