[PM-8217] Add local feature flag to ignore environment validation (#4521)

This commit is contained in:
André Bispo
2025-01-06 15:32:53 +00:00
committed by GitHub
parent d5a02e6285
commit 8bde8741dd
7 changed files with 68 additions and 71 deletions

View File

@@ -1388,7 +1388,9 @@ class AuthRepositoryImpl(
* - Cannot have two-factor authentication enabled.
*/
private fun newDeviceNoticePreConditionsValid(): Boolean {
if (environmentRepository.environment.type == Environment.Type.SELF_HOSTED) {
val checkEnvironment = !featureFlagManager.getFeatureFlag(FlagKey.IgnoreEnvironmentCheck)
val isSelfHosted = environmentRepository.environment.type == Environment.Type.SELF_HOSTED
if (checkEnvironment && isSelfHosted) {
return false
}

View File

@@ -38,6 +38,7 @@ sealed class FlagKey<out T : Any> {
AppReviewPrompt,
NewDevicePermanentDismiss,
NewDeviceTemporaryDismiss,
IgnoreEnvironmentCheck,
)
}
}
@@ -161,6 +162,15 @@ sealed class FlagKey<out T : Any> {
override val isRemotelyConfigured: Boolean = true
}
/**
* Data object holding the feature flag key to ignore an environment check.
*/
data object IgnoreEnvironmentCheck : FlagKey<Boolean>() {
override val keyName: String = "ignore-environment-check"
override val defaultValue: Boolean = false
override val isRemotelyConfigured: Boolean = false
}
//region Dummy keys for testing
/**
* Data object holding the key for a [Boolean] flag to be used in tests.

View File

@@ -35,6 +35,7 @@ fun <T : Any> FlagKey<T>.ListItemContent(
FlagKey.CipherKeyEncryption,
FlagKey.NewDevicePermanentDismiss,
FlagKey.NewDeviceTemporaryDismiss,
FlagKey.IgnoreEnvironmentCheck,
-> BooleanFlagItem(
label = flagKey.getDisplayLabel(),
key = flagKey as FlagKey<Boolean>,
@@ -85,4 +86,5 @@ private fun <T : Any> FlagKey<T>.getDisplayLabel(): String = when (this) {
FlagKey.CipherKeyEncryption -> stringResource(R.string.cipher_key_encryption)
FlagKey.NewDevicePermanentDismiss -> stringResource(R.string.new_device_permanent_dismiss)
FlagKey.NewDeviceTemporaryDismiss -> stringResource(R.string.new_device_temporary_dismiss)
FlagKey.IgnoreEnvironmentCheck -> stringResource(R.string.ignore_environment_check)
}

View File

@@ -24,5 +24,6 @@
<string name="cipher_key_encryption">Cipher Key Encryption</string>">
<string name="new_device_permanent_dismiss">New device notice permanent dismiss</string>">
<string name="new_device_temporary_dismiss">New device notice temporary dismiss</string>">
<string name="ignore_environment_check">Ignore environment check</string>">
<!-- /Debug Menu -->
</resources>

View File

@@ -250,7 +250,10 @@ class AuthRepositoryTest {
}
private val featureFlagManager: FeatureFlagManager = mockk(relaxed = true) {
every { getFeatureFlag(FlagKey.NewDeviceTemporaryDismiss) } returns true
every { getFeatureFlag(FlagKey.NewDevicePermanentDismiss) } returns true
every { getFeatureFlag(FlagKey.OnboardingFlow) } returns false
every { getFeatureFlag(FlagKey.IgnoreEnvironmentCheck) } returns false
}
private val firstTimeActionManager = mockk<FirstTimeActionManager> {
@@ -6530,12 +6533,6 @@ class AuthRepositoryTest {
@Suppress("MaxLineLength")
fun `checkUserNeedsNewDeviceTwoFactorNotice flags on, is cloud user, profile at least week old, no required sso policy, no two factor enable returns true`() =
runTest {
every {
featureFlagManager.getFeatureFlag(FlagKey.NewDevicePermanentDismiss)
} returns true
every {
featureFlagManager.getFeatureFlag(FlagKey.NewDeviceTemporaryDismiss)
} returns true
every {
policyManager.getActivePolicies(type = PolicyTypeJson.REQUIRE_SSO)
} returns listOf()
@@ -6571,14 +6568,47 @@ class AuthRepositoryTest {
}
@Test
fun `checkUserNeedsNewDeviceTwoFactorNotice has required SSO policy returns false`() =
@Suppress("MaxLineLength")
fun `checkUserNeedsNewDeviceTwoFactorNotice IgnoreEnvironmentCheck flag enabled should not check for a cloud environment and return true`() =
runTest {
every {
featureFlagManager.getFeatureFlag(FlagKey.NewDevicePermanentDismiss)
featureFlagManager.getFeatureFlag(FlagKey.IgnoreEnvironmentCheck)
} returns true
every {
featureFlagManager.getFeatureFlag(FlagKey.NewDeviceTemporaryDismiss)
} returns true
policyManager.getActivePolicies(type = PolicyTypeJson.REQUIRE_SSO)
} returns listOf()
fakeEnvironmentRepository.environment = Environment.SelfHosted(
EnvironmentUrlDataJson(base = "https://myselfhosted.environment.com"),
)
fakeAuthDiskSource.userState = SINGLE_USER_STATE_1
val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice()
assertTrue(shouldShowNewDeviceNotice)
}
@Test
@Suppress("MaxLineLength")
fun `checkUserNeedsNewDeviceTwoFactorNotice if environment is selfhosted return false`() =
runTest {
every {
policyManager.getActivePolicies(type = PolicyTypeJson.REQUIRE_SSO)
} returns listOf()
fakeEnvironmentRepository.environment = Environment.SelfHosted(
EnvironmentUrlDataJson(base = "https://myselfhosted.environment.com"),
)
fakeAuthDiskSource.userState = SINGLE_USER_STATE_1
val shouldShowNewDeviceNotice = repository.checkUserNeedsNewDeviceTwoFactorNotice()
assertFalse(shouldShowNewDeviceNotice)
}
@Test
fun `checkUserNeedsNewDeviceTwoFactorNotice has required SSO policy returns false`() =
runTest {
every {
policyManager.getActivePolicies(type = PolicyTypeJson.REQUIRE_SSO)
} returns listOf(
@@ -6599,12 +6629,6 @@ class AuthRepositoryTest {
@Test
fun `checkUserNeedsNewDeviceTwoFactorNotice with two factor enable returns false`() =
runTest {
every {
featureFlagManager.getFeatureFlag(FlagKey.NewDevicePermanentDismiss)
} returns true
every {
featureFlagManager.getFeatureFlag(FlagKey.NewDeviceTemporaryDismiss)
} returns true
every {
policyManager.getActivePolicies(type = PolicyTypeJson.REQUIRE_SSO)
} returns listOf()
@@ -6620,12 +6644,6 @@ class AuthRepositoryTest {
@Test
fun `checkUserNeedsNewDeviceTwoFactorNotice account less than a week old returns false`() =
runTest {
every {
featureFlagManager.getFeatureFlag(FlagKey.NewDevicePermanentDismiss)
} returns true
every {
featureFlagManager.getFeatureFlag(FlagKey.NewDeviceTemporaryDismiss)
} returns true
every {
policyManager.getActivePolicies(type = PolicyTypeJson.REQUIRE_SSO)
} returns listOf()
@@ -6652,12 +6670,6 @@ class AuthRepositoryTest {
@Suppress("MaxLineLength")
fun `checkUserNeedsNewDeviceTwoFactorNotice with NewDeviceNoticeDisplayStatus CAN_ACCESS_EMAIL_PERMANENT return false`() =
runTest {
every {
featureFlagManager.getFeatureFlag(FlagKey.NewDevicePermanentDismiss)
} returns true
every {
featureFlagManager.getFeatureFlag(FlagKey.NewDeviceTemporaryDismiss)
} returns true
every {
policyManager.getActivePolicies(type = PolicyTypeJson.REQUIRE_SSO)
} returns listOf()
@@ -6681,12 +6693,6 @@ class AuthRepositoryTest {
@Suppress("MaxLineLength")
fun `checkUserNeedsNewDeviceTwoFactorNotice with NewDeviceNoticeDisplayStatus HAS_NOT_SEEN return true`() =
runTest {
every {
featureFlagManager.getFeatureFlag(FlagKey.NewDevicePermanentDismiss)
} returns true
every {
featureFlagManager.getFeatureFlag(FlagKey.NewDeviceTemporaryDismiss)
} returns true
every {
policyManager.getActivePolicies(type = PolicyTypeJson.REQUIRE_SSO)
} returns listOf()
@@ -6710,12 +6716,6 @@ class AuthRepositoryTest {
@Suppress("MaxLineLength")
fun `checkUserNeedsNewDeviceTwoFactorNotice with NewDeviceNoticeDisplayStatus HAS_SEEN return true if date is older than 7 days`() =
runTest {
every {
featureFlagManager.getFeatureFlag(FlagKey.NewDevicePermanentDismiss)
} returns true
every {
featureFlagManager.getFeatureFlag(FlagKey.NewDeviceTemporaryDismiss)
} returns true
every {
policyManager.getActivePolicies(type = PolicyTypeJson.REQUIRE_SSO)
} returns listOf()
@@ -6739,12 +6739,6 @@ class AuthRepositoryTest {
@Suppress("MaxLineLength")
fun `checkUserNeedsNewDeviceTwoFactorNotice with NewDeviceNoticeDisplayStatus HAS_SEEN return false if date is not older than 7 days`() =
runTest {
every {
featureFlagManager.getFeatureFlag(FlagKey.NewDevicePermanentDismiss)
} returns true
every {
featureFlagManager.getFeatureFlag(FlagKey.NewDeviceTemporaryDismiss)
} returns true
every {
policyManager.getActivePolicies(type = PolicyTypeJson.REQUIRE_SSO)
} returns listOf()
@@ -6768,12 +6762,6 @@ class AuthRepositoryTest {
@Suppress("MaxLineLength")
fun `checkUserNeedsNewDeviceTwoFactorNotice with NewDeviceNoticeDisplayStatus CAN_ACCESS_EMAIL return permanent flag value`() =
runTest {
every {
featureFlagManager.getFeatureFlag(FlagKey.NewDevicePermanentDismiss)
} returns true
every {
featureFlagManager.getFeatureFlag(FlagKey.NewDeviceTemporaryDismiss)
} returns true
every {
policyManager.getActivePolicies(type = PolicyTypeJson.REQUIRE_SSO)
} returns listOf()
@@ -6800,12 +6788,6 @@ class AuthRepositoryTest {
@Test
fun `checkUserNeedsNewDeviceTwoFactorNotice with no active user returns false`() =
runTest {
every {
featureFlagManager.getFeatureFlag(FlagKey.NewDevicePermanentDismiss)
} returns true
every {
featureFlagManager.getFeatureFlag(FlagKey.NewDeviceTemporaryDismiss)
} returns true
every {
policyManager.getActivePolicies(type = PolicyTypeJson.REQUIRE_SSO)
} returns listOf()
@@ -6819,12 +6801,6 @@ class AuthRepositoryTest {
@Test
fun `checkUserNeedsNewDeviceTwoFactorNotice account with null creationDate returns false`() =
runTest {
every {
featureFlagManager.getFeatureFlag(FlagKey.NewDevicePermanentDismiss)
} returns true
every {
featureFlagManager.getFeatureFlag(FlagKey.NewDeviceTemporaryDismiss)
} returns true
every {
policyManager.getActivePolicies(type = PolicyTypeJson.REQUIRE_SSO)
} returns listOf()
@@ -6848,12 +6824,6 @@ class AuthRepositoryTest {
@Suppress("MaxLineLength")
fun `checkUserNeedsNewDeviceTwoFactorNotice account with null isTwoFactorEnabled returns true`() =
runTest {
every {
featureFlagManager.getFeatureFlag(FlagKey.NewDevicePermanentDismiss)
} returns true
every {
featureFlagManager.getFeatureFlag(FlagKey.NewDeviceTemporaryDismiss)
} returns true
every {
policyManager.getActivePolicies(type = PolicyTypeJson.REQUIRE_SSO)
} returns listOf()

View File

@@ -66,4 +66,14 @@ class FlagKeyTest {
fun `NewDeviceTemporaryDismiss is remotely configured value should be true`() {
assertTrue(FlagKey.NewDeviceTemporaryDismiss.isRemotelyConfigured)
}
@Test
fun `IgnoreEnvironmentCheck default value should be false`() {
assertFalse(FlagKey.IgnoreEnvironmentCheck.defaultValue)
}
@Test
fun `IgnoreEnvironmentCheck is remotely configured value should be false`() {
assertFalse(FlagKey.IgnoreEnvironmentCheck.isRemotelyConfigured)
}
}

View File

@@ -119,6 +119,7 @@ private val DEFAULT_MAP_VALUE: Map<FlagKey<Any>, Any> = mapOf(
FlagKey.AppReviewPrompt to true,
FlagKey.NewDeviceTemporaryDismiss to true,
FlagKey.NewDevicePermanentDismiss to true,
FlagKey.IgnoreEnvironmentCheck to true,
)
private val UPDATED_MAP_VALUE: Map<FlagKey<Any>, Any> = mapOf(
@@ -134,6 +135,7 @@ private val UPDATED_MAP_VALUE: Map<FlagKey<Any>, Any> = mapOf(
FlagKey.AppReviewPrompt to false,
FlagKey.NewDeviceTemporaryDismiss to false,
FlagKey.NewDevicePermanentDismiss to false,
FlagKey.IgnoreEnvironmentCheck to false,
)
private val DEFAULT_STATE = DebugMenuState(