mirror of
https://github.com/bitwarden/android.git
synced 2025-12-15 16:40:20 -06:00
PM-23692: Remove auth sync feature flag from password manager (#5514)
This commit is contained in:
parent
bc50c0d873
commit
0feac46711
@ -138,13 +138,11 @@ object PlatformManagerModule {
|
||||
addTotpItemFromAuthenticatorManager: AddTotpItemFromAuthenticatorManager,
|
||||
@ApplicationContext context: Context,
|
||||
dispatcherManager: DispatcherManager,
|
||||
featureFlagManager: FeatureFlagManager,
|
||||
): AuthenticatorBridgeProcessor = AuthenticatorBridgeProcessorImpl(
|
||||
authenticatorBridgeRepository = authenticatorBridgeRepository,
|
||||
addTotpItemFromAuthenticatorManager = addTotpItemFromAuthenticatorManager,
|
||||
context = context,
|
||||
dispatcherManager = dispatcherManager,
|
||||
featureFlagManager = featureFlagManager,
|
||||
)
|
||||
|
||||
@Provides
|
||||
|
||||
@ -21,7 +21,6 @@ sealed class FlagKey<out T : Any> {
|
||||
*/
|
||||
val activeFlags: List<FlagKey<*>> by lazy {
|
||||
listOf(
|
||||
AuthenticatorSync,
|
||||
EmailVerification,
|
||||
ImportLoginsFlow,
|
||||
CredentialExchangeProtocolImport,
|
||||
@ -40,14 +39,6 @@ sealed class FlagKey<out T : Any> {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Data object holding the key for syncing with the Bitwarden Authenticator app.
|
||||
*/
|
||||
data object AuthenticatorSync : FlagKey<Boolean>() {
|
||||
override val keyName: String = "enable-pm-bwa-sync"
|
||||
override val defaultValue: Boolean = false
|
||||
}
|
||||
|
||||
/**
|
||||
* Data object holding the key for Email Verification feature.
|
||||
*/
|
||||
|
||||
@ -18,8 +18,6 @@ import com.bitwarden.authenticatorbridge.util.toSymmetricEncryptionKeyData
|
||||
import com.bitwarden.core.util.isBuildVersionAtLeast
|
||||
import com.bitwarden.data.manager.DispatcherManager
|
||||
import com.x8bit.bitwarden.data.auth.manager.AddTotpItemFromAuthenticatorManager
|
||||
import com.x8bit.bitwarden.data.platform.manager.FeatureFlagManager
|
||||
import com.x8bit.bitwarden.data.platform.manager.model.FlagKey
|
||||
import com.x8bit.bitwarden.data.platform.repository.AuthenticatorBridgeRepository
|
||||
import com.x8bit.bitwarden.data.platform.util.createAddTotpItemFromAuthenticatorIntent
|
||||
import com.x8bit.bitwarden.ui.vault.util.getTotpDataOrNull
|
||||
@ -33,7 +31,6 @@ import timber.log.Timber
|
||||
class AuthenticatorBridgeProcessorImpl(
|
||||
private val authenticatorBridgeRepository: AuthenticatorBridgeRepository,
|
||||
private val addTotpItemFromAuthenticatorManager: AddTotpItemFromAuthenticatorManager,
|
||||
private val featureFlagManager: FeatureFlagManager,
|
||||
dispatcherManager: DispatcherManager,
|
||||
context: Context,
|
||||
) : AuthenticatorBridgeProcessor {
|
||||
@ -44,12 +41,9 @@ class AuthenticatorBridgeProcessorImpl(
|
||||
|
||||
override val binder: IAuthenticatorBridgeService.Stub?
|
||||
get() {
|
||||
return if (
|
||||
!featureFlagManager.getFeatureFlag(FlagKey.AuthenticatorSync) ||
|
||||
!isBuildVersionAtLeast(Build.VERSION_CODES.S)
|
||||
) {
|
||||
// If the feature flag is not enabled, OR if version is below Android 12,
|
||||
// return a null binder which will no-op all service calls
|
||||
return if (!isBuildVersionAtLeast(Build.VERSION_CODES.S)) {
|
||||
// If version is below Android 12, return a null binder which will no-op all
|
||||
// service calls
|
||||
null
|
||||
} else {
|
||||
// Otherwise, return real binder implementation:
|
||||
|
||||
@ -25,7 +25,6 @@ fun <T : Any> FlagKey<T>.ListItemContent(
|
||||
Unit
|
||||
}
|
||||
|
||||
FlagKey.AuthenticatorSync,
|
||||
FlagKey.EmailVerification,
|
||||
FlagKey.ImportLoginsFlow,
|
||||
FlagKey.CredentialExchangeProtocolImport,
|
||||
@ -82,7 +81,6 @@ private fun <T : Any> FlagKey<T>.getDisplayLabel(): String = when (this) {
|
||||
FlagKey.DummyString,
|
||||
-> this.keyName
|
||||
|
||||
FlagKey.AuthenticatorSync -> stringResource(R.string.authenticator_sync)
|
||||
FlagKey.EmailVerification -> stringResource(R.string.email_verification)
|
||||
FlagKey.ImportLoginsFlow -> stringResource(R.string.import_logins_flow)
|
||||
FlagKey.CredentialExchangeProtocolImport -> stringResource(R.string.cxp_import)
|
||||
|
||||
@ -17,10 +17,8 @@ import com.x8bit.bitwarden.data.auth.repository.model.PolicyInformation
|
||||
import com.x8bit.bitwarden.data.auth.repository.model.UserFingerprintResult
|
||||
import com.x8bit.bitwarden.data.auth.repository.util.policyInformation
|
||||
import com.x8bit.bitwarden.data.platform.manager.BiometricsEncryptionManager
|
||||
import com.x8bit.bitwarden.data.platform.manager.FeatureFlagManager
|
||||
import com.x8bit.bitwarden.data.platform.manager.FirstTimeActionManager
|
||||
import com.x8bit.bitwarden.data.platform.manager.PolicyManager
|
||||
import com.x8bit.bitwarden.data.platform.manager.model.FlagKey
|
||||
import com.x8bit.bitwarden.data.platform.repository.EnvironmentRepository
|
||||
import com.x8bit.bitwarden.data.platform.repository.SettingsRepository
|
||||
import com.x8bit.bitwarden.data.platform.repository.model.BiometricsKeyResult
|
||||
@ -51,7 +49,6 @@ class AccountSecurityViewModel @Inject constructor(
|
||||
private val settingsRepository: SettingsRepository,
|
||||
private val environmentRepository: EnvironmentRepository,
|
||||
private val biometricsEncryptionManager: BiometricsEncryptionManager,
|
||||
private val featureFlagManager: FeatureFlagManager,
|
||||
private val firstTimeActionManager: FirstTimeActionManager,
|
||||
policyManager: PolicyManager,
|
||||
savedStateHandle: SavedStateHandle,
|
||||
@ -74,9 +71,7 @@ class AccountSecurityViewModel @Inject constructor(
|
||||
?.activeAccount
|
||||
?.hasMasterPassword != false,
|
||||
isUnlockWithPinEnabled = settingsRepository.isUnlockWithPinEnabled,
|
||||
shouldShowEnableAuthenticatorSync =
|
||||
featureFlagManager.getFeatureFlag(FlagKey.AuthenticatorSync) &&
|
||||
isBuildVersionAtLeast(Build.VERSION_CODES.S),
|
||||
shouldShowEnableAuthenticatorSync = isBuildVersionAtLeast(Build.VERSION_CODES.S),
|
||||
userId = userId,
|
||||
vaultTimeout = settingsRepository.vaultTimeout,
|
||||
vaultTimeoutAction = settingsRepository.vaultTimeoutAction,
|
||||
@ -123,13 +118,6 @@ class AccountSecurityViewModel @Inject constructor(
|
||||
.onEach(::sendAction)
|
||||
.launchIn(viewModelScope)
|
||||
|
||||
featureFlagManager
|
||||
.getFeatureFlagFlow(FlagKey.AuthenticatorSync)
|
||||
.onEach {
|
||||
trySendAction(AccountSecurityAction.Internal.AuthenticatorSyncFeatureFlagUpdate(it))
|
||||
}
|
||||
.launchIn(viewModelScope)
|
||||
|
||||
firstTimeActionManager
|
||||
.firstTimeStateFlow
|
||||
.map {
|
||||
@ -374,10 +362,6 @@ class AccountSecurityViewModel @Inject constructor(
|
||||
|
||||
private fun handleInternalAction(action: AccountSecurityAction.Internal) {
|
||||
when (action) {
|
||||
is AccountSecurityAction.Internal.AuthenticatorSyncFeatureFlagUpdate -> {
|
||||
handleAuthenticatorSyncFeatureFlagUpdate(action)
|
||||
}
|
||||
|
||||
is AccountSecurityAction.Internal.BiometricsKeyResultReceive -> {
|
||||
handleBiometricsKeyResultReceive(action)
|
||||
}
|
||||
@ -448,18 +432,6 @@ class AccountSecurityViewModel @Inject constructor(
|
||||
}
|
||||
}
|
||||
|
||||
private fun handleAuthenticatorSyncFeatureFlagUpdate(
|
||||
action: AccountSecurityAction.Internal.AuthenticatorSyncFeatureFlagUpdate,
|
||||
) {
|
||||
val shouldShowAuthenticatorSync =
|
||||
action.isEnabled && isBuildVersionAtLeast(Build.VERSION_CODES.S)
|
||||
mutableStateFlow.update {
|
||||
it.copy(
|
||||
shouldShowEnableAuthenticatorSync = shouldShowAuthenticatorSync,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
private fun handleBiometricsKeyResultReceive(
|
||||
action: AccountSecurityAction.Internal.BiometricsKeyResultReceive,
|
||||
) {
|
||||
@ -778,13 +750,6 @@ sealed class AccountSecurityAction {
|
||||
*/
|
||||
sealed class Internal : AccountSecurityAction() {
|
||||
|
||||
/**
|
||||
* The feature flag value for authenticator syncing was updated.
|
||||
*/
|
||||
data class AuthenticatorSyncFeatureFlagUpdate(
|
||||
val isEnabled: Boolean,
|
||||
) : Internal()
|
||||
|
||||
/**
|
||||
* A biometrics key result has been received.
|
||||
*/
|
||||
|
||||
@ -9,10 +9,6 @@ class FlagKeyTest {
|
||||
|
||||
@Test
|
||||
fun `Feature flags have the correct key name set`() {
|
||||
assertEquals(
|
||||
FlagKey.AuthenticatorSync.keyName,
|
||||
"enable-pm-bwa-sync",
|
||||
)
|
||||
assertEquals(
|
||||
FlagKey.EmailVerification.keyName,
|
||||
"email-verification",
|
||||
@ -75,7 +71,6 @@ class FlagKeyTest {
|
||||
fun `All feature flags have the correct default value set`() {
|
||||
assertTrue(
|
||||
listOf(
|
||||
FlagKey.AuthenticatorSync,
|
||||
FlagKey.EmailVerification,
|
||||
FlagKey.ImportLoginsFlow,
|
||||
FlagKey.CredentialExchangeProtocolImport,
|
||||
|
||||
@ -21,8 +21,6 @@ import com.bitwarden.core.data.util.asSuccess
|
||||
import com.bitwarden.core.util.isBuildVersionAtLeast
|
||||
import com.bitwarden.data.datasource.disk.base.FakeDispatcherManager
|
||||
import com.x8bit.bitwarden.data.auth.manager.AddTotpItemFromAuthenticatorManagerImpl
|
||||
import com.x8bit.bitwarden.data.platform.manager.FeatureFlagManager
|
||||
import com.x8bit.bitwarden.data.platform.manager.model.FlagKey
|
||||
import com.x8bit.bitwarden.data.platform.repository.AuthenticatorBridgeRepository
|
||||
import com.x8bit.bitwarden.data.platform.util.createAddTotpItemFromAuthenticatorIntent
|
||||
import com.x8bit.bitwarden.ui.vault.model.TotpData
|
||||
@ -49,7 +47,6 @@ import org.junit.jupiter.api.Test
|
||||
|
||||
class AuthenticatorBridgeProcessorTest {
|
||||
|
||||
private val featureFlagManager = mockk<FeatureFlagManager>()
|
||||
private val addTotpItemFromAuthenticatorManager = AddTotpItemFromAuthenticatorManagerImpl()
|
||||
private val authenticatorBridgeRepository = mockk<AuthenticatorBridgeRepository>()
|
||||
private val context = mockk<Context> {
|
||||
@ -64,7 +61,6 @@ class AuthenticatorBridgeProcessorTest {
|
||||
addTotpItemFromAuthenticatorManager = addTotpItemFromAuthenticatorManager,
|
||||
authenticatorBridgeRepository = authenticatorBridgeRepository,
|
||||
context = context,
|
||||
featureFlagManager = featureFlagManager,
|
||||
dispatcherManager = FakeDispatcherManager(),
|
||||
)
|
||||
mockkStatic(::createAddTotpItemFromAuthenticatorIntent)
|
||||
@ -91,30 +87,16 @@ class AuthenticatorBridgeProcessorTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `when AuthenticatorSync feature flag is off, should return null binder`() {
|
||||
fun `when running Android level greater than S, should return non-null binder`() {
|
||||
mockkStatic(::isBuildVersionAtLeast)
|
||||
every { isBuildVersionAtLeast(Build.VERSION_CODES.S) } returns true
|
||||
every { featureFlagManager.getFeatureFlag(FlagKey.AuthenticatorSync) } returns false
|
||||
assertNull(bridgeServiceProcessor.binder)
|
||||
}
|
||||
|
||||
@Test
|
||||
@Suppress("MaxLineLength")
|
||||
fun `when AuthenticatorSync feature flag is on and running Android level greater than S, should return non-null binder`() {
|
||||
mockkStatic(::isBuildVersionAtLeast)
|
||||
every { isBuildVersionAtLeast(Build.VERSION_CODES.S) } returns true
|
||||
every { featureFlagManager.getFeatureFlag(FlagKey.AuthenticatorSync) } returns true
|
||||
assertNotNull(bridgeServiceProcessor.binder)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `when below Android level S, should never return a binder regardless of feature flag`() {
|
||||
fun `when below Android level S, should never return a binder`() {
|
||||
mockkStatic(::isBuildVersionAtLeast)
|
||||
every { isBuildVersionAtLeast(Build.VERSION_CODES.S) } returns false
|
||||
every { featureFlagManager.getFeatureFlag(FlagKey.AuthenticatorSync) } returns false
|
||||
assertNull(bridgeServiceProcessor.binder)
|
||||
|
||||
every { featureFlagManager.getFeatureFlag(FlagKey.AuthenticatorSync) } returns true
|
||||
assertNull(bridgeServiceProcessor.binder)
|
||||
}
|
||||
|
||||
@ -310,7 +292,6 @@ class AuthenticatorBridgeProcessorTest {
|
||||
private fun getDefaultBinder(): IAuthenticatorBridgeService.Stub {
|
||||
mockkStatic(::isBuildVersionAtLeast)
|
||||
every { isBuildVersionAtLeast(Build.VERSION_CODES.S) } returns true
|
||||
every { featureFlagManager.getFeatureFlag(FlagKey.AuthenticatorSync) } returns true
|
||||
return bridgeServiceProcessor.binder!!
|
||||
}
|
||||
}
|
||||
|
||||
@ -144,7 +144,6 @@ class DebugMenuViewModelTest : BaseViewModelTest() {
|
||||
}
|
||||
|
||||
private val DEFAULT_MAP_VALUE: ImmutableMap<FlagKey<Any>, Any> = persistentMapOf(
|
||||
FlagKey.AuthenticatorSync to true,
|
||||
FlagKey.EmailVerification to true,
|
||||
FlagKey.ImportLoginsFlow to true,
|
||||
FlagKey.CredentialExchangeProtocolImport to true,
|
||||
@ -162,7 +161,6 @@ private val DEFAULT_MAP_VALUE: ImmutableMap<FlagKey<Any>, Any> = persistentMapOf
|
||||
)
|
||||
|
||||
private val UPDATED_MAP_VALUE: ImmutableMap<FlagKey<Any>, Any> = persistentMapOf(
|
||||
FlagKey.AuthenticatorSync to false,
|
||||
FlagKey.EmailVerification to false,
|
||||
FlagKey.ImportLoginsFlow to false,
|
||||
FlagKey.CredentialExchangeProtocolImport to false,
|
||||
|
||||
@ -22,11 +22,9 @@ import com.x8bit.bitwarden.data.auth.repository.model.UserFingerprintResult
|
||||
import com.x8bit.bitwarden.data.auth.repository.model.UserState
|
||||
import com.x8bit.bitwarden.data.platform.error.NoActiveUserException
|
||||
import com.x8bit.bitwarden.data.platform.manager.BiometricsEncryptionManager
|
||||
import com.x8bit.bitwarden.data.platform.manager.FeatureFlagManager
|
||||
import com.x8bit.bitwarden.data.platform.manager.FirstTimeActionManager
|
||||
import com.x8bit.bitwarden.data.platform.manager.PolicyManager
|
||||
import com.x8bit.bitwarden.data.platform.manager.model.FirstTimeState
|
||||
import com.x8bit.bitwarden.data.platform.manager.model.FlagKey
|
||||
import com.x8bit.bitwarden.data.platform.repository.EnvironmentRepository
|
||||
import com.x8bit.bitwarden.data.platform.repository.SettingsRepository
|
||||
import com.x8bit.bitwarden.data.platform.repository.model.BiometricsKeyResult
|
||||
@ -46,7 +44,6 @@ import io.mockk.runs
|
||||
import io.mockk.unmockkStatic
|
||||
import io.mockk.verify
|
||||
import kotlinx.coroutines.flow.MutableStateFlow
|
||||
import kotlinx.coroutines.flow.emptyFlow
|
||||
import kotlinx.coroutines.flow.update
|
||||
import kotlinx.coroutines.test.runTest
|
||||
import kotlinx.serialization.json.Json
|
||||
@ -105,9 +102,6 @@ class AccountSecurityViewModelTest : BaseViewModelTest() {
|
||||
getActivePoliciesFlow(type = PolicyTypeJson.REMOVE_UNLOCK_WITH_PIN)
|
||||
} returns mutableRemovePinPolicyFlow
|
||||
}
|
||||
private val featureFlagManager: FeatureFlagManager = mockk(relaxed = true) {
|
||||
every { getFeatureFlag(FlagKey.AuthenticatorSync) } returns false
|
||||
}
|
||||
|
||||
@BeforeEach
|
||||
fun setup() {
|
||||
@ -818,29 +812,19 @@ class AccountSecurityViewModelTest : BaseViewModelTest() {
|
||||
}
|
||||
}
|
||||
|
||||
@Suppress("MaxLineLength")
|
||||
@Test
|
||||
fun `when featureFlagManger returns true for AuthenticatorSync, and version is at least 31 should show authenticator sync UI`() {
|
||||
fun `when version is at least 31 should show authenticator sync UI`() {
|
||||
every { isBuildVersionAtLeast(Build.VERSION_CODES.S) } returns true
|
||||
val vm = createViewModel(
|
||||
initialState = null,
|
||||
featureFlagManager = mockk {
|
||||
every { getFeatureFlag(FlagKey.AuthenticatorSync) } returns true
|
||||
every { getFeatureFlagFlow(FlagKey.AuthenticatorSync) } returns emptyFlow()
|
||||
},
|
||||
)
|
||||
val vm = createViewModel(initialState = null)
|
||||
assertEquals(
|
||||
DEFAULT_STATE.copy(shouldShowEnableAuthenticatorSync = true),
|
||||
vm.stateFlow.value,
|
||||
)
|
||||
}
|
||||
|
||||
@Suppress("MaxLineLength")
|
||||
@Test
|
||||
fun `when featureFlagManger returns true for AuthenticatorSync, and version is under 31 should not show authenticator sync UI`() {
|
||||
fun `when version is under 31 should not show authenticator sync UI`() {
|
||||
every { isBuildVersionAtLeast(Build.VERSION_CODES.S) } returns false
|
||||
every { featureFlagManager.getFeatureFlag(FlagKey.AuthenticatorSync) } returns true
|
||||
every { featureFlagManager.getFeatureFlagFlow(FlagKey.AuthenticatorSync) } returns emptyFlow()
|
||||
val vm = createViewModel(initialState = null)
|
||||
assertEquals(
|
||||
DEFAULT_STATE,
|
||||
@ -848,45 +832,6 @@ class AccountSecurityViewModelTest : BaseViewModelTest() {
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `when featureFlagManger updates value AuthenticatorSync, should update UI`() = runTest {
|
||||
every { isBuildVersionAtLeast(Build.VERSION_CODES.S) } returns true
|
||||
val featureFlagFlow = MutableStateFlow(false)
|
||||
val vm = createViewModel(
|
||||
initialState = null,
|
||||
featureFlagManager = mockk {
|
||||
every { getFeatureFlag(FlagKey.AuthenticatorSync) } returns false
|
||||
every { getFeatureFlagFlow(FlagKey.AuthenticatorSync) } returns featureFlagFlow
|
||||
},
|
||||
)
|
||||
vm.stateFlow.test {
|
||||
assertEquals(DEFAULT_STATE, awaitItem())
|
||||
featureFlagFlow.value = true
|
||||
assertEquals(DEFAULT_STATE.copy(shouldShowEnableAuthenticatorSync = true), awaitItem())
|
||||
featureFlagFlow.value = false
|
||||
assertEquals(DEFAULT_STATE, awaitItem())
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
@Suppress("MaxLineLength")
|
||||
fun `when featureFlagManger updates value AuthenticatorSync, authenticator sync row should never show if below API 31`() =
|
||||
runTest {
|
||||
every { isBuildVersionAtLeast(Build.VERSION_CODES.S) } returns false
|
||||
val featureFlagFlow = MutableStateFlow(false)
|
||||
every { featureFlagManager.getFeatureFlag(FlagKey.AuthenticatorSync) } returns false
|
||||
every {
|
||||
featureFlagManager.getFeatureFlagFlow(FlagKey.AuthenticatorSync)
|
||||
} returns featureFlagFlow
|
||||
val vm = createViewModel(initialState = null)
|
||||
vm.stateFlow.test {
|
||||
assertEquals(DEFAULT_STATE, awaitItem())
|
||||
featureFlagFlow.value = true
|
||||
featureFlagFlow.value = false
|
||||
expectNoEvents()
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `when showUnlockBadgeFlow updates value, should update state`() = runTest {
|
||||
val viewModel = createViewModel()
|
||||
@ -951,7 +896,6 @@ class AccountSecurityViewModelTest : BaseViewModelTest() {
|
||||
private fun createViewModel(
|
||||
initialState: AccountSecurityState? = DEFAULT_STATE,
|
||||
authRepository: AuthRepository = this.authRepository,
|
||||
featureFlagManager: FeatureFlagManager = this.featureFlagManager,
|
||||
vaultRepository: VaultRepository = this.vaultRepository,
|
||||
environmentRepository: EnvironmentRepository = this.fakeEnvironmentRepository,
|
||||
settingsRepository: SettingsRepository = this.settingsRepository,
|
||||
@ -959,7 +903,6 @@ class AccountSecurityViewModelTest : BaseViewModelTest() {
|
||||
policyManager: PolicyManager = this.policyManager,
|
||||
): AccountSecurityViewModel = AccountSecurityViewModel(
|
||||
authRepository = authRepository,
|
||||
featureFlagManager = featureFlagManager,
|
||||
vaultRepository = vaultRepository,
|
||||
settingsRepository = settingsRepository,
|
||||
environmentRepository = environmentRepository,
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user