aws-amplify / amplify-swift

A declarative library for application development using cloud services.
Apache License 2.0
453 stars 196 forks source link

receiveValue not called when using resetPassword() or confirmResetPassword() #3227

Closed Dave-London closed 1 year ago

Dave-London commented 1 year ago

Describe the bug

I'm trying to implement a "forgot password" flow in my iOS app, and facing an issue when using Amplify.Auth.resetPassword & Amplify.Auth.confirmResetPassword with Combine - I call the functions as instructed in the example provided in the Amplify dev docs, and while the actual action seems to work (for resetPassword - I get an email with a verification code as expected, for confirmResetPassword - the provided new password becomes the one in use - validated using web app and appsync console), I never get an event with a result or error in the sink attached to the Amplify.Publisher.

Steps To Reproduce

Steps to reproduce the behavior:
1. call Amplify.Auth.resetPassword using Amplify.Publisher
2. wait for the request to complete
3. no error or result is triggered in the sink attached to the publisher
4. same goes for Amplify.Auth.confirmResetPassword 

code used for reset:

Amplify.Publisher.create {
    print("going to call Amplify.Auth.resetPassword.")
    try await Amplify.Auth.resetPassword(for: email)
    print("Amplify.Auth.resetPassword called.")
}.sink {
    if case let .failure(authError) = $0 {
        print("Reset password failed with error \(authError)")
    }
} receiveValue: { resetResult in
    print("got reset result for Amplify.Auth.resetPassword: \(resetResult)")
}

code used for confirm:

Amplify.Publisher.create {
    print("going to call Amplify.Auth.confirmResetPassword.")
    try await Amplify.Auth.confirmResetPassword(for: email, with: newPassword,
                                                confirmationCode: verificationCode)
    print("Amplify.Auth.confirmResetPassword called.")
}.sink {
    if case let .failure(authError) = $0 {
        print("Reset password failed with error \(authError)")
    }
} receiveValue: {
    print("Password reset confirmed")
}

Expected behavior

once the request is completed - the sink should be triggered with either an error or a result, like it is explained in your documentation - https://docs.amplify.aws/lib/auth/password_management/q/platform/ios/

Amplify Framework Version

2.17.1 & 2.17.2

Amplify Categories

Auth

Dependency manager

Swift PM

Swift version

5.8, 5.9

CLI version

11.0.0 & 12.4.0

Xcode version

Xcode 15.0 Build version 15A240d, also happened before update to 15.0

Relevant log output

<details>
<summary>Log Messages</summary>

going to call Amplify.Auth.resetPassword.
2023-09-19T12:10:06+0300 info CognitoIdentityProviderClient : [Logging] Request: POST https:443 
 Path: / 
 Content-Length: 5884, 
Content-Type: application/x-amz-json-1.1, 
X-Amz-Target: AWSCognitoIdentityProviderService.ForgotPassword, 
x-amz-user-agent: aws-sdk-swift/1.0, 
User-Agent: aws-sdk-swift/1.0 api/cognito-identity-provider/1.0 os/iOS/17.0.0 lang/swift/5.9 lib/amplify-swift/2.17.2, 
Host: cognito-idp.us-east-1.amazonaws.com 
 Optional([])
2023-09-19T12:10:06+0300 info SerialExecutor : [Logging] Creating connection pool for Optional("https://cognito-idp.us-east-1.amazonaws.com/?")with max connections: 50
2023-09-19T12:10:06+0300 info CRTClientEngine : [Logging] Connection was acquired to: Optional("https://cognito-idp.us-east-1.amazonaws.com/?")
Amplify.Auth.resetPassword called.
-----------------------------
going to call Amplify.Auth.confirmResetPassword.
2023-09-19T12:11:23+0300 info CognitoIdentityProviderClient : [Logging] Request: POST https:443 
 Path: / 
 x-amz-user-agent: aws-sdk-swift/1.0, 
User-Agent: aws-sdk-swift/1.0 api/cognito-identity-provider/1.0 os/iOS/17.0.0 lang/swift/5.9 lib/amplify-swift/2.17.2, 
Host: cognito-idp.us-east-1.amazonaws.com, 
X-Amz-Target: AWSCognitoIdentityProviderService.ConfirmForgotPassword, 
Content-Type: application/x-amz-json-1.1, 
Content-Length: 5934 
 Optional([])
2023-09-19T12:11:23+0300 info SerialExecutor : [Logging] Creating connection pool for Optional("https://cognito-idp.us-east-1.amazonaws.com/?")with max connections: 50
2023-09-19T12:11:24+0300 info CRTClientEngine : [Logging] Connection was acquired to: Optional("https://cognito-idp.us-east-1.amazonaws.com/?")
Amplify.Auth.confirmResetPassword called.```
</details>

Is this a regression?

No

Regression additional context

No response

Platforms

iOS

OS Version

16.4, 17.0

Device

physical iPhone 12 as well as iPhone 14 & 15 simulators

Specific to simulators

no

Additional context

atierian commented 1 year ago

Thanks for opening this @Dave-London.

Does it work as expected if you remove the print statements in the create closure? Alternatively, just the second one and add a return statement before Amplify.Auth.resetPassword(for:) / Amplify.Auth.confirmResetPassword(for:with:confirmationCode:)?

For example:

Amplify.Publisher.create {
    print("going to call Amplify.Auth.resetPassword.")
    return try await Amplify.Auth.resetPassword(for: email)
}.sink {
    if case let .failure(authError) = $0 {
        print("Reset password failed with error \(authError)")
    }
} receiveValue: { resetResult in
    print("got reset result for Amplify.Auth.resetPassword: \(resetResult)")
}

The Amplify.Publisher.create is generic over the return type of the closure; it returns AnyPublisher<Success, Error> with Success being dependent what you return in the closure passed into it.

In your example, the closure returns Void due to the print statements. Changing to what I suggested above will also resolve the warning you're likely seeing ⚠️ Result of call to 'resetPassword(for:options:)' is unused.

The examples in our documentation use implicit returns - we should probably add an explicit return statement to those examples to make the intent clearer.

Dave-London commented 1 year ago

Hi @atierian, tried using the code you provided and got the same behaviour - the reset password request was processed and i got an email with a verification code, but the sink wasn't triggered with any event - error or result.

atierian commented 1 year ago

Ok, thanks for the update. We'll investigate and come back with any updates or follow up questions.

atierian commented 1 year ago

@Dave-London are you holding on to the cancellable anywhere - either by assigning it to stored property or .store(in &cancellables)?

Looking over your example again, it's not clear that's happening.

Dave-London commented 1 year ago

that was the issue. it was missing and after i added .store() the functions work as expected.

Thanks @atierian

atierian commented 1 year ago

Great, happy to hear!

There's an opportunity to improve our documentation here to make this clearer. Thanks again for opening the issue.