From 7057ee93ace02b56ac588619b6ecc6d2561421e2 Mon Sep 17 00:00:00 2001 From: edde746 <86283021+edde746@users.noreply.github.com> Date: Fri, 1 May 2026 03:16:32 +0200 Subject: [PATCH] fix: defer profile binding before selection --- lib/main.dart | 24 ++++--- lib/profiles/active_profile_binder.dart | 48 ++++++++++++-- lib/profiles/profile_activation.dart | 10 ++- lib/screens/auth_screen.dart | 47 +++++++++++--- test/profiles/active_profile_binder_test.dart | 36 +++++++++++ test/screens/auth_screen_test.dart | 62 +++++++++++++++++++ 6 files changed, 206 insertions(+), 21 deletions(-) diff --git a/lib/main.dart b/lib/main.dart index 09c7b5d8..b2d11679 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -687,14 +687,22 @@ class _MainAppState extends State with WidgetsBindingObserver { // mounts (and discover/libraries query) before any client exists. Provider( lazy: false, - create: (context) => ActiveProfileBinder( - activeProfile: context.read(), - connections: context.read(), - profileConnections: context.read(), - serverManager: _serverManager, - multiServerProvider: context.read(), - pinPrompt: _rootPinPrompt, - )..start(), + create: (context) { + final activeProfile = context.read(); + return ActiveProfileBinder( + activeProfile: activeProfile, + connections: context.read(), + profileConnections: context.read(), + serverManager: _serverManager, + multiServerProvider: context.read(), + pinPrompt: _rootPinPrompt, + shouldDeferInitialBind: (_) async { + final settings = await SettingsService.getInstance(); + return settings.read(SettingsService.requireProfileSelectionOnOpen) && + activeProfile.hasMultipleProfiles; + }, + )..start(); + }, dispose: (_, binder) => binder.dispose(), ), // Offline mode provider - depends on MultiServerProvider diff --git a/lib/profiles/active_profile_binder.dart b/lib/profiles/active_profile_binder.dart index 22298bb1..f90ca5d2 100644 --- a/lib/profiles/active_profile_binder.dart +++ b/lib/profiles/active_profile_binder.dart @@ -20,6 +20,8 @@ import 'profile_connection_registry.dart'; /// (typically in `main_screen.dart`) should call `showPinEntryDialog`. typedef PlexHomePinPrompt = Future Function(Profile profile, {String? errorMessage}); +typedef ShouldDeferInitialBind = FutureOr Function(Profile profile); + @visibleForTesting bool shouldUsePlexHomeTokenCache({required bool preVerified, required bool hasBoundOnce, required bool plexProtected}) { return preVerified || (!hasBoundOnce && !plexProtected); @@ -52,6 +54,7 @@ class ActiveProfileBinder { required this.serverManager, required this.multiServerProvider, required this.pinPrompt, + this.shouldDeferInitialBind, PlexAuthService? plexAuth, }) : _plexAuth = plexAuth; @@ -61,6 +64,7 @@ class ActiveProfileBinder { final MultiServerManager serverManager; final MultiServerProvider multiServerProvider; final PlexHomePinPrompt pinPrompt; + final ShouldDeferInitialBind? shouldDeferInitialBind; PlexAuthService? _plexAuth; @@ -87,6 +91,7 @@ class ActiveProfileBinder { /// once by [_bindPlexHome] to permit the freshly cached user-token for /// that single rebind and avoid a duplicate PIN prompt. final Set _plexHomePreVerified = {}; + final Set _userInitiatedActivations = {}; bool get isSwitching => _isSwitching; @@ -97,11 +102,20 @@ class ActiveProfileBinder { _plexHomePreVerified.add(profileId); } + void markUserInitiatedActivation(String profileId) { + _userInitiatedActivations.add(profileId); + } + @visibleForTesting bool consumePlexHomePreVerified(String profileId) { return _plexHomePreVerified.remove(profileId); } + @visibleForTesting + bool consumeUserInitiatedActivation(String profileId) { + return _userInitiatedActivations.remove(profileId); + } + void start() { if (_started) return; _started = true; @@ -194,14 +208,21 @@ class ActiveProfileBinder { // waiting, doesn't surface a spurious "switch failed" error. Also // clear the runtime filter so stale clients from the previous // profile cannot leak into the no-selection state. - for (final serverId in serverManager.serverIds.toList()) { - serverManager.removeServer(serverId); - } - multiServerProvider.setVisibleServerIds({}); + _clearBoundServers(); success = true; return; } attemptedProfileId = profile.id; + + final userInitiated = consumeUserInitiatedActivation(profile.id); + if (!userInitiated && !_hasBoundOnce && await _shouldDeferInitialBind(profile)) { + appLogger.i('ActiveProfileBinder: deferring initial bind for ${profile.displayName} until profile selection'); + _clearBoundServers(); + attemptedProfileId = null; + success = true; + return; + } + appLogger.i('ActiveProfileBinder: rebinding for ${profile.displayName} (${profile.id})'); final visibleServerIds = {}; @@ -507,10 +528,29 @@ class ActiveProfileBinder { return _plexAuth ??= await PlexAuthService.create(); } + Future _shouldDeferInitialBind(Profile profile) async { + final shouldDefer = shouldDeferInitialBind; + if (shouldDefer == null) return false; + try { + return await shouldDefer(profile); + } catch (e, st) { + appLogger.w('ActiveProfileBinder: defer check failed; continuing with bind', error: e, stackTrace: st); + return false; + } + } + + void _clearBoundServers() { + for (final serverId in serverManager.serverIds.toList()) { + serverManager.removeServer(serverId); + } + multiServerProvider.setVisibleServerIds({}); + } + void dispose() { if (!_started) return; activeProfile.removeListener(_onActiveProfileChanged); _plexHomePreVerified.clear(); + _userInitiatedActivations.clear(); _plexAuth?.dispose(); _plexAuth = null; _started = false; diff --git a/lib/profiles/profile_activation.dart b/lib/profiles/profile_activation.dart index 1f4e1822..dff129bf 100644 --- a/lib/profiles/profile_activation.dart +++ b/lib/profiles/profile_activation.dart @@ -27,16 +27,19 @@ import 'profile_connection_registry.dart'; /// token instead of re-prompting for the same PIN. Future activateProfileWithPin(BuildContext context, Profile profile) async { final active = context.read(); + final binder = context.read(); if (profile.isPlexHome) { if (profile.plexProtected) { final ok = await _preVerifyPlexHomePin(context, profile); if (!ok) return false; } + binder.markUserInitiatedActivation(profile.id); return active.activate(profile); } if (!profile.isPinProtected) { + binder.markUserInitiatedActivation(profile.id); return active.activate(profile); } @@ -45,8 +48,11 @@ Future activateProfileWithPin(BuildContext context, Profile profile) async if (!context.mounted) return false; final pin = await showPinEntryDialog(context, profile.displayName, errorMessage: errorMessage); if (pin == null) return false; // user cancelled - final ok = await active.activate(profile, pin: pin); - if (ok) return true; + final hash = profile.pinHash; + if (hash != null && verifyPin(pin, hash)) { + binder.markUserInitiatedActivation(profile.id); + return active.activate(profile, pin: pin); + } errorMessage = 'Incorrect PIN. Please try again.'; } } diff --git a/lib/screens/auth_screen.dart b/lib/screens/auth_screen.dart index 5ab058f3..006bc3a5 100644 --- a/lib/screens/auth_screen.dart +++ b/lib/screens/auth_screen.dart @@ -8,6 +8,7 @@ import '../profiles/active_profile_provider.dart'; import '../profiles/plex_home_service.dart'; import '../profiles/profile.dart'; import '../services/plex_auth_service.dart'; +import '../services/settings_service.dart'; import '../services/storage_service.dart'; import '../providers/user_profile_provider.dart'; import '../i18n/strings.g.dart'; @@ -20,6 +21,7 @@ import '../widgets/backend_badge.dart'; import '../widgets/dialog_action_button.dart'; import 'auth/plex_pin_auth_flow.dart'; import 'main_screen.dart'; +import 'profile/profile_switch_screen.dart'; import 'settings/add_jellyfin_screen.dart'; class AuthScreen extends StatefulWidget { @@ -77,11 +79,10 @@ class _AuthScreenState extends State { await activeProfiles.activate(profile); } - /// Persist the new Plex account into the connection pipeline and - /// navigate to the main screen. The [ActiveProfileBinder] (mounted by - /// [MainScreen]) takes over from there: it picks up the active profile - /// id we set below and connects servers via - /// [MultiServerManager.refreshTokensForProfile]. + /// Persist the new Plex account into the connection pipeline, resolve the + /// initial active profile when possible, and navigate to the main screen. + /// The top-level [ActiveProfileBinder] picks up the active profile id and + /// connects servers via [MultiServerManager.refreshTokensForProfile]. Future _connectToAllServersAndNavigate(String plexToken) async { if (!mounted) return; @@ -130,14 +131,36 @@ class _AuthScreenState extends State { await connectionRegistry.upsert(accountConnection); await plexHome.refresh(accountConnection); if (!mounted) return; - await _selectInitialProfile(plexHome, context.read(), accountConnection); + final activeProfiles = context.read(); + await _selectInitialProfile(plexHome, activeProfiles, accountConnection); if (!mounted) return; + final settings = await SettingsService.getInstance(); + if (!mounted) return; + + final promptHandled = shouldPromptForInitialProfileSelection( + activeProfile: activeProfiles.active, + hasProfiles: activeProfiles.profiles.isNotEmpty, + accountHasHomeUsers: plexHome.current[accountConnection.id]?.isNotEmpty == true, + requireProfileSelectionOnOpen: + settings.read(SettingsService.requireProfileSelectionOnOpen) && activeProfiles.hasMultipleProfiles, + ); + if (promptHandled) { + final selected = await Navigator.of( + context, + ).push(MaterialPageRoute(builder: (_) => const ProfileSwitchScreen(requireSelection: true))); + if (!mounted) return; + if (selected != true || activeProfiles.active == null) { + setState(() => _isAuthenticating = false); + return; + } + } + await context.read().initialize(); if (!mounted) return; - unawaited(Navigator.pushReplacement(context, fadeRoute(const MainScreen()))); + unawaited(Navigator.pushReplacement(context, fadeRoute(MainScreen(initialPromptHandled: promptHandled)))); } catch (e) { appLogger.e('Failed to connect to servers', error: e); if (!mounted) return; @@ -363,6 +386,16 @@ Profile? initialPlexHomeProfileFromCache(PlexHomeService plexHome, PlexAccountCo return Profile.virtualPlexHome(connectionId: accountConn.id, homeUser: users.single); } +@visibleForTesting +bool shouldPromptForInitialProfileSelection({ + required Profile? activeProfile, + required bool hasProfiles, + required bool accountHasHomeUsers, + required bool requireProfileSelectionOnOpen, +}) { + return requireProfileSelectionOnOpen || (activeProfile == null && (hasProfiles || accountHasHomeUsers)); +} + /// Stateful so the [TextEditingController] is disposed when the dialog /// closes — the previous inline `showDialog` builder created the /// controller in a closure and leaked it on every dismissal. diff --git a/test/profiles/active_profile_binder_test.dart b/test/profiles/active_profile_binder_test.dart index 0739e8a6..27d801f5 100644 --- a/test/profiles/active_profile_binder_test.dart +++ b/test/profiles/active_profile_binder_test.dart @@ -26,6 +26,7 @@ void main() { late MultiServerProvider multiServerProvider; late ActiveProfileBinder binder; late StorageService storage; + late bool shouldDeferInitialBind; setUp(() async { resetSharedPreferencesForTest(); @@ -48,6 +49,7 @@ void main() { ); manager = MultiServerManager(); multiServerProvider = MultiServerProvider(manager, DataAggregationService(manager)); + shouldDeferInitialBind = false; binder = ActiveProfileBinder( activeProfile: activeProfile, connections: connections, @@ -55,6 +57,7 @@ void main() { serverManager: manager, multiServerProvider: multiServerProvider, pinPrompt: (_, {String? errorMessage}) async => null, + shouldDeferInitialBind: (_) async => shouldDeferInitialBind, ); }); @@ -67,6 +70,14 @@ void main() { await db.close(); }); + Future createActiveLocalProfile(String id) async { + final profile = Profile(id: id, kind: ProfileKind.local, displayName: 'Owner', createdAt: DateTime(2026, 1, 1)); + await profiles.upsert(profile); + await storage.setActiveProfileId(profile.id); + await activeProfile.initialize(); + return profile; + } + test('local profile with no connections binds successfully with empty visibility', () async { final profile = Profile( id: 'local-owner', @@ -108,6 +119,31 @@ void main() { expect(notifications, lessThan(8)); }); + test('initial bind can be deferred until profile selection', () async { + final profile = await createActiveLocalProfile('local-deferred'); + shouldDeferInitialBind = true; + + await binder.rebindActive(); + + expect(activeProfile.lastBindingSucceeded, isTrue); + expect(activeProfile.isBinding, isFalse); + expect(binder.debugLastBoundProfileId, isNull); + expect(binder.consumeUserInitiatedActivation(profile.id), isFalse); + expect(multiServerProvider.serverIds, isEmpty); + }); + + test('user initiated activation bypasses initial bind defer', () async { + final profile = await createActiveLocalProfile('local-user-initiated'); + shouldDeferInitialBind = true; + binder.markUserInitiatedActivation(profile.id); + + await binder.rebindActive(); + + expect(activeProfile.lastBindingSucceeded, isTrue); + expect(binder.debugLastBoundProfileId, profile.id); + expect(binder.consumeUserInitiatedActivation(profile.id), isFalse); + }); + group('Plex Home token cache policy', () { test('protected cold start revalidates PIN even when profile selection is not required', () { expect(shouldUsePlexHomeTokenCache(preVerified: false, hasBoundOnce: false, plexProtected: true), isFalse); diff --git a/test/screens/auth_screen_test.dart b/test/screens/auth_screen_test.dart index f6ec74ce..0668d56b 100644 --- a/test/screens/auth_screen_test.dart +++ b/test/screens/auth_screen_test.dart @@ -61,4 +61,66 @@ void main() { expect(profile.parentConnectionId, account.id); expect(profile.displayName, 'Home User'); }); + + test('initial profile selection is required when home users exist but no profile is active', () { + expect( + shouldPromptForInitialProfileSelection( + activeProfile: null, + hasProfiles: false, + accountHasHomeUsers: true, + requireProfileSelectionOnOpen: false, + ), + isTrue, + ); + }); + + test('initial profile selection is skipped when a profile was auto-selected', () { + final profile = Profile( + id: 'local-owner', + kind: ProfileKind.local, + displayName: 'Owner', + createdAt: DateTime(2026, 1, 1), + ); + + expect( + shouldPromptForInitialProfileSelection( + activeProfile: profile, + hasProfiles: true, + accountHasHomeUsers: true, + requireProfileSelectionOnOpen: false, + ), + isFalse, + ); + }); + + test('initial profile selection is required when the launch setting is enabled', () { + final profile = Profile( + id: 'local-owner', + kind: ProfileKind.local, + displayName: 'Owner', + createdAt: DateTime(2026, 1, 1), + ); + + expect( + shouldPromptForInitialProfileSelection( + activeProfile: profile, + hasProfiles: true, + accountHasHomeUsers: true, + requireProfileSelectionOnOpen: true, + ), + isTrue, + ); + }); + + test('initial profile selection is skipped when no profiles are available', () { + expect( + shouldPromptForInitialProfileSelection( + activeProfile: null, + hasProfiles: false, + accountHasHomeUsers: false, + requireProfileSelectionOnOpen: false, + ), + isFalse, + ); + }); }