From 750fdaeb0d768f9e5f8193263a91f80f128833ec Mon Sep 17 00:00:00 2001 From: null Date: Fri, 21 Aug 2026 01:47:01 -0500 Subject: [PATCH] feat: let the app be opened by a PIN, a fingerprint, or either The screens and the rules behind the choice. The storage and the migration landed in the previous commit. The state machine lives in the ViewModel and only a verified outcome advances it. That is not tidiness: the old screen advanced its own step in the same breath as asking, without waiting for the answer, and any four digits reached "choose a PIN". Keeping the stage where the verification happens removes the class of bug rather than one instance of it. Three rules, each with a test: Any change authenticates with the method that is on NOW -- otherwise whoever is holding the unlocked phone simply switches the lock off, which is the adversary SECURITY.md names first. Turning it on from nothing needs no proof, because there is nothing yet to prove. A fingerprint is never turned on without one working on this phone. Consent, then a successful scan, and only then is anything written. A cancelled prompt after a PIN has been committed leaves a working PIN lock rather than nothing, and says so. Moving to fingerprint-only drops the PIN only once a scan has worked, and then drops it properly -- the verifier and the Keystore key both go, so a stale secret is not left in the file. The lock screen in fingerprint-only mode is one line, one button and a notice, with no PIN field: a disabled text box on a screen that will never accept a PIN is an invitation to try. An unavailable sensor there must NOT switch the lock off -- that would let anyone who knows the phone's own PIN remove the enrolled fingerprints and walk in -- so the notice names the way back instead: enrol again, or erase. All of it says what it means. The PIN option states that it belongs to this app and not the phone, because somebody who assumes otherwise will assume they can reset it the way they reset a phone PIN. The fingerprint option says who it lets in: anyone enrolled on this phone, now or later. LockSettingsCopyTest holds both of those, and holds the whole surface to the same no-sensitive-words, no-"secure" rules the lock screen has. Proved: a failed scan committing nothing reddens exactly one test. Removing the authenticate-first rule or the scan-before-commit rule reddens several, which is that rule's whole surface rather than a coincidence. Part of #63 Co-Authored-By: Claude Opus 5 (1M context) --- .../period/feature/lock/LockCopy.kt | 40 ++++ .../period/feature/lock/LockScreen.kt | 49 ++++- .../period/feature/lock/LockSettingsCopy.kt | 107 +++++++++ .../period/feature/lock/LockSettingsScreen.kt | 22 +- .../feature/lock/LockSettingsViewModel.kt | 204 ++++++++++++++++-- .../dev/privacyllc/period/lock/AppLockGate.kt | 4 +- .../period/lock/AppLockViewModel.kt | 41 +++- .../privacyllc/period/lock/BiometricUnlock.kt | 44 +++- .../feature/lock/LockSettingsCopyTest.kt | 62 ++++++ .../feature/lock/LockSettingsViewModelTest.kt | 105 ++++++++- 10 files changed, 636 insertions(+), 42 deletions(-) create mode 100644 app/src/main/kotlin/dev/privacyllc/period/feature/lock/LockSettingsCopy.kt create mode 100644 app/src/test/kotlin/dev/privacyllc/period/feature/lock/LockSettingsCopyTest.kt diff --git a/app/src/main/kotlin/dev/privacyllc/period/feature/lock/LockCopy.kt b/app/src/main/kotlin/dev/privacyllc/period/feature/lock/LockCopy.kt index a9102df..16d4117 100644 --- a/app/src/main/kotlin/dev/privacyllc/period/feature/lock/LockCopy.kt +++ b/app/src/main/kotlin/dev/privacyllc/period/feature/lock/LockCopy.kt @@ -25,6 +25,43 @@ internal object LockCopy { const val FORGOT = "Forgot your PIN?" const val WRONG = "That is not the PIN." + // ---- Fingerprint or face, with no PIN behind it ----------------------- + + const val TITLE_BIOMETRIC = "Unlock" + const val BIOMETRIC_HINT = "Use your fingerprint or face to open the app." + const val USE_BIOMETRIC_ONLY = "Use fingerprint or face" + const val TRY_AGAIN = "Try again" + + /** The link on a screen with no PIN to have forgotten. */ + const val CANNOT_USE_BIOMETRIC = "Can't use fingerprint or face?" + + /** + * The notice when the phone has nothing enrolled any more. + * + * It names the way back that costs nothing first. This is the state somebody + * reaches by removing their fingerprints — sometimes their own doing, + * sometimes not — and the app must not answer it by unlocking itself, which + * would make the lock removable by anyone who knows the phone's own PIN. + */ + const val NOT_SET_UP = + "This phone has no fingerprint or face set up. Add one in your phone's settings, " + + "then come back — the app opens with it." + + const val NO_HARDWARE = "This phone cannot read a fingerprint or face right now." + + const val LOCKED_OUT_FOR_NOW = "Too many tries. Wait a moment, then try again." + + /** + * The phone's own biometric lockout, which this app cannot clear and does + * not share. Says what actually ends it rather than "try later". + */ + const val LOCKED_OUT_UNTIL_PHONE_UNLOCK = + "Your phone has paused fingerprint and face unlock after too many tries. Lock your " + + "phone, unlock it with its own PIN, pattern or password, then come back." + + const val UNAVAILABLE_NOW = + "Fingerprint or face is not available right now. Try again in a moment." + /** * Deliberately says nothing about what is stored — only that this device can * no longer check the PIN, and what the way out is. @@ -39,5 +76,8 @@ internal object LockCopy { val all: List = listOf( TITLE, FIELD_LABEL, SHOW, HIDE, UNLOCK, USE_BIOMETRIC, FORGOT, WRONG, KEY_UNAVAILABLE, tryAgainIn("1:00"), + TITLE_BIOMETRIC, BIOMETRIC_HINT, USE_BIOMETRIC_ONLY, TRY_AGAIN, + CANNOT_USE_BIOMETRIC, NOT_SET_UP, NO_HARDWARE, LOCKED_OUT_FOR_NOW, + LOCKED_OUT_UNTIL_PHONE_UNLOCK, UNAVAILABLE_NOW, ) } diff --git a/app/src/main/kotlin/dev/privacyllc/period/feature/lock/LockScreen.kt b/app/src/main/kotlin/dev/privacyllc/period/feature/lock/LockScreen.kt index 31d5296..d674117 100644 --- a/app/src/main/kotlin/dev/privacyllc/period/feature/lock/LockScreen.kt +++ b/app/src/main/kotlin/dev/privacyllc/period/feature/lock/LockScreen.kt @@ -42,6 +42,7 @@ import androidx.activity.compose.LocalActivity import androidx.fragment.app.FragmentActivity import androidx.lifecycle.compose.collectAsStateWithLifecycle import dev.privacyllc.period.designsystem.PeriodTheme +import dev.privacyllc.period.core.security.LockMethod import dev.privacyllc.period.lock.AppLockViewModel import dev.privacyllc.period.lock.rememberBiometricUnlock import dev.privacyllc.period.lock.LockScreenState @@ -76,7 +77,7 @@ import kotlinx.coroutines.delay * read the unencrypted database instead. */ @Composable -fun LockScreen(viewModel: AppLockViewModel) { +fun LockScreen(viewModel: AppLockViewModel, method: LockMethod) { val state by viewModel.screen.collectAsStateWithLifecycle() val biometricAllowed by viewModel.biometricAllowed.collectAsStateWithLifecycle() var erasing by remember { mutableStateOf(false) } @@ -99,11 +100,17 @@ fun LockScreen(viewModel: AppLockViewModel) { onBeginAuth = viewModel::beginBiometricAuth, onEndAuth = viewModel::endBiometricAuth, onUnlocked = viewModel::unlockFromBiometric, - onUnavailable = viewModel::disableBiometric, + // In fingerprint-only mode an unavailable sensor must NOT switch the + // lock off: that would let anyone who knows the phone's own PIN remove + // the enrolled fingerprints and walk in. She stays locked out, and the + // screen tells her how to get back — re-enrol, or erase. + onUnavailable = if (method.requiresPin) viewModel::disableBiometric else ({ }), + onNotice = viewModel::showBiometricNotice, ) LockScreenContent( state = state, + method = method, biometricAllowed = biometricAllowed && biometric.available, onSubmit = viewModel::submit, onTick = viewModel::refreshLockout, @@ -115,6 +122,7 @@ fun LockScreen(viewModel: AppLockViewModel) { @Composable internal fun LockScreenContent( state: LockScreenState, + method: LockMethod = LockMethod.PIN, biometricAllowed: Boolean, onSubmit: (CharArray) -> Unit, onTick: () -> Unit, @@ -147,13 +155,48 @@ internal fun LockScreenContent( horizontalAlignment = Alignment.CenterHorizontally, ) { Text( - text = LockCopy.TITLE, + text = if (method.requiresPin) LockCopy.TITLE else LockCopy.TITLE_BIOMETRIC, style = MaterialTheme.typography.headlineMedium, textAlign = TextAlign.Center, ) Spacer(Modifier.height(24.dp)) + // Fingerprint or face, with nothing behind it: one line, one button, + // and no PIN field to stare at. A disabled text box on a screen that + // will never accept a PIN is an invitation to try. + if (!method.requiresPin) { + Text( + text = LockCopy.BIOMETRIC_HINT, + style = MaterialTheme.typography.bodyMedium, + textAlign = TextAlign.Center, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + + state.biometricNotice?.let { notice -> + Spacer(Modifier.height(16.dp)) + Text( + text = notice, + style = MaterialTheme.typography.bodyMedium, + textAlign = TextAlign.Center, + color = MaterialTheme.colorScheme.error, + // Announced when it changes: the button looks the same + // before and after a failed attempt, so without this the + // only feedback is visual. + modifier = Modifier.semantics { liveRegion = LiveRegionMode.Polite }, + ) + } + + Spacer(Modifier.height(24.dp)) + Button(onClick = onUseBiometric, modifier = Modifier.fillMaxWidth()) { + Text(if (state.biometricNotice == null) LockCopy.USE_BIOMETRIC_ONLY else LockCopy.TRY_AGAIN) + } + + Spacer(Modifier.height(16.dp)) + TextButton(onClick = onForgot) { Text(LockCopy.CANNOT_USE_BIOMETRIC) } + return@Column + } + OutlinedTextField( value = pin, onValueChange = { entered -> pin = entered.filter(Char::isDigit).take(MAX_PIN) }, diff --git a/app/src/main/kotlin/dev/privacyllc/period/feature/lock/LockSettingsCopy.kt b/app/src/main/kotlin/dev/privacyllc/period/feature/lock/LockSettingsCopy.kt new file mode 100644 index 0000000..9a23b37 --- /dev/null +++ b/app/src/main/kotlin/dev/privacyllc/period/feature/lock/LockSettingsCopy.kt @@ -0,0 +1,107 @@ +package dev.privacyllc.period.feature.lock + +/** + * Everything the App lock screen says. + * + * Centralised for the reason [LockCopy] gives about the lock screen, plus one of + * its own: these strings are where the app explains a choice that cannot be + * undone by support, a password reset, or us. If any of it is vague, somebody + * loses their history to a decision they did not understand they were making. + * + * Two rules a test enforces. Nothing here names what the app records — the + * settings screen is not semi-public like the lock screen, but the wording + * travels into screenshots and support conversations. And the PIN option says, + * in the same breath, that it belongs to *this app* and not to the phone — + * because a person who assumes otherwise will assume they can reset it the way + * they reset a phone PIN, and they cannot. + */ +internal object LockSettingsCopy { + + const val TITLE = "App lock" + const val HOW = "How the app unlocks" + + const val OPTION_OFF = "Off" + const val OPTION_OFF_DETAIL = "The app opens like any other." + + const val OPTION_PIN = "A PIN just for this app" + const val OPTION_PIN_DETAIL = + "A PIN you choose here, separate from your phone's. Nobody can reset it: if you " + + "forget it, the only way back in is to erase what you have recorded." + + const val OPTION_BIOMETRIC = "Fingerprint or face" + const val OPTION_BIOMETRIC_DETAIL = + "Anyone whose fingerprint or face is set up on this phone — now or later — can open " + + "the app. There is no PIN to fall back on." + + const val OPTION_EITHER = "Either" + const val OPTION_EITHER_DETAIL = + "Your app PIN, or a fingerprint or face set up on this phone. Whichever is quicker." + + const val UNAVAILABLE_NOT_ENROLLED = "Not available — this phone has no fingerprint or face set up." + const val UNAVAILABLE_NO_HARDWARE = "Not available on this phone." + + const val WHILE_ON = + "While the lock is on, the app is hidden in the task switcher and screenshots are blocked." + + const val CHANGE_PIN = "Change PIN" + const val CHANGE_PIN_DETAIL = "You will be asked for the current one first." + + const val CONFIRM_TO_TURN_OFF = "Enter your app PIN to turn the lock off" + const val CONFIRM_CURRENT = "Enter your current app PIN" + const val CONFIRM_TO_CHANGE_METHOD = "Enter your app PIN to change how the app unlocks" + + /** Said before the first digit, and the sentence Kaspa asked for by name. */ + const val PIN_IS_THIS_APPS = "This PIN is for this app only" + const val PIN_IS_THIS_APPS_DETAIL = + "It is separate from your phone's own PIN, pattern or password — knowing one does " + + "not open the other." + + const val BIOMETRIC_CONSENT_TITLE = "Before you choose fingerprint or face" + const val BIOMETRIC_WHO_HEADING = "Who it lets in" + const val BIOMETRIC_WHO_BODY = + "Anyone whose fingerprint or face is set up on this phone can open the app — " + + "including anyone who adds one later. The app cannot tell them apart." + const val BIOMETRIC_NO_PIN_HEADING = "There is no PIN behind it" + const val BIOMETRIC_NO_PIN_BODY = + "If this phone's fingerprint or face unlock stops working, or every fingerprint is " + + "removed, the app stays closed until one is set up again. If that worries you, " + + "choose Either." + const val BIOMETRIC_CONSENT_ACCEPT = "I understand — use fingerprint or face" + const val NOT_NOW = "Not now" + + const val PROVE_TITLE = "Check it works on this phone" + const val PROVE_BODY = + "Unlock once with your fingerprint or face now. Nothing changes until it succeeds." + + const val MESSAGE_PIN = "App lock is on. The app asks for your app PIN." + const val MESSAGE_BIOMETRIC = "App lock is on. The app opens with a fingerprint or face." + const val MESSAGE_EITHER = "App lock is on. Your app PIN or a fingerprint or face opens it." + const val MESSAGE_OFF = "App lock is off." + const val MESSAGE_PIN_CHANGED = "Your app PIN has been changed." + const val MESSAGE_WRONG_PIN = "That is not the PIN." + const val MESSAGE_COULD_NOT_SET = "This device could not store a PIN, so nothing has changed." + const val MESSAGE_BIOMETRIC_NOT_CONFIRMED = + "Fingerprint or face was not confirmed, so nothing has changed." + const val MESSAGE_PIN_ON_BIOMETRIC_NOT_ADDED = + "App lock is on with your app PIN. Fingerprint or face was not added — you can add " + + "it here any time." + + /** The whole surface, for the test that checks it at once. */ + val all: List = listOf( + TITLE, HOW, + OPTION_OFF, OPTION_OFF_DETAIL, + OPTION_PIN, OPTION_PIN_DETAIL, + OPTION_BIOMETRIC, OPTION_BIOMETRIC_DETAIL, + OPTION_EITHER, OPTION_EITHER_DETAIL, + UNAVAILABLE_NOT_ENROLLED, UNAVAILABLE_NO_HARDWARE, + WHILE_ON, CHANGE_PIN, CHANGE_PIN_DETAIL, + CONFIRM_TO_TURN_OFF, CONFIRM_CURRENT, CONFIRM_TO_CHANGE_METHOD, + PIN_IS_THIS_APPS, PIN_IS_THIS_APPS_DETAIL, + BIOMETRIC_CONSENT_TITLE, BIOMETRIC_WHO_HEADING, BIOMETRIC_WHO_BODY, + BIOMETRIC_NO_PIN_HEADING, BIOMETRIC_NO_PIN_BODY, BIOMETRIC_CONSENT_ACCEPT, NOT_NOW, + PROVE_TITLE, PROVE_BODY, + MESSAGE_PIN, MESSAGE_BIOMETRIC, MESSAGE_EITHER, MESSAGE_OFF, MESSAGE_PIN_CHANGED, + MESSAGE_WRONG_PIN, MESSAGE_COULD_NOT_SET, MESSAGE_BIOMETRIC_NOT_CONFIRMED, + MESSAGE_PIN_ON_BIOMETRIC_NOT_ADDED, + ) +} diff --git a/app/src/main/kotlin/dev/privacyllc/period/feature/lock/LockSettingsScreen.kt b/app/src/main/kotlin/dev/privacyllc/period/feature/lock/LockSettingsScreen.kt index faa7d19..530fce7 100644 --- a/app/src/main/kotlin/dev/privacyllc/period/feature/lock/LockSettingsScreen.kt +++ b/app/src/main/kotlin/dev/privacyllc/period/feature/lock/LockSettingsScreen.kt @@ -250,12 +250,24 @@ private fun Overview( state.message?.let { message -> Spacer(Modifier.height(16.dp)) Text( + // Every message names the method, so "it is on" is never + // ambiguous about which way in she now has. text = when (message) { - LockSettings.Message.PIN_SET -> "App lock is on." - LockSettings.Message.PIN_REMOVED -> "App lock is off." - LockSettings.Message.WRONG_PIN -> "That is not the PIN." - LockSettings.Message.COULD_NOT_SET -> - "This device could not store a PIN, so the lock has not been turned on." + LockSettings.Message.PIN_SET -> LockSettingsCopy.MESSAGE_PIN + LockSettings.Message.PIN_REMOVED -> LockSettingsCopy.MESSAGE_OFF + LockSettings.Message.WRONG_PIN -> LockSettingsCopy.MESSAGE_WRONG_PIN + LockSettings.Message.COULD_NOT_SET -> LockSettingsCopy.MESSAGE_COULD_NOT_SET + LockSettings.Message.METHOD_CHANGED -> when (state.method) { + dev.privacyllc.period.core.security.LockMethod.BIOMETRIC -> + LockSettingsCopy.MESSAGE_BIOMETRIC + dev.privacyllc.period.core.security.LockMethod.PIN_AND_BIOMETRIC -> + LockSettingsCopy.MESSAGE_EITHER + else -> LockSettingsCopy.MESSAGE_PIN + } + LockSettings.Message.BIOMETRIC_NOT_CONFIRMED -> + LockSettingsCopy.MESSAGE_BIOMETRIC_NOT_CONFIRMED + LockSettings.Message.PIN_ON_BIOMETRIC_NOT_ADDED -> + LockSettingsCopy.MESSAGE_PIN_ON_BIOMETRIC_NOT_ADDED }, style = MaterialTheme.typography.bodyMedium, color = if (message == LockSettings.Message.WRONG_PIN) { diff --git a/app/src/main/kotlin/dev/privacyllc/period/feature/lock/LockSettingsViewModel.kt b/app/src/main/kotlin/dev/privacyllc/period/feature/lock/LockSettingsViewModel.kt index 0b81501..a600f3c 100644 --- a/app/src/main/kotlin/dev/privacyllc/period/feature/lock/LockSettingsViewModel.kt +++ b/app/src/main/kotlin/dev/privacyllc/period/feature/lock/LockSettingsViewModel.kt @@ -5,6 +5,7 @@ import androidx.lifecycle.viewModelScope import dagger.hilt.android.lifecycle.HiltViewModel import dev.privacyllc.period.core.datastore.UserPreferencesRepository import dev.privacyllc.period.core.security.AppLockRepository +import dev.privacyllc.period.core.security.LockMethod import dev.privacyllc.period.core.security.UnlockResult import dev.privacyllc.period.lock.AppLockController import kotlinx.coroutines.CoroutineExceptionHandler @@ -20,12 +21,48 @@ import javax.inject.Inject /** What the App lock settings screen is showing. */ data class LockSettings( + val method: LockMethod = LockMethod.NONE, val hasPin: Boolean = false, val biometricEnabled: Boolean = false, val busy: Boolean = false, val message: Message? = null, + /** The change being made, and how far through it the user is. */ + val pending: PendingChange? = null, ) { - enum class Message { PIN_SET, PIN_REMOVED, WRONG_PIN, COULD_NOT_SET } + enum class Message { + PIN_SET, PIN_REMOVED, WRONG_PIN, COULD_NOT_SET, + METHOD_CHANGED, BIOMETRIC_NOT_CONFIRMED, PIN_ON_BIOMETRIC_NOT_ADDED, + } +} + +/** + * A method change in flight. + * + * The stage lives here rather than in the composable, and **only a verified + * outcome advances it**. That is not tidiness: the previous screen advanced its + * own step in the same breath as asking, without waiting for the answer, and any + * four digits reached "choose a PIN". Keeping the stage where the verification + * happens removes that whole class of bug rather than fixing one instance. + */ +data class PendingChange( + val target: LockMethod, + val stage: Stage, + /** A successful scan already given during this flow. */ + val scanned: Boolean = false, +) { + enum class Stage { + /** Prove the method that is on now, before changing it. */ + CONFIRM_CURRENT, + + /** Say plainly who a fingerprint lets in, before it is turned on. */ + BIOMETRIC_CONSENT, + + /** Choose a PIN — the first, or a replacement. */ + SET_PIN, + + /** One successful scan on this phone, before committing to it. */ + PROVE_BIOMETRIC, + } } /** @@ -48,23 +85,167 @@ class LockSettingsViewModel @Inject constructor( private val _busy = MutableStateFlow(false) private val _message = MutableStateFlow(null) + private val _pending = MutableStateFlow(null) + val state: StateFlow = combine( - lock.hasPin, - preferences.preferences.map { it.biometricLockEnabled }, + lock.method, _busy, _message, - ) { hasPin, biometric, busy, message -> + _pending, + ) { method, busy, message, pending -> LockSettings( - hasPin = hasPin, - // Meaningless without a PIN to stand in for, so never shown as on - // without one. The stored flag is left alone rather than corrected, - // so turning the lock back on restores the choice the user made. - biometricEnabled = biometric && hasPin, + method = method, + hasPin = method.requiresPin, + biometricEnabled = method.allowsBiometric, busy = busy, message = message, + pending = pending, ) }.stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), LockSettings()) + /** + * Begin moving to [target]. + * + * The rule, in one place: **any change authenticates with the method that is + * on now**, and any target that allows a fingerprint needs one successful + * scan during this flow before it is committed. Turning the lock on from + * nothing needs neither, because there is nothing yet to prove. + */ + fun request(target: LockMethod) { + val current = state.value.method + if (target == current || _busy.value) return + _message.value = null + + _pending.value = PendingChange( + target = target, + stage = when { + // Something is on: prove it before changing it. Otherwise + // whoever is holding the unlocked phone simply switches it off, + // which is the adversary this feature exists for. + current.isOn -> PendingChange.Stage.CONFIRM_CURRENT + target.allowsBiometric -> PendingChange.Stage.BIOMETRIC_CONSENT + else -> PendingChange.Stage.SET_PIN + }, + ) + } + + /** Abandon it, and with it any permission the flow had earned. */ + fun cancelChange() { + _pending.value = null + replacementAuthorised = false + } + + /** The current PIN, offered to authorise a change. */ + fun confirmWithPin(pin: CharArray) = verifyCurrent(pin) { onCurrentProven() } + + /** + * A fingerprint result arriving during a change. + * + * Two different meanings, and the stage says which: proving the method that + * is on now, or proving the one being turned on. Only a success moves + * anything. + */ + fun onBiometricOutcome(succeeded: Boolean) { + val pending = _pending.value ?: return + if (!succeeded) { + _message.value = LockSettings.Message.BIOMETRIC_NOT_CONFIRMED + _pending.value = null + return + } + when (pending.stage) { + PendingChange.Stage.CONFIRM_CURRENT -> onCurrentProven(scanned = true) + PendingChange.Stage.PROVE_BIOMETRIC -> commit(pending.copy(scanned = true)) + else -> Unit + } + } + + fun acceptBiometricConsent() { + val pending = _pending.value ?: return + _pending.value = pending.copy(stage = PendingChange.Stage.PROVE_BIOMETRIC) + } + + /** The new PIN for a change that has already been authorised. */ + fun submitNewPin(pin: CharArray) { + val pending = _pending.value ?: return + if (pending.stage != PendingChange.Stage.SET_PIN) return + setPinFor(pending, pin) + } + + private fun onCurrentProven(scanned: Boolean = false) { + val pending = _pending.value ?: return + val next = pending.copy(scanned = pending.scanned || scanned) + _pending.value = when { + // Turning it off entirely: nothing left to ask. + next.target == LockMethod.NONE -> return commit(next) + // Adding or moving to a fingerprint: say who it lets in, then prove + // it works on this phone before anything is written. + next.target.allowsBiometric && !next.scanned -> + next.copy(stage = PendingChange.Stage.BIOMETRIC_CONSENT) + // Needs a PIN it does not have yet. + next.target.requiresPin && !state.value.method.requiresPin -> + next.copy(stage = PendingChange.Stage.SET_PIN) + else -> return commit(next) + } + } + + private fun commit(pending: PendingChange) { + _busy.value = true + viewModelScope.launch(handler) { + // Open this session before anything is written, for the reason + // setPin gives: the gate closes on (locked, not unlocked), and that + // pair must never be observable while she is standing in Settings. + controller.unlock() + + val ok = when (pending.target) { + LockMethod.NONE -> { lock.clearLock(); true } + LockMethod.BIOMETRIC -> { lock.setBiometricOnly(); true } + LockMethod.PIN, LockMethod.PIN_AND_BIOMETRIC -> lock.setMethod(pending.target) + } + + _pending.value = null + replacementAuthorised = false + _busy.value = false + _message.value = if (!ok) { + LockSettings.Message.COULD_NOT_SET + } else { + messageFor(pending.target) + } + } + } + + private fun setPinFor(pending: PendingChange, pin: CharArray) { + _busy.value = true + viewModelScope.launch(handler) { + controller.unlock() + // A PIN-and-fingerprint target commits the PIN first and adds the + // fingerprint only after a scan, so a cancelled prompt leaves a + // working PIN lock rather than nothing. + val enrolAs = if (pending.target.allowsBiometric && !pending.scanned) LockMethod.PIN else pending.target + val ok = lock.setPin(pin, enrolAs) + pin.fill('\u0000') + _busy.value = false + + if (!ok) { + _pending.value = null + _message.value = LockSettings.Message.COULD_NOT_SET + return@launch + } + + if (enrolAs != pending.target) { + _pending.value = pending.copy(stage = PendingChange.Stage.BIOMETRIC_CONSENT) + } else { + _pending.value = null + _message.value = messageFor(pending.target) + } + } + } + + private fun messageFor(method: LockMethod) = when (method) { + LockMethod.NONE -> LockSettings.Message.PIN_REMOVED + LockMethod.PIN -> LockSettings.Message.PIN_SET + LockMethod.BIOMETRIC, LockMethod.PIN_AND_BIOMETRIC -> LockSettings.Message.METHOD_CHANGED + } + private val handler = CoroutineExceptionHandler { _, _ -> _busy.value = false _message.value = LockSettings.Message.COULD_NOT_SET @@ -166,11 +347,6 @@ class LockSettingsViewModel @Inject constructor( onAuthorised() } - /** Abandoning the change withdraws the permission it was granted. */ - fun cancelChange() { - replacementAuthorised = false - } - fun removePin(current: CharArray, onRemoved: () -> Unit = {}) = verifyCurrent(current) { lock.clearLock() preferences.setBiometricLockEnabled(false) diff --git a/app/src/main/kotlin/dev/privacyllc/period/lock/AppLockGate.kt b/app/src/main/kotlin/dev/privacyllc/period/lock/AppLockGate.kt index 11f2d83..7a03a90 100644 --- a/app/src/main/kotlin/dev/privacyllc/period/lock/AppLockGate.kt +++ b/app/src/main/kotlin/dev/privacyllc/period/lock/AppLockGate.kt @@ -64,14 +64,14 @@ fun AppLockGate( onDispose { lifecycleOwner.lifecycle.removeObserver(observer) } } - when (state) { + when (val current = state) { // One frame, drawing nothing. Same choice as RootState.Loading in // PeriodApp.kt: a spinner that appears for 16ms reads as jank. Crucially // it is not the unlocked branch, so no cycle data composes while the // stores are still answering. LockState.Unknown -> Unit - LockState.Locked -> LockScreen(viewModel = viewModel) + is LockState.Locked -> LockScreen(viewModel = viewModel, method = current.method) LockState.Unlocked -> { // Drains any notification action parked while the lock was closed. diff --git a/app/src/main/kotlin/dev/privacyllc/period/lock/AppLockViewModel.kt b/app/src/main/kotlin/dev/privacyllc/period/lock/AppLockViewModel.kt index 4c3064d..6fde689 100644 --- a/app/src/main/kotlin/dev/privacyllc/period/lock/AppLockViewModel.kt +++ b/app/src/main/kotlin/dev/privacyllc/period/lock/AppLockViewModel.kt @@ -6,6 +6,7 @@ import dagger.hilt.android.lifecycle.HiltViewModel import dev.privacyllc.period.core.datastore.UserPreferencesRepository import dev.privacyllc.period.core.notifications.ReminderActionRequest import dev.privacyllc.period.core.security.AppLockRepository +import dev.privacyllc.period.core.security.LockMethod import dev.privacyllc.period.core.security.UnlockResult import dev.privacyllc.period.notifications.NotificationActionHandler import kotlinx.coroutines.CoroutineExceptionHandler @@ -24,7 +25,11 @@ sealed interface LockState { /** The stores have not answered yet. Renders nothing — see [AppLockGate]. */ data object Unknown : LockState - data object Locked : LockState + /** + * Shut, and by which method — the lock screen has to know whether there is a + * PIN field on it at all. + */ + data class Locked(val method: LockMethod) : LockState data object Unlocked : LockState } @@ -36,6 +41,14 @@ data class LockScreenState( val waitMillis: Long = 0L, val keyUnavailable: Boolean = false, val biometricOffered: Boolean = false, + /** + * What the last biometric attempt said, when it said anything. + * + * Only ever set from a decision that persists nothing — an unavailable + * sensor must not switch a fingerprint-only lock off, or anyone who knows + * the phone's own PIN could remove the enrolled prints and walk in. + */ + val biometricNotice: String? = null, ) /** @@ -84,14 +97,14 @@ class AppLockViewModel @Inject constructor( * screen, with a forecast on it, before the lock had a chance to close. */ val state: StateFlow = - combine(lock.hasPin, controller.unlocked, ::lockStateOf) + combine(lock.method, controller.unlocked, ::lockStateOf) .stateIn(viewModelScope, SharingStarted.Eagerly, LockState.Unknown) /** True only when a fingerprint may stand in for the PIN, which needs a PIN to stand in for. */ val biometricAllowed: StateFlow = combine( - preferences.preferences.map { it.biometricLockEnabled }, - lock.hasPin, - ) { enabled, hasPin -> enabled && hasPin } + lock.method, + controller.unlocked, + ) { method, _ -> method.allowsBiometric } .stateIn(viewModelScope, SharingStarted.Eagerly, false) private val handler = CoroutineExceptionHandler { _, _ -> @@ -102,6 +115,10 @@ class AppLockViewModel @Inject constructor( fun submit(pin: CharArray) { if (_screen.value.checking) return + // A PIN cannot open a lock that does not have one. The repository + // refuses too; this is the near half of the same rule, so a fingerprint- + // only screen with a stale record cannot be typed past. + if (state.value.let { it is LockState.Locked && !it.method.requiresPin }) return _screen.value = _screen.value.copy(checking = true, wrong = false) viewModelScope.launch(handler) { when (val outcome = lock.check(pin)) { @@ -165,6 +182,14 @@ class AppLockViewModel @Inject constructor( viewModelScope.launch(handler) { preferences.setBiometricLockEnabled(false) } } + /** + * What the last biometric attempt said. Cleared by a successful unlock, + * which replaces the whole screen state. + */ + fun showBiometricNotice(notice: String) { + _screen.value = _screen.value.copy(biometricNotice = notice) + } + fun refreshLockout() { viewModelScope.launch(handler) { val wait = lock.lockoutRemainingMillis() @@ -183,8 +208,8 @@ class AppLockViewModel @Inject constructor( * never returns [LockState.Locked] while it happens, and a copy of the rule in * the test would prove that about the copy. */ -internal fun lockStateOf(hasPin: Boolean, unlocked: Boolean): LockState = when { - !hasPin -> LockState.Unlocked +internal fun lockStateOf(method: LockMethod, unlocked: Boolean): LockState = when { + !method.isOn -> LockState.Unlocked unlocked -> LockState.Unlocked - else -> LockState.Locked + else -> LockState.Locked(method) } diff --git a/app/src/main/kotlin/dev/privacyllc/period/lock/BiometricUnlock.kt b/app/src/main/kotlin/dev/privacyllc/period/lock/BiometricUnlock.kt index dfc5250..9bfbacf 100644 --- a/app/src/main/kotlin/dev/privacyllc/period/lock/BiometricUnlock.kt +++ b/app/src/main/kotlin/dev/privacyllc/period/lock/BiometricUnlock.kt @@ -7,6 +7,7 @@ import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.core.content.ContextCompat import androidx.fragment.app.FragmentActivity +import dev.privacyllc.period.feature.lock.LockCopy /** What the biometric affordance should do, decided once per composition. */ internal class BiometricUnlock( @@ -54,6 +55,13 @@ internal fun rememberBiometricUnlock( onEndAuth: () -> Unit, onUnlocked: () -> Unit, onUnavailable: () -> Unit, + /** + * Say what happened, without recording it anywhere. + * + * The distinction matters most in fingerprint-only mode: the screen has to + * explain why nothing opened, and must not respond by removing the lock. + */ + onNotice: (String) -> Unit = {}, ): BiometricUnlock { // Suppressed for the life of the process once the hardware reports a // permanent lockout: only the device credential clears that, and this app's @@ -92,20 +100,40 @@ internal fun rememberBiometricUnlock( -> Unit // No fingerprint enrolled any more, or the hardware is gone. - // Turn the offer off so the user is not sent back to a - // button that cannot work. - BiometricPrompt.ERROR_NO_BIOMETRICS, + // + // Where a PIN sits behind this, turn the offer off so the + // user is not sent back to a button that cannot work. Where + // it does not, `onUnavailable` is deliberately inert — see + // LockScreen — and the notice says how to get back in + // instead. Switching the lock off here would let anybody who + // knows the phone's own PIN remove the fingerprints and walk + // straight in. + BiometricPrompt.ERROR_NO_BIOMETRICS -> { + onUnavailable() + onNotice(LockCopy.NOT_SET_UP) + } + BiometricPrompt.ERROR_HW_NOT_PRESENT, - -> onUnavailable() + BiometricPrompt.ERROR_HW_UNAVAILABLE, + -> { + onUnavailable() + onNotice(LockCopy.NO_HARDWARE) + } // Cleared only by the device credential, which this app's // PIN is not — so retrying produces the same error forever. - BiometricPrompt.ERROR_LOCKOUT_PERMANENT -> + // The notice names what actually ends it; hiding the button + // would leave a fingerprint-only screen with nothing on it. + BiometricPrompt.ERROR_LOCKOUT_PERMANENT -> { permanentlyLockedOut.value = true + onNotice(LockCopy.LOCKED_OUT_UNTIL_PHONE_UNLOCK) + } - // Transient: temporary lockout, hardware busy, a vendor - // string. Leave the setting alone; the PIN still works. - else -> Unit + BiometricPrompt.ERROR_LOCKOUT -> onNotice(LockCopy.LOCKED_OUT_FOR_NOW) + + // Transient: hardware busy, a vendor string, a timeout. + // Leave every setting alone. + else -> onNotice(LockCopy.UNAVAILABLE_NOW) } } diff --git a/app/src/test/kotlin/dev/privacyllc/period/feature/lock/LockSettingsCopyTest.kt b/app/src/test/kotlin/dev/privacyllc/period/feature/lock/LockSettingsCopyTest.kt new file mode 100644 index 0000000..bc59cd3 --- /dev/null +++ b/app/src/test/kotlin/dev/privacyllc/period/feature/lock/LockSettingsCopyTest.kt @@ -0,0 +1,62 @@ +package dev.privacyllc.period.feature.lock + +import dev.privacyllc.period.core.notifications.NotificationCopy +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * What the App lock screen promises, held to the same standard as the lock + * screen's own copy. + * + * The settings screen is not semi-public the way a lock screen is, but its + * wording ends up in screenshots and support conversations — and more to the + * point, it explains a choice nobody can undo for her. Vague here costs somebody + * their history. + */ +class LockSettingsCopyTest { + + @Test fun `no setting explains itself by naming what the app records`() { + val surface = LockSettingsCopy.all.joinToString(" ").lowercase() + NotificationCopy.SENSITIVE_WORDS.forEach { + assertFalse("the App lock screen says \"$it\"", surface.contains(it)) + } + } + + @Test fun `the PIN option says it belongs to this app and not the phone`() { + // The misunderstanding this prevents: somebody assuming an app PIN can + // be reset the way a phone PIN can. It cannot, and by then it is too + // late to explain. + val pin = (LockSettingsCopy.OPTION_PIN + " " + LockSettingsCopy.OPTION_PIN_DETAIL).lowercase() + assertTrue("the PIN option does not say it is this app's", pin.contains("app")) + assertTrue("the PIN option does not distinguish it from the phone's", pin.contains("phone")) + + val consent = (LockSettingsCopy.PIN_IS_THIS_APPS + " " + LockSettingsCopy.PIN_IS_THIS_APPS_DETAIL) + .lowercase() + assertTrue(consent.contains("this app only") || consent.contains("for this app")) + assertTrue(consent.contains("phone")) + } + + @Test fun `the fingerprint option says who it lets in`() { + val text = LockSettingsCopy.OPTION_BIOMETRIC_DETAIL.lowercase() + assertTrue("it does not say anyone enrolled can open it", text.contains("anyone")) + assertTrue("it does not say which phone", text.contains("this phone")) + // Including somebody who enrols later, which is the case a user cannot + // work out for herself. + assertTrue("it does not mention later enrolment", text.contains("later")) + } + + @Test fun `no lock setting calls anything safe or secure`() { + // Same rule as everywhere else: those words promise more than a gate in + // front of an unencrypted database can deliver. + val surface = LockSettingsCopy.all.joinToString(" ").lowercase() + listOf("safe", "secure", "encrypted", "protected").forEach { + assertFalse("the App lock screen says \"$it\"", surface.contains(it)) + } + } + + @Test fun `the checked surface is not accidentally empty`() { + assertTrue(LockSettingsCopy.all.size >= 30) + assertTrue(LockSettingsCopy.all.none { it.isBlank() }) + } +} diff --git a/app/src/test/kotlin/dev/privacyllc/period/feature/lock/LockSettingsViewModelTest.kt b/app/src/test/kotlin/dev/privacyllc/period/feature/lock/LockSettingsViewModelTest.kt index ea6ef7d..64b0365 100644 --- a/app/src/test/kotlin/dev/privacyllc/period/feature/lock/LockSettingsViewModelTest.kt +++ b/app/src/test/kotlin/dev/privacyllc/period/feature/lock/LockSettingsViewModelTest.kt @@ -3,6 +3,8 @@ package dev.privacyllc.period.feature.lock import androidx.datastore.preferences.core.PreferenceDataStoreFactory import dev.privacyllc.period.core.datastore.UserPreferencesRepository import dev.privacyllc.period.core.security.AppLockRepository +import dev.privacyllc.period.core.security.LockMethod +import org.junit.Assert.assertNull import dev.privacyllc.period.core.security.MacProvider import dev.privacyllc.period.core.security.UnlockResult import dev.privacyllc.period.lock.AppLockController @@ -153,7 +155,7 @@ class LockSettingsViewModelTest { fun `setting the first PIN never closes the gate`() { val seen = mutableListOf() val watching = CoroutineScope(dispatcher).launch { - combine(lock.hasPin, controller.unlocked, ::lockStateOf).collect { seen += it } + combine(lock.method, controller.unlocked, ::lockStateOf).collect { seen += it } } vm.setPin(pin("2468")) @@ -163,7 +165,7 @@ class LockSettingsViewModelTest { // Not "ends unlocked" — never locked, at any instant. The gate disposes // the whole app subtree when it closes, so a single frame of Locked is // the user back at the lock screen typing the PIN she just chose. - assertFalse("the gate closed while the PIN was being set: $seen", LockState.Locked in seen) + assertFalse("the gate closed while the PIN was being set: $seen", seen.any { it is LockState.Locked }) assertTrue(runBlocking { lock.hasPin.first() }) assertTrue(controller.unlocked.value) } @@ -245,4 +247,103 @@ class LockSettingsViewModelTest { assertFalse(runBlocking { lock.hasPin.first() }) assertEquals(LockSettings.Message.PIN_REMOVED, vm.state.value.message) } + + // ----------------------------------------------------------------------- + // Choosing how the app unlocks + // ----------------------------------------------------------------------- + + @Test fun `turning the lock on from nothing asks for no permission it cannot have`() { + vm.request(LockMethod.PIN) + await("the PIN step") { vm.state.value.pending?.stage == PendingChange.Stage.SET_PIN } + + vm.submitNewPin(pin("2468")) + await("the lock to be on") { vm.state.value.method == LockMethod.PIN } + } + + @Test fun `changing the method asks for the method that is on now`() { + vm.setPin(pin("2468")) + await("a PIN") { vm.state.value.method == LockMethod.PIN } + + vm.request(LockMethod.NONE) + await("the confirm step") { vm.state.value.pending?.stage == PendingChange.Stage.CONFIRM_CURRENT } + + // Whoever is holding the unlocked phone must not simply switch it off. + vm.confirmWithPin(pin("1111")) + await("the refusal") { vm.state.value.message == LockSettings.Message.WRONG_PIN } + assertEquals(LockMethod.PIN, runBlocking { lock.method.first() }) + + vm.confirmWithPin(pin("2468")) + await("the lock to come off") { runBlocking { lock.method.first() } == LockMethod.NONE } + } + + @Test fun `a fingerprint is never turned on without one working on this phone`() { + vm.setPin(pin("2468")) + await("a PIN") { vm.state.value.method == LockMethod.PIN } + + vm.request(LockMethod.PIN_AND_BIOMETRIC) + vm.confirmWithPin(pin("2468")) + await("the consent step") { vm.state.value.pending?.stage == PendingChange.Stage.BIOMETRIC_CONSENT } + + // Nothing has changed yet, and consent alone must not change it either. + assertEquals(LockMethod.PIN, runBlocking { lock.method.first() }) + vm.acceptBiometricConsent() + await("the prove step") { vm.state.value.pending?.stage == PendingChange.Stage.PROVE_BIOMETRIC } + assertEquals(LockMethod.PIN, runBlocking { lock.method.first() }) + + // A scan that does not succeed leaves everything as it was. + vm.onBiometricOutcome(succeeded = false) + await("the report") { vm.state.value.message == LockSettings.Message.BIOMETRIC_NOT_CONFIRMED } + assertEquals(LockMethod.PIN, runBlocking { lock.method.first() }) + + vm.request(LockMethod.PIN_AND_BIOMETRIC) + vm.confirmWithPin(pin("2468")) + await("consent again") { vm.state.value.pending?.stage == PendingChange.Stage.BIOMETRIC_CONSENT } + vm.acceptBiometricConsent() + vm.onBiometricOutcome(succeeded = true) + + await("either") { runBlocking { lock.method.first() } == LockMethod.PIN_AND_BIOMETRIC } + // The PIN still opens it — that is what "either" means. + assertTrue(runBlocking { lock.check(pin("2468")) } is UnlockResult.Unlocked) + } + + @Test fun `moving to fingerprint-only drops the PIN only once a scan has worked`() { + vm.setPin(pin("2468")) + await("a PIN") { vm.state.value.method == LockMethod.PIN } + + vm.request(LockMethod.BIOMETRIC) + vm.confirmWithPin(pin("2468")) + await("consent") { vm.state.value.pending?.stage == PendingChange.Stage.BIOMETRIC_CONSENT } + vm.acceptBiometricConsent() + + // Still a PIN at this point: abandoning here must not leave her with a + // method she never proved she can use. + assertEquals(LockMethod.PIN, runBlocking { lock.method.first() }) + + vm.onBiometricOutcome(succeeded = true) + await("fingerprint only") { runBlocking { lock.method.first() } == LockMethod.BIOMETRIC } + + // And the PIN is genuinely gone, not merely unused. + assertTrue(runBlocking { lock.check(pin("2468")) } is UnlockResult.NoPin) + } + + @Test fun `abandoning a change leaves everything as it was`() { + vm.setPin(pin("2468")) + await("a PIN") { vm.state.value.method == LockMethod.PIN } + + vm.request(LockMethod.NONE) + await("the confirm step") { vm.state.value.pending != null } + vm.cancelChange() + + await("no pending change") { vm.state.value.pending == null } + assertEquals(LockMethod.PIN, runBlocking { lock.method.first() }) + } + + @Test fun `choosing the method already in force does nothing`() { + vm.setPin(pin("2468")) + await("a PIN") { vm.state.value.method == LockMethod.PIN } + + vm.request(LockMethod.PIN) + + assertNull(vm.state.value.pending) + } }