Skip to content

Commit

Permalink
wip
Browse files Browse the repository at this point in the history
  • Loading branch information
@luckyrat
luckyrat committed Jan 25, 2025
1 parent ef2b708 commit b58169b
Show file tree
Hide file tree
Showing 7 changed files with 69 additions and 76 deletions.
13 changes: 1 addition & 12 deletions lib/cubit/account_cubit.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import 'package:bloc/bloc.dart';
import 'package:flutter_inapp_purchase/flutter_inapp_purchase.dart';
import 'package:kdbx/kdbx.dart';
import 'package:keevault/config/platform.dart';
//import 'package:keevault/cubit/vault_cubit.dart';
import 'package:keevault/payment_service.dart';
import 'package:keevault/user_repository.dart';
import 'package:keevault/vault_backend/account_verification_status.dart';
Expand Down Expand Up @@ -345,24 +344,17 @@ class AccountCubit extends Cubit<AccountState> {
final success = await onChangeStarted(currentUser);

if (!success) {
throw KeeException('Password change aborted.');
throw VaultPasswordChangeException('Password change aborted.');
}

await _userRepo.changePasswordFinish(currentUser, newPassKey);
return true;
} on KeeLoginFailedMITMException {
rethrow;

//TODO: All error handling. sample from email change is below...
} on KeeLoginRequiredException {
l.w('Unable to change password due to a 403.');
throw VaultPasswordChangeException(
'Due to an authentication problem, we were unable to change your password. Probably it has been too long since you last signed in. Please sign out and then sign in again with your previous password and try again when you have enough time to complete the operation within 10 minutes.');
} on FormatException {
// Local validation
l.i('Unable to change password due to FormatException.');
emit(AccountEmailChangeRequested(
currentUser, 'Please enter the correct password for your existing Kee Vault account.'));
} on KeeNotFoundException {
l.i('Unable to change password due to 404 response.');
throw VaultPasswordChangeException('We cannot find your account. Have you recently deleted it?');
Expand All @@ -371,7 +363,6 @@ class AccountCubit extends Cubit<AccountState> {
throw VaultPasswordChangeException(
'Due to a network failure, we cannot say whether your request succeeded or not. If possible, try signing in to a different device with your new password to find out if the change took effect. If unsure if it worked, sign in with your previous password next time and try again when you have a more stable network connection.');
}
return false;
}

Future<bool> changeEmailAddress(String password, String newEmailAddress) async {
Expand All @@ -391,8 +382,6 @@ class AccountCubit extends Cubit<AccountState> {
final newUser = await User.fromEmail(newEmailAddress);
emit(AccountChosen(newUser));
await signout();
// await AppConfig.router.navigateTo(AppConfig.navigatorKey.currentContext!, Routes.root, clearStack: true);
//await BlocProvider.of<AccountCubit>(context).forgetUser(vc.signout);
return true;
} on KeeLoginFailedMITMException {
rethrow;
Expand Down
84 changes: 57 additions & 27 deletions lib/cubit/vault_cubit.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -643,14 +643,24 @@ class VaultCubit extends Cubit<VaultState> {
final tempLockedFile = await _remoteVaultRepo.download(user, credsRemote, lastRemoteEtag);

if (recovery == PasswordMismatchRecoverySituation.remoteUserDiffers) {
// If we're in state 3 (or 4) this should get us into 2; User may need to enter the other password again when the next refresh operation happens (assuming we found no change to the kdbx to download this time) but they'll get the problem resolved eventually.
// If we're in state 3 (or 4) this should get us into 2; User may need to
// enter the other password again when the next refresh operation happens
// (assuming we found no change to the kdbx to download this time) but
// they'll get the problem resolved eventually.

l.d('Updating QU with newly successful service password');
await _qu.saveQuickUnlockUserPassKey(user.passKey);

// If we're in state 2,3 or 4 we need to update the local file password. In state 1, we don't, but I don't think we can definitively know if we are in that situation so we do it anyway, even if it is essentially a NOOP
// If we're in state 2,3 or 4 we need to update the local file password.
// In state 1, we don't, but I don't think we can definitively know if
// we are in that situation so we do it anyway, even if it is essentially a NOOP

l.d('Updating local kdbx with same password that just worked for remote user authentication.');
// typically this is the right thing to do. Maybe rare bugs would mean we should be treating the local KDBX file password as the user's chosen password but we have to pick one and the impact of getting it wrong is only that the user would have to use what they consider to be their old password, at least until they change it to their new password successfully after this recovery is complete.
// typically this is the right thing to do. Maybe rare bugs would mean we should
// be treating the local KDBX file password as the user's chosen password but we
// have to pick one and the impact of getting it wrong is only that the user
// would have to use what they consider to be their old password, at least until
// they change it to their new password successfully after this recovery is complete.

updatedLocalFile = LocalVaultFile(
await s.vault.files.copyWithNewCredentials(credentialsOverrideWithStrength!),
Expand All @@ -660,7 +670,9 @@ class VaultCubit extends Cubit<VaultState> {
s.vault.etag,
s.vault.versionId);

// May be some cases where this is not necessary. Probably all are edge cases but maybe could try harder to identify times we can safely skip this step without disabling biometrics for the user.
// May be some cases where this is not necessary. Probably all are edge cases
// but maybe could try harder to identify times we can safely skip this step
// without disabling biometrics for the user.
l.d('Updating QU with newly modified local KDBX password');
final requireFullPasswordPeriod =
int.tryParse(Settings.getValue<String>('requireFullPasswordPeriod') ?? '60') ?? 60;
Expand All @@ -673,27 +685,31 @@ class VaultCubit extends Cubit<VaultState> {
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?

// save and upload LK to become the RK.
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;
// If there was a change that we downloaded, we will try later to merge it with
// the newly edited local KDBX file that has the user's new password.
} else if (tempLockedFile == null) {
// We're not in a recovery mode that requires us to take account of a new user
// account password and we have found no changes to the remote file. This is
// the case almost all of the times that we execute this refresh function.
l.d("Latest remote file not changed so didn't download it");
safeEmitLoaded(s.vault);
if (KeeVaultPlatform.isIOS) {
await autofillMerge(user, onlyIfAttemptAlreadyDue: true);
}
return;
}
lockedFile = tempLockedFile;
} on KdbxInvalidKeyException {
// Pretty sure this can't happen - download doesn't actually attempt to unlock the downloaded file and autofillMerge handles the exception itself.
// Pretty sure this can't happen - download doesn't actually attempt to unlock
// the downloaded file and autofillMerge handles the exception itself.
handleRefreshAuthError(s.vault, recovery: PasswordMismatchRecoverySituation.remoteFileDiffers);
return;
} on KeeLoginRequiredException {
Expand Down Expand Up @@ -736,23 +752,29 @@ class VaultCubit extends Cubit<VaultState> {
try {
file = await RemoteVaultFile.unlock(lockedFile);
successfulCredentials = lockedFile.credentials;
// no password problem or situation ???
// It is typical to reach this point successfully. Situations where we fail to
// reach here would include recovery cases 1 (where we have used the remote
// user account password to try to unlock the remote KDBX file, since we
// first assume that we are in situation 3) and 2 (where only the remote
// file password is different from the one the user has supplied to get this far)
} on KdbxInvalidKeyException {
// Try again with the other password or let the higher catch statement handle this

// Usually the remote and local creds will match and the reason we got here
// is that the user entered an incorrect password as part of the mismatched
// password recovery process.

// 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
// 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 @@ -772,8 +794,15 @@ class VaultCubit extends Cubit<VaultState> {
await emitVaultLoaded(newFile, user, immediateRemoteRefresh: false, safe: true);
}
} on KdbxInvalidKeyException {
// This is the first point we might be trying to unlock the remote file. If we reach this point we have tried both the user's local and remote passwords. They may have just entered the 2nd password incorrectly. Or this may be the first time we are discovering that their 1st password (local) can no longer open their remote file. We do know that it can authenticate them though.
// So we know we are in situation 2 if we tried the same password everywhere, or situation 2, 3 or 4 if we tried a separate remote file password already (2 if the user just put in the wrong password).
// This is the first point we might be trying to unlock the remote file. If we reach
// this point we have tried both the user's local and remote passwords (if they have
// been prompted for a 2nd password for recovery purposes). They may have just entered
// the 2nd password incorrectly. Or this may be the first time we are discovering that
// their 1st password (local) can no longer open their remote file. We do know that it
// can authenticate them though.
// So we know we are in situation 2 if we tried the same password everywhere, or
// situation 2, 3 or 4 if we tried a separate remote file password already (2 if the
// user just put in the wrong password).

l.w('Remote file failed to unlock using supplied password(s).');
// We've staged the update so startup can handle trying to fix the problem later.
Expand Down Expand Up @@ -1173,10 +1202,11 @@ class VaultCubit extends Cubit<VaultState> {
}
}

//TODO: In what case do we need to change the local file during an upload? When password was successfully changed on a remote machine - so circumstance 3. But we already handle that during refresh. So perhaps we never need to do it here. Unless the user also made a change to this local password file before signing in with the new password... probably very rare though so will defer support for that for now.
// If user has supplied the correct password for the upload to succeed, we must make sure the locked data uploaded is encrypted using that new password. remoteMergeTarget and current are identical at this time because user has just supplied a new password through the UI so can't have any outstanding modifications in the current vault file. There must also be no pending files.
// updatedLocalFile = LocalVaultFile(await vault.files.copyWithNewCredentials(credentialsWithStrength),
// vault.lastOpenedAt, vault.persistedAt, vault.uuid, vault.etag, vault.versionId);
//TODO:f: In what case do we need to change the local file during an upload? When password
// was successfully changed on a remote machine - so circumstance 3. But we already handle
// that during refresh. So perhaps we never need to do it here. Unless the user also made a
// change to this local password file before signing in with the new password... probably
// very rare though so will defer support for that for now.

String? lastRemoteEtag;
try {
Expand Down Expand Up @@ -1259,7 +1289,7 @@ class VaultCubit extends Cubit<VaultState> {
try {
latestRemoteFile = await RemoteVaultFile.unlock(latestLockedRemoteFile!);
} on KdbxInvalidKeyException catch (e, s) {
//TODO: Maybe support trying with local creds in case we are in a mismatch situation?
//TODO:f: Maybe support trying with local creds in case we are in a mismatch situation?

// Creds aren't able to unlock the remote file
// Could be because creds were changed on a different device since the most recent download happened on this one.
Expand Down
4 changes: 3 additions & 1 deletion lib/password_mismatch_recovery_situation.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ enum PasswordMismatchRecoverySituation {
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?
//TODO:f: 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
}
Expand Down
30 changes: 0 additions & 30 deletions lib/widgets/account_email_change.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,6 @@ class _AccountEmailChangeWidgetState extends State<AccountEmailChangeWidget> {
bool passwordObscured = true;
bool disableChange = true;
bool changing = false;
bool showPreviousError = true;

//TODO: If widget created with an error string, show it if above is true

// @override
// void initState() {
// super.initState();
// unawaited(_enableButtonsAfterDelay());
// }

@override
Widget build(BuildContext context) {
Expand Down Expand Up @@ -258,11 +249,7 @@ class _AccountEmailChangeWidgetState extends State<AccountEmailChangeWidget> {
onPressed: changing || disableChange
? null
: () async {
// final navigator = Navigator.of(context);
final sm = ScaffoldMessenger.of(context);
setState(() {
showPreviousError = false;
});
if (_formKey.currentState!.validate()) {
_formKey.currentState!.save();
try {
Expand Down Expand Up @@ -307,21 +294,4 @@ class _AccountEmailChangeWidgetState extends State<AccountEmailChangeWidget> {
}
});
}

// Flutter does not support async validators and not worth implementing a work around for this I expect.
// Future<bool> checkCurrentAccountPassword(User user, String? value) async {
// // We check the again after submission but don't expect any
// // difference from this result so this lets us give early feedback for typos
// if (value == null) {
// return false;
// }
// final protectedValue = ProtectedValue.fromString(value);
// final key = protectedValue.hash;
// final oldPassKey = await derivePassKey(user.email!, key);

// if (user.passKey == oldPassKey) {
// return true;
// }
// return false;
// }
}
4 changes: 0 additions & 4 deletions lib/widgets/account_wrapper.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,6 @@ class AccountWrapperState extends State<AccountWrapperWidget> {
} else if (state is AccountExpired) {
return AccountExpiredWidget(trialAvailable: state.trialAvailable);
} else if (state is AccountEmailChangeRequested) {
//TODO: verify below is true. even if network is slow to respond / times out.
// Success, failure and cancel all will sign out user, remove QuickUnlock data, and thus bump them back to the sign-in page for fresh authentication.
return AccountEmailChangeWidget();
} else if (state is AccountEmailNotVerified) {
return AccountEmailNotVerifiedWidget();
Expand All @@ -134,8 +132,6 @@ class AccountWrapperState extends State<AccountWrapperWidget> {
return Text(str.vaultStatusUnknownState);
}, listenWhen: (prev, current) {
if (current is AccountAuthenticated && prev is! AccountEmailNotVerified && prev is! AccountExpired) {
// &&
//prev is! AccountEmailChangeRequested) {
return false;
}
return true;
Expand Down
2 changes: 1 addition & 1 deletion lib/widgets/root.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class RootWidgetState extends State<RootWidget> {
await BlocProvider.of<AutofillCubit>(context).refresh();
await accountCubit.startup();
final AccountState state = accountCubit.state;
//TODO: need more exceptions like for email verification etc?
//TODO:f: 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) {
Expand Down
8 changes: 7 additions & 1 deletion lib/widgets/vault_loader.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,13 @@ class VaultLoaderState extends State<VaultLoaderWidget> {
}
return LoadingSpinner(tooltip: 'Unknown app state: ${state.toString()}');
}, listener: (context, state) async {
//TODO: why do we need the refreshing exception? do we need any others? does this fix any autofill reliability issues too?
// Once the state becomes VaultLoaded we want to redirect to the main vault widget
// (and thus clear the loading spinner that is rendering in this widget). This
// can take a moment to complete and in the mean time we will typically have
// changed state to VaultRefreshing and thus entered this listener function a
// second time. To prevent wasted effort and duplicated metrics being recorded,
// we take no action when in VaultRefreshing state. Perhaps we should have
// exceptions for other states too but I'm yet to find any real-world need.
if (state is VaultLoaded && state is! VaultRefreshing) {
final AutofillState autofillState = BlocProvider.of<AutofillCubit>(context).state;
final filterContext = BlocProvider.of<FilterCubit>(context);
Expand Down

0 comments on commit b58169b

Please sign in to comment.