From eaaa3f2edef415d02309035f72b7c29f73466c6a Mon Sep 17 00:00:00 2001 From: randogoth Date: Sun, 27 Sep 2026 00:09:38 +0300 Subject: [PATCH] fix: onboarding restore no longer traps on an abandoned identity or a missing pin --- .../screens/onboarding_screen.dart | 120 ++++++++++++++---- test/dbg_test.dart | 61 --------- test/recall_flow_test.dart | 74 ++++++++++- 3 files changed, 164 insertions(+), 91 deletions(-) delete mode 100644 test/dbg_test.dart diff --git a/lib/presentation/screens/onboarding_screen.dart b/lib/presentation/screens/onboarding_screen.dart index b6b0b3c..5e9498d 100644 --- a/lib/presentation/screens/onboarding_screen.dart +++ b/lib/presentation/screens/onboarding_screen.dart @@ -31,6 +31,10 @@ class _OnboardingScreenState extends ConsumerState { _Step step = _Step.welcome; Uint8List? createdSeed; bool busy = false; + // The register step doubles as "pin a key to finish recalling" when a + // restore's recall can't proceed without one yet — same fields, different + // framing and default action, not the generic "register a new address" copy. + bool recallIntent = false; final seedController = TextEditingController(); final restoreAddressController = TextEditingController(); @@ -57,13 +61,33 @@ class _OnboardingScreenState extends ConsumerState { context, error is SmolError ? error.message : error.toString()); } - void _createIdentity() { + // Reaching onboarding at all means HomeGuard already found no complete + // identity+account, so a seed still sitting in the store here can only be + // an abandoned attempt from earlier in this same flow (wrong seed, failed + // recall, "Back") — safe to replace rather than reject. + // wipe() is fire-and-forget here, like every other store write in this + // screen (setIdentity, pinServer, ...) — Hive updates its in-memory state + // synchronously and persists to disk in the background, so the identity + // check right after is already consistent without awaiting the write. + void _clearAbandonedIdentity() { final client = ref.read(clientProvider); - final fresh = client.createIdentity(); - setState(() { - createdSeed = fresh.seed; - step = _Step.backup; - }); + if (client.identity != null && client.accountAddress() == null) { + ref.read(storeProvider).wipe(); + } + } + + void _createIdentity() { + _clearAbandonedIdentity(); + final client = ref.read(clientProvider); + try { + final fresh = client.createIdentity(); + setState(() { + createdSeed = fresh.seed; + step = _Step.backup; + }); + } on Exception catch (err) { + _showError(err); + } } void _restoreIdentity() { @@ -74,6 +98,7 @@ class _OnboardingScreenState extends ConsumerState { // pinned yet, offline, typo) — recall can always be retried from the // register step or settings afterward. void _submitRestore() { + _clearAbandonedIdentity(); final client = ref.read(clientProvider); try { client.restoreIdentity(seedController.text); @@ -86,6 +111,34 @@ class _OnboardingScreenState extends ConsumerState { setState(() => step = _Step.register); return; } + // The register step has its own address field (it also needs a server + // key, which restore doesn't collect) — carry over what was already + // typed rather than making the user re-enter it. + addressController.text = addressText; + final SmolAddress addr; + try { + addr = parseAddress(addressText); + } on Exception catch (err) { + // A genuine typo, distinct from the merely-unpinned case below — still + // worth an error, but land on the same recall-oriented step to fix it. + _showError(err); + setState(() { + recallIntent = true; + step = _Step.register; + }); + return; + } + // Recall needs the host pinned first (SPEC.md §4). That's the expected, + // common state right after a restore — not an error — so check for it + // up front instead of letting recallAccount fail and surfacing that as + // one: this address just needs a key before its first recall can proceed. + if (ref.read(storeProvider).serverPin(addr.host) == null) { + setState(() { + recallIntent = true; + step = _Step.register; + }); + return; + } () async { try { await client.recallAccount(addressText); @@ -95,7 +148,10 @@ class _OnboardingScreenState extends ConsumerState { } on Exception catch (err) { if (!mounted) return; _showError(err); - setState(() => step = _Step.register); + setState(() { + recallIntent = true; + step = _Step.register; + }); } }(); } @@ -142,7 +198,7 @@ class _OnboardingScreenState extends ConsumerState { @override Widget build(BuildContext context) { return Scaffold( - body: Padding( + body: SingleChildScrollView( padding: const EdgeInsets.symmetric(horizontal: 25), child: switch (step) { _Step.welcome => _welcome(context), @@ -176,7 +232,7 @@ class _OnboardingScreenState extends ConsumerState { children: [ _header(context, "One keypair is the whole identity: an address, a key and a message."), - const Spacer(), + const SizedBox(height: 40), PrimaryButton( onPressed: _createIdentity, child: const Text("Create Identity"), @@ -222,7 +278,7 @@ class _OnboardingScreenState extends ConsumerState { }, child: const Text("Copy seed"), ), - const Spacer(), + const SizedBox(height: 40), PrimaryButton( onPressed: () => setState(() => step = _Step.register), child: const Text("I have backed it up"), @@ -266,7 +322,7 @@ class _OnboardingScreenState extends ConsumerState { text: "Back", onPressed: () => setState(() => step = _Step.welcome), ), - const Spacer(), + const SizedBox(height: 40), ], ); } @@ -275,8 +331,12 @@ class _OnboardingScreenState extends ConsumerState { return Column( crossAxisAlignment: CrossAxisAlignment.start, children: [ - _header(context, - "Register an address. Pin the server's public key first — obtain it from the operator through a trusted channel."), + _header( + context, + recallIntent + ? "Pin this server's public key to finish restoring your address." + : "Register an address. Pin the server's public key first — obtain it from the operator through a trusted channel.", + ), const SizedBox(height: 30), TextField( controller: addressController, @@ -297,25 +357,33 @@ class _OnboardingScreenState extends ConsumerState { fillColor: Theme.of(context).extension()!.cardFill, ), ), - const SizedBox(height: 15), - TextField( - controller: tokenController, - decoration: InputDecoration( - labelText: "Invite token (optional)", - filled: true, - fillColor: Theme.of(context).extension()!.cardFill, + if (!recallIntent) ...[ + const SizedBox(height: 15), + TextField( + controller: tokenController, + decoration: InputDecoration( + labelText: "Invite token (optional)", + filled: true, + fillColor: Theme.of(context).extension()!.cardFill, + ), ), - ), - const Spacer(), + ], + const SizedBox(height: 40), PrimaryButton( - onPressed: busy ? () {} : _submitRegistration, + onPressed: busy + ? () {} + : (recallIntent ? _submitRecall : _submitRegistration), child: busy ? const SmallLoadingSpinner() - : const Text("Pin and Register"), + : Text(recallIntent ? "Pin and Recall" : "Pin and Register"), ), TextButton( - onPressed: busy ? () {} : _submitRecall, - child: const Text("Already registered? Recall"), + onPressed: busy + ? () {} + : (recallIntent ? _submitRegistration : _submitRecall), + child: Text(recallIntent + ? "Register a new address instead" + : "Already registered? Recall"), ), const SizedBox(height: 80), ], diff --git a/test/dbg_test.dart b/test/dbg_test.dart deleted file mode 100644 index 9df2990..0000000 --- a/test/dbg_test.dart +++ /dev/null @@ -1,61 +0,0 @@ -import "dart:io"; - -import "package:auto_route/auto_route.dart"; -import "package:flutter/material.dart"; -import "package:flutter_riverpod/flutter_riverpod.dart"; -import "package:flutter_test/flutter_test.dart"; -import "package:hive_flutter/hive_flutter.dart"; - -import "package:smol_mail/data/providers/providers.dart"; -import "package:smol_mail/presentation/app_widget.dart"; -import "package:smol_mail/presentation/routes/app_router.gr.dart"; -import "package:smol_mail/smol/client.dart"; -import "package:smol_mail/smol/crypto.dart"; -import "package:smol_mail/smol/proto.dart"; -import "package:smol_mail/smol/store.dart"; - -class StubClient extends SmolClient { - StubClient(super.store); - - @override - Future recallAccount(String addressText) async { - final addr = parseAddress(addressText); - store.setAccount(addr); - return addr; - } -} - -void main() { - late SmolStore store; - setUpAll(() async { - TestWidgetsFlutterBinding.ensureInitialized(); - final dir = await Directory.systemTemp.createTemp("dbg4"); - Hive.init(dir.path); - store = await SmolStore.open(stateBox: "dbg-state", mailBox: "dbg-mail"); - }); - - testWidgets("direct replace", (tester) async { - final seed = randomBytes(32); - await tester.pumpWidget(ProviderScope( - overrides: [ - storeProvider.overrideWithValue(store), - clientProvider.overrideWithValue(StubClient(store)), - ], - child: const AppWidget(), - )); - await tester.pumpAndSettle(); - await tester.tap(find.text("Restore From Seed")); - await tester.pumpAndSettle(); - final fields = find.byType(TextField); - await tester.enterText(fields.at(0), hex(seed)); - await tester.enterText(fields.at(1), "randogoth@smol.place"); - await tester.tap(find.text("Restore")); - await tester.pumpAndSettle(); - expect(store.account(), isNotNull, reason: "recall stub ran"); - final element = tester.element(find.text("Back")); - final router = AutoRouter.of(element); - await router.replace(InboxRoute()); - await tester.pumpAndSettle(); - expect(find.text("Inbox"), findsOneWidget); - }); -} diff --git a/test/recall_flow_test.dart b/test/recall_flow_test.dart index 144d861..7f48832 100644 --- a/test/recall_flow_test.dart +++ b/test/recall_flow_test.dart @@ -33,7 +33,7 @@ class StubClient extends SmolClient { void main() { // Hive box opening is real file IO and must happen outside testWidgets' // fake-async zone — including the per-test stores. - late final SmolStore storeA, storeB; + late final SmolStore storeA, storeB, storeC; setUpAll(() async { TestWidgetsFlutterBinding.ensureInitialized(); final dir = await Directory.systemTemp.createTemp("smol-recall-flow"); @@ -42,6 +42,8 @@ void main() { stateBox: "recall-a-state", mailBox: "recall-a-mail"); storeB = await SmolStore.open( stateBox: "recall-b-state", mailBox: "recall-b-mail"); + storeC = await SmolStore.open( + stateBox: "recall-c-state", mailBox: "recall-c-mail"); }); Widget app(SmolStore store) => ProviderScope( @@ -52,8 +54,9 @@ void main() { child: const AppWidget(), ); - testWidgets("restore with an address recalls and lands in the inbox", - (tester) async { + testWidgets( + "restore with an address, but no pin yet, asks for the server key " + "and then recalls into the inbox", (tester) async { final store = storeA; final seed = randomBytes(32); await tester.pumpWidget(app(store)); @@ -63,13 +66,26 @@ void main() { await tester.tap(find.text("Restore From Seed")); await tester.pumpAndSettle(); - final fields = find.byType(TextField); + var fields = find.byType(TextField); expect(fields, findsNWidgets(2)); await tester.enterText(fields.at(0), hex(seed)); await tester.enterText(fields.at(1), "randogoth@smol.place"); await tester.tap(find.text("Restore")); await tester.pumpAndSettle(); + // Recall needs the server pinned first (SPEC.md §4) — that's the normal + // state right after a restore, so this lands on the register step framed + // for recall (no error, no Invite token field) rather than the inbox yet. + expect(find.text("Inbox"), findsNothing); + expect(find.text("Pin this server's public key to finish restoring your address."), + findsOneWidget); + expect(find.text("Invite token (optional)"), findsNothing); + fields = find.byType(TextField); + expect(fields, findsNWidgets(2)); // address (carried over), server key + await tester.enterText(fields.at(1), b32encode(randomBytes(32))); + await tester.tap(find.text("Pin and Recall")); + await tester.pumpAndSettle(); + expect(find.text("Inbox"), findsOneWidget); expect(store.seed(), seed); expect(store.account()!.user, "randogoth"); @@ -101,4 +117,54 @@ void main() { expect(find.text("Inbox"), findsOneWidget); expect(store.account()!.user, "randogoth"); }); + + testWidgets( + "restoring again after an incomplete attempt replaces the identity " + "instead of refusing it", (tester) async { + final store = storeC; + final abandonedSeed = randomBytes(32); + final realSeed = randomBytes(32); + await tester.pumpWidget(app(store)); + await tester.pumpAndSettle(); + + // First attempt: restore a seed but never finish registering — lands on + // the register step, identity set, no account bound. + await tester.tap(find.text("Restore From Seed")); + await tester.pumpAndSettle(); + var fields = find.byType(TextField); + await tester.enterText(fields.at(0), hex(abandonedSeed)); + await tester.tap(find.text("Restore")); + await tester.pumpAndSettle(); + expect(store.seed(), abandonedSeed); + expect(store.account(), isNull); + + // Simulate returning to onboarding later (e.g. a cold restart). Pumping + // app(store) directly would just rebuild the existing OnboardingScreen + // state in place (still parked on the register step) rather than really + // restarting, so tear the tree down first to force a fresh app state — + // HomeGuard then sends an identity-without-account back to welcome. + await tester.pumpWidget(const SizedBox()); + await tester.pumpWidget(app(store)); + await tester.pumpAndSettle(); + expect(find.text("Create Identity"), findsOneWidget); + + // Restoring a different seed must not throw "identity already exists". + await tester.tap(find.text("Restore From Seed")); + await tester.pumpAndSettle(); + fields = find.byType(TextField); + await tester.enterText(fields.at(0), hex(realSeed)); + await tester.enterText(fields.at(1), "randogoth@smol.place"); + await tester.tap(find.text("Restore")); + await tester.pumpAndSettle(); + + // Lands on the recall-framed register step (no pin yet); pin it and finish. + fields = find.byType(TextField); + expect(fields, findsNWidgets(2)); + await tester.enterText(fields.at(1), b32encode(randomBytes(32))); + await tester.tap(find.text("Pin and Recall")); + await tester.pumpAndSettle(); + + expect(find.text("Inbox"), findsOneWidget); + expect(store.seed(), realSeed); + }); }