fix(auth): Delete user api get stuck on no network (#2656)

royjit · web-flow · commit fb3966f20126 · 2023-01-27T09:39:07.000-08:00
diff --git a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Actions/DeleteUser/DeleteUser.swift b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Actions/DeleteUser/DeleteUser.swift
@@ -47,7 +47,7 @@ struct DeleteUser: Action {
                 _ = try await client.deleteUser(input: input)
                 event = DeleteUserEvent(eventType: .signOutDeletedUser)
                 logVerbose("\(#fileID) Delete User succeeded", environment: environment)
-            } catch let error as DeleteUserOutputError {
+            } catch let error as AuthErrorConvertible {
                 event = DeleteUserEvent(eventType: .throwError(error.authError))
                 logVerbose("\(#fileID) Delete User failed \(error)", environment: environment)
             } catch let error {
diff --git a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/ClientBehavior/AWSCognitoAuthPlugin+ClientBehavior.swift b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/ClientBehavior/AWSCognitoAuthPlugin+ClientBehavior.swift
@@ -159,7 +159,9 @@ extension AWSCognitoAuthPlugin: AuthCategoryBehavior {
     }
 
     public func deleteUser() async throws {
-        let task = AWSAuthDeleteUserTask(authStateMachine: self.authStateMachine)
+        let task = AWSAuthDeleteUserTask(
+            authStateMachine: self.authStateMachine,
+            authConfiguraiton: authConfiguration)
         _ = try await taskQueue.sync {
             return try await task.value
         }
diff --git a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Operations/Helpers/FetchAuthSessionOperationHelper.swift b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Operations/Helpers/FetchAuthSessionOperationHelper.swift
@@ -174,13 +174,18 @@ class FetchAuthSessionOperationHelper {
 
         switch error {
 
-        case .notAuthorized:
+        case .notAuthorized, .noCredentialsToRefresh:
             if !isSignedIn {
                 return AuthCognitoSignedOutSessionHelper.makeSessionWithNoGuestAccess()
             }
-        case .noCredentialsToRefresh:
-            if !isSignedIn {
-                return AuthCognitoSignedOutSessionHelper.makeSessionWithNoGuestAccess()
+
+        case .service(let error):
+            if let authError = (error as? AuthErrorConvertible)?.authError {
+                let session = AWSAuthCognitoSession(isSignedIn: isSignedIn,
+                                                    identityIdResult: .failure(authError),
+                                                    awsCredentialsResult: .failure(authError),
+                                                    cognitoTokensResult: .failure(authError))
+                return session
             }
         default: break
 
diff --git a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Service/ErrorMapping/AuthErrorConvertible.swift b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Service/ErrorMapping/AuthErrorConvertible.swift
@@ -14,3 +14,9 @@ protocol AuthErrorConvertible {
 
     var authError: AuthError { get }
 }
+
+extension AuthError: AuthErrorConvertible {
+    var authError: AuthError {
+        return self
+    }
+}
diff --git a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Service/ErrorMapping/SdkError+AuthError.swift b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Service/ErrorMapping/SdkError+AuthError.swift
@@ -70,9 +70,13 @@ extension SdkError: AuthErrorConvertible {
             if let authError = error as? AuthErrorConvertible {
                 return authError.authError
             } else {
-                return AuthError.service(error.localizedDescription,
-                                         "",
-                                         AWSCognitoAuthError.network)
+                return AuthError.service(
+                    error.localizedDescription,
+                    """
+                    Check your network connection, retry when the network is available.
+                    HTTP Response stauts code: \(String(describing: httpResponse?.statusCode))
+                    """,
+                    AWSCognitoAuthError.network)
 
             }
 
diff --git a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/StateMachine/CodeGen/Events/AuthEvents.swift b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/StateMachine/CodeGen/Events/AuthEvents.swift
@@ -22,6 +22,8 @@ struct AuthEvent: StateMachineEvent {
         case authenticationConfigured(AuthConfiguration, AmplifyCredentials)
 
         case authorizationConfigured
+
+        case reconfigure(AuthConfiguration)
     }
 
     var id: String
@@ -38,6 +40,7 @@ struct AuthEvent: StateMachineEvent {
         case .authenticationConfigured: return "AuthEvent.authenticationConfigured"
         case .authorizationConfigured: return "AuthEvent.authorizationConfigured"
         case .validateCredentialAndConfiguration: return "AuthEvent.validateCredentialAndConfiguration"
+        case .reconfigure: return "AuthEvent.reconfigure"
         }
     }
 
diff --git a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/StateMachine/Resolvers/AuthState/AuthState+Resolver.swift b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/StateMachine/Resolvers/AuthState/AuthState+Resolver.swift
@@ -80,6 +80,11 @@ extension AuthState {
                 return .init(newState: newState, actions: authNresolution.actions + authZresolution.actions)
 
             case .configured(let authenticationState, let authorizationState):
+                if case .reconfigure(let authConfiguration) = isAuthEvent(event)?.eventType {
+                    let newState = AuthState.configuringAuth
+                    let action = InitializeAuthConfiguration(authConfiguration: authConfiguration)
+                    return .init(newState: newState, actions: [action])
+                }
                 let authenticationResolver = AuthenticationState.Resolver()
                 let authorizationResolver = AuthorizationState.Resolver()
                 let authNresolution = authenticationResolver.resolve(oldState: authenticationState, byApplying: event)
diff --git a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/StateMachine/Resolvers/RefreshSession/RefreshSessionState+Resolver.swift b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/StateMachine/Resolvers/RefreshSession/RefreshSessionState+Resolver.swift
@@ -18,6 +18,7 @@ extension RefreshSessionState {
                      byApplying event: StateMachineEvent) -> StateResolution<RefreshSessionState> {
 
             switch oldState {
+
             case .notStarted:
 
                 if case .refreshCognitoUserPool(let signedInData) = event.isRefreshSessionEvent {
diff --git a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Task/AWSAuthDeleteUserTask.swift b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Task/AWSAuthDeleteUserTask.swift
@@ -12,20 +12,29 @@ import AWSPluginsCore
 class AWSAuthDeleteUserTask: AuthDeleteUserTask {
     private let authStateMachine: AuthStateMachine
     private let taskHelper: AWSAuthTaskHelper
+    private let configuration: AuthConfiguration
 
     var eventName: HubPayloadEventName {
         HubPayload.EventName.Auth.deleteUserAPI
     }
 
-    init(authStateMachine: AuthStateMachine) {
+    init(authStateMachine: AuthStateMachine,
+         authConfiguraiton: AuthConfiguration) {
         self.authStateMachine = authStateMachine
+        self.configuration = authConfiguraiton
         self.taskHelper = AWSAuthTaskHelper(authStateMachine: authStateMachine)
     }
 
     func execute() async throws {
         await taskHelper.didStateMachineConfigured()
         let accessToken = try await taskHelper.getAccessToken()
-        try await deleteUser(with: accessToken)
+
+        do {
+            try  await deleteUser(with: accessToken)
+        } catch {
+            await waitForReConfigure()
+            throw error
+        }
     }
 
     private func deleteUser(with token: String) async throws {
@@ -52,4 +61,10 @@ class AWSAuthDeleteUserTask: AuthDeleteUserTask {
             }
         }
     }
+
+    private func waitForReConfigure() async {
+        let event = AuthEvent(eventType: .reconfigure(configuration))
+        await authStateMachine.send(event)
+        await taskHelper.didStateMachineConfigured()
+    }
 }
diff --git a/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/TaskTests/AuthenticationProviderDeleteUserTests.swift b/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/TaskTests/AuthenticationProviderDeleteUserTests.swift
@@ -12,6 +12,8 @@ import XCTest
 @testable import Amplify
 @testable import AWSCognitoAuthPlugin
 import AWSCognitoIdentityProvider
+import ClientRuntime
+import AwsCommonRuntimeKit
 
 class AuthenticationProviderDeleteUserTests: BasePluginTest {
 
@@ -34,6 +36,97 @@ class AuthenticationProviderDeleteUserTests: BasePluginTest {
         }
     }
 
+
+    /// Test a deleteUser with network error from service
+    ///
+    /// - Given: Given an auth plugin with mocked service. Mocked service should mock a
+    ///   network error
+    ///
+    /// - When:
+    ///    - I invoke deleteUser
+    /// - Then:
+    ///    - I should get a .service error with .network as underlying error
+    ///
+    func testOfflineDeleteUser() async {
+        mockIdentityProvider = MockIdentityProvider(
+            mockRevokeTokenResponse: { _ in
+                try RevokeTokenOutputResponse(httpResponse: .init(body: .empty, statusCode: .ok))
+            }, mockGlobalSignOutResponse: { _ in
+                try GlobalSignOutOutputResponse(httpResponse: .init(body: .empty, statusCode: .ok))
+            },
+            mockDeleteUserOutputResponse: { _ in
+                let crtError = ClientRuntime.ClientError.retryError(
+                    CRTError.crtError(
+                        AWSError(errorCode: 1059)))
+                throw SdkError<DeleteUserOutputError>.client(crtError)
+            }
+        )
+        do {
+            try await plugin.deleteUser()
+            XCTFail("Should not get success")
+        } catch {
+            guard case AuthError.service(_, _, let underlyingError) = error,
+                  case .network = (underlyingError as? AWSCognitoAuthError) else {
+                XCTFail("Should produce network error instead of \(error)")
+                return
+            }
+        }
+    }
+
+    /// Test a deleteUser with network error from service and retry with success on no network error
+    ///
+    /// - Given: Given an auth plugin with mocked service. Mocked service should mock a
+    ///   network error for the first call and mock an actual result on the second call
+    ///
+    /// - When:
+    ///    - I invoke deleteUser and retry it
+    /// - Then:
+    ///    - I should get a .service error with .network as underlying error for the first call
+    ///    - I should get a valid response for the second call
+    ///
+    func testOfflineDeleteUserAndRetry() async {
+        mockIdentityProvider = MockIdentityProvider(
+            mockRevokeTokenResponse: { _ in
+                try RevokeTokenOutputResponse(httpResponse: .init(body: .empty, statusCode: .ok))
+            }, mockGlobalSignOutResponse: { _ in
+                try GlobalSignOutOutputResponse(httpResponse: .init(body: .empty, statusCode: .ok))
+            },
+            mockDeleteUserOutputResponse: { _ in
+                let crtError = ClientRuntime.ClientError.retryError(
+                    CRTError.crtError(
+                        AWSError(errorCode: 1059)))
+                throw SdkError<DeleteUserOutputError>.client(crtError)
+            }
+        )
+        do {
+            try await plugin.deleteUser()
+            XCTFail("Should not get success")
+        } catch {
+            guard case AuthError.service(_, _, let underlyingError) = error,
+                  case .network = (underlyingError as? AWSCognitoAuthError) else {
+                XCTFail("Should produce network error instead of \(error)")
+                return
+            }
+        }
+
+        mockIdentityProvider = MockIdentityProvider(
+            mockRevokeTokenResponse: { _ in
+                try RevokeTokenOutputResponse(httpResponse: .init(body: .empty, statusCode: .ok))
+            }, mockGlobalSignOutResponse: { _ in
+                try GlobalSignOutOutputResponse(httpResponse: .init(body: .empty, statusCode: .ok))
+            },
+            mockDeleteUserOutputResponse: { _ in
+                try DeleteUserOutputResponse(httpResponse: .init(body: .empty, statusCode: .ok))
+            }
+        )
+        do {
+            try await plugin.deleteUser()
+            print("Delete user success")
+        } catch {
+            XCTFail("Received failure with error \(error)")
+        }
+    }
+
     // MARK: - Service Exceptions
 
     /// Test a deleteUser with `InternalErrorException` from service

Original file line number	Diff line number	Diff line change
`@@ -159,7 +159,9 @@ extension AWSCognitoAuthPlugin: AuthCategoryBehavior {`
`159`	`159`	`}`
`160`	`160`
`161`	`161`	`public func deleteUser() async throws {`
`162`		`- let task = AWSAuthDeleteUserTask(authStateMachine: self.authStateMachine)`
	`162`	`+ let task = AWSAuthDeleteUserTask(`
	`163`	`+ authStateMachine: self.authStateMachine,`
	`164`	`+ authConfiguraiton: authConfiguration)`
`163`	`165`	`_ = try await taskQueue.sync {`
`164`	`166`	`return try await task.value`
`165`	`167`	`}`
Original file line number	Diff line number	Diff line change
`@@ -14,3 +14,9 @@ protocol AuthErrorConvertible {`
`14`	`14`
`15`	`15`	`var authError: AuthError { get }`
`16`	`16`	`}`
	`17`	`+`
	`18`	`+extension AuthError: AuthErrorConvertible {`
	`19`	`+ var authError: AuthError {`
	`20`	`+ return self`
	`21`	`+ }`
	`22`	`+}`
Original file line number	Diff line number	Diff line change
`@@ -22,6 +22,8 @@ struct AuthEvent: StateMachineEvent {`
`22`	`22`	`case authenticationConfigured(AuthConfiguration, AmplifyCredentials)`
`23`	`23`
`24`	`24`	`case authorizationConfigured`
	`25`	`+`
	`26`	`+ case reconfigure(AuthConfiguration)`
`25`	`27`	`}`
`26`	`28`
`27`	`29`	`var id: String`
`@@ -38,6 +40,7 @@ struct AuthEvent: StateMachineEvent {`
`38`	`40`	`case .authenticationConfigured: return "AuthEvent.authenticationConfigured"`
`39`	`41`	`case .authorizationConfigured: return "AuthEvent.authorizationConfigured"`
`40`	`42`	`case .validateCredentialAndConfiguration: return "AuthEvent.validateCredentialAndConfiguration"`
	`43`	`+ case .reconfigure: return "AuthEvent.reconfigure"`
`41`	`44`	`}`
`42`	`45`	`}`
`43`	`46`
Original file line number	Diff line number	Diff line change
`@@ -12,20 +12,29 @@ import AWSPluginsCore`
`12`	`12`	`class AWSAuthDeleteUserTask: AuthDeleteUserTask {`
`13`	`13`	`private let authStateMachine: AuthStateMachine`
`14`	`14`	`private let taskHelper: AWSAuthTaskHelper`
	`15`	`+ private let configuration: AuthConfiguration`
`15`	`16`
`16`	`17`	`var eventName: HubPayloadEventName {`
`17`	`18`	`HubPayload.EventName.Auth.deleteUserAPI`
`18`	`19`	`}`
`19`	`20`
`20`		`- init(authStateMachine: AuthStateMachine) {`
	`21`	`+ init(authStateMachine: AuthStateMachine,`
	`22`	`+ authConfiguraiton: AuthConfiguration) {`
`21`	`23`	`self.authStateMachine = authStateMachine`
	`24`	`+ self.configuration = authConfiguraiton`
`22`	`25`	`self.taskHelper = AWSAuthTaskHelper(authStateMachine: authStateMachine)`
`23`	`26`	`}`
`24`	`27`
`25`	`28`	`func execute() async throws {`
`26`	`29`	`await taskHelper.didStateMachineConfigured()`
`27`	`30`	`let accessToken = try await taskHelper.getAccessToken()`
`28`		`- try await deleteUser(with: accessToken)`
	`31`	`+`
	`32`	`+ do {`
	`33`	`+ try await deleteUser(with: accessToken)`
	`34`	`+ } catch {`
	`35`	`+ await waitForReConfigure()`
	`36`	`+ throw error`
	`37`	`+ }`
`29`	`38`	`}`
`30`	`39`
`31`	`40`	`private func deleteUser(with token: String) async throws {`
`@@ -52,4 +61,10 @@ class AWSAuthDeleteUserTask: AuthDeleteUserTask {`
`52`	`61`	`}`
`53`	`62`	`}`
`54`	`63`	`}`
	`64`	`+`
	`65`	`+ private func waitForReConfigure() async {`
	`66`	`+ let event = AuthEvent(eventType: .reconfigure(configuration))`
	`67`	`+ await authStateMachine.send(event)`
	`68`	`+ await taskHelper.didStateMachineConfigured()`
	`69`	`+ }`
`55`	`70`	`}`