From b303e25c2507ba328c273e554ded0d1747c55e72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?An=C4=B1lcan=20=C3=87ak=C4=B1r?= Date: Tue, 8 Sep 2026 23:23:00 +0300 Subject: [PATCH 01/12] feat(auth): return a user to the deep link they asked for after login Wave 1 of the deep-link plan. EnsureAuthenticated bounced an unauthenticated visitor to login and recorded nothing, so a cold start at a deep link lost its destination. It now records the location through MagicRouter.setIntendedUrl, which existed with zero callers, and a new NavigatesRoutes.navigateHome reads it back once the user authenticates. Nothing is recorded for the login route or any other guest-only auth route, since a visitor bounced off one of those cannot use it as a destination either. All five post-auth navigations now go through navigateHome, which makes it the single seam. Known limit, recorded in the docblock: redirectTarget only ever sees state.matchedLocation, which carries no query string. --- CHANGELOG.md | 3 + .../concerns/navigates_routes.dart | 30 ++++++ .../magic_starter_auth_controller.dart | 6 +- .../magic_starter_guest_auth_controller.dart | 3 +- .../magic_starter_otp_controller.dart | 2 +- lib/src/middleware/ensure_authenticated.dart | 43 +++++++- .../concerns/navigates_routes_test.dart | 101 ++++++++++++++++++ .../middleware/ensure_authenticated_test.dart | 36 +++++++ 8 files changed, 215 insertions(+), 9 deletions(-) create mode 100644 test/http/controllers/concerns/navigates_routes_test.dart diff --git a/CHANGELOG.md b/CHANGELOG.md index c830e65..a38f6de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +### Added +- **A deep link that lands on a signed-out device now survives the login bounce.** `EnsureAuthenticated.redirectTarget` records the requested location via `MagicRouter.setIntendedUrl` before bouncing an unauthenticated visitor to login, and a new `NavigatesRoutes.navigateHome` reads it back with `pullIntendedUrl` once they authenticate, falling back to `MagicStarterConfig.homeRoute()` when no intent was stored or the stored value is not an in-app path. Nothing is recorded for the login route itself or for any other guest-only auth route (register, forgot-password, reset-password, two-factor-challenge, otp), since a bounced visitor cannot use one of those as a destination either. All five post-auth navigations (login, register auto-login, two-factor challenge, OTP verification, guest login) now call `navigateHome()` instead of navigating straight to the home route. Known limit: `redirectTarget` only ever sees `state.matchedLocation`, which carries no query string, so a recorded intent loses any `?token=...` the original link carried. + ## [0.0.1-alpha.26] - 2026-09-03 ### Changed diff --git a/lib/src/http/controllers/concerns/navigates_routes.dart b/lib/src/http/controllers/concerns/navigates_routes.dart index c965eb9..46e24d2 100644 --- a/lib/src/http/controllers/concerns/navigates_routes.dart +++ b/lib/src/http/controllers/concerns/navigates_routes.dart @@ -1,5 +1,7 @@ import 'package:magic/magic.dart'; +import '../../../configuration/magic_starter_config.dart'; + /// Shared navigation helper for Magic Starter controllers. /// /// Provides a safe [navigateTo] method that checks for navigator context @@ -17,4 +19,32 @@ mixin NavigatesRoutes { MagicRoute.to(path, query: query); } + + /// Navigate to wherever a signed-in user should land after authenticating. + /// + /// This is the ONLY post-auth navigation seam: every controller action + /// that follows a successful login, registration, two-factor challenge, + /// OTP verification or guest login must call this instead of navigating + /// to [MagicStarterConfig.homeRoute] directly, or the deep-link-through- + /// login flow below silently keeps sending everyone home. + /// + /// Pulls the URL `EnsureAuthenticated` recorded before bouncing the user + /// to login (see [MagicRouter.pullIntendedUrl], a one-time read) and + /// targets it when it is a well-formed in-app path (non-null, leading + /// slash). Any other value, including no stored intent at all, falls + /// back to [MagicStarterConfig.homeRoute]. The leading-slash check is + /// defence in depth: `setIntendedUrl` is only ever called with a router + /// location internally, so this guards against a value that should not + /// be reachable rather than one that is. + void navigateHome() { + if (MagicRouter.instance.navigatorKey.currentContext == null) return; + + final String? intended = MagicRouter.instance.pullIntendedUrl(); + final bool hasValidIntent = intended != null && intended.startsWith('/'); + final String target = hasValidIntent + ? intended + : MagicStarterConfig.homeRoute(); + + navigateTo(target); + } } diff --git a/lib/src/http/controllers/magic_starter_auth_controller.dart b/lib/src/http/controllers/magic_starter_auth_controller.dart index 79dd290..ce90b52 100644 --- a/lib/src/http/controllers/magic_starter_auth_controller.dart +++ b/lib/src/http/controllers/magic_starter_auth_controller.dart @@ -118,7 +118,7 @@ class MagicStarterAuthController extends MagicController // 3. Authenticate the user and navigate home. await Auth.login({'token': token}, MagicStarter.createUser(userData)); setSuccess(true); - navigateTo(MagicStarterConfig.homeRoute()); + navigateHome(); } on TimeoutException catch (e, stackTrace) { Log.error( '[MagicStarterAuthController.doLogin] Timeout: $e\n$stackTrace', @@ -186,7 +186,7 @@ class MagicStarterAuthController extends MagicController // 3. Auto-login when the server returns credentials immediately. await Auth.login({'token': token}, MagicStarter.createUser(userData)); setSuccess(true); - navigateTo(MagicStarterConfig.homeRoute()); + navigateHome(); return; } @@ -321,7 +321,7 @@ class MagicStarterAuthController extends MagicController // 2. Log the user in and navigate to home. await Auth.login({'token': token}, MagicStarter.createUser(userData)); setSuccess(true); - navigateTo(MagicStarterConfig.homeRoute()); + navigateHome(); } catch (e, stackTrace) { Log.error( '[MagicStarterAuthController.doTwoFactorChallenge] $e\n$stackTrace', diff --git a/lib/src/http/controllers/magic_starter_guest_auth_controller.dart b/lib/src/http/controllers/magic_starter_guest_auth_controller.dart index 39163ff..aa93c54 100644 --- a/lib/src/http/controllers/magic_starter_guest_auth_controller.dart +++ b/lib/src/http/controllers/magic_starter_guest_auth_controller.dart @@ -3,7 +3,6 @@ import 'dart:math'; import 'package:magic/magic.dart'; import 'concerns/navigates_routes.dart'; -import '../../configuration/magic_starter_config.dart'; import '../../facades/magic_starter.dart'; import '../../models/magic_starter_auth_user.dart'; @@ -85,7 +84,7 @@ class MagicStarterGuestAuthController extends MagicController setSuccess(true); // 5. Navigate home. - navigateTo(MagicStarterConfig.homeRoute()); + navigateHome(); } catch (e, stackTrace) { Log.error( '[MagicStarterGuestAuthController.doGuestLogin] $e\n$stackTrace', diff --git a/lib/src/http/controllers/magic_starter_otp_controller.dart b/lib/src/http/controllers/magic_starter_otp_controller.dart index 64a7deb..edddc4a 100644 --- a/lib/src/http/controllers/magic_starter_otp_controller.dart +++ b/lib/src/http/controllers/magic_starter_otp_controller.dart @@ -135,7 +135,7 @@ class MagicStarterOtpController extends MagicController // 3. Navigate home on successful authentication. setSuccess(data); - navigateTo(MagicStarterConfig.homeRoute()); + navigateHome(); } catch (e, stackTrace) { Log.error('[MagicStarterOtpController.verifyOtp] $e\n$stackTrace'); setError(trans('errors.unexpected')); diff --git a/lib/src/middleware/ensure_authenticated.dart b/lib/src/middleware/ensure_authenticated.dart index b9412ee..931507d 100644 --- a/lib/src/middleware/ensure_authenticated.dart +++ b/lib/src/middleware/ensure_authenticated.dart @@ -11,6 +11,18 @@ import '../configuration/magic_starter_config.dart'; /// `redirect` callback before any page builds, so an unauthenticated boot /// lands on the login route and the gated page never mounts. /// +/// Before bouncing to login it records [location] via +/// [MagicRouter.setIntendedUrl], so `NavigatesRoutes.navigateHome` can send +/// the user back there once they authenticate. Nothing is recorded for the +/// login route itself (nothing to return to) or for any other guest-only +/// route registered in `auth_routes.dart` (register, forgot-password, +/// reset-password, two-factor-challenge, otp): a visitor bounced off one of +/// those cannot use it as a post-login destination either. +/// +/// **Known limit**: [redirectTarget] receives `state.matchedLocation` (see +/// `magic_router.dart`'s `_handleRedirect`), which carries no query string, +/// so a recorded intent loses any `?token=...` the original link carried. +/// /// ```dart /// MagicRoute.group( /// middleware: ['auth'], @@ -18,14 +30,39 @@ import '../configuration/magic_starter_config.dart'; /// ); /// ``` class EnsureAuthenticated extends MagicMiddleware { + /// Path suffixes (under the configured auth prefix) that are themselves + /// guest-only screens: see `auth_routes.dart` for the route registrations + /// this mirrors. + static const List _guestRouteSuffixes = [ + '/register', + '/forgot-password', + '/reset-password', + '/two-factor-challenge', + '/otp', + ]; + @override String? redirectTarget(String location) { + if (Auth.check()) return null; + // Guard the login route itself so the redirect can never loop: go_router // raises after more than five successive redirects. final String login = MagicStarterConfig.loginRoute(); - if (!Auth.check() && location != login) { - return login; + if (location == login) return null; + + if (!_isGuestRoute(location)) { + MagicRouter.instance.setIntendedUrl(location); } - return null; + + return login; + } + + /// Whether [location] is one of the guest-only auth routes registered in + /// `auth_routes.dart` (the login route itself is handled by the caller). + bool _isGuestRoute(String location) { + final String authPrefix = MagicStarterConfig.authPrefix(); + return _guestRouteSuffixes.any( + (suffix) => location == '$authPrefix$suffix', + ); } } diff --git a/test/http/controllers/concerns/navigates_routes_test.dart b/test/http/controllers/concerns/navigates_routes_test.dart new file mode 100644 index 0000000..9466e36 --- /dev/null +++ b/test/http/controllers/concerns/navigates_routes_test.dart @@ -0,0 +1,101 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:magic/magic.dart'; +import 'package:magic_starter/src/configuration/magic_starter_config.dart'; +import 'package:magic_starter/src/http/controllers/concerns/navigates_routes.dart'; + +/// Bare host exercising [NavigatesRoutes] in isolation, without pulling in a +/// full controller's HTTP/auth surface. +class _TestHost with NavigatesRoutes {} + +void main() { + setUpAll(() { + TestWidgetsFlutterBinding.ensureInitialized(); + }); + + setUp(() { + MagicApp.reset(); + Magic.flush(); + TitleManager.reset(); + MagicRouter.reset(); + Auth.fake(); + }); + + tearDown(() { + Auth.unfake(); + }); + + group('NavigatesRoutes.navigateHome', () { + testWidgets('targets the stored intended URL when one is set', ( + tester, + ) async { + final host = _TestHost(); + MagicRoute.page('/', () => const Text('home')); + MagicRoute.page('/incidents/123', () => const Text('incident')); + + await tester.pumpWidget( + MaterialApp.router(routerConfig: MagicRouter.instance.routerConfig), + ); + await tester.pumpAndSettle(); + + MagicRouter.instance.setIntendedUrl('/incidents/123'); + host.navigateHome(); + await tester.pumpAndSettle(); + + expect(MagicRouter.instance.currentPath, '/incidents/123'); + }); + + testWidgets( + 'falls back to the configured home route when no intent is stored', + (tester) async { + final host = _TestHost(); + MagicRoute.page('/', () => const Text('home')); + MagicRoute.page('/monitors', () => const Text('monitors')); + + MagicRouter.instance.setInitialLocation('/monitors'); + + await tester.pumpWidget( + MaterialApp.router(routerConfig: MagicRouter.instance.routerConfig), + ); + await tester.pumpAndSettle(); + + host.navigateHome(); + await tester.pumpAndSettle(); + + expect( + MagicRouter.instance.currentPath, + MagicStarterConfig.homeRoute(), + ); + }, + ); + + testWidgets( + 'falls back to the configured home route when the stored intent is ' + 'poisoned', + (tester) async { + final host = _TestHost(); + MagicRoute.page('/', () => const Text('home')); + MagicRoute.page('/monitors', () => const Text('monitors')); + + MagicRouter.instance.setInitialLocation('/monitors'); + + await tester.pumpWidget( + MaterialApp.router(routerConfig: MagicRouter.instance.routerConfig), + ); + await tester.pumpAndSettle(); + + // Set directly rather than via a deep link parse, to prove + // navigateHome itself rejects a non-leading-slash value regardless + // of how it got stored. + MagicRouter.instance.setIntendedUrl('https://evil.example/x'); + host.navigateHome(); + await tester.pumpAndSettle(); + + expect( + MagicRouter.instance.currentPath, + MagicStarterConfig.homeRoute(), + ); + }, + ); + }); +} diff --git a/test/middleware/ensure_authenticated_test.dart b/test/middleware/ensure_authenticated_test.dart index b54fc7f..18a3bd2 100644 --- a/test/middleware/ensure_authenticated_test.dart +++ b/test/middleware/ensure_authenticated_test.dart @@ -26,6 +26,7 @@ void main() { setUp(() { MagicApp.reset(); Magic.flush(); + MagicRouter.reset(); }); tearDown(() { @@ -57,5 +58,40 @@ void main() { expect(middleware.redirectTarget('/'), isNull); expect(middleware.redirectTarget('/monitors'), isNull); }); + + test('records the requested location as the intended URL before bouncing ' + 'to login', () { + Auth.fake(); + final middleware = EnsureAuthenticated(); + + middleware.redirectTarget('/incidents/123'); + + expect(MagicRouter.instance.hasIntendedUrl, isTrue); + expect(MagicRouter.instance.pullIntendedUrl(), '/incidents/123'); + }); + + test( + 'records nothing when the bounce target is itself a guest-only auth ' + 'route, so a bounced visitor is never sent back to one they cannot use', + () { + Auth.fake(); + final middleware = EnsureAuthenticated(); + + middleware.redirectTarget('/auth/register'); + expect(MagicRouter.instance.hasIntendedUrl, isFalse); + + middleware.redirectTarget('/auth/forgot-password'); + expect(MagicRouter.instance.hasIntendedUrl, isFalse); + }, + ); + + test('records nothing for an authenticated navigation', () { + Auth.fake(user: _fakeUser()); + final middleware = EnsureAuthenticated(); + + middleware.redirectTarget('/monitors'); + + expect(MagicRouter.instance.hasIntendedUrl, isFalse); + }); }); } From 19ac5544003a433f48a8e6c5af19fce15998c74f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?An=C4=B1lcan=20=C3=87ak=C4=B1r?= Date: Wed, 9 Sep 2026 00:44:14 +0300 Subject: [PATCH 02/12] chore(release): 0.0.1-alpha.27 Carries the intended-URL replay: a deep link that lands on a signed-out device returns to its destination after login. --- CHANGELOG.md | 2 +- pubspec.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a38f6de..96b8830 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ All notable changes to this project will be documented in this file. -## [Unreleased] +## [0.0.1-alpha.27] - 2026-09-09 ### Added - **A deep link that lands on a signed-out device now survives the login bounce.** `EnsureAuthenticated.redirectTarget` records the requested location via `MagicRouter.setIntendedUrl` before bouncing an unauthenticated visitor to login, and a new `NavigatesRoutes.navigateHome` reads it back with `pullIntendedUrl` once they authenticate, falling back to `MagicStarterConfig.homeRoute()` when no intent was stored or the stored value is not an in-app path. Nothing is recorded for the login route itself or for any other guest-only auth route (register, forgot-password, reset-password, two-factor-challenge, otp), since a bounced visitor cannot use one of those as a destination either. All five post-auth navigations (login, register auto-login, two-factor challenge, OTP verification, guest login) now call `navigateHome()` instead of navigating straight to the home route. Known limit: `redirectTarget` only ever sees `state.matchedLocation`, which carries no query string, so a recorded intent loses any `?token=...` the original link carried. diff --git a/pubspec.yaml b/pubspec.yaml index 803fe92..9270ffa 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,6 +1,6 @@ name: magic_starter description: Starter kit for Magic Framework. Auth, Profile, Teams, Notifications — 14 opt-in features with overridable views. -version: 0.0.1-alpha.26 +version: 0.0.1-alpha.27 homepage: https://magic.fluttersdk.com/starter documentation: https://magic.fluttersdk.com/packages/starter/getting-started/installation repository: https://github.com/fluttersdk/magic_starter From f03e2bf34e0b04cec7afa3636c33bd6936713921 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?An=C4=B1lcan=20=C3=87ak=C4=B1r?= Date: Wed, 9 Sep 2026 00:48:31 +0300 Subject: [PATCH 03/12] chore(release): carry alpha.27 into the version banner constant The two starter:* banners read magicStarterVersion, and its own test compares it against pubspec.yaml for exactly this reason: a literal nothing compares against drifts silently, and this one was already twenty-four releases stale once before. --- lib/src/cli/starter_artisan_provider.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/cli/starter_artisan_provider.dart b/lib/src/cli/starter_artisan_provider.dart index 5a0d89e..e7b487d 100644 --- a/lib/src/cli/starter_artisan_provider.dart +++ b/lib/src/cli/starter_artisan_provider.dart @@ -13,7 +13,7 @@ import 'commands/magic_starter_uninstall_command.dart'; /// both and fails when they disagree, so a banner cannot drift behind a release /// the way the two hand-written `'0.0.1'` literals did: they were written before /// the first alpha and were still claiming 0.0.1 twenty-four releases later. -const String magicStarterVersion = '0.0.1-alpha.26'; +const String magicStarterVersion = '0.0.1-alpha.27'; /// Magic Starter's contribution to the host application's artisan registry. /// From 9a24892793fe60505d059b698b0b3acda4adf040 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?An=C4=B1lcan=20=C3=87ak=C4=B1r?= Date: Wed, 9 Sep 2026 01:24:18 +0300 Subject: [PATCH 04/12] fix(auth): refuse a protocol-relative intended URL //host/path starts with a slash and is a different origin, so the leading-slash test alone admitted it. Nothing reachable can store one today, since only a router location is ever recorded, which is why the extra condition is cheap enough to keep. --- lib/src/http/controllers/concerns/navigates_routes.dart | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/src/http/controllers/concerns/navigates_routes.dart b/lib/src/http/controllers/concerns/navigates_routes.dart index 46e24d2..5347b8f 100644 --- a/lib/src/http/controllers/concerns/navigates_routes.dart +++ b/lib/src/http/controllers/concerns/navigates_routes.dart @@ -40,7 +40,14 @@ mixin NavigatesRoutes { if (MagicRouter.instance.navigatorKey.currentContext == null) return; final String? intended = MagicRouter.instance.pullIntendedUrl(); - final bool hasValidIntent = intended != null && intended.startsWith('/'); + // `//host/path` is protocol-relative: it starts with a slash and is a + // different ORIGIN, so the leading-slash test alone lets it through. + // Nothing reachable can store one today, since only a router location is + // ever recorded, which is exactly why the check is cheap to keep. + final bool hasValidIntent = + intended != null && + intended.startsWith('/') && + !intended.startsWith('//'); final String target = hasValidIntent ? intended : MagicStarterConfig.homeRoute(); From 3e2dc0cb592a1332e75578376bad33df5ba484c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?An=C4=B1lcan=20=C3=87ak=C4=B1r?= Date: Wed, 9 Sep 2026 01:57:31 +0300 Subject: [PATCH 05/12] chore(release): gate the agent-facing reference against this package's version The reference an agent adopting this package reads lives in the magic repo, not here: .pubignore keeps CLAUDE.md and .claude/ out of the published archive, so pub.dev ships doc/ and README.md and nothing else an agent is pointed at. That reference is versioned by magic's releases rather than by this package's, and it drifted far enough to still document the notification UI that had moved out of this package entirely. A test compares its first-line stamp against this pubspec's version. It cannot run in CI, which clones no siblings, so it skips there instead of failing; releases are cut locally and that is where it fires. release.md carries the same requirement in prose for anyone running the suite elsewhere. --- .claude/commands/release.md | 1 + test/skill_reference_stamp_test.dart | 65 ++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+) create mode 100644 test/skill_reference_stamp_test.dart diff --git a/.claude/commands/release.md b/.claude/commands/release.md index 2c6c3d0..63f9920 100644 --- a/.claude/commands/release.md +++ b/.claude/commands/release.md @@ -28,6 +28,7 @@ You are preparing a new release for the **Magic Starter** Flutter plugin. Follow 3. **Tests** — All tests must pass (see context above). If failing, STOP and report. 4. **Analyzer** — Zero issues required (see context above). If issues, STOP and report. 5. **Version** — Determine the new version from $ARGUMENTS or auto-increment. +6. **Agent-facing reference** — Update `../magic/skills/magic-framework/references/plugin-starter.md` against this release and move its first-line stamp to the new version. That file, not this repo's `CLAUDE.md`, is what an agent adopting this package reads: `.pubignore` keeps `CLAUDE.md` and `.claude/` out of the published archive. It lives in another repository, so nothing here forces it; `test/skill_reference_stamp_test.dart` catches a stale stamp when a sibling checkout exists and skips when it does not, which is why this line is here too. ### Phase 2: Version Bump diff --git a/test/skill_reference_stamp_test.dart b/test/skill_reference_stamp_test.dart new file mode 100644 index 0000000..73e6bca --- /dev/null +++ b/test/skill_reference_stamp_test.dart @@ -0,0 +1,65 @@ +// The agent-facing reference for this package lives in ANOTHER repository. +// +// `.pubignore` excludes `CLAUDE.md` and `.claude/`, so an agent adopting this +// package from pub.dev never sees them. What it lands on is +// `magic/skills/magic-framework/references/plugin-starter.md`, which is +// versioned by `magic`'s releases rather than by this package's, and therefore +// drifts silently. Measured once: the reference was stamped alpha.23 against a +// shipped alpha.27 and still documented the notification UI that had moved out +// of this package entirely. +// +// This test is the cheapest gate that exists for a cross-repository document. +// It cannot run in CI, which clones no siblings, so it SKIPS there rather than +// failing. Releases are cut locally, which is where it fires, and +// `.claude/commands/release.md` carries the same requirement in prose for the +// case where somebody runs the suite somewhere else entirely. +import 'dart:io'; + +import 'package:flutter_test/flutter_test.dart'; + +/// The reference this package owns, in the sibling `magic` checkout. +const String _referencePath = + '../magic/skills/magic-framework/references/plugin-starter.md'; + +/// Reads `version:` out of this package's own manifest. +String _shippedVersion() { + final RegExp pattern = RegExp(r'^version:\s*(\S+)', multiLine: true); + final RegExpMatch? match = pattern.firstMatch( + File('pubspec.yaml').readAsStringSync(), + ); + + expect(match, isNotNull, reason: 'pubspec.yaml declares no version'); + + return match!.group(1)!; +} + +void main() { + test('the magic skill reference is stamped with the shipped version', () { + final File reference = File(_referencePath); + + if (!reference.existsSync()) { + // No sibling checkout. CI is the normal case here, and a hard failure + // would make every merge depend on a repository this one does not + // declare. + markTestSkipped( + 'no sibling magic checkout at $_referencePath, so the reference ' + 'stamp cannot be compared here', + ); + + return; + } + + final String version = _shippedVersion(); + final String firstLine = reference.readAsLinesSync().first; + + expect( + firstLine, + contains('magic_starter v$version'), + reason: + 'the reference an agent reads is stamped for a different version ' + 'than this package ships, which is how it came to document a ' + 'contract that no longer compiles. Update $_referencePath against ' + 'the current source, then move its stamp to v$version.', + ); + }); +} From ecf6ffeae3131014e65e84394eb9d6cc8ce22b0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?An=C4=B1lcan=20=C3=87ak=C4=B1r?= Date: Wed, 9 Sep 2026 23:00:54 +0300 Subject: [PATCH 06/12] chore(deps): follow magic_notifications to ^0.3.0 0.3.0 is where a push tapped on a cold start stops depending on which order the consumer's provider list happens to be in, which was decided by install order rather than by anything a consumer chose. `^0.2.0` is `>=0.2.0 <0.3.0`, so it excludes that release: leaving this line alone would hold every adopter of this starter on the version where a cold tap silently opens the wrong screen. This is the step in a release train that gets skipped, because nothing fails until an adopter resolves the published graph and finds the fix absent. --- pubspec.yaml | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/pubspec.yaml b/pubspec.yaml index 9270ffa..0cea898 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -49,9 +49,15 @@ dependencies: # a `0.0.x` is `>=0.0.x <0.1.0`, because pub_semver raises the MINOR whenever # the major is zero, so `^0.0.2` equally admits 0.0.3, which has no # `Notify.view` at all, and a solver could hand this package a version its own - # routes cannot compile against. `^0.2.0` is `>=0.2.0 <0.3.0` by that same + # routes cannot compile against. `^0.3.0` is `>=0.3.0 <0.4.0` by that same # rule and admits nothing below the release that carries the signature. - magic_notifications: ^0.2.0 + # + # Raised from `^0.2.0` for the 0.3.0 release, and the raise is not optional + # bookkeeping: 0.3.0 is where a push tapped on a COLD START stops depending + # on provider order, and `^0.2.0` excludes it, so a starter left on the old + # constraint holds every adopter on the version where that tap silently opens + # the wrong screen. + magic_notifications: ^0.3.0 # Published, and carrying what the billing screen needs: magic_payments 0.0.2 # is on pub.dev and ships both `BillingCycle` and `PlanStatus.isDunning`. This # comment used to say the package had no release at all and that those two From ef36db97d6ef0f10e2a9c7f6703ec8760a941f69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?An=C4=B1lcan=20=C3=87ak=C4=B1r?= Date: Thu, 10 Sep 2026 01:08:47 +0300 Subject: [PATCH 07/12] Revert "chore(deps): follow magic_notifications to ^0.3.0" This reverts commit ecf6ffeae3131014e65e84394eb9d6cc8ce22b0b. --- pubspec.yaml | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/pubspec.yaml b/pubspec.yaml index 0cea898..9270ffa 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -49,15 +49,9 @@ dependencies: # a `0.0.x` is `>=0.0.x <0.1.0`, because pub_semver raises the MINOR whenever # the major is zero, so `^0.0.2` equally admits 0.0.3, which has no # `Notify.view` at all, and a solver could hand this package a version its own - # routes cannot compile against. `^0.3.0` is `>=0.3.0 <0.4.0` by that same + # routes cannot compile against. `^0.2.0` is `>=0.2.0 <0.3.0` by that same # rule and admits nothing below the release that carries the signature. - # - # Raised from `^0.2.0` for the 0.3.0 release, and the raise is not optional - # bookkeeping: 0.3.0 is where a push tapped on a COLD START stops depending - # on provider order, and `^0.2.0` excludes it, so a starter left on the old - # constraint holds every adopter on the version where that tap silently opens - # the wrong screen. - magic_notifications: ^0.3.0 + magic_notifications: ^0.2.0 # Published, and carrying what the billing screen needs: magic_payments 0.0.2 # is on pub.dev and ships both `BillingCycle` and `PlanStatus.isDunning`. This # comment used to say the package had no release at all and that those two From 0ba0a74f925c4ddfbee7cf68007028939b3b44f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?An=C4=B1lcan=20=C3=87ak=C4=B1r?= Date: Thu, 10 Sep 2026 01:43:32 +0300 Subject: [PATCH 08/12] fix(auth): forget the intended url the sign-out itself recorded Raised in review. Flipping the auth state makes go_router re-run its redirects while the app is still sitting on the protected route, so `EnsureAuthenticated` writes that route down as somewhere to return to, in the gap between `Auth.logout()` and the navigation to login. It belongs to the session that just ended. Sign out on /teams/settings, hand the device over, and the next person to sign in lands there rather than on home. The destination refetches under the new token so this is not an exposure, but it is somebody else's page and nobody asked for it. The account-deletion path had the sharper version of the same thing: it sent the next sign-in to a deleted account's settings screen. Read-and-discard through `pullIntendedUrl`, which is the one-time read; `MagicRouter` exposes no separate clear. Also from the same review: `doc/basics/authentication.md` still said login "navigates to `MagicStarterConfig.homeRoute()`", which has been only the fallback since this branch landed `navigateHome()`. Test verified to fail without the clear. --- doc/basics/authentication.md | 2 +- .../controllers/magic_starter_auth_controller.dart | 11 +++++++++++ .../magic_starter_profile_controller.dart | 8 ++++++++ .../magic_starter_auth_controller_test.dart | 13 +++++++++++++ 4 files changed, 33 insertions(+), 1 deletion(-) diff --git a/doc/basics/authentication.md b/doc/basics/authentication.md index 9df0c82..4b2d606 100644 --- a/doc/basics/authentication.md +++ b/doc/basics/authentication.md @@ -52,7 +52,7 @@ await MagicStarterAuthController.instance.doLogin( The login flow has three possible outcomes: -1. **Success** — the controller extracts `token` and `user` from the nested `data` key, calls `Auth.login()`, sets success state, and navigates to `MagicStarterConfig.homeRoute()`. +1. **Success** — the controller extracts `token` and `user` from the nested `data` key, calls `Auth.login()`, sets success state, and calls `navigateHome()`. That is the intended url a bounced deep link recorded, read once through `MagicRouter.pullIntendedUrl()`, and `MagicStarterConfig.homeRoute()` only as the fallback when nothing was recorded or the recorded value is not an in-app path. A sign-out clears any intent first, so it belongs to the session that asked for it rather than to the next person on the device. 2. **Two-factor required** — the controller detects the challenge flag and navigates to `MagicStarterConfig.twoFactorChallengeRoute()` with the encrypted token as a query parameter. No login occurs yet. 3. **Failure** — `handleApiError()` sets the error state with a localized fallback message. diff --git a/lib/src/http/controllers/magic_starter_auth_controller.dart b/lib/src/http/controllers/magic_starter_auth_controller.dart index ce90b52..8a5c82b 100644 --- a/lib/src/http/controllers/magic_starter_auth_controller.dart +++ b/lib/src/http/controllers/magic_starter_auth_controller.dart @@ -352,6 +352,17 @@ class MagicStarterAuthController extends MagicController // 2. Clear authentication tokens and navigate to login. await Auth.logout(); + + // Drop any intended url the sign-out itself just recorded. Flipping the + // auth state makes go_router re-run its redirects while the app is still + // sitting on the protected route, so `EnsureAuthenticated` writes that + // route down as somewhere to return to, between this line and the one + // above it. It belongs to the session that just ended: without this, a + // sign-out on /teams/settings sends the NEXT person who signs in on this + // device straight to it. Read-and-discard because `pullIntendedUrl` is the + // one-time read and there is no separate clear. + MagicRouter.instance.pullIntendedUrl(); + navigateTo(MagicStarterConfig.loginRoute()); } diff --git a/lib/src/http/controllers/magic_starter_profile_controller.dart b/lib/src/http/controllers/magic_starter_profile_controller.dart index 98334eb..9dabea2 100644 --- a/lib/src/http/controllers/magic_starter_profile_controller.dart +++ b/lib/src/http/controllers/magic_starter_profile_controller.dart @@ -191,6 +191,14 @@ class MagicStarterProfileController extends MagicController } await Auth.logout(); + + // Same reason as the sign-out path in MagicStarterAuthController: the + // auth flip makes go_router re-run redirects while still on the deleted + // account's page, so `EnsureAuthenticated` records it as somewhere to + // return to. Sending the next person who signs in to a deleted account's + // settings screen is the worse version of that bug. + MagicRouter.instance.pullIntendedUrl(); + navigateTo(MagicStarterConfig.loginRoute()); setSuccess(true); return true; diff --git a/test/http/controllers/magic_starter_auth_controller_test.dart b/test/http/controllers/magic_starter_auth_controller_test.dart index 1e12b6f..73a71ae 100644 --- a/test/http/controllers/magic_starter_auth_controller_test.dart +++ b/test/http/controllers/magic_starter_auth_controller_test.dart @@ -587,6 +587,19 @@ void main() { expect(mockGuard.logoutCalled, isTrue); }); + test('clears an intended url the sign-out itself recorded', () async { + // Flipping the auth state makes go_router re-run its redirects while + // the app is still on the protected route, so `EnsureAuthenticated` + // writes that route down as somewhere to return to. It belongs to the + // session that just ended: leaving it sends the NEXT person who signs + // in on this device straight to the previous one's page. + MagicRouter.instance.setIntendedUrl('/teams/settings'); + + await controller.logout(); + + expect(MagicRouter.instance.hasIntendedUrl, isFalse); + }); + test( 'stops notification polling when notification features are enabled', () async { From af6e0ea62592228fa27e755549dcaa58879bff34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?An=C4=B1lcan=20=C3=87ak=C4=B1r?= Date: Thu, 10 Sep 2026 01:57:50 +0300 Subject: [PATCH 09/12] fix(auth): clear the intended url on the session ending, not at each logout call site Second review pass, and the reviewer was right that my first fix was the wrong shape. Clearing after each `Auth.logout()` covered the two logouts a user asks for and missed the one that matters most: magic's `AuthInterceptor` calls `Auth.logout()` itself when a token refresh fails (`auth_interceptor.dart:77`) and `AuthServiceProvider` installs that interceptor unconditionally, so a session that simply EXPIRES on a protected route still recorded it. No call site can reach that one, and it is a regression this branch introduced: before it, the value was never recorded at all. `SessionScopeSync` already listens to `Auth.stateNotifier`, which is the single funnel all three pass through, and already has the branch for the identity dropping to null. The clear goes there and the two call-site clears come out. Deferred by a microtask, which is load-bearing rather than caution. The notifier fans out in registration order: this listener attaches during the starter provider's boot, go_router's attaches when magic builds the router AFTER boot, so a synchronous clear runs BEFORE the redirect that records the value and clears nothing. Named limit, carried into the changelog: a host route with an async `redirect` records after the microtask and is not covered. Three tests. Two fail against the previous commit (a session ending, and a logout no call site performed). The third pins the opposite direction, that an intent survives the login it was recorded for, and it takes TWO guards failing together to break, so it survives removing either alone; verified against a mutant that drops both. Its comment says so rather than claiming more than it measures. The changelog gained the rule itself, which the previous commit synced into `doc/` and not there. --- CHANGELOG.md | 2 + .../magic_starter_auth_controller.dart | 13 +---- .../magic_starter_profile_controller.dart | 8 --- lib/src/http/session_scope_sync.dart | 43 +++++++++++++- .../magic_starter_auth_controller_test.dart | 13 ----- test/http/session_scoped_controller_test.dart | 56 +++++++++++++++++++ 6 files changed, 103 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 96b8830..ed42db8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ All notable changes to this project will be documented in this file. ### Added - **A deep link that lands on a signed-out device now survives the login bounce.** `EnsureAuthenticated.redirectTarget` records the requested location via `MagicRouter.setIntendedUrl` before bouncing an unauthenticated visitor to login, and a new `NavigatesRoutes.navigateHome` reads it back with `pullIntendedUrl` once they authenticate, falling back to `MagicStarterConfig.homeRoute()` when no intent was stored or the stored value is not an in-app path. Nothing is recorded for the login route itself or for any other guest-only auth route (register, forgot-password, reset-password, two-factor-challenge, otp), since a bounced visitor cannot use one of those as a destination either. All five post-auth navigations (login, register auto-login, two-factor challenge, OTP verification, guest login) now call `navigateHome()` instead of navigating straight to the home route. Known limit: `redirectTarget` only ever sees `state.matchedLocation`, which carries no query string, so a recorded intent loses any `?token=...` the original link carried. + **An intent belongs to the session that asked for it, and ending that session discards it.** Signing out flips the auth state, which re-runs go_router's redirects while the app is still on the protected route, so `EnsureAuthenticated` writes that route down as somewhere to return to. Nobody asked for it: a sign-out on `/teams/settings` would otherwise send the NEXT person who signs in on that device straight there, and the account-deletion path would send them to a deleted account's settings. `SessionScopeSync` clears it when the authenticated identity drops to null, which is the one funnel all three logouts pass through: the two a user asks for, and the one the app performs on its own when magic's `AuthInterceptor` fails a token refresh, which no call-site clear can reach. The clear is deferred by a microtask because this listener attaches during provider boot while go_router's attaches when the router is built after it, so a synchronous clear would run before the redirect that records the value. A host route with an ASYNC `redirect` records after that microtask and is not covered. + ## [0.0.1-alpha.26] - 2026-09-03 ### Changed diff --git a/lib/src/http/controllers/magic_starter_auth_controller.dart b/lib/src/http/controllers/magic_starter_auth_controller.dart index 8a5c82b..2437df5 100644 --- a/lib/src/http/controllers/magic_starter_auth_controller.dart +++ b/lib/src/http/controllers/magic_starter_auth_controller.dart @@ -353,16 +353,9 @@ class MagicStarterAuthController extends MagicController // 2. Clear authentication tokens and navigate to login. await Auth.logout(); - // Drop any intended url the sign-out itself just recorded. Flipping the - // auth state makes go_router re-run its redirects while the app is still - // sitting on the protected route, so `EnsureAuthenticated` writes that - // route down as somewhere to return to, between this line and the one - // above it. It belongs to the session that just ended: without this, a - // sign-out on /teams/settings sends the NEXT person who signs in on this - // device straight to it. Read-and-discard because `pullIntendedUrl` is the - // one-time read and there is no separate clear. - MagicRouter.instance.pullIntendedUrl(); - + // The intended url this sign-out just recorded is dropped by + // `SessionScopeSync`, which listens to the one notifier all three logout + // paths pass through, including the passive 401 one no call site can see. navigateTo(MagicStarterConfig.loginRoute()); } diff --git a/lib/src/http/controllers/magic_starter_profile_controller.dart b/lib/src/http/controllers/magic_starter_profile_controller.dart index 9dabea2..98334eb 100644 --- a/lib/src/http/controllers/magic_starter_profile_controller.dart +++ b/lib/src/http/controllers/magic_starter_profile_controller.dart @@ -191,14 +191,6 @@ class MagicStarterProfileController extends MagicController } await Auth.logout(); - - // Same reason as the sign-out path in MagicStarterAuthController: the - // auth flip makes go_router re-run redirects while still on the deleted - // account's page, so `EnsureAuthenticated` records it as somewhere to - // return to. Sending the next person who signs in to a deleted account's - // settings screen is the worse version of that bug. - MagicRouter.instance.pullIntendedUrl(); - navigateTo(MagicStarterConfig.loginRoute()); setSuccess(true); return true; diff --git a/lib/src/http/session_scope_sync.dart b/lib/src/http/session_scope_sync.dart index 7efacf6..7296485 100644 --- a/lib/src/http/session_scope_sync.dart +++ b/lib/src/http/session_scope_sync.dart @@ -125,7 +125,12 @@ class SessionScopeSync { if (identity == _identity) return; _identity = identity; - if (identity == null) return; + + if (identity == null) { + _forgetIntendedUrl(); + + return; + } // Snapshot first: a reset may resolve another controller and register it, // which would otherwise mutate the registry mid-iteration. @@ -145,4 +150,40 @@ class SessionScopeSync { ); } } + + /// Drops the intended url when the session it belongs to ends. + /// + /// Signing out re-runs go_router's redirects while the app is still on the + /// protected route, so `EnsureAuthenticated` writes that route down as + /// somewhere to return to. Nobody asked for it: a sign-out on + /// `/teams/settings` would send the NEXT person who signs in on this device + /// straight there. + /// + /// Here rather than at each `Auth.logout()` call site, which is where this + /// first landed and is a shape that covers only the logouts a user asks for. + /// The one that matters most is the one the app performs on its own: + /// magic's `AuthInterceptor` calls `Auth.logout()` when a token refresh fails + /// (`auth_interceptor.dart:77`) and `AuthServiceProvider` installs that + /// interceptor unconditionally, so a session that simply EXPIRES on a + /// protected route leaked it too. This notifier is the one funnel all three + /// pass through. + /// + /// Deferred by a microtask, and that is load-bearing rather than caution. + /// `Auth.stateNotifier` fans out to its listeners in registration order, and + /// this one attaches during the starter provider's boot while go_router's + /// attaches when magic builds the router AFTER boot. Clearing synchronously + /// would therefore run BEFORE the redirect that records the value, and clear + /// nothing. The microtask runs once the synchronous fan-out is done. + /// + /// Read-and-discard because `pullIntendedUrl` is the one-time read and + /// `MagicRouter` exposes no separate clear. Known limit: a host whose route + /// carries an ASYNC `redirect` records after the microtask, and this misses + /// it. + static void _forgetIntendedUrl() { + scheduleMicrotask(() { + if (Auth.check()) return; + + MagicRouter.instance.pullIntendedUrl(); + }); + } } diff --git a/test/http/controllers/magic_starter_auth_controller_test.dart b/test/http/controllers/magic_starter_auth_controller_test.dart index 73a71ae..1e12b6f 100644 --- a/test/http/controllers/magic_starter_auth_controller_test.dart +++ b/test/http/controllers/magic_starter_auth_controller_test.dart @@ -587,19 +587,6 @@ void main() { expect(mockGuard.logoutCalled, isTrue); }); - test('clears an intended url the sign-out itself recorded', () async { - // Flipping the auth state makes go_router re-run its redirects while - // the app is still on the protected route, so `EnsureAuthenticated` - // writes that route down as somewhere to return to. It belongs to the - // session that just ended: leaving it sends the NEXT person who signs - // in on this device straight to the previous one's page. - MagicRouter.instance.setIntendedUrl('/teams/settings'); - - await controller.logout(); - - expect(MagicRouter.instance.hasIntendedUrl, isFalse); - }); - test( 'stops notification polling when notification features are enabled', () async { diff --git a/test/http/session_scoped_controller_test.dart b/test/http/session_scoped_controller_test.dart index f05c358..a564015 100644 --- a/test/http/session_scoped_controller_test.dart +++ b/test/http/session_scoped_controller_test.dart @@ -271,6 +271,62 @@ void main() { expect(controller.rows, ['rows-for-1']); }); + test('forgets the intended url when the session ends', () async { + await loginAs(1, teamId: 10); + SessionScopeSync.attach(); + + // What `EnsureAuthenticated` writes down when the auth flip re-runs + // go_router's redirects while the app is still on a protected route. + MagicRouter.instance.setIntendedUrl('/teams/settings'); + + await Auth.logout(); + await pumpEventQueue(); + + expect(MagicRouter.instance.hasIntendedUrl, isFalse); + }); + + test( + 'forgets the intended url on a logout NO call site performed', + () async { + await loginAs(1, teamId: 10); + SessionScopeSync.attach(); + MagicRouter.instance.setIntendedUrl('/teams/settings'); + + // The path that matters and that a per-call-site clear cannot reach: + // magic's AuthInterceptor calls `Auth.logout()` itself when a token + // refresh fails, and AuthServiceProvider installs it unconditionally, so + // a session that simply EXPIRES on a protected route took this route. + // Driven through the facade directly, because that is all the + // interceptor does. + await Auth.logout(); + await pumpEventQueue(); + + expect(MagicRouter.instance.hasIntendedUrl, isFalse); + }, + ); + + test('keeps an intended url across the login it was recorded for', () async { + SessionScopeSync.attach(); + + // The whole point of the intent: a deep link lands on a signed-out + // device, `EnsureAuthenticated` records it, and the login that follows + // is supposed to consume it. Signing in bumps the same notifier a + // sign-out does, so a clear hung off the bump itself would eat the + // feature it is protecting. + // + // Two guards have to fail together for that to happen, which is why this + // survives removing either one alone: the branch only calls the clear on + // a transition TO signed-out, and the clear re-checks `Auth.check()` + // inside its microtask because by then the state may have moved again. + // Verified against a mutant that drops both. + MagicRouter.instance.setIntendedUrl('/incidents/1'); + + await loginAs(1, teamId: 10); + await pumpEventQueue(); + + expect(MagicRouter.instance.pullIntendedUrl(), '/incidents/1'); + }); + test('resets when the same user signs back in after a logout', () async { await loginAs(1, teamId: 10); SessionScopeSync.attach(); From 7347f453884c9e6bd2386c95efc4bd10bae90331 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?An=C4=B1lcan=20=C3=87ak=C4=B1r?= Date: Thu, 10 Sep 2026 02:10:06 +0300 Subject: [PATCH 10/12] fix(auth): put the intended-url clear somewhere every app actually runs Third review pass on the same defect, and the reviewer was right twice running. The clear moved onto the auth notifier, which was the right funnel, but it went into `SessionScopeSync` and that class is OPT-IN: `attach()` is the host's call (`doc/basics/session-scope.md:111`) and nothing in `lib/src/providers/`, `install.yaml` or `assets/stubs/` calls or scaffolds it. Verified by grep. So an app that never adopted `SessionScopedController` lost the clear entirely, and the previous commit had deleted the two unconditional call-site clears on the way past. Strictly worse than either shape before it. It now hangs off `MagicStarterServiceProvider.boot()`, which every starter app runs. Same notifier, same microtask deferral, same `Auth.check()` re-check, and the docblock says why it is not in `SessionScopeSync` so the next person does not move it back. The tests moved with it, and this is the point rather than bookkeeping: they lived in the session-scope suite, which calls `attach()` itself, so they proved the clear worked in exactly the configuration an adopter might not have. They now boot the real provider, which is what an adopter has. Verified to fail against a provider without the listener. The duplicate the reviewer flagged is gone too. "A logout no call site performed" drove the same `Auth.logout()` as the test above it, so it was a second copy rather than coverage of the interceptor path; one test on the notifier covers every route into it, which is the whole argument for putting the clear there. `doc/basics/authentication.md` and the changelog both said `SessionScopeSync` clears it and read as unconditional guarantees. Both now name the provider and say no host wiring is needed. --- CHANGELOG.md | 2 +- doc/basics/authentication.md | 2 +- lib/src/http/session_scope_sync.dart | 43 +------------ .../magic_starter_service_provider.dart | 62 +++++++++++++++++++ test/http/session_scoped_controller_test.dart | 56 ----------------- .../middleware/ensure_authenticated_test.dart | 49 +++++++++++++++ 6 files changed, 114 insertions(+), 100 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ed42db8..739504d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ All notable changes to this project will be documented in this file. ### Added - **A deep link that lands on a signed-out device now survives the login bounce.** `EnsureAuthenticated.redirectTarget` records the requested location via `MagicRouter.setIntendedUrl` before bouncing an unauthenticated visitor to login, and a new `NavigatesRoutes.navigateHome` reads it back with `pullIntendedUrl` once they authenticate, falling back to `MagicStarterConfig.homeRoute()` when no intent was stored or the stored value is not an in-app path. Nothing is recorded for the login route itself or for any other guest-only auth route (register, forgot-password, reset-password, two-factor-challenge, otp), since a bounced visitor cannot use one of those as a destination either. All five post-auth navigations (login, register auto-login, two-factor challenge, OTP verification, guest login) now call `navigateHome()` instead of navigating straight to the home route. Known limit: `redirectTarget` only ever sees `state.matchedLocation`, which carries no query string, so a recorded intent loses any `?token=...` the original link carried. - **An intent belongs to the session that asked for it, and ending that session discards it.** Signing out flips the auth state, which re-runs go_router's redirects while the app is still on the protected route, so `EnsureAuthenticated` writes that route down as somewhere to return to. Nobody asked for it: a sign-out on `/teams/settings` would otherwise send the NEXT person who signs in on that device straight there, and the account-deletion path would send them to a deleted account's settings. `SessionScopeSync` clears it when the authenticated identity drops to null, which is the one funnel all three logouts pass through: the two a user asks for, and the one the app performs on its own when magic's `AuthInterceptor` fails a token refresh, which no call-site clear can reach. The clear is deferred by a microtask because this listener attaches during provider boot while go_router's attaches when the router is built after it, so a synchronous clear would run before the redirect that records the value. A host route with an ASYNC `redirect` records after that microtask and is not covered. + **An intent belongs to the session that asked for it, and ending that session discards it.** Signing out flips the auth state, which re-runs go_router's redirects while the app is still on the protected route, so `EnsureAuthenticated` writes that route down as somewhere to return to. Nobody asked for it: a sign-out on `/teams/settings` would otherwise send the NEXT person who signs in on that device straight there, and the account-deletion path would send them to a deleted account's settings. `MagicStarterServiceProvider` now listens to `Auth.stateNotifier` and discards the intent whenever the state goes to signed-out, which is the one funnel all three logouts pass through: the two a user asks for, and the one the app performs on its own when magic's `AuthInterceptor` fails a token refresh, which no call-site clear can reach. In this provider rather than in `SessionScopeSync`, which listens to the same notifier: that one is opt-in and nothing in this package calls `attach()`, so an app that never adopted `SessionScopedController` would have had no clear at all. This provider boots in every starter app, so no host action is needed. The clear is deferred by a microtask because the whole record path is synchronous and clearing inline would run before the redirect that writes the value. A host route with an ASYNC `redirect` records after that microtask and is not covered. ## [0.0.1-alpha.26] - 2026-09-03 diff --git a/doc/basics/authentication.md b/doc/basics/authentication.md index 4b2d606..4aa837f 100644 --- a/doc/basics/authentication.md +++ b/doc/basics/authentication.md @@ -52,7 +52,7 @@ await MagicStarterAuthController.instance.doLogin( The login flow has three possible outcomes: -1. **Success** — the controller extracts `token` and `user` from the nested `data` key, calls `Auth.login()`, sets success state, and calls `navigateHome()`. That is the intended url a bounced deep link recorded, read once through `MagicRouter.pullIntendedUrl()`, and `MagicStarterConfig.homeRoute()` only as the fallback when nothing was recorded or the recorded value is not an in-app path. A sign-out clears any intent first, so it belongs to the session that asked for it rather than to the next person on the device. +1. **Success** — the controller extracts `token` and `user` from the nested `data` key, calls `Auth.login()`, sets success state, and calls `navigateHome()`. That is the intended url a bounced deep link recorded, read once through `MagicRouter.pullIntendedUrl()`, and `MagicStarterConfig.homeRoute()` only as the fallback when nothing was recorded or the recorded value is not an in-app path. An intent belongs to the session that asked for it: `MagicStarterServiceProvider` listens to `Auth.stateNotifier` and discards it whenever the state goes to signed-out, so it never reaches the next person on the device. That covers the sign-out a user asks for, the account deletion, and the one the app performs on its own when a token refresh fails, since all three go through the same notifier. No host wiring is needed; the provider boots in every starter app. 2. **Two-factor required** — the controller detects the challenge flag and navigates to `MagicStarterConfig.twoFactorChallengeRoute()` with the encrypted token as a query parameter. No login occurs yet. 3. **Failure** — `handleApiError()` sets the error state with a localized fallback message. diff --git a/lib/src/http/session_scope_sync.dart b/lib/src/http/session_scope_sync.dart index 7296485..7efacf6 100644 --- a/lib/src/http/session_scope_sync.dart +++ b/lib/src/http/session_scope_sync.dart @@ -125,12 +125,7 @@ class SessionScopeSync { if (identity == _identity) return; _identity = identity; - - if (identity == null) { - _forgetIntendedUrl(); - - return; - } + if (identity == null) return; // Snapshot first: a reset may resolve another controller and register it, // which would otherwise mutate the registry mid-iteration. @@ -150,40 +145,4 @@ class SessionScopeSync { ); } } - - /// Drops the intended url when the session it belongs to ends. - /// - /// Signing out re-runs go_router's redirects while the app is still on the - /// protected route, so `EnsureAuthenticated` writes that route down as - /// somewhere to return to. Nobody asked for it: a sign-out on - /// `/teams/settings` would send the NEXT person who signs in on this device - /// straight there. - /// - /// Here rather than at each `Auth.logout()` call site, which is where this - /// first landed and is a shape that covers only the logouts a user asks for. - /// The one that matters most is the one the app performs on its own: - /// magic's `AuthInterceptor` calls `Auth.logout()` when a token refresh fails - /// (`auth_interceptor.dart:77`) and `AuthServiceProvider` installs that - /// interceptor unconditionally, so a session that simply EXPIRES on a - /// protected route leaked it too. This notifier is the one funnel all three - /// pass through. - /// - /// Deferred by a microtask, and that is load-bearing rather than caution. - /// `Auth.stateNotifier` fans out to its listeners in registration order, and - /// this one attaches during the starter provider's boot while go_router's - /// attaches when magic builds the router AFTER boot. Clearing synchronously - /// would therefore run BEFORE the redirect that records the value, and clear - /// nothing. The microtask runs once the synchronous fan-out is done. - /// - /// Read-and-discard because `pullIntendedUrl` is the one-time read and - /// `MagicRouter` exposes no separate clear. Known limit: a host whose route - /// carries an ASYNC `redirect` records after the microtask, and this misses - /// it. - static void _forgetIntendedUrl() { - scheduleMicrotask(() { - if (Auth.check()) return; - - MagicRouter.instance.pullIntendedUrl(); - }); - } } diff --git a/lib/src/providers/magic_starter_service_provider.dart b/lib/src/providers/magic_starter_service_provider.dart index 2ec46dc..1ce0b11 100644 --- a/lib/src/providers/magic_starter_service_provider.dart +++ b/lib/src/providers/magic_starter_service_provider.dart @@ -1,3 +1,5 @@ +import 'dart:async'; + import 'package:flutter/widgets.dart'; import 'package:magic/magic.dart'; @@ -54,6 +56,66 @@ class MagicStarterServiceProvider extends ServiceProvider { // 2. If not, register 'indigo' as the fallback primary color. // 3. Emit info log to notify about the fallback. _bootPrimaryColorFallback(); + + _forgetIntendedUrlOnSignOut(); + } + + /// The notifier [_forgetIntendedUrlOnSignOut] subscribed to. + /// + /// Held rather than resolved again at removal time for the same reason + /// `SessionScopeSync` holds its own: `Auth.stateNotifier` resolves through + /// the container, so re-binding the guard hands back a DIFFERENT notifier and + /// unsubscribing through the facade would leave this listener on the old one. + /// Doubles as the attached flag, so a hot restart cannot double-subscribe. + static ValueNotifier? _authState; + + /// Discards the intended url when the session that recorded it ends. + /// + /// [EnsureAuthenticated] records a protected route before bouncing to login, + /// so `navigateHome()` can send the visitor back to it afterwards. Signing + /// out flips the auth state, which re-runs go_router's redirects while the + /// app is STILL on that route, so the sign-out records it too. Nobody asked + /// for that: it would send the next person who signs in on this device to the + /// previous one's page, and on the account-deletion path to a deleted + /// account's settings. + /// + /// Hung off the auth notifier rather than off each `Auth.logout()` call site, + /// which is where this first landed and covers only the logouts a user asks + /// for. The one that matters most is the one the app performs on its own: + /// magic's `AuthInterceptor` calls `Auth.logout()` when a token refresh fails + /// (`auth_interceptor.dart:77`) and `AuthServiceProvider` installs it + /// unconditionally, so a session that simply EXPIRES on a protected route + /// took that path. The notifier is the one funnel all three pass through. + /// + /// Here rather than in `SessionScopeSync`, which listens to the same notifier + /// and was the second thing tried: that class is OPT-IN and nothing in this + /// package calls `attach()`, so an app that never adopted + /// `SessionScopedController` would have had no clear at all. This provider + /// boots in every starter app. + /// + /// Deferred by a microtask because the whole record path (`stateNotifier` -> + /// `GoRouteInformationProvider.notifyListeners` -> parse -> redirect) is + /// synchronous: clearing inline would run before the redirect that writes the + /// value. Known limit: a host route with an ASYNC `redirect` records after + /// the microtask and is not covered. Read-and-discard because + /// `pullIntendedUrl` is the one-time read and `MagicRouter` exposes no + /// separate clear. + void _forgetIntendedUrlOnSignOut() { + if (_authState != null) return; + + _authState = Auth.stateNotifier + ..addListener(() { + if (Auth.check()) return; + + scheduleMicrotask(() { + // Re-checked inside the microtask: by the time it runs the state may + // have moved again, and clearing after a login would eat the deep + // link the intent exists to serve. + if (Auth.check()) return; + + MagicRouter.instance.pullIntendedUrl(); + }); + }); } /// Registers Gate abilities that control profile section visibility. diff --git a/test/http/session_scoped_controller_test.dart b/test/http/session_scoped_controller_test.dart index a564015..f05c358 100644 --- a/test/http/session_scoped_controller_test.dart +++ b/test/http/session_scoped_controller_test.dart @@ -271,62 +271,6 @@ void main() { expect(controller.rows, ['rows-for-1']); }); - test('forgets the intended url when the session ends', () async { - await loginAs(1, teamId: 10); - SessionScopeSync.attach(); - - // What `EnsureAuthenticated` writes down when the auth flip re-runs - // go_router's redirects while the app is still on a protected route. - MagicRouter.instance.setIntendedUrl('/teams/settings'); - - await Auth.logout(); - await pumpEventQueue(); - - expect(MagicRouter.instance.hasIntendedUrl, isFalse); - }); - - test( - 'forgets the intended url on a logout NO call site performed', - () async { - await loginAs(1, teamId: 10); - SessionScopeSync.attach(); - MagicRouter.instance.setIntendedUrl('/teams/settings'); - - // The path that matters and that a per-call-site clear cannot reach: - // magic's AuthInterceptor calls `Auth.logout()` itself when a token - // refresh fails, and AuthServiceProvider installs it unconditionally, so - // a session that simply EXPIRES on a protected route took this route. - // Driven through the facade directly, because that is all the - // interceptor does. - await Auth.logout(); - await pumpEventQueue(); - - expect(MagicRouter.instance.hasIntendedUrl, isFalse); - }, - ); - - test('keeps an intended url across the login it was recorded for', () async { - SessionScopeSync.attach(); - - // The whole point of the intent: a deep link lands on a signed-out - // device, `EnsureAuthenticated` records it, and the login that follows - // is supposed to consume it. Signing in bumps the same notifier a - // sign-out does, so a clear hung off the bump itself would eat the - // feature it is protecting. - // - // Two guards have to fail together for that to happen, which is why this - // survives removing either one alone: the branch only calls the clear on - // a transition TO signed-out, and the clear re-checks `Auth.check()` - // inside its microtask because by then the state may have moved again. - // Verified against a mutant that drops both. - MagicRouter.instance.setIntendedUrl('/incidents/1'); - - await loginAs(1, teamId: 10); - await pumpEventQueue(); - - expect(MagicRouter.instance.pullIntendedUrl(), '/incidents/1'); - }); - test('resets when the same user signs back in after a logout', () async { await loginAs(1, teamId: 10); SessionScopeSync.attach(); diff --git a/test/middleware/ensure_authenticated_test.dart b/test/middleware/ensure_authenticated_test.dart index 18a3bd2..992385f 100644 --- a/test/middleware/ensure_authenticated_test.dart +++ b/test/middleware/ensure_authenticated_test.dart @@ -23,6 +23,10 @@ _FakeUser _fakeUser() { } void main() { + // The starter provider's boot reads `WidgetsBinding.instance` for its primary + // colour fallback, and one group below boots it for real. + TestWidgetsFlutterBinding.ensureInitialized(); + setUp(() { MagicApp.reset(); Magic.flush(); @@ -94,4 +98,49 @@ void main() { expect(MagicRouter.instance.hasIntendedUrl, isFalse); }); }); + + group('the intended url a sign-out recorded', () { + /// Boots the starter provider, which is what registers the clear. + /// + /// Through the provider rather than a helper, because the finding this + /// covers was that the clear used to hang off `SessionScopeSync.attach()`, + /// which is OPT-IN and which nothing in this package calls: an app that + /// never adopted session scoping had no clear at all. Booting the provider + /// is what every starter app does, so that is what the test does. + Future bootProvider() async { + final provider = MagicStarterServiceProvider(MagicApp.instance); + provider.register(); + await provider.boot(); + } + + test('is discarded when the session ends', () async { + Auth.fake(user: _fakeUser()); + await bootProvider(); + + // What EnsureAuthenticated writes down when the auth flip re-runs + // go_router's redirects while the app is still on a protected route. + MagicRouter.instance.setIntendedUrl('/teams/settings'); + + await Auth.logout(); + await pumpEventQueue(); + + expect(MagicRouter.instance.hasIntendedUrl, isFalse); + }); + + test('survives the login it was recorded for', () async { + Auth.fake(); + await bootProvider(); + + // The whole point of the intent: a deep link lands on a signed-out + // device, the middleware records it, and the login that follows consumes + // it. Signing in bumps the same notifier a sign-out does, so a clear hung + // off the bump itself would eat the feature it protects. + MagicRouter.instance.setIntendedUrl('/incidents/1'); + + await Auth.login({'token': 't'}, _fakeUser()); + await pumpEventQueue(); + + expect(MagicRouter.instance.pullIntendedUrl(), '/incidents/1'); + }); + }); } From f1cd7450970c5b33104991a4a0d57f49d07343d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?An=C4=B1lcan=20=C3=87ak=C4=B1r?= Date: Thu, 10 Sep 2026 03:09:52 +0300 Subject: [PATCH 11/12] fix(auth): move the auth subscription when the guard is re-bound Third finding on the same clear, and the reviewer proved it by running the suite rather than reading it: `flutter test test/middleware/ensure_authenticated_test.dart --test-randomize-ordering-seed=1` failed, and declaration order passed. I reproduced both. `if (_authState != null) return;` was a one-way latch over a static nothing clears, while `Auth.stateNotifier` resolves through the container and hands back a DIFFERENT notifier every time the guard is re-bound. So the first boot in a process subscribed and every later one returned early, leaving the listener on a notifier nobody bumps: no clear at all. `SessionScopeSync` solves exactly this with `detach()` and says why in its own docblock; I copied the field and not the escape. It now compares the held notifier against the current one by identity and MOVES the subscription. That needs the listener to be a named static rather than a closure, since `removeListener` matches by identity and a fresh closure never equals the one that was added. The knock-on the reviewer also caught: with the latch, the second test booted a provider that early-returned, so its listener sat on the previous test's notifier and the assertion held vacuously. Its comment claimed a mutation verification that the ordering had quietly disabled. Both are real again, and verified: five seeds (1, 2, 3, 12345, 777) all pass, and a mutant with both `Auth.check()` guards removed fails the survives-login test under seed 1. Dropped the "a hot restart cannot double-subscribe" line while I was there. A hot restart re-runs main with statics reset, so the field is null again anyway; the case it has to survive is the re-bind, which is the one it was not surviving. --- .../magic_starter_service_provider.dart | 44 +++++++++++++------ 1 file changed, 30 insertions(+), 14 deletions(-) diff --git a/lib/src/providers/magic_starter_service_provider.dart b/lib/src/providers/magic_starter_service_provider.dart index 1ce0b11..6722255 100644 --- a/lib/src/providers/magic_starter_service_provider.dart +++ b/lib/src/providers/magic_starter_service_provider.dart @@ -60,13 +60,20 @@ class MagicStarterServiceProvider extends ServiceProvider { _forgetIntendedUrlOnSignOut(); } - /// The notifier [_forgetIntendedUrlOnSignOut] subscribed to. + /// The notifier [_forgetIntendedUrlOnSignOut] is currently subscribed to. /// /// Held rather than resolved again at removal time for the same reason /// `SessionScopeSync` holds its own: `Auth.stateNotifier` resolves through /// the container, so re-binding the guard hands back a DIFFERENT notifier and /// unsubscribing through the facade would leave this listener on the old one. - /// Doubles as the attached flag, so a hot restart cannot double-subscribe. + /// + /// Compared by identity rather than treated as a one-way latch, which is what + /// it was first written as and what a review caught. A latch never cleared, + /// so a second boot after a re-bind returned early and left the subscription + /// on a notifier nobody bumps any more: no clear at all, and a test suite + /// where the assertion holds only because an earlier test happened to attach + /// first. It failed under `--test-randomize-ordering-seed=1` and passed in + /// declaration order, which is the worst way for it to be wrong. static ValueNotifier? _authState; /// Discards the intended url when the session that recorded it ends. @@ -101,21 +108,30 @@ class MagicStarterServiceProvider extends ServiceProvider { /// `pullIntendedUrl` is the one-time read and `MagicRouter` exposes no /// separate clear. void _forgetIntendedUrlOnSignOut() { - if (_authState != null) return; + final ValueNotifier notifier = Auth.stateNotifier; + if (identical(_authState, notifier)) return; + + // Moves rather than adds. Booting twice against the same notifier is the + // no-op above; booting against a NEW one has to take the subscription with + // it, or the listener sits on a notifier nothing bumps. + _authState?.removeListener(_forgetIntendedUrl); + _authState = notifier..addListener(_forgetIntendedUrl); + } - _authState = Auth.stateNotifier - ..addListener(() { - if (Auth.check()) return; + /// The listener itself, a named static so [_forgetIntendedUrlOnSignOut] can + /// remove it: `removeListener` matches by identity and a fresh closure never + /// equals the one that was added. + static void _forgetIntendedUrl() { + if (Auth.check()) return; - scheduleMicrotask(() { - // Re-checked inside the microtask: by the time it runs the state may - // have moved again, and clearing after a login would eat the deep - // link the intent exists to serve. - if (Auth.check()) return; + scheduleMicrotask(() { + // Re-checked inside the microtask: by the time it runs the state may + // have moved again, and clearing after a login would eat the deep link + // the intent exists to serve. + if (Auth.check()) return; - MagicRouter.instance.pullIntendedUrl(); - }); - }); + MagicRouter.instance.pullIntendedUrl(); + }); } /// Registers Gate abilities that control profile section visibility. From 9a72bcece25d413de29c646db2864955ed71bfe2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?An=C4=B1lcan=20=C3=87ak=C4=B1r?= Date: Thu, 10 Sep 2026 04:10:26 +0300 Subject: [PATCH 12/12] docs(auth): answer why _isGuestRoute stays, after three reviews asking Raised in three consecutive reviews and never answered, which is my omission rather than the reviewer's persistence. The branch IS unreachable through this package's own routes: `auth_routes.dart:19` registers the whole auth group under `middleware: ['guest']`, so `EnsureAuthenticated` never sees `/auth/register` or its siblings. Verified. It stays because this middleware is public API and a host applying `auth` globally over a shell route is a supported configuration, not a hypothetical. Without the branch such an app bounces a visitor off `/auth/register`, records it, and sends them back to a guest-only route after they sign in. Dead for us, live for an adopter, and the two tests over it call `redirectTarget` directly so nothing shows the router reaching it. All of that now sits in the docblock instead of in a review thread. --- lib/src/middleware/ensure_authenticated.dart | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/lib/src/middleware/ensure_authenticated.dart b/lib/src/middleware/ensure_authenticated.dart index 931507d..57d7f72 100644 --- a/lib/src/middleware/ensure_authenticated.dart +++ b/lib/src/middleware/ensure_authenticated.dart @@ -59,6 +59,21 @@ class EnsureAuthenticated extends MagicMiddleware { /// Whether [location] is one of the guest-only auth routes registered in /// `auth_routes.dart` (the login route itself is handled by the caller). + /// + /// Unreachable through THIS package's own route table, and kept anyway. + /// `auth_routes.dart:19` registers the whole auth group under + /// `middleware: ['guest']`, so this middleware never sees `/auth/register` + /// or its siblings there. What it is for is a host that applies `auth` + /// globally, over a shell route wrapping everything, which the middleware + /// being public API makes a supported configuration rather than a + /// hypothetical: without this, such an app would bounce a visitor off + /// `/auth/register`, record it, and send them back to a guest-only route + /// after they sign in. + /// + /// Named here because a reviewer asked three times whether the branch was + /// dead. It is dead for us and live for an adopter, and the two tests over + /// it call [redirectTarget] directly, so nothing shows the router reaching + /// it. bool _isGuestRoute(String location) { final String authPrefix = MagicStarterConfig.authPrefix(); return _guestRouteSuffixes.any(