fix passwordless recovery

dependencies

@@ -1 +1 @@
-Subproject commit 60da6275f8c82c4238f0c3cf88a798e63b4a2ffd
+Subproject commit 4d22ef849031619d6bb4dcf8ea1709d3c1ff8b3f

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:

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';

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';

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",

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",

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<void> 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) {

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);
   }
 

lib/src/services/notifications/fcm.notifications.dart

@@ -30,6 +30,10 @@ class FcmNotificationService {
   }
 
   static Future<void> 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) {

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;
     }
 

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.

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<bool> 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<bool> deleteLocalUserData({bool removeCredentials = false}) async {
   locator
     ..unregister<TwonlyDB>()
     ..registerLazySingleton<TwonlyDB>(TwonlyDB.new);
+  Log.info('Deleted the local user data.');
   return true;
 }

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<void> _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<BetterText> {
     _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<void> _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 = <TextSpan>[];

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<RecoveryProgressView> {
       _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<RecoveryProgressView> {
 
     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) {

lib/src/visual/views/settings/backup/passwordless_recovery/help_a_friend.passwordless_recovery.view.dart

@@ -78,6 +78,19 @@ class _HelpAFriendPasswordlessRecoveryViewState
   }
 
   Future<void> _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,

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,
         })
     }

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(&current_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(&current_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);

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<Option<AdminKeys>>,
-    ) -> Result<()> {
+    ) -> Result<bool> {
         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<bool> {
         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?;