diff --git a/.claude/context/architecture.md b/.claude/context/architecture.md index 0ac3fad..7a18964 100644 --- a/.claude/context/architecture.md +++ b/.claude/context/architecture.md @@ -57,8 +57,8 @@ new provider instance): saved-accounts list, which one is active, `sessionGeneration` — see below), auth/login-flow state (`isLoggedIn`, `isRestoringSession`, `loginFlowStatus`), the active `NextcloudService` instance, and login lock - (`loginLockEnabled`/`lockAccountSwitching`/`lockHiddenFiles`/ - `needsUnlock`/`passGate`). Exposes `addAccountClearedListener`/ + (`loginLockEnabled`/`lockAccountSwitching`/`lockHiddenFiles` - three + independent locks - plus `needsUnlock`/`passGate`). Exposes `addAccountClearedListener`/ `addAccountActivatedListener` (plain `List`) so sibling controllers — constructed after `SessionController` and unable to hold a forward reference to it — can react to login/logout/account-switch diff --git a/.claude/context/server.md b/.claude/context/server.md index ddac4dc..74f0e99 100644 --- a/.claude/context/server.md +++ b/.claude/context/server.md @@ -798,6 +798,19 @@ plugin's own manifest via merge, but kept explicit here too). own requirement) via `maxOf(24, flutter.minSdkVersion)` rather than trusting Flutter's own default to already be high enough. +**The three locks are independent** (Settings -> Security; +`SessionController`): `loginLockEnabled` (unlock to open the app - the only one +`needsUnlock`/the lock screen look at), `lockAccountSwitching` (unlock to +switch accounts) and `lockHiddenFiles` (unlock to turn on showing hidden +files). Each gate works with the others off - `passGate(gate, reason)` +prompts whenever its own `gate` is on, whether or not login lock is. Turning +any of them on **or off** needs a successful `AppLockService.authenticate` +(on also needs `isDeviceSupported`, so a gate nobody can pass can't be set; +off needs it so someone with a momentarily unlocked phone can't remove it), +and `disableLoginLock` leaves the other two untouched. The old master/sub-toggle +dependency is gone. Tests replace the prompt through +`AppLockService.debugAuthenticate`/`debugIsDeviceSupported`. + `isRestoringSession` still gates the splash screen until the above resolves — see `standards.md` for why widget tests must mock both storage channels rather than relying on this async path throwing naturally. @@ -903,16 +916,16 @@ Reminders/Notes - the destination is picked *inside* the sheet: setting of their own. `hide` (default) drops dot-folders, `only` lists just them, `include` lists everything; a folder is hidden when it *or any ancestor* starts with a dot (`HiddenFilter`, same rule as the app). - When the filter isn't `hide` and the app has login lock + "lock hidden - files" on, the sheet asks for Face ID/passcode first (`DeviceAuth`, the + When the filter isn't `hide` and the app's "lock hidden files" is on, + the sheet asks for Face ID/passcode first (`DeviceAuth`, the `.deviceOwnerAuthentication` policy - biometrics with passcode fallback, like `local_auth` with `biometricOnly: false`); cancelling falls back to hiding them, with a note. - **Account switching**: choosing any account other than the app's - active one asks for the same unlock when login lock + "lock account - switching" are on. One successful unlock covers the rest of that sheet. + active one asks for the same unlock when "lock account switching" is on. One successful unlock covers the rest of that sheet. - Opening the sheet itself is not gated - only these two actions are - (as in the app, where login lock guards launch and these toggles). + (as in the app, where login lock guards launch and these are separate + locks). 3. **Upload** calls `ShareUpload.enqueue`: one `PUT` per file on a *background* `URLSession` (`dev.ayushya.noo.transfers.share`, with `sharedContainerIdentifier`) that outlives the extension, and posts diff --git a/ios/RunnerTests/RunnerTests.swift b/ios/RunnerTests/RunnerTests.swift index ff1b899..f390cdd 100644 --- a/ios/RunnerTests/RunnerTests.swift +++ b/ios/RunnerTests/RunnerTests.swift @@ -204,18 +204,17 @@ class RunnerTests: XCTestCase { XCTAssertEqual(shared.active?.id, "alice") } - func testUnlockRulesNeedTheMasterLockAndTheirOwnToggle() { + func testUnlockRulesAreIndependentOfLoginLock() { func rules(master: Bool, switching: Bool, hidden: Bool) -> (Bool, Bool) { let shared = SharedAccounts( accounts: [account("a")], activeId: "a", loginLockEnabled: master, lockAccountSwitching: switching, lockHiddenFiles: hidden) return (shared.needsUnlockToSwitchAccount, shared.needsUnlockForHidden) } - XCTAssertTrue(rules(master: true, switching: true, hidden: false) == (true, false)) - XCTAssertTrue(rules(master: true, switching: false, hidden: true) == (false, true)) - XCTAssertTrue( - rules(master: false, switching: true, hidden: true) == (false, false), - "the sub-toggles mean nothing while the login lock is off, as in the app") + XCTAssertTrue(rules(master: false, switching: true, hidden: false) == (true, false)) + XCTAssertTrue(rules(master: false, switching: false, hidden: true) == (false, true)) + XCTAssertTrue(rules(master: true, switching: false, hidden: false) == (false, false)) + XCTAssertTrue(rules(master: true, switching: true, hidden: true) == (true, true)) } // MARK: - TransferBatchStore diff --git a/ios/Shared/SharedAccount.swift b/ios/Shared/SharedAccount.swift index bf98259..2423ff8 100644 --- a/ios/Shared/SharedAccount.swift +++ b/ios/Shared/SharedAccount.swift @@ -23,8 +23,9 @@ struct SharedAccount: Codable, Equatable, Identifiable { struct SharedAccounts: Codable, Equatable { var accounts: [SharedAccount] var activeId: String? - /// Settings -> Security: the master "login lock" and its two sub-toggles. - /// The sub-toggles only count while the master one is on (as in the app). + /// Settings -> Security: the three independent locks. `loginLockEnabled` + /// (unlock to open the app) is carried for completeness; the extension's own + /// gates are the other two. var loginLockEnabled: Bool var lockAccountSwitching: Bool var lockHiddenFiles: Bool @@ -36,11 +37,11 @@ struct SharedAccounts: Codable, Equatable { /// Uploading to an account other than the active one is "switching" in /// the app's terms, so it needs the same unlock. - var needsUnlockToSwitchAccount: Bool { loginLockEnabled && lockAccountSwitching } + var needsUnlockToSwitchAccount: Bool { lockAccountSwitching } /// Showing hidden folders needs the same unlock the app asks for when you /// turn hidden files on. - var needsUnlockForHidden: Bool { loginLockEnabled && lockHiddenFiles } + var needsUnlockForHidden: Bool { lockHiddenFiles } } /// Keeps the [SharedAccounts] in a Keychain access group both the app and the diff --git a/lib/providers/session_controller.dart b/lib/providers/session_controller.dart index 2504d80..42f8636 100644 --- a/lib/providers/session_controller.dart +++ b/lib/providers/session_controller.dart @@ -516,7 +516,7 @@ class SessionController extends ChangeNotifier with WidgetsBindingObserver { /// session-restore that failed, e.g. transient network trouble at cold /// start), this still retries rather than no-op, since that's exactly the /// case LoginView's "Continue as" tile exists to recover from. Gated - /// behind login lock when [lockAccountSwitching] is on. Returns whether + /// behind its own unlock when [lockAccountSwitching] is on. Returns whether /// the account ended up logged in, so callers (LoginView's saved-account /// tile) can surface a failure - e.g. a stored app password that no /// longer works and needs the account removed/re-added. @@ -640,10 +640,10 @@ class SessionController extends ChangeNotifier with WidgetsBindingObserver { return true; } - /// Turns login lock off, along with both of its sub-toggles (meaningless - /// once the base lock is gone). Requires a successful auth first, same as - /// turning it on - otherwise anyone with momentary access to an unlocked - /// phone could just switch it off. + /// Turns login lock off. Requires a successful auth first, same as turning + /// it on - otherwise anyone with momentary access to an unlocked phone + /// could just switch it off. Only this lock: the account-switching and + /// hidden-files locks are independent and stay as they are. Future disableLoginLock() async { if (!_loginLockEnabled) return true; final confirmed = await AppLockService.authenticate( @@ -651,30 +651,55 @@ class SessionController extends ChangeNotifier with WidgetsBindingObserver { ); if (!confirmed) return false; _loginLockEnabled = false; - _lockAccountSwitching = false; - _lockHiddenFiles = false; _isUnlocked = false; notifyListeners(); - prefsFuture.then((p) { - p.setBool(_prefLoginLockEnabled, false); - p.setBool(_prefLockAccountSwitching, false); - p.setBool(_prefLockHiddenFiles, false); - }); + prefsFuture.then((p) => p.setBool(_prefLoginLockEnabled, false)); return true; } - void setLockAccountSwitching(bool value) { - if (!_loginLockEnabled) return; + /// Turns the "unlock to switch accounts" gate on or off. Independent of + /// login lock. Both directions need a successful auth - enabling proves the + /// device can authenticate at all (a gate nobody can pass would lock the + /// user out of switching), disabling stops anyone with momentary access to + /// an unlocked phone from just removing it. Returns whether it changed. + Future setLockAccountSwitching(bool value) async { + if (value == _lockAccountSwitching) return true; + if (!await _confirmLockChange( + value, + 'Confirm to lock account switching', + 'Confirm to unlock account switching', + )) { + return false; + } _lockAccountSwitching = value; notifyListeners(); prefsFuture.then((p) => p.setBool(_prefLockAccountSwitching, value)); + return true; } - void setLockHiddenFiles(bool value) { - if (!_loginLockEnabled) return; + /// Same as [setLockAccountSwitching], for revealing hidden files. + Future setLockHiddenFiles(bool value) async { + if (value == _lockHiddenFiles) return true; + if (!await _confirmLockChange( + value, + 'Confirm to lock hidden files', + 'Confirm to unlock hidden files', + )) { + return false; + } _lockHiddenFiles = value; notifyListeners(); prefsFuture.then((p) => p.setBool(_prefLockHiddenFiles, value)); + return true; + } + + Future _confirmLockChange( + bool enabling, + String enableReason, + String disableReason, + ) async { + if (enabling && !await AppLockService.isDeviceSupported()) return false; + return AppLockService.authenticate(enabling ? enableReason : disableReason); } /// Called by the lock screen. Returns whether it actually unlocked. @@ -687,12 +712,12 @@ class SessionController extends ChangeNotifier with WidgetsBindingObserver { return success; } - /// Prompts for auth if [gate] is on and login lock is configured; - /// returns true immediately (no prompt) otherwise. Shared by the - /// account-switching gate above and [FilesController]'s/`PhotosController`'s - /// hidden-files gates. + /// Prompts for auth if [gate] is on; returns true immediately (no prompt) + /// otherwise. Each gate stands on its own - it doesn't depend on login lock + /// being on. Shared by the account-switching gate above and + /// [FilesController]'s/`PhotosController`'s hidden-files gates. Future passGate(bool gate, String reason) async { - if (!_loginLockEnabled || !gate) return true; + if (!gate) return true; return AppLockService.authenticate(reason); } diff --git a/lib/services/app_lock_service.dart b/lib/services/app_lock_service.dart index 8a205d7..7a0cda6 100644 --- a/lib/services/app_lock_service.dart +++ b/lib/services/app_lock_service.dart @@ -1,3 +1,4 @@ +import 'package:flutter/foundation.dart'; import 'package:local_auth/local_auth.dart'; /// Thin wrapper around `local_auth`. Deliberately doesn't implement its own @@ -8,9 +9,17 @@ import 'package:local_auth/local_auth.dart'; class AppLockService { static final LocalAuthentication _auth = LocalAuthentication(); + /// Replace the platform prompt in tests (null = the real one). + @visibleForTesting + static Future Function(String reason)? debugAuthenticate; + + @visibleForTesting + static Future Function()? debugIsDeviceSupported; + /// Whether this device can do *some* form of local auth - biometric /// enrolled, or at minimum a device PIN/pattern/password set up. static Future isDeviceSupported() async { + if (debugIsDeviceSupported != null) return debugIsDeviceSupported!(); try { final canCheckBiometrics = await _auth.canCheckBiometrics; if (canCheckBiometrics) return true; @@ -24,6 +33,7 @@ class AppLockService { /// throws) on cancellation, failure, or any platform error, so callers /// can treat every non-true result the same way: stay locked/blocked. static Future authenticate(String reason) async { + if (debugAuthenticate != null) return debugAuthenticate!(reason); try { return await _auth.authenticate( localizedReason: reason, diff --git a/lib/widgets/avatar_menu.dart b/lib/widgets/avatar_menu.dart index 3c13dcc..898c88e 100644 --- a/lib/widgets/avatar_menu.dart +++ b/lib/widgets/avatar_menu.dart @@ -332,7 +332,6 @@ class _OtherAccountRow extends StatelessWidget { ), const SizedBox(width: 12), NooAvatar(initials: accountInitial(name), current: false, size: 40), - const SizedBox(width: 44), ], ), ), diff --git a/lib/widgets/settings/settings_security.dart b/lib/widgets/settings/settings_security.dart index 341ab93..e9de0de 100644 --- a/lib/widgets/settings/settings_security.dart +++ b/lib/widgets/settings/settings_security.dart @@ -26,26 +26,28 @@ String biometricLabel(TargetPlatform platform) { } } -/// Settings section 3: login lock, which gates opening the app, switching -/// accounts, and revealing hidden files behind the device's own PIN/ +/// Settings section 3: three independent locks - opening the app, switching +/// accounts, and revealing hidden files - each behind the device's own PIN/ /// biometric credential (see `AppLockService` - this app never stores or -/// handles a PIN itself). +/// handles a PIN itself). Turning one on or off asks for that credential, and +/// none of them requires or implies another. class SettingsSecuritySection extends StatelessWidget { const SettingsSecuritySection({super.key}); - Future _handleLoginLockChanged( - BuildContext context, - SessionController session, - bool value, - ) async { - final success = value ? await session.setupLoginLock() : await session.disableLoginLock(); + Future _handleLockChanged( + BuildContext context, { + required Future Function() change, + required bool turningOn, + required String name, + }) async { + final success = await change(); if (!success && context.mounted) { ScaffoldMessenger.of(context).showSnackBar( SnackBar( content: Text( - value - ? "Could not set up login lock - make sure this device has a PIN, pattern, password, or biometric configured" - : 'Could not turn off login lock', + turningOn + ? "Could not turn on $name - make sure this device has a PIN, pattern, password, or biometric configured" + : 'Could not turn off $name', ), behavior: SnackBarBehavior.floating, ), @@ -64,10 +66,17 @@ class SettingsSecuritySection extends StatelessWidget { NooSettingsRow( icon: LucideIcons.lock, label: Text(biometricLabel(platform)), - subtitle: const Text("Require this device's PIN or biometric to open Noo"), + subtitle: const Text( + "Require this device's PIN or biometric to open Noo", + ), trailing: NooToggle( checked: session.loginLockEnabled, - onChanged: (value) => _handleLoginLockChanged(context, session, value), + onChanged: (value) => _handleLockChanged( + context, + change: value ? session.setupLoginLock : session.disableLoginLock, + turningOn: value, + name: 'login lock', + ), ), ), NooSettingsRow( @@ -76,7 +85,12 @@ class SettingsSecuritySection extends StatelessWidget { subtitle: const Text('Unlock to switch between saved accounts'), trailing: NooToggle( checked: session.lockAccountSwitching, - onChanged: session.loginLockEnabled ? session.setLockAccountSwitching : null, + onChanged: (value) => _handleLockChanged( + context, + change: () => session.setLockAccountSwitching(value), + turningOn: value, + name: 'account switching lock', + ), ), ), NooSettingsRow( @@ -85,7 +99,12 @@ class SettingsSecuritySection extends StatelessWidget { subtitle: const Text('Unlock to reveal hidden files and folders'), trailing: NooToggle( checked: session.lockHiddenFiles, - onChanged: session.loginLockEnabled ? session.setLockHiddenFiles : null, + onChanged: (value) => _handleLockChanged( + context, + change: () => session.setLockHiddenFiles(value), + turningOn: value, + name: 'hidden files lock', + ), ), ), ], diff --git a/test/providers/security_locks_test.dart b/test/providers/security_locks_test.dart new file mode 100644 index 0000000..be09cab --- /dev/null +++ b/test/providers/security_locks_test.dart @@ -0,0 +1,156 @@ +import 'package:flutter/services.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:shared_preferences/shared_preferences.dart'; +import 'package:noo/providers/connectivity_controller.dart'; +import 'package:noo/providers/session_controller.dart'; +import 'package:noo/services/app_lock_service.dart'; + +/// The three Settings -> Security locks (open the app, switch accounts, reveal +/// hidden files) each work on their own - none needs or implies another. The +/// device prompt is replaced with a recorder so the gates can be exercised. +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + setUpAll(() { + final messenger = + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger; + messenger.setMockMethodCallHandler( + const MethodChannel('plugins.it_nomads.com/flutter_secure_storage'), + (call) async => call.method == 'readAll' ? {} : null, + ); + messenger.setMockMethodCallHandler( + const MethodChannel('dev.fluttercommunity.plus/connectivity'), + (call) async => call.method == 'check' ? ['none'] : null, + ); + messenger.setMockStreamHandler( + const EventChannel('dev.fluttercommunity.plus/connectivity_status'), + MockStreamHandler.inline(onListen: (arguments, events) {}), + ); + }); + + late List prompts; + var authSucceeds = true; + var deviceSupported = true; + + setUp(() { + SharedPreferences.setMockInitialValues({}); + prompts = []; + authSucceeds = true; + deviceSupported = true; + AppLockService.debugAuthenticate = (reason) async { + prompts.add(reason); + return authSucceeds; + }; + AppLockService.debugIsDeviceSupported = () async => deviceSupported; + }); + + tearDown(() { + AppLockService.debugAuthenticate = null; + AppLockService.debugIsDeviceSupported = null; + }); + + SessionController build() => SessionController(ConnectivityController()); + + test( + 'hidden files and account switching can be locked without login lock', + () async { + final session = build(); + + expect(await session.setLockHiddenFiles(true), isTrue); + expect(await session.setLockAccountSwitching(true), isTrue); + + expect(session.lockHiddenFiles, isTrue); + expect(session.lockAccountSwitching, isTrue); + expect(session.loginLockEnabled, isFalse); + expect( + session.needsUnlock, + isFalse, + reason: 'opening the app stays open', + ); + }, + ); + + test('a gate prompts on its own, with login lock off', () async { + final session = build(); + + expect(await session.passGate(false, 'unused'), isTrue); + expect(prompts, isEmpty, reason: 'a gate that is off never prompts'); + + expect(await session.passGate(true, 'Unlock to show hidden files'), isTrue); + expect(prompts, ['Unlock to show hidden files']); + + authSucceeds = false; + expect(await session.passGate(true, 'again'), isFalse); + }); + + test( + 'turning a lock on needs a capable device and a successful auth', + () async { + final session = build(); + + deviceSupported = false; + expect(await session.setLockHiddenFiles(true), isFalse); + expect(session.lockHiddenFiles, isFalse); + + deviceSupported = true; + authSucceeds = false; + expect(await session.setLockHiddenFiles(true), isFalse); + expect(session.lockHiddenFiles, isFalse); + + authSucceeds = true; + expect(await session.setLockHiddenFiles(true), isTrue); + expect(session.lockHiddenFiles, isTrue); + }, + ); + + test( + 'turning a lock off needs auth, so it cannot just be flipped away', + () async { + final session = build(); + await session.setLockAccountSwitching(true); + prompts.clear(); + + authSucceeds = false; + expect(await session.setLockAccountSwitching(false), isFalse); + expect(session.lockAccountSwitching, isTrue); + + authSucceeds = true; + expect(await session.setLockAccountSwitching(false), isTrue); + expect(session.lockAccountSwitching, isFalse); + expect(prompts, hasLength(2)); + }, + ); + + test('setting a lock to its current value does not prompt', () async { + final session = build(); + expect(await session.setLockHiddenFiles(false), isTrue); + expect(prompts, isEmpty); + }); + + test('turning login lock off leaves the other two locks alone', () async { + final session = build(); + await session.setupLoginLock(); + await session.setLockAccountSwitching(true); + await session.setLockHiddenFiles(true); + expect(session.loginLockEnabled, isTrue); + + expect(await session.disableLoginLock(), isTrue); + + expect(session.loginLockEnabled, isFalse); + expect(session.lockAccountSwitching, isTrue); + expect(session.lockHiddenFiles, isTrue); + }); + + test('each lock is saved on its own and restored', () async { + final first = build(); + await first.setLockHiddenFiles(true); + // Let the fire-and-forget pref write land. + await Future.delayed(const Duration(milliseconds: 50)); + + final second = build(); + await Future.delayed(const Duration(milliseconds: 100)); + expect(second.lockHiddenFiles, isTrue); + expect(second.lockAccountSwitching, isFalse); + expect(second.loginLockEnabled, isFalse); + }); +} diff --git a/test/widgets/avatar_menu_test.dart b/test/widgets/avatar_menu_test.dart index 2d703fc..6fc1dd9 100644 --- a/test/widgets/avatar_menu_test.dart +++ b/test/widgets/avatar_menu_test.dart @@ -15,6 +15,8 @@ import 'package:noo/providers/settings_controller.dart'; import 'package:noo/providers/sync_status_controller.dart'; import 'package:noo/providers/trash_controller.dart'; import 'package:noo/theme/app_theme.dart'; +import 'package:noo/theme/design_tokens.dart'; +import 'package:noo/widgets/noo/core/noo_avatar.dart'; import 'package:noo/widgets/app_top_bar.dart'; import 'package:noo/widgets/noo/nav/noo_top_bar.dart'; @@ -174,4 +176,48 @@ void main() { expect(find.text('Add account'), findsOneWidget); }, ); + + testWidgets("other accounts' avatars sit flush right in the dropdown", ( + tester, + ) async { + SharedPreferences.setMockInitialValues({ + 'account_migration_v1_done': true, + 'accounts_list': jsonEncode([ + const SavedAccount( + id: accountId, + serverUrl: 'https://server.example.com', + username: 'alice', + ).toJson(), + const SavedAccount( + id: 'other_example_org__bob', + serverUrl: 'https://other.example.org', + username: 'bob', + ).toJson(), + ]), + 'active_account_id': accountId, + }); + await pumpTopBar(tester, navMenuStyle: NooNavMenuStyle.avatarMenu); + + await tester.tap(find.byTooltip('Menu')); + await tester.pump(); + await tester.pump(const Duration(milliseconds: 200)); + // Expand the account list from the header. + await tester.tap(find.text('alice')); + await tester.pump(); + await tester.pump(const Duration(milliseconds: 400)); + + final row = find.ancestor( + of: find.text('bob'), + matching: find.byType(InkWell), + ); + final avatar = find.descendant(of: row, matching: find.byType(NooAvatar)); + expect(avatar, findsOneWidget); + + // Only the row's own padding between the avatar and the card's edge - + // no extra gutter pushing it in from the right. + expect( + tester.getTopRight(row).dx - tester.getTopRight(avatar).dx, + NooSpace.md, + ); + }); }