wip

Author: luckyrat

lib/cubit/account_cubit.dart

@@ -348,9 +348,6 @@ class AccountCubit extends Cubit<AccountState> {
       }
 
       await _userRepo.changePasswordFinish(currentUser, newPassKey);
-      //TODO: What do we do now? emit a new state to force the user to sign in again? may not be needed since we updated
-      // JWTs... can maybe just do nothing and let the caller pop the navigation or show some feedback to say all worked
-      // fine. But maybe want to at least emit the currentuser again since it changed? but mutated so nothing will actually happen?
       return true;
     } on KeeLoginFailedMITMException {
       rethrow;

lib/cubit/vault_cubit.dart

@@ -12,6 +12,7 @@ import 'package:keevault/cubit/generator_profiles_cubit.dart';
 import 'package:keevault/extension_methods.dart';
 import 'package:keevault/local_vault_repository.dart';
 import 'package:keevault/locked_vault_file.dart';
+import 'package:keevault/password_mismatch_recovery_situation.dart';
 import 'package:keevault/password_strength.dart';
 import 'package:keevault/credentials/quick_unlocker.dart';
 import 'package:keevault/vault_backend/exceptions.dart';
@@ -29,16 +30,6 @@ import 'account_cubit.dart';
 
 part 'vault_state.dart';
 
-enum PasswordMismatchRecoverySituation {
-  None,
-  RemoteUserDiffers,
-  RemoteFileDiffers,
-
-  //TODO: Probably can never know that these are the situation - can't tell the difference between user typing the wrong password and can't try the remote file password anyway until the service password is correct. Maybe delete these enum values?
-  //RemoteUserAndFileDiffers,
-  //AllThreeAreDifferent
-}
-
 class VaultCubit extends Cubit<VaultState> {
   final RemoteVaultRepository _remoteVaultRepo;
   final LocalVaultRepository _localVaultRepo;
@@ -574,6 +565,18 @@ class VaultCubit extends Cubit<VaultState> {
         l.i('refresh called during an ongoing upload. Will not refresh now.');
         return;
       }
+      if (s is VaultRefreshing) {
+        l.i('refresh called during an ongoing refresh operation. Will not start a new refresh now.');
+        return;
+      }
+      if (s is VaultRefreshCredentialsRequired && recovery == PasswordMismatchRecoverySituation.None) {
+        l.i('refresh called during an ongoing refresh credentials repair operation. Will not refresh now.');
+        return;
+      }
+      if (s is VaultUploadCredentialsRequired && recovery == PasswordMismatchRecoverySituation.None) {
+        l.i('refresh called during an ongoing upload credentials repair operation. Will not refresh now.');
+        return;
+      }
       if (s is VaultChangingPassword) {
         l.i('refresh called during a password change. Will not refresh now.');
         return;
@@ -666,15 +669,27 @@ class VaultCubit extends Cubit<VaultState> {
               credentialsOverrideWithStrength.credentials,
               DateTime.now().add(Duration(days: requireFullPasswordPeriod)).millisecondsSinceEpoch,
               await updatedLocalFile.files.current.kdfCacheKey);
-        }
 
-        if (tempLockedFile == null) {
-          l.d("Latest remote file not changed so didn't download it");
-          safeEmitLoaded(updatedLocalFile ?? s.vault);
-          if (KeeVaultPlatform.isIOS) {
-            await autofillMerge(user, onlyIfAttemptAlreadyDue: true);
+          if (tempLockedFile == null) {
+            l.d("Latest remote file not changed so didn't download it");
+            safeEmitLoaded(updatedLocalFile);
+            //TODO: save and upload LK to become RK. need to provide any override passwords/status?
+            await save(user);
+            if (KeeVaultPlatform.isIOS) {
+              await autofillMerge(user, onlyIfAttemptAlreadyDue: true);
+            }
+            return;
+          }
+          //TODO: else... probably already handled by next step? need to check we use the updatedlocalfile as our local one since that now has the correct kdbx password.
+        } else {
+          if (tempLockedFile == null) {
+            l.d("Latest remote file not changed so didn't download it");
+            safeEmitLoaded(s.vault);
+            if (KeeVaultPlatform.isIOS) {
+              await autofillMerge(user, onlyIfAttemptAlreadyDue: true);
+            }
+            return;
           }
-          return;
         }
         lockedFile = tempLockedFile;
       } on KdbxInvalidKeyException {
@@ -721,18 +736,23 @@ class VaultCubit extends Cubit<VaultState> {
         try {
           file = await RemoteVaultFile.unlock(lockedFile);
           successfulCredentials = lockedFile.credentials;
+          // no password problem or situation ???
         } on KdbxInvalidKeyException {
           // Try again with the other password or let the higher catch statement handle this
           // We know we tried the remote creds first so if they are different to local, we should try those too.
           if (credsRemote != credsLocal) {
             final lockedFileWithLocalCreds = lockedFile.copyWith(credentials: credsLocal);
             file = await RemoteVaultFile.unlock(lockedFileWithLocalCreds);
             successfulCredentials = lockedFileWithLocalCreds.credentials;
+            // 2 or 3?
+            // if (updatedLocalFile != null) 3 else 2?
           } else {
             rethrow;
           }
         }
 
+// RemoteFileDiffers only time and safe time to save the credentials? we are saying whatever creds worked to open the remote file is what we will save as the credentials needed to open the local file.
+// updatedLocalFile must be null if we are in remotefilediffers mode
         if (recovery == PasswordMismatchRecoverySituation.RemoteFileDiffers && successfulCredentials != null) {
           l.d('Updating QU with newly successful KDBX password');
           final requireFullPasswordPeriod =
@@ -747,7 +767,7 @@ class VaultCubit extends Cubit<VaultState> {
         await prefs.setString('user.${user.email}.lastRemoteEtag', file.etag!);
         await prefs.setString('user.${user.email}.lastRemoteVersionId', file.versionId!);
         emit(VaultUpdatingLocalFromRemote(updatedLocalFile ?? s.vault));
-        final newFile = await update(user, s.vault, file);
+        final newFile = await update(user, updatedLocalFile ?? s.vault, file);
         if (newFile != null) {
           await emitVaultLoaded(newFile, user, immediateRemoteRefresh: false, safe: true);
         }
@@ -774,7 +794,7 @@ class VaultCubit extends Cubit<VaultState> {
         l.d('refresh called during an import. Will not refresh now.');
         return;
       }
-      throw Exception('Vault not loaded when refresh called');
+      l.w('Vault not loaded when refresh called');
     }
   }
 
@@ -1610,8 +1630,10 @@ class VaultCubit extends Cubit<VaultState> {
 
   void startEmailChange() {
     if (currentVaultFile == null || (!currentVaultFile!.files.current.isDirty && _entryCubit.state is! EntryLoaded)) {
-      //TODO: test if just signing out triggers the request to sign in again automatically or if it is to do with the change to the account cubit
-      //  _accountCubit.startEmailChange();
+      //TODO: test if just signing out triggers the request to sign in again automatically or if it is
+      // to do with the change to the account cubit
+      // result: Yes, signout results in immediate re-signin so cubit account request can never work. need to do something different to sign-out. maybe forget the user or sign out the account level user too? would be good to remeember their email address though since really we only want to sign them out from the vault, not their user account.
+      _accountCubit.startEmailChange();
       signout();
     } else {
       emitError('You must save your changes first!', toast: true);

lib/password_mismatch_recovery_situation.dart

@@ -0,0 +1,88 @@
+enum PasswordMismatchRecoverySituation {
+  None,
+  RemoteUserDiffers,
+  RemoteFileDiffers,
+
+  //TODO: Probably can never know that these are the situation - can't tell the difference between user typing the wrong password and can't try the remote file password anyway until the service password is correct. Maybe delete these enum values?
+  //RemoteUserAndFileDiffers,
+  //AllThreeAreDifferent
+}
+
+/*
+
+We try to automatically resolve all mismatched password situations here.
+
+There are 3 current passwords we need to align - they generally always are aligned but crashes or bugs, particularly during a password change procedure, can cause them to get out of sync.
+
+The 3 passwords are:
+Local KDBX file - the remoteMargeTarget unless current is newer
+Remote KDBX file
+Remote user authentication password
+
+We'll need to align on the remote authentication password so that the user doesn't get stuck in a constant battle between conflicting local passwords on multiple devices.
+
+Slightly old tokens being used to upload modified RK could result in state 1 or 2.
+
+p1 p2 and p3 have no specific temporal ordering. Depending on how we ended up in one of these states, any of those could be considered "newest".
+
+Thus the possible failure modes are:
+
+1) RemoteUserDiffers
+
+LK = p1
+RK = p1
+RU = p2
+
+Symptom:
+User will be unable to download or upload the RK file
+
+Solution: 
+User provides an override password which we use for authentication.
+Need to use non-override password to unlock RK. But we don't know in advance if we are in this state or 3. So we assume 3 and actually progress towards state 2. Ideally we resolve in the same step by just trying again with p1 but in worst case user could enter a password again.
+If RK is newer, download it, change its password to p2, change LK password to p2, merge it, store result as LK and upload as RK.
+If RK is not newer, change LK password to p2, store result as LK and upload as RK.
+
+
+2) RemoteFileDiffers
+
+LK = p1
+RK = p2
+RU = p1
+
+Symptom:
+User will be unable to unlock the RK file while merging for upload or downloading for refresh/syncing.
+
+Solution: 
+User provides an override password which we use for unlocking the RK.
+Need to use non-override password to download/upload RK. So we need to know that RU worked when getting p2 from the user.
+If RK is newer, download it, change its password to p1, merge it, store result as LK and upload as RK.
+If RK is not newer, upload LK as RK.
+
+
+3) RemoteUserAndFileDiffers
+
+LK = p1
+RK = p2
+RU = p2
+
+Symptom:
+User will be unable to download or modify the RK file, nor unlock it using their local password
+
+Solution: 
+User provides an override password which we use for authentication and to unlock RK.
+If RK is newer, download it, change LK password to p2, merge it, store result as LK and upload as RK.
+If RK is not newer, change LK password to p2, store result as LK and upload as RK.
+
+4) AllThreeAreDifferent
+
+LK = p1
+RK = p2
+RU = p3
+
+Solution: 
+Ignore. We hope that existing support for other situations will allow the user to muddle through to a resolution in multiple steps.
+
+
+
+Generally, we will need to track whether it was the RK or RU that failed. That lets us determine between states 1,2 and 3+4. We know we are in 3+4 if trying to resolve 3 fails at either the access request or RK unlocking stage. Once the user enters p3, we can change LK with that password and thus effectively reduce the problem to state 1 (which user can then resolve by entering what was p2 in this scenario). We won't implement all of this edge case but expect at least that forcibly killing the app at the right time will allow the user to recover.
+*/

lib/vault_file.dart

@@ -79,7 +79,7 @@ class VaultFileVersions {
       ..header.writeKdfParameters(credentialsWithStrength.createNewKdfParameters());
     final kdbxData = await unlockedFile.save();
     return VaultFileVersions(
-        current: _current,
+        current: unlockedFile,
         pending: null,
         pendingLocked: null,
         remoteMergeTarget: null,

lib/widgets/account_email_change.dart

@@ -4,6 +4,8 @@ import 'package:email_validator/email_validator.dart';
 import 'package:flutter/material.dart';
 import 'package:flutter_bloc/flutter_bloc.dart';
 import 'package:keevault/cubit/account_cubit.dart';
+import '../config/app.dart';
+import '../config/routes.dart';
 import '../generated/l10n.dart';
 
 typedef SubmitCallback = Future<void> Function(String string);
@@ -22,6 +24,7 @@ class _AccountEmailChangeWidgetState extends State<AccountEmailChangeWidget> {
     // We always sign the user out. Might be nice to send them back to their Vault if that's where they came from (i.e. they have a validated and active account) but might be hard to do securely so will ignore that edge case initially.
     final accountCubit = BlocProvider.of<AccountCubit>(context);
     await accountCubit.signout();
+    await AppConfig.router.navigateTo(AppConfig.navigatorKey.currentContext!, Routes.root, clearStack: true);
   }
 
   Future<void> changeEmailAddress(String password, String newEmailAddress) async {

lib/widgets/root.dart

@@ -36,7 +36,8 @@ class RootWidgetState extends State<RootWidget> {
     await BlocProvider.of<AutofillCubit>(context).refresh();
     await accountCubit.startup();
     final AccountState state = accountCubit.state;
-    if (state is AccountChosen) {
+    //TODO: need more exceptions like for email verification etc?
+    if (state is AccountChosen && state is! AccountEmailChangeRequested) {
       await vaultCubit.startup(state.user, null);
     } else if (state is AccountLocalOnly) {
       await vaultCubit.startupFreeMode(null);

lib/widgets/vault.dart

@@ -10,6 +10,7 @@ import 'package:keevault/cubit/account_cubit.dart';
 import 'package:keevault/cubit/app_settings_cubit.dart';
 import 'package:keevault/cubit/autofill_cubit.dart';
 import 'package:keevault/cubit/filter_cubit.dart';
+import 'package:keevault/password_mismatch_recovery_situation.dart';
 import 'package:keevault/vault_backend/exceptions.dart';
 import 'package:keevault/widgets/vault_password_credentials.dart';
 
@@ -56,7 +57,7 @@ class _VaultWidgetState extends State<VaultWidget> with WidgetsBindingObserver {
     final user = BlocProvider.of<AccountCubit>(context).currentUser;
     final VaultState vaultState = BlocProvider.of<VaultCubit>(context).state;
     PasswordMismatchRecoverySituation recovery = PasswordMismatchRecoverySituation.None;
-    if (vaultState is VaultUploadCredentialsRequired) {
+    if (vaultState is VaultRefreshCredentialsRequired) {
       recovery = vaultState.recovery;
     }
     await BlocProvider.of<VaultCubit>(context).refresh(user, overridePasswordRemote: password, recovery: recovery);

lib/widgets/vault_loader.dart

@@ -153,7 +153,8 @@ class VaultLoaderState extends State<VaultLoaderWidget> {
       }
       return LoadingSpinner(tooltip: 'Unknown app state: ${state.toString()}');
     }, listener: (context, state) async {
-      if (state is VaultLoaded) {
+      //TODO: why do we need the refreshing exception? do we need any others? does this fix any autofill reliability issues too?
+      if (state is VaultLoaded && state is! VaultRefreshing) {
         final AutofillState autofillState = BlocProvider.of<AutofillCubit>(context).state;
         final filterContext = BlocProvider.of<FilterCubit>(context);
         final interactionContext = BlocProvider.of<InteractionCubit>(context);