Fixed certificate template fetch failing with DNS errors (and other issues) (#42625)

<!-- Add the related story/sub-task/bug number, like Resolves #123, or
remove if NA -->
**Related issue:** Resolves #42624
**Related issue:** Resolves #37546

- Fixed certificate template fetch failing with DNS errors (known
Android issue)
- stop polling certs that failed permanently
- CertificateOrchestrator: When server returns template status "failed",
mark the certificate as locally failed (markCertificateForceFailed) and
stop polling
- CertificateOrchestrator: Non-retryable SCEP failures (e.g.
ScepEnrollmentException) now immediately mark as failed and report to
server, skipping the 3-attempt retry logic
- CertificateOrchestrator: recordEnrollmentAttemptFailure now stores the
uuid, fixing a bug where the FAILED guard was bypassed because stored
uuid was empty
- CertificateOrchestrator: Renamed markCertificateFailure to
recordEnrollmentAttemptFailure and added markCertificateForceFailed for
clarity

# Checklist for submitter

- [x] Changes file added for user-visible changes in `changes/`,
`orbit/changes/` or `ee/fleetd-chrome/changes`.

## Testing

- [x] Added/updated automated tests
- [x] QA'd all new/changed functionality manually

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
* Fixed certificate template retrieval failures that displayed
misleading DNS errors. Optimized HTTP request header handling for GET
requests to prevent these errors during certificate enrollment
operations.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

android/app/detekt.yml

@@ -13,8 +13,7 @@ style:
   MaxLineLength:
     active: false # Handled by ktlint
   ReturnCount:
-    active: true
-    max: 5
+    active: false
 
 complexity:
   CognitiveComplexMethod:

android/app/src/main/java/com/fleetdm/agent/ApiClient.kt

@@ -1,6 +1,7 @@
 package com.fleetdm.agent
 
 import android.content.Context
+import android.net.ConnectivityManager
 import android.util.Log
 import androidx.datastore.core.DataStore
 import androidx.datastore.preferences.core.Preferences
@@ -10,10 +11,7 @@ import androidx.datastore.preferences.preferencesDataStore
 import java.math.BigInteger
 import java.net.HttpURLConnection
 import java.net.URL
-import java.text.SimpleDateFormat
 import java.util.Date
-import java.util.Locale
-import java.util.TimeZone
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.first
@@ -64,6 +62,7 @@ object ApiClient : CertificateApiClient {
     private val json = Json { ignoreUnknownKeys = true }
 
     private lateinit var dataStore: DataStore<Preferences>
+    private lateinit var appContext: Context
     private val API_KEY = stringPreferencesKey("api_key")
     private val SERVER_URL_KEY = stringPreferencesKey("server_url")
     private val ENROLL_SECRET = stringPreferencesKey("enroll_secret")
@@ -74,8 +73,9 @@ object ApiClient : CertificateApiClient {
 
     fun initialize(context: Context) {
         Log.d(TAG, "initializing api client")
+        appContext = context.applicationContext
         if (!::dataStore.isInitialized) {
-            dataStore = context.applicationContext.prefDataStore
+            dataStore = appContext.prefDataStore
         }
     }
 
@@ -116,6 +116,8 @@ object ApiClient : CertificateApiClient {
         responseSerializer: KSerializer<T>,
         authorized: Boolean = true,
     ): Result<T> = withContext(Dispatchers.IO) {
+        require(method != "GET" || body == null) { "GET requests must not include a body" }
+
         var connection: HttpURLConnection? = null
         try {
             val baseUrl = getBaseUrl() ?: return@withContext Result.failure(
@@ -137,13 +139,12 @@ object ApiClient : CertificateApiClient {
             }
 
             val url = URL("$baseUrl$endpoint")
-            connection = url.openConnection() as HttpURLConnection
+            connection = openConnectionOnActiveNetwork(url)
 
             connection.apply {
                 requestMethod = method
                 useCaches = false
                 doInput = true
-                setRequestProperty("Content-Type", "application/json")
                 if (authorized) {
                     getNodeKeyOrEnroll().fold(
                         onFailure = { throwable -> return@withContext Result.failure(throwable) },
@@ -155,8 +156,9 @@ object ApiClient : CertificateApiClient {
                 connectTimeout = 15000
                 readTimeout = 15000
 
-                if (body != null && method != "GET") {
+                if (body != null) {
                     requireNotNull(bodySerializer) { "bodySerializer required when body is provided" }
+                    setRequestProperty("Content-Type", "application/json")
                     doOutput = true
                     val bodyJson = json.encodeToString(value = body, serializer = bodySerializer)
                     outputStream.use { it.write(bodyJson.toByteArray()) }
@@ -178,6 +180,8 @@ object ApiClient : CertificateApiClient {
                 Result.success(parsed)
             } else if (responseCode == 401) {
                 Result.failure(UnauthorizedException(response))
+            } else if (responseCode == 404) {
+                Result.failure(NotFoundException(response))
             } else {
                 Result.failure(Exception("HTTP $responseCode: $response"))
             }
@@ -193,6 +197,26 @@ object ApiClient : CertificateApiClient {
      * This typically indicates the node key has been invalidated (e.g., host was deleted).
      */
     class UnauthorizedException(message: String) : Exception("HTTP 401: $message")
+    class NotFoundException(message: String) : Exception("HTTP 404: $message")
+
+    /**
+     * Opens an HTTP connection bound to the active network when available. This ensures DNS resolution uses
+     * the active network's DNS servers, avoiding failures when Android reports connectivity before DNS is ready.
+     * Falls back to a default connection if no active network is available.
+     */
+    internal fun openConnectionOnActiveNetwork(url: URL): HttpURLConnection {
+        if (useActiveNetworkBinding) {
+            val connectivityManager = appContext.getSystemService(ConnectivityManager::class.java)
+            val activeNetwork = connectivityManager?.activeNetwork
+            if (activeNetwork != null) {
+                return activeNetwork.openConnection(url) as HttpURLConnection
+            }
+        }
+        return url.openConnection() as HttpURLConnection
+    }
+
+    // Disabled in tests where Network.openConnection is not available (Robolectric)
+    internal var useActiveNetworkBinding = true
 
     /**
      * Executes a request block with automatic re-enrollment on 401 Unauthorized.
@@ -262,19 +286,12 @@ object ApiClient : CertificateApiClient {
     }
 
     override suspend fun getCertificateTemplate(certificateId: Int): Result<CertificateTemplateResult> = withReenrollOnUnauthorized {
-        val nodeKeyResult = getNodeKeyOrEnroll()
-        val orbitNodeKey = nodeKeyResult.getOrElse { error ->
-            return@withReenrollOnUnauthorized Result.failure(error)
-        }
-
         val credentials = getEnrollmentCredentials()
             ?: return@withReenrollOnUnauthorized Result.failure(Exception("enroll credentials not set"))
 
-        makeRequest(
+        makeRequest<Unit, GetCertificateTemplateResponseWrapper>(
             endpoint = "/api/fleetd/certificates/$certificateId",
             method = "GET",
-            body = GetCertificateTemplateRequest(orbitNodeKey = orbitNodeKey),
-            bodySerializer = GetCertificateTemplateRequest.serializer(),
             responseSerializer = GetCertificateTemplateResponseWrapper.serializer(),
         ).fold(
             onSuccess = { wrapper ->
@@ -326,8 +343,14 @@ object ApiClient : CertificateApiClient {
                 }
             },
             onFailure = { throwable ->
-                FleetLog.e(TAG, "failed to update certificate status $certificateId: ${throwable.message}")
-                Result.failure(throwable)
+                if (throwable is NotFoundException) {
+                    // Certificate template was deleted from the server -- nothing to report to
+                    Log.i(TAG, "certificate template $certificateId no longer exists on server, nothing to report")
+                    Result.success(Unit)
+                } else {
+                    FleetLog.e(TAG, "failed to update certificate status $certificateId: ${throwable.message}")
+                    Result.failure(throwable)
+                }
             },
         )
     }
@@ -481,12 +504,6 @@ data class OrbitUpdateChannels(
     val desktop: String = "",
 )
 
-@Serializable
-private data class GetCertificateTemplateRequest(
-    @SerialName("orbit_node_key")
-    val orbitNodeKey: String,
-)
-
 @Serializable
 data class UpdateCertificateStatusRequest(
     @SerialName("status")

android/app/src/main/java/com/fleetdm/agent/CertificateOrchestrator.kt

@@ -150,16 +150,29 @@ class CertificateOrchestrator(
         storeCertificateState(context = context, certificateId = certificateId, certInstallInfo = newInfo)
     }
 
-    internal suspend fun markCertificateFailure(context: Context, certificateId: Int, alias: String): CertificateState {
+    internal suspend fun markCertificateForceFailed(context: Context, certificateId: Int, alias: String, uuid: String) {
         val existingInfo = getCertificateState(context = context, certificateId = certificateId)
-            ?: CertificateState(alias = alias, status = CertificateStatus.RETRY, retries = 0)
+            ?: CertificateState(alias = alias, status = CertificateStatus.FAILED, retries = 0, uuid = uuid)
+
+        val newInfo = existingInfo.copy(alias = alias, status = CertificateStatus.FAILED, uuid = uuid)
+        storeCertificateState(context = context, certificateId = certificateId, certInstallInfo = newInfo)
+    }
+
+    internal suspend fun recordEnrollmentAttemptFailure(
+        context: Context,
+        certificateId: Int,
+        alias: String,
+        uuid: String,
+    ): CertificateState {
+        val existingInfo = getCertificateState(context = context, certificateId = certificateId)
+            ?: CertificateState(alias = alias, status = CertificateStatus.RETRY, retries = 0, uuid = uuid)
 
         if (existingInfo.status != CertificateStatus.RETRY) {
-            Log.d(TAG, "markCertificateFailure: skipping cert $certificateId, status is ${existingInfo.status}")
+            Log.d(TAG, "recordEnrollmentAttemptFailure: skipping cert $certificateId, status is ${existingInfo.status}")
             return existingInfo
         }
 
-        var newInfo = existingInfo.copy(retries = existingInfo.retries + 1)
+        var newInfo = existingInfo.copy(retries = existingInfo.retries + 1, uuid = uuid)
 
         if (newInfo.retries >= MAX_CERT_INSTALL_RETRIES) {
             newInfo = newInfo.copy(status = CertificateStatus.FAILED)
@@ -701,9 +714,15 @@ class CertificateOrchestrator(
 
         Log.d(TAG, "Successfully fetched certificate template: ${template.name}")
 
+        if (template.status == "failed") {
+            // Server says this certificate is permanently failed. Mark it locally as failed so we stop polling
+            Log.i(TAG, "Certificate template ${template.name} has terminal status \"failed\", marking locally as failed")
+            markCertificateForceFailed(context, certificateId, template.name, uuid)
+            return CertificateEnrollmentHandler.EnrollmentResult.PermanentlyFailed(template.name)
+        }
+
         if (template.status != "delivered") {
-            // The certificate template hasn't failed on the device, but isn't ready to be processed yet.
-            // Retry next time we fetch but don't mark as failed locally
+            // Not ready to be processed yet (e.g. pending, delivering). Retry next time.
             Log.i(TAG, "Certificate template ${template.name} does not have status \"delivered\": status \"${template.status}\"")
             return CertificateEnrollmentHandler.EnrollmentResult.Success(
                 alias = template.name,
@@ -769,8 +788,20 @@ class CertificateOrchestrator(
                 }
             }
             is CertificateEnrollmentHandler.EnrollmentResult.Failure -> {
-                val updatedInfo = markCertificateFailure(context = context, certificateId = certificateId, alias = template.name)
-                if (!updatedInfo.shouldRetry()) {
+                val shouldReportFailure = if (result.isRetryable) {
+                    val updatedInfo = recordEnrollmentAttemptFailure(
+                        context = context,
+                        certificateId = certificateId,
+                        alias = template.name,
+                        uuid = uuid,
+                    )
+                    !updatedInfo.shouldRetry()
+                } else {
+                    markCertificateForceFailed(context, certificateId, template.name, uuid)
+                    true
+                }
+
+                if (shouldReportFailure) {
                     FleetLog.e(TAG, "Certificate enrollment failed for ID $certificateId: ${result.reason}", result.exception)
                     apiClient.updateCertificateStatus(
                         certificateId = certificateId,

android/app/src/main/java/com/fleetdm/agent/scep/ScepClientImpl.kt

@@ -116,9 +116,14 @@ class ScepClientImpl : ScepClient {
                         "Enrollment is pending - requires CA administrator approval",
                     )
                 }
+                response.isFailure -> {
+                    throw ScepEnrollmentException(
+                        "Enrollment failed with SCEP failInfo: ${response.failInfo}",
+                    )
+                }
                 else -> {
                     throw ScepEnrollmentException(
-                        "Enrollment failed - certificate not issued by SCEP server",
+                        "Enrollment failed - unexpected SCEP response",
                     )
                 }
             }

android/app/src/test/java/com/fleetdm/agent/ApiClientReenrollTest.kt

@@ -7,6 +7,7 @@ import androidx.datastore.preferences.core.edit
 import androidx.datastore.preferences.core.stringPreferencesKey
 import org.junit.After
 import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNull
 import org.junit.Assert.assertTrue
 import org.junit.Before
 import org.junit.Test
@@ -42,6 +43,7 @@ class ApiClientReenrollTest {
         mockWebServer.start()
 
         ApiClient.initialize(context)
+        ApiClient.useActiveNetworkBinding = false
         clearDataStore()
 
         // Set up enrollment credentials pointing to mock server
@@ -187,6 +189,14 @@ class ApiClientReenrollTest {
             "Expected second-node-key in Authorization header",
             retryRequest.getHeader("Authorization")?.contains("second-node-key") == true,
         )
+
+        // GET requests should not send Content-Type since they have no body.
+        // A Content-Type header on a bodyless GET can cause intermediaries (proxies, CDNs)
+        // to reject the request, which surfaces as misleading DNS errors on Android.
+        assertNull(
+            "GET request should not have Content-Type header",
+            retryRequest.getHeader("Content-Type"),
+        )
     }
 
     @Test

android/app/src/test/java/com/fleetdm/agent/CertificateOrchestratorTest.kt

@@ -1291,4 +1291,116 @@ class CertificateOrchestratorTest {
         assertEquals(CertificateStatus.INSTALLED, getStoredCertificates()[1]?.status)
         assertEquals(CertificateStatus.REMOVED_UNREPORTED, getStoredCertificates()[2]?.status)
     }
+
+    @Test
+    fun `enrollCertificate marks locally failed when server template status is failed`() = runTest {
+        val certificateId = 42
+        val uuid = "test-uuid"
+
+        // Arrange: configure API to return a template with status "failed"
+        fakeApiClient.getCertificateTemplateHandler = { certId ->
+            Result.success(
+                CertificateTemplateResult(
+                    template = TestCertificateTemplateFactory.create(
+                        id = certId,
+                        name = "cert-failed",
+                        status = "failed",
+                    ),
+                    scepUrl = TestCertificateTemplateFactory.DEFAULT_SCEP_URL,
+                ),
+            )
+        }
+
+        // Act
+        val result = orchestrator.enrollCertificate(context, certificateId, uuid, mockInstaller)
+
+        // Assert: returns PermanentlyFailed
+        assertTrue(
+            "Expected PermanentlyFailed but got: $result",
+            result is CertificateEnrollmentHandler.EnrollmentResult.PermanentlyFailed,
+        )
+
+        // Assert: local state is FAILED so future runs skip this certificate
+        val stored = getStoredCertificates()
+        assertEquals(CertificateStatus.FAILED, stored[certificateId]?.status)
+        assertEquals(uuid, stored[certificateId]?.uuid)
+
+        // Assert: no SCEP enrollment was attempted
+        assertNull("SCEP client should not have been called", mockScepClient.capturedConfig)
+
+        // Assert: no status update sent to server (server already knows it failed)
+        assertTrue("No status update should be sent", fakeApiClient.updateStatusCalls.isEmpty())
+    }
+
+    @Test
+    fun `non-retryable SCEP failure immediately marks FAILED and reports to server`() = runTest {
+        val certificateId = 50
+        val uuid = "test-uuid"
+
+        // Arrange: SCEP enrollment will throw ScepEnrollmentException (non-retryable)
+        mockScepClient.shouldThrowEnrollmentException = true
+        fakeApiClient.getCertificateTemplateHandler = { certId ->
+            Result.success(
+                CertificateTemplateResult(
+                    template = TestCertificateTemplateFactory.create(id = certId, name = "cert-nonretry", status = "delivered"),
+                    scepUrl = TestCertificateTemplateFactory.DEFAULT_SCEP_URL,
+                ),
+            )
+        }
+
+        // Act
+        val result = orchestrator.enrollCertificate(context, certificateId, uuid, mockInstaller)
+
+        // Assert: returns Failure (non-retryable)
+        assertTrue("Expected Failure but got: $result", result is CertificateEnrollmentHandler.EnrollmentResult.Failure)
+        assertFalse((result as CertificateEnrollmentHandler.EnrollmentResult.Failure).isRetryable)
+
+        // Assert: immediately marked FAILED locally (no retry attempts)
+        val stored = getStoredCertificates()
+        assertEquals(CertificateStatus.FAILED, stored[certificateId]?.status)
+        assertEquals(uuid, stored[certificateId]?.uuid)
+        assertEquals(0, stored[certificateId]?.retries)
+
+        // Assert: reported FAILED to server exactly once
+        assertEquals(1, fakeApiClient.updateStatusCalls.size)
+        assertEquals(UpdateCertificateStatusStatus.FAILED, fakeApiClient.updateStatusCalls[0].status)
+    }
+
+    @Test
+    fun `retryable SCEP failure increments retries and does not report until max retries`() = runTest {
+        val certificateId = 51
+        val uuid = "test-uuid"
+
+        // Arrange: SCEP enrollment will throw ScepNetworkException (retryable)
+        mockScepClient.shouldThrowNetworkException = true
+        fakeApiClient.getCertificateTemplateHandler = { certId ->
+            Result.success(
+                CertificateTemplateResult(
+                    template = TestCertificateTemplateFactory.create(id = certId, name = "cert-retry", status = "delivered"),
+                    scepUrl = TestCertificateTemplateFactory.DEFAULT_SCEP_URL,
+                ),
+            )
+        }
+
+        // Act: first attempt
+        orchestrator.enrollCertificate(context, certificateId, uuid, mockInstaller)
+
+        // Assert: status is RETRY, not FAILED
+        var stored = getStoredCertificates()
+        assertEquals(CertificateStatus.RETRY, stored[certificateId]?.status)
+        assertEquals(1, stored[certificateId]?.retries)
+        assertEquals(uuid, stored[certificateId]?.uuid)
+        assertTrue("Should not report FAILED yet", fakeApiClient.updateStatusCalls.isEmpty())
+
+        // Act: exhaust remaining retries
+        repeat(MAX_CERT_INSTALL_RETRIES - 1) {
+            orchestrator.enrollCertificate(context, certificateId, uuid, mockInstaller)
+        }
+
+        // Assert: now FAILED and reported to server exactly once
+        stored = getStoredCertificates()
+        assertEquals(CertificateStatus.FAILED, stored[certificateId]?.status)
+        assertEquals(1, fakeApiClient.updateStatusCalls.size)
+        assertEquals(UpdateCertificateStatusStatus.FAILED, fakeApiClient.updateStatusCalls[0].status)
+    }
 }

android/changes/42624-fix-certificate-template-get-content-type

@@ -0,0 +1,6 @@
+- Fixed background DNS resolution failures.
+- Stop polling certificates when the server reports them as permanently failed.
+- Non-retryable SCEP failures (e.g. server rejection) now immediately mark the certificate as failed instead of retrying 3 times.
+- Fixed duplicate FAILED status reports.
+- Treat HTTP 404 on certificate status updates as success when the template has been deleted server-side.
+- Include SCEP `failInfo` details in enrollment failure messages instead of a generic error.