Skip to content

Commit

Permalink
wip
Browse files Browse the repository at this point in the history
  • Loading branch information
luckyrat committed Jan 24, 2025
1 parent a114324 commit c14ac5e
Show file tree
Hide file tree
Showing 8 changed files with 141 additions and 28 deletions.
3 changes: 0 additions & 3 deletions lib/cubit/account_cubit.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
64 changes: 43 additions & 21 deletions lib/cubit/vault_cubit.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 =
Expand All @@ -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);
}
Expand All @@ -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');
}
}

Expand Down Expand Up @@ -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);
Expand Down
88 changes: 88 additions & 0 deletions lib/password_mismatch_recovery_situation.dart
Original file line number Diff line number Diff line change
@@ -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.
*/
2 changes: 1 addition & 1 deletion lib/vault_file.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 3 additions & 0 deletions lib/widgets/account_email_change.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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 {
Expand Down
3 changes: 2 additions & 1 deletion lib/widgets/root.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion lib/widgets/vault.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion lib/widgets/vault_loader.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit c14ac5e

Please sign in to comment.