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

Crash on AWSCognitoAuthPlugin+UserBehavior.swift line 95 [protocol witness for AuthCategoryUserBehavior.getCurrentUser() in conformance AWSCognitoAuthPlugin] #1138

Closed
hellopandaxx opened this issue Apr 2, 2021 · 20 comments
Assignees
Labels
auth Issues related to the Auth category bug Something isn't working closing soon This issue will be closed in 7 days unless further comments are made. duplicate This issue or pull request already exists

Comments

@hellopandaxx
Copy link

Describe the bug
I call Amplify.Auth.fetchUserAttributes() during the app start. That leads me to some strange crash at AWSCognitoAuthPlugin+UserBehavior.swift:95 ()

To Reproduce
Can't provide any clear steps to reproduce. But it occurs sometimes and app crashes on start (our synthetic splash screen).

Environment(please complete the following information):

  • Amplify Framework Version: 1.6.1
  • Dependency Manager: Cocoapods
  • Swift Version: 5.0
  • CLI Version: --
  • Include any relevant log output under ~/.amplify/logs/amplify-cli-<issue-date>.log

0 AmplifyPlugins 0x105159d14 protocol witness for AuthCategoryUserBehavior.getCurrentUser() in conformance AWSCognitoAuthPlugin + 95 (AWSCognitoAuthPlugin+UserBehavior.swift:95)
1 Amplify 0x10500f9e0 AuthCategory.getCurrentUser() + 13 (AuthCategory+UserBehavior.swift:13)

Additional log from test flight:
Thread 7 name:
Thread 7 Crashed:
0 libdispatch.dylib 0x00000001807217d8 dispatch_group_leave.cold.1 + 36 (semaphore.c:303)
1 libdispatch.dylib 0x00000001806eec00 dispatch_group_leave + 140 (semaphore.c:0)
2 AWSMobileClient 0x0000000105230330 AWSMobileClient.releaseSignInWait() + 1360 (AWSMobileClientExtensions.swift:0)
3 AmplifyPlugins 0x00000001057f274c closure #1 in AuthorizationProviderAdapter.setupListener() + 204 (AWSMobileClientAdapter.swift:254)
4 AWSMobileClient 0x0000000105223714 partial apply for thunk for @escaping @callee_guaranteed (@unowned UserState, @guaranteed [String : String]) -> () + 28 (:0)
5 AWSMobileClient 0x0000000105235700 mobileClientStatusChanged + 12 (:0)
6 AWSMobileClient 0x0000000105235700 specialized AWSMobileClient.getPasswordDetails(:passwordAuthenticationCompletionSource:) + 1224 (AWSMobileClientExtensions.swift:1007)
7 AWSMobileClient 0x000000010523c640 getPasswordDetails + 8 (:0)
8 AWSMobileClient 0x000000010523c640 getPasswordDetails + 12 (AWSUserPoolOperationsHandler.swift:133)
9 AWSMobileClient 0x000000010523c640 specialized UserPoolOperationsHandler.getDetails(
:passwordAuthenticationCompletionSource:) + 132
10 AWSMobileClient 0x000000010523c12c @objc UserPoolOperationsHandler.getDetails(_:passwordAuthenticationCompletionSource:) + 76
11 AWSCognitoIdentityProvider 0x00000001053a1f48 0x105364000 + 253768
12 AWSCognitoIdentityProvider 0x00000001053a1dbc 0x105364000 + 253372
13 AWSCognitoIdentityProvider 0x000000010539dad0 0x105364000 + 236240
14 AWSCore 0x0000000105552e70 __56-[AWSTask continueWithExecutor:block:cancellationToken:]_block_invoke + 88 (AWSTask.m:331)
15 AWSCore 0x0000000105521f9c __30+[AWSExecutor defaultExecutor]_block_invoke_2 + 128 (AWSExecutor.m:65)
16 AWSCore 0x00000001055224b0 -[AWSExecutor execute:] + 76 (AWSExecutor.m:131)
17 AWSCore 0x0000000105552a70 -[AWSTask runContinuations] + 352 (AWSTask.m:307)
18 AWSCore 0x000000010555271c -[AWSTask trySetError:] + 164 (AWSTask.m:266)
19 AWSCore 0x0000000105553ab0 -[AWSTaskCompletionSource setError:] + 80 (AWSTaskCompletionSource.m:52)
20 AWSCore 0x00000001055629d0 0x1054f8000 + 436688
21 AWSCore 0x00000001055533fc __63-[AWSTask continueWithExecutor:successBlock:cancellationToken:]_block_invoke + 92 (AWSTask.m:398)
22 AWSCore 0x0000000105552e70 __56-[AWSTask continueWithExecutor:block:cancellationToken:]_block_invoke + 88 (AWSTask.m:331)
23 AWSCore 0x0000000105521f9c __30+[AWSExecutor defaultExecutor]_block_invoke_2 + 128 (AWSExecutor.m:65)
24 AWSCore 0x00000001055224b0 -[AWSExecutor execute:] + 76 (AWSExecutor.m:131)
25 AWSCore 0x0000000105552cd8 -[AWSTask continueWithExecutor:block:cancellationToken:] + 304 (AWSTask.m:368)
26 AWSCore 0x000000010555335c -[AWSTask continueWithExecutor:successBlock:cancellationToken:] + 204 (AWSTask.m:394)
27 AWSCore 0x0000000105553484 -[AWSTask continueWithSuccessBlock:] + 100 (AWSTask.m:404)
28 AWSCore 0x0000000105561848 0x1054f8000 + 432200
29 CFNetwork 0x00000001812d27cc __51-[NSURLSession delegate_task:didCompleteWithError:]_block_invoke.226 + 676 (Session.mm:794)
30 libdispatch.dylib 0x00000001806ec24c _dispatch_call_block_and_release + 32 (init.c:1454)
31 libdispatch.dylib 0x00000001806eddb0 _dispatch_client_callout + 20 (object.m:559)
32 libdispatch.dylib 0x00000001806f510c _dispatch_lane_serial_drain + 580 (inline_internal.h:2548)
33 libdispatch.dylib 0x00000001806f5c90 _dispatch_lane_invoke + 460 (queue.c:3862)
34 libdispatch.dylib 0x00000001806ffd78 _dispatch_workloop_worker_thread + 708 (queue.c:6601)
35 libsystem_pthread.dylib 0x00000001cc5a9814 _pthread_wqthread + 276 (pthread.c:2211)
36 libsystem_pthread.dylib 0x00000001cc5b076c start_wqthread + 8

Thank you!

@ruiguoamz
Copy link
Contributor

Hi, @hellopandaxx
Thanks for reporting this.

I think what you saw is similar or the same as this issue.

The crash is because the session of your signed in user has expired, and then when fetchUserAttribute, it sees the session expires and call releaseSignInWait() which is going to release something that isn't hold onto. You can verify if this is the reason that you are seeing the crash.

I am currently working on it. The workaround now is every time the App starts, should call fetchAuthSession to ensure the session is still valid, if sessionExpired, need to perform sign in again

@ruiguoamz ruiguoamz added auth Issues related to the Auth category bug Something isn't working duplicate This issue or pull request already exists pending-community-response Issue is pending response from the issue requestor labels Apr 2, 2021
@hellopandaxx
Copy link
Author

Ok, thank you, @ruiguoamz - I'll try!

@hellopandaxx
Copy link
Author

@ruiguoamz, can you advise me please, am I able to just clear current sign in info in silent mode (sign out) in case when my session is already expired? Lets say I'll call fetchAuthSession at app start and session will be expired, and I want to show some state like if I was signed out, with out any sign in dialog at this point.

@ruiguoamz
Copy link
Contributor

If I understand correctly, you were saying the App first do fetchAuthSession when it starts, and sign out if it sees sessionExpired without letting user know? If yes, I think this is a workable flow. You can try it out.

@hellopandaxx
Copy link
Author

Ok, thank you, @ruiguoamz. And is there any way to perform some kind of refresh of the session using refresh token and etc?

@ruiguoamz
Copy link
Contributor

the fetchAuthSession is to refresh the session if the refresh token is valid. You can check in your AWS Cognito Console to see what the expiration time for your refresh token.

Below is what you can check in your Cognito console:
107404466-29581880-6abb-11eb-9494-c3622e1bdd61

But if the refresh token itself has expired, I am afraid there is no api to refresh and user need to re-authenticate to get a new refresh token

@ruiguoamz
Copy link
Contributor

@hellopandaxx
I am going to close this issue since it's a duplicate of this one
But feel free to reopen it or open a new one if you think it's not.

@hellopandaxx
Copy link
Author

@ruiguoamz ok, thank you 🙏

@hellopandaxx
Copy link
Author

@ruiguoamz can you advise me please? I faced with some kind of confuse.. How can I actually check if the session is valid in correct way during the app start?

@ruiguoamz
Copy link
Contributor

Say you have AuthenticatedView(Signed In) and UnauthenticatedView(Signed Out) and want to switch between these two views based on whether the user is signed in or not.

Everytime the App launches, after Amplify.configure() is called, you can call Auth.fetchAuthSession() which returns AuthSession indicating whether the user is signed in or not. Based on the session returned, the code sets some state variable, for example, var isSignedIn: Bool, so that the App shows the correct View (Authenticated or not). Hope this helps you. Fell free to reach out if you think my suggestion is still unclear

@hellopandaxx
Copy link
Author

@ruiguoamz, thank you.
Ok, what exactly check should I perform on the returned AuthSession in order to ensure it will be valid for further fetchUserAttributes call?

At this point I have the implementation as following:

  1. Call fetchAuthSession()
  2. check isSignedIn value
  3. if isSignedIn is true - call fetchUserAttributes

And something in this chain triggers native alert "App wants to use "amazoncognito.com" to sign in" from #1120. This is definitely what I don't expect to see during app start. So, in case I choose "No" -> app crashes.. Probably with the same reason it crashed in initial post of this thread.. 🤔

I'm trying to figure out:

  1. Is it correct to use session.isSignedIn property to validate current session?
  2. Can session.isSignedIn return true even in case the session actually isn't valid for fetchUserAttributes call?
  3. Is fetchAuthSession triggers "App wants to use "amazoncognito.com.." dialog each time when it wants to refresh expired session? And if yes, can we suppress this dialog or grand such permission once per app install during first sign in and do not show this dialog each time?

@ruiguoamz
Copy link
Contributor

Ah! I am really sorry about the mistake I made here.

You also set up an auth event listener when the App launches so that you can get a specific sessionExpired event indicating although the user is signed in but the token has expired. Merely depending on fetchAuthSession() is not enough.

Sorry again for the wrong suggestion I previously provided. Please try out the Auth Event and see if it works

@hellopandaxx
Copy link
Author

hellopandaxx commented Apr 9, 2021

@ruiguoamz Thank you! I'll try it 👌
So, am I understand correctly that after Amplify.configure() I make this subscription and use it:

  1. if Auth.signedIn -> can call fetchUserAttributes
  2. if Auth.sessionExpired -> call fetchAuthSession
  3. if Auth.signedOut -> do nothing
    Is it correct way?

In this 2nd case how should I check fetchAuthSession's result in order to be ok to call fetchUserAttributes?

@palpatim
Copy link
Member

palpatim commented Apr 9, 2021

Reopening to finish answering @hellopandaxx's questions

@palpatim palpatim reopened this Apr 9, 2021
@palpatim palpatim added follow up Requires follow up from maintainers and removed pending-community-response Issue is pending response from the issue requestor labels Apr 9, 2021
@ruiguoamz
Copy link
Contributor

A user is signed in with a valid token or not should be determined by Auth Events api and fetchAuthSession api

  1. if AuthSession.signedIn && !AuthEvent.sessionExpired -> user is signed in with valid token -> can call fetchUserAttributes
  2. if AuthSession.signedIn && AuthEvent.sessionExpired -> user is signed in but with invalid token -> call signIn to re-authenticate
  3. if AuthSession.signedOut -> user is signed out -> not sure what nothing means (probably present UnauthenticatedView and let user to perform sign in )

Note:
AuthSession is the result of fetchAuthSession, AuthEvent is the event comes from Amplify.Hub.listen(to: .auth)

@hellopandaxx
Copy link
Author

@ruiguoamzю thank you for your answers 🙏

@palpatim
Copy link
Member

palpatim commented Apr 9, 2021

Could you please provide any additional info about "App wants to use "amazoncognito.com.." dialog from this #1138 (comment) ? Should I expect this dialog during app start for the session renewing etc? Are there any additional ways to manage this dialog (suppress, remember choice, know when to expect it)?

As discussed in #1120 you can suppress this alert by setting the preferPrivateSession option when you invoke signInWithWebUI. If you have other questions about that behavior, please feel free to continue the conversation on that issue.

@ruiguoamz
Copy link
Contributor

ruiguoamz commented Apr 9, 2021

So, will this fix affect this flow in some way?

The PR is to avoid the crash.

And I just found out another workaround:

func fetchAuthSession() {
    Amplify.Auth.fetchAuthSession { result in
        switch result {
        case .success(let session):
            if let session = session as? AWSAuthCognitoSession {
                switch session.getCognitoTokens() {
                case .success(let token):
                    // Do something
                case .failure(let error):
                    // User needs to re-authenticate
                }
        case .failure:
            // Do something
        }
    }
}

The common question about Auth Events api in the terms of this conversation is whether it is intended to be some kind of global app subscription or it mostly usable during app start, specifically to resolve such issue as our current conversation?

Well, it depends on your use case which I am not sure about. You can use Auth Events Listener as global app subscription like you said

@ruiguoamz ruiguoamz removed the follow up Requires follow up from maintainers label Apr 9, 2021
@hellopandaxx
Copy link
Author

Thank you guys, will try it out 🙏

@palpatim palpatim added the pending-community-response Issue is pending response from the issue requestor label Apr 12, 2021
@ruiguoamz ruiguoamz self-assigned this Apr 14, 2021
@ruiguoamz ruiguoamz removed the pending-community-response Issue is pending response from the issue requestor label Apr 14, 2021
@royjit royjit added the closing soon This issue will be closed in 7 days unless further comments are made. label Apr 16, 2021
@github-actions
Copy link
Contributor

This issue is being automatically closed due to inactivity. If you believe it was closed by mistake, provide an update and re-open it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auth Issues related to the Auth category bug Something isn't working closing soon This issue will be closed in 7 days unless further comments are made. duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

4 participants