PM-11883: Improve handling of biometric unlock errors (#917)

Author: matt-livefront

BitwardenShared/Core/Auth/Services/Biometrics/BiometricsRepository.swift

@@ -173,11 +173,9 @@ class DefaultBiometricsRepository: BiometricsRepository {
                      kLAErrorUserFallback:
                     throw BiometricsServiceError.biometryFailed
                 default:
-                    throw BiometricsServiceError.getAuthKeyFailed
+                    throw error
                 }
             }
-        } catch {
-            throw BiometricsServiceError.getAuthKeyFailed
         }
     }
 }

BitwardenShared/Core/Auth/Services/Biometrics/BiometricsRepositoryTests.swift

@@ -350,7 +350,7 @@ final class BiometricsRepositoryTests: BitwardenTestCase { // swiftlint:disable:
         }
     }
 
-    /// `getUserAuthKey` retrieves the key from keychain and updates integrity state.
+    /// `getUserAuthKey` throws an error if one occurs.
     func test_getUserAuthKey_unknownError() async throws {
         let active = Account.fixture()
         stateService.activeAccount = active
@@ -360,7 +360,22 @@ final class BiometricsRepositoryTests: BitwardenTestCase { // swiftlint:disable:
             active.profile.userId: true,
         ]
         keychainService.getResult = .failure(BitwardenTestError.example)
-        await assertAsyncThrows(error: BiometricsServiceError.getAuthKeyFailed) {
+        await assertAsyncThrows(error: BitwardenTestError.example) {
+            _ = try await subject.getUserAuthKey()
+        }
+    }
+
+    /// `getUserAuthKey` throws an error if an unknown OS error occurs.
+    func test_getUserAuthKey_unknownOSError() async throws {
+        let active = Account.fixture()
+        stateService.activeAccount = active
+        let integrity = Data("Face/Off".utf8)
+        biometricsService.biometricIntegrityState = integrity
+        stateService.biometricsEnabled = [
+            active.profile.userId: true,
+        ]
+        keychainService.getResult = .failure(KeychainServiceError.osStatusError(errSecParam))
+        await assertAsyncThrows(error: KeychainServiceError.osStatusError(errSecParam)) {
             _ = try await subject.getUserAuthKey()
         }
     }

BitwardenShared/Core/Platform/Services/ErrorReporter/BitwardenError.swift

@@ -45,14 +45,17 @@ enum BitwardenError {
     /// - Parameters:
     ///   - type: The type of error. This is used to group the errors in the Crashlytics dashboard.
     ///   - message: A message describing the error that occurred.
+    ///   - error: An optional underlying error that caused the error.
     ///
-    static func generalError(type: String, message: String) -> NSError {
-        NSError(
+    static func generalError(type: String, message: String, error: Error? = nil) -> NSError {
+        var userInfo: [String: Any] = ["ErrorMessage": message]
+        if let error {
+            userInfo[NSUnderlyingErrorKey] = error
+        }
+        return NSError(
             domain: "General Error: \(type)",
             code: Code.generalError.rawValue,
-            userInfo: [
-                "ErrorMessage": message,
-            ]
+            userInfo: userInfo
         )
     }
 

BitwardenShared/UI/Auth/VaultUnlock/VaultUnlockProcessor.swift

@@ -243,29 +243,32 @@ class VaultUnlockProcessor: StateProcessor<
             await coordinator.handleEvent(.didCompleteAuth)
             state.unsuccessfulUnlockAttemptsCount = 0
             await services.stateService.setUnsuccessfulUnlockAttempts(0)
-        } catch let error as BiometricsServiceError {
-            Logger.processor.error("BiometricsServiceError unlocking vault with biometrics: \(error)")
+        } catch BiometricsServiceError.biometryCancelled {
+            Logger.processor.error("Biometric unlock cancelled.")
+            // Do nothing if the user cancels.
+        } catch BiometricsServiceError.biometryLocked {
+            Logger.processor.error("Biometric unlock failed duo to biometrics lockout.")
             // If the user has locked biometry, logout immediately.
-            if case .biometryLocked = error {
-                await logoutUser(userInitiated: true)
-                return
-            }
-            if case .biometryCancelled = error {
-                // Do nothing if the user cancels.
-                return
-            }
-            // There is no biometric auth key stored, set user preference to false.
-            if case .getAuthKeyFailed = error {
-                try? await services.authRepository.allowBioMetricUnlock(false)
-            }
+            await logoutUser(userInitiated: true)
+        } catch BiometricsServiceError.getAuthKeyFailed {
+            services.errorReporter.log(error: BitwardenError.generalError(
+                type: "VaultUnlock: Get Biometrics Auth Key Failed",
+                message: "Biometrics auth is enabled but key was unable to be found. Disabling biometric unlock."
+            ))
+            try? await services.authRepository.allowBioMetricUnlock(false)
             await loadData()
         } catch let error as StateServiceError {
             // If there is no active account, don't add to the unsuccessful count.
-            Logger.processor.error("StateServiceError unlocking vault with biometrics: \(error)")
+            services.errorReporter.log(error: error)
             // Just send the user back to landing.
             coordinator.navigate(to: .landing)
         } catch {
-            Logger.processor.error("Error unlocking vault with biometrics: \(error)")
+            services.errorReporter.log(error: BitwardenError.generalError(
+                type: "VaultUnlock: Biometrics Unlock Error",
+                message: "A biometrics error occurred.",
+                error: error
+            ))
+
             state.unsuccessfulUnlockAttemptsCount += 1
             await services.stateService
                 .setUnsuccessfulUnlockAttempts(state.unsuccessfulUnlockAttemptsCount)

BitwardenShared/UI/Auth/VaultUnlock/VaultUnlockProcessorTests.swift

@@ -487,6 +487,21 @@ class VaultUnlockProcessorTests: BitwardenTestCase { // swiftlint:disable:this t
         XCTAssertEqual(attemptsInUserDefaults, 0)
     }
 
+    /// `perform(_:)` with `.unlockVaultWithBiometrics` logs the user out if biometrics is locked
+    /// due to too many failed attempts.
+    @MainActor
+    func test_perform_unlockWithBiometrics_biometryLocked() async throws {
+        stateService.activeAccount = .fixture()
+        biometricsRepository.biometricUnlockStatus = .success(
+            .available(.touchID, enabled: true, hasValidIntegrity: true)
+        )
+        authRepository.unlockVaultWithBiometricsResult = .failure(BiometricsServiceError.biometryLocked)
+
+        await subject.perform(.unlockVaultWithBiometrics)
+        XCTAssertNil(coordinator.routes.last)
+        XCTAssertEqual(coordinator.events, [.action(.logout(userId: nil, userInitiated: true))])
+    }
+
     /// `perform(_:)` with `.unlockVaultWithBiometrics` shows the KDF warning in an extension if the
     /// KDF memory is potentially too high.
     @MainActor
@@ -521,6 +536,7 @@ class VaultUnlockProcessorTests: BitwardenTestCase { // swiftlint:disable:this t
         await subject.perform(.unlockVaultWithBiometrics)
         let route = try XCTUnwrap(coordinator.routes.last)
         XCTAssertEqual(route, .landing)
+        XCTAssertEqual(errorReporter.errors as? [StateServiceError], [.noActiveAccount])
     }
 
     /// `perform(_:)` with `.unlockWithBiometrics` requires a set user preference.
@@ -573,6 +589,12 @@ class VaultUnlockProcessorTests: BitwardenTestCase { // swiftlint:disable:this t
         await subject.perform(.unlockVaultWithBiometrics)
         XCTAssertNil(coordinator.routes.last)
         XCTAssertEqual(1, subject.state.unsuccessfulUnlockAttemptsCount)
+
+        XCTAssertEqual(errorReporter.errors.count, 1)
+        XCTAssertEqual(
+            (errorReporter.errors[0] as NSError).domain,
+            "General Error: VaultUnlock: Biometrics Unlock Error"
+        )
     }
 
     /// `perform(_:)` with `.unlockWithBiometrics` requires successful biometrics.
@@ -594,6 +616,12 @@ class VaultUnlockProcessorTests: BitwardenTestCase { // swiftlint:disable:this t
                 .logout(userId: nil, userInitiated: true)
             )
         )
+
+        XCTAssertEqual(errorReporter.errors.count, 1)
+        XCTAssertEqual(
+            (errorReporter.errors[0] as NSError).domain,
+            "General Error: VaultUnlock: Biometrics Unlock Error"
+        )
     }
 
     /// `perform(_:)` with `.unlockWithBiometrics` requires successful biometrics.