feat: give the lock a method, and every old install the one it had
Groundwork for offering a PIN, a fingerprint, or either. This commit is
the storage and the migration; the screens follow.
LockMethod is an enum rather than two booleans because BIOMETRIC is the
state with no fallback, and everything that has to be careful -- the
migration, the settings transitions, what happens when a sensor stops
working -- is careful specifically about the absence of a PIN. A boolean
pair spreads that condition across two fields nothing stops disagreeing.
It lives in the lock's own DataStore, not UserPreferences, and that
placement is the point: resetToDefaults() there is edit { clear() }, so a
fingerprint-only lock recorded beside the theme would be one "reset my
settings" away from silently vanishing.
resolve() decides what the stored state actually means, and each of its
three rules closes a way somebody could be locked out or wrongly let in. A
verifier with no recorded method reads as PIN, so the gate is shut from
the first frame of an old install rather than waiting for a migration. A
method needing a PIN with no verifier reads as NONE -- the same policy
VerifierRecord.decode already applies, because a lock nobody can open is
worse than no lock when the way out is behind it. An unrecognised name
falls back to the verifier, so a newer build's value cannot brick an older
one.
A PIN is never checked for a method that does not use one, even with a
stale record in the file.
The migration can never produce BIOMETRIC. recordMigratedMethod refuses it
at the API, and both its conditions -- nothing recorded, verifier agrees --
are evaluated inside the DataStore transaction, so a migration racing an
erase cannot resurrect a lock the user just removed. Getting this wrong
does not show a wrong number on a screen: it locks somebody out of their
own history on an update they did not ask for.
The migration lives in app because it reads two stores and core/security
depends on nothing.
24 repository tests and 9 migration tests, including all four legacy
combinations asserting the never-PIN-less invariant, proved by removing
the require and watching exactly one go red.
Part of #63
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
f99197ffd5
commit
9b2b332a98
|
|
@ -8,6 +8,7 @@ import kotlinx.coroutines.CoroutineExceptionHandler
|
||||||
import kotlinx.coroutines.CoroutineScope
|
import kotlinx.coroutines.CoroutineScope
|
||||||
import kotlinx.coroutines.Dispatchers
|
import kotlinx.coroutines.Dispatchers
|
||||||
import kotlinx.coroutines.SupervisorJob
|
import kotlinx.coroutines.SupervisorJob
|
||||||
|
import kotlinx.coroutines.launch
|
||||||
import javax.inject.Inject
|
import javax.inject.Inject
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|
@ -27,6 +28,8 @@ class PeriodApplication : Application(), Configuration.Provider {
|
||||||
|
|
||||||
@Inject lateinit var reminderCoordinator: dev.privacyllc.period.notifications.ReminderCoordinator
|
@Inject lateinit var reminderCoordinator: dev.privacyllc.period.notifications.ReminderCoordinator
|
||||||
|
|
||||||
|
@Inject lateinit var lockMethodMigration: dev.privacyllc.period.lock.LockMethodMigration
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Lives as long as the process, because what it watches does.
|
* Lives as long as the process, because what it watches does.
|
||||||
*
|
*
|
||||||
|
|
@ -39,6 +42,13 @@ class PeriodApplication : Application(), Configuration.Provider {
|
||||||
|
|
||||||
override fun onCreate() {
|
override fun onCreate() {
|
||||||
super.onCreate()
|
super.onCreate()
|
||||||
|
|
||||||
|
// Give an older install the lock method it was already using. Idempotent
|
||||||
|
// and cheap, and the gate is already shut without it — AppLockRepository
|
||||||
|
// reads a verifier with no recorded method as a PIN — so this runs on
|
||||||
|
// the ordinary scope rather than blocking startup.
|
||||||
|
applicationScope.launch { lockMethodMigration.run() }
|
||||||
|
|
||||||
reminderCoordinator.start(applicationScope)
|
reminderCoordinator.start(applicationScope)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,77 @@
|
||||||
|
package dev.privacyllc.period.lock
|
||||||
|
|
||||||
|
import dev.privacyllc.period.core.datastore.UserPreferencesRepository
|
||||||
|
import dev.privacyllc.period.core.security.AppLockRepository
|
||||||
|
import dev.privacyllc.period.core.security.LockMethod
|
||||||
|
import kotlinx.coroutines.flow.first
|
||||||
|
import javax.inject.Inject
|
||||||
|
import javax.inject.Singleton
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Give an existing install the lock method it was already using.
|
||||||
|
*
|
||||||
|
* ## Why this lives in `app`
|
||||||
|
*
|
||||||
|
* It reads two stores — the lock's own file and the user preferences — and
|
||||||
|
* `core/security` deliberately depends on nothing. A migration is the one place
|
||||||
|
* those two facts have to meet, so it meets them here rather than dragging a
|
||||||
|
* dependency into the module that holds the secret.
|
||||||
|
*
|
||||||
|
* ## The invariant that matters
|
||||||
|
*
|
||||||
|
* **It can never produce [LockMethod.BIOMETRIC].** Every method it writes has a
|
||||||
|
* PIN behind it, so nobody wakes up after an update facing a sensor as their
|
||||||
|
* only way in — which, under a no-recovery policy, would mean a broken sensor
|
||||||
|
* costs somebody their entire history. `recordMigratedMethod` refuses that value
|
||||||
|
* at the API, and a test proves the refusal by removing it.
|
||||||
|
*
|
||||||
|
* ## Why it is safe to run late, and often
|
||||||
|
*
|
||||||
|
* `AppLockRepository.resolve` already reads a verifier with no recorded method
|
||||||
|
* as `PIN`, so the gate is shut from the first frame whether or not this has
|
||||||
|
* run. The only visible effect of it completing is the fingerprint option
|
||||||
|
* appearing where it was already switched on. It is idempotent, so running it in
|
||||||
|
* every process — including the ones WorkManager starts — costs one read.
|
||||||
|
*/
|
||||||
|
@Singleton
|
||||||
|
class LockMethodMigration @Inject constructor(
|
||||||
|
private val lock: AppLockRepository,
|
||||||
|
private val preferences: UserPreferencesRepository,
|
||||||
|
) {
|
||||||
|
|
||||||
|
suspend fun run() {
|
||||||
|
if (lock.hasStoredMethod()) return
|
||||||
|
|
||||||
|
if (!lock.hasPin.first()) {
|
||||||
|
lock.recordMigratedMethod(LockMethod.NONE)
|
||||||
|
clearLegacyFlag()
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
// A flag that cannot be read is a flag that was not set: the worst that
|
||||||
|
// costs is a fingerprint shortcut the user switches back on, and the
|
||||||
|
// alternative — failing the migration — leaves the method unrecorded
|
||||||
|
// forever.
|
||||||
|
val fingerprintWasOn = runCatching { preferences.legacyBiometricLockEnabled.first() }
|
||||||
|
.getOrNull() == true
|
||||||
|
|
||||||
|
lock.recordMigratedMethod(
|
||||||
|
if (fingerprintWasOn) LockMethod.PIN_AND_BIOMETRIC else LockMethod.PIN,
|
||||||
|
)
|
||||||
|
// After the write, not before. A failed clear leaves a stale key nothing
|
||||||
|
// reads again; a clear that happened before a failed write would lose
|
||||||
|
// the choice.
|
||||||
|
clearLegacyFlag()
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Tidying up, and never a reason to fail.
|
||||||
|
*
|
||||||
|
* The file this writes to may be the unreadable one that made the flag
|
||||||
|
* unreadable to begin with — and by this point the method is already
|
||||||
|
* recorded, so the only thing left to lose is a key nothing reads again.
|
||||||
|
*/
|
||||||
|
private suspend fun clearLegacyFlag() {
|
||||||
|
runCatching { preferences.clearLegacyBiometricLock() }
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,178 @@
|
||||||
|
package dev.privacyllc.period.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 dev.privacyllc.period.core.security.MacProvider
|
||||||
|
import kotlinx.coroutines.CoroutineScope
|
||||||
|
import kotlinx.coroutines.Dispatchers
|
||||||
|
import kotlinx.coroutines.flow.first
|
||||||
|
import kotlinx.coroutines.runBlocking
|
||||||
|
import org.junit.Assert.assertEquals
|
||||||
|
import org.junit.Assert.assertFalse
|
||||||
|
import org.junit.Assert.assertNull
|
||||||
|
import org.junit.Assert.assertTrue
|
||||||
|
import org.junit.Before
|
||||||
|
import org.junit.Rule
|
||||||
|
import org.junit.Test
|
||||||
|
import org.junit.rules.TemporaryFolder
|
||||||
|
import org.junit.runner.RunWith
|
||||||
|
import org.robolectric.RobolectricTestRunner
|
||||||
|
import org.robolectric.annotation.Config
|
||||||
|
import javax.crypto.Mac
|
||||||
|
import javax.crypto.spec.SecretKeySpec
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Giving an existing install the lock method it was already using.
|
||||||
|
*
|
||||||
|
* The reason this has its own test file rather than a couple of cases somewhere:
|
||||||
|
* a migration that gets this wrong does not show a wrong number on a screen. It
|
||||||
|
* locks somebody out of their own history, permanently, on an update they did
|
||||||
|
* not ask for — because the policy behind this lock is that a forgotten PIN is
|
||||||
|
* not recoverable.
|
||||||
|
*
|
||||||
|
* So the load-bearing assertion here is a negative one: whatever the old install
|
||||||
|
* looked like, this can never produce a method with no PIN behind it.
|
||||||
|
*/
|
||||||
|
@RunWith(RobolectricTestRunner::class)
|
||||||
|
@Config(sdk = [34])
|
||||||
|
class LockMethodMigrationTest {
|
||||||
|
|
||||||
|
@get:Rule val temp = TemporaryFolder()
|
||||||
|
|
||||||
|
/** See LockSettingsViewModelTest: AndroidKeyStore does not exist off-device. */
|
||||||
|
private class InMemoryMacProvider : MacProvider {
|
||||||
|
private var key: ByteArray? = null
|
||||||
|
override fun hasKey() = key != null
|
||||||
|
override fun ensureKey() { if (key == null) key = ByteArray(32) { it.toByte() } }
|
||||||
|
override fun mac(data: ByteArray): ByteArray {
|
||||||
|
val k = key ?: error("the verifier key is absent")
|
||||||
|
return Mac.getInstance("HmacSHA256").apply { init(SecretKeySpec(k, "HmacSHA256")) }.doFinal(data)
|
||||||
|
}
|
||||||
|
override fun deleteKey() { key = null }
|
||||||
|
}
|
||||||
|
|
||||||
|
private lateinit var lock: AppLockRepository
|
||||||
|
private lateinit var preferences: UserPreferencesRepository
|
||||||
|
private lateinit var migration: LockMethodMigration
|
||||||
|
|
||||||
|
@Before fun setUp() {
|
||||||
|
val scope = CoroutineScope(Dispatchers.IO)
|
||||||
|
// Unique names: several tests re-run this to walk every legacy shape,
|
||||||
|
// and TemporaryFolder refuses to hand out the same file twice.
|
||||||
|
val id = System.nanoTime()
|
||||||
|
lock = AppLockRepository(
|
||||||
|
PreferenceDataStoreFactory.create(scope = scope) { temp.newFile("app_lock_$id.preferences_pb") },
|
||||||
|
InMemoryMacProvider(),
|
||||||
|
)
|
||||||
|
preferences = UserPreferencesRepository(
|
||||||
|
PreferenceDataStoreFactory.create(scope = scope) { temp.newFile("prefs_$id.preferences_pb") },
|
||||||
|
)
|
||||||
|
migration = LockMethodMigration(lock, preferences)
|
||||||
|
}
|
||||||
|
|
||||||
|
/** What an install from before the method key looked like. */
|
||||||
|
private fun legacyInstall(hasPin: Boolean, fingerprintFlag: Boolean) = runBlocking {
|
||||||
|
if (hasPin) lock.setPin("2468".toCharArray())
|
||||||
|
preferences.setBiometricLockEnabled(fingerprintFlag)
|
||||||
|
// The method key is what the old build never wrote.
|
||||||
|
if (hasPin) lock.clearStoredMethodForTest()
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test fun `a PIN with the fingerprint shortcut on becomes either`() = runBlocking {
|
||||||
|
legacyInstall(hasPin = true, fingerprintFlag = true)
|
||||||
|
|
||||||
|
migration.run()
|
||||||
|
|
||||||
|
assertEquals(LockMethod.PIN_AND_BIOMETRIC, lock.method.first())
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test fun `a PIN without the shortcut becomes a PIN`() = runBlocking {
|
||||||
|
legacyInstall(hasPin = true, fingerprintFlag = false)
|
||||||
|
|
||||||
|
migration.run()
|
||||||
|
|
||||||
|
assertEquals(LockMethod.PIN, lock.method.first())
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test fun `no PIN becomes off, whatever the old flag said`() = runBlocking {
|
||||||
|
listOf(true, false).forEach { flag ->
|
||||||
|
setUp()
|
||||||
|
legacyInstall(hasPin = false, fingerprintFlag = flag)
|
||||||
|
|
||||||
|
migration.run()
|
||||||
|
|
||||||
|
assertEquals(LockMethod.NONE, lock.method.first())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test fun `it never produces a lock with no PIN behind it`() = runBlocking {
|
||||||
|
// All four legacy combinations. A sensor as the only way in is a
|
||||||
|
// decision the user has to make deliberately, never one an update makes
|
||||||
|
// for her.
|
||||||
|
listOf(true to true, true to false, false to true, false to false).forEach { (hasPin, flag) ->
|
||||||
|
setUp()
|
||||||
|
legacyInstall(hasPin = hasPin, fingerprintFlag = flag)
|
||||||
|
|
||||||
|
migration.run()
|
||||||
|
|
||||||
|
val method = lock.method.first()
|
||||||
|
assertTrue(
|
||||||
|
"a legacy install (pin=$hasPin, flag=$flag) migrated to $method",
|
||||||
|
method == LockMethod.NONE || method.requiresPin,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test fun `running it twice changes nothing`() = runBlocking {
|
||||||
|
legacyInstall(hasPin = true, fingerprintFlag = true)
|
||||||
|
|
||||||
|
migration.run()
|
||||||
|
migration.run()
|
||||||
|
|
||||||
|
assertEquals(LockMethod.PIN_AND_BIOMETRIC, lock.method.first())
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test fun `it leaves a method the user has already chosen alone`() = runBlocking {
|
||||||
|
// She turned the fingerprint shortcut off in a newer build; the stale
|
||||||
|
// flag must not turn it back on.
|
||||||
|
runBlocking { lock.setPin("2468".toCharArray(), LockMethod.PIN) }
|
||||||
|
preferences.setBiometricLockEnabled(true)
|
||||||
|
|
||||||
|
migration.run()
|
||||||
|
|
||||||
|
assertEquals(LockMethod.PIN, lock.method.first())
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test fun `the retired flag is cleared once it has been read`() = runBlocking {
|
||||||
|
legacyInstall(hasPin = true, fingerprintFlag = true)
|
||||||
|
|
||||||
|
migration.run()
|
||||||
|
|
||||||
|
assertNull(preferences.legacyBiometricLockEnabled.first())
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test fun `an unreadable preferences file still leaves a PIN path`() = runBlocking {
|
||||||
|
legacyInstall(hasPin = true, fingerprintFlag = true)
|
||||||
|
// A flag that cannot be read is a flag that was not set: the worst it
|
||||||
|
// costs is a shortcut she turns back on.
|
||||||
|
val broken = UserPreferencesRepository(
|
||||||
|
PreferenceDataStoreFactory.create(scope = CoroutineScope(Dispatchers.IO)) {
|
||||||
|
temp.newFile("broken_${System.nanoTime()}.preferences_pb")
|
||||||
|
.apply { writeBytes(byteArrayOf(9, 9, 9)) }
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
LockMethodMigration(lock, broken).run()
|
||||||
|
|
||||||
|
assertTrue(lock.method.first().requiresPin)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test fun `a fresh install is simply off`() = runBlocking {
|
||||||
|
migration.run()
|
||||||
|
|
||||||
|
assertEquals(LockMethod.NONE, lock.method.first())
|
||||||
|
assertFalse(lock.hasPin.first())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -52,6 +52,20 @@ class UserPreferencesRepository(
|
||||||
suspend fun setFertileWindowReminderEnabled(value: Boolean) = edit { it[Keys.FertileReminder] = value }
|
suspend fun setFertileWindowReminderEnabled(value: Boolean) = edit { it[Keys.FertileReminder] = value }
|
||||||
suspend fun setOvulationReminderEnabled(value: Boolean) = edit { it[Keys.OvulationReminder] = value }
|
suspend fun setOvulationReminderEnabled(value: Boolean) = edit { it[Keys.OvulationReminder] = value }
|
||||||
suspend fun setBiometricLockEnabled(value: Boolean) = edit { it[Keys.BiometricLock] = value }
|
suspend fun setBiometricLockEnabled(value: Boolean) = edit { it[Keys.BiometricLock] = value }
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The retired fingerprint flag, read once so the lock can inherit it.
|
||||||
|
*
|
||||||
|
* It used to live here, in the store `resetToDefaults()` clears — which is
|
||||||
|
* exactly why the lock's method moved to the lock's own file. This is the
|
||||||
|
* one remaining reader: [LockMethodMigration] takes it, records the
|
||||||
|
* equivalent method, and clears it. Null when it was never set.
|
||||||
|
*/
|
||||||
|
val legacyBiometricLockEnabled: Flow<Boolean?> = dataStore.data
|
||||||
|
.catch { cause -> if (cause is IOException) emit(EMPTY) else throw cause }
|
||||||
|
.map { it[Keys.BiometricLock] }
|
||||||
|
|
||||||
|
suspend fun clearLegacyBiometricLock() = edit { it.remove(Keys.BiometricLock) }
|
||||||
suspend fun setTheme(value: AppTheme) = edit { it[Keys.Theme] = value.name }
|
suspend fun setTheme(value: AppTheme) = edit { it[Keys.Theme] = value.name }
|
||||||
suspend fun setOnboardingCompleted(value: Boolean) = edit { it[Keys.OnboardingCompleted] = value }
|
suspend fun setOnboardingCompleted(value: Boolean) = edit { it[Keys.OnboardingCompleted] = value }
|
||||||
suspend fun setIncognitoLauncherEnabled(value: Boolean) = edit { it[Keys.IncognitoLauncher] = value }
|
suspend fun setIncognitoLauncherEnabled(value: Boolean) = edit { it[Keys.IncognitoLauncher] = value }
|
||||||
|
|
|
||||||
|
|
@ -114,9 +114,38 @@ class AppLockRepository internal constructor(
|
||||||
* does not encrypt anyway. The alternative is an app that cannot be opened
|
* does not encrypt anyway. The alternative is an app that cannot be opened
|
||||||
* *or* erased, because the way out is behind the lock that is broken.
|
* *or* erased, because the way out is behind the lock that is broken.
|
||||||
*/
|
*/
|
||||||
val hasPin: Flow<Boolean> = store.data
|
val method: Flow<LockMethod> = store.data
|
||||||
.catch { failure -> if (failure is IOException) emit(emptyPreferences()) else throw failure }
|
.catch { failure -> if (failure is IOException) emit(emptyPreferences()) else throw failure }
|
||||||
.map { it[Keys.Verifier] != null }
|
.map(::resolve)
|
||||||
|
|
||||||
|
val hasPin: Flow<Boolean> = method.map { it.requiresPin }
|
||||||
|
|
||||||
|
/**
|
||||||
|
* What the stored state actually means, which is not always what it says.
|
||||||
|
*
|
||||||
|
* Three rules, and each closes a way somebody could end up locked out or
|
||||||
|
* wrongly let in:
|
||||||
|
*
|
||||||
|
* - **No method recorded, but a verifier present** reads as [LockMethod.PIN].
|
||||||
|
* That is every install from before this key existed, and it means the
|
||||||
|
* gate is shut from the first frame rather than waiting for a migration
|
||||||
|
* to run.
|
||||||
|
* - **A method that needs a PIN, with no verifier**, reads as
|
||||||
|
* [LockMethod.NONE]. Same policy `VerifierRecord.decode` already applies
|
||||||
|
* to a corrupt record: a lock nobody can open is worse than no lock,
|
||||||
|
* because the way out is behind it.
|
||||||
|
* - **An unrecognised value** falls back to the verifier. A newer build's
|
||||||
|
* method name should not brick an older one.
|
||||||
|
*/
|
||||||
|
private fun resolve(prefs: Preferences): LockMethod {
|
||||||
|
val hasVerifier = VerifierRecord.decode(prefs[Keys.Verifier]) != null
|
||||||
|
val stored = LockMethod.fromStored(prefs[Keys.Method])
|
||||||
|
return when {
|
||||||
|
stored == null -> if (hasVerifier) LockMethod.PIN else LockMethod.NONE
|
||||||
|
stored.requiresPin && !hasVerifier -> LockMethod.NONE
|
||||||
|
else -> stored
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
suspend fun hasPinNow(): Boolean = hasPin.first()
|
suspend fun hasPinNow(): Boolean = hasPin.first()
|
||||||
|
|
||||||
|
|
@ -127,16 +156,92 @@ class AppLockRepository internal constructor(
|
||||||
* is the one case where the honest thing is to leave the lock off rather
|
* is the one case where the honest thing is to leave the lock off rather
|
||||||
* than enable a lock that cannot be opened.
|
* than enable a lock that cannot be opened.
|
||||||
*/
|
*/
|
||||||
suspend fun setPin(pin: CharArray): Boolean {
|
suspend fun setPin(pin: CharArray, method: LockMethod = LockMethod.PIN): Boolean {
|
||||||
|
require(method.requiresPin) { "a PIN cannot be enrolled for a method that has none" }
|
||||||
val record = runCatching { verifier.enroll(pin) }.getOrNull() ?: return false
|
val record = runCatching { verifier.enroll(pin) }.getOrNull() ?: return false
|
||||||
|
// One edit: a PIN written without its method, or the other way round, is
|
||||||
|
// a moment where the resolved state is a lie.
|
||||||
store.edit {
|
store.edit {
|
||||||
it[Keys.Verifier] = record.encode()
|
it[Keys.Verifier] = record.encode()
|
||||||
|
it[Keys.Method] = method.name
|
||||||
it.remove(Keys.Lockout)
|
it.remove(Keys.Lockout)
|
||||||
it.remove(Keys.LockoutMac)
|
it.remove(Keys.LockoutMac)
|
||||||
}
|
}
|
||||||
return true
|
return true
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Move between the two methods that keep a PIN.
|
||||||
|
*
|
||||||
|
* Refuses inside the transaction if the verifier is missing, so a method
|
||||||
|
* needing a PIN can never be recorded without one.
|
||||||
|
*/
|
||||||
|
suspend fun setMethod(method: LockMethod): Boolean {
|
||||||
|
require(method.requiresPin) { "use setBiometricOnly or clearLock for methods without a PIN" }
|
||||||
|
var ok = false
|
||||||
|
store.edit {
|
||||||
|
if (it[Keys.Verifier] != null) {
|
||||||
|
it[Keys.Method] = method.name
|
||||||
|
ok = true
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return ok
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Fingerprint or face, with no PIN behind it.
|
||||||
|
*
|
||||||
|
* The record goes first and the key second: an orphaned key is harmless,
|
||||||
|
* while a verifier left behind a method that does not use it is a stale
|
||||||
|
* secret sitting in the file.
|
||||||
|
*/
|
||||||
|
suspend fun setBiometricOnly() {
|
||||||
|
store.edit {
|
||||||
|
it[Keys.Method] = LockMethod.BIOMETRIC.name
|
||||||
|
it.remove(Keys.Verifier)
|
||||||
|
it.remove(Keys.Lockout)
|
||||||
|
it.remove(Keys.LockoutMac)
|
||||||
|
}
|
||||||
|
macs.deleteKey()
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Remove the recorded method, leaving the verifier — what an install from
|
||||||
|
* before this key existed looks like.
|
||||||
|
*
|
||||||
|
* Only the migration's test has any use for it, and it lives here rather
|
||||||
|
* than reaching into `Keys` from another module because that would make the
|
||||||
|
* key names part of a public surface.
|
||||||
|
*/
|
||||||
|
suspend fun clearStoredMethodForTest() {
|
||||||
|
store.edit { it.remove(Keys.Method) }
|
||||||
|
}
|
||||||
|
|
||||||
|
/** True once a method has been recorded, which is what the migration asks. */
|
||||||
|
suspend fun hasStoredMethod(): Boolean = store.data.first()[Keys.Method] != null
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Record the method an older install was already using.
|
||||||
|
*
|
||||||
|
* Refuses [LockMethod.BIOMETRIC] at the API, and writes only when nothing is
|
||||||
|
* recorded and the verifier agrees — both checks inside the transaction, so
|
||||||
|
* a migration racing an erase cannot resurrect a lock the user just removed.
|
||||||
|
*
|
||||||
|
* The refusal is the important half: every method this can produce has a PIN
|
||||||
|
* path, so no migration can leave somebody facing a sensor as their only way
|
||||||
|
* in.
|
||||||
|
*/
|
||||||
|
suspend fun recordMigratedMethod(method: LockMethod) {
|
||||||
|
require(method != LockMethod.BIOMETRIC) {
|
||||||
|
"a migration may never produce a method with no PIN behind it"
|
||||||
|
}
|
||||||
|
store.edit {
|
||||||
|
val absent = it[Keys.Method] == null
|
||||||
|
val agrees = (it[Keys.Verifier] != null) == method.requiresPin
|
||||||
|
if (absent && agrees) it[Keys.Method] = method.name
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Offer a PIN.
|
* Offer a PIN.
|
||||||
*
|
*
|
||||||
|
|
@ -145,6 +250,9 @@ class AppLockRepository internal constructor(
|
||||||
*/
|
*/
|
||||||
suspend fun check(pin: CharArray): UnlockResult {
|
suspend fun check(pin: CharArray): UnlockResult {
|
||||||
val prefs = store.data.first()
|
val prefs = store.data.first()
|
||||||
|
// A PIN is never checked for a method that does not use one, even if a
|
||||||
|
// stale record is sitting in the file.
|
||||||
|
if (!resolve(prefs).requiresPin) return UnlockResult.NoPin
|
||||||
val record = VerifierRecord.decode(prefs[Keys.Verifier]) ?: return UnlockResult.NoPin
|
val record = VerifierRecord.decode(prefs[Keys.Verifier]) ?: return UnlockResult.NoPin
|
||||||
|
|
||||||
val state = readLockout(prefs)
|
val state = readLockout(prefs)
|
||||||
|
|
@ -191,6 +299,10 @@ class AppLockRepository internal constructor(
|
||||||
it.remove(Keys.Verifier)
|
it.remove(Keys.Verifier)
|
||||||
it.remove(Keys.Lockout)
|
it.remove(Keys.Lockout)
|
||||||
it.remove(Keys.LockoutMac)
|
it.remove(Keys.LockoutMac)
|
||||||
|
// Written rather than removed: NONE says "this install has decided",
|
||||||
|
// and an absent key means "never asked", which resolve() reads as a
|
||||||
|
// pre-migration install and would answer from the verifier.
|
||||||
|
it[Keys.Method] = LockMethod.NONE.name
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -264,8 +376,26 @@ class AppLockRepository internal constructor(
|
||||||
private fun String.decodeB64(): ByteArray =
|
private fun String.decodeB64(): ByteArray =
|
||||||
runCatching { Base64.getDecoder().decode(this) }.getOrDefault(ByteArray(0))
|
runCatching { Base64.getDecoder().decode(this) }.getOrDefault(ByteArray(0))
|
||||||
|
|
||||||
private object Keys {
|
/**
|
||||||
|
* Internal rather than private so this module's own tests can write the
|
||||||
|
* states a partial write or a tampered file produces — a method with no
|
||||||
|
* verifier, a name from a newer build — which is exactly what `resolve` is
|
||||||
|
* there to survive. Nothing outside `core/security` can see it.
|
||||||
|
*/
|
||||||
|
internal object Keys {
|
||||||
val Verifier = stringPreferencesKey("pin_verifier_v1")
|
val Verifier = stringPreferencesKey("pin_verifier_v1")
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Which of PIN / fingerprint / either opens the app.
|
||||||
|
*
|
||||||
|
* In *this* store, not `UserPreferences`, and that placement is the
|
||||||
|
* whole point: `resetToDefaults()` there is `edit { clear() }`, so a
|
||||||
|
* fingerprint-only lock recorded beside the theme would be one "reset
|
||||||
|
* my settings" away from silently vanishing. Not MAC'd, for the same
|
||||||
|
* reason the lockout counter's KDoc gives — whoever can rewrite this
|
||||||
|
* file has the app's private storage and reads the database anyway.
|
||||||
|
*/
|
||||||
|
val Method = stringPreferencesKey("lock_method_v1")
|
||||||
val Lockout = stringPreferencesKey("lockout_state_v1")
|
val Lockout = stringPreferencesKey("lockout_state_v1")
|
||||||
val LockoutMac = stringPreferencesKey("lockout_state_mac_v1")
|
val LockoutMac = stringPreferencesKey("lockout_state_mac_v1")
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,44 @@
|
||||||
|
package dev.privacyllc.period.core.security
|
||||||
|
|
||||||
|
/**
|
||||||
|
* How the app asks to be let in.
|
||||||
|
*
|
||||||
|
* ## Why an enum rather than two booleans
|
||||||
|
*
|
||||||
|
* "Has a PIN" and "allows a fingerprint" would express the same four states and
|
||||||
|
* make two of them unnameable in conversation. The settings screen offers a
|
||||||
|
* choice of four, the copy explains four, and the migration has to guarantee it
|
||||||
|
* never produces one of them — all of which read better as a name than as a
|
||||||
|
* pair of flags, and the compiler can then check a `when` covers every case.
|
||||||
|
*
|
||||||
|
* ## Why [BIOMETRIC] is a state and not a flag on [PIN]
|
||||||
|
*
|
||||||
|
* Because it is the one with no fallback. Everything that has to be careful —
|
||||||
|
* the migration, the settings transitions, what the lock screen shows, what
|
||||||
|
* happens when the sensor stops working — is careful specifically about the
|
||||||
|
* absence of a PIN, and a boolean pair spreads that condition across two fields
|
||||||
|
* that nothing stops disagreeing.
|
||||||
|
*/
|
||||||
|
enum class LockMethod {
|
||||||
|
/** The app opens like any other. */
|
||||||
|
NONE,
|
||||||
|
|
||||||
|
/** A PIN chosen in this app, and only this app. */
|
||||||
|
PIN,
|
||||||
|
|
||||||
|
/** A fingerprint or face already set up on the phone. No PIN behind it. */
|
||||||
|
BIOMETRIC,
|
||||||
|
|
||||||
|
/** Either opens it. */
|
||||||
|
PIN_AND_BIOMETRIC,
|
||||||
|
;
|
||||||
|
|
||||||
|
val isOn: Boolean get() = this != NONE
|
||||||
|
val requiresPin: Boolean get() = this == PIN || this == PIN_AND_BIOMETRIC
|
||||||
|
val allowsBiometric: Boolean get() = this == BIOMETRIC || this == PIN_AND_BIOMETRIC
|
||||||
|
|
||||||
|
companion object {
|
||||||
|
/** Null for anything unrecognised — a value written by a newer build, or nonsense. */
|
||||||
|
fun fromStored(name: String?): LockMethod? = entries.firstOrNull { it.name == name }
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -12,6 +12,8 @@ import kotlinx.coroutines.test.StandardTestDispatcher
|
||||||
import kotlinx.coroutines.test.TestScope
|
import kotlinx.coroutines.test.TestScope
|
||||||
import kotlinx.coroutines.test.runTest
|
import kotlinx.coroutines.test.runTest
|
||||||
import org.junit.After
|
import org.junit.After
|
||||||
|
import kotlinx.coroutines.runBlocking
|
||||||
|
import org.junit.Assert.assertThrows
|
||||||
import org.junit.Assert.assertEquals
|
import org.junit.Assert.assertEquals
|
||||||
import org.junit.Assert.assertFalse
|
import org.junit.Assert.assertFalse
|
||||||
import org.junit.Assert.assertTrue
|
import org.junit.Assert.assertTrue
|
||||||
|
|
@ -54,6 +56,110 @@ class AppLockRepositoryTest {
|
||||||
|
|
||||||
@After fun tearDown() = scope.cancel()
|
@After fun tearDown() = scope.cancel()
|
||||||
|
|
||||||
|
// -----------------------------------------------------------------------
|
||||||
|
// Which method opens the app
|
||||||
|
// -----------------------------------------------------------------------
|
||||||
|
|
||||||
|
@Test fun `a fresh install has no method and is not on`() = scope.runTest {
|
||||||
|
assertEquals(LockMethod.NONE, repo.method.first())
|
||||||
|
assertFalse(repo.hasStoredMethod())
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test fun `a verifier with no recorded method reads as a PIN`() = scope.runTest {
|
||||||
|
// Every install from before the method key existed. The gate has to be
|
||||||
|
// shut from the first frame, not from whenever a migration runs.
|
||||||
|
repo.setPin(cheapPin)
|
||||||
|
store.edit { it.remove(AppLockRepository.Keys.Method) }
|
||||||
|
|
||||||
|
assertEquals(LockMethod.PIN, repo.method.first())
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test fun `setting a PIN records its method in the same write`() = scope.runTest {
|
||||||
|
repo.setPin(cheapPin, LockMethod.PIN_AND_BIOMETRIC)
|
||||||
|
|
||||||
|
assertEquals(LockMethod.PIN_AND_BIOMETRIC, repo.method.first())
|
||||||
|
assertTrue(repo.hasPin.first())
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test fun `fingerprint-only keeps no verifier and no key`() = scope.runTest {
|
||||||
|
repo.setPin(cheapPin)
|
||||||
|
repo.setBiometricOnly()
|
||||||
|
|
||||||
|
assertEquals(LockMethod.BIOMETRIC, repo.method.first())
|
||||||
|
assertFalse(repo.hasPin.first())
|
||||||
|
assertFalse("the signing key outlived the record it signed", macs.hasKey())
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test fun `a PIN is never accepted in fingerprint-only mode`() = scope.runTest {
|
||||||
|
repo.setPin(cheapPin)
|
||||||
|
// Leave the record behind, as a partial write or a tampered file would.
|
||||||
|
store.edit { it[AppLockRepository.Keys.Method] = LockMethod.BIOMETRIC.name }
|
||||||
|
|
||||||
|
assertTrue(repo.check(cheapPin) is UnlockResult.NoPin)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test fun `a method that needs a PIN without one reads as off`() = scope.runTest {
|
||||||
|
store.edit { it[AppLockRepository.Keys.Method] = LockMethod.PIN.name }
|
||||||
|
|
||||||
|
// A lock nobody can open is worse than no lock: the way out is behind it.
|
||||||
|
assertEquals(LockMethod.NONE, repo.method.first())
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test fun `a method name this build does not know falls back to the verifier`() = scope.runTest {
|
||||||
|
repo.setPin(cheapPin)
|
||||||
|
store.edit { it[AppLockRepository.Keys.Method] = "SOMETHING_NEWER" }
|
||||||
|
|
||||||
|
assertEquals(LockMethod.PIN, repo.method.first())
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test fun `turning the lock off records that decision`() = scope.runTest {
|
||||||
|
repo.setPin(cheapPin)
|
||||||
|
repo.clearLock()
|
||||||
|
|
||||||
|
assertEquals(LockMethod.NONE, repo.method.first())
|
||||||
|
// Recorded, not absent: absent means "never asked", which would be read
|
||||||
|
// as a pre-migration install.
|
||||||
|
assertTrue(repo.hasStoredMethod())
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test fun `switching between the two PIN methods keeps the PIN`() = scope.runTest {
|
||||||
|
repo.setPin(cheapPin)
|
||||||
|
|
||||||
|
assertTrue(repo.setMethod(LockMethod.PIN_AND_BIOMETRIC))
|
||||||
|
assertEquals(LockMethod.PIN_AND_BIOMETRIC, repo.method.first())
|
||||||
|
assertTrue(repo.check(cheapPin) is UnlockResult.Unlocked)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test fun `a PIN method cannot be recorded without a PIN`() = scope.runTest {
|
||||||
|
assertFalse(repo.setMethod(LockMethod.PIN))
|
||||||
|
assertEquals(LockMethod.NONE, repo.method.first())
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test fun `the migration refuses to produce a method with no PIN behind it`() = scope.runTest {
|
||||||
|
// The invariant that keeps an update from leaving somebody facing a
|
||||||
|
// sensor as their only way in.
|
||||||
|
assertThrows(IllegalArgumentException::class.java) {
|
||||||
|
runBlocking { repo.recordMigratedMethod(LockMethod.BIOMETRIC) }
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test fun `the migration never overwrites a method the user chose`() = scope.runTest {
|
||||||
|
repo.setPin(cheapPin, LockMethod.PIN_AND_BIOMETRIC)
|
||||||
|
|
||||||
|
repo.recordMigratedMethod(LockMethod.PIN)
|
||||||
|
|
||||||
|
assertEquals(LockMethod.PIN_AND_BIOMETRIC, repo.method.first())
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test fun `the migration writes nothing when the verifier disagrees`() = scope.runTest {
|
||||||
|
// An erase landing between the read and the write: the method says a PIN
|
||||||
|
// and there is none, so recording it would resurrect a lock she removed.
|
||||||
|
repo.recordMigratedMethod(LockMethod.PIN)
|
||||||
|
|
||||||
|
assertFalse(repo.hasStoredMethod())
|
||||||
|
assertEquals(LockMethod.NONE, repo.method.first())
|
||||||
|
}
|
||||||
|
|
||||||
// -----------------------------------------------------------------------
|
// -----------------------------------------------------------------------
|
||||||
// Failing safely
|
// Failing safely
|
||||||
// -----------------------------------------------------------------------
|
// -----------------------------------------------------------------------
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue