Skip to content

add a standalone dispose(), and make it work on every platform - #156

Open
hpoul wants to merge 2 commits into
mainfrom
standalone-dispose
Open

add a standalone dispose(), and make it work on every platform#156
hpoul wants to merge 2 commits into
mainfrom
standalone-dispose

Conversation

@hpoul

@hpoul hpoul commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Adds BiometricStorageFile.dispose(), which forgets a store's initialization without touching its content, so the next getStorage applies a fresh StorageFileInitOptions.

Today the options for a name are resolved once and then ignored for the lifetime of the process, so a store cannot be reconfigured at runtime — changing an authentication validity duration means restarting the app.

Credit

@luckyrat identified this gap and proposed an API for it in #138, following #75 and the fork consolidation in #137. This implements the standalone dispose() rather than that PR's deleteAndDispose: the use case is reconfiguring, and coupling it to deletion made you destroy the stored secret in order to change a duration. delete() then dispose() composes if you want both.

Implemented fresh on current main rather than rebasing #138, which branched from ab59b2b (5.1.1-dev.2); lib/src/biometric_storage.dart has moved +329/-185 and example/lib/main.dart +134 since. #138 also changed no native code, and the method did not actually work on three of the five backends.

What was broken

dispose was already reachable natively. It did not work.

  • Linux had no handler at all, so the call fell through to fl_method_not_implemented_response_new() and reached Dart as a MissingPluginException.
  • iOS/macOS replied true from a no-op that never cleared stores, so the next init took its short circuit, discarded the freshly passed options and replied false — reporting success at every step while leaving the file on its original configuration. This silent one is why the load-bearing test asserts through forceInit rather than trusting a returned bool.
  • Android threw NoSuchStorage on a repeat, the only backend where disposing twice was an error. It now reports whether there was anything to forget.
  • win32 and web needed a fix that predates this API: _initialized was keyed by the bare name while BiometricStorageFile carries the prefixed one, so removing by the name dispose is handed would have matched nothing and quietly returned false forever. Both now key on the prefixed form.

The stored content is deliberately untouched on every platform — this reconfigures rather than erases, and a test pins that.

Two contract decisions worth knowing

A disposed handle is spent. read, write and delete throw a StateError; dispose itself does not, so disposing twice stays legal. Without this the platforms disagreed — Android and darwin fail against their registry, Linux/Windows/web carry on from the name alone — which is a mistake you make on one platform and find on another. The flag is set before the call is awaited, so a failed dispose still spends the handle; the race that closes is reachable, the failure it risks is not.

dispose is scoped to the name, not the handle. Every handle is a view onto one name-keyed initialization, so a spent handle still forgets an initialization created after it. Documented and tested, because short-circuiting a repeat dispose on the local flag would quietly make it handle-scoped and answer wrongly.

Verification

Executed on all five backends — the eleven integration tests are the same suite everywhere, so the counts below are all of them:

what ran mutation-proven
Android emulator, 11/11 integration yes — restoring the NoSuchStorage throw turned 2 red
iOS simulator, 11/11 integration yes — restoring the darwin no-op turned 2 red
macOS real login keychain, 11/11 integration — (shares BiometricStorageImpl.swift with iOS)
Linux CI, real libsecret, 11/11 integration by construction: no handler existed before
Windows CI, real credential store, 19 unit (1 group skipped)

macOS had never run this suite before — it was signing-blocked, and a provisioning profile now exists. It writes to the real login keychain, so the run was checked against a before/after baseline: zero integration_* entries either side, confirming teardown cleans up. No keychain prompt appeared, which is the point of authenticationRequired: false. What it adds beyond iOS is narrow but real: the #if os(macOS) messenger branch in BiometricStoragePlugin.swift, and a real keychain rather than a sandboxed simulator one.

Two honest qualifications. The mutation runs predate the last two tests: the darwin and Android mutations were performed when the suite had 9 and 10 tests respectively, so they prove those fixes are caught, not that every current test is. And Windows never runs the integration suite — it runs biometric_storage_win32_test.dart, which reaches the same dispose paths through the plugin directly rather than through the method channel.

The dispose unit tests are skipped on a Windows host — they drive MethodChannelBiometricStorage, which is unreachable there because the registrant installs Win32BiometricStoragePlugin; biometric_storage_win32_test.dart covers dispose on that host.

flutter analyze --fatal-infos and flutter test clean in both packages.

Breaking

For platform implementations only: dispose is a new member on the BiometricStorage platform interface, so a third-party implementation extending it must add one. No change for callers of the package. Taken now deliberately, while 6.0.0 is still pre-release.

Closes #138.

getStorage resolves a name's StorageFileInitOptions once and then ignores
them, so a store could not be reconfigured while the process lived —
changing an authentication validity duration meant restarting the app.
@luckyrat identified this in #138, following #75 and #137, and proposed
deleteAndDispose. This implements the standalone dispose() instead: the
use case is reconfiguring, and coupling it to delete made you destroy the
stored secret to change a duration. delete() first if you want both.

The method was already reachable natively and did not work.

Linux had no handler, so `dispose` fell through to
fl_method_not_implemented_response_new() and reached Dart as a
MissingPluginException. darwin replied `true` from a no-op that never
cleared `stores`, so the next init took its short circuit, discarded the
freshly passed options and replied `false` — success at every step, with
the file still on its original configuration. That silent one is why the
load-bearing test asserts through forceInit rather than trusting a bool.

Android reported the same operation by throwing NoSuchStorage on a
repeat, the only backend where disposing twice was an error. It now
returns whether there was anything to forget, which is the contract every
platform now shares.

win32 and web needed a fix that predates this API: `_initialized` was
keyed by the bare name while BiometricStorageFile carries the prefixed
one, so removing by the name dispose is handed would have matched nothing
and quietly returned false forever. Both now key on the prefixed form.

The stored content is deliberately untouched everywhere; this
reconfigures rather than erases, and a test pins that.

Verified: nine integration tests green on the iOS simulator, which
compiles the shared darwin sources and uses a sandboxed keychain rather
than the login one. Reintroducing the darwin no-op turned two of the
three new tests red — the reinit one and the idempotence one, while the
content-survives one correctly stayed green — and reverting restored
them. flutter analyze --fatal-infos and flutter test clean in both
packages. Linux and Windows are covered by CI.

Breaking for platform implementations only: `dispose` is a new member on
the BiometricStorage platform interface. No change for callers.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new public API, BiometricStorageFile.dispose(), to forget a store’s runtime initialization (so the next getStorage can apply new StorageFileInitOptions) while deliberately leaving the stored secret intact. This fits the plugin’s cross-platform “single Dart surface, multiple backends” design by wiring the same dispose semantics through Dart, MethodChannel platforms, and the pure-Dart web/win32 implementations.

Changes:

  • Add BiometricStorageFile.dispose() plus a Dart-side “spent handle” guard so disposed handles consistently reject read/write/delete across all platforms.
  • Implement/repair native + platform-layer dispose handling (Linux handler added; Android + darwin fixed to actually forget initialization; win32/web fix name-keying so dispose can find entries).
  • Add tests (integration + unit) pinning: re-init after dispose, idempotent dispose, content preserved, and disposed-handle behavior.

Reviewed changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated no comments.

Show a summary per file
File Description
lib/src/biometric_storage.dart Adds the platform-interface dispose method, MethodChannel implementation, and the BiometricStorageFile.dispose() API + disposed-handle guard.
linux/biometric_storage_plugin.cc Implements a missing Linux dispose handler by removing the name from the initialization registry without deleting the secret.
android/src/main/kotlin/design/codeux/biometric_storage/BiometricStoragePlugin.kt Makes Android dispose idempotent and returns a boolean indicating whether an initialization was forgotten.
darwin/biometric_storage/Sources/biometric_storage/BiometricStorageImpl.swift Fixes darwin dispose from a no-op to actually removing the store entry, returning whether anything was removed.
lib/src/biometric_storage_win32.dart Keys initialization tracking by prefixed name and implements dispose by forgetting that entry.
lib/src/biometric_storage_web.dart Keys initialization tracking by prefixed name and implements dispose by forgetting that entry (leaving localStorage intact).
example/integration_test/storage_test.dart Adds integration tests that prove dispose works (re-init, idempotency, content preserved, spent handle semantics).
test/biometric_storage_test.dart Adds a MethodChannel-level unit test asserting the dispose call reaches the platform with the expected arguments (skipped on Windows).
test/biometric_storage_win32_test.dart Adds win32-specific unit tests covering name-forgetting, content preservation, and spent-handle enforcement.
example/lib/main.dart Adds a UI action to call dispose() and logs the result + expected post-dispose behavior.
CHANGELOG.md Documents the new API, behavioral contracts, and platform fixes.
pubspec.yaml Bumps version to 6.0.0-dev.5.
example/pubspec.lock Updates the example’s locked version to 6.0.0-dev.5.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@hpoul
hpoul force-pushed the standalone-dispose branch from 870ed3e to 0db555b Compare August 25, 2026 19:35
Two findings from review, a failure CI caught that the review did not,
and a footgun found while checking the fix.

A disposed BiometricStorageFile documented that it must not be used
again, and nothing enforced it — so the consequence of ignoring the doc
differed by platform. Android and darwin resolve read, write and delete
against their registry and fail once the entry is gone, while Linux,
Windows and web answer from the name alone and carry on working. That is
a mistake you make on one platform and discover on another. The guard now
sits in Dart, so every backend answers alike: read, write and delete
throw a StateError, and dispose deliberately does not, because disposing
twice has to stay legal.

The flag is set before the call is awaited, not after. That brick a
handle whose native dispose then fails, which is worth it: Dart always
sends `name`, so the argument-error paths are dead, while the race it
closes is reachable by any caller who does not await dispose() and would
otherwise slip a read into the window — succeeding on three platforms and
failing on two, which is the divergence this guard exists to remove.
Recovering costs one getStorage.

dispose is scoped to the name, not to the handle: every handle is a view
onto one name-keyed initialization, so a spent handle still forgets an
initialization made after it. Documented and tested, because the
tempting optimization — short-circuiting a repeat dispose on the local
flag rather than asking the platform — would quietly make it
handle-scoped and return a wrong answer.

The `null reply is reported as nothing-to-forget` test was vacuous and is
gone. No deployable combination replies null — the handlers that predate
this either answer true, or throw, or are missing entirely, which
surfaces as MissingPluginException — and the `?? false` it claimed to
guard is enforced by the compiler anyway, since invokeMethod<bool> is
typed Future<bool?>. The coercion stays, with a comment saying what it is
actually for.

The remaining dispose unit tests are skipped on a Windows host. They
drive MethodChannelBiometricStorage, whose dispose asks
_promptInfoForCurrentPlatform for a prompt it has no windows case for, so
they failed there with "Unsupported Platform windows". Not reachable by
users — the registrant installs Win32BiometricStoragePlugin — and
biometric_storage_win32_test.dart covers dispose on that host.

Adding the guard invalidated two things written alongside it: a win32
teardown that deleted through the handle it had just disposed, and the
example's claim that read and write keep working afterwards. Both fixed.

Verified by execution on all five backends. Eleven integration tests
green on the Android emulator, the iOS simulator and macOS; Linux runs
the same suite against real libsecret in CI, and Windows runs the win32
unit suite, which reaches the same dispose paths by a different route.
Reintroducing the Android NoSuchStorage throw turned two red — the
idempotence test and the guard test's closing assertion — and reverting
restored them; the darwin no-op did the same earlier. Both mutation runs
predate the last two tests, so they show those fixes are caught, not that
every test in the suite is.

macOS had never run this suite before, having been signing-blocked; a
provisioning profile now exists. It writes to the real login keychain, so
the run was bracketed by a keychain baseline — zero integration_ entries
before and after, which is what says teardown cleans up rather than that
the tests passed. No prompt appeared, which is what authenticationRequired
false is for.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@hpoul
hpoul force-pushed the standalone-dispose branch from 0db555b to 1cdd305 Compare August 25, 2026 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants