Fix missing NETP_DEBUG_SERVER_TOKEN for internal builds (#5495)

Task/Issue URL: https://app.asana.com/0/488551667048375/1209187729140301/f

### Description
When we migrated to GHA, the `NETP_DEBUG_SERVER_TOKEN` was no longer passed as env variable, breaking VPN internal settings

Incidentally I changed the way we intercept requests, using annotations rather than endpoint string matching

### Steps to test this PR

- [ ] Build internal build and verify `VPN server` option in NetP dev settings works as expected

.github/workflows/release_nightly.yml

@@ -100,6 +100,8 @@ jobs:
           gradle clean          
 
       - name: Assemble the bundle
+        env:
+          NETP_DEBUG_SERVER_TOKEN: ${{ secrets.NETP_DEBUG_SERVER_TOKEN }}
         if: steps.check_for_changes.outputs.has_changes == 'true'
         run: gradle bundleInternalRelease -PversionNameSuffix=-nightly -PuseUploadSigning -PlatestTag=${{ steps.get_latest_tag.outputs.latest_tag }} -Pbuild-date-time
 

common/common-test/build.gradle

@@ -20,6 +20,7 @@ dependencies {
     implementation "com.squareup.moshi:moshi-kotlin:_"
     implementation Square.okHttp3.okHttp
     implementation KotlinX.coroutines.core
+    implementation Square.retrofit2.retrofit
     // api because TestDispatcher is in the CoroutineTestRule public API
     api (KotlinX.coroutines.test) {
         // https://github.com/Kotlin/kotlinx.coroutines/issues/2023

common/common-test/src/main/java/com/duckduckgo/common/test/api/FakeChain.kt

@@ -16,12 +16,15 @@
 
 package com.duckduckgo.common.test.api
 
+import java.lang.reflect.Method
 import java.util.concurrent.TimeUnit
 import okhttp3.*
+import retrofit2.Invocation
 
 open class FakeChain(
     private val url: String,
     private val expectedResponseCode: Int? = null,
+    private val serviceMethod: Method? = null,
 ) : Interceptor.Chain {
     override fun call(): Call {
         TODO("Not yet implemented")
@@ -50,7 +53,12 @@ open class FakeChain(
     }
 
     override fun request(): Request {
-        return Request.Builder().url(url).build()
+        return Request.Builder().apply {
+            url(url)
+            serviceMethod?.let {
+                tag(Invocation::class.java, Invocation.of(it, emptyList<Unit>()))
+            }
+        }.build()
     }
 
     override fun withConnectTimeout(

network-protection/network-protection-impl/src/main/java/com/duckduckgo/networkprotection/impl/configuration/NetpControllerRequestInterceptor.kt

@@ -29,6 +29,7 @@ import logcat.logcat
 import okhttp3.Interceptor
 import okhttp3.Interceptor.Chain
 import okhttp3.Response
+import retrofit2.Invocation
 
 @ContributesMultibinding(
     scope = AppScope::class,
@@ -42,9 +43,16 @@ class NetpControllerRequestInterceptor @Inject constructor(
     override fun getInterceptor(): Interceptor = this
 
     override fun intercept(chain: Chain): Response {
-        val url = chain.request().url
-        val newRequest = chain.request().newBuilder()
-        return if (ENDPOINTS_PATTERN_MATCHER.any { url.toString().endsWith(it) }) {
+        val request = chain.request()
+        val url = request
+
+        val authRequired = chain.request().tag(Invocation::class.java)
+            ?.method()
+            ?.isAnnotationPresent(AuthRequired::class.java) == true
+
+        return if (authRequired) {
+            val newRequest = chain.request().newBuilder()
+
             logcat { "Adding Authorization Bearer token to request $url" }
             newRequest.addHeader(
                 name = "Authorization",
@@ -63,20 +71,11 @@ class NetpControllerRequestInterceptor @Inject constructor(
                 newRequest.build().also { logcat { "headers: ${it.headers}" } },
             )
         } else {
-            chain.proceed(newRequest.build())
+            chain.proceed(request)
         }
     }
 
     private suspend fun authorizationHeaderValue(): String {
         return "bearer ddg:${subscriptions.getAccessToken()}"
     }
-
-    companion object {
-        // The NetP environments are for now https://<something>.netp.duckduckgo.com/<endpoint>
-        private val ENDPOINTS_PATTERN_MATCHER = listOf(
-            "netp.duckduckgo.com/servers",
-            "netp.duckduckgo.com/register",
-            "netp.duckduckgo.com/locations",
-        )
-    }
 }

network-protection/network-protection-impl/src/main/java/com/duckduckgo/networkprotection/impl/configuration/WgVpnControllerService.kt

@@ -111,6 +111,7 @@ object WgVpnControllerServiceModule {
 @ProtectedVpnControllerService
 interface WgVpnControllerService {
     @GET("$NETP_ENVIRONMENT_URL/servers")
+    @AuthRequired
     suspend fun getServers(): List<RegisteredServerInfo>
 
     @GET("$NETP_ENVIRONMENT_URL/servers/{serverName}/status")
@@ -119,12 +120,14 @@ interface WgVpnControllerService {
     ): ServerStatus
 
     @Headers("Content-Type: application/json")
+    @AuthRequired
     @POST("$NETP_ENVIRONMENT_URL/register")
     suspend fun registerKey(
         @Body registerKeyBody: RegisterKeyBody,
     ): List<EligibleServerInfo>
 
     @GET("$NETP_ENVIRONMENT_URL/locations")
+    @AuthRequired
     suspend fun getEligibleLocations(): List<EligibleLocation>
 }
 
@@ -171,3 +174,10 @@ data class EligibleLocation(
 data class EligibleCity(
     val name: String,
 )
+
+/**
+ * This annotation is used in interceptors to be able to intercept the annotated service calls
+ */
+@Target(AnnotationTarget.FUNCTION)
+@Retention(AnnotationRetention.RUNTIME)
+annotation class AuthRequired

network-protection/network-protection-impl/src/test/java/com/duckduckgo/networkprotection/impl/configuration/NetpControllerRequestInterceptorTest.kt

@@ -2,14 +2,10 @@ package com.duckduckgo.networkprotection.impl.configuration
 
 import com.duckduckgo.appbuildconfig.api.AppBuildConfig
 import com.duckduckgo.appbuildconfig.api.BuildFlavor
-import com.duckduckgo.appbuildconfig.api.BuildFlavor.INTERNAL
-import com.duckduckgo.appbuildconfig.api.BuildFlavor.PLAY
 import com.duckduckgo.common.test.api.FakeChain
 import com.duckduckgo.subscriptions.api.Subscriptions
 import kotlinx.coroutines.flow.flowOf
 import kotlinx.coroutines.runBlocking
-import kotlinx.coroutines.test.runTest
-import org.junit.Assert
 import org.junit.Assert.assertFalse
 import org.junit.Assert.assertTrue
 import org.junit.Before
@@ -38,54 +34,43 @@ class NetpControllerRequestInterceptorTest {
     }
 
     @Test
-    fun whenInterceptNotNetPUrlThenDoNothing() {
-        val chain = FakeChain("https://this.is.not.the.url/servers")
+    fun `when @AuthRequired annotation is not present then authorization header not is added`() {
+        val nonAnnotatedMethod = FakeApiService::class.java.getMethod("endpointNotRequiringAuthentication")
 
-        val headers = interceptor.intercept(chain).headers
-
-        assertTrue(headers.size == 0)
-    }
-
-    @Test
-    fun whenInterceptServersCallThenAddAuthHeader() {
-        val chain = FakeChain("https://staging.netp.duckduckgo.com/servers")
-
-        val headers = interceptor.intercept(chain).headers
-
-        assertTrue(headers.size == 1)
-        assertFalse(headers.map { it.first }.any { it.contains("NetP-Debug-Code") })
-        assertTrue(headers.map { it.first }.any { it.contains("Authorization") })
-    }
-
-    @Test
-    fun whenInterceptRegisterCallThenAddAuthHeader() {
-        val chain = FakeChain("https://staging.netp.duckduckgo.com/register")
+        val chain = FakeChain(
+            url = "https://this.is.not.the.url/servers",
+            serviceMethod = nonAnnotatedMethod,
+        )
 
         val headers = interceptor.intercept(chain).headers
 
-        assertTrue(headers.size == 1)
-        assertFalse(headers.map { it.first }.any { it.contains("NetP-Debug-Code") })
-        assertTrue(headers.map { it.first }.any { it.contains("Authorization") })
+        assertTrue(headers.size == 0)
     }
 
     @Test
-    fun whenInterceptServersCallInternalBuildThenAddAuthAndDebugHeaders() {
+    fun `when @AuthRequired annotation is not present and internal build then authorization header not is added`() {
         whenever(appBuildConfig.flavor).thenReturn(BuildFlavor.INTERNAL)
 
-        val chain = FakeChain("https://staging.netp.duckduckgo.com/servers")
+        val nonAnnotatedMethod = FakeApiService::class.java.getMethod("endpointNotRequiringAuthentication")
+
+        val chain = FakeChain(
+            url = "https://this.is.not.the.url/servers",
+            serviceMethod = nonAnnotatedMethod,
+        )
 
         val headers = interceptor.intercept(chain).headers
 
-        assertTrue(headers.size == 2)
-        assertTrue(headers.map { it.first }.any { it.contains("NetP-Debug-Code") })
-        assertTrue(headers.map { it.first }.any { it.contains("Authorization") })
+        assertTrue(headers.size == 0)
     }
 
     @Test
-    fun whenInterceptRegisterCallInternalBuildThenAddAuthAndDebugHeaders() {
+    fun `when @AuthRequired annotation is present and internal then authorization header is added`() {
         whenever(appBuildConfig.flavor).thenReturn(BuildFlavor.INTERNAL)
-
-        val chain = FakeChain("https://staging.netp.duckduckgo.com/register")
+        val annotatedMethod = FakeApiService::class.java.getMethod("endpointRequiringAuthentication")
+        val chain = FakeChain(
+            url = "https://staging.netp.duckduckgo.com/servers",
+            serviceMethod = annotatedMethod,
+        )
 
         val headers = interceptor.intercept(chain).headers
 
@@ -95,23 +80,13 @@ class NetpControllerRequestInterceptorTest {
     }
 
     @Test
-    fun whenInterceptServersCallPlayBuildThenAddAuthHeader() {
-        whenever(appBuildConfig.flavor).thenReturn(BuildFlavor.PLAY)
-
-        val chain = FakeChain("https://staging.netp.duckduckgo.com/servers")
-
-        val headers = interceptor.intercept(chain).headers
-
-        assertTrue(headers.size == 1)
-        assertFalse(headers.map { it.first }.any { it.contains("NetP-Debug-Code") })
-        assertTrue(headers.map { it.first }.any { it.contains("Authorization") })
-    }
-
-    @Test
-    fun whenInterceptRegisterCallPlayBuildThenAddAuthHeader() {
+    fun `when @AuthRequired annotation is present and not internal then netp debug code not added`() {
         whenever(appBuildConfig.flavor).thenReturn(BuildFlavor.PLAY)
-
-        val chain = FakeChain("https://staging.netp.duckduckgo.com/register")
+        val annotatedMethod = FakeApiService::class.java.getMethod("endpointRequiringAuthentication")
+        val chain = FakeChain(
+            url = "https://staging.netp.duckduckgo.com/servers",
+            serviceMethod = annotatedMethod,
+        )
 
         val headers = interceptor.intercept(chain).headers
 
@@ -120,64 +95,9 @@ class NetpControllerRequestInterceptorTest {
         assertTrue(headers.map { it.first }.any { it.contains("Authorization") })
     }
 
-    // ----------------------------------------------------------------------------------------------------------------------------------
-    // Here starts the tests for the subscriptions
-    // ----------------------------------------------------------------------------------------------------------------------------------
-    @Test
-    fun whenUrlIsServersAndFlavorIsPlayThenOnlyAddTokenToHeader() = runTest {
-        val fakeChain = FakeChain(url = "https://controller.netp.duckduckgo.com/servers")
-        whenever(appBuildConfig.flavor).thenReturn(PLAY)
-        whenever(subscriptions.getAccessToken()).thenReturn("token123")
-
-        interceptor.intercept(fakeChain).run {
-            Assert.assertEquals("bearer ddg:token123", headers["Authorization"])
-            assertFalse(headers.names().contains("NetP-Debug-Code"))
-        }
-    }
-
-    @Test
-    fun whenUrlIsLocationsAndFlavorIsPlayThenOnlyAddTokenToHeader() = runTest {
-        val fakeChain = FakeChain(url = "https://controller.netp.duckduckgo.com/locations")
-        whenever(appBuildConfig.flavor).thenReturn(PLAY)
-        whenever(subscriptions.getAccessToken()).thenReturn("token123")
-
-        interceptor.intercept(fakeChain).run {
-            Assert.assertEquals("bearer ddg:token123", headers["Authorization"])
-            assertFalse(headers.names().contains("NetP-Debug-Code"))
-        }
-    }
-
-    @Test
-    fun whenUrlIsRegisterAndFlavorIsPlayThenOnlyAddTokenToHeader() = runTest {
-        val fakeChain = FakeChain(url = "https://staging1.netp.duckduckgo.com/register")
-        whenever(appBuildConfig.flavor).thenReturn(PLAY)
-        whenever(subscriptions.getAccessToken()).thenReturn("token123")
-
-        interceptor.intercept(fakeChain).run {
-            Assert.assertEquals("bearer ddg:token123", headers["Authorization"])
-            assertFalse(headers.names().contains("NetP-Debug-Code"))
-        }
-    }
-
-    @Test
-    fun whenUrlIsNotNetPAndFlavorIsInternalThenDoNothingWithHeaders() = runTest {
-        val fakeChain = FakeChain(url = "https://improving.duckduckgo.com/t/m_netp_ev_enabled_android_phone?atb=v336-7&appVersion=5.131.0&test=1")
-
-        interceptor.intercept(fakeChain).run {
-            Assert.assertNull(headers["Authorization"])
-            assertFalse(headers.names().contains("NetP-Debug-Code"))
-        }
-    }
-
-    @Test
-    fun whenUrlIsNetPAndFlavorIsInternalThenAddTokenAndDebugCodeToHeader() = runTest {
-        val fakeChain = FakeChain(url = "https://controller.netp.duckduckgo.com/servers")
-        whenever(appBuildConfig.flavor).thenReturn(INTERNAL)
-        whenever(subscriptions.getAccessToken()).thenReturn("token123")
-
-        interceptor.intercept(fakeChain).run {
-            Assert.assertEquals("bearer ddg:token123", headers["Authorization"])
-            assertTrue(headers.names().contains("NetP-Debug-Code"))
-        }
+    private interface FakeApiService {
+        @AuthRequired
+        fun endpointRequiringAuthentication()
+        fun endpointNotRequiringAuthentication()
     }
 }