Skip to content

[codex] Reject stale auth watcher metadata #3485

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
CharlVS wants to merge 1 commit into
base: dev
Choose a base branch
from
+225 −11
Draft

[codex] Reject stale auth watcher metadata #3485

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 73 additions & 11 deletions lib/bloc/auth_bloc/auth_bloc.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import 'dart:async';

import 'package:collection/collection.dart';
import 'package:equatable/equatable.dart';
import 'package:flutter_bloc/flutter_bloc.dart';
import 'package:komodo_defi_rpc_methods/komodo_defi_rpc_methods.dart'
Expand Down Expand Up @@ -34,6 +35,20 @@ class AuthBloc extends Bloc<AuthBlocEvent, AuthBlocState> with TrezorAuthMixin {
'This wallet appears to have already been migrated. '
'Use the migrated wallet entry and its current password.';
static const Duration _postLoginStepTimeout = Duration(seconds: 5);
static const DeepCollectionEquality _metadataEquality =
DeepCollectionEquality();
static const Set<String> _optimisticAuthMetadataKeys = <String>{
'type',
'wallet_provenance',
'wallet_created_at',
'has_backup',
'activated_coins',
legacySourceKindMetadataKey,
legacySourceWalletIdMetadataKey,
legacySourceWalletNameMetadataKey,
legacyCleanupStatusMetadataKey,
legacyWalletExtrasMetadataKey,
};

/// Handles [AuthBlocEvent]s and emits [AuthBlocState]s.
/// [_kdfSdk] is an instance of [KomodoDefiSdk] used for authentication.
Expand Down Expand Up @@ -222,10 +237,9 @@ class AuthBloc extends Bloc<AuthBlocEvent, AuthBlocState> with TrezorAuthMixin {
}

if (event.currentUser != null) {
// After optimistic login, the SDK watcher fires with the bare user
// before the background finalizer persists metadata. Suppress only if
// the incoming metadata carries no new or changed values; allow updates
// from finalizers (e.g. cleanup status, activated coins) through.
// After optimistic login, the SDK watcher can fire with a less-initialized
// user before the background finalizer persists metadata. Suppress those
// stale snapshots while still allowing complete finalizer updates through.
if (state.status == AuthenticationStatus.completed &&
state.currentUser?.walletId == event.currentUser!.walletId &&
!_hasNewerMetadata(
Expand Down Expand Up @@ -517,8 +531,7 @@ class AuthBloc extends Bloc<AuthBlocEvent, AuthBlocState> with TrezorAuthMixin {
allowWeakPassword: weakPasswordsAllowed,
),
);
final LegacyWalletSource? linkageSource =
event.sourceWallet.legacySource;
final LegacyWalletSource? linkageSource = event.sourceWallet.legacySource;
if (linkageSource != null) {
await _kdfSdk.setMigratedLegacySource(
source: linkageSource,
Expand Down Expand Up @@ -987,22 +1000,71 @@ class AuthBloc extends Bloc<AuthBlocEvent, AuthBlocState> with TrezorAuthMixin {
}
}

/// Returns `true` if [incoming] contains at least one key whose value
/// differs from [current], or a key that [current] does not have at all.
/// Used to distinguish bare-user watcher re-emissions (no new data) from
/// post-login finalizer updates that carry meaningful metadata changes.
/// Returns `true` when [incoming] carries metadata that should replace
/// [current] for same-wallet watcher updates.
bool _hasNewerMetadata(
Map<String, dynamic> incoming,
Map<String, dynamic> current,
) {
if (_dropsOptimisticMetadata(incoming, current)) {
return false;
}
if (_regressesLegacyCleanupStatus(incoming, current)) {
return false;
}

for (final entry in incoming.entries) {
if (!current.containsKey(entry.key) || current[entry.key] != entry.value) {
if (!current.containsKey(entry.key) ||
!_metadataEquality.equals(current[entry.key], entry.value)) {
return true;
}
}
return false;
}

bool _dropsOptimisticMetadata(
Map<String, dynamic> incoming,
Map<String, dynamic> current,
) {
for (final key in _optimisticAuthMetadataKeys) {
if (_hasOptimisticMetadataValue(current, key) &&
!_preservesOptimisticMetadataValue(incoming, key)) {
return true;
}
}
return false;
}

bool _hasOptimisticMetadataValue(Map<String, dynamic> metadata, String key) {
if (!_preservesOptimisticMetadataValue(metadata, key)) return false;

final value = metadata[key];
if (value is Iterable || value is Map) {
return value.isNotEmpty;
}
return true;
}

bool _preservesOptimisticMetadataValue(
Map<String, dynamic> metadata,
String key,
) {
if (!metadata.containsKey(key)) return false;

final value = metadata[key];
return !_isMissingMetadataStringValue(value);
}

bool _regressesLegacyCleanupStatus(
Map<String, dynamic> incoming,
Map<String, dynamic> current,
) {
final currentStatus = current[legacyCleanupStatusMetadataKey];
final incomingStatus = incoming[legacyCleanupStatusMetadataKey];
return currentStatus == LegacyMigrationCleanupStatus.complete.name &&
incomingStatus == LegacyMigrationCleanupStatus.incomplete.name;
}

bool _isMissingMetadataStringValue(dynamic value) {
return value == null || value is String && value.trim().isEmpty;
}
Expand Down
152 changes: 152 additions & 0 deletions test_units/tests/wallet/legacy_native_wallet_migration_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -601,6 +601,158 @@ void main() {
});

group('AuthBloc legacy migration', () {
test(
'auth watcher ignores less-initialized metadata for the active wallet',
() async {
final auth = _FakeAuth(users: const <KdfUser>[]);
final bloc = AuthBloc(
_FakeSdk(auth: auth),
WalletsRepository(
_FakeSdk(auth: auth),
_FakeMm2Api(),
_FakeStorage(),
),
SettingsRepository(storage: _FakeStorage()),
_FakeTradingStatusService(),
);
addTearDown(bloc.close);

final optimisticUser = _buildUser(
walletName: 'Migrated_Wallet',
derivationMethod: DerivationMethod.iguana,
metadata: <String, dynamic>{
'type': WalletType.iguana.name,
'wallet_provenance': WalletProvenance.imported.name,
'wallet_created_at': 1,
'has_backup': true,
'activated_coins': <String>['BTC'],
legacySourceKindMetadataKey: LegacyWalletSourceKind.nativeApp.name,
legacySourceWalletIdMetadataKey: 'native-1',
legacySourceWalletNameMetadataKey: 'Legacy Wallet!',
legacyCleanupStatusMetadataKey:
LegacyMigrationCleanupStatus.incomplete.name,
},
);
final staleUser = _buildUser(
walletName: 'Migrated_Wallet',
derivationMethod: DerivationMethod.iguana,
metadata: const <String, dynamic>{'activated_coins': <String>[]},
);

final optimisticStateFuture = bloc.stream.firstWhere(
(state) => state.currentUser?.metadata['wallet_created_at'] == 1,
);
bloc.add(
AuthModeChanged(
mode: AuthorizeMode.logIn,
currentUser: optimisticUser,
),
);
await optimisticStateFuture;

bloc.add(
AuthModeChanged(mode: AuthorizeMode.logIn, currentUser: staleUser),
);
await pumpEventQueue(times: 10);

expect(bloc.state.currentUser?.metadata['activated_coins'], <String>[
'BTC',
]);
expect(
bloc.state.currentUser?.metadata['type'],
WalletType.iguana.name,
);
},
);

test(
'auth watcher accepts complete finalizer metadata for the active wallet',
() async {
final auth = _FakeAuth(users: const <KdfUser>[]);
final bloc = AuthBloc(
_FakeSdk(auth: auth),
WalletsRepository(
_FakeSdk(auth: auth),
_FakeMm2Api(),
_FakeStorage(),
),
SettingsRepository(storage: _FakeStorage()),
_FakeTradingStatusService(),
);
addTearDown(bloc.close);

final optimisticUser = _buildUser(
walletName: 'Migrated_Wallet',
derivationMethod: DerivationMethod.iguana,
metadata: <String, dynamic>{
'type': WalletType.iguana.name,
'wallet_provenance': WalletProvenance.imported.name,
'wallet_created_at': 1,
'has_backup': true,
'activated_coins': <String>['BTC'],
legacySourceKindMetadataKey: LegacyWalletSourceKind.nativeApp.name,
legacySourceWalletIdMetadataKey: 'native-1',
legacySourceWalletNameMetadataKey: 'Legacy Wallet!',
legacyCleanupStatusMetadataKey:
LegacyMigrationCleanupStatus.incomplete.name,
},
);
final finalizedUser = _buildUser(
walletName: 'Migrated_Wallet',
derivationMethod: DerivationMethod.iguana,
metadata: <String, dynamic>{
'type': WalletType.iguana.name,
'wallet_provenance': WalletProvenance.imported.name,
'wallet_created_at': 1,
'has_backup': true,
'activated_coins': <String>['BTC', 'KMD'],
legacySourceKindMetadataKey: LegacyWalletSourceKind.nativeApp.name,
legacySourceWalletIdMetadataKey: 'native-1',
legacySourceWalletNameMetadataKey: 'Legacy Wallet!',
legacyCleanupStatusMetadataKey:
LegacyMigrationCleanupStatus.complete.name,
},
);

final optimisticStateFuture = bloc.stream.firstWhere(
(state) => state.currentUser?.metadata['wallet_created_at'] == 1,
);
bloc.add(
AuthModeChanged(
mode: AuthorizeMode.logIn,
currentUser: optimisticUser,
),
);
await optimisticStateFuture;

final finalizedStateFuture = bloc.stream.firstWhere(
(state) =>
state.currentUser?.metadata[legacyCleanupStatusMetadataKey] ==
LegacyMigrationCleanupStatus.complete.name,
);
bloc.add(
AuthModeChanged(
mode: AuthorizeMode.logIn,
currentUser: finalizedUser,
),
);
final finalizedState = await finalizedStateFuture;

expect(
finalizedState.currentUser?.metadata['activated_coins'],
<String>['BTC', 'KMD'],
);
expect(
finalizedState
.currentUser
?.wallet
.migratedLegacySource
?.cleanupStatus,
LegacyMigrationCleanupStatus.complete,
);
},
);

test(
'legacy migration registers with the KDF password and stores migrated metadata',
() async {
Expand Down
Loading