Aws-sdk-ios: AWSCognitoAuth doesn't called completion handler when "Done" button pressed.

[johnflanagan-spok]

Describe the bug
When attempting to sign in with AWSCognitoAuth if the user taps the Done button on the SFSafariViewController the getSessionBlock is not called.

To Reproduce
Steps to reproduce the behavior:

1. Start user authentication by calling AWSCognitoAuth.getSession:completion:
2. Click Done on the SFSafariViewController presented by the SDK
3. Observe that the completion handler is not called

Which AWS service(s) are affected?
AWSCognitoAuth

Expected behavior
The completion handler should be called with a AWSCognitoAuthClientErrorUserCanceledOperation error.

Environment(please complete the following information):

• SDK Version: 2.7.3
• Dependency Manager: Cocoapods
• Swift Version : 4.0

Device Information (please complete the following information):

• Device: Tested on iPhone X and Simulator
• iOS Version: iOS 12.1
• Specific to simulators: No

Additional context
I suspect the issue is that https://github.com/aws-amplify/aws-sdk-ios/blob/6ea656a78050a9092da16f380cbbdd920584a357/AWSCognitoAuth/AWSCognitoAuth.m#L688 is never calling its completion handler because the done button has already started dismissing the SFSafariViewController.

[rohandubal]

Hello @johnflanagan-spok

We recently fixed this issue by moving the de-allocation of svc before the completion handler. Are you consistently experiencing this error on latest version of the SDK? Asking since we had another customer who verified that the fix made sure they got the correct callback.

Also, for iOS 12.1 (iOS 11+) it is recommended to use SFAuthenticationSession for which we recently added support. You can use it via this constructor:
https://github.com/aws-amplify/aws-sdk-ios/blob/6ea656a78050a9092da16f380cbbdd920584a357/AWSCognitoAuth/AWSCognitoAuth.h#L276

Thanks,
Rohan

[johnflanagan-spok]

Hi @rohandubal

I saw that conversation on the other issue, but I'm still experiencing the issue. It's never entering the dismiss completion handler, so it doesn't run either dismissBlock() or the svc de-allocation.

I saw the SFAuthenticationSession option for iOS 11+, but our app is targeted to support iOS 10+. 12.1 is just what my development device that I did the testing on was running.

Thanks,
-John

[rohandubal]

Interesting, I will look into moving the completion handler callback outside the dismiss block to ensure we call it.

Regarding SFAuthenticationSession, you can switch to it as it will fall back to SFSafariViewController if iOS version is 9 or 10. It is used only for iOS 11 and 12.

Thanks,
Rohan

[rohandubal]

Hello @johnflanagan-spok

Could you please specify some steps to reproduce this? I am using the latest version of the SDK and unable to reproduce this issue. A sample app or steps to reproduce would be very helpful. Also if you could provide information like how frequently this happens and if it only happens on specific devices, that would be helpful too.

We have some other customers who confirmed that this issue does not happen to them on the latest SDK version. Calling the block before dismissing might not be ideal since it can cause race conditions on transitions. The de-allocation should not happen until the block is called.

Thanks,
Rohan

[johnflanagan-spok]

@rohandubal

This demo app exhibits the problem: https://github.com/johnflanagan-spok/aws-test

[rohandubal]

Thanks for the sample app @johnflanagan-spok

I tried the app and I can confirm there was no callback given. This will be helpful in debugging this issue.

I will update back once I identify the root cause.

Thanks,
Rohan

[rohandubal]

I think this was introduced as a regression of #1000

We will have to update the completion block to be called after the dismiss call outside of the completionBlock.

[rohandubal]

Hello @johnflanagan-spok

I have identified the root cause for this issue and working on a fix. It will roll out in a future version.

Thanks,
Rohan

[johnflanagan-spok]

Thanks @rohandubal !

[minbi]

Hi @johnflanagan-spok ,

We have just released version 2.8.2 of the SDK. Please check if this fixes the issue for you.

[johnflanagan-spok]

@minbi ,

Looks good to me. Thanks!