Align UI and specs for blockmalware

duckduckgo · Dec 11, 2024 · e2e26b4 · e2e26b4
1 parent 67690c0
commit e2e26b4
Show file tree

Hide file tree

Showing 8 changed files with 68 additions and 19 deletions.
diff --git a/...-protection-impl/src/main/java/com/duckduckgo/networkprotection/impl/VpnRemoteFeatures.kt b/...-protection-impl/src/main/java/com/duckduckgo/networkprotection/impl/VpnRemoteFeatures.kt
@@ -48,6 +48,9 @@ interface VpnRemoteFeatures {
 
     @DefaultValue(true)
     fun showExcludeAppPrompt(): Toggle // kill switch
+
+    @DefaultValue(true)
+    fun allowBlockMalware(): Toggle // kill switch
 }
 
 @ContributesBinding(AppScope::class)

diff --git a/...-protection-impl/src/main/java/com/duckduckgo/networkprotection/impl/WgVpnNetworkStack.kt b/...-protection-impl/src/main/java/com/duckduckgo/networkprotection/impl/WgVpnNetworkStack.kt
@@ -58,6 +58,7 @@ class WgVpnNetworkStack @Inject constructor(
     private val dnsProvider: DnsProvider,
     private val crashLogger: CrashLogger,
     private val netPSettingsLocalConfig: NetPSettingsLocalConfig,
+    private val vpnRemoteFeatures: VpnRemoteFeatures,
 ) : VpnNetworkStack {
     private var wgConfig: Config? = null
 
@@ -75,7 +76,7 @@ class WgVpnNetworkStack @Inject constructor(
             logcat { "Wireguard configuration:\n$wgConfig" }
 
             val privateDns = dnsProvider.getPrivateDns()
-            val dns = if (netPSettingsLocalConfig.blockMalware().isEnabled()) {
+            val dns = if (netPSettingsLocalConfig.blockMalware().isEnabled() && vpnRemoteFeatures.allowBlockMalware().isEnabled()) {
                 // if the user has configured "block malware" we calculate the malware DNS from the DDG default DNS(s)
                 wgConfig!!.`interface`.dnsServers.map { it.computeBlockMalwareDnsOrSame() }.toSet()
             } else {

diff --git a/...in/java/com/duckduckgo/networkprotection/impl/settings/custom_dns/VpnCustomDnsActivity.kt b/...in/java/com/duckduckgo/networkprotection/impl/settings/custom_dns/VpnCustomDnsActivity.kt
@@ -24,8 +24,6 @@ import android.widget.CompoundButton.OnCheckedChangeListener
 import androidx.lifecycle.lifecycleScope
 import com.duckduckgo.anvil.annotations.ContributeToActivityStarter
 import com.duckduckgo.anvil.annotations.InjectWith
-import com.duckduckgo.appbuildconfig.api.AppBuildConfig
-import com.duckduckgo.appbuildconfig.api.isInternalBuild
 import com.duckduckgo.common.ui.DuckDuckGoActivity
 import com.duckduckgo.common.ui.view.gone
 import com.duckduckgo.common.ui.view.quietlySetIsChecked
@@ -64,9 +62,6 @@ import kotlinx.coroutines.launch
 @ContributeToActivityStarter(Default::class)
 class VpnCustomDnsActivity : DuckDuckGoActivity() {
 
-    @Inject
-    lateinit var appBuildConfig: AppBuildConfig
-
     private val binding: ActivityNetpCustomDnsBinding by viewBinding()
     private val viewModel: VpnCustomDnsViewModel by bindViewModel()
 
@@ -167,8 +162,7 @@ class VpnCustomDnsActivity : DuckDuckGoActivity() {
                 binding.customDns.isEditable = false
                 binding.customDnsSection.gone()
 
-                // for now we only want to show this to internal users
-                if (appBuildConfig.isInternalBuild()) {
+                if (state.allowBlockMalware) {
                     binding.blockMalwareSection.show()
                     binding.blockMalwareToggle.quietlySetIsChecked(state.blockMalware, blockMalwareToggleListener)
                 } else {
@@ -190,6 +184,7 @@ class VpnCustomDnsActivity : DuckDuckGoActivity() {
                 binding.customDns.addTextChangedListener(customDnsTextWatcher)
                 binding.applyDnsChanges.isEnabled = state.applyEnabled
             }
+
             is Done -> {
                 networkProtectionState.restart()
                 if (state.finish) {
@@ -263,9 +258,18 @@ class VpnCustomDnsActivity : DuckDuckGoActivity() {
     }
 
     internal sealed class State {
-        // data class NeedApply(val value: Boolean) : State()
-        data class DefaultDns(val allowChange: Boolean, val blockMalware: Boolean) : State()
-        data class CustomDns(val dns: String?, val allowChange: Boolean, val applyEnabled: Boolean) : State()
+        data class DefaultDns(
+            val allowChange: Boolean,
+            val blockMalware: Boolean,
+            val allowBlockMalware: Boolean,
+        ) : State()
+
+        data class CustomDns(
+            val dns: String?,
+            val allowChange: Boolean,
+            val applyEnabled: Boolean,
+        ) : State()
+
         data class Done(val finish: Boolean = true) : State()
     }
 }

diff --git a/...n/java/com/duckduckgo/networkprotection/impl/settings/custom_dns/VpnCustomDnsViewModel.kt b/...n/java/com/duckduckgo/networkprotection/impl/settings/custom_dns/VpnCustomDnsViewModel.kt
@@ -23,6 +23,7 @@ import com.duckduckgo.anvil.annotations.ContributesViewModel
 import com.duckduckgo.common.utils.DispatcherProvider
 import com.duckduckgo.di.scopes.ActivityScope
 import com.duckduckgo.feature.toggles.api.Toggle
+import com.duckduckgo.networkprotection.impl.VpnRemoteFeatures
 import com.duckduckgo.networkprotection.impl.pixels.NetworkProtectionPixels
 import com.duckduckgo.networkprotection.impl.settings.NetPSettingsLocalConfig
 import com.duckduckgo.networkprotection.impl.settings.NetpVpnSettingsDataStore
@@ -52,6 +53,7 @@ class VpnCustomDnsViewModel @Inject constructor(
     private val netpVpnSettingsDataStore: NetpVpnSettingsDataStore,
     private val networkProtectionPixels: NetworkProtectionPixels,
     private val netPSettingsLocalConfig: NetPSettingsLocalConfig,
+    private val vpnRemoteFeatures: VpnRemoteFeatures,
     dispatcherProvider: DispatcherProvider,
 ) : ViewModel() {
 
@@ -60,6 +62,9 @@ class VpnCustomDnsViewModel @Inject constructor(
     private val blockMalware: Deferred<Boolean> = viewModelScope.async(context = dispatcherProvider.io(), start = CoroutineStart.LAZY) {
         netPSettingsLocalConfig.blockMalware().isEnabled()
     }
+    private val allowBlockMalware: Deferred<Boolean> = viewModelScope.async(context = dispatcherProvider.io(), start = CoroutineStart.LAZY) {
+        vpnRemoteFeatures.allowBlockMalware().isEnabled()
+    }
 
     internal fun reduce(event: Event): Flow<State> {
         return when (event) {
@@ -78,7 +83,7 @@ class VpnCustomDnsViewModel @Inject constructor(
     private fun handleBlockMalwareState(isEnabled: Boolean) = flow {
         netPSettingsLocalConfig.blockMalware().setRawStoredState(Toggle.State(enable = isEnabled))
         netpVpnSettingsDataStore.customDns = null
-        emit(State.DefaultDns(true, isEnabled))
+        emit(State.DefaultDns(true, isEnabled, true))
         emit(State.Done(finish = false))
     }
 
@@ -103,7 +108,7 @@ class VpnCustomDnsViewModel @Inject constructor(
 
     private fun handleDefaultDnsSelected() = flow {
         currentState = DefaultDns
-        emit(State.DefaultDns(true, blockMalware.await()))
+        emit(State.DefaultDns(true, blockMalware.await(), allowBlockMalware.await()))
     }
 
     private fun handleCustomDnsSelected() = flow {
@@ -126,7 +131,7 @@ class VpnCustomDnsViewModel @Inject constructor(
         }
         customDns?.let {
             emit(State.CustomDns(it, !isPrivateDnsActive, applyEnabled = false))
-        } ?: emit(State.DefaultDns(!isPrivateDnsActive, blockMalware.await()))
+        } ?: emit(State.DefaultDns(!isPrivateDnsActive, blockMalware.await(), allowBlockMalware.await()))
     }
 
     private fun String.isValidAddress(): Boolean {

diff --git a/...duckduckgo/networkprotection/impl/settings/custom_dns/VpnCustomDnsViewSettingViewModel.kt b/...duckduckgo/networkprotection/impl/settings/custom_dns/VpnCustomDnsViewSettingViewModel.kt
@@ -18,6 +18,7 @@ package com.duckduckgo.networkprotection.impl.settings.custom_dns
 
 import androidx.lifecycle.ViewModel
 import androidx.lifecycle.ViewModelProvider
+import com.duckduckgo.networkprotection.impl.VpnRemoteFeatures
 import com.duckduckgo.networkprotection.impl.settings.NetPSettingsLocalConfig
 import com.duckduckgo.networkprotection.impl.settings.NetpVpnSettingsDataStore
 import com.duckduckgo.networkprotection.impl.settings.custom_dns.VpnCustomDnsSettingView.Event
@@ -30,6 +31,7 @@ import kotlinx.coroutines.flow.flow
 class VpnCustomDnsViewSettingViewModel(
     private val netpVpnSettingsDataStore: NetpVpnSettingsDataStore,
     private val netPSettingsLocalConfig: NetPSettingsLocalConfig,
+    private val vpnRemoteFeatures: VpnRemoteFeatures,
 ) : ViewModel() {
 
     internal fun reduce(event: Event): Flow<State> {
@@ -41,7 +43,7 @@ class VpnCustomDnsViewSettingViewModel(
     private fun onInit(): Flow<State> = flow {
         netpVpnSettingsDataStore.customDns?.let {
             emit(State.CustomDns(it))
-        } ?: if (netPSettingsLocalConfig.blockMalware().isEnabled()) {
+        } ?: if (netPSettingsLocalConfig.blockMalware().isEnabled() && vpnRemoteFeatures.allowBlockMalware().isEnabled()) {
             emit(State.DefaultBlockMalware)
         } else {
             emit(State.Default)
@@ -52,12 +54,18 @@ class VpnCustomDnsViewSettingViewModel(
     class Factory @Inject constructor(
         private val store: NetpVpnSettingsDataStore,
         private val netPSettingsLocalConfig: NetPSettingsLocalConfig,
+        private val vpnRemoteFeatures: VpnRemoteFeatures,
     ) : ViewModelProvider.NewInstanceFactory() {
 
         override fun <T : ViewModel> create(modelClass: Class<T>): T {
             return with(modelClass) {
                 when {
-                    isAssignableFrom(VpnCustomDnsViewSettingViewModel::class.java) -> VpnCustomDnsViewSettingViewModel(store, netPSettingsLocalConfig)
+                    isAssignableFrom(VpnCustomDnsViewSettingViewModel::class.java) -> VpnCustomDnsViewSettingViewModel(
+                        store,
+                        netPSettingsLocalConfig,
+                        vpnRemoteFeatures,
+                    )
+
                     else -> throw IllegalArgumentException("Unknown ViewModel class: ${modelClass.name}")
                 }
             } as T

diff --git a/network-protection/network-protection-impl/src/main/res/layout/activity_netp_custom_dns.xml b/network-protection/network-protection-impl/src/main/res/layout/activity_netp_custom_dns.xml
@@ -104,6 +104,11 @@
                         android:layout_width="match_parent"
                         android:layout_height="wrap_content" />
 
+                <com.duckduckgo.common.ui.view.listitem.SectionHeaderListItem
+                    android:layout_width="wrap_content"
+                    android:layout_height="wrap_content"
+                    app:primaryText="@string/netpDnsBlockMalwareHeader"/>
+
                 <com.duckduckgo.common.ui.view.listitem.TwoLineListItem
                         android:id="@+id/block_malware_toggle"
                         android:layout_width="match_parent"

diff --git a/network-protection/network-protection-impl/src/main/res/values/strings-netp.xml b/network-protection/network-protection-impl/src/main/res/values/strings-netp.xml
@@ -235,8 +235,9 @@
     <string name="netpDdgDnsByLine">DuckDuckGo routes DNS queries through our DNS servers so your internet provider can\'t see what websites you visit.</string>
 
     <!-- Block malware DNS -->
-    <string name="netpDnsBlockMalwarePrimary">Block Malware</string>
-    <string name="netpDnsBlockMalwareByline">Block malware with a DNS-level blocklist. If a website doesn\'t load, try turning blocking off.</string>
+    <string name="netpDnsBlockMalwareHeader">Block lists</string>
+    <string name="netpDnsBlockMalwarePrimary">Block malware and more</string>
+    <string name="netpDnsBlockMalwareByline">Block malware, phishing attacks, and online scams with a DNS-level blocklist. If a website doesn\'t load, try turning it off.</string>
 
     <!-- Auto-exclude -->
     <string name="netpAutoExcludePromptTitle">Would you like to exclude apps that are not compatible with VPNs?</string>

diff --git a/...tection-impl/src/test/java/com/duckduckgo/networkprotection/impl/WgVpnNetworkStackTest.kt b/...tection-impl/src/test/java/com/duckduckgo/networkprotection/impl/WgVpnNetworkStackTest.kt
@@ -18,6 +18,7 @@ package com.duckduckgo.networkprotection.impl
 
 import android.annotation.SuppressLint
 import com.duckduckgo.data.store.api.FakeSharedPreferencesProvider
+import com.duckduckgo.feature.toggles.api.FakeFeatureToggleFactory
 import com.duckduckgo.feature.toggles.api.Toggle
 import com.duckduckgo.mobile.android.vpn.network.FakeDnsProvider
 import com.duckduckgo.mobile.android.vpn.network.VpnNetworkStack.VpnTunnelConfig
@@ -103,11 +104,13 @@ class WgVpnNetworkStackTest {
 
     private lateinit var wgVpnNetworkStack: WgVpnNetworkStack
     private lateinit var netPSettingsLocalConfig: NetPSettingsLocalConfig
+    private lateinit var vpnRemoteFeatures: VpnRemoteFeatures
 
     @Before
     fun setUp() {
         MockitoAnnotations.openMocks(this)
         netPSettingsLocalConfig = FakeNetPSettingsLocalConfigFactory.create()
+        vpnRemoteFeatures = FakeFeatureToggleFactory.create(VpnRemoteFeatures::class.java)
 
         privateDnsProvider = FakeDnsProvider()
         networkProtectionRepository = RealNetworkProtectionRepository(
@@ -127,6 +130,7 @@ class WgVpnNetworkStackTest {
             privateDnsProvider,
             mock(),
             netPSettingsLocalConfig,
+            vpnRemoteFeatures,
         )
     }
 
@@ -149,9 +153,10 @@ class WgVpnNetworkStackTest {
 
     @SuppressLint("DenyListedApi")
     @Test
-    fun whenBlockMalwareIsConfigureDNSIsConputed() = runTest {
+    fun whenBlockMalwareIsConfigureDNSIsComputed() = runTest {
         whenever(wgTunnel.createAndSetWgConfig()).thenReturn(wgConfig.success())
         netPSettingsLocalConfig.blockMalware().setRawStoredState(Toggle.State(enable = true))
+        vpnRemoteFeatures.allowBlockMalware().setRawStoredState(Toggle.State(enable = true))
 
         val actual = wgVpnNetworkStack.onPrepareVpn().getOrNull()
         val expected = wgConfig.toTunnelConfig().copy(
@@ -163,6 +168,23 @@ class WgVpnNetworkStackTest {
         verify(netpPixels).reportEnableAttempt()
     }
 
+    @SuppressLint("DenyListedApi")
+    @Test
+    fun whenBlockMalwareKillSwitched() = runTest {
+        whenever(wgTunnel.createAndSetWgConfig()).thenReturn(wgConfig.success())
+        netPSettingsLocalConfig.blockMalware().setRawStoredState(Toggle.State(enable = true))
+        vpnRemoteFeatures.allowBlockMalware().setRawStoredState(Toggle.State(enable = false))
+
+        val actual = wgVpnNetworkStack.onPrepareVpn().getOrNull()
+        val expected = wgConfig.toTunnelConfig().copy(
+            dns = wgConfig.toTunnelConfig().dns.toSet(),
+        )
+        assertNotNull(actual)
+        assertEquals(expected, actual)
+
+        verify(netpPixels).reportEnableAttempt()
+    }
+
     @Test
     fun whenOnPrepareVpnAndPrivateDnsConfiguredThenReturnEmptyDnsList() = runTest {
         whenever(wgTunnel.createAndSetWgConfig()).thenReturn(wgConfig.success())