PM-19873: Migrate the screen capture feature from user-scoped to app-scoped (#4972)

This commit is contained in:
David Perez
2025-04-03 17:02:53 -05:00
committed by GitHub
parent 853069ee1c
commit 4df1b245e8
6 changed files with 75 additions and 132 deletions

View File

@@ -24,6 +24,16 @@ interface SettingsDiskSource {
*/
val appLanguageFlow: Flow<AppLanguage?>
/**
* The currently persisted setting for whether screen capture is allowed.
*/
var screenCaptureAllowed: Boolean?
/**
* Emits updates that track [screenCaptureAllowed].
*/
val screenCaptureAllowedFlow: Flow<Boolean?>
/**
* Has the initial autofill dialog been shown to the user.
*/
@@ -237,21 +247,6 @@ interface SettingsDiskSource {
blockedAutofillUris: List<String>?,
)
/**
* Gets whether or not the given [userId] has enabled screen capture.
*/
fun getScreenCaptureAllowed(userId: String): Boolean?
/**
* Emits updates that track [getScreenCaptureAllowed] for the given [userId].
*/
fun getScreenCaptureAllowedFlow(userId: String): Flow<Boolean?>
/**
* Stores whether or not [isScreenCaptureAllowed] for the given [userId].
*/
fun storeScreenCaptureAllowed(userId: String, isScreenCaptureAllowed: Boolean?)
/**
* Records a user sign in for the given [userId]. This data is expected to remain on
* disk until storage is cleared or the app is uninstalled.

View File

@@ -1,6 +1,7 @@
package com.x8bit.bitwarden.data.platform.datasource.disk
import android.content.SharedPreferences
import androidx.core.content.edit
import com.bitwarden.core.data.repository.util.bufferedMutableSharedFlow
import com.bitwarden.core.data.util.decodeFromStringOrNull
import com.bitwarden.data.datasource.disk.BaseDiskSource
@@ -50,7 +51,7 @@ private const val RESUME_SCREEN = "resumeScreen"
*/
@Suppress("TooManyFunctions")
class SettingsDiskSourceImpl(
val sharedPreferences: SharedPreferences,
private val sharedPreferences: SharedPreferences,
private val json: Json,
) : BaseDiskSource(sharedPreferences = sharedPreferences),
SettingsDiskSource {
@@ -87,12 +88,15 @@ class SettingsDiskSourceImpl(
private val mutableHasSeenGeneratorCoachMarkFlow = bufferedMutableSharedFlow<Boolean?>()
private val mutableScreenCaptureAllowedFlowMap =
mutableMapOf<String, MutableSharedFlow<Boolean?>>()
private val mutableScreenCaptureAllowedFlow = MutableSharedFlow<Boolean?>()
private val mutableVaultRegisteredForExportFlow =
mutableMapOf<String, MutableSharedFlow<Boolean?>>()
init {
migrateScreenCaptureSetting()
}
override var appLanguage: AppLanguage?
get() = getString(key = APP_LANGUAGE_KEY)
?.let { storedValue ->
@@ -109,6 +113,16 @@ class SettingsDiskSourceImpl(
override val appLanguageFlow: Flow<AppLanguage?>
get() = mutableAppLanguageFlow.onSubscription { emit(appLanguage) }
override var screenCaptureAllowed: Boolean?
get() = getBoolean(key = SCREEN_CAPTURE_ALLOW_KEY)
set(value) {
putBoolean(key = SCREEN_CAPTURE_ALLOW_KEY, value = value)
mutableScreenCaptureAllowedFlow.tryEmit(value)
}
override val screenCaptureAllowedFlow: Flow<Boolean?>
get() = mutableScreenCaptureAllowedFlow.onSubscription { emit(screenCaptureAllowed) }
override var initialAutofillDialogShown: Boolean?
get() = getBoolean(key = INITIAL_AUTOFILL_DIALOG_SHOWN)
set(value) {
@@ -370,25 +384,6 @@ class SettingsDiskSourceImpl(
)
}
override fun getScreenCaptureAllowed(userId: String): Boolean? {
return getBoolean(key = SCREEN_CAPTURE_ALLOW_KEY.appendIdentifier(userId))
}
override fun getScreenCaptureAllowedFlow(userId: String): Flow<Boolean?> =
getMutableScreenCaptureAllowedFlow(userId)
.onSubscription { emit(getScreenCaptureAllowed(userId)) }
override fun storeScreenCaptureAllowed(
userId: String,
isScreenCaptureAllowed: Boolean?,
) {
putBoolean(
key = SCREEN_CAPTURE_ALLOW_KEY.appendIdentifier(userId),
value = isScreenCaptureAllowed,
)
getMutableScreenCaptureAllowedFlow(userId).tryEmit(isScreenCaptureAllowed)
}
override fun storeUseHasLoggedInPreviously(userId: String) {
putBoolean(
key = HAS_USER_LOGGED_IN_OR_CREATED_AN_ACCOUNT_KEY.appendIdentifier(userId),
@@ -568,11 +563,6 @@ class SettingsDiskSourceImpl(
bufferedMutableSharedFlow(replay = 1)
}
private fun getMutableScreenCaptureAllowedFlow(userId: String): MutableSharedFlow<Boolean?> =
mutableScreenCaptureAllowedFlowMap.getOrPut(userId) {
bufferedMutableSharedFlow(replay = 1)
}
private fun getMutableShowAutoFillSettingBadgeFlow(
userId: String,
): MutableSharedFlow<Boolean?> = mutableShowAutoFillSettingBadgeFlowMap.getOrPut(userId) {
@@ -596,4 +586,20 @@ class SettingsDiskSourceImpl(
): MutableSharedFlow<Boolean?> = mutableVaultRegisteredForExportFlow.getOrPut(userId) {
bufferedMutableSharedFlow(replay = 1)
}
/**
* Migrates the user-scoped screen capture state to an app-wide state.
*
* In the case that multiple users have different values stored for this feature, we will
* default to _not_ allowing screen capture.
*/
private fun migrateScreenCaptureSetting() {
val screenCaptureSettings = sharedPreferences.all.filter {
// The underscore is important to not grab the new app-scoped key
it.key.contains(other = "${SCREEN_CAPTURE_ALLOW_KEY}_")
}
if (screenCaptureSettings.isEmpty()) return
screenCaptureAllowed = screenCaptureSettings.all { it.value == true }
screenCaptureSettings.forEach { sharedPreferences.edit(commit = true) { remove(it.key) } }
}
}

View File

@@ -24,12 +24,10 @@ import com.x8bit.bitwarden.data.vault.datasource.sdk.VaultSdkSource
import com.x8bit.bitwarden.ui.platform.feature.settings.appearance.model.AppLanguage
import com.x8bit.bitwarden.ui.platform.feature.settings.appearance.model.AppTheme
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.flatMapLatest
import kotlinx.coroutines.flow.flowOf
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.map
@@ -333,36 +331,19 @@ class SettingsRepositoryImpl(
autofillEnabledManager.isAutofillEnabledStateFlow
override var isScreenCaptureAllowed: Boolean
get() = activeUserId
?.let { settingsDiskSource.getScreenCaptureAllowed(it) }
?: DEFAULT_IS_SCREEN_CAPTURE_ALLOWED
get() = settingsDiskSource.screenCaptureAllowed ?: DEFAULT_IS_SCREEN_CAPTURE_ALLOWED
set(value) {
val userId = activeUserId ?: return
settingsDiskSource.storeScreenCaptureAllowed(
userId = userId,
isScreenCaptureAllowed = value,
)
settingsDiskSource.screenCaptureAllowed = value
}
@OptIn(ExperimentalCoroutinesApi::class)
override val isScreenCaptureAllowedStateFlow: StateFlow<Boolean>
get() = authDiskSource
.userStateFlow
.flatMapLatest { userState ->
userState
?.activeUserId
?.let {
settingsDiskSource.getScreenCaptureAllowedFlow(userId = it)
.map { isAllowed -> isAllowed ?: DEFAULT_IS_SCREEN_CAPTURE_ALLOWED }
}
?: flowOf(DEFAULT_IS_SCREEN_CAPTURE_ALLOWED)
}
get() = settingsDiskSource
.screenCaptureAllowedFlow
.map { isAllowed -> isAllowed ?: DEFAULT_IS_SCREEN_CAPTURE_ALLOWED }
.stateIn(
scope = unconfinedScope,
started = SharingStarted.Lazily,
initialValue = activeUserId
?.let { settingsDiskSource.getScreenCaptureAllowed(userId = it) }
?: DEFAULT_IS_SCREEN_CAPTURE_ALLOWED,
initialValue = isScreenCaptureAllowed,
)
init {

View File

@@ -148,10 +148,6 @@ class SettingsDiskSourceTest {
userId = userId,
lastSyncTime = Instant.parse("2023-10-27T12:00:00Z"),
)
settingsDiskSource.storeScreenCaptureAllowed(
userId = userId,
isScreenCaptureAllowed = true,
)
settingsDiskSource.storeClearClipboardFrequencySeconds(userId = userId, frequency = 5)
val systemBioIntegrityState = "system_biometrics_integrity_state"
settingsDiskSource.storeAccountBiometricIntegrityValidity(
@@ -166,7 +162,6 @@ class SettingsDiskSourceTest {
settingsDiskSource.clearData(userId = userId)
// We do not clear these even when you call clear storage
assertEquals(true, settingsDiskSource.getScreenCaptureAllowed(userId = userId))
assertTrue(settingsDiskSource.getShowUnlockSettingBadge(userId = userId) ?: false)
assertTrue(settingsDiskSource.getShowAutoFillSettingBadge(userId = userId) ?: false)
@@ -867,50 +862,31 @@ class SettingsDiskSourceTest {
@Test
fun `getScreenCaptureAllowed should pull from SharedPreferences`() {
val screenCaptureAllowBaseKey = "bwPreferencesStorage:screenCaptureAllowed"
val mockUserId = "mockUserId"
val screenCaptureAllowKey = "bwPreferencesStorage:screenCaptureAllowed"
val isScreenCaptureAllowed = true
fakeSharedPreferences.edit {
putBoolean("${screenCaptureAllowBaseKey}_$mockUserId", isScreenCaptureAllowed)
putBoolean(screenCaptureAllowKey, isScreenCaptureAllowed)
}
val actual = settingsDiskSource.getScreenCaptureAllowed(userId = mockUserId)
assertEquals(
isScreenCaptureAllowed,
actual,
)
val actual = settingsDiskSource.screenCaptureAllowed
assertEquals(isScreenCaptureAllowed, actual)
}
@Test
fun `storeScreenCaptureAllowed for non-null values should update SharedPreferences`() {
val screenCaptureAllowBaseKey = "bwPreferencesStorage:screenCaptureAllowed"
val mockUserId = "mockUserId"
val screenCaptureAllowKey = "bwPreferencesStorage:screenCaptureAllowed"
val isScreenCaptureAllowed = true
settingsDiskSource.storeScreenCaptureAllowed(
userId = mockUserId,
isScreenCaptureAllowed = isScreenCaptureAllowed,
)
val actual = fakeSharedPreferences.getBoolean(
"${screenCaptureAllowBaseKey}_$mockUserId",
false,
)
assertEquals(
isScreenCaptureAllowed,
actual,
)
settingsDiskSource.screenCaptureAllowed = isScreenCaptureAllowed
val actual = fakeSharedPreferences.getBoolean(screenCaptureAllowKey, false)
assertEquals(isScreenCaptureAllowed, actual)
}
@Test
fun `storeScreenCaptureAllowed for null values should clear SharedPreferences`() {
val screenCaptureAllowBaseKey = "bwPreferencesStorage:screenCaptureAllowed"
val mockUserId = "mockUserId"
val screenCaptureAllowKey = "${screenCaptureAllowBaseKey}_$mockUserId"
val screenCaptureAllowKey = "bwPreferencesStorage:screenCaptureAllowed"
fakeSharedPreferences.edit {
putBoolean(screenCaptureAllowKey, true)
}
settingsDiskSource.storeScreenCaptureAllowed(
userId = mockUserId,
isScreenCaptureAllowed = null,
)
settingsDiskSource.screenCaptureAllowed = null
assertFalse(fakeSharedPreferences.contains(screenCaptureAllowKey))
}

View File

@@ -23,6 +23,8 @@ class FakeSettingsDiskSource : SettingsDiskSource {
private val mutableAppThemeFlow = bufferedMutableSharedFlow<AppTheme>(replay = 1)
private val mutableScreenCaptureAllowedFlow = bufferedMutableSharedFlow<Boolean?>(replay = 1)
private val mutableLastSyncCallFlowMap = mutableMapOf<String, MutableSharedFlow<Instant?>>()
private val mutableVaultTimeoutActionsFlowMap =
@@ -45,9 +47,6 @@ class FakeSettingsDiskSource : SettingsDiskSource {
private val mutableShouldShowAddLoginCoachMarkFlow = bufferedMutableSharedFlow<Boolean?>()
private val mutableScreenCaptureAllowedFlowMap =
mutableMapOf<String, MutableSharedFlow<Boolean?>>()
private val mutableShouldShowGeneratorCoachMarkFlow =
bufferedMutableSharedFlow<Boolean?>()
@@ -67,7 +66,7 @@ class FakeSettingsDiskSource : SettingsDiskSource {
private var storedIsCrashLoggingEnabled: Boolean? = null
private var storedHasUserLoggedInOrCreatedAccount: Boolean? = null
private var storedInitialAutofillDialogShown: Boolean? = null
private val storedScreenCaptureAllowed = mutableMapOf<String, Boolean?>()
private var storedScreenCaptureAllowed: Boolean? = null
private var storedSystemBiometricIntegritySource: String? = null
private val storedAccountBiometricIntegrityValidity = mutableMapOf<String, Boolean?>()
private val storedAppResumeScreenData = mutableMapOf<String, String?>()
@@ -116,6 +115,16 @@ class FakeSettingsDiskSource : SettingsDiskSource {
emit(appTheme)
}
override var screenCaptureAllowed: Boolean?
get() = storedScreenCaptureAllowed
set(value) {
storedScreenCaptureAllowed = value
mutableScreenCaptureAllowedFlow.tryEmit(value)
}
override val screenCaptureAllowedFlow: Flow<Boolean?>
get() = mutableScreenCaptureAllowedFlow.onSubscription { emit(screenCaptureAllowed) }
override var systemBiometricIntegritySource: String?
get() = storedSystemBiometricIntegritySource
set(value) {
@@ -304,21 +313,6 @@ class FakeSettingsDiskSource : SettingsDiskSource {
storedBlockedAutofillUris[userId] = blockedAutofillUris
}
override fun getScreenCaptureAllowed(userId: String): Boolean? =
storedScreenCaptureAllowed[userId]
override fun getScreenCaptureAllowedFlow(userId: String): Flow<Boolean?> {
return getMutableScreenCaptureAllowedFlow(userId)
}
override fun storeScreenCaptureAllowed(
userId: String,
isScreenCaptureAllowed: Boolean?,
) {
storedScreenCaptureAllowed[userId] = isScreenCaptureAllowed
getMutableScreenCaptureAllowedFlow(userId).tryEmit(isScreenCaptureAllowed)
}
override fun storeUseHasLoggedInPreviously(userId: String) {
userSignIns[userId] = true
}
@@ -437,12 +431,6 @@ class FakeSettingsDiskSource : SettingsDiskSource {
}
//region Private helper functions
private fun getMutableScreenCaptureAllowedFlow(userId: String): MutableSharedFlow<Boolean?> {
return mutableScreenCaptureAllowedFlowMap.getOrPut(userId) {
bufferedMutableSharedFlow(replay = 1)
}
}
private fun getMutableLastSyncTimeFlow(
userId: String,
): MutableSharedFlow<Instant?> =

View File

@@ -1003,22 +1003,19 @@ class SettingsRepositoryTest {
@Test
fun `isScreenCaptureAllowed property should update SettingsDiskSource and emit changes`() =
runTest {
fakeAuthDiskSource.userState = MOCK_USER_STATE
fakeSettingsDiskSource.storeScreenCaptureAllowed(USER_ID, false)
fakeSettingsDiskSource.screenCaptureAllowed = false
settingsRepository.isScreenCaptureAllowedStateFlow.test {
assertFalse(awaitItem())
settingsRepository.isScreenCaptureAllowed = true
assertTrue(awaitItem())
assertEquals(true, fakeSettingsDiskSource.getScreenCaptureAllowed(USER_ID))
assertEquals(true, fakeSettingsDiskSource.screenCaptureAllowed)
settingsRepository.isScreenCaptureAllowed = false
assertFalse(awaitItem())
assertEquals(false, fakeSettingsDiskSource.getScreenCaptureAllowed(USER_ID))
assertEquals(false, fakeSettingsDiskSource.screenCaptureAllowed)
}
}