fix: Fixing service errors being reported as .unknown when sign in fa…

…ils (#170) * fix: Fixing service errors being reported as .unknown when sign in fails. Also adding proper WebAuthn cases to the AWSCognitoAuthError enum. * addressing PR comment
aws-amplify · Nov 13, 2024 · ed7fbf9 · ed7fbf9
1 parent f55d2bb
commit ed7fbf9
Show file tree

Hide file tree

Showing 9 changed files with 93 additions and 43 deletions.
diff --git a/...Auth/Sources/AWSCognitoAuthPlugin/Actions/SignIn/WebAuthn/AssertWebAuthnCredentials.swift b/...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/...ns/Auth/Sources/AWSCognitoAuthPlugin/Actions/SignIn/WebAuthn/FetchCredentialOptions.swift b/...ns/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/...lugins/Auth/Sources/AWSCognitoAuthPlugin/Actions/SignIn/WebAuthn/InitializeWebAuthn.swift b/...lugins/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/.../Auth/Sources/AWSCognitoAuthPlugin/Actions/SignIn/WebAuthn/VerifyWebAuthnCredential.swift b/.../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/...itoAuthPlugin/Service/ErrorMapping/AWSCongnitoIdentityProvider+AuthErrorConvertible.swift b/...itoAuthPlugin/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/...uginUnitTests/TaskTests/WebAuthnBehaviourTests/AssociateWebAuthnCredentialTaskTests.swift b/...uginUnitTests/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/...hPluginUnitTests/TaskTests/WebAuthnBehaviourTests/DeleteWebAuthnCredentialTaskTests.swift b/...hPluginUnitTests/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/...thPluginUnitTests/TaskTests/WebAuthnBehaviourTests/ListWebAuthnCredentialsTaskTests.swift b/...thPluginUnitTests/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)")
         }