diff --git a/lib/screens/pin/bloc/setup_pin/setup_pin_cubit.dart b/lib/screens/pin/bloc/setup_pin/setup_pin_cubit.dart index 11e77ac5..59a599ed 100644 --- a/lib/screens/pin/bloc/setup_pin/setup_pin_cubit.dart +++ b/lib/screens/pin/bloc/setup_pin/setup_pin_cubit.dart @@ -19,6 +19,8 @@ class SetupPinCubit extends Cubit { String? _createPin; + Future? _inflightConfirm; + void addDigit(int digit) { if (state.currentPin.length >= pinLength) return; @@ -57,7 +59,13 @@ class SetupPinCubit extends Cubit { } } - Future _confirmPin(String confirmPin) async { + // A confirm re-typed while the slow PBKDF2 hash is still in flight must not + // start a second salt+hash round-trip: coalesce onto the first so the + // persisted salt and hash always come from one run (never a torn pair). + Future _confirmPin(String confirmPin) => + _inflightConfirm ??= _runConfirmPin(confirmPin).whenComplete(() => _inflightConfirm = null); + + Future _runConfirmPin(String confirmPin) async { if (confirmPin == _createPin) { final salt = SecureStorage.generatePinSalt(); final hash = await SecureStorage.hashPinAsync(confirmPin, salt); diff --git a/lib/screens/pin/bloc/verify_pin/verify_pin_cubit.dart b/lib/screens/pin/bloc/verify_pin/verify_pin_cubit.dart index 8a43a36b..0973fccc 100644 --- a/lib/screens/pin/bloc/verify_pin/verify_pin_cubit.dart +++ b/lib/screens/pin/bloc/verify_pin/verify_pin_cubit.dart @@ -19,6 +19,8 @@ class VerifyPinCubit extends Cubit { final BiometricService _biometricService; final bool enableLockout; + Future? _inflightCheck; + void addDigit(int digit) { if (state is VerifyPinTemporarilyLocked || state is VerifyPinLocked) return; if (state.pin.length == pinLength) return; @@ -32,7 +34,13 @@ class VerifyPinCubit extends Cubit { emit(state.copyWith(pin: state.pin.substring(0, state.pin.length - 1))); } - Future checkPin() async { + // Concurrent checkPin() calls always verify the same submission, so coalesce + // them onto one in-flight round-trip: the read-modify-write lockout counter + // advances exactly once instead of under-counting on an interleaved race. + Future checkPin() => + _inflightCheck ??= _runCheckPin().whenComplete(() => _inflightCheck = null); + + Future _runCheckPin() async { final isCorrect = await _secureStorage.verifyPin(state.pin); if (isCorrect) { if (enableLockout) await _secureStorage.resetPinLockout(); diff --git a/test/screens/pin/setup_pin_cubit_test.dart b/test/screens/pin/setup_pin_cubit_test.dart index cc0256c6..67aa6179 100644 --- a/test/screens/pin/setup_pin_cubit_test.dart +++ b/test/screens/pin/setup_pin_cubit_test.dart @@ -158,5 +158,46 @@ void main() { expect(result, isTrue); verify(() => biometricService.enable()).called(1); }); + + group('concurrency (F-09: salt/hash setup race)', () { + test('confirm re-typed during the in-flight hash writes one atomic salt+hash', () async { + Uint8List? savedSalt; + String? savedHash; + when(() => secureStorage.setPinSalt(any())).thenAnswer((inv) async { + savedSalt = inv.positionalArguments.first as Uint8List; + }); + when(() => secureStorage.setPinHash(any())).thenAnswer((inv) async { + savedHash = inv.positionalArguments.first as String; + }); + final cubit = build(); + final completed = cubit.stream.firstWhere((s) => s.isComplete); + + // Create the pin -> confirm mode (with _createPin set). + for (final d in [1, 2, 3, 4, 5, 6]) { + cubit.addDigit(d); + } + // Confirm the pin -> fires _confirmPin #1 (in-flight on the slow PBKDF2 + // compute()). currentPin stays '123456', so the field still looks full. + for (final d in [1, 2, 3, 4, 5, 6]) { + cubit.addDigit(d); + } + // An impatient user backspaces and re-types the last digit while the + // hash is still computing -> fires _confirmPin #2 concurrently. + cubit.deleteDigit(); + cubit.addDigit(6); + + await completed.timeout(const Duration(seconds: 30)); + await pumpEventQueue(); + + // Single-effect + atomic: exactly one salt and one hash persisted, and + // the persisted hash must be PBKDF2(pin, persisted salt) — i.e. salt and + // hash provably come from the SAME run. At HEAD two interleaved runs + // write twice and can tear the pair (saltB + hashA) -> RED. + verify(() => secureStorage.setPinSalt(any())).called(1); + verify(() => secureStorage.setPinHash(any())).called(1); + expect(savedSalt, isNotNull); + expect(savedHash, SecureStorage.hashPin('123456', savedSalt!)); + }); + }); }); } diff --git a/test/screens/pin/verify_pin_cubit_test.dart b/test/screens/pin/verify_pin_cubit_test.dart index 3bf86183..015509bc 100644 --- a/test/screens/pin/verify_pin_cubit_test.dart +++ b/test/screens/pin/verify_pin_cubit_test.dart @@ -262,5 +262,30 @@ void main() { verifyNever(() => biometricService.authenticate()); }); }); + + group('concurrency (F-01: PIN-lockout under-count race)', () { + test('two parallel checkPin of one submission advance the counter once', () async { + when(() => secureStorage.verifyPin(any())).thenAnswer((_) async => false); + when(() => secureStorage.getPinFailedAttempts()).thenAnswer((_) async => 0); + final cubit = build(); + + // The PIN is fixed for the lifetime of a submission, so concurrent + // checkPin() calls always verify the SAME pin — one logical attempt + // double-fired. The 6th digit auto-fires checkPin() #1; a second + // checkPin() races it against the read-modify-write failure counter. + addPin(cubit, '99999'); + cubit.addDigit(9); // 6th digit -> auto checkPin() #1 (in-flight) + final second = cubit.checkPin(); // checkPin() #2, same state.pin + + await second.timeout(const Duration(seconds: 30)); + await pumpEventQueue(); + + // Single-effect: the counter must advance to exactly 1, written once. + // At HEAD both reads see 0 and both write 1 -> called twice (RED). + verify(() => secureStorage.setPinFailedAttempts(1)).called(1); + verifyNever(() => secureStorage.setPinFailedAttempts(2)); + expect(cubit.state.failedAttempts, 1); + }); + }); }); }