Merge branch 'fix-security-screen-discard-dialog'

lib/app/cubits/repo_security.dart

@@ -17,39 +17,51 @@ class RepoSecurityState {
  required this.oldLocalSecretMode,
  required this.oldLocalSecret,
  SecretKeyOrigin? origin,
- bool? store,
+ // Reflects the user's preference on storing a password for
+ // `LocalSecretMode`s where storing is not implicit.
+ this.userPrefersToStoreSecret = const None(),
  bool? secureWithBiometrics,
  this.localPassword = const None(),
+ this.updatedLocalPassword = const None(),
  this.isBiometricsAvailable = false,
  }) : origin = origin ?? oldLocalSecretMode.origin,
- store = store ?? _initialStore(oldLocalSecretMode),
  secureWithBiometrics = secureWithBiometrics ??
  oldLocalSecretMode.store.isSecuredWithBiometrics;
 
  final LocalSecretMode oldLocalSecretMode;
  final LocalSecret oldLocalSecret;
  final SecretKeyOrigin origin;
- final bool store;
+ final Option<bool> userPrefersToStoreSecret;
  final bool secureWithBiometrics;
  final Option<LocalPassword> localPassword;
+ // This is set to the above `localPassword` once the user clicks the "UPDATE"
+ // button. It is used to check whether the `localPassword` has changed
+ // between the user clicking the "UPDATE" button and leaving the security
+ // page.
+ final Option<LocalPassword> updatedLocalPassword;
  final bool isBiometricsAvailable;
+ final bool _defaultStoreIfOriginIsManual = false;
 
  RepoSecurityState copyWith({
  LocalSecretMode? oldLocalSecretMode,
  LocalSecret? oldLocalSecret,
  SecretKeyOrigin? origin,
- bool? store,
+ bool? userPrefersToStoreSecret,
  bool? secureWithBiometrics,
  Option<LocalPassword>? localPassword,
+ Option<LocalPassword>? updatedLocalPassword,
  bool? isBiometricsAvailable,
  }) =>
  RepoSecurityState(
  oldLocalSecretMode: oldLocalSecretMode ?? this.oldLocalSecretMode,
  oldLocalSecret: oldLocalSecret ?? this.oldLocalSecret,
  origin: origin ?? this.origin,
- store: store ?? this.store,
+ userPrefersToStoreSecret: userPrefersToStoreSecret != null
+ ? Some(userPrefersToStoreSecret)
+ : this.userPrefersToStoreSecret,
  secureWithBiometrics: secureWithBiometrics ?? this.secureWithBiometrics,
  localPassword: localPassword ?? this.localPassword,
+ updatedLocalPassword: updatedLocalPassword ?? this.updatedLocalPassword,
  isBiometricsAvailable:
  isBiometricsAvailable ?? this.isBiometricsAvailable,
  );
@@ -67,48 +79,70 @@ class RepoSecurityState {
  };
 
  bool get isSecureWithBiometricsEnabled =>
- isBiometricsAvailable && (origin == SecretKeyOrigin.random || store);
+ isBiometricsAvailable && secretWillBeStored;
 
  bool get isValid => newLocalSecretInput != null;
 
- bool get hasPendingChanges =>
- origin != oldLocalSecretMode.origin ||
- store != oldLocalSecretMode.store.isStored ||
- secureWithBiometrics !=
- oldLocalSecretMode.store.isSecuredWithBiometrics ||
- localPassword is Some;
-
- LocalSecretInput? get newLocalSecretInput =>
- switch ((localPassword, origin, store, secureWithBiometrics)) {
- (Some(value: final password), SecretKeyOrigin.manual, false, _) =>
- LocalSecretManual(
- password: password,
- store: SecretKeyStore.notStored,
- ),
- (Some(value: final password), SecretKeyOrigin.manual, true, false) =>
- LocalSecretManual(
- password: password,
- store: SecretKeyStore.stored,
- ),
- (Some(value: final password), SecretKeyOrigin.manual, true, true) =>
- LocalSecretManual(
- password: password,
- store: SecretKeyStore.securedWithBiometrics,
- ),
- (None(), SecretKeyOrigin.manual, _, _) => null,
- (_, SecretKeyOrigin.random, _, false) =>
- LocalSecretRandom(secureWithBiometrics: false),
- (_, SecretKeyOrigin.random, _, true) =>
- LocalSecretRandom(secureWithBiometrics: true),
+ bool get secretWillBeStored =>
+ origin == SecretKeyOrigin.random ||
+ switch (userPrefersToStoreSecret) {
+ Some(value: final store) => store,
+ None() => _defaultStoreIfOriginIsManual
  };
 
+ bool get hasPendingChanges {
+ final originChanged = origin != oldLocalSecretMode.origin;
+
+ final localPasswordChanged =
+ localPassword is Some && localPassword != updatedLocalPassword;
+
+ final storeChanged =
+ secretWillBeStored != oldLocalSecretMode.store.isStored;
+
+ final biometricsChanged = secureWithBiometrics !=
+ oldLocalSecretMode.store.isSecuredWithBiometrics;
+
+ return originChanged ||
+ storeChanged ||
+ biometricsChanged ||
+ localPasswordChanged;
+ }
+
+ LocalSecretInput? get newLocalSecretInput {
+ final willStore = secretWillBeStored;
+
+ return switch ((localPassword, origin, willStore, secureWithBiometrics)) {
+ (Some(value: final password), SecretKeyOrigin.manual, false, _) =>
+ LocalSecretManual(
+ password: password,
+ store: SecretKeyStore.notStored,
+ ),
+ (Some(value: final password), SecretKeyOrigin.manual, true, false) =>
+ LocalSecretManual(
+ password: password,
+ store: SecretKeyStore.stored,
+ ),
+ (Some(value: final password), SecretKeyOrigin.manual, true, true) =>
+ LocalSecretManual(
+ password: password,
+ store: SecretKeyStore.securedWithBiometrics,
+ ),
+ (None(), SecretKeyOrigin.manual, _, _) => null,
+ (_, SecretKeyOrigin.random, _, false) =>
+ LocalSecretRandom(secureWithBiometrics: false),
+ (_, SecretKeyOrigin.random, _, true) =>
+ LocalSecretRandom(secureWithBiometrics: true),
+ };
+ }
+
  LocalPassword? get newLocalPassword => switch ((localPassword, origin)) {
  (Some(value: final value), SecretKeyOrigin.manual) => value,
  (None(), SecretKeyOrigin.manual) || (_, SecretKeyOrigin.random) => null,
  };
 
  @override
- String toString() => '$runtimeType(origin: $origin, store: $store, ...)';
+ String toString() =>
+ '$runtimeType(origin: $origin, userPrefersToStoreSecret: $userPrefersToStoreSecret, ...)';
 }
 
 class RepoSecurityCubit extends Cubit<RepoSecurityState> with AppLogger {
@@ -134,7 +168,7 @@ class RepoSecurityCubit extends Cubit<RepoSecurityState> with AppLogger {
  }
 
  void setStore(bool value) {
- emit(state.copyWith(store: value));
+ emit(state.copyWith(userPrefersToStoreSecret: value));
  }
 
  void setSecureWithBiometrics(bool value) {
@@ -170,7 +204,17 @@ class RepoSecurityCubit extends Cubit<RepoSecurityState> with AppLogger {
  // Save the new auth mode
  try {
  await repoCubit.setAuthMode(newAuthMode);
- emit(state.copyWith(oldLocalSecretMode: newAuthMode.localSecretMode));
+
+ Option<LocalPassword> newLocalPassword =
+ newLocalSecretInput is LocalSecretManual
+ ? Some(newLocalSecretInput.password)
+ : None<LocalPassword>();
+
+ emit(state.copyWith(
+ oldLocalSecretMode: newAuthMode.localSecretMode,
+ updatedLocalPassword: newLocalPassword,
+ ));
+
  loggy.debug('Repo auth mode updated: $newAuthMode');
  } catch (e, st) {
  loggy.error(
@@ -216,8 +260,8 @@ class RepoSecurityCubit extends Cubit<RepoSecurityState> with AppLogger {
  //}
 }
 
-// We want store to be explicitly opt-in so the switch must be initially off even if the
-// initial origin is random which is implicitly stored.
+// We want `store` to be explicitly opt-in so the switch must be initially off even if the
+// initial `origin` is random which is implicitly stored.
 bool _initialStore(LocalSecretMode localSecretMode) =>
  switch (localSecretMode) {
  LocalSecretMode.manualStored ||

lib/app/pages/repo_security_page.dart

@@ -81,7 +81,7 @@ class RepoSecurityPage extends StatelessWidget {
  child: Text(S.current.actionCancel),
  onPressed: () => Navigator.of(context).pop(false)),
  TextButton(
- child: Text(S.current.actionAccept),
+ child: Text(S.current.actionDiscard),
  onPressed: () => Navigator.of(context).pop(true))
  ],
  ).then((pop) {

lib/app/utils/option.dart

@@ -5,18 +5,39 @@ sealed class Option<T> {
  const Option();
 
  T? get value;
+
+ @override
+ bool operator ==(Object other) {
+ if (other is! Option<T>) return false;
+ switch ((this, other)) {
+ case (Some(value: var lv), Some(value: var rv)):
+ return lv == rv;
+ case (None, None):
+ return true;
+ case (_, _):
+ return false;
+ }
+ }
+
+ String toString();
 }
 
 class Some<T> extends Option<T> {
  const Some(this.value);
 
  @override
  final T value;
+
+ @override
+ String toString() => "Some($value)";
 }
 
 class None<T> extends Option<T> {
  const None();
 
  @override
  T? get value => null;
+
+ @override
+ String toString() => "None";
 }

lib/app/widgets/repo_security.dart

@@ -62,7 +62,7 @@ class RepoSecurity extends StatelessWidget {
 
  Widget _buildStoreSwitch(RepoSecurityState state) => switch (state.origin) {
  SecretKeyOrigin.manual => _buildSwitch(
- value: state.store,
+ value: state.secretWillBeStored,
  title: S.current.labelRememberPassword,
  onChanged: cubit.setStore,
  ),

lib/generated/intl/messages_all.dart

@@ -11,6 +11,7 @@
 
 import 'dart:async';
 
+import 'package:flutter/foundation.dart';
 import 'package:intl/intl.dart';
 import 'package:intl/message_lookup_by_library.dart';
 import 'package:intl/src/intl_helpers.dart';
@@ -27,15 +28,15 @@ import 'messages_zh-TW.dart' as messages_zh_tw;
 
 typedef Future<dynamic> LibraryLoader();
 Map<String, LibraryLoader> _deferredLibraries = {
- 'en': () => new Future.value(null),
- 'es': () => new Future.value(null),
- 'fa': () => new Future.value(null),
- 'fr': () => new Future.value(null),
- 'my': () => new Future.value(null),
- 'ru': () => new Future.value(null),
- 'uk': () => new Future.value(null),
- 'zh_CN': () => new Future.value(null),
- 'zh_TW': () => new Future.value(null),
+ 'en': () => new SynchronousFuture(null),
+ 'es': () => new SynchronousFuture(null),
+ 'fa': () => new SynchronousFuture(null),
+ 'fr': () => new SynchronousFuture(null),
+ 'my': () => new SynchronousFuture(null),
+ 'ru': () => new SynchronousFuture(null),
+ 'uk': () => new SynchronousFuture(null),
+ 'zh_CN': () => new SynchronousFuture(null),
+ 'zh_TW': () => new SynchronousFuture(null),
 };
 
 MessageLookupByLibrary? _findExact(String localeName) {
@@ -64,18 +65,18 @@ MessageLookupByLibrary? _findExact(String localeName) {
 }
 
 /// User programs should call this before using [localeName] for messages.
-Future<bool> initializeMessages(String localeName) async {
+Future<bool> initializeMessages(String localeName) {
  var availableLocale = Intl.verifiedLocale(
  localeName, (locale) => _deferredLibraries[locale] != null,
  onFailure: (_) => null);
  if (availableLocale == null) {
- return new Future.value(false);
+ return new SynchronousFuture(false);
  }
  var lib = _deferredLibraries[availableLocale];
- await (lib == null ? new Future.value(false) : lib());
+ lib == null ? new SynchronousFuture(false) : lib();
  initializeInternalMessageLookup(() => new CompositeMessageLookup());
  messageLookup.addLocale(availableLocale, _findGeneratedMessagesFor);
- return new Future.value(true);
+ return new SynchronousFuture(true);
 }
 
 bool _messagesExistFor(String locale) {

lib/generated/intl/messages_en.dart

@@ -753,7 +753,7 @@ class MessageLookup extends MessageLookupByLibrary {
  "messageUnlockUsingBiometrics":
  MessageLookupByLibrary.simpleMessage("Unlock using biometrics"),
  "messageUnsavedChanges": MessageLookupByLibrary.simpleMessage(
- "You have unsaved changes.\n\nDo you want to discard them?"),
+ "You have unsaved changes.\n\nWould you like to discard them?"),
  "messageUpdateLocalPasswordConfirmation":
  MessageLookupByLibrary.simpleMessage(
  "Update this repository localpassword?"),

lib/l10n/intl_en.arb

@@ -426,7 +426,7 @@
  "messageVPN":"VPN",
  "messageNone":"None",
  "messagePeerExchange":"Peer Exchange",
- "messageUnsavedChanges":"You have unsaved changes.\n\nDo you want to discard them?",
+ "messageUnsavedChanges":"You have unsaved changes.\n\nWould you like to discard them?",
  "messageAccessingSecureStorage":"Accessing secure storage",
  "messageSavingChanges":"Do you want to save the current changes?",
  "messageUpdateLocalSecretFailed":"Updating security properties of the repository failed.",
@@ -704,4 +704,4 @@
  "dokanUrl": {}
  }
  }
-}
+}