Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sign in callback invoked before all sign in operations are complete #1700

Closed
royjit opened this issue Jul 29, 2019 · 8 comments
Closed

Sign in callback invoked before all sign in operations are complete #1700

royjit opened this issue Jul 29, 2019 · 8 comments
Assignees
Labels
bug Something isn't working mobile client Issues related to AWSMobileClient pending-response Issue is pending response from the issue requestor

Comments

@royjit
Copy link
Member

royjit commented Jul 29, 2019

Describe the bug
Callback for sign is called before the actual status is set internally.

To Reproduce
We call self.performUserPoolSuccessfulSignInTasks and then calls completionHandler without waiting for the first task to complete.
https://github.com/aws-amplify/aws-sdk-ios/blob/master/AWSAuthSDK/Sources/AWSMobileClient/AWSMobileClient.swift#L480
i) Inside self.performUserPoolSuccessfulSignInTasks we call ‘mobileClientStatusChanged’
ii) After that we remove keychain values and update with new ones
iii) After that we call the two lines below
self.internalCredentialsProvider?.clearCredentials()
self.internalCredentialsProvider?.credentials()

This means that if we rely on AWSMobileClient.sharedInstance().addUserStateListener, and perform task based on the status. It will be wrong.

@royjit royjit added mobile client Issues related to AWSMobileClient bug Something isn't working labels Jul 29, 2019
@palpatim palpatim changed the title Race condition in AWSMobileClient Sign in callback invoked before all sign in operations are complete Aug 1, 2019
@royjit
Copy link
Member Author

royjit commented Aug 7, 2019

PR for the fix - #1721

@royjit royjit self-assigned this Aug 8, 2019
@ghost
Copy link

ghost commented Aug 12, 2019

So would this be why using getIdentityId() or invoking a Lambda based on addUserStateListener's .signedIn state fails for a certain period of time but then suddenly starts succeeding?

Any ETA on when this will be fixed?

@palpatim
Copy link
Member

No ETA yet but we are actively working on it.

@royjit
Copy link
Member Author

royjit commented Aug 13, 2019

PR #1721 approved and pushed to develop branch.

@royjit
Copy link
Member Author

royjit commented Aug 13, 2019

Had to revert the PR since we found another callback which was invoked before signIn completes. This is caused by currentSignInHandlerCallback which is called in getTokens method during signIn.

@royjit royjit removed the work in progress Issues was triaged and investigation done label Aug 21, 2019
@royjit royjit added the work in progress Issues was triaged and investigation done label Aug 27, 2019
@royjit
Copy link
Member Author

royjit commented Sep 6, 2019

This should be solved in the PR #1822.

@palpatim
Copy link
Member

We released this in https://github.com/aws-amplify/aws-sdk-ios/releases/2.11.0

Please let us know if you have any problems with this.

@palpatim palpatim added pending-response Issue is pending response from the issue requestor and removed work in progress Issues was triaged and investigation done labels Sep 10, 2019
@stale
Copy link

stale bot commented Sep 29, 2019

This issue has been automatically closed because of inactivity. Please open a new issue if are still encountering problems.

@stale stale bot closed this as completed Sep 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working mobile client Issues related to AWSMobileClient pending-response Issue is pending response from the issue requestor
Projects
None yet
Development

No branches or pull requests

2 participants