fix: Fixing service errors being reported as .unknown when sign in fails (#170)

sebaland · web-flow · commit ed7fbf943395 · 2024-11-13T18:23:48.000-05:00
* fix: Fixing service errors being reported as .unknown when sign in fails. Also adding proper WebAuthn cases to the AWSCognitoAuthError enum.

* addressing PR comment
diff --git a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Actions/SignIn/WebAuthn/AssertWebAuthnCredentials.swift b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Actions/SignIn/WebAuthn/AssertWebAuthnCredentials.swift
@@ -52,24 +52,27 @@ struct AssertWebAuthnCredentials: Action {
             ))
             logVerbose("\(#fileID) Sending event \(event)", environment: environment)
             await dispatcher.send(event)
-        } catch let error as WebAuthnError {
-            logVerbose("\(#fileID) Raised error \(error)", environment: environment)
-            let event = WebAuthnEvent(
-                eventType: .error(error, respondToAuthChallenge)
-            )
-            await dispatcher.send(event)
         } catch {
             logVerbose("\(#fileID) Raised error \(error)", environment: environment)
-            let webAuthnError = WebAuthnError.unknown(
-                message: "Unable to assert WebAuthn credentials",
-                error: error
-            )
             let event = WebAuthnEvent(
-                eventType: .error(webAuthnError, respondToAuthChallenge)
+                eventType: .error(webAuthnError(from: error), respondToAuthChallenge)
             )
             await dispatcher.send(event)
         }
     }
+
+    private func webAuthnError(from error: Error) -> WebAuthnError {
+        if let webAuthnError = error as? WebAuthnError {
+            return webAuthnError
+        }
+        if let authError = error as? AuthErrorConvertible {
+            return .service(error: authError.authError)
+        }
+        return .unknown(
+            message: "Unable to assert WebAuthn credentials",
+            error: error
+        )
+    }
 }
 
 @available(iOS 17.4, macOS 13.5, *)
diff --git a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Actions/SignIn/WebAuthn/FetchCredentialOptions.swift b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Actions/SignIn/WebAuthn/FetchCredentialOptions.swift
@@ -67,16 +67,25 @@ struct FetchCredentialOptions: Action {
             await dispatcher.send(event)
         } catch {
             logVerbose("\(#fileID) Caught error \(error)", environment: environment)
-            let webAuthnError = WebAuthnError.unknown(
-                message: "Unable to fetch credential creation options",
-                error: error
-            )
             let event = WebAuthnEvent(
-                eventType: .error(webAuthnError, respondToAuthChallenge)
+                eventType: .error(webAuthnError(from: error), respondToAuthChallenge)
             )
             await dispatcher.send(event)
         }
     }
+
+    private func webAuthnError(from error: Error) -> WebAuthnError {
+        if let webAuthnError = error as? WebAuthnError {
+            return webAuthnError
+        }
+        if let authError = error as? AuthErrorConvertible {
+            return .service(error: authError.authError)
+        }
+        return .unknown(
+            message: "Unable to fetch credential creation options",
+            error: error
+        )
+    }
 }
 
 extension FetchCredentialOptions: DefaultLogger { }
diff --git a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Actions/SignIn/WebAuthn/InitializeWebAuthn.swift b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Actions/SignIn/WebAuthn/InitializeWebAuthn.swift
@@ -52,16 +52,25 @@ struct InitializeWebAuthn: Action {
             await dispatcher.send(event)
         } catch {
             logVerbose("\(#fileID) Caught error \(error)", environment: environment)
-            let webAuthnError = WebAuthnError.unknown(
-                message: "Unable to initiate WebAuthn flow",
-                error: error
-            )
             let event = WebAuthnEvent(
-                eventType: .error(webAuthnError, respondToAuthChallenge)
+                eventType: .error(webAuthnError(from: error), respondToAuthChallenge)
             )
             await dispatcher.send(event)
         }
     }
+    
+    private func webAuthnError(from error: Error) -> WebAuthnError {
+        if let webAuthnError = error as? WebAuthnError {
+            return webAuthnError
+        }
+        if let authError = error as? AuthErrorConvertible {
+            return .service(error: authError.authError)
+        }
+        return .unknown(
+            message: "Unable to initiate WebAuthn flow",
+            error: error
+        )
+    }
 }
 
 extension InitializeWebAuthn: DefaultLogger { }
diff --git a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Actions/SignIn/WebAuthn/VerifyWebAuthnCredential.swift b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Actions/SignIn/WebAuthn/VerifyWebAuthnCredential.swift
@@ -70,16 +70,25 @@ struct VerifyWebAuthnCredential: Action {
             await dispatcher.send(event)
         } catch {
             logVerbose("\(#fileID) Caught error \(error)", environment: environment)
-            let webAuthnError = WebAuthnError.unknown(
-                message: "Unable to verify WebAuthn credential",
-                error: error
-            )
             let event = WebAuthnEvent(
-                eventType: .error(webAuthnError, respondToAuthChallenge)
+                eventType: .error(webAuthnError(from: error), respondToAuthChallenge)
             )
             await dispatcher.send(event)
         }
     }
+
+    private func webAuthnError(from error: Error) -> WebAuthnError {
+        if let webAuthnError = error as? WebAuthnError {
+            return webAuthnError
+        }
+        if let authError = error as? AuthErrorConvertible {
+            return .service(error: authError.authError)
+        }
+        return .unknown(
+            message: "Unable to verify WebAuthn credential",
+            error: error
+        )
+    }
 }
 
 @available(iOS 17.4, macOS 13.5, *)
diff --git a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Models/Errors/AWSCognitoAuthError.swift b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Models/Errors/AWSCognitoAuthError.swift
@@ -89,6 +89,24 @@ public enum AWSCognitoAuthError: Error {
     /// Thrown when a user tries to use a login which is already linked to another account.
     case resourceConflictException
 
-    /// WebAuthn related issue
-    case webAuthn
+    /// The WebAuthn credentials don't match an existing request
+    case webAuthnChallengeNotFound
+
+    /// The client doesn't support WebAuhn authentication
+    case webAuthnClientMismatch
+
+    /// WebAuthn is not supported on this device
+    case webAuthnNotSupported
+
+    /// WebAuthn is not enabled
+    case webAuthnNotEnabled
+
+    /// The device origin is not registered as an allowed origin
+    case webAuthnOriginNotAllowed
+
+    /// The relying party ID doesn't match
+    case webAuthnRelyingPartyMismatch
+
+    /// The WebAuthm configuration is missing or incomplete
+    case webAuthnConfigurationMissing
 }
diff --git a/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Service/ErrorMapping/AWSCongnitoIdentityProvider+AuthErrorConvertible.swift b/AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Service/ErrorMapping/AWSCongnitoIdentityProvider+AuthErrorConvertible.swift
@@ -345,7 +345,7 @@ extension AWSCognitoIdentityProvider.WebAuthnChallengeNotFoundException: AuthErr
         .service(
             message ?? fallbackDescription,
             AuthPluginErrorConstants.webAuthnChallengeNotFound,
-            AWSCognitoAuthError.webAuthn
+            AWSCognitoAuthError.webAuthnChallengeNotFound
         )
     }
 }
@@ -357,7 +357,7 @@ extension AWSCognitoIdentityProvider.WebAuthnClientMismatchException: AuthErrorC
         .service(
             message ?? fallbackDescription,
             AuthPluginErrorConstants.webAuthnClientMismatch,
-            AWSCognitoAuthError.webAuthn
+            AWSCognitoAuthError.webAuthnClientMismatch
         )
     }
 }
@@ -369,7 +369,7 @@ extension AWSCognitoIdentityProvider.WebAuthnCredentialNotSupportedException: Au
         .service(
             message ?? fallbackDescription,
             AuthPluginErrorConstants.webAuthnCredentialNotSupported,
-            AWSCognitoAuthError.webAuthn
+            AWSCognitoAuthError.webAuthnNotSupported
         )
     }
 }
@@ -381,7 +381,7 @@ extension AWSCognitoIdentityProvider.WebAuthnNotEnabledException: AuthErrorConve
         .service(
             message ?? fallbackDescription,
             AuthPluginErrorConstants.webAuthnNotEnabled,
-            AWSCognitoAuthError.webAuthn
+            AWSCognitoAuthError.webAuthnNotEnabled
         )
     }
 }
@@ -393,7 +393,7 @@ extension AWSCognitoIdentityProvider.WebAuthnOriginNotAllowedException: AuthErro
         .service(
             message ?? fallbackDescription,
             AuthPluginErrorConstants.webAuthnOriginNotAllowed,
-            AWSCognitoAuthError.webAuthn
+            AWSCognitoAuthError.webAuthnOriginNotAllowed
         )
     }
 }
@@ -405,7 +405,7 @@ extension AWSCognitoIdentityProvider.WebAuthnRelyingPartyMismatchException: Auth
         .service(
             message ?? fallbackDescription,
             AuthPluginErrorConstants.webAuthnRelyingPartyMismatch,
-            AWSCognitoAuthError.webAuthn
+            AWSCognitoAuthError.webAuthnRelyingPartyMismatch
         )
     }
 }
@@ -417,7 +417,7 @@ extension AWSCognitoIdentityProvider.WebAuthnConfigurationMissingException: Auth
         .service(
             message ?? fallbackDescription,
             AuthPluginErrorConstants.webAuthnConfigurationMissing,
-            nil
+            AWSCognitoAuthError.webAuthnConfigurationMissing
         )
     }
 }
diff --git a/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/TaskTests/WebAuthnBehaviourTests/AssociateWebAuthnCredentialTaskTests.swift b/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/TaskTests/WebAuthnBehaviourTests/AssociateWebAuthnCredentialTaskTests.swift
@@ -160,7 +160,7 @@ class AssociateWebAuthnCredentialTaskTests: XCTestCase {
                 return
             }
 
-            XCTAssertEqual(underlyingError as? AWSCognitoAuthError, AWSCognitoAuthError.webAuthn)
+            XCTAssertEqual(underlyingError as? AWSCognitoAuthError, AWSCognitoAuthError.webAuthnNotEnabled)
             XCTAssertEqual(credentialRegistrant.createCallCount, 0)
             XCTAssertEqual(completeWebAuthnRegistrationCallCount, 0)
         } catch {
@@ -216,7 +216,7 @@ class AssociateWebAuthnCredentialTaskTests: XCTestCase {
                 return
             }
 
-            XCTAssertEqual(underlyingError as? AWSCognitoAuthError, AWSCognitoAuthError.webAuthn)
+            XCTAssertEqual(underlyingError as? AWSCognitoAuthError, AWSCognitoAuthError.webAuthnNotEnabled)
             XCTAssertEqual(startWebAuthnRegistrationCallCount, 1)
             XCTAssertEqual(credentialRegistrant.createCallCount, 1)
         } catch {
diff --git a/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/TaskTests/WebAuthnBehaviourTests/DeleteWebAuthnCredentialTaskTests.swift b/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/TaskTests/WebAuthnBehaviourTests/DeleteWebAuthnCredentialTaskTests.swift
@@ -8,7 +8,7 @@
 @testable import AWSCognitoAuthPlugin
 import enum Amplify.AuthError
 import enum AWSCognitoIdentity.CognitoIdentityClientTypes
-import struct AWSCognitoIdentityProvider.ForbiddenException
+import struct AWSCognitoIdentityProvider.WebAuthnClientMismatchException
 import XCTest
 
 class DeleteWebAuthnCredentialTaskTests: XCTestCase {
@@ -83,17 +83,18 @@ class DeleteWebAuthnCredentialTaskTests: XCTestCase {
 
     func testExecute_withServiceError_shouldFailWithServiceError() async {
         identityProvider.mockDeleteWebAuthnCredentialResponse = { _ in
-            throw ForbiddenException(message: "Operation is forbidden")
+            throw WebAuthnClientMismatchException(message: "Client mismatch")
         }
 
         do {
             _ = try await task.execute()
             XCTFail("Task should have failed")
         } catch let error as AuthError {
-            guard case .service = error else {
+            guard case .service(_, _, let underlyingError) = error else {
                 XCTFail("Expected AuthError.service error, got \(error)")
                 return
             }
+            XCTAssertEqual(underlyingError as? AWSCognitoAuthError, AWSCognitoAuthError.webAuthnClientMismatch)
         } catch {
             XCTFail("Expected AuthError error, got \(error)")
         }
diff --git a/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/TaskTests/WebAuthnBehaviourTests/ListWebAuthnCredentialsTaskTests.swift b/AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/TaskTests/WebAuthnBehaviourTests/ListWebAuthnCredentialsTaskTests.swift
@@ -8,7 +8,7 @@
 @testable import AWSCognitoAuthPlugin
 import enum Amplify.AuthError
 import enum AWSCognitoIdentity.CognitoIdentityClientTypes
-import struct AWSCognitoIdentityProvider.ForbiddenException
+import struct AWSCognitoIdentityProvider.WebAuthnRelyingPartyMismatchException
 import XCTest
 
 class ListWebAuthnCredentialsTaskTests: XCTestCase {
@@ -109,17 +109,18 @@ class ListWebAuthnCredentialsTaskTests: XCTestCase {
 
     func testExecute_withServiceError_shouldFailWithServiceError() async {
         identityProvider.mockListWebAuthnCredentialsResponse = { _ in
-            throw ForbiddenException(message: "Operation is forbidden")
+            throw WebAuthnRelyingPartyMismatchException(message: "Operation is forbidden")
         }
 
         do {
             _ = try await task.execute()
             XCTFail("Task should have failed")
         } catch let error as AuthError {
-            guard case .service = error else {
+            guard case .service(_, _, let underlyingError) = error else {
                 XCTFail("Expected AuthError.service error, got \(error)")
                 return
             }
+            XCTAssertEqual(underlyingError as? AWSCognitoAuthError, AWSCognitoAuthError.webAuthnRelyingPartyMismatch)
         } catch {
             XCTFail("Expected AuthError error, got \(error)")
         }

Original file line number	Diff line number	Diff line change
`@@ -345,7 +345,7 @@ extension AWSCognitoIdentityProvider.WebAuthnChallengeNotFoundException: AuthErr`
`345`	`345`	`.service(`
`346`	`346`	`message ?? fallbackDescription,`
`347`	`347`	`AuthPluginErrorConstants.webAuthnChallengeNotFound,`
`348`		`- AWSCognitoAuthError.webAuthn`
	`348`	`+ AWSCognitoAuthError.webAuthnChallengeNotFound`
`349`	`349`	`)`
`350`	`350`	`}`
`351`	`351`	`}`
`@@ -357,7 +357,7 @@ extension AWSCognitoIdentityProvider.WebAuthnClientMismatchException: AuthErrorC`
`357`	`357`	`.service(`
`358`	`358`	`message ?? fallbackDescription,`
`359`	`359`	`AuthPluginErrorConstants.webAuthnClientMismatch,`
`360`		`- AWSCognitoAuthError.webAuthn`
	`360`	`+ AWSCognitoAuthError.webAuthnClientMismatch`
`361`	`361`	`)`
`362`	`362`	`}`
`363`	`363`	`}`
`@@ -369,7 +369,7 @@ extension AWSCognitoIdentityProvider.WebAuthnCredentialNotSupportedException: Au`
`369`	`369`	`.service(`
`370`	`370`	`message ?? fallbackDescription,`
`371`	`371`	`AuthPluginErrorConstants.webAuthnCredentialNotSupported,`
`372`		`- AWSCognitoAuthError.webAuthn`
	`372`	`+ AWSCognitoAuthError.webAuthnNotSupported`
`373`	`373`	`)`
`374`	`374`	`}`
`375`	`375`	`}`
`@@ -381,7 +381,7 @@ extension AWSCognitoIdentityProvider.WebAuthnNotEnabledException: AuthErrorConve`
`381`	`381`	`.service(`
`382`	`382`	`message ?? fallbackDescription,`
`383`	`383`	`AuthPluginErrorConstants.webAuthnNotEnabled,`
`384`		`- AWSCognitoAuthError.webAuthn`
	`384`	`+ AWSCognitoAuthError.webAuthnNotEnabled`
`385`	`385`	`)`
`386`	`386`	`}`
`387`	`387`	`}`
`@@ -393,7 +393,7 @@ extension AWSCognitoIdentityProvider.WebAuthnOriginNotAllowedException: AuthErro`
`393`	`393`	`.service(`
`394`	`394`	`message ?? fallbackDescription,`
`395`	`395`	`AuthPluginErrorConstants.webAuthnOriginNotAllowed,`
`396`		`- AWSCognitoAuthError.webAuthn`
	`396`	`+ AWSCognitoAuthError.webAuthnOriginNotAllowed`
`397`	`397`	`)`
`398`	`398`	`}`
`399`	`399`	`}`
`@@ -405,7 +405,7 @@ extension AWSCognitoIdentityProvider.WebAuthnRelyingPartyMismatchException: Auth`
`405`	`405`	`.service(`
`406`	`406`	`message ?? fallbackDescription,`
`407`	`407`	`AuthPluginErrorConstants.webAuthnRelyingPartyMismatch,`
`408`		`- AWSCognitoAuthError.webAuthn`
	`408`	`+ AWSCognitoAuthError.webAuthnRelyingPartyMismatch`
`409`	`409`	`)`
`410`	`410`	`}`
`411`	`411`	`}`
`@@ -417,7 +417,7 @@ extension AWSCognitoIdentityProvider.WebAuthnConfigurationMissingException: Auth`
`417`	`417`	`.service(`
`418`	`418`	`message ?? fallbackDescription,`
`419`	`419`	`AuthPluginErrorConstants.webAuthnConfigurationMissing,`
`420`		`- nil`
	`420`	`+ AWSCognitoAuthError.webAuthnConfigurationMissing`
`421`	`421`	`)`
`422`	`422`	`}`
`423`	`423`	`}`