Setting a PIN can lock you out of the session you set it in #62

New Issue

Closed

opened 2026-08-20 21:22:37 -05:00 by null · 0 comments

null commented

2026-08-20 21:22:37 -05:00

Owner

What is true now. app/src/main/kotlin/dev/privacyllc/period/feature/lock/LockSettingsViewModel.kt:74-90 writes the PIN and then unlocks the session: lock.setPin(pin) at :78, controller.unlock() at :85. Those are two independent dispatches. hasPin is a DataStore flow (core/security/.../AppLockRepository.kt:81); if its emission reaches AppLockViewModel (app/.../lock/AppLockViewModel.kt:85-94) before line 85 runs, the state flips to Locked, AppLockGate (AppLockGate.kt:67-86) invokes content() only in the Unlocked branch and therefore disposes the whole PeriodApp subtree — including rememberNavController() and every nav-scoped ViewModel. LockSettingsViewModel is cleared, its viewModelScope is cancelled, and controller.unlock() never runs.

The user is thrown to the lock screen and must type the PIN she created seconds ago; unlocking lands her on Today, not back in Settings.

What it costs. docs/history/DEVELOPMENT_LOG.md:352-355 records this defect as found and fixed ('Setting a PIN locked you out of the session you set it in'). The fix closed the common ordering but not the race, because the fix itself lives in the scope the race destroys. It was found by driving the emulator, not by a test, and there is still no test.

What to do. Unlock BEFORE writing. AppLockController.unlock() is MutableStateFlow.value = true — synchronous, on the caller's thread, complete before setPin is entered; hasPin flips inside store.edit, strictly later, and reaches the gate's combine by dispatch. Since combine emits with the latest value of each input, the pair (hasPin = true, unlocked = false) becomes unobservable. If setPin fails there is no PIN, and Unlocked is the correct state anyway, so nothing needs undoing.

Traps. Never call controller.lock() on failure — during a PIN change that throws the user to the lock screen behind their old PIN. Any later 'tidy-up' that moves the unlock back after the write re-introduces this; the test and the KDoc are the guard. Note PBKDF2 at 210k iterations runs synchronously inside setPin on the calling thread — that is pre-existing jank on this path, not this race; do not conflate them.

Verify: setting the first PIN never closes the gate in LockSettingsViewModelTest — collect combine(hasPin, unlocked, ::lockStateOf) throughout and assert LockState.Locked never appears; swapping the two lines back to the old order must redden exactly that test.

**What is true now.** `app/src/main/kotlin/dev/privacyllc/period/feature/lock/LockSettingsViewModel.kt:74-90` writes the PIN and then unlocks the session: `lock.setPin(pin)` at `:78`, `controller.unlock()` at `:85`. Those are two independent dispatches. `hasPin` is a DataStore flow (`core/security/.../AppLockRepository.kt:81`); if its emission reaches `AppLockViewModel` (`app/.../lock/AppLockViewModel.kt:85-94`) before line 85 runs, the state flips to Locked, `AppLockGate` (`AppLockGate.kt:67-86`) invokes `content()` only in the Unlocked branch and therefore disposes the whole `PeriodApp` subtree — including `rememberNavController()` and every nav-scoped ViewModel. `LockSettingsViewModel` is cleared, its `viewModelScope` is cancelled, and `controller.unlock()` never runs. The user is thrown to the lock screen and must type the PIN she created seconds ago; unlocking lands her on Today, not back in Settings. **What it costs.** `docs/history/DEVELOPMENT_LOG.md:352-355` records this defect as found and fixed ('Setting a PIN locked you out of the session you set it in'). The fix closed the common ordering but not the race, because the fix itself lives in the scope the race destroys. It was found by driving the emulator, not by a test, and there is still no test. **What to do.** Unlock BEFORE writing. `AppLockController.unlock()` is `MutableStateFlow.value = true` — synchronous, on the caller's thread, complete before `setPin` is entered; `hasPin` flips inside `store.edit`, strictly later, and reaches the gate's `combine` by dispatch. Since `combine` emits with the latest value of each input, the pair (hasPin = true, unlocked = false) becomes unobservable. If `setPin` fails there is no PIN, and Unlocked is the correct state anyway, so nothing needs undoing. **Traps.** Never call `controller.lock()` on failure — during a PIN change that throws the user to the lock screen behind their old PIN. Any later 'tidy-up' that moves the unlock back after the write re-introduces this; the test and the KDoc are the guard. Note PBKDF2 at 210k iterations runs synchronously inside `setPin` on the calling thread — that is pre-existing jank on this path, not this race; do not conflate them. Verify: `setting the first PIN never closes the gate` in `LockSettingsViewModelTest` — collect `combine(hasPin, unlocked, ::lockStateOf)` throughout and assert `LockState.Locked` never appears; swapping the two lines back to the old order must redden exactly that test.