Fixed Android agent to send the issued certificate's serial number to Fleet as hexadecimal (#45413)
<!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> **Related issue:** Resolves #45405 The serial numbers are currently unused (they are only for debug), so we are making a hard switch to hex without a migration. # Checklist for submitter - [x] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. ## 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 ## Release Notes * **Bug Fixes** * Fixed Android agent certificate serial number reporting to use hexadecimal format for proper Fleet compatibility. * **Tests** * Added test coverage to ensure certificate serial numbers are correctly encoded as hexadecimal when reporting status. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/fleetdm/fleet/pull/45413) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -358,7 +358,7 @@ object ApiClient : CertificateApiClient {
|
||||
detail = detail,
|
||||
notAfter = notAfter?.toISO8601String(),
|
||||
notBefore = notBefore?.toISO8601String(),
|
||||
serialNumber = serialNumber?.toString(),
|
||||
serialNumber = serialNumber?.toString(16), // hex
|
||||
),
|
||||
bodySerializer = UpdateCertificateStatusRequest.serializer(),
|
||||
responseSerializer = UpdateCertificateStatusResponse.serializer(),
|
||||
|
||||
@@ -628,7 +628,7 @@ class CertificateOrchestrator(
|
||||
operationType = operationType,
|
||||
notAfter = state.notAfter?.let { parseISO8601(it) },
|
||||
notBefore = state.notBefore?.let { parseISO8601(it) },
|
||||
serialNumber = state.serialNumber?.let { BigInteger(it) },
|
||||
serialNumber = state.serialNumber?.let { BigInteger(it, 16) },
|
||||
)
|
||||
|
||||
if (result.isSuccess) {
|
||||
@@ -751,7 +751,7 @@ class CertificateOrchestrator(
|
||||
// Convert certificate metadata to ISO8601 for storage
|
||||
val notAfterStr = result.notAfter?.toISO8601String()
|
||||
val notBeforeStr = result.notBefore?.toISO8601String()
|
||||
val serialNumberStr = result.serialNumber?.toString()
|
||||
val serialNumberStr = result.serialNumber?.toString(16) // hex
|
||||
|
||||
// First, mark as unreported (persisted before network call)
|
||||
markCertificateUnreported(
|
||||
|
||||
@@ -15,6 +15,7 @@ import org.junit.runner.RunWith
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
import org.robolectric.RuntimeEnvironment
|
||||
import org.robolectric.annotation.Config
|
||||
import java.math.BigInteger
|
||||
import kotlinx.coroutines.test.runTest
|
||||
|
||||
/**
|
||||
@@ -231,6 +232,42 @@ class ApiClientReenrollTest {
|
||||
assertEquals(1, mockWebServer.requestCount - initialRequestCount)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `updateCertificateStatus serializes serial as lowercase hex`() = runTest {
|
||||
enqueueEnrollmentSuccess("test-node-key")
|
||||
mockWebServer.enqueue(MockResponse().setResponseCode(200).setBody("{}"))
|
||||
|
||||
val serial = BigInteger("ff737b63e4516749aea89c59b26fae3d0dd8c48d", 16)
|
||||
val expectedHex = serial.toString(16)
|
||||
val decimalForm = serial.toString(10)
|
||||
|
||||
val result = ApiClient.updateCertificateStatus(
|
||||
certificateId = 42,
|
||||
status = UpdateCertificateStatusStatus.VERIFIED,
|
||||
operationType = UpdateCertificateStatusOperation.INSTALL,
|
||||
serialNumber = serial,
|
||||
)
|
||||
|
||||
assertTrue("updateCertificateStatus should succeed: ${result.exceptionOrNull()}", result.isSuccess)
|
||||
|
||||
mockWebServer.takeRequest() // enrollment
|
||||
val statusReq = mockWebServer.takeRequest()
|
||||
val body = statusReq.body.readUtf8()
|
||||
|
||||
assertTrue(
|
||||
"Expected status update at /api/fleetd/certificates/42/status, got: ${statusReq.path}",
|
||||
statusReq.path?.endsWith("/api/fleetd/certificates/42/status") == true,
|
||||
)
|
||||
assertTrue(
|
||||
"Expected serial as hex \"$expectedHex\" in body, got: $body",
|
||||
body.contains("\"serial\":\"$expectedHex\""),
|
||||
)
|
||||
assertTrue(
|
||||
"Body should not contain the decimal-format serial \"$decimalForm\", got: $body",
|
||||
!body.contains("\"serial\":\"$decimalForm\""),
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `re-enrollment failure propagates error`() = runTest {
|
||||
// Establish initial enrollment
|
||||
|
||||
@@ -8,7 +8,6 @@ import com.fleetdm.agent.testutil.FakeCertificateApiClient
|
||||
import com.fleetdm.agent.testutil.FakeDeviceKeystoreManager
|
||||
import com.fleetdm.agent.testutil.MockCertificateInstaller
|
||||
import com.fleetdm.agent.testutil.TestCertificateTemplateFactory
|
||||
import com.fleetdm.agent.testutil.UpdateStatusCall
|
||||
import org.junit.After
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertFalse
|
||||
@@ -21,6 +20,7 @@ import org.junit.runner.RunWith
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
import org.robolectric.RuntimeEnvironment
|
||||
import org.robolectric.annotation.Config
|
||||
import java.math.BigInteger
|
||||
import kotlinx.coroutines.flow.first
|
||||
import kotlinx.coroutines.launch
|
||||
import kotlinx.coroutines.test.runTest
|
||||
@@ -97,13 +97,21 @@ class CertificateOrchestratorTest {
|
||||
retries: Int = 0,
|
||||
statusReportRetries: Int = 0,
|
||||
uuid: String = "uuid-1",
|
||||
serialNumber: String? = null,
|
||||
) {
|
||||
context.prefDataStore.edit { preferences ->
|
||||
val existing = preferences[stringPreferencesKey("installed_certificates")]?.let {
|
||||
json.decodeFromString<CertificateStateMap>(it)
|
||||
} ?: emptyMap()
|
||||
|
||||
val certInfo = CertificateState(alias, status, retries, statusReportRetries, uuid)
|
||||
val certInfo = CertificateState(
|
||||
alias = alias,
|
||||
status = status,
|
||||
retries = retries,
|
||||
statusReportRetries = statusReportRetries,
|
||||
uuid = uuid,
|
||||
serialNumber = serialNumber,
|
||||
)
|
||||
val updated = existing.toMutableMap().apply {
|
||||
put(certificateId, certInfo)
|
||||
}
|
||||
@@ -1273,6 +1281,26 @@ class CertificateOrchestratorTest {
|
||||
assertEquals(2, fakeApiClient.updateStatusCalls.size)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `retryUnreportedStatuses parses stored serial as hex when reporting to server`() = runTest {
|
||||
storeTestCertificateInDataStore(
|
||||
certificateId = 1,
|
||||
alias = "test-cert",
|
||||
status = CertificateStatus.INSTALLED_UNREPORTED,
|
||||
serialNumber = "a1b2c3",
|
||||
)
|
||||
|
||||
val results = orchestrator.retryUnreportedStatuses(context)
|
||||
|
||||
assertEquals(mapOf(1 to true), results)
|
||||
assertEquals(1, fakeApiClient.updateStatusCalls.size)
|
||||
assertEquals(
|
||||
"Stored hex serial 'a1b2c3' should parse to BigInteger 0xa1b2c3",
|
||||
BigInteger.valueOf(0xa1b2c3L),
|
||||
fakeApiClient.updateStatusCalls[0].serialNumber,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `retryUnreportedStatuses handles mixed success and failure`() = runTest {
|
||||
storeTestCertificateInDataStore(1, "cert-1", CertificateStatus.INSTALLED_UNREPORTED)
|
||||
|
||||
@@ -0,0 +1 @@
|
||||
- Fixed Android agent to send the issued certificate's serial number to Fleet as hexadecimal.
|
||||
Reference in New Issue
Block a user