diff --git a/dependencies b/dependencies index 60da6275..4d22ef84 160000 --- a/dependencies +++ b/dependencies @@ -1 +1 @@ -Subproject commit 60da6275f8c82c4238f0c3cf88a798e63b4a2ffd +Subproject commit 4d22ef849031619d6bb4dcf8ea1709d3c1ff8b3f diff --git a/lib/src/localization/generated/app_localizations.dart b/lib/src/localization/generated/app_localizations.dart index 9e408735..14b7a54a 100644 --- a/lib/src/localization/generated/app_localizations.dart +++ b/lib/src/localization/generated/app_localizations.dart @@ -4088,6 +4088,18 @@ abstract class AppLocalizations { /// **'Recovery share sent!'** String get passwordlessRecoveryShareSent; + /// No description provided for @passwordlessRecoveryAuthReason. + /// + /// In en, this message translates to: + /// **'Authenticate to send your recovery share'** + String get passwordlessRecoveryAuthReason; + + /// No description provided for @passwordlessRecoveryAuthFailed. + /// + /// In en, this message translates to: + /// **'Authentication failed, the recovery share was not sent.'** + String get passwordlessRecoveryAuthFailed; + /// No description provided for @passwordlessRecoveryNetworkError. /// /// In en, this message translates to: diff --git a/lib/src/localization/generated/app_localizations_de.dart b/lib/src/localization/generated/app_localizations_de.dart index 8f829e83..f2c22dbf 100644 --- a/lib/src/localization/generated/app_localizations_de.dart +++ b/lib/src/localization/generated/app_localizations_de.dart @@ -2364,6 +2364,14 @@ class AppLocalizationsDe extends AppLocalizations { String get passwordlessRecoveryShareSent => 'Wiederherstellungs-Teil gesendet!'; + @override + String get passwordlessRecoveryAuthReason => + 'Authentifiziere dich, um deinen Wiederherstellungs-Teil zu senden'; + + @override + String get passwordlessRecoveryAuthFailed => + 'Authentifizierung fehlgeschlagen, der Wiederherstellungs-Teil wurde nicht gesendet.'; + @override String get passwordlessRecoveryNetworkError => 'Netzwerkfehler, bitte stelle sicher, dass du Internet hast'; diff --git a/lib/src/localization/generated/app_localizations_en.dart b/lib/src/localization/generated/app_localizations_en.dart index c85c9a20..acd518fa 100644 --- a/lib/src/localization/generated/app_localizations_en.dart +++ b/lib/src/localization/generated/app_localizations_en.dart @@ -2343,6 +2343,14 @@ class AppLocalizationsEn extends AppLocalizations { @override String get passwordlessRecoveryShareSent => 'Recovery share sent!'; + @override + String get passwordlessRecoveryAuthReason => + 'Authenticate to send your recovery share'; + + @override + String get passwordlessRecoveryAuthFailed => + 'Authentication failed, the recovery share was not sent.'; + @override String get passwordlessRecoveryNetworkError => 'Network error, please ensure you have internet'; diff --git a/lib/src/localization/translations/de.arb b/lib/src/localization/translations/de.arb index 04c7cff0..8a2a686f 100644 --- a/lib/src/localization/translations/de.arb +++ b/lib/src/localization/translations/de.arb @@ -870,6 +870,8 @@ "passwordlessRecoveryNoShareStored": "Kein Wiederherstellungs-Teil für diesen Kontakt gespeichert.", "passwordlessRecoveryEmailSent": "Wiederherstellungs-E-Mail gesendet!", "passwordlessRecoveryShareSent": "Wiederherstellungs-Teil gesendet!", + "passwordlessRecoveryAuthReason": "Authentifiziere dich, um deinen Wiederherstellungs-Teil zu senden", + "passwordlessRecoveryAuthFailed": "Authentifizierung fehlgeschlagen, der Wiederherstellungs-Teil wurde nicht gesendet.", "passwordlessRecoveryNetworkError": "Netzwerkfehler, bitte stelle sicher, dass du Internet hast", "passwordlessRecoveryInvalidEmail": "Die E-Mail-Adresse ist ungültig.", "passwordlessRecoveryResendEmail": "E-Mail erneut senden", diff --git a/lib/src/localization/translations/en.arb b/lib/src/localization/translations/en.arb index d64da670..eeb36aca 100644 --- a/lib/src/localization/translations/en.arb +++ b/lib/src/localization/translations/en.arb @@ -880,6 +880,8 @@ "passwordlessRecoveryNoShareStored": "No recovery share stored for this contact.", "passwordlessRecoveryEmailSent": "Recovery email sent!", "passwordlessRecoveryShareSent": "Recovery share sent!", + "passwordlessRecoveryAuthReason": "Authenticate to send your recovery share", + "passwordlessRecoveryAuthFailed": "Authentication failed, the recovery share was not sent.", "passwordlessRecoveryNetworkError": "Network error, please ensure you have internet", "passwordlessRecoveryInvalidEmail": "The email address is invalid.", "passwordlessRecoveryResendEmail": "Resend recovery email", diff --git a/lib/src/services/api/api.service.dart b/lib/src/services/api/api.service.dart index c221de8d..f41841cd 100644 --- a/lib/src/services/api/api.service.dart +++ b/lib/src/services/api/api.service.dart @@ -53,6 +53,14 @@ class ApiService { // Function is called after the user is authenticated at the server Future onAuthenticated() async { + // A passwordless recovery restores the identity into the key manager + // before a user config exists, so the socket can authenticate while + // `currentUser` is still unset. Everything below reads that config. + if (!userService.isUserCreated) { + Log.info('Skipping onAuthenticated: the user config is not loaded yet'); + return; + } + await FcmNotificationService.initFCMAfterAuthenticated(); if (!AppState.isAppInBackground) { diff --git a/lib/src/services/backup.service.dart b/lib/src/services/backup.service.dart index 08809efe..77bcfa2c 100644 --- a/lib/src/services/backup.service.dart +++ b/lib/src/services/backup.service.dart @@ -338,12 +338,15 @@ class BackupService { await RustBackupArchive.restoreBackupArchive( filePath: archiveFile.path, ); + Log.info('Restored the backup archive.'); await UserService.update((u) { u.deviceId += 1; }); + Log.info('Bumped the device id after the recovery.'); await KeyValueStore.delete( KeyValueKeys.backupRecoveryState, ); + Log.info('Recovery finished, restarting the app.'); } catch (e) { Log.error(e); return RecoveryError.unkownError; @@ -411,8 +414,10 @@ class BackupService { // Import KeyManager keys into secure storage & in-memory key manager await RustKeyManager.importSerialized(serializedBytes: keyManagerBytes); + Log.info('Imported the recovered key manager.'); await KeyValueStore.put(KeyValueKeys.backupRecoveryState, state.toJson()); + Log.info('Stored the recovery state, entering the archive stage.'); return _nextBackupStage(onProgress: onProgress); } diff --git a/lib/src/services/notifications/fcm.notifications.dart b/lib/src/services/notifications/fcm.notifications.dart index 8231bf05..fa746772 100644 --- a/lib/src/services/notifications/fcm.notifications.dart +++ b/lib/src/services/notifications/fcm.notifications.dart @@ -30,6 +30,10 @@ class FcmNotificationService { } static Future initFCMAfterAuthenticated({bool force = false}) async { + // Reading `currentUser` before a config is loaded throws a + // LateInitializationError, which an authenticated socket can trigger + // during a recovery. + if (!userService.isUserCreated) return; final fcmToken = userService.currentUser.fcmToken; if (userService.currentUser.updateFcmToken || force) { if (fcmToken == null) { diff --git a/lib/src/services/passwordless_recovery.service.dart b/lib/src/services/passwordless_recovery.service.dart index fe483a70..51c909a7 100644 --- a/lib/src/services/passwordless_recovery.service.dart +++ b/lib/src/services/passwordless_recovery.service.dart @@ -384,10 +384,9 @@ class PasswordlessRecoveryService { alreadyReceivedMessageIds: alreadyReceivedIds, ); } catch (error) { - Log.error( - 'Failed to load passwordless recovery messages', - error: error, - ); + // This runs on a 10s poll, so a socket that is still connecting or + // briefly offline is expected. The next tick retries. + Log.warn('Failed to load passwordless recovery messages', error); return false; } diff --git a/lib/src/utils/log.dart b/lib/src/utils/log.dart index 1b2bb80d..9192f21c 100644 --- a/lib/src/utils/log.dart +++ b/lib/src/utils/log.dart @@ -29,7 +29,7 @@ class Log { record.level >= Level.WARNING) { // ignore: avoid_print print( - '${record.level.name} [f] [twonly] ${record.loggerName} > ${record.message}', + '${record.level.name} [f] [twonly] ${record.loggerName} > ${_formatRecord(record)}', ); } } @@ -71,7 +71,7 @@ class Log { _ => rust_logging.LogLevel.finest, }, source: record.loggerName, - message: record.message, + message: _formatRecord(record), // Background work runs natively now; anything logged from Dart is by // definition the foreground runtime. inBackground: false, @@ -86,6 +86,20 @@ class Log { } } + /// Folds the optional [LogRecord.error] and [LogRecord.stackTrace] into the + /// rendered line. Without this, callers passing an error see only their own + /// message and never the cause. + static String _formatRecord(LogRecord record) { + final buffer = StringBuffer(record.message); + if (record.error != null) { + buffer.write(': ${filterLogMessage('${record.error}')}'); + } + if (record.stackTrace != null) { + buffer.write('\n${record.stackTrace}'); + } + return buffer.toString(); + } + static String filterLogMessage(String msg) { if (msg.contains('SqliteException')) { // Do not log data which would be inserted into the DB. diff --git a/lib/src/utils/storage.dart b/lib/src/utils/storage.dart index 8c293823..2e09b546 100644 --- a/lib/src/utils/storage.dart +++ b/lib/src/utils/storage.dart @@ -5,6 +5,7 @@ import 'package:path_provider/path_provider.dart'; import 'package:twonly/core/bridge/wrapper/key_manager.dart'; import 'package:twonly/locator.dart'; import 'package:twonly/src/database/twonly.db.dart'; +import 'package:twonly/src/utils/log.dart'; /// Deletes local databases and files. /// @@ -13,8 +14,16 @@ import 'package:twonly/src/database/twonly.db.dart'; Future deleteLocalUserData({bool removeCredentials = false}) async { if (removeCredentials) { await RustKeyManager.removeLocalCredentials(); + Log.info('Removed the local credentials.'); + } + // The database files are deleted a few lines down either way, so a drift + // isolate that no longer answers must not block the whole recovery. + try { + await twonlyDB.close().timeout(const Duration(seconds: 5)); + Log.info('Closed the app database.'); + } catch (e) { + Log.warn('Could not close the app database, deleting it anyway', e); } - await twonlyDB.close(); // Wait for the background drift isolate to potentially shut down await Future.delayed(const Duration(milliseconds: 200)); @@ -29,5 +38,6 @@ Future deleteLocalUserData({bool removeCredentials = false}) async { locator ..unregister() ..registerLazySingleton(TwonlyDB.new); + Log.info('Deleted the local user data.'); return true; } diff --git a/lib/src/visual/elements/better_text.element.dart b/lib/src/visual/elements/better_text.element.dart index 80287ba4..a5640806 100644 --- a/lib/src/visual/elements/better_text.element.dart +++ b/lib/src/visual/elements/better_text.element.dart @@ -1,5 +1,6 @@ import 'package:flutter/gestures.dart'; import 'package:flutter/material.dart'; +import 'package:twonly/src/services/intent/links.intent.dart'; import 'package:twonly/src/utils/log.dart'; import 'package:url_launcher/url_launcher.dart'; @@ -10,15 +11,6 @@ final _urlRegExp = RegExp( caseSensitive: false, ); -Future _openUrl(String url) async { - final lUrl = Uri.parse(url.startsWith('http') ? url : 'http://$url'); - try { - await launchUrl(lUrl, mode: LaunchMode.externalApplication); - } catch (e) { - Log.error('Could not launch $e'); - } -} - class BetterText extends StatefulWidget { const BetterText({required this.text, required this.textColor, super.key}); final String text; @@ -64,6 +56,19 @@ class _BetterTextState extends State { _recognizers.clear(); } + /// twonly's own links (profile, QR and passwordless recovery links) are + /// handled in the app instead of being handed to the browser, which would + /// only land on a page that cannot do anything with the fragment. + Future _openUrl(String url) async { + final lUrl = Uri.parse(url.startsWith('http') ? url : 'http://$url'); + if (mounted && await handleIntentUrl(context, lUrl)) return; + try { + await launchUrl(lUrl, mode: LaunchMode.externalApplication); + } catch (e) { + Log.error('Could not launch $e'); + } + } + void _buildSpans() { final text = widget.text; final spans = []; diff --git a/lib/src/visual/views/onboarding/recovery_progress.view.dart b/lib/src/visual/views/onboarding/recovery_progress.view.dart index 66435415..814e732a 100644 --- a/lib/src/visual/views/onboarding/recovery_progress.view.dart +++ b/lib/src/visual/views/onboarding/recovery_progress.view.dart @@ -3,6 +3,7 @@ import 'dart:async'; import 'package:flutter/material.dart'; import 'package:restart_app/restart_app.dart'; import 'package:twonly/src/services/backup.service.dart'; +import 'package:twonly/src/utils/log.dart'; import 'package:twonly/src/utils/misc.dart'; import 'package:twonly/src/visual/elements/my_button.element.dart'; import 'package:twonly/src/visual/views/onboarding/components/link_logo_animation.dart'; @@ -67,12 +68,20 @@ class _RecoveryProgressViewState extends State { _error = null; }); - final error = await widget.runRecovery((progress) { - final index = widget.steps.indexOf(progress); - if (mounted && index >= 0) { - setState(() => _currentIndex = index); - } - }); + RecoveryError? error; + try { + error = await widget.runRecovery((progress) { + final index = widget.steps.indexOf(progress); + if (mounted && index >= 0) { + setState(() => _currentIndex = index); + } + }); + } catch (e, stackTrace) { + // Without this the step keeps spinning forever and the user is stuck on + // a screen they cannot leave, with no way to retry. + Log.error('Recovery failed', error: e, stackTrace: stackTrace); + error = RecoveryError.unkownError; + } if (!mounted) return; if (error != null) { @@ -82,11 +91,22 @@ class _RecoveryProgressViewState extends State { setState(() => _isFinishing = true); - await Restart.restartApp( - notificationTitle: context.lang.recoverSuccessTitle, - notificationBody: context.lang.recoverSuccessBody, - forceKill: true, - ); + try { + await Restart.restartApp( + notificationTitle: context.lang.recoverSuccessTitle, + notificationBody: context.lang.recoverSuccessBody, + forceKill: true, + ); + } catch (e, stackTrace) { + // The data is restored at this point, only the restart failed. Show the + // failure so the user knows to reopen the app themselves. + Log.error( + 'Restart after recovery failed', + error: e, + stackTrace: stackTrace, + ); + if (mounted) setState(() => _error = RecoveryError.unkownError); + } } _StepStatus _statusFor(int index) { diff --git a/lib/src/visual/views/settings/backup/passwordless_recovery/help_a_friend.passwordless_recovery.view.dart b/lib/src/visual/views/settings/backup/passwordless_recovery/help_a_friend.passwordless_recovery.view.dart index ed83ce64..bf0a7ac3 100644 --- a/lib/src/visual/views/settings/backup/passwordless_recovery/help_a_friend.passwordless_recovery.view.dart +++ b/lib/src/visual/views/settings/backup/passwordless_recovery/help_a_friend.passwordless_recovery.view.dart @@ -78,6 +78,19 @@ class _HelpAFriendPasswordlessRecoveryViewState } Future _submitShare(Contact contact) async { + final verified = await authenticateUser( + context.lang.passwordlessRecoveryAuthReason, + force: false, + ); + if (!mounted) return; + if (!verified) { + showSnackbar( + context, + context.lang.passwordlessRecoveryAuthFailed, + ); + return; + } + setState(() => _isLoading = true); final res = await PasswordlessRecoveryService.submitRecoveryShare( widget.notificationId, diff --git a/rust/src/api/server/passwordless.rs b/rust/src/api/server/passwordless.rs index 290b87ed..8c632117 100644 --- a/rust/src/api/server/passwordless.rs +++ b/rust/src/api/server/passwordless.rs @@ -124,6 +124,11 @@ impl Server { decode_ok_value(bytes, |value| match value { ResponseOk::PasswordlessNotificationMessages(msgs) => Some(msgs), + // The server answers with `None` whenever the poll finds no unseen + // message, which is the normal outcome once every share arrived. + ResponseOk::None(_) => Some( + proto::server_to_client::response::PasswordlessNotificationMessages::default(), + ), _ => None, }) } diff --git a/rust/src/backup/backup_archive.rs b/rust/src/backup/backup_archive.rs index d6b5eb52..188b124a 100644 --- a/rust/src/backup/backup_archive.rs +++ b/rust/src/backup/backup_archive.rs @@ -20,6 +20,19 @@ use zeroize::Zeroize; use zip::write::SimpleFileOptions; use zip::{CompressionMethod, ZipArchive, ZipWriter}; +/// How long a database pool gets to hand its connections back before the +/// restore stops waiting for it. +const POOL_CLOSE_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); + +async fn close_pool_or_log(pool: &sqlx::SqlitePool, name: &str) { + if tokio::time::timeout(POOL_CLOSE_TIMEOUT, pool.close()) + .await + .is_err() + { + tracing::warn!("{name} pool did not close within the timeout, replacing it anyway"); + } +} + pub(crate) struct BackupArchive {} const BACKUP_MANIFEST_FILE: &str = "backup-manifest.json"; @@ -261,10 +274,16 @@ impl BackupArchive { // app_db.sqlite is owned by a replaceable Rust handle. Close it before // replacing the file so subsequent DAO calls cannot continue using an // unlinked pre-restore database. + // `Pool::close` waits for every checked out connection to come back. A + // caller that still holds one would hang the restore here forever, + // while this task keeps the KeyManager locked and every later recovery + // attempt blocks on it too. The files are replaced right below and the + // handles are swapped out, so a pool that refuses to drain is not worth + // waiting for. let current_app_database = ctx.app_db.read().await.clone(); - current_app_database.pool.close().await; + close_pool_or_log(¤t_app_database.pool, "app_db").await; let current_rust_database = ctx.rust_db.read().await.clone(); - current_rust_database.pool.close().await; + close_pool_or_log(¤t_rust_database.pool, "rust_db").await; for (file_name, target_dir, is_db, _) in Self::get_backup_files(ctx, &key_manager)? { let src = restore_temp_dir.join(file_name); diff --git a/rust/src/services/groups/mod.rs b/rust/src/services/groups/mod.rs index 2574c0cb..f87d1c7d 100644 --- a/rust/src/services/groups/mod.rs +++ b/rust/src/services/groups/mod.rs @@ -478,20 +478,21 @@ impl GroupService { /// function of the state rather than a finished state: it can simply be /// applied again to the version that won. /// - /// `change` returning `None` means there is nothing left to write. + /// `change` returning `None` means there is nothing left to write, which + /// this reports back as `false`. async fn update_state_with_retry( &self, group: &GroupRecord, mut change: impl FnMut(&mut EncryptedGroupState) -> Result>, - ) -> Result<()> { + ) -> Result { let mut attempt = 1; loop { let (version, mut state) = GroupApi::load_state(group).await?; let Some(keys) = change(&mut state)? else { - return Ok(()); + return Ok(false); }; match GroupApi::update_remote(group, version, &state, keys.add, keys.remove).await { - Ok(()) => return Ok(()), + Ok(()) => return Ok(true), Err(TwonlyError::GroupStateConflict) if attempt < STATE_UPDATE_ATTEMPTS => { attempt += 1; tracing::info!( @@ -675,27 +676,32 @@ impl GroupService { .member_public_key(&db, &g, &group_id, contact_id) .await .ok(); - self.update_state_with_retry(&g, |state| { - if !state.member_ids.contains(&contact_id) { - return Ok(None); - } - state.member_ids.retain(|x| *x != contact_id); - let was_admin = state.admin_ids.contains(&contact_id); - state.admin_ids.retain(|x| *x != contact_id); - if !was_admin { - return Ok(Some(AdminKeys::default())); - } - let revoked = public_key.clone().ok_or_else(|| { - TwonlyError::Generic(format!( - "group public key for contact {contact_id} not found" - )) - })?; - Ok(Some(AdminKeys { - add: None, - remove: Some(revoked), - })) - }) - .await?; + let removed = self + .update_state_with_retry(&g, |state| { + if !state.member_ids.contains(&contact_id) { + return Ok(None); + } + state.member_ids.retain(|x| *x != contact_id); + let was_admin = state.admin_ids.contains(&contact_id); + state.admin_ids.retain(|x| *x != contact_id); + if !was_admin { + return Ok(Some(AdminKeys::default())); + } + let revoked = public_key.clone().ok_or_else(|| { + TwonlyError::Generic(format!( + "group public key for contact {contact_id} not found" + )) + })?; + Ok(Some(AdminKeys { + add: None, + remove: Some(revoked), + })) + }) + .await?; + if !removed { + // Somebody else already took them out of the group. + return Ok(true); + } self.announce(&group_id, "removedMember", Some(contact_id), None, None) .await?; let mut tr = db.pool.begin().await?; @@ -715,37 +721,60 @@ impl GroupService { pub async fn leave_group(&self, group_id: String) -> Result { let (db, group) = self.load_group(&group_id).await?; let user_id = self.ctx.user_id().await?; - let (_, group_state) = GroupApi::load_state(&group).await?; - if group_state.admin_ids.contains(&user_id) { - return self.remove_member(group_id, user_id).await; - } - let identity = group.identity()?; let public_key = identity.identity_key().serialize().to_vec(); - let append = EncryptedAppendedGroupState { - r#type: encrypted_appended_group_state::Type::LeftGroup as i32, - }; - let append_tbs = append_group_state::AppendTbs { - encrypted_group_state_append: crypto::encrypt( - group.state_key()?, - &append.encode_to_vec(), - )?, - public_key: public_key.clone(), - group_id: group_id.clone(), - nonce: GroupApi::get_challenge(&public_key).await?, - }; - let mut rng = rand::rngs::StdRng::from_os_rng(); - let signature = identity - .private_key() - .calculate_signature(&append_tbs.encode_to_vec(), &mut rng) - .map_err(|error| TwonlyError::Signal(error.to_string()))? - .into_vec(); - GroupApi::append(AppendGroupState { - signature, - append_tbs: Some(append_tbs), - version_id: group.state_version_id as u64 + 1, - }) - .await?; + + // The append is addressed to the version the server holds right now, + // not to the local mirror of it: an admin promoting somebody, or + // anyone else's leave, moves the group on without this member having + // refreshed, and the server rejects an append that does not follow its + // current version. Reading it here is also what decides which of the + // two ways out of a group applies. + let mut attempt = 1; + loop { + let (version, group_state) = GroupApi::load_state(&group).await?; + if group_state.admin_ids.contains(&user_id) { + return self.remove_member(group_id, user_id).await; + } + + let append = EncryptedAppendedGroupState { + r#type: encrypted_appended_group_state::Type::LeftGroup as i32, + }; + let append_tbs = append_group_state::AppendTbs { + encrypted_group_state_append: crypto::encrypt( + group.state_key()?, + &append.encode_to_vec(), + )?, + public_key: public_key.clone(), + group_id: group_id.clone(), + nonce: GroupApi::get_challenge(&public_key).await?, + }; + let mut rng = rand::rngs::StdRng::from_os_rng(); + let signature = identity + .private_key() + .calculate_signature(&append_tbs.encode_to_vec(), &mut rng) + .map_err(|error| TwonlyError::Signal(error.to_string()))? + .into_vec(); + match GroupApi::append(AppendGroupState { + signature, + append_tbs: Some(append_tbs), + version_id: version + 1, + }) + .await + { + Ok(()) => break, + Err(TwonlyError::GroupStateConflict) if attempt < STATE_UPDATE_ATTEMPTS => { + attempt += 1; + tracing::info!( + group_id, + attempt, + "group state moved on while leaving it, appending to the new version" + ); + } + Err(error) => return Err(error), + } + } + self.announce(&group_id, "leftGroup", None, None, None) .await?; let mut tr = db.pool.begin().await?;