From a79aded94dad192f5fc9d735d293153d3c85f221 Mon Sep 17 00:00:00 2001 From: Austin Chang Date: Mon, 13 Jul 2026 14:51:26 +0800 Subject: [PATCH 1/5] fix(auth): unify USP login check so Remote Assistance can open Wi-Fi/Internet Settings (#1119) In Remote Assistance mode the WASM USP client is pre-authorized via authToken, so `usp.isAuthenticated` stays false by design. The Wi-Fi and Internet Settings providers gated their fetch on that raw transport flag, short-circuiting with "You are not signed in" even though the backend serves data fine. The router already guards all /usp routes on loginType (RA-aware) and 9 sibling feature providers have no such gate, making these two gates both redundant and wrong. - Add `AuthState.isRemoteAssistance` as the canonical login-intent check, replacing scattered `loginType == LoginType.remote` comparisons. - Add `uspAuthReadyProvider` as the single source of truth for "USP layer is authorized to serve data" (RA || usp.isAuthenticated), documenting that usp.isAuthenticated is non-reactive. - Remove the raw isAuthenticated fetch gate in the Wi-Fi and Internet Settings providers (keep the usp == null guard). This fixes #1119. - Converge dashboard_orchestrator, router_provider and the RA session guard onto the new getter; the orchestrator keeps direct usp.isAuthenticated reads because it observes restoreSession flips. - Tests: repurpose the internet "unauthenticated" test into an RA-bypass proof, add wifi RA-bypass + usp==null tests, and add unit tests for the new getter and provider. Follow-up (out of scope): session_service.dart and sse_bootstrap still read the raw flag but are off the RA render path. Co-Authored-By: Claude Opus 4.8 --- .../orchestrator/dashboard_orchestrator.dart | 10 ++- .../usp_internet_settings_notifier.dart | 10 ++- .../remote_assistance_session_guard.dart | 3 +- .../providers/usp_wifi_settings_provider.dart | 15 ++-- lib/providers/auth/auth_state.dart | 9 ++ .../auth/usp_auth_ready_provider.dart | 24 +++++ lib/route/router_provider.dart | 6 +- .../usp_internet_settings_notifier_test.dart | 33 ++++++- .../usp_wifi_settings_notifier_test.dart | 30 +++++++ test/providers/auth/auth_state_test.dart | 9 ++ .../auth/usp_auth_ready_provider_test.dart | 87 +++++++++++++++++++ 11 files changed, 211 insertions(+), 25 deletions(-) create mode 100644 lib/providers/auth/usp_auth_ready_provider.dart create mode 100644 test/providers/auth/usp_auth_ready_provider_test.dart diff --git a/lib/page/dashboard/orchestrator/dashboard_orchestrator.dart b/lib/page/dashboard/orchestrator/dashboard_orchestrator.dart index 25b824373..56e9cc8e0 100644 --- a/lib/page/dashboard/orchestrator/dashboard_orchestrator.dart +++ b/lib/page/dashboard/orchestrator/dashboard_orchestrator.dart @@ -128,9 +128,13 @@ class DashboardOrchestrator extends AsyncNotifier { } // Remote Assistance mode: client is pre-authenticated via authToken, - // skip local USP auth check. - final loginType = ref.read(authProvider).value?.loginType; - final isRemoteAssistance = loginType == LoginType.remote; + // skip local USP auth check. Note we keep the direct usp.isAuthenticated + // reads below rather than uspAuthReadyProvider: this code performs + // restoreSession and must observe the flag flip synchronously (that plain + // getter does not trigger Riverpod invalidation, so a cached provider read + // would be stale). + final isRemoteAssistance = + ref.read(authProvider).value?.isRemoteAssistance ?? false; // On page reload WASM state is lost — attempt session restore (local only). // Use isRecovering: true because we handle auth failure via NotAuthenticatedError diff --git a/lib/page/internet_settings/providers/usp_internet_settings_notifier.dart b/lib/page/internet_settings/providers/usp_internet_settings_notifier.dart index ef16135b1..93d9aabed 100644 --- a/lib/page/internet_settings/providers/usp_internet_settings_notifier.dart +++ b/lib/page/internet_settings/providers/usp_internet_settings_notifier.dart @@ -85,10 +85,12 @@ class UspInternetSettingsNotifier detail: 'USP service not available'); } - if (!usp.isAuthenticated) { - throw const ConnectivityError(detail: 'USP not authenticated'); - } - + // No auth gate here: the router already guards all /usp routes on + // loginType (RA-aware), and the WAN service surfaces real errors. The raw + // usp.isAuthenticated flag is a WASM transport signal that stays false in + // Remote Assistance (authToken bypass), so gating on it here wrongly + // blocked RA sessions (issue #1119). The usp == null gate above (and the + // service provider itself) still cover the "USP unavailable" case. final service = ref.read(uspInternetSettingsServiceProvider); final result = await service.fetchSettings(); diff --git a/lib/page/remote_assistance/views/remote_assistance_session_guard.dart b/lib/page/remote_assistance/views/remote_assistance_session_guard.dart index d753f18a0..6c9a8e58b 100644 --- a/lib/page/remote_assistance/views/remote_assistance_session_guard.dart +++ b/lib/page/remote_assistance/views/remote_assistance_session_guard.dart @@ -62,8 +62,7 @@ class _RemoteAssistanceSessionGuardState Future _checkAndRestoreSession() async { // Skip for Remote Assistance mode (CA side) - double check - final loginType = ref.read(authProvider).value?.loginType; - if (loginType == LoginType.remote) return; + if (ref.read(authProvider).value?.isRemoteAssistance ?? false) return; // Get device credentials from unified provider final credentials = ref.read(deviceCredentialsProvider); diff --git a/lib/page/wifi_settings/providers/usp_wifi_settings_provider.dart b/lib/page/wifi_settings/providers/usp_wifi_settings_provider.dart index b63e899de..cbe1e5313 100644 --- a/lib/page/wifi_settings/providers/usp_wifi_settings_provider.dart +++ b/lib/page/wifi_settings/providers/usp_wifi_settings_provider.dart @@ -75,15 +75,12 @@ class UspWifiSettingsNotifier extends AutoDisposeNotifier ); } - if (!usp.isAuthenticated) { - return ( - null, - WifiSettingsStatus( - error: const NotAuthenticatedError(detail: 'USP not authenticated'), - ) - ); - } - + // No auth gate here: the router already guards all /usp routes on + // loginType (RA-aware), and the WiFi data layer surfaces real errors. The + // raw usp.isAuthenticated flag is a WASM transport signal that stays false + // in Remote Assistance (authToken bypass), so gating on it here wrongly + // blocked RA sessions (issue #1119). The usp == null gate above still + // covers the "USP unavailable" case. logger.d('[USP][WiFi]: Fetching WiFi data...'); // Read from WiFi Data Provider (Layer 1) to avoid duplicate fetch. diff --git a/lib/providers/auth/auth_state.dart b/lib/providers/auth/auth_state.dart index c1d57a23e..eea842254 100644 --- a/lib/providers/auth/auth_state.dart +++ b/lib/providers/auth/auth_state.dart @@ -27,6 +27,15 @@ class AuthState extends Equatable { return const AuthState(loginType: LoginType.none); } + /// Whether this is a Remote Assistance session. + /// + /// In RA mode the WASM client is pre-authorized via authToken, so + /// [UspClient.isAuthenticated] stays false by design. RA must therefore be + /// detected from login intent (this getter), not from the client's login + /// state. This is the canonical replacement for scattered inline + /// `loginType == LoginType.remote` checks. + bool get isRemoteAssistance => loginType == LoginType.remote; + /// Creates an [AuthState] from a JSON map. factory AuthState.fromJson(Map json) { final loginType = diff --git a/lib/providers/auth/usp_auth_ready_provider.dart b/lib/providers/auth/usp_auth_ready_provider.dart new file mode 100644 index 000000000..0bf4d41b0 --- /dev/null +++ b/lib/providers/auth/usp_auth_ready_provider.dart @@ -0,0 +1,24 @@ +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:privacy_gui/core/usp/providers/usp_client_provider.dart'; +import 'package:privacy_gui/providers/auth/auth_provider.dart'; + +/// Single source of truth for "is the USP layer authorized to serve data". +/// +/// - Local login: `usp.isAuthenticated` is true. +/// - Remote Assistance: the client is pre-authorized via authToken, so +/// `usp.isAuthenticated` stays false by design; RA counts as ready via +/// [AuthState.isRemoteAssistance]. +/// +/// IMPORTANT: read this at the decision point (`ref.read` at fetch/guard time). +/// [UspClient.isAuthenticated] is a plain getter that does NOT trigger Riverpod +/// invalidation when it flips, so this provider is only recomputed when the +/// auth state or the client *instance* changes — not when `isAuthenticated` +/// flips underneath a stable client. Consumers that themselves mutate auth +/// state (e.g. the dashboard orchestrator's `restoreSession`) must re-read +/// `usp.isAuthenticated` directly rather than rely on this provider's cache. +final uspAuthReadyProvider = Provider((ref) { + final isRemoteAssistance = + ref.watch(authProvider).value?.isRemoteAssistance ?? false; + if (isRemoteAssistance) return true; + return ref.watch(uspClientProvider)?.isAuthenticated ?? false; +}); diff --git a/lib/route/router_provider.dart b/lib/route/router_provider.dart index 28f83e849..047fdbaa1 100644 --- a/lib/route/router_provider.dart +++ b/lib/route/router_provider.dart @@ -140,9 +140,9 @@ final routerProvider = Provider((ref) { // In Remote build mode, redirect to confirm page with restored session params. if (GlobalConfig.remote.isActive) { // If already connected (USP layer active), allow access - final loginType = - ref.read(authProvider.select((value) => value.value?.loginType)); - if (loginType == LoginType.remote) { + final isRemoteAssistance = ref.read(authProvider + .select((value) => value.value?.isRemoteAssistance ?? false)); + if (isRemoteAssistance) { return state.uri.toString(); } diff --git a/test/page/internet_settings/providers/usp_internet_settings_notifier_test.dart b/test/page/internet_settings/providers/usp_internet_settings_notifier_test.dart index 301afeacd..6fbda46df 100644 --- a/test/page/internet_settings/providers/usp_internet_settings_notifier_test.dart +++ b/test/page/internet_settings/providers/usp_internet_settings_notifier_test.dart @@ -421,9 +421,13 @@ void main() { container.dispose(); }); - test('fetch with unauthenticated service sets error status', () async { + // Regression: issue #1119. The provider must NOT gate its fetch on the raw + // usp.isAuthenticated flag — in Remote Assistance that flag stays false by + // design (authToken bypass), yet the data layer serves fine. Auth is + // enforced by the router, not here. + test('fetch proceeds when usp.isAuthenticated is false (RA bypass)', + () async { when(() => mockUsp.isAuthenticated).thenReturn(false); - when(() => mockAuthCoordinator.restoreSession()).thenAnswer((_) async {}); when(() => mockService.fetchSettings()) .thenAnswer((_) async => testFetchResult); @@ -431,8 +435,29 @@ void main() { await Future.delayed(Duration.zero); final state = container.read(uspInternetSettingsProvider); - // Should hit the restore path which won't succeed with our mock. - expect(state.status.error, isNotNull); + expect(state.status.error, isNull); + expect(state.status.isLoading, isFalse); + expect(state.settings.current.form.connectionType, + UspWanConnectionType.dhcp); + verify(() => mockService.fetchSettings()).called(1); + container.dispose(); + }); + + test('fetch throws ServiceNotInitializedError when usp is null', () async { + final container = ProviderContainer( + overrides: [ + uspClientProvider.overrideWithValue(null), + uspInternetSettingsServiceProvider.overrideWithValue(mockService), + uspMutationLockProvider.overrideWithValue(UspMutationLock()), + uspAuthCoordinatorProvider.overrideWithValue(mockAuthCoordinator), + sseManagerProvider.overrideWithValue(mockSseManager), + ], + ); + container.listen(uspInternetSettingsProvider, (_, __) {}); + await Future.delayed(Duration.zero); + + final state = container.read(uspInternetSettingsProvider); + expect(state.status.error, isA()); container.dispose(); }); }); diff --git a/test/page/wifi_settings/providers/usp_wifi_settings_notifier_test.dart b/test/page/wifi_settings/providers/usp_wifi_settings_notifier_test.dart index 7053601cf..f3f589e82 100644 --- a/test/page/wifi_settings/providers/usp_wifi_settings_notifier_test.dart +++ b/test/page/wifi_settings/providers/usp_wifi_settings_notifier_test.dart @@ -137,6 +137,36 @@ void main() { container.dispose(); }); + // Regression: issue #1119. The provider must NOT gate its fetch on the raw + // usp.isAuthenticated flag — in Remote Assistance that flag stays false by + // design (authToken bypass), yet the WiFi data layer serves fine. Auth is + // enforced by the router, not here. + test('fetch proceeds when usp.isAuthenticated is false (RA bypass)', + () async { + when(() => mockUsp.isAuthenticated).thenReturn(false); + final networks = WifiSettingsTestData.createNetworks(); + when(() => mockService.buildWifiNetworks( + ssids: any(named: 'ssids'), + accessPoints: any(named: 'accessPoints'), + radios: any(named: 'radios'), + )).thenReturn(networks); + when(() => mockService.buildQuickSetupNetworks(any())).thenReturn(( + main: WifiSettingsTestData.createQuickSetupAggregate(), + guest: null, + isQuickSetup: true, + )); + + final container = createContainer(); + await Future.delayed(Duration.zero); + await Future.delayed(Duration.zero); + + final state = container.read(uspWifiSettingsProvider); + expect(state.status.error, isNull); + expect(state.settings.current.networks, hasLength(3)); + expect(state.status.isLoading, isFalse); + container.dispose(); + }); + // ----------------------------------------------------------------------- // updateNetworkField // ----------------------------------------------------------------------- diff --git a/test/providers/auth/auth_state_test.dart b/test/providers/auth/auth_state_test.dart index d94e64ab2..2cc7077d6 100644 --- a/test/providers/auth/auth_state_test.dart +++ b/test/providers/auth/auth_state_test.dart @@ -62,5 +62,14 @@ void main() { final b = AuthState(localPasswordHint: 'y', loginType: LoginType.local); expect(a, isNot(b)); }); + + test('isRemoteAssistance is true only for LoginType.remote', () { + expect(const AuthState(loginType: LoginType.none).isRemoteAssistance, + isFalse); + expect(const AuthState(loginType: LoginType.local).isRemoteAssistance, + isFalse); + expect(const AuthState(loginType: LoginType.remote).isRemoteAssistance, + isTrue); + }); }); } diff --git a/test/providers/auth/usp_auth_ready_provider_test.dart b/test/providers/auth/usp_auth_ready_provider_test.dart new file mode 100644 index 000000000..23e1b6727 --- /dev/null +++ b/test/providers/auth/usp_auth_ready_provider_test.dart @@ -0,0 +1,87 @@ +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:mocktail/mocktail.dart'; +import 'package:privacy_gui/core/usp/providers/usp_client_provider.dart'; +import 'package:privacy_gui/core/usp/services/usp_client.dart'; +import 'package:privacy_gui/providers/auth/auth_provider.dart'; +import 'package:privacy_gui/providers/auth/usp_auth_ready_provider.dart'; + +class MockUspClient extends Mock implements UspClient {} + +class _AuthNotifier extends AsyncNotifier + with Mock + implements AuthNotifier { + _AuthNotifier(this._loginType); + + final LoginType _loginType; + + @override + Future build() async => AuthState(loginType: _loginType); +} + +void main() { + late MockUspClient mockUsp; + + setUp(() { + mockUsp = MockUspClient(); + }); + + ProviderContainer createContainer({ + required LoginType loginType, + UspClient? usp, + }) { + final container = ProviderContainer( + overrides: [ + authProvider.overrideWith(() => _AuthNotifier(loginType)), + uspClientProvider.overrideWithValue(usp), + ], + ); + addTearDown(container.dispose); + return container; + } + + Future readReady(ProviderContainer container) async { + // authProvider.build is async — let it settle before reading. + await container.read(authProvider.future); + return container.read(uspAuthReadyProvider); + } + + group('uspAuthReadyProvider', () { + test('RA session is ready even when usp.isAuthenticated is false (#1119)', + () async { + when(() => mockUsp.isAuthenticated).thenReturn(false); + final container = + createContainer(loginType: LoginType.remote, usp: mockUsp); + + expect(await readReady(container), isTrue); + }); + + test('local login with authenticated client is ready', () async { + when(() => mockUsp.isAuthenticated).thenReturn(true); + final container = + createContainer(loginType: LoginType.local, usp: mockUsp); + + expect(await readReady(container), isTrue); + }); + + test('logged out with unauthenticated client is not ready', () async { + when(() => mockUsp.isAuthenticated).thenReturn(false); + final container = + createContainer(loginType: LoginType.none, usp: mockUsp); + + expect(await readReady(container), isFalse); + }); + + test('null client, non-RA is not ready', () async { + final container = createContainer(loginType: LoginType.local, usp: null); + + expect(await readReady(container), isFalse); + }); + + test('null client, RA is still ready (authToken bypass)', () async { + final container = createContainer(loginType: LoginType.remote, usp: null); + + expect(await readReady(container), isTrue); + }); + }); +} From a74e62c8171bd2d737735f519408c1cc1e111ed0 Mon Sep 17 00:00:00 2001 From: Austin Chang Date: Mon, 13 Jul 2026 16:36:28 +0800 Subject: [PATCH 2/5] =?UTF-8?q?refactor(auth):=20drop=20unused=20uspAuthRe?= =?UTF-8?q?adyProvider=20=E2=80=94=20auth=20stays=20a=20router=20concern?= =?UTF-8?q?=20(PR=20#1122=20review=20W-1)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The provider was introduced as a "single source of truth" for USP auth readiness but ended up with zero production consumers: wifi/internet fixed #1119 by removing their fetch gate (relying on the router guard) rather than routing through the provider, so it was dead code. Keeping it would also push the auth concern back into page providers (each page would ref.read it), which is the coupling we want to avoid. Auth is a navigation-layer concern: the router already gates every /usp route on loginType (RA-aware), so pages render data without an auth dependency — matching the 9 sibling USP settings providers that never had a gate. - Delete usp_auth_ready_provider.dart and its test. - Drop the stale provider reference from the orchestrator comment. - Keep AuthState.isRemoteAssistance: it is consumed by the router (the auth boundary) and by RA-strategy decisions in the orchestrator/session guard, not as a per-page login gate. Co-Authored-By: Claude Opus 4.8 --- .../orchestrator/dashboard_orchestrator.dart | 6 +- .../auth/usp_auth_ready_provider.dart | 24 ----- .../auth/usp_auth_ready_provider_test.dart | 87 ------------------- 3 files changed, 1 insertion(+), 116 deletions(-) delete mode 100644 lib/providers/auth/usp_auth_ready_provider.dart delete mode 100644 test/providers/auth/usp_auth_ready_provider_test.dart diff --git a/lib/page/dashboard/orchestrator/dashboard_orchestrator.dart b/lib/page/dashboard/orchestrator/dashboard_orchestrator.dart index 56e9cc8e0..9ae3ebb23 100644 --- a/lib/page/dashboard/orchestrator/dashboard_orchestrator.dart +++ b/lib/page/dashboard/orchestrator/dashboard_orchestrator.dart @@ -128,11 +128,7 @@ class DashboardOrchestrator extends AsyncNotifier { } // Remote Assistance mode: client is pre-authenticated via authToken, - // skip local USP auth check. Note we keep the direct usp.isAuthenticated - // reads below rather than uspAuthReadyProvider: this code performs - // restoreSession and must observe the flag flip synchronously (that plain - // getter does not trigger Riverpod invalidation, so a cached provider read - // would be stale). + // skip local USP auth check. final isRemoteAssistance = ref.read(authProvider).value?.isRemoteAssistance ?? false; diff --git a/lib/providers/auth/usp_auth_ready_provider.dart b/lib/providers/auth/usp_auth_ready_provider.dart deleted file mode 100644 index 0bf4d41b0..000000000 --- a/lib/providers/auth/usp_auth_ready_provider.dart +++ /dev/null @@ -1,24 +0,0 @@ -import 'package:flutter_riverpod/flutter_riverpod.dart'; -import 'package:privacy_gui/core/usp/providers/usp_client_provider.dart'; -import 'package:privacy_gui/providers/auth/auth_provider.dart'; - -/// Single source of truth for "is the USP layer authorized to serve data". -/// -/// - Local login: `usp.isAuthenticated` is true. -/// - Remote Assistance: the client is pre-authorized via authToken, so -/// `usp.isAuthenticated` stays false by design; RA counts as ready via -/// [AuthState.isRemoteAssistance]. -/// -/// IMPORTANT: read this at the decision point (`ref.read` at fetch/guard time). -/// [UspClient.isAuthenticated] is a plain getter that does NOT trigger Riverpod -/// invalidation when it flips, so this provider is only recomputed when the -/// auth state or the client *instance* changes — not when `isAuthenticated` -/// flips underneath a stable client. Consumers that themselves mutate auth -/// state (e.g. the dashboard orchestrator's `restoreSession`) must re-read -/// `usp.isAuthenticated` directly rather than rely on this provider's cache. -final uspAuthReadyProvider = Provider((ref) { - final isRemoteAssistance = - ref.watch(authProvider).value?.isRemoteAssistance ?? false; - if (isRemoteAssistance) return true; - return ref.watch(uspClientProvider)?.isAuthenticated ?? false; -}); diff --git a/test/providers/auth/usp_auth_ready_provider_test.dart b/test/providers/auth/usp_auth_ready_provider_test.dart deleted file mode 100644 index 23e1b6727..000000000 --- a/test/providers/auth/usp_auth_ready_provider_test.dart +++ /dev/null @@ -1,87 +0,0 @@ -import 'package:flutter_riverpod/flutter_riverpod.dart'; -import 'package:flutter_test/flutter_test.dart'; -import 'package:mocktail/mocktail.dart'; -import 'package:privacy_gui/core/usp/providers/usp_client_provider.dart'; -import 'package:privacy_gui/core/usp/services/usp_client.dart'; -import 'package:privacy_gui/providers/auth/auth_provider.dart'; -import 'package:privacy_gui/providers/auth/usp_auth_ready_provider.dart'; - -class MockUspClient extends Mock implements UspClient {} - -class _AuthNotifier extends AsyncNotifier - with Mock - implements AuthNotifier { - _AuthNotifier(this._loginType); - - final LoginType _loginType; - - @override - Future build() async => AuthState(loginType: _loginType); -} - -void main() { - late MockUspClient mockUsp; - - setUp(() { - mockUsp = MockUspClient(); - }); - - ProviderContainer createContainer({ - required LoginType loginType, - UspClient? usp, - }) { - final container = ProviderContainer( - overrides: [ - authProvider.overrideWith(() => _AuthNotifier(loginType)), - uspClientProvider.overrideWithValue(usp), - ], - ); - addTearDown(container.dispose); - return container; - } - - Future readReady(ProviderContainer container) async { - // authProvider.build is async — let it settle before reading. - await container.read(authProvider.future); - return container.read(uspAuthReadyProvider); - } - - group('uspAuthReadyProvider', () { - test('RA session is ready even when usp.isAuthenticated is false (#1119)', - () async { - when(() => mockUsp.isAuthenticated).thenReturn(false); - final container = - createContainer(loginType: LoginType.remote, usp: mockUsp); - - expect(await readReady(container), isTrue); - }); - - test('local login with authenticated client is ready', () async { - when(() => mockUsp.isAuthenticated).thenReturn(true); - final container = - createContainer(loginType: LoginType.local, usp: mockUsp); - - expect(await readReady(container), isTrue); - }); - - test('logged out with unauthenticated client is not ready', () async { - when(() => mockUsp.isAuthenticated).thenReturn(false); - final container = - createContainer(loginType: LoginType.none, usp: mockUsp); - - expect(await readReady(container), isFalse); - }); - - test('null client, non-RA is not ready', () async { - final container = createContainer(loginType: LoginType.local, usp: null); - - expect(await readReady(container), isFalse); - }); - - test('null client, RA is still ready (authToken bypass)', () async { - final container = createContainer(loginType: LoginType.remote, usp: null); - - expect(await readReady(container), isTrue); - }); - }); -} From 04d0e44b2e5691a25f8971579ac17ba96790bb80 Mon Sep 17 00:00:00 2001 From: Austin Chang Date: Mon, 13 Jul 2026 16:48:35 +0800 Subject: [PATCH 3/5] refactor(auth): add AuthState.isLoggedIn and converge login checks (PR #1122 review W-4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The app could identify the login *kind* (isRemoteAssistance) but had no canonical "is the user logged in" check — that was scattered as inline `loginType == LoginType.none` comparisons across 8 sites. Adding isLoggedIn completes the pair: isLoggedIn answers "logged in?", isRemoteAssistance answers "which kind?". - Add AuthState.isLoggedIn => loginType != LoginType.none, named to avoid confusion with the transport-layer usp.isAuthenticated (WASM flag). - Migrate the none-checks in app, router redirect, connection state, top bar, login view, root container and general settings widget to isLoggedIn. - One router site (redirectLogic) keeps the loginType local because it reuses the enum value later, not just the logged-in boolean. - Add isLoggedIn truth-table test. Co-Authored-By: Claude Opus 4.8 --- lib/app.dart | 3 +-- lib/components/layouts/root_container.dart | 2 +- .../general_settings_widget/general_settings_widget.dart | 8 +++----- .../providers/app_connection_state_provider.dart | 3 +-- lib/page/login/views/login_local_view.dart | 3 +-- lib/page/shell/usp_top_bar.dart | 5 ++--- lib/providers/auth/auth_state.dart | 9 +++++++++ lib/route/router_provider.dart | 8 ++++---- test/providers/auth/auth_state_test.dart | 6 ++++++ 9 files changed, 28 insertions(+), 19 deletions(-) diff --git a/lib/app.dart b/lib/app.dart index 78e0a5412..a96ced3b9 100644 --- a/lib/app.dart +++ b/lib/app.dart @@ -233,8 +233,7 @@ class _LinksysAppState extends ConsumerState } void _tryResumeSse() { - final loginType = ref.read(authProvider).value?.loginType; - if (loginType == null || loginType == LoginType.none) return; + if (!(ref.read(authProvider).value?.isLoggedIn ?? false)) return; final sseManager = ref.read(sseManagerProvider); if (sseManager == null) return; final sseState = sseManager.connection.connectionState.value; diff --git a/lib/components/layouts/root_container.dart b/lib/components/layouts/root_container.dart index b94346dea..352b307fa 100644 --- a/lib/components/layouts/root_container.dart +++ b/lib/components/layouts/root_container.dart @@ -45,7 +45,7 @@ class _AppRootContainerState extends ConsumerState { return; } // not log in yet - if (ref.read(authProvider).value?.loginType == LoginType.none) { + if (!(ref.read(authProvider).value?.isLoggedIn ?? false)) { return; } // not go into dashboard yet diff --git a/lib/components/styled/general_settings_widget/general_settings_widget.dart b/lib/components/styled/general_settings_widget/general_settings_widget.dart index ce9010025..3270badcf 100644 --- a/lib/components/styled/general_settings_widget/general_settings_widget.dart +++ b/lib/components/styled/general_settings_widget/general_settings_widget.dart @@ -25,9 +25,8 @@ class GeneralSettingsWidget extends ConsumerStatefulWidget { class _GeneralSettingsWidgetState extends ConsumerState { @override Widget build(BuildContext context) { - final loginType = - ref.watch(authProvider.select((state) => state.value?.loginType)) ?? - LoginType.none; + final isLoggedIn = ref.watch( + authProvider.select((state) => state.value?.isLoggedIn ?? false)); // Watch Theme.of(context) to trigger rebuild when global theme changes Theme.of(context); @@ -110,8 +109,7 @@ class _GeneralSettingsWidgetState extends ConsumerState { ), // Legal links and logout (hidden in remote mode) - if (!GlobalConfig.remote.isActive && - loginType != LoginType.none) ...[ + if (!GlobalConfig.remote.isActive && isLoggedIn) ...[ AppGap.md(), const AppDivider(), AppGap.md(), diff --git a/lib/core/connection/providers/app_connection_state_provider.dart b/lib/core/connection/providers/app_connection_state_provider.dart index 5df20194b..4630ebb82 100644 --- a/lib/core/connection/providers/app_connection_state_provider.dart +++ b/lib/core/connection/providers/app_connection_state_provider.dart @@ -63,8 +63,7 @@ class AppConnectionStateNotifier extends Notifier { ref.listen(authProvider, (_, next) { if (next.isLoading) return; - final loginType = next.value?.loginType; - if (loginType == null || loginType == LoginType.none) { + if (!(next.value?.isLoggedIn ?? false)) { _probeTimer?.cancel(); _probeTimer = null; _cooldownTimer?.cancel(); diff --git a/lib/page/login/views/login_local_view.dart b/lib/page/login/views/login_local_view.dart index d11eb5c5e..4c71f7487 100644 --- a/lib/page/login/views/login_local_view.dart +++ b/lib/page/login/views/login_local_view.dart @@ -107,8 +107,7 @@ class _LoginViewState extends ConsumerState { previous.isLoading && next.hasValue && !next.hasError) { - final loginType = next.value?.loginType; - if (loginType != null && loginType != LoginType.none) { + if (next.value?.isLoggedIn ?? false) { if (!context.mounted) return; context.go('/'); } diff --git a/lib/page/shell/usp_top_bar.dart b/lib/page/shell/usp_top_bar.dart index 45895b3d5..786425c3d 100644 --- a/lib/page/shell/usp_top_bar.dart +++ b/lib/page/shell/usp_top_bar.dart @@ -70,9 +70,8 @@ class _UspTopBarState extends ConsumerState with DebugObserver { Row( mainAxisSize: MainAxisSize.min, children: [ - if (ref.watch(authProvider.select((v) => - v.value?.loginType != null && - v.value?.loginType != LoginType.none)) && + if (ref.watch(authProvider + .select((v) => v.value?.isLoggedIn ?? false)) && (ref.watch(appsCapabilityProvider).valueOrNull ?? false)) Tooltip( diff --git a/lib/providers/auth/auth_state.dart b/lib/providers/auth/auth_state.dart index eea842254..ee30297bc 100644 --- a/lib/providers/auth/auth_state.dart +++ b/lib/providers/auth/auth_state.dart @@ -27,6 +27,15 @@ class AuthState extends Equatable { return const AuthState(loginType: LoginType.none); } + /// Whether the user is logged in, by either a local or a Remote Assistance + /// session. + /// + /// This is login *intent* — distinct from the transport-layer + /// [UspClient.isAuthenticated] (a WASM flag that stays false in RA mode). + /// Canonical replacement for scattered inline `loginType != LoginType.none` + /// checks. Use [isRemoteAssistance] when the login *kind* matters. + bool get isLoggedIn => loginType != LoginType.none; + /// Whether this is a Remote Assistance session. /// /// In RA mode the WASM client is pre-authorized via authToken, so diff --git a/lib/route/router_provider.dart b/lib/route/router_provider.dart index 047fdbaa1..a7c43ec05 100644 --- a/lib/route/router_provider.dart +++ b/lib/route/router_provider.dart @@ -159,9 +159,9 @@ final routerProvider = Provider((ref) { logger.i('[Route]: Remote mode no session, redirecting to RA page'); return RoutePath.remoteAssistanceConfirm; } - final loginType = - ref.watch(authProvider.select((value) => value.value?.loginType)); - if (loginType == null || loginType == LoginType.none) { + final isLoggedIn = ref.watch( + authProvider.select((value) => value.value?.isLoggedIn ?? false)); + if (!isLoggedIn) { return router._home(); } return state.uri.toString(); @@ -221,7 +221,7 @@ class RouterNotifier extends ChangeNotifier { final loginType = _ref.watch(authProvider.select((data) => data.value?.loginType)); - // if have no login type and navigate into dashboard, then back to home + // if not logged in and navigate into dashboard, then back to home if ((loginType == null || loginType == LoginType.none) && (state.matchedLocation.startsWith('/dashboard') || state.matchedLocation.startsWith('/usp'))) { diff --git a/test/providers/auth/auth_state_test.dart b/test/providers/auth/auth_state_test.dart index 2cc7077d6..7dacb722d 100644 --- a/test/providers/auth/auth_state_test.dart +++ b/test/providers/auth/auth_state_test.dart @@ -63,6 +63,12 @@ void main() { expect(a, isNot(b)); }); + test('isLoggedIn is true for local and remote, false for none', () { + expect(const AuthState(loginType: LoginType.none).isLoggedIn, isFalse); + expect(const AuthState(loginType: LoginType.local).isLoggedIn, isTrue); + expect(const AuthState(loginType: LoginType.remote).isLoggedIn, isTrue); + }); + test('isRemoteAssistance is true only for LoginType.remote', () { expect(const AuthState(loginType: LoginType.none).isRemoteAssistance, isFalse); From d957f2371916e09377323595b691a5d87273d94b Mon Sep 17 00:00:00 2001 From: Austin Chang Date: Mon, 13 Jul 2026 16:56:22 +0800 Subject: [PATCH 4/5] docs(auth): clarify loginType is the source of truth, getters are shortcuts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Document that AuthState.loginType is the single three-state source of truth and that isLoggedIn / isRemoteAssistance are derived named shortcuts, not a separate mechanism — so readers know when to use the enum vs the booleans. Co-Authored-By: Claude Opus 4.8 --- lib/providers/auth/auth_state.dart | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/lib/providers/auth/auth_state.dart b/lib/providers/auth/auth_state.dart index ee30297bc..52738dac6 100644 --- a/lib/providers/auth/auth_state.dart +++ b/lib/providers/auth/auth_state.dart @@ -14,7 +14,13 @@ class AuthState extends Equatable { /// Password hint for the local router admin password. final String? localPasswordHint; - /// Current login type indicating authentication method. + /// The single source of truth for login state: `none` / `local` / `remote`. + /// + /// The [isLoggedIn] and [isRemoteAssistance] getters are just named shortcuts + /// derived from this value for the two most common yes/no questions — they are + /// not a separate mechanism. Read [loginType] directly only when the full + /// three-state value is needed (e.g. detecting a transition between kinds, or + /// branching specifically on `local`); prefer the getters otherwise. final LoginType loginType; const AuthState({ @@ -27,21 +33,21 @@ class AuthState extends Equatable { return const AuthState(loginType: LoginType.none); } - /// Whether the user is logged in, by either a local or a Remote Assistance - /// session. + /// Whether the user is logged in (local OR Remote Assistance). /// - /// This is login *intent* — distinct from the transport-layer - /// [UspClient.isAuthenticated] (a WASM flag that stays false in RA mode). - /// Canonical replacement for scattered inline `loginType != LoginType.none` - /// checks. Use [isRemoteAssistance] when the login *kind* matters. + /// Shortcut for `loginType != LoginType.none`. This is login *intent* — + /// distinct from the transport-layer [UspClient.isAuthenticated] (a WASM flag + /// that stays false in RA mode). Canonical replacement for scattered inline + /// `loginType != LoginType.none` checks; use [isRemoteAssistance] when the + /// login *kind* matters. bool get isLoggedIn => loginType != LoginType.none; /// Whether this is a Remote Assistance session. /// - /// In RA mode the WASM client is pre-authorized via authToken, so - /// [UspClient.isAuthenticated] stays false by design. RA must therefore be - /// detected from login intent (this getter), not from the client's login - /// state. This is the canonical replacement for scattered inline + /// Shortcut for `loginType == LoginType.remote`. In RA mode the WASM client + /// is pre-authorized via authToken, so [UspClient.isAuthenticated] stays false + /// by design — RA must be detected from login intent (this getter), not from + /// the client's login state. Canonical replacement for scattered inline /// `loginType == LoginType.remote` checks. bool get isRemoteAssistance => loginType == LoginType.remote; From 2cdbe91ea82737b7440b8426104cafd0d496cfa1 Mon Sep 17 00:00:00 2001 From: Austin Chang Date: Mon, 13 Jul 2026 16:59:43 +0800 Subject: [PATCH 5/5] test(auth): use const literal in fromJson test to clear analyzer info Fixes the pre-existing prefer_const_literals_to_create_immutables hint on the empty-map fromJson case, now that this file is touched by the auth getter tests. Co-Authored-By: Claude Opus 4.8 --- test/providers/auth/auth_state_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/providers/auth/auth_state_test.dart b/test/providers/auth/auth_state_test.dart index 7dacb722d..a414fb104 100644 --- a/test/providers/auth/auth_state_test.dart +++ b/test/providers/auth/auth_state_test.dart @@ -47,7 +47,7 @@ void main() { }); test('fromJson defaults to LoginType.none for missing type', () { - final state = AuthState.fromJson({}); + final state = AuthState.fromJson(const {}); expect(state.loginType, LoginType.none); });