Skip to content

Commit

Permalink
Simplify LoginDialog code (#1707)
Browse files Browse the repository at this point in the history
* [login_store] simplify code a bit

* [login_dialog] remove obsolete `autoclose_on_success` arg

* [login_dialog] remove unused `canPop` arg

* [login_dialog] refactor `getPin` to `ensureAuthenticated`

* better documentation

* [AccountManagePage] fix redundant pop

* fix typos in integration test
  • Loading branch information
clangenb authored Nov 12, 2024
1 parent 289fa7b commit 1708a27
Show file tree
Hide file tree
Showing 15 changed files with 64 additions and 46 deletions.
1 change: 1 addition & 0 deletions app/lib/l10n/arb/app_de.arb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
"addToContactFromQrContact": "Kontakt per Qr hinzufügen",
"alreadyEndorsedErrorBody": "Dieses Konto wurde bereits für diesen Key-Signing Cycle endorsed.",
"alreadyEndorsedErrorTitle": "Bereits endorsed",
"authenticationNeeded": "Authentifizierung notwendig",
"amountError": "Ungültiger Betrag",
"amountToBeTransferred": "Betrag",
"appSettings": "App-Einstellungen",
Expand Down
1 change: 1 addition & 0 deletions app/lib/l10n/arb/app_en.arb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
"addToContactFromQrContact": "Add Contact-Qr",
"alreadyEndorsedErrorBody": "This account has already been endorsed for this cycle.",
"alreadyEndorsedErrorTitle": "Already Endorsed",
"authenticationNeeded": "Authentication Needed",
"amountError": "Invalid amount",
"amountToBeTransferred": "Send amount",
"appSettings": "App settings",
Expand Down
1 change: 1 addition & 0 deletions app/lib/l10n/arb/app_fr.arb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
"addToContactFromQrContact": "Add Contact-Qr",
"alreadyEndorsedErrorBody": "Ce compte a déjà été endossé pour ce cycle",
"alreadyEndorsedErrorTitle": "Déjà endossé",
"authenticationNeeded": "Authentification nécessaire",
"amountError": "Solde non valide",
"amountToBeTransferred": "Montant",
"appSettings": "Paramètres de l'app",
Expand Down
1 change: 1 addition & 0 deletions app/lib/l10n/arb/app_ru.arb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
"addToContactFromQrContact": "Add Contact-Qr",
"alreadyEndorsedErrorBody": "Этот аккаунт уже был подтвержден в этом цикле.",
"alreadyEndorsedErrorTitle": "Уже подтверждено",
"authenticationNeeded": "Необходима аутентификация",
"amountError": "Недопустимая сумма",
"amountToBeTransferred": "Отправить сумму",
"appSettings": "Настройки приложения",
Expand Down
1 change: 1 addition & 0 deletions app/lib/l10n/arb/app_sw.arb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
"addToContactFromQrContact": "Ongeza Qr ya mawasiliano",
"alreadyEndorsedErrorBody": "Akaunti hii tayari imeidhinishwa kwa ajili ya mzunguko huu",
"alreadyEndorsedErrorTitle": "imeidhinishwa tayari",
"authenticationNeeded": "Uthibitishaji Unahitajika",
"amountError": "Kiasi kisichokubalika",
"amountToBeTransferred": "Tuma kiasi",
"appSettings": "Mipangilio ya app",
Expand Down
7 changes: 5 additions & 2 deletions app/lib/modules/account/view/add_account_view.dart
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import 'package:encointer_wallet/utils/snack_bar.dart';
import 'package:ew_test_keys/ew_test_keys.dart';
import 'package:flutter/material.dart';
import 'package:flutter_mobx/flutter_mobx.dart';
Expand Down Expand Up @@ -108,14 +109,16 @@ class AddAccountForm extends StatelessWidget with HandleNewAccountResultMixin {
final newAccount = context.read<NewAccountStore>();
if (_formKey.currentState!.validate() && !newAccount.loading) {
newAccount.setName(_nameCtrl.text.trim());
final pin = await context.read<LoginStore>().getPin(context);
if (pin != null) {
final authenticated = await context.read<LoginStore>().ensureAuthenticated(context);
if (authenticated) {
final res = await newAccount.generateAccount();
await navigate(
context: context,
type: res.operationResult,
onOk: () => Navigator.of(context).popUntil((route) => route.isFirst),
);
} else {
RootSnackBar.showMsg(context.l10n.authenticationNeeded);
}
}
},
Expand Down
7 changes: 5 additions & 2 deletions app/lib/modules/account/view/import_account_view.dart
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import 'package:encointer_wallet/utils/snack_bar.dart';
import 'package:ew_test_keys/ew_test_keys.dart';
import 'package:flutter/cupertino.dart';
import 'package:flutter/material.dart';
Expand Down Expand Up @@ -118,15 +119,17 @@ class ImportAccountForm extends StatelessWidget with HandleNewAccountResultMixin
),
);
} else {
final pin = await context.read<LoginStore>().getPin(context);
if (pin != null) {
final authenticated = await context.read<LoginStore>().ensureAuthenticated(context);
if (authenticated) {
final res = await newAccount.importAccount();
await navigate(
context: context,
type: res.operationResult,
onOk: () => Navigator.of(context).popUntil((route) => route.isFirst),
onDuplicateAccount: () => _onDuplicateAccount(context, res.duplicateAccountData),
);
} else {
RootSnackBar.showMsg(l10n.authenticationNeeded);
}
}
}
Expand Down
14 changes: 7 additions & 7 deletions app/lib/modules/login/logic/login_store.dart
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@ abstract class _LoginStoreBase with Store {
@observable
bool loading = false;

FutureOr<String?> getPin(BuildContext context) async {
if (cachedPin != null) return cachedPin!;
await LoginDialog.verifyPinOrBioAuth(
context,
onSuccess: (v) async => cachedPin = await loginService.getPin(),
);
return cachedPin;
/// If the user has already authenticated this session, true will be returned.
/// if not, the user will be asked to authenticate with PIN or biometric depending on the
/// settings.
FutureOr<bool> ensureAuthenticated(BuildContext context) async {
if (cachedPin != null) return true;
await LoginDialog.verifyPinOrBioAuth(context);
return cachedPin != null;
}

/// Persists the new PIN in the secure storage.
Expand Down
45 changes: 24 additions & 21 deletions app/lib/modules/login/view/login_dialog.dart
Original file line number Diff line number Diff line change
Expand Up @@ -39,21 +39,19 @@ final class LoginDialog {
? showLocalAuth(
context,
titleText: context.l10n.biometricAuthEnableDisableDescription,
onSuccess: (_) => context.read<LoginStore>().setBiometricAuthState(BiometricAuthState.enabled),
onSuccess: () => context.read<LoginStore>().setBiometricAuthState(BiometricAuthState.enabled),
)
: showPasswordInputDialog(
context,
titleText: context.l10n.biometricAuthEnableDisableDescription,
onSuccess: (_) => context.read<LoginStore>().setBiometricAuthState(BiometricAuthState.disabled),
onSuccess: () => context.read<LoginStore>().setBiometricAuthState(BiometricAuthState.disabled),
);

static Future<void> verifyPinOrBioAuth(
BuildContext context, {
required Future<void> Function(String password) onSuccess,
Future<void> Function()? onSuccess,
bool barrierDismissible = true,
bool autoCloseOnSuccess = true,
bool showCancelButton = true,
bool canPop = true,
bool stickyAuth = false,
String? titleText,
}) async {
Expand All @@ -72,9 +70,7 @@ final class LoginDialog {
context,
onSuccess: onSuccess,
barrierDismissible: barrierDismissible,
autoCloseOnSuccess: autoCloseOnSuccess,
showCancelButton: showCancelButton,
canPop: canPop,
titleText: titleText,
);
}
Expand All @@ -83,36 +79,41 @@ final class LoginDialog {
context,
onSuccess: onSuccess,
barrierDismissible: barrierDismissible,
autoCloseOnSuccess: autoCloseOnSuccess,
showCancelButton: showCancelButton,
canPop: canPop,
titleText: titleText,
);
}
}

static Future<void> showLocalAuth(
BuildContext context, {
required Future<void> Function(String password) onSuccess,
required Future<void> Function()? onSuccess,
String? titleText,
bool stickyAuth = false,
}) async {
final loginStore = context.read<LoginStore>();
try {
final value = await loginStore.localAuthenticate(titleText ?? context.l10n.verifyAuthTitle('true'), stickyAuth);
if (value) await onSuccess(loginStore.cachedPin ?? await loginStore.loginService.getPin() ?? '');
final success = await loginStore.localAuthenticate(titleText ?? context.l10n.verifyAuthTitle('true'), stickyAuth);

if (success) {
loginStore.cachedPin = await loginStore.loginService.getPin();

if (loginStore.cachedPin == null) {
throw Exception(['Pin retrieved from storage is null, fatal error']);
}

if (onSuccess != null) await onSuccess();
}
} catch (e) {
rethrow;
}
}

static Future<void> showPasswordInputDialog(
BuildContext context, {
required Future<void> Function(String password) onSuccess,
Future<void> Function()? onSuccess,
bool barrierDismissible = true,
bool autoCloseOnSuccess = true,
bool showCancelButton = true,
bool canPop = true,
String? titleText,
}) async {
final l10n = context.l10n;
Expand Down Expand Up @@ -142,15 +143,17 @@ final class LoginDialog {
child: Text(l10n.cancel),
),
PopScope(
onPopInvoked: (bool didPop) async => canPop,
child: CupertinoButton(
key: const Key(EWTestKeys.passwordOk),
onPressed: () async {
loginStore.loading = true;
final value = await _onOk(context, passCtrl.text.trim());
if (value) {
await onSuccess(passCtrl.text.trim());
if (autoCloseOnSuccess) Navigator.of(context).pop();
final success = await _checkPassword(context, passCtrl.text.trim());
if (success) {
loginStore.cachedPin = passCtrl.text.trim();

if (onSuccess != null) await onSuccess();

Navigator.of(context).pop();
} else {
AppAlert.showErrorDialog(
context,
Expand All @@ -170,7 +173,7 @@ final class LoginDialog {
);
}

static Future<bool> _onOk(BuildContext context, String password) async {
static Future<bool> _checkPassword(BuildContext context, String password) async {
final loginStore = context.read<LoginStore>();
return loginStore.isValid(password);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import 'dart:async';

import 'package:encointer_wallet/utils/snack_bar.dart';
import 'package:flutter/cupertino.dart';
import 'package:flutter/material.dart';
import 'package:iconsax/iconsax.dart';
Expand Down Expand Up @@ -141,8 +142,8 @@ class _PaymentConfirmationPageState extends State<PaymentConfirmationPage> {
Address recipientAddress,
double amount,
) async {
final pin = await context.read<LoginStore>().getPin(context);
if (pin != null) {
final authenticated = await context.read<LoginStore>().ensureAuthenticated(context);
if (authenticated) {
setState(() {
_transferState = TransferState.submitting;
});
Expand All @@ -164,6 +165,8 @@ class _PaymentConfirmationPageState extends State<PaymentConfirmationPage> {
Log.d('TransferState after callback: $_transferState', 'PaymentConfirmationPage');
// trigger rebuild after state update in callback
setState(() {});
} else {
RootSnackBar.showMsg(context.l10n.authenticationNeeded);
}
}

Expand Down
6 changes: 2 additions & 4 deletions app/lib/page/profile/account/account_manage_page.dart
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ class _AccountManagePageState extends State<AccountManagePage> {
LoginDialog.verifyPinOrBioAuth(
context,
titleText: context.l10n.accountDelete,
onSuccess: (v) async {
onSuccess: () async {
await _appStore.account
.removeAccount(accountToBeEdited)
.then((_) => _appStore.loadAccountCache())
Expand Down Expand Up @@ -142,9 +142,7 @@ class _AccountManagePageState extends State<AccountManagePage> {
await LoginDialog.verifyPinOrBioAuth(
context,
titleText: context.l10n.confirmPin,
autoCloseOnSuccess: false,
onSuccess: (password) async {
Navigator.of(context).pop();
onSuccess: () async {
final account = _appStore.account.getKeyringAccount(accountToBeEdited.pubKey);
await Navigator.of(context).pushNamed(ExportResultPage.route, arguments: {
'key': account.uri,
Expand Down
2 changes: 1 addition & 1 deletion app/lib/page/profile/index.dart
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ class _ProfileState extends State<Profile> {
LoginDialog.verifyPinOrBioAuth(
context,
titleText: l10n.accountsDelete,
onSuccess: (v) async {
onSuccess: () async {
for (final acc in context.read<AppStore>().account.accountListAll) {
await store.account.removeAccount(acc);
}
Expand Down
7 changes: 5 additions & 2 deletions app/lib/service/tx/lib/src/submit_tx_wrappers.dart
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import 'package:encointer_wallet/utils/snack_bar.dart';
import 'package:flutter/cupertino.dart';
import 'package:provider/provider.dart';

Expand Down Expand Up @@ -43,8 +44,8 @@ Future<void> submitTx(
dynamic Function(BuildContext txPageContext, ExtrinsicReport report)? onFinish,
void Function(DispatchError report)? onError,
}) async {
final pin = await context.read<LoginStore>().getPin(context);
if (pin != null) {
final authenticated = await context.read<LoginStore>().ensureAuthenticated(context);
if (authenticated) {
return submitTxInner(
context,
store,
Expand All @@ -54,6 +55,8 @@ Future<void> submitTx(
onError: onError,
onFinish: onFinish,
);
} else {
RootSnackBar.showMsg(context.l10n.authenticationNeeded);
}
}

Expand Down
2 changes: 1 addition & 1 deletion app/test_driver/app/profile/profile_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ Future<void> getNextPhase(FlutterDriver driver) async {
await tapNextPhase(driver);
}

Future<void> checkPeputationCount(FlutterDriver driver, int count) async {
Future<void> checkReputationCount(FlutterDriver driver, int count) async {
await driver.waitFor(find.text('$count'));
}

Expand Down
8 changes: 4 additions & 4 deletions app/test_driver/app_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ void main() async {

test('Go to Profile Page and Check reputation count', () async {
await goToProfileViewFromNavBar(driver);
await checkPeputationCount(driver, 2);
await checkReputationCount(driver, 2);
}, timeout: timeout240);

test('Get Registering phase', () async {
Expand All @@ -253,7 +253,7 @@ void main() async {
await sendEndorse(driver);
}, timeout: timeout240);

test('send money to account from Bootstraper account', () async {
test('send money to account from Bootstrapper account', () async {
await senMoneyToContact(driver);
await sendMoneyToSelectedAccount(driver, '0.2');
await goToHomeViewFromNavBar(driver);
Expand All @@ -270,7 +270,7 @@ void main() async {
await verifyInputPin(driver);
}, timeout: timeout240);

test('create niewbie Account', () async {
test('create newbie Account', () async {
await goToHomeViewFromNavBar(driver);
await goToAddAcoountViewFromPanel(driver);
await createNewbieAccount(driver, 'Li');
Expand Down Expand Up @@ -353,7 +353,7 @@ void main() async {
await verifyInputPin(driver);
}, timeout: timeout240);

test('import account with menemonic phrase', () async {
test('import account with mnemonic phrase', () async {
await goToHomeViewFromNavBar(driver);
await goToAddAcoountViewFromPanel(driver);
await importAccount(driver, 'Bob', menemonic);
Expand Down

0 comments on commit 1708a27

Please sign in to comment.