chore: Now using timestamped tombstones to store deleted TOTPs.

Author: Skyost

drift_schemas/app/drift_schema_v2.json

@@ -169,6 +169,16 @@
             "default_dart": null,
             "default_client_dart": null,
             "dsl_features": []
+          },
+          {
+            "name": "deleted_at",
+            "getter_name": "deletedAt",
+            "moor_type": "dateTime",
+            "nullable": false,
+            "customConstraints": null,
+            "default_dart": "const CustomExpression('CAST(strftime(\\'%s\\', CURRENT_TIMESTAMP) AS INTEGER)')",
+            "default_client_dart": null,
+            "dsl_features": []
           }
         ],
         "is_virtual": false,
@@ -324,7 +334,7 @@
       "sql": [
         {
           "dialect": "sqlite",
-          "sql": "CREATE TABLE IF NOT EXISTS \"deleted_totps\" (\"uuid\" TEXT NOT NULL, PRIMARY KEY (\"uuid\"));"
+          "sql": "CREATE TABLE IF NOT EXISTS \"deleted_totps\" (\"uuid\" TEXT NOT NULL, \"deleted_at\" INTEGER NOT NULL DEFAULT (CAST(strftime('%s', CURRENT_TIMESTAMP) AS INTEGER)), PRIMARY KEY (\"uuid\"));"
         }
       ]
     },

lib/model/backend/request/request.dart

@@ -76,7 +76,7 @@ class InvalidJsonResponse extends LocalizableException {
 /// Represents a backend request with a body.
 mixin BackendWithBodyRequest<T extends BackendResponse> on BackendRequest<T> {
   /// The body.
-  Object? get body => null;
+  Object? get jsonBody => null;
 
   /// The encoding.
   Encoding? get encoding => null;
@@ -112,7 +112,7 @@ abstract class BackendPostRequest<T extends BackendResponse> extends BackendRequ
       if (headers != null) ...headers,
       'Content-Type': 'application/json',
     },
-    body: jsonEncode(body),
+    body: jsonEncode(jsonBody),
     encoding: encoding,
   );
 }
@@ -132,7 +132,7 @@ abstract class BackendDeleteRequest<T extends BackendResponse> extends BackendRe
       if (headers != null) ...headers,
       'Content-Type': 'application/json',
     },
-    body: jsonEncode(body),
+    body: jsonEncode(jsonBody),
     encoding: encoding,
   );
 }
@@ -190,7 +190,7 @@ class RefreshSessionRequest extends BackendPostRequest<RefreshSessionResponse> {
        );
 
   @override
-  Object? get body => {'refreshToken': refreshToken};
+  Object? get jsonBody => {'refreshToken': refreshToken};
 
   @override
   RefreshSessionResponse _toResponseIfNoError(dynamic data) => RefreshSessionResponse.fromJson(data);
@@ -210,7 +210,7 @@ class UserLogoutRequest extends BackendPostRequest<UserLogoutResponse> {
        );
 
   @override
-  Object? get body => {'refreshToken': refreshToken};
+  Object? get jsonBody => {'refreshToken': refreshToken};
 
   @override
   UserLogoutResponse _toResponseIfNoError(dynamic data) => UserLogoutResponse.fromJson(data);
@@ -234,7 +234,7 @@ class EmailConfirmRequest extends BackendPostRequest<EmailConfirmResponse> {
        );
 
   @override
-  Object? get body => {
+  Object? get jsonBody => {
     'email': email,
     'verificationCode': verificationCode,
   };
@@ -261,7 +261,7 @@ class EmailConfirmationCancelRequest extends BackendPostRequest<EmailConfirmatio
        );
 
   @override
-  Object? get body => {
+  Object? get jsonBody => {
     'email': email,
     'cancelCode': cancelCode,
   };
@@ -288,7 +288,7 @@ class ProviderLoginRequest extends BackendPostRequest<ProviderLoginResponse> {
        );
 
   @override
-  Object? get body => {
+  Object? get jsonBody => {
     'authorizationCode': authorizationCode,
     if (codeVerifier != null) 'codeVerifier': codeVerifier,
   };
@@ -316,7 +316,7 @@ class ProviderLinkRequest extends BackendPostRequest<ProviderLinkResponse> {
        );
 
   @override
-  Object? get body => {
+  Object? get jsonBody => {
     'authorizationCode': authorizationCode,
     if (codeVerifier != null) 'codeVerifier': codeVerifier,
   };
@@ -353,7 +353,7 @@ class SynchronizationPushRequest extends BackendPostRequest<SynchronizationPushR
        );
 
   @override
-  Object? get body => [
+  Object? get jsonBody => [
     for (PushOperation operation in operations) operation.toJson(httpRequest: true),
   ];
 
@@ -363,21 +363,29 @@ class SynchronizationPushRequest extends BackendPostRequest<SynchronizationPushR
 
 /// A request that allows to pull TOTPs.
 class SynchronizationPullRequest extends BackendPostRequest<SynchronizationPullResponse> {
-  /// The timestamps.
-  final Map<String, DateTime> timestamps;
+  /// The TOTPs that are still active.
+  final Map<String, DateTime> active;
+
+  /// The TOTPs that are still deleted.
+  final Map<String, DateTime> deleted;
 
   /// Creates a new synchronization pull request instance.
   const SynchronizationPullRequest({
-    this.timestamps = const {},
+    this.active = const {},
+    this.deleted = const {},
   }) : super(
          route: '/totps/sync/pull',
          needsAuthorization: true,
        );
 
   @override
-  Object? get body => {
-    for (String uuid in timestamps.keys) uuid: timestamps[uuid]!.millisecondsSinceEpoch,
-  };
+  Object? get jsonBody {
+    MapEntry<String, dynamic> convert(String key, DateTime value) => MapEntry(key, value.millisecondsSinceEpoch);
+    return {
+      'active': active.map(convert),
+      'deleted': deleted.map(convert),
+    };
+  }
 
   @override
   SynchronizationPullResponse _toResponseIfNoError(dynamic data) => SynchronizationPullResponse.fromJson(data);

lib/model/backend/request/response.dart

@@ -189,21 +189,21 @@ class SynchronizationPullResponse extends BackendResponse {
   final List<Totp> updates;
 
   /// The TOTPs to delete.
-  final List<String> deletes;
+  final Map<String, DateTime> deletes;
 
   /// Creates a new synchronization pull response instance.
   const SynchronizationPullResponse({
     this.inserts = const [],
     this.updates = const [],
-    this.deletes = const [],
+    this.deletes = const {},
   });
 
   /// Creates a new synchronization pull response instance from a JSON map.
   SynchronizationPullResponse.fromJson(Map<String, dynamic> json)
     : this(
         inserts: _totpListFromJson(json['inserts']),
         updates: _totpListFromJson(json['updates']),
-        deletes: (json['deletes'] as List).cast<String>(),
+        deletes: (json['deletes'] as Map).map((key, value) => MapEntry(key, DateTime.fromMillisecondsSinceEpoch(value))),
       );
 
   /// Creates a new synchronization pull response instance from a JSON map.

lib/model/backend/synchronization/push/operation.dart

@@ -1,4 +1,5 @@
 import 'package:equatable/equatable.dart';
+import 'package:open_authenticator/model/database/database.dart';
 import 'package:open_authenticator/model/totp/json.dart';
 import 'package:open_authenticator/model/totp/totp.dart';
 import 'package:uuid/uuid.dart';
@@ -89,7 +90,7 @@ class SetTotpsPushOperation extends PushOperation<Map<String, dynamic>> {
 }
 
 /// Represents a `delete` push operation.
-class DeleteTotpsPushOperation extends PushOperation<List<String>> {
+class DeleteTotpsPushOperation extends PushOperation<Map<String, int>> {
   /// Creates a new `delete` push operation instance from a raw payload.
   DeleteTotpsPushOperation.raw({
     super.uuid,
@@ -100,11 +101,13 @@ class DeleteTotpsPushOperation extends PushOperation<List<String>> {
   /// Creates a new `delete` push operation instance from a UUIDs list.
   DeleteTotpsPushOperation({
     String? uuid,
-    required List<String> uuids,
+    required DeletedTotpMap tombstones,
     DateTime? createdAt,
   }) : this.raw(
          uuid: uuid,
-         payload: uuids,
+         payload: {
+           for (MapEntry<String, DateTime> entry in tombstones.entries) entry.key: entry.value.millisecondsSinceEpoch,
+         },
          createdAt: createdAt,
        );
 
@@ -113,11 +116,50 @@ class DeleteTotpsPushOperation extends PushOperation<List<String>> {
 
   @override
   DeleteTotpsPushOperation copyWith({
-    List<String>? payload,
+    Map<String, int>? payload,
     DateTime? createdAt,
   }) => DeleteTotpsPushOperation.raw(
     uuid: uuid,
     payload: payload ?? this.payload,
     createdAt: createdAt ?? this.createdAt,
   );
 }
+
+/// Allows to compact a list of push operations.
+extension Compact on List<PushOperation> {
+  /// Compacts the current push operations while preserving their relative order.
+  /// Only the latest operation for each TOTP UUID is kept.
+  List<PushOperation> get compacted {
+    if (isEmpty) {
+      return [];
+    }
+
+    Set<String> processedTotpUuids = {};
+    List<PushOperation> result = [];
+
+    for (PushOperation operation in reversed) {
+      switch (operation) {
+        case SetTotpsPushOperation(:final payload):
+          Map<String, dynamic> newPayload = {
+            for (MapEntry<String, dynamic> entry in payload.entries)
+              if (processedTotpUuids.add(entry.key)) entry.key: entry.value,
+          };
+          if (newPayload.isNotEmpty) {
+            result.add(operation.copyWith(payload: newPayload));
+          }
+          break;
+        case DeleteTotpsPushOperation(:final payload):
+          Map<String, int> newPayload = {
+            for (MapEntry<String, int> entry in payload.entries)
+              if (processedTotpUuids.add(entry.key)) entry.key: entry.value,
+          };
+          if (newPayload.isNotEmpty) {
+            result.add(operation.copyWith(payload: newPayload));
+          }
+          break;
+      }
+    }
+
+    return result.reversed.toList();
+  }
+}

lib/model/backend/synchronization/push/result.dart

@@ -105,6 +105,12 @@ enum PushOperationErrorKind {
   /// The update timestamp is invalid.
   invalidUpdateTimestamp(isPermanent: true),
 
+  /// The TOTP has been deleted more recently.
+  deletedTotp(isPermanent: true),
+
+  /// The delete timestamp is invalid.
+  invalidDeleteTimestamp(isPermanent: true),
+
   /// The max TOTPs count has been exceeded.
   maxCountExceeded,
 

lib/model/backend/synchronization/queue.dart

@@ -53,7 +53,7 @@ class PushOperationsQueue extends AsyncNotifier<List<PushOperation>> {
       }
     }
     List<PushOperation> operations = await future;
-    List<PushOperation> compactedOperations = _compact(operations);
+    List<PushOperation> compactedOperations = operations.compacted;
     AppDatabase database = ref.read(appDatabaseProvider);
     if (compactedOperations.length != operations.length) {
       await database.replacePendingBackendPushOperations(compactedOperations);
@@ -78,38 +78,6 @@ class PushOperationsQueue extends AsyncNotifier<List<PushOperation>> {
     return const ResultSuccess();
   }
 
-  /// Compacts the push operations list.
-  List<PushOperation> _compact(List<PushOperation> operations) {
-    if (operations.isEmpty) {
-      return [];
-    }
-
-    Set<String> processedTotpUuids = {};
-    List<PushOperation> result = [];
-
-    for (PushOperation operation in operations.reversed) {
-      switch (operation) {
-        case SetTotpsPushOperation(:final payload):
-          Map<String, dynamic> newPayload = {
-            for (MapEntry<String, dynamic> entry in payload.entries)
-              if (processedTotpUuids.add(entry.key)) entry.key: entry.value,
-          };
-          if (newPayload.isNotEmpty) {
-            result.add(operation.copyWith(payload: newPayload));
-          }
-          break;
-        case DeleteTotpsPushOperation(:final payload):
-          List<String> newPayload = payload.where((uuid) => processedTotpUuids.add(uuid)).toList();
-          if (newPayload.isNotEmpty) {
-            result.add(operation.copyWith(payload: newPayload));
-          }
-          break;
-      }
-    }
-
-    return result.reversed.toList();
-  }
-
   /// Triggered when the database updates.
   void _onDatabaseUpdate(List<PushOperation> operations) {
     if (ref.mounted) {
@@ -297,33 +265,46 @@ class SynchronizationController extends Notifier<SynchronizationStatus> {
         return const ResultCancelled();
       }
     }
+
     List<Totp> totps = await ref.read(totpRepositoryProvider.future);
+    DeletedTotpMap deletedTotps = await ref.read(appDatabaseProvider).getDeletedTotps();
     Result<SynchronizationPullResponse> result = await ref
         .read(backendClientProvider.notifier)
         .sendHttpRequest(
           SynchronizationPullRequest(
-            timestamps: {
+            active: {
               for (Totp totp in totps) totp.uuid: totp.updatedAt,
             },
+            deleted: deletedTotps,
           ),
         );
+
     if (result is! ResultSuccess<SynchronizationPullResponse>) {
       return result;
     }
 
     TotpRepository repository = ref.read(totpRepositoryProvider.notifier);
-    await repository.addTotps(
+    Result addResult = await repository.addTotps(
       result.value.inserts,
       fromNetwork: true,
     );
-    await repository.updateTotps(
+    if (addResult is! ResultSuccess) {
+      return addResult;
+    }
+    Result updateResult = await repository.updateTotps(
       result.value.updates,
       fromNetwork: true,
     );
-    await repository.deleteTotps(
+    if (updateResult is! ResultSuccess) {
+      return updateResult;
+    }
+    Result deleteResult = await repository.deleteTotps(
       result.value.deletes,
       fromNetwork: true,
     );
+    if (deleteResult is! ResultSuccess) {
+      return deleteResult;
+    }
     return const ResultSuccess();
   }
 }