Six pre-existing bugs found reviewing 6.0.0-dev.1

CHANGELOG.md

@@ -1,3 +1,59 @@
+## 6.0.0-dev.2
+
+Six pre-existing bugs, all found while reviewing 6.0.0-dev.1 and none introduced
+by it.
+
+* android: an authentication requested while the activity cannot host a dialog
+  no longer hangs forever. `androidx.biometric` refuses to start after
+  `onSaveInstanceState` — logging `Unable to start authentication` — and returns
+  without invoking any callback, so the pending Flutter result never completed.
+  The state is checked up front and reported as
+  `AuthException(AuthExceptionCode.unknown)` instead. Reachable whenever the app
+  is backgrounded, independently of the configuration-change bug below.
+* android: the plugin kept using a destroyed `Activity` after a configuration
+  change. `onReattachedToActivityForConfigChanges` and
+  `onDetachedFromActivityForConfigChanges` were both empty, so after a rotation
+  every authenticated read or write handed `BiometricPrompt` a dead
+  `FragmentActivity`. `ActivityAware` documents that the old reference must be
+  cleared and the new binding adopted; now both happen.
+* android: a storage-not-initialized error delivered its message as the
+  `PlatformException.code` — the field callers match on. The code is now
+  `NotInitialized`.
+* iOS/macOS: `init` replied twice when an argument was missing or mistyped, so
+  the caller saw the error and then a success. The success reply now only
+  happens on the success path.
+* **Breaking**: iOS/macOS `canAuthenticate()` reports an unrecognised `LAError`
+  as `CanAuthenticateResponse.statusUnknown` rather than `unsupported`, matching
+  Android. `unsupported` means "the plugin does not support this platform", so a
+  recoverable `biometryLockout` was telling callers to give up entirely. Callers
+  that treat `statusUnknown` as usable will now attempt authentication where
+  they previously did not.
+* **Breaking**: iOS/macOS, a repeat `getStorage()` for a name that is already
+  open no longer rebuilds the store. It used to replace it outright, which threw
+  away the cached `LAContext` — so any
+  `darwinTouchIDAuthenticationForceReuseContextDuration` in progress — and
+  quietly adopted whatever options the second call passed. The first call now
+  wins, as on Android. If you relied on re-initializing to change options, give
+  each set of options its own store name, or restart — there is no API to close
+  a store, so none can be reopened differently within a run. Passing different
+  options to a repeat call was never reported back to Dart, and now definitively
+  does nothing.
+* **Breaking**: `forceInit` now does what it documents on Windows, web, iOS and
+  macOS. It was implemented on Android only; the others accepted the flag and
+  dropped it. It throws `BiometricStorageException` on every platform — the
+  `PlatformException(code: 'AlreadyInitialized')` that the method-channel
+  platforms raise is translated, so one catch clause covers all of them.
+  **Linux still ignores the flag**: its `init` keeps no per-store state at all,
+  so implementing it there is a separate change.
+* **Breaking**: windows `read()` and `delete()` throw `BiometricStorageException`
+  when the credential store fails, instead of returning `null` and `false`.
+  Those values still mean "no value stored" and "there was nothing to delete" —
+  previously they doubled as the answer for a failing store, so a failure read
+  as data loss.
+* iOS/macOS: `canAuthenticate()` names `biometryLockout` explicitly rather than
+  letting it fall through the unmapped-code path, and the nil-error branch
+  reports `statusUnknown` too rather than `unsupported`.
+
 ## 6.0.0-dev.1
 
 **Breaking**: requires Dart 3.10 / Flutter 3.44 or newer.

android/src/main/kotlin/design/codeux/biometric_storage/BiometricStoragePlugin.kt

@@ -149,7 +149,13 @@ class BiometricStoragePlugin : FlutterPlugin, ActivityAware, MethodCallHandler {
                 val name = getName()
                 storageFiles[name]?.apply(cb) ?: run {
                     logger.warn { "User tried to access storage '$name', before initialization" }
-                    result.error("Storage $name was not initialized.", null, null)
+                    // error(code, message, details) — the sentence used to be
+                    // passed as the code, which is the field callers match on.
+                    result.error(
+                        "NotInitialized",
+                        "Storage $name was not initialized.",
+                        null
+                    )
                     return
                 }
             }
@@ -371,6 +377,34 @@ class BiometricStoragePlugin : FlutterPlugin, ActivityAware, MethodCallHandler {
                 )
             )
         }
+        // Being attached is not the same as being able to show a dialog.
+        // androidx's BiometricPrompt refuses to start after onSaveInstanceState
+        // and — this is the part that hurts — returns without invoking any
+        // callback, logging "Unable to start authentication. Called after
+        // onSaveInstanceState()". The pending Flutter result would then never
+        // complete: not an error the caller can catch, a permanent silent hang.
+        // Reachable whenever the app is backgrounded, independently of whether
+        // the activity reference itself is stale.
+        if (activity.isFinishing ||
+            activity.isDestroyed ||
+            activity.supportFragmentManager.isStateSaved
+        ) {
+            return run {
+                logger.error {
+                    "Activity cannot host a prompt right now " +
+                            "(finishing=${activity.isFinishing} " +
+                            "destroyed=${activity.isDestroyed} " +
+                            "stateSaved=${activity.supportFragmentManager.isStateSaved})."
+                }
+                onError(
+                    AuthenticationErrorInfo(
+                        AuthenticationError.Failed,
+                        "Activity is not in a state where an authentication " +
+                                "prompt can be shown."
+                    )
+                )
+            }
+        }
         val prompt =
             BiometricPrompt(activity, executor, object : BiometricPrompt.AuthenticationCallback() {
                 override fun onAuthenticationError(errorCode: Int, errString: CharSequence) {
@@ -436,6 +470,8 @@ class BiometricStoragePlugin : FlutterPlugin, ActivityAware, MethodCallHandler {
     }
 
     override fun onReattachedToActivityForConfigChanges(binding: ActivityPluginBinding) {
+        logger.debug { "Reattached to activity after a configuration change." }
+        updateAttachedActivity(binding.activity)
     }
 
     override fun onAttachedToActivity(binding: ActivityPluginBinding) {
@@ -452,6 +488,12 @@ class BiometricStoragePlugin : FlutterPlugin, ActivityAware, MethodCallHandler {
     }
 
     override fun onDetachedFromActivityForConfigChanges() {
+        logger.trace { "onDetachedFromActivityForConfigChanges" }
+        // ActivityAware's contract: "By the end of this method, the Activity ...
+        // is no longer valid. Any references ... should be cleared." Holding on
+        // to it meant every authenticated read or write after a rotation handed
+        // BiometricPrompt a destroyed FragmentActivity.
+        attachedActivity = null
     }
 }
 

darwin/biometric_storage/Sources/biometric_storage/BiometricStorageImpl.swift

@@ -89,12 +89,32 @@ class BiometricStorageImpl {
         canAuthenticate(options: initOptions, result: result)
       }
     } else if ("init" == call.method) {
-      requiredArg("name") { name in
-        requiredArg("options") { options in
+      requiredArg("name") { (name: String) in
+        requiredArg("options") { (options: [String: Any]) in
+          // The success reply belongs on the success path. It used to sit after
+          // this block and ran unconditionally, so a missing or mistyped
+          // argument replied with the error and then replied `true` as well —
+          // Flutter discards the second reply and logs "Reply already
+          // submitted", leaving the caller believing init had succeeded.
+          if stores[name] != nil {
+            // Mirrors Android: forceInit means "assert this was not already
+            // initialised", and without it a repeat init is a no-op reporting
+            // false. Neither was implemented here at all before.
+            let args = call.arguments as? [String: Any]
+            if args?["forceInit"] as? Bool == true {
+              result(storageError(
+                code: "AlreadyInitialized",
+                message: "A storage file with the name '\(name)' was already initialized.",
+                details: nil))
+            } else {
+              result(false)
+            }
+            return
+          }
           stores[name] = BiometricStorageFile(name: name, initOptions: InitOptions(params: options), storageError: storageError)
+          result(true)
         }
       }
-      result(true)
     } else if ("dispose" == call.method) {
       // nothing to dispose
       result(true)
@@ -139,7 +159,9 @@ class BiometricStorageImpl {
       return
     }
     guard let err = error else {
-      result("ErrorUnknown")
+      // Same reasoning as the default arm below: "cannot tell" is not
+      // "this plugin does not work on this platform".
+      result("ErrorStatusUnknown")
       return
     }
     let laError = LAError(_nsError: err)
@@ -154,9 +176,26 @@ class BiometricStorageImpl {
     case .touchIDNotEnrolled:
       result("ErrorNoBiometricEnrolled")
       break;
+    case .biometryLockout:
+      // Recoverable — too many failed attempts, and the user can retry later or
+      // fall back to the device credential. Named explicitly rather than left
+      // to the default arm so that the "unmapped" log there keeps meaning
+      // "something we have not seen", instead of firing on the common case.
+      // androidx's BiometricManager has no lockout status at all, so Android
+      // callers already get Success here and discover the lockout at the
+      // prompt; reporting "cannot tell, try it" converges the two platforms.
+      result("ErrorStatusUnknown")
+      break;
     case .invalidContext: fallthrough
     default:
-      result("ErrorUnknown")
+      // Not "ErrorUnknown": the Dart side maps that to
+      // CanAuthenticateResponse.unsupported, whose own doc reads "Plugin does
+      // not support platform", so a biometryLockout — too many failed attempts,
+      // and recoverable — told callers the plugin did not work on iOS at all.
+      // ErrorStatusUnknown is what Android reports for a code it does not
+      // recognise, and it says what is actually true: we cannot tell.
+      NSLog("Unmapped LAError \(laError.code.rawValue), reporting status unknown");
+      result("ErrorStatusUnknown")
       break;
     }
   }

example/pubspec.lock

@@ -15,7 +15,7 @@ packages:
       path: ".."
       relative: true
     source: path
-    version: "6.0.0-dev.1"
+    version: "6.0.0-dev.2"
   boolean_selector:
     dependency: transitive
     description:

lib/src/biometric_storage.dart

@@ -403,6 +403,29 @@ class MethodChannelBiometricStorage extends BiometricStorage {
       });
       _logger.finest('getting storage. was created: $result');
       return BiometricStorageFile(this, name, promptInfo);
+    } on PlatformException catch (e, stackTrace) {
+      // The platforms implemented in Dart — Windows and web — raise this
+      // themselves, so without translating here the same failure would reach
+      // callers as two different types and need two catch clauses.
+      if (e.code == 'AlreadyInitialized') {
+        _logger.warning(
+          'Storage $name was already initialized.',
+          e,
+          stackTrace,
+        );
+        // Not a plain `throw`: that would restart the trace here and lose the
+        // frames showing which call re-initialized the store.
+        Error.throwWithStackTrace(
+          BiometricStorageException(e.message ?? e.code),
+          stackTrace,
+        );
+      }
+      _logger.warning(
+        'Error while initializing biometric storage.',
+        e,
+        stackTrace,
+      );
+      rethrow;
     } catch (e, stackTrace) {
       _logger.warning(
         'Error while initializing biometric storage.',

lib/src/biometric_storage_web.dart

@@ -10,6 +10,10 @@ class BiometricStoragePluginWeb extends BiometricStorage {
 
   static const namePrefix = 'design.codeux.authpass.';
 
+  /// See the win32 implementation: [forceInit] has to be tracked here, because
+  /// there is no native side holding a per-store handle.
+  final _initialized = <String>{};
+
   static void registerWith(Registrar registrar) {
     BiometricStorage.instance = BiometricStoragePluginWeb();
   }
@@ -26,6 +30,11 @@ class BiometricStoragePluginWeb extends BiometricStorage {
     bool forceInit = false,
     PromptInfo promptInfo = PromptInfo.defaultValues,
   }) async {
+    if (!_initialized.add(name) && forceInit) {
+      throw BiometricStorageException(
+        "A storage file with the name '$name' was already initialized.",
+      );
+    }
     return BiometricStorageFile(this, namePrefix + name, promptInfo);
   }
 

lib/src/biometric_storage_win32.dart

@@ -17,6 +17,11 @@ class Win32BiometricStoragePlugin extends BiometricStorage {
 
   static const _userName = 'flutter.biometric_storage';
 
+  /// Names handed out by [getStorage] in this runtime, so that [forceInit] can
+  /// mean what the API documents. Windows has no per-store native handle to
+  /// hang this off, unlike the method-channel platforms.
+  final _initialized = <String>{};
+
   /// Registers this class as the default instance of [BiometricStorage].
   static void registerWith() {
     BiometricStorage.instance = Win32BiometricStoragePlugin();
@@ -36,6 +41,13 @@ class Win32BiometricStoragePlugin extends BiometricStorage {
     bool forceInit = false,
     PromptInfo promptInfo = PromptInfo.defaultValues,
   }) async {
+    // forceInit was accepted and dropped here, so the documented "will throw if
+    // the store was already created in this runtime" held on Android alone.
+    if (!_initialized.add(name) && forceInit) {
+      throw BiometricStorageException(
+        "A storage file with the name '$name' was already initialized.",
+      );
+    }
     return BiometricStorageFile(this, namePrefix + name, promptInfo);
   }
 
@@ -51,6 +63,13 @@ class Win32BiometricStoragePlugin extends BiometricStorage {
       );
       if (!result.value) {
         _logFailure('deleting', name, result.error);
+        // Same distinction read() makes: `false` means there was nothing to
+        // delete, so a store that is failing must not borrow that answer.
+        if (result.error != ERROR_NOT_FOUND) {
+          throw BiometricStorageException(
+            'Error deleting credential $name: ${result.error}',
+          );
+        }
         return false;
       }
       return true;
@@ -69,6 +88,14 @@ class Win32BiometricStoragePlugin extends BiometricStorage {
       );
       if (!result.value) {
         _logFailure('reading', name, result.error);
+        // `null` is the documented answer to "no value stored". Reporting a
+        // credential-store failure the same way left a caller unable to tell
+        // an empty store from a broken one, so it read as data loss.
+        if (result.error != ERROR_NOT_FOUND) {
+          throw BiometricStorageException(
+            'Error reading credential $name: ${result.error}',
+          );
+        }
         return null;
       }
       final credential = credentialPointer.value;

pubspec.yaml

@@ -2,7 +2,7 @@ name: biometric_storage
 description: |
   Secure Storage: Encrypted data store optionally secured by biometric lock with support
   for iOS, Android, MacOS. Partial support for Linux, Windows and web (localStorage).
-version: 6.0.0-dev.1
+version: 6.0.0-dev.2
 homepage: https://github.com/authpass/biometric_storage/
 
 environment:

test/biometric_storage_win32_test.dart

@@ -66,6 +66,21 @@ void main() {
     expect(file.name, 'design.codeux.authpass.example');
   });
 
+  test('forceInit rejects a second getStorage for the same name', () async {
+    final name = 'test_force_${DateTime.now().microsecondsSinceEpoch}';
+
+    // The first call establishes it; a plain second call is a no-op. Only
+    // forceInit turns "already initialized" into an error, which is what the
+    // API documents and what Android has always done.
+    await plugin.getStorage(name);
+    await plugin.getStorage(name);
+
+    expect(
+      () => plugin.getStorage(name, forceInit: true),
+      throwsA(isA<BiometricStorageException>()),
+    );
+  });
+
   test('reading an unknown name returns null rather than throwing', () async {
     final file = await plugin.getStorage(
       'test_absent_${DateTime.now().microsecondsSinceEpoch}',