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) + } }