From f97284cf13990f19e5e92f401b368ab80a5cd31d Mon Sep 17 00:00:00 2001 From: Tim Lee Date: Fri, 19 Dec 2025 16:35:58 -0700 Subject: [PATCH] Fix: filter out verified remove android certs in host details (#37539) --- .../mysql/host_certificate_templates.go | 12 ++++ .../mysql/host_certificate_templates_test.go | 58 +++++++++++++++++++ server/fleet/datastore.go | 3 + server/mock/datastore_mock.go | 12 ++++ server/service/certificates.go | 5 ++ 5 files changed, 90 insertions(+) diff --git a/server/datastore/mysql/host_certificate_templates.go b/server/datastore/mysql/host_certificate_templates.go index df706993be..32198a68e2 100644 --- a/server/datastore/mysql/host_certificate_templates.go +++ b/server/datastore/mysql/host_certificate_templates.go @@ -206,6 +206,18 @@ func (ds *Datastore) DeleteHostCertificateTemplates(ctx context.Context, hostCer return nil } +// DeleteHostCertificateTemplate deletes a single host_certificate_template record +// identified by host_uuid and certificate_template_id. +func (ds *Datastore) DeleteHostCertificateTemplate(ctx context.Context, hostUUID string, certificateTemplateID uint) error { + const stmt = `DELETE FROM host_certificate_templates WHERE host_uuid = ? AND certificate_template_id = ?` + + if _, err := ds.writer(ctx).ExecContext(ctx, stmt, hostUUID, certificateTemplateID); err != nil { + return ctxerr.Wrap(ctx, err, "delete host_certificate_template") + } + + return nil +} + func (ds *Datastore) UpsertCertificateStatus( ctx context.Context, hostUUID string, diff --git a/server/datastore/mysql/host_certificate_templates_test.go b/server/datastore/mysql/host_certificate_templates_test.go index 99992b534c..ab6ec20199 100644 --- a/server/datastore/mysql/host_certificate_templates_test.go +++ b/server/datastore/mysql/host_certificate_templates_test.go @@ -24,6 +24,7 @@ func TestHostCertificateTemplates(t *testing.T) { {"ListCertificateTemplatesForHosts", testListCertificateTemplatesForHosts}, {"GetCertificateTemplateForHostNoTeam", testGetCertificateTemplateForHostNoTeam}, {"BulkInsertAndDeleteHostCertificateTemplates", testBulkInsertAndDeleteHostCertificateTemplates}, + {"DeleteHostCertificateTemplate", testDeleteHostCertificateTemplate}, {"UpsertHostCertificateTemplateStatus", testUpsertHostCertificateTemplateStatus}, {"CreatePendingCertificateTemplatesForExistingHosts", testCreatePendingCertificateTemplatesForExistingHosts}, {"CreatePendingCertificateTemplatesForNewHost", testCreatePendingCertificateTemplatesForNewHost}, @@ -475,6 +476,63 @@ func testBulkInsertAndDeleteHostCertificateTemplates(t *testing.T, ds *Datastore }) } +func testDeleteHostCertificateTemplate(t *testing.T, ds *Datastore) { + ctx := t.Context() + + t.Run("no error when deleting non-existent record", func(t *testing.T) { + defer TruncateTables(t, ds) + + err := ds.DeleteHostCertificateTemplate(ctx, "non-existent-host", 999) + require.NoError(t, err) + }) + + t.Run("only deletes specified record", func(t *testing.T) { + defer TruncateTables(t, ds) + setup := createCertTemplateTestSetup(t, ctx, ds, "") + + templateTwo, err := ds.CreateCertificateTemplate(ctx, &fleet.CertificateTemplate{ + Name: "Cert2", + TeamID: setup.team.ID, + CertificateAuthorityID: setup.ca.ID, + SubjectName: "CN=Test Subject 2", + }) + require.NoError(t, err) + + // Insert multiple host certificate template records + hostCerts := []fleet.HostCertificateTemplate{ + {HostUUID: "host-1", CertificateTemplateID: setup.template.ID, FleetChallenge: ptr.String("challenge-1"), Status: fleet.CertificateTemplateVerified, OperationType: fleet.MDMOperationTypeRemove, Name: setup.template.Name}, + {HostUUID: "host-1", CertificateTemplateID: templateTwo.ID, FleetChallenge: ptr.String("challenge-2"), Status: fleet.CertificateTemplateVerified, OperationType: fleet.MDMOperationTypeInstall, Name: templateTwo.Name}, + {HostUUID: "host-2", CertificateTemplateID: setup.template.ID, FleetChallenge: ptr.String("challenge-3"), Status: fleet.CertificateTemplateVerified, OperationType: fleet.MDMOperationTypeRemove, Name: setup.template.Name}, + } + err = ds.BulkInsertHostCertificateTemplates(ctx, hostCerts) + require.NoError(t, err) + + // Delete only host-1's first certificate + err = ds.DeleteHostCertificateTemplate(ctx, "host-1", setup.template.ID) + require.NoError(t, err) + + // Verify only 2 records remain + var count int + err = ds.writer(ctx).GetContext(ctx, &count, "SELECT COUNT(*) FROM host_certificate_templates") + require.NoError(t, err) + require.Equal(t, 2, count) + + // Verify the correct records remain + var remaining []struct { + HostUUID string `db:"host_uuid"` + CertificateTemplateID uint `db:"certificate_template_id"` + } + err = ds.writer(ctx).SelectContext(ctx, &remaining, + "SELECT host_uuid, certificate_template_id FROM host_certificate_templates ORDER BY host_uuid, certificate_template_id") + require.NoError(t, err) + require.Len(t, remaining, 2) + require.Equal(t, "host-1", remaining[0].HostUUID) + require.Equal(t, templateTwo.ID, remaining[0].CertificateTemplateID) + require.Equal(t, "host-2", remaining[1].HostUUID) + require.Equal(t, setup.template.ID, remaining[1].CertificateTemplateID) + }) +} + func testUpsertHostCertificateTemplateStatus(t *testing.T, ds *Datastore) { ctx := t.Context() diff --git a/server/fleet/datastore.go b/server/fleet/datastore.go index b5538d9172..5b7ca327e2 100644 --- a/server/fleet/datastore.go +++ b/server/fleet/datastore.go @@ -2576,6 +2576,9 @@ type Datastore interface { // DeleteHostCertificateTemplates deletes specific host_certificate_templates records // identified by (host_uuid, certificate_template_id) pairs. DeleteHostCertificateTemplates(ctx context.Context, hostCertTemplates []HostCertificateTemplate) error + // DeleteHostCertificateTemplate deletes a single host_certificate_template record + // identified by host_uuid and certificate_template_id. + DeleteHostCertificateTemplate(ctx context.Context, hostUUID string, certificateTemplateID uint) error // ListAndroidHostUUIDsWithPendingCertificateTemplates returns hosts that have // certificate templates in 'pending' status ready for delivery. diff --git a/server/mock/datastore_mock.go b/server/mock/datastore_mock.go index a068cc7121..38ff00fe1a 100644 --- a/server/mock/datastore_mock.go +++ b/server/mock/datastore_mock.go @@ -1687,6 +1687,8 @@ type BulkInsertHostCertificateTemplatesFunc func(ctx context.Context, hostCertTe type DeleteHostCertificateTemplatesFunc func(ctx context.Context, hostCertTemplates []fleet.HostCertificateTemplate) error +type DeleteHostCertificateTemplateFunc func(ctx context.Context, hostUUID string, certificateTemplateID uint) error + type ListAndroidHostUUIDsWithPendingCertificateTemplatesFunc func(ctx context.Context, offset int, limit int) ([]string, error) type GetAndTransitionCertificateTemplatesToDeliveringFunc func(ctx context.Context, hostUUID string) (*fleet.HostCertificateTemplatesForDelivery, error) @@ -4202,6 +4204,9 @@ type DataStore struct { DeleteHostCertificateTemplatesFunc DeleteHostCertificateTemplatesFunc DeleteHostCertificateTemplatesFuncInvoked bool + DeleteHostCertificateTemplateFunc DeleteHostCertificateTemplateFunc + DeleteHostCertificateTemplateFuncInvoked bool + ListAndroidHostUUIDsWithPendingCertificateTemplatesFunc ListAndroidHostUUIDsWithPendingCertificateTemplatesFunc ListAndroidHostUUIDsWithPendingCertificateTemplatesFuncInvoked bool @@ -10056,6 +10061,13 @@ func (s *DataStore) DeleteHostCertificateTemplates(ctx context.Context, hostCert return s.DeleteHostCertificateTemplatesFunc(ctx, hostCertTemplates) } +func (s *DataStore) DeleteHostCertificateTemplate(ctx context.Context, hostUUID string, certificateTemplateID uint) error { + s.mu.Lock() + s.DeleteHostCertificateTemplateFuncInvoked = true + s.mu.Unlock() + return s.DeleteHostCertificateTemplateFunc(ctx, hostUUID, certificateTemplateID) +} + func (s *DataStore) ListAndroidHostUUIDsWithPendingCertificateTemplates(ctx context.Context, offset int, limit int) ([]string, error) { s.mu.Lock() s.ListAndroidHostUUIDsWithPendingCertificateTemplatesFuncInvoked = true diff --git a/server/service/certificates.go b/server/service/certificates.go index a75af609d8..458b79c188 100644 --- a/server/service/certificates.go +++ b/server/service/certificates.go @@ -592,5 +592,10 @@ func (svc *Service) UpdateCertificateStatus( return nil } + // If operation_type is "remove" and status is "verified", delete the host_certificate_template row + if opType == fleet.MDMOperationTypeRemove && status == fleet.MDMDeliveryVerified { + return svc.ds.DeleteHostCertificateTemplate(ctx, host.UUID, certificateTemplateID) + } + return svc.ds.UpsertCertificateStatus(ctx, host.UUID, certificateTemplateID, status, detail, opType) }