From af1e150a2b1ccb85670aac31ba2366a99d39087b Mon Sep 17 00:00:00 2001 From: Victor Lyuboslavsky <2685025+getvictor@users.noreply.github.com> Date: Tue, 6 Jan 2026 10:20:07 -0600 Subject: [PATCH] Deleting/adding Android certs to host on team transfer (#37616) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Related issue:** Resolves #37580 Resolves unreleased 4.79 bug and needs to be cherry picked. Also includes fixes from manually going through the test plan at: [#30876](https://github.com/fleetdm/fleet/issues/30876) # Checklist for submitter ## Testing - [x] Added/updated automated tests - [x] QA'd all new/changed functionality manually For unreleased bug fixes in a release candidate, one of: - [x] Confirmed that the fix is not expected to adversely impact load test results ## Summary by CodeRabbit ## Release Notes * **New Features** * Per-template versioning and explicit operation/status fields for host certificate templates; delivery payloads now include per-template details. * **Bug Fixes** * Removal preparation broadened to also clear failed entries and handle per-host removals; delivery/transition ordering adjusted to avoid race conditions. * **Tests** * Extensive tests added for team-transfer flows, per-host removal/preparation, and end-to-end Android certificate template scenarios. ✏️ Tip: You can customize this high-level summary in your review settings. --- .../datastore/mysql/certificate_templates.go | 27 +- .../mysql/certificate_templates_test.go | 5 + .../mysql/host_certificate_templates.go | 152 +++++-- .../mysql/host_certificate_templates_test.go | 401 +++++++++++++++++- server/fleet/certificate_templates.go | 20 +- server/fleet/datastore.go | 4 + server/fleet/host_certificate_template.go | 2 + server/mdm/android/android.go | 5 +- server/mdm/android/service/profiles_test.go | 29 +- server/mdm/android/service/service.go | 31 +- server/mock/datastore_mock.go | 12 + server/service/certificate_templates_test.go | 11 +- server/service/certificates.go | 14 +- ...tion_android_certificate_templates_test.go | 399 +++++++++++++++++ server/service/integration_core_test.go | 32 +- server/service/integration_enterprise_test.go | 19 +- server/worker/software_worker.go | 15 +- server/worker/software_worker_test.go | 6 + 18 files changed, 1076 insertions(+), 108 deletions(-) diff --git a/server/datastore/mysql/certificate_templates.go b/server/datastore/mysql/certificate_templates.go index 91e53251b9..a44948fd60 100644 --- a/server/datastore/mysql/certificate_templates.go +++ b/server/datastore/mysql/certificate_templates.go @@ -76,6 +76,7 @@ func (ds *Datastore) GetCertificateTemplateByIdForHost(ctx context.Context, id u certificate_authorities.type AS certificate_authority_type, certificate_authorities.challenge_encrypted AS scep_challenge_encrypted, host_certificate_templates.status AS status, + COALESCE(BIN_TO_UUID(host_certificate_templates.uuid, true), '') AS uuid, host_certificate_templates.fleet_challenge AS fleet_challenge FROM certificate_templates INNER JOIN certificate_authorities ON certificate_templates.certificate_authority_id = certificate_authorities.id @@ -86,6 +87,9 @@ func (ds *Datastore) GetCertificateTemplateByIdForHost(ctx context.Context, id u WHERE certificate_templates.id = ? `, fleet.MDMOperationTypeInstall) if err := sqlx.GetContext(ctx, ds.reader(ctx), &template, stmt, hostUUID, id); err != nil { + if errors.Is(err, sql.ErrNoRows) { + return nil, ctxerr.Wrap(ctx, notFound("CertificateTemplateForHost")) + } return nil, ctxerr.Wrap(ctx, err, "getting certificate_template by id for host") } @@ -305,7 +309,8 @@ func (ds *Datastore) CreatePendingCertificateTemplatesForExistingHosts( fleet_challenge, status, operation_type, - name + name, + uuid ) SELECT hosts.uuid, @@ -313,7 +318,8 @@ func (ds *Datastore) CreatePendingCertificateTemplatesForExistingHosts( NULL, '%s', '%s', - ct.name + ct.name, + UUID_TO_BIN(UUID(), true) FROM hosts INNER JOIN host_mdm ON host_mdm.host_id = hosts.id INNER JOIN certificate_templates ct ON ct.id = ? @@ -344,18 +350,27 @@ func (ds *Datastore) CreatePendingCertificateTemplatesForNewHost( certificate_template_id, status, operation_type, - name + name, + uuid ) SELECT ?, id, '%s', '%s', - name + name, + UUID_TO_BIN(UUID(), true) FROM certificate_templates WHERE team_id = ? - ON DUPLICATE KEY UPDATE host_uuid = host_uuid - `, fleet.CertificateTemplatePending, fleet.MDMOperationTypeInstall) + ON DUPLICATE KEY UPDATE + -- allow 'remove' to transition to 'pending install', generating new uuid + uuid = IF(operation_type = '%s', UUID_TO_BIN(UUID(), true), uuid), + status = IF(operation_type = '%s', '%s', status), + operation_type = IF(operation_type = '%s', '%s', operation_type) + `, fleet.CertificateTemplatePending, fleet.MDMOperationTypeInstall, + fleet.MDMOperationTypeRemove, + fleet.MDMOperationTypeRemove, fleet.CertificateTemplatePending, + fleet.MDMOperationTypeRemove, fleet.MDMOperationTypeInstall) result, err := ds.writer(ctx).ExecContext(ctx, stmt, hostUUID, teamID) if err != nil { return 0, ctxerr.Wrap(ctx, err, "create pending certificate templates for new host") diff --git a/server/datastore/mysql/certificate_templates_test.go b/server/datastore/mysql/certificate_templates_test.go index 1d49dbf218..cd4cd5e612 100644 --- a/server/datastore/mysql/certificate_templates_test.go +++ b/server/datastore/mysql/certificate_templates_test.go @@ -274,6 +274,11 @@ func testGetCertificateTemplateByID(t *testing.T, ds *Datastore) { require.Equal(t, fleet.CertificateTemplateDelivered, templateForHost.Status) require.Equal(t, "fleet-challenge", *templateForHost.FleetChallenge) require.Equal(t, "test-challenge", *templateForHost.SCEPChallenge) + + // GetCertificateTemplateByIdForHost should return notFound for a host without a host_certificate_template record + _, err = ds.GetCertificateTemplateByIdForHost(ctx, certificateTemplateID, "non-existent-host-uuid") + require.Error(t, err) + require.True(t, fleet.IsNotFound(err), "expected notFound error, got: %v", err) }, }, { diff --git a/server/datastore/mysql/host_certificate_templates.go b/server/datastore/mysql/host_certificate_templates.go index 32198a68e2..99c286bead 100644 --- a/server/datastore/mysql/host_certificate_templates.go +++ b/server/datastore/mysql/host_certificate_templates.go @@ -40,19 +40,27 @@ func (ds *Datastore) ListAndroidHostUUIDsWithDeliverableCertificateTemplates(ctx return hostUUIDs, nil } -// ListCertificateTemplatesForHosts returns ALL certificate templates for the given host UUIDs +// ListCertificateTemplatesForHosts returns ALL certificate templates for the given host UUIDs. +// This includes: +// 1. Templates matching the host's current team (for install operations) +// 2. Templates marked for removal (which may be from a previous team after team transfer) func (ds *Datastore) ListCertificateTemplatesForHosts(ctx context.Context, hostUUIDs []string) ([]fleet.CertificateTemplateForHost, error) { if len(hostUUIDs) == 0 { return nil, nil } - query, args, err := sqlx.In(` + // Query 1: Templates matching host's current team + // Query 2: UNION with removal entries from host_certificate_templates + // (these may reference templates from a different team after team transfer) + // UNION removes duplicates if a template appears in both result sets + query, args, err := sqlx.In(fmt.Sprintf(` SELECT hosts.uuid AS host_uuid, certificate_templates.id AS certificate_template_id, host_certificate_templates.fleet_challenge AS fleet_challenge, host_certificate_templates.status AS status, host_certificate_templates.operation_type AS operation_type, + COALESCE(BIN_TO_UUID(host_certificate_templates.uuid, true), '') AS uuid, certificate_authorities.type AS ca_type, certificate_authorities.name AS ca_name FROM certificate_templates @@ -63,8 +71,27 @@ func (ds *Datastore) ListCertificateTemplatesForHosts(ctx context.Context, hostU AND host_certificate_templates.certificate_template_id = certificate_templates.id WHERE hosts.uuid IN (?) - ORDER BY hosts.uuid, certificate_templates.id - `, hostUUIDs) + + UNION + + SELECT + hct.host_uuid AS host_uuid, + hct.certificate_template_id AS certificate_template_id, + hct.fleet_challenge AS fleet_challenge, + hct.status AS status, + hct.operation_type AS operation_type, + COALESCE(BIN_TO_UUID(hct.uuid, true), '') AS uuid, + ca.type AS ca_type, + ca.name AS ca_name + FROM host_certificate_templates hct + INNER JOIN certificate_templates ct ON ct.id = hct.certificate_template_id + INNER JOIN certificate_authorities ca ON ca.id = ct.certificate_authority_id + WHERE + hct.host_uuid IN (?) + AND hct.operation_type = '%s' + + ORDER BY host_uuid, certificate_template_id + `, fleet.MDMOperationTypeRemove), hostUUIDs, hostUUIDs) if err != nil { return nil, ctxerr.Wrap(ctx, err, "build query for certificate templates") } @@ -86,6 +113,7 @@ func (ds *Datastore) GetCertificateTemplateForHost(ctx context.Context, hostUUID host_certificate_templates.fleet_challenge AS fleet_challenge, host_certificate_templates.status AS status, host_certificate_templates.operation_type AS operation_type, + COALESCE(BIN_TO_UUID(host_certificate_templates.uuid, true), '') AS uuid, certificate_authorities.type AS ca_type, certificate_authorities.name AS ca_name FROM certificate_templates @@ -122,6 +150,7 @@ func (ds *Datastore) GetHostCertificateTemplateRecord(ctx context.Context, hostU status, operation_type, detail, + COALESCE(BIN_TO_UUID(uuid, true), '') AS uuid, created_at, updated_at FROM host_certificate_templates @@ -154,7 +183,8 @@ func (ds *Datastore) BulkInsertHostCertificateTemplates(ctx context.Context, hos fleet_challenge, status, operation_type, - name + name, + uuid ) VALUES %s ` @@ -163,7 +193,7 @@ func (ds *Datastore) BulkInsertHostCertificateTemplates(ctx context.Context, hos for _, hct := range hostCertTemplates { args = append(args, hct.HostUUID, hct.CertificateTemplateID, hct.FleetChallenge, hct.Status, hct.OperationType, hct.Name) - placeholders.WriteString("(?,?,?,?,?,?),") + placeholders.WriteString("(?,?,?,?,?,?,UUID_TO_BIN(UUID(), true)),") } stmt := fmt.Sprintf(sqlInsert, strings.TrimSuffix(placeholders.String(), ",")) @@ -266,8 +296,8 @@ func (ds *Datastore) UpsertCertificateStatus( } insertStmt := ` - INSERT INTO host_certificate_templates (host_uuid, certificate_template_id, status, detail, fleet_challenge, operation_type, name) - VALUES (?, ?, ?, ?, ?, ?, ?)` + INSERT INTO host_certificate_templates (host_uuid, certificate_template_id, status, detail, fleet_challenge, operation_type, name, uuid) + VALUES (?, ?, ?, ?, ?, ?, ?, UUID_TO_BIN(UUID(), true))` params := []any{hostUUID, certificateTemplateID, status, detail, "", operationType, templateInfo.Name} if _, err := ds.writer(ctx).ExecContext(ctx, insertStmt, params...); err != nil { @@ -279,7 +309,7 @@ func (ds *Datastore) UpsertCertificateStatus( } // ListAndroidHostUUIDsWithPendingCertificateTemplates returns hosts that have -// certificate templates in 'pending' status ready for delivery. +// certificate templates in 'pending' status ready for delivery (both install and remove operations). func (ds *Datastore) ListAndroidHostUUIDsWithPendingCertificateTemplates( ctx context.Context, offset int, @@ -288,12 +318,10 @@ func (ds *Datastore) ListAndroidHostUUIDsWithPendingCertificateTemplates( stmt := fmt.Sprintf(` SELECT DISTINCT host_uuid FROM host_certificate_templates - WHERE - status = '%s' AND - operation_type = '%s' + WHERE status = '%s' ORDER BY host_uuid LIMIT ? OFFSET ? - `, fleet.CertificateTemplatePending, fleet.MDMOperationTypeInstall) + `, fleet.CertificateTemplatePending) var hostUUIDs []string if err := sqlx.SelectContext(ctx, ds.reader(ctx), &hostUUIDs, stmt, limit, offset); err != nil { return nil, ctxerr.Wrap(ctx, err, "list host uuids with pending certificate templates") @@ -302,7 +330,7 @@ func (ds *Datastore) ListAndroidHostUUIDsWithPendingCertificateTemplates( } // GetAndTransitionCertificateTemplatesToDelivering retrieves all certificate templates -// with operation_type='install' for a host, transitions any pending ones to 'delivering' status, +// for a host (both install and remove operations), transitions any pending ones to 'delivering' status, // and returns both the newly delivering template IDs and the existing (verified/delivered) ones. // This prevents concurrent cron runs from processing the same templates. func (ds *Datastore) GetAndTransitionCertificateTemplatesToDelivering( @@ -311,41 +339,59 @@ func (ds *Datastore) GetAndTransitionCertificateTemplatesToDelivering( ) (*fleet.HostCertificateTemplatesForDelivery, error) { result := &fleet.HostCertificateTemplatesForDelivery{} err := ds.withRetryTxx(ctx, func(tx sqlx.ExtContext) error { - // Select ALL templates with operation_type='install' for this host + // Select ALL templates (both install and remove) for this host var rows []struct { ID uint `db:"id"` CertificateTemplateID uint `db:"certificate_template_id"` Status fleet.CertificateTemplateStatus `db:"status"` + OperationType fleet.MDMOperationType `db:"operation_type"` + UUID string `db:"uuid"` } - selectStmt := fmt.Sprintf(` - SELECT id, certificate_template_id, status + const selectStmt = ` + SELECT id, certificate_template_id, status, operation_type, COALESCE(BIN_TO_UUID(uuid, true), '') AS uuid FROM host_certificate_templates - WHERE - host_uuid = ? AND - operation_type = '%s' + WHERE host_uuid = ? FOR UPDATE - `, fleet.MDMOperationTypeInstall) + ` if err := sqlx.SelectContext(ctx, tx, &rows, selectStmt, hostUUID); err != nil { - return ctxerr.Wrap(ctx, err, "select install templates") + return ctxerr.Wrap(ctx, err, "select templates") } if len(rows) == 0 { return nil } - // Separate templates by status + // Separate templates by status and build the Templates list var pendingIDs []uint // primary key IDs for UPDATE (only pending ones need transitioning) for _, r := range rows { switch r.Status { case fleet.CertificateTemplatePending: pendingIDs = append(pendingIDs, r.ID) result.DeliveringTemplateIDs = append(result.DeliveringTemplateIDs, r.CertificateTemplateID) + // Status will be delivering after transition + result.Templates = append(result.Templates, fleet.HostCertificateTemplateForDelivery{ + CertificateTemplateID: r.CertificateTemplateID, + Status: fleet.CertificateTemplateDelivering, + OperationType: r.OperationType, + UUID: r.UUID, + }) case fleet.CertificateTemplateDelivering: // Already delivering (from a previous failed run), include in delivering list; should be very rare result.DeliveringTemplateIDs = append(result.DeliveringTemplateIDs, r.CertificateTemplateID) + result.Templates = append(result.Templates, fleet.HostCertificateTemplateForDelivery{ + CertificateTemplateID: r.CertificateTemplateID, + Status: r.Status, + OperationType: r.OperationType, + UUID: r.UUID, + }) default: // delivered, verified, failed - result.OtherTemplateIDs = append(result.OtherTemplateIDs, r.CertificateTemplateID) + result.Templates = append(result.Templates, fleet.HostCertificateTemplateForDelivery{ + CertificateTemplateID: r.CertificateTemplateID, + Status: r.Status, + OperationType: r.OperationType, + UUID: r.UUID, + }) } } @@ -471,28 +517,32 @@ func (ds *Datastore) RevertStaleCertificateTemplates( } // SetHostCertificateTemplatesToPendingRemove prepares certificate templates for removal. -// For a given certificate template ID, it deletes any rows with status=pending and -// updates all other rows to status=pending, operation_type=remove. +// For a given certificate template ID, it deletes any rows with status in (pending, failed) +// and operation_type=install, then updates rows with operation_type=install to pending remove. +// Rows already in remove state are left unchanged (idempotent). func (ds *Datastore) SetHostCertificateTemplatesToPendingRemove( ctx context.Context, certificateTemplateID uint, ) error { return ds.withRetryTxx(ctx, func(tx sqlx.ExtContext) error { - // Delete rows with status=pending + // Delete rows with status in (pending, failed) and operation_type=install + // These certificates were never successfully installed on the device deleteStmt := fmt.Sprintf(` DELETE FROM host_certificate_templates - WHERE certificate_template_id = ? AND status = '%s' - `, fleet.CertificateTemplatePending) + WHERE certificate_template_id = ? AND status IN ('%s', '%s') AND operation_type = '%s' + `, fleet.CertificateTemplatePending, fleet.CertificateTemplateFailed, fleet.MDMOperationTypeInstall) if _, err := tx.ExecContext(ctx, deleteStmt, certificateTemplateID); err != nil { - return ctxerr.Wrap(ctx, err, "delete pending host certificate templates") + return ctxerr.Wrap(ctx, err, "delete pending/failed host certificate templates") } - // Update all remaining rows to status=pending, operation_type=remove + // Update rows with operation_type=install to status=pending, operation_type=remove with new UUID + // Only generate new UUID when transitioning from install to remove + // Rows already in remove state are left unchanged (idempotent) updateStmt := fmt.Sprintf(` UPDATE host_certificate_templates - SET status = '%s', operation_type = '%s' - WHERE certificate_template_id = ? - `, fleet.CertificateTemplatePending, fleet.MDMOperationTypeRemove) + SET status = '%s', operation_type = '%s', uuid = UUID_TO_BIN(UUID(), true) + WHERE certificate_template_id = ? AND operation_type = '%s' + `, fleet.CertificateTemplatePending, fleet.MDMOperationTypeRemove, fleet.MDMOperationTypeInstall) if _, err := tx.ExecContext(ctx, updateStmt, certificateTemplateID); err != nil { return ctxerr.Wrap(ctx, err, "update host certificate templates to pending remove") } @@ -500,3 +550,37 @@ func (ds *Datastore) SetHostCertificateTemplatesToPendingRemove( return nil }) } + +// SetHostCertificateTemplatesToPendingRemoveForHost prepares all certificate templates +// for a specific host for removal. Used during team transfer to mark old team's templates +// for removal before creating new pending templates for the new team. +// Records with operation_type=remove are left unchanged (removal already in progress). +func (ds *Datastore) SetHostCertificateTemplatesToPendingRemoveForHost( + ctx context.Context, + hostUUID string, +) error { + return ds.withRetryTxx(ctx, func(tx sqlx.ExtContext) error { + // Delete rows with status in (pending, failed) and operation_type=install + // These certificates were never successfully installed on the device + deleteStmt := fmt.Sprintf(` + DELETE FROM host_certificate_templates + WHERE host_uuid = ? AND status IN ('%s', '%s') AND operation_type = '%s' + `, fleet.CertificateTemplatePending, fleet.CertificateTemplateFailed, fleet.MDMOperationTypeInstall) + if _, err := tx.ExecContext(ctx, deleteStmt, hostUUID); err != nil { + return ctxerr.Wrap(ctx, err, "delete pending/failed install host certificate templates for host") + } + + // Update remaining install rows to status=pending, operation_type=remove with new UUID + // New UUID ensures the agent can distinguish this removal from previous operations + updateStmt := fmt.Sprintf(` + UPDATE host_certificate_templates + SET status = '%s', operation_type = '%s', uuid = UUID_TO_BIN(UUID(), true) + WHERE host_uuid = ? AND operation_type = '%s' + `, fleet.CertificateTemplatePending, fleet.MDMOperationTypeRemove, fleet.MDMOperationTypeInstall) + if _, err := tx.ExecContext(ctx, updateStmt, hostUUID); err != nil { + return ctxerr.Wrap(ctx, err, "update host certificate templates to pending remove for host") + } + + return nil + }) +} diff --git a/server/datastore/mysql/host_certificate_templates_test.go b/server/datastore/mysql/host_certificate_templates_test.go index ab6ec20199..3b204bb0d0 100644 --- a/server/datastore/mysql/host_certificate_templates_test.go +++ b/server/datastore/mysql/host_certificate_templates_test.go @@ -32,6 +32,11 @@ func TestHostCertificateTemplates(t *testing.T) { {"CertificateTemplateFullStateMachine", testCertificateTemplateFullStateMachine}, {"RevertStaleCertificateTemplates", testRevertStaleCertificateTemplates}, {"SetHostCertificateTemplatesToPendingRemove", testSetHostCertificateTemplatesToPendingRemove}, + {"SetHostCertificateTemplatesToPendingRemoveForHost", testSetHostCertificateTemplatesToPendingRemoveForHost}, + {"ListCertificateTemplatesForHostsIncludesRemovalAfterTeamTransfer", testListCertificateTemplatesForHostsIncludesRemovalAfterTeamTransfer}, + {"ListAndroidHostUUIDsWithPendingCertificateTemplatesIncludesRemoval", testListAndroidHostUUIDsWithPendingCertificateTemplatesIncludesRemoval}, + {"GetAndTransitionCertificateTemplatesToDeliveringIncludesRemoval", testGetAndTransitionCertificateTemplatesToDeliveringIncludesRemoval}, + {"CertificateTemplateReinstalledAfterTransferBackToOriginalTeam", testCertificateTemplateReinstalledAfterTransferBackToOriginalTeam}, } for _, c := range cases { @@ -828,7 +833,7 @@ func testCertificateTemplateFullStateMachine(t *testing.T, ds *Datastore) { require.NoError(t, err) require.Len(t, certTemplates.DeliveringTemplateIDs, 2) require.ElementsMatch(t, []uint{setup.template.ID, templateTwo.ID}, certTemplates.DeliveringTemplateIDs) - require.Empty(t, certTemplates.OtherTemplateIDs) // No existing verified/delivered templates yet + require.Len(t, certTemplates.Templates, 2) // Verify host is no longer in pending list hostUUIDs, err = ds.ListAndroidHostUUIDsWithPendingCertificateTemplates(ctx, 0, 10) @@ -840,7 +845,7 @@ func testCertificateTemplateFullStateMachine(t *testing.T, ds *Datastore) { require.NoError(t, err) require.Len(t, certTemplates.DeliveringTemplateIDs, 2) // Already delivering from previous call require.ElementsMatch(t, []uint{setup.template.ID, templateTwo.ID}, certTemplates.DeliveringTemplateIDs) - require.Empty(t, certTemplates.OtherTemplateIDs) // No delivered/verified/failed yet + require.Len(t, certTemplates.Templates, 2) // Verify database shows delivering status records, err := ds.ListCertificateTemplatesForHosts(ctx, []string{"android-host"}) @@ -1028,17 +1033,17 @@ func testRevertStaleCertificateTemplates(t *testing.T, ds *Datastore) { func testSetHostCertificateTemplatesToPendingRemove(t *testing.T, ds *Datastore) { ctx := t.Context() - t.Run("deletes pending rows and updates others to pending remove", func(t *testing.T) { + t.Run("deletes pending and failed rows and updates others to pending remove", func(t *testing.T) { defer TruncateTables(t, ds) setup := createCertTemplateTestSetup(t, ctx, ds, "") // Insert records with various statuses for the same template _, err := ds.writer(ctx).ExecContext(ctx, ` - INSERT INTO host_certificate_templates (host_uuid, certificate_template_id, status, operation_type, fleet_challenge, name) VALUES - (?, ?, ?, ?, ?, ?), - (?, ?, ?, ?, ?, ?), - (?, ?, ?, ?, ?, ?), - (?, ?, ?, ?, ?, ?) + INSERT INTO host_certificate_templates (host_uuid, certificate_template_id, status, operation_type, fleet_challenge, name, uuid) VALUES + (?, ?, ?, ?, ?, ?, UUID_TO_BIN(UUID(), true)), + (?, ?, ?, ?, ?, ?, UUID_TO_BIN(UUID(), true)), + (?, ?, ?, ?, ?, ?, UUID_TO_BIN(UUID(), true)), + (?, ?, ?, ?, ?, ?, UUID_TO_BIN(UUID(), true)) `, "host-pending", setup.template.ID, fleet.CertificateTemplatePending, fleet.MDMOperationTypeInstall, nil, setup.template.Name, "host-delivered", setup.template.ID, fleet.CertificateTemplateDelivered, fleet.MDMOperationTypeInstall, "challenge1", setup.template.Name, @@ -1057,20 +1062,47 @@ func testSetHostCertificateTemplatesToPendingRemove(t *testing.T, ds *Datastore) require.NoError(t, err) require.Equal(t, 0, count) - // Verify remaining rows have status=pending and operation_type=remove + // Verify the failed row was also deleted + err = ds.writer(ctx).GetContext(ctx, &count, + "SELECT COUNT(*) FROM host_certificate_templates WHERE host_uuid = ?", "host-failed") + require.NoError(t, err) + require.Equal(t, 0, count) + + // Verify remaining rows (delivered, verified) have status=pending and operation_type=remove var remaining []struct { HostUUID string `db:"host_uuid"` Status string `db:"status"` OperationType fleet.MDMOperationType `db:"operation_type"` + UUID string `db:"uuid"` } err = ds.writer(ctx).SelectContext(ctx, &remaining, - "SELECT host_uuid, status, operation_type FROM host_certificate_templates ORDER BY host_uuid") + "SELECT host_uuid, status, operation_type, COALESCE(BIN_TO_UUID(uuid, true), '') AS uuid FROM host_certificate_templates ORDER BY host_uuid") require.NoError(t, err) - require.Len(t, remaining, 3) + require.Len(t, remaining, 2) for _, r := range remaining { require.Equal(t, string(fleet.CertificateTemplatePending), r.Status) require.Equal(t, fleet.MDMOperationTypeRemove, r.OperationType) + require.NotEmpty(t, r.UUID, "UUID should be set after transition to remove") + } + + // Capture UUIDs before second call + uuidsBefore := make(map[string]string) + for _, r := range remaining { + uuidsBefore[r.HostUUID] = r.UUID + } + + // Second call should be idempotent - UUIDs should not change + err = ds.SetHostCertificateTemplatesToPendingRemove(ctx, setup.template.ID) + require.NoError(t, err) + + err = ds.writer(ctx).SelectContext(ctx, &remaining, + "SELECT host_uuid, status, operation_type, COALESCE(BIN_TO_UUID(uuid, true), '') AS uuid FROM host_certificate_templates ORDER BY host_uuid") + require.NoError(t, err) + require.Len(t, remaining, 2) + + for _, r := range remaining { + require.Equal(t, uuidsBefore[r.HostUUID], r.UUID, "UUID should not change when already in remove state") } }) @@ -1131,3 +1163,350 @@ func testSetHostCertificateTemplatesToPendingRemove(t *testing.T, ds *Datastore) require.NoError(t, err) }) } + +func testSetHostCertificateTemplatesToPendingRemoveForHost(t *testing.T, ds *Datastore) { + ctx := t.Context() + + t.Run("deletes pending and failed installs, updates other installs, leaves removes unchanged", func(t *testing.T) { + setup := createCertTemplateTestSetup(t, ctx, ds, "") + + // Create additional templates + templateTwo, err := ds.CreateCertificateTemplate(ctx, &fleet.CertificateTemplate{ + Name: "Cert2", + TeamID: setup.team.ID, + CertificateAuthorityID: setup.ca.ID, + SubjectName: "CN=Test2", + }) + require.NoError(t, err) + + templateThree, err := ds.CreateCertificateTemplate(ctx, &fleet.CertificateTemplate{ + Name: "Cert3", + TeamID: setup.team.ID, + CertificateAuthorityID: setup.ca.ID, + SubjectName: "CN=Test3", + }) + require.NoError(t, err) + + templateFour, err := ds.CreateCertificateTemplate(ctx, &fleet.CertificateTemplate{ + Name: "Cert4", + TeamID: setup.team.ID, + CertificateAuthorityID: setup.ca.ID, + SubjectName: "CN=Test4", + }) + require.NoError(t, err) + + // Insert records for host-1 (the target host) and host-2 (should not be affected) + _, err = ds.writer(ctx).ExecContext(ctx, ` + INSERT INTO host_certificate_templates (host_uuid, certificate_template_id, status, operation_type, fleet_challenge, name) VALUES + (?, ?, ?, ?, ?, ?), + (?, ?, ?, ?, ?, ?), + (?, ?, ?, ?, ?, ?), + (?, ?, ?, ?, ?, ?), + (?, ?, ?, ?, ?, ?) + `, + "host-1", setup.template.ID, fleet.CertificateTemplatePending, fleet.MDMOperationTypeInstall, nil, setup.template.Name, + "host-1", templateTwo.ID, fleet.CertificateTemplateDelivered, fleet.MDMOperationTypeInstall, "challenge1", templateTwo.Name, + "host-1", templateThree.ID, fleet.CertificateTemplateFailed, fleet.MDMOperationTypeInstall, nil, templateThree.Name, + "host-1", templateFour.ID, fleet.CertificateTemplateDelivering, fleet.MDMOperationTypeRemove, nil, templateFour.Name, + "host-2", setup.template.ID, fleet.CertificateTemplateDelivered, fleet.MDMOperationTypeInstall, "challenge2", setup.template.Name, + ) + require.NoError(t, err) + + // Call the method for host-1 only + err = ds.SetHostCertificateTemplatesToPendingRemoveForHost(ctx, "host-1") + require.NoError(t, err) + + // Verify host-1's pending install was deleted + var count int + err = ds.writer(ctx).GetContext(ctx, &count, + "SELECT COUNT(*) FROM host_certificate_templates WHERE host_uuid = ? AND certificate_template_id = ?", + "host-1", setup.template.ID) + require.NoError(t, err) + require.Equal(t, 0, count, "pending install should be deleted") + + // Verify host-1's failed install was also deleted + err = ds.writer(ctx).GetContext(ctx, &count, + "SELECT COUNT(*) FROM host_certificate_templates WHERE host_uuid = ? AND certificate_template_id = ?", + "host-1", templateThree.ID) + require.NoError(t, err) + require.Equal(t, 0, count, "failed install should be deleted") + + // Verify host-1's delivered install was updated to pending remove + var row struct { + Status string `db:"status"` + OperationType fleet.MDMOperationType `db:"operation_type"` + } + err = ds.writer(ctx).GetContext(ctx, &row, + "SELECT status, operation_type FROM host_certificate_templates WHERE host_uuid = ? AND certificate_template_id = ?", + "host-1", templateTwo.ID) + require.NoError(t, err) + require.Equal(t, string(fleet.CertificateTemplatePending), row.Status) + require.Equal(t, fleet.MDMOperationTypeRemove, row.OperationType) + + // Verify host-1's delivering remove was NOT changed (removal in progress) + err = ds.writer(ctx).GetContext(ctx, &row, + "SELECT status, operation_type FROM host_certificate_templates WHERE host_uuid = ? AND certificate_template_id = ?", + "host-1", templateFour.ID) + require.NoError(t, err) + require.Equal(t, string(fleet.CertificateTemplateDelivering), row.Status, "remove operation should not change status") + require.Equal(t, fleet.MDMOperationTypeRemove, row.OperationType, "remove operation should stay as remove") + + // Verify host-2 was NOT affected + err = ds.writer(ctx).GetContext(ctx, &row, + "SELECT status, operation_type FROM host_certificate_templates WHERE host_uuid = ?", "host-2") + require.NoError(t, err) + require.Equal(t, string(fleet.CertificateTemplateDelivered), row.Status) + require.Equal(t, fleet.MDMOperationTypeInstall, row.OperationType) + }) +} + +// testListCertificateTemplatesForHostsIncludesRemovalAfterTeamTransfer verifies that +// ListCertificateTemplatesForHosts returns removal entries for templates from a previous team +// after the host has transferred to a new team. +func testListCertificateTemplatesForHostsIncludesRemovalAfterTeamTransfer(t *testing.T, ds *Datastore) { + ctx := t.Context() + + // Setup: Create two teams with certificate templates using existing helpers + setupA := createCertTemplateTestSetup(t, ctx, ds, "Team A for removal test") + setupB := createCertTemplateTestSetup(t, ctx, ds, "Team B for removal test") + + // Create host initially in Team A + host := createEnrolledAndroidHost(t, ctx, ds, uuid.New().String(), &setupA.team.ID) + + // Insert verified certificate from Team A using BulkInsertHostCertificateTemplates + challenge := "challenge-a" + err := ds.BulkInsertHostCertificateTemplates(ctx, []fleet.HostCertificateTemplate{{ + HostUUID: host.UUID, + CertificateTemplateID: setupA.template.ID, + Status: fleet.CertificateTemplateVerified, + OperationType: fleet.MDMOperationTypeInstall, + FleetChallenge: &challenge, + Name: setupA.template.Name, + }}) + require.NoError(t, err) + + // Simulate team transfer: move host to Team B using UpdateHost + host.TeamID = &setupB.team.ID + err = ds.UpdateHost(ctx, host) + require.NoError(t, err) + + // Mark Team A template for removal using the datastore method + err = ds.SetHostCertificateTemplatesToPendingRemoveForHost(ctx, host.UUID) + require.NoError(t, err) + + // Insert pending install for Team B template + err = ds.BulkInsertHostCertificateTemplates(ctx, []fleet.HostCertificateTemplate{{ + HostUUID: host.UUID, + CertificateTemplateID: setupB.template.ID, + Status: fleet.CertificateTemplatePending, + OperationType: fleet.MDMOperationTypeInstall, + Name: setupB.template.Name, + }}) + require.NoError(t, err) + + // Act: List certificate templates for host + results, err := ds.ListCertificateTemplatesForHosts(ctx, []string{host.UUID}) + require.NoError(t, err) + + require.Len(t, results, 2, "should include both install and removal templates") + + templatesByID := make(map[uint]fleet.CertificateTemplateForHost) + for _, r := range results { + templatesByID[r.CertificateTemplateID] = r + } + + // Team A template should be marked for removal + require.Contains(t, templatesByID, setupA.template.ID, "should include Team A template marked for removal") + require.NotNil(t, templatesByID[setupA.template.ID].OperationType) + require.Equal(t, fleet.MDMOperationTypeRemove, *templatesByID[setupA.template.ID].OperationType) + require.NotNil(t, templatesByID[setupA.template.ID].Status) + require.Equal(t, fleet.CertificateTemplatePending, *templatesByID[setupA.template.ID].Status) + + // Team B template should be pending install + require.Contains(t, templatesByID, setupB.template.ID, "should include Team B template") + require.NotNil(t, templatesByID[setupB.template.ID].OperationType) + require.Equal(t, fleet.MDMOperationTypeInstall, *templatesByID[setupB.template.ID].OperationType) +} + +// testListAndroidHostUUIDsWithPendingCertificateTemplatesIncludesRemoval verifies that +// ListAndroidHostUUIDsWithPendingCertificateTemplates returns hosts with pending removal templates. +func testListAndroidHostUUIDsWithPendingCertificateTemplatesIncludesRemoval(t *testing.T, ds *Datastore) { + ctx := t.Context() + + setup := createCertTemplateTestSetup(t, ctx, ds, "") + host := createEnrolledAndroidHost(t, ctx, ds, uuid.New().String(), &setup.team.ID) + + // Insert a pending removal template using BulkInsertHostCertificateTemplates + err := ds.BulkInsertHostCertificateTemplates(ctx, []fleet.HostCertificateTemplate{{ + HostUUID: host.UUID, + CertificateTemplateID: setup.template.ID, + Status: fleet.CertificateTemplatePending, + OperationType: fleet.MDMOperationTypeRemove, + Name: setup.template.Name, + }}) + require.NoError(t, err) + + // Act: List hosts with pending templates + results, err := ds.ListAndroidHostUUIDsWithPendingCertificateTemplates(ctx, 0, 100) + require.NoError(t, err) + + require.Len(t, results, 1, "should include host with pending removal") + require.Equal(t, host.UUID, results[0]) +} + +// testGetAndTransitionCertificateTemplatesToDeliveringIncludesRemoval verifies that +// GetAndTransitionCertificateTemplatesToDelivering handles both install and remove operations. +func testGetAndTransitionCertificateTemplatesToDeliveringIncludesRemoval(t *testing.T, ds *Datastore) { + ctx := t.Context() + + setup := createCertTemplateTestSetup(t, ctx, ds, "") + host := createEnrolledAndroidHost(t, ctx, ds, uuid.New().String(), &setup.team.ID) + + // Create second template for removal + templateForRemoval, err := ds.CreateCertificateTemplate(ctx, &fleet.CertificateTemplate{ + Name: "Template for Removal", + TeamID: setup.team.ID, + CertificateAuthorityID: setup.ca.ID, + SubjectName: "CN=Template for Removal", + }) + require.NoError(t, err) + + // Insert both pending install and pending removal using BulkInsertHostCertificateTemplates + err = ds.BulkInsertHostCertificateTemplates(ctx, []fleet.HostCertificateTemplate{ + { + HostUUID: host.UUID, + CertificateTemplateID: setup.template.ID, + Status: fleet.CertificateTemplatePending, + OperationType: fleet.MDMOperationTypeInstall, + Name: setup.template.Name, + }, + { + HostUUID: host.UUID, + CertificateTemplateID: templateForRemoval.ID, + Status: fleet.CertificateTemplatePending, + OperationType: fleet.MDMOperationTypeRemove, + Name: templateForRemoval.Name, + }, + }) + require.NoError(t, err) + + // Act: Transition to delivering + result, err := ds.GetAndTransitionCertificateTemplatesToDelivering(ctx, host.UUID) + require.NoError(t, err) + + // Assert: Should include BOTH install and removal templates + // Currently only install is included + require.Len(t, result.Templates, 2, "should include both install and removal") + require.Len(t, result.DeliveringTemplateIDs, 2, "should transition both to delivering") + + // Verify both templates are in delivering state in the database + var statuses []struct { + CertificateTemplateID uint `db:"certificate_template_id"` + Status string `db:"status"` + OperationType string `db:"operation_type"` + } + err = ds.writer(ctx).SelectContext(ctx, &statuses, + `SELECT certificate_template_id, status, operation_type + FROM host_certificate_templates WHERE host_uuid = ?`, host.UUID) + require.NoError(t, err) + require.Len(t, statuses, 2) + + for _, s := range statuses { + require.Equal(t, string(fleet.CertificateTemplateDelivering), s.Status, + "template %d should be in delivering status", s.CertificateTemplateID) + } + + // Verify the result contains templates with correct operation types + hasInstall := false + hasRemove := false + for _, tmpl := range result.Templates { + if tmpl.OperationType == fleet.MDMOperationTypeInstall { + hasInstall = true + } + if tmpl.OperationType == fleet.MDMOperationTypeRemove { + hasRemove = true + } + } + require.True(t, hasInstall, "should include install operation") + require.True(t, hasRemove, "should include remove operation") +} + +// testCertificateTemplateReinstalledAfterTransferBackToOriginalTeam verifies that when a host +// transfers back to its original team, the certificate template that was marked for removal +// is correctly transitioned back to pending install. +func testCertificateTemplateReinstalledAfterTransferBackToOriginalTeam(t *testing.T, ds *Datastore) { + ctx := t.Context() + + setupA := createCertTemplateTestSetup(t, ctx, ds, "Team A") + setupB := createCertTemplateTestSetup(t, ctx, ds, "Team B") + host := createEnrolledAndroidHost(t, ctx, ds, uuid.New().String(), &setupA.team.ID) + + // Host starts with verified cert in Team A + challenge := "challenge-a" + err := ds.BulkInsertHostCertificateTemplates(ctx, []fleet.HostCertificateTemplate{{ + HostUUID: host.UUID, + CertificateTemplateID: setupA.template.ID, + Status: fleet.CertificateTemplateVerified, + OperationType: fleet.MDMOperationTypeInstall, + FleetChallenge: &challenge, + Name: setupA.template.Name, + }}) + require.NoError(t, err) + + // Capture initial UUID + initialResults, err := ds.ListCertificateTemplatesForHosts(ctx, []string{host.UUID}) + require.NoError(t, err) + var initialUUID string + for _, r := range initialResults { + if r.CertificateTemplateID == setupA.template.ID { + require.NotNil(t, r.UUID, "initial UUID should be set") + initialUUID = *r.UUID + break + } + } + require.NotEmpty(t, initialUUID, "initial UUID should not be empty") + + // Transfer to Team B: mark Team A cert for removal, create pending install for Team B + host.TeamID = &setupB.team.ID + require.NoError(t, ds.UpdateHost(ctx, host)) + require.NoError(t, ds.SetHostCertificateTemplatesToPendingRemoveForHost(ctx, host.UUID)) + _, err = ds.CreatePendingCertificateTemplatesForNewHost(ctx, host.UUID, setupB.team.ID) + require.NoError(t, err) + + // Capture UUID after marking for removal (should be different from initial) + removeResults, err := ds.ListCertificateTemplatesForHosts(ctx, []string{host.UUID}) + require.NoError(t, err) + var uuidAfterRemove string + for _, r := range removeResults { + if r.CertificateTemplateID == setupA.template.ID { + require.NotNil(t, r.UUID, "UUID after remove should be set") + uuidAfterRemove = *r.UUID + break + } + } + require.NotEqual(t, initialUUID, uuidAfterRemove, "UUID should change when marked for removal") + + // Transfer back to Team A: mark Team B cert for removal, re-create pending install for Team A + host.TeamID = &setupA.team.ID + require.NoError(t, ds.UpdateHost(ctx, host)) + require.NoError(t, ds.SetHostCertificateTemplatesToPendingRemoveForHost(ctx, host.UUID)) + _, err = ds.CreatePendingCertificateTemplatesForNewHost(ctx, host.UUID, setupA.team.ID) + require.NoError(t, err) + + // Team A's cert should now be pending install (not pending remove) with a new UUID + results, err := ds.ListCertificateTemplatesForHosts(ctx, []string{host.UUID}) + require.NoError(t, err) + + var certA *fleet.CertificateTemplateForHost + for _, r := range results { + if r.CertificateTemplateID == setupA.template.ID { + certA = &r + break + } + } + require.NotNil(t, certA, "Team A cert should exist") + require.Equal(t, fleet.MDMOperationTypeInstall, *certA.OperationType) + require.Equal(t, fleet.CertificateTemplatePending, *certA.Status) + require.NotNil(t, certA.UUID, "UUID should be set") + require.NotEqual(t, uuidAfterRemove, *certA.UUID, "UUID should change when reinstalled") +} diff --git a/server/fleet/certificate_templates.go b/server/fleet/certificate_templates.go index eb1e6e8d79..5aa1df5bb3 100644 --- a/server/fleet/certificate_templates.go +++ b/server/fleet/certificate_templates.go @@ -39,6 +39,7 @@ type CertificateTemplateResponse struct { type CertificateTemplateResponseForHost struct { CertificateTemplateResponse Status CertificateTemplateStatus `json:"status" db:"status"` + UUID string `json:"uuid" db:"uuid"` SCEPChallenge *string `json:"scep_challenge" db:"scep_challenge"` FleetChallenge *string `json:"fleet_challenge" db:"fleet_challenge"` SCEPChallengeEncrypted []byte `json:"-" db:"scep_challenge_encrypted"` @@ -68,13 +69,22 @@ func CertificateTemplateStatusToMDMDeliveryStatus(s CertificateTemplateStatus) M } } +// HostCertificateTemplateForDelivery represents a certificate template being prepared +// for delivery to a host, including its current status and operation type. +type HostCertificateTemplateForDelivery struct { + CertificateTemplateID uint + Status CertificateTemplateStatus + OperationType MDMOperationType + UUID string +} + // HostCertificateTemplatesForDelivery contains the result of preparing certificate templates -// for delivery to a host. It includes both the templates being transitioned to delivering -// status and the templates that are already installed (verified/delivered). +// for delivery to a host. type HostCertificateTemplatesForDelivery struct { // DeliveringTemplateIDs are the certificate template IDs that were transitioned - // from pending to delivering status in this operation. + // from pending to delivering status in this operation. Used for challenge generation. DeliveringTemplateIDs []uint - // OtherTemplateIDs are other certificate template IDs. - OtherTemplateIDs []uint + // Templates contains all certificate templates with their current status and operation. + // Pending templates will show as delivering (their post-transition status). + Templates []HostCertificateTemplateForDelivery } diff --git a/server/fleet/datastore.go b/server/fleet/datastore.go index a9d7e1374f..a5a5fc3496 100644 --- a/server/fleet/datastore.go +++ b/server/fleet/datastore.go @@ -2607,6 +2607,10 @@ type Datastore interface { // updates all other rows to status=pending, operation_type=remove. SetHostCertificateTemplatesToPendingRemove(ctx context.Context, certificateTemplateID uint) error + // SetHostCertificateTemplatesToPendingRemoveForHost prepares all certificate templates + // for a specific host for removal. + SetHostCertificateTemplatesToPendingRemoveForHost(ctx context.Context, hostUUID string) error + // GetCurrentTime gets the current time from the database GetCurrentTime(ctx context.Context) (time.Time, error) diff --git a/server/fleet/host_certificate_template.go b/server/fleet/host_certificate_template.go index b8422efae8..62dcc98441 100644 --- a/server/fleet/host_certificate_template.go +++ b/server/fleet/host_certificate_template.go @@ -12,6 +12,7 @@ type HostCertificateTemplate struct { Status CertificateTemplateStatus `db:"status"` OperationType MDMOperationType `db:"operation_type"` Detail *string `db:"detail" json:"-"` + UUID string `db:"uuid"` CreatedAt string `db:"created_at"` UpdatedAt string `db:"updated_at"` } @@ -43,6 +44,7 @@ type CertificateTemplateForHost struct { FleetChallenge *string `db:"fleet_challenge"` Status *CertificateTemplateStatus `db:"status"` OperationType *MDMOperationType `db:"operation_type"` + UUID *string `db:"uuid"` CAType CAConfigAssetType `db:"ca_type"` CAName string `db:"ca_name"` } diff --git a/server/mdm/android/android.go b/server/mdm/android/android.go index 4b07161ed8..e6518da082 100644 --- a/server/mdm/android/android.go +++ b/server/mdm/android/android.go @@ -60,7 +60,10 @@ type AgentManagedConfiguration struct { } type AgentCertificateTemplate struct { - ID uint `json:"id"` + ID uint `json:"id"` + Status string `json:"status"` + Operation string `json:"operation"` + UUID string `json:"uuid"` } // MDMAndroidPolicyRequest represents a request made to the Android Management diff --git a/server/mdm/android/service/profiles_test.go b/server/mdm/android/service/profiles_test.go index 68ddd4ce9d..8d93a8f084 100644 --- a/server/mdm/android/service/profiles_test.go +++ b/server/mdm/android/service/profiles_test.go @@ -916,13 +916,16 @@ func testCertificateTemplates(t *testing.T, ds fleet.Datastore, client *mock.Cli require.Equal(t, fmt.Sprintf("%s/policies/%s", reconciler.Enterprise.Name(), host1.Host.UUID), capturedPolicyName) require.Len(t, capturedPolicies, 1) - // Verify the managed configuration contains certificate template IDs + // Verify the managed configuration contains certificate template IDs with status and operation var managedConfig android.AgentManagedConfiguration err = json.Unmarshal(capturedPolicies[0].ManagedConfiguration, &managedConfig) require.NoError(t, err) require.Len(t, managedConfig.CertificateTemplateIDs, 2) for _, certTemplate := range managedConfig.CertificateTemplateIDs { require.Contains(t, certificateTemplateIDs, certTemplate.ID) + // When sent to API, templates are in "delivering" status (transition to "delivered" happens after API success) + require.EqualValues(t, fleet.CertificateTemplateDelivering, certTemplate.Status) + require.EqualValues(t, fleet.MDMOperationTypeInstall, certTemplate.Operation) } // Verify that host_certificate_template records were created with pending status @@ -1214,14 +1217,22 @@ func testCertificateTemplatesIncludesExistingVerified(t *testing.T, ds fleet.Dat require.Len(t, managedConfig.CertificateTemplateIDs, 5, "Agent config should include all certificate templates (verified, delivered, delivering, failed, and pending)") - // Verify all certificate template IDs are present - templateIDs := make(map[uint]bool) + // Verify all certificate template IDs are present with correct status and operation + templatesByID := make(map[uint]android.AgentCertificateTemplate) for _, tmpl := range managedConfig.CertificateTemplateIDs { - templateIDs[tmpl.ID] = true + templatesByID[tmpl.ID] = tmpl } - require.True(t, templateIDs[verifiedCert.ID], "Verified certificate should be in the config") - require.True(t, templateIDs[deliveredCert.ID], "Delivered certificate should be in the config") - require.True(t, templateIDs[deliveringCert.ID], "Delivering certificate should be in the config") - require.True(t, templateIDs[failedCert.ID], "Failed certificate should be in the config") - require.True(t, templateIDs[pendingCert.ID], "Pending certificate should be in the config") + + assertCertTemplate := func(certID uint, expectedStatus fleet.CertificateTemplateStatus, expectedOp fleet.MDMOperationType) { + require.Contains(t, templatesByID, certID) + require.EqualValues(t, expectedStatus, templatesByID[certID].Status) + require.EqualValues(t, expectedOp, templatesByID[certID].Operation) + } + + assertCertTemplate(verifiedCert.ID, fleet.CertificateTemplateVerified, fleet.MDMOperationTypeInstall) + assertCertTemplate(deliveredCert.ID, fleet.CertificateTemplateDelivered, fleet.MDMOperationTypeInstall) + assertCertTemplate(deliveringCert.ID, fleet.CertificateTemplateDelivering, fleet.MDMOperationTypeInstall) + assertCertTemplate(failedCert.ID, fleet.CertificateTemplateFailed, fleet.MDMOperationTypeInstall) + // Pending certificate transitions to delivering before the API call + assertCertTemplate(pendingCert.ID, fleet.CertificateTemplateDelivering, fleet.MDMOperationTypeInstall) } diff --git a/server/mdm/android/service/service.go b/server/mdm/android/service/service.go index ea2c0bbf19..e8f720d0d3 100644 --- a/server/mdm/android/service/service.go +++ b/server/mdm/android/service/service.go @@ -920,6 +920,7 @@ func buildFleetAgentAppPolicy(packageName, sha256Fingerprint string, managedConf RoleType: "COMPANION_APP", }, }, + AutoUpdateMode: "AUTO_UPDATE_HIGH_PRIORITY", }, nil } @@ -1004,9 +1005,19 @@ func (svc *Service) buildAgentManagedConfig(ctx context.Context, hostUUID string var certificateTemplateIDs []android.AgentCertificateTemplate for _, ct := range certTemplates { - certificateTemplateIDs = append(certificateTemplateIDs, android.AgentCertificateTemplate{ + template := android.AgentCertificateTemplate{ ID: ct.CertificateTemplateID, - }) + } + if ct.Status != nil { + template.Status = string(*ct.Status) + } + if ct.OperationType != nil { + template.Operation = string(*ct.OperationType) + } + if ct.UUID != nil { + template.UUID = *ct.UUID + } + certificateTemplateIDs = append(certificateTemplateIDs, template) } return &android.AgentManagedConfiguration{ @@ -1233,8 +1244,8 @@ func (svc *Service) BuildAndSendFleetAgentConfig(ctx context.Context, enterprise return secrets, nil } - // Helper to build config for a single host - buildHostConfig := func(hostUUID string, templateIDs []uint) (*android.AgentManagedConfiguration, error) { + // Helper to build config for a single host using pre-fetched certificate templates + buildHostConfig := func(hostUUID string, templates []fleet.HostCertificateTemplateForDelivery) (*android.AgentManagedConfiguration, error) { androidHost, err := svc.ds.AndroidHostLiteByHostUUID(ctx, hostUUID) if err != nil { return nil, ctxerr.Wrapf(ctx, err, "get android host %s", hostUUID) @@ -1248,11 +1259,13 @@ func (svc *Service) BuildAndSendFleetAgentConfig(ctx context.Context, enterprise return nil, ctxerr.Errorf(ctx, "no enroll secrets found for team %v", androidHost.Host.TeamID) } - // Build certificate template IDs list var certificateTemplateIDs []android.AgentCertificateTemplate - for _, templateID := range templateIDs { + for _, ct := range templates { certificateTemplateIDs = append(certificateTemplateIDs, android.AgentCertificateTemplate{ - ID: templateID, + ID: ct.CertificateTemplateID, + Status: string(ct.Status), + Operation: string(ct.OperationType), + UUID: ct.UUID, }) } @@ -1280,7 +1293,7 @@ func (svc *Service) BuildAndSendFleetAgentConfig(ctx context.Context, enterprise } // Send config without new certificates (needed for new host enrollment) // There should be no other certificates either, but including them just in case. - config, err := buildHostConfig(hostUUID, certTemplates.OtherTemplateIDs) + config, err := buildHostConfig(hostUUID, certTemplates.Templates) if err != nil { level.Error(svc.logger).Log("msg", "failed to build host config without certs", "host_uuid", hostUUID, "err", err) return ctxerr.Wrapf(ctx, err, "build host config without certs for host %s", hostUUID) @@ -1294,7 +1307,7 @@ func (svc *Service) BuildAndSendFleetAgentConfig(ctx context.Context, enterprise } // Step 2: Build and send config to AMAPI with ALL certificate templates - config, err := buildHostConfig(hostUUID, append(certTemplates.DeliveringTemplateIDs, certTemplates.OtherTemplateIDs...)) + config, err := buildHostConfig(hostUUID, certTemplates.Templates) if err != nil { level.Error(svc.logger).Log("msg", "failed to build host config", "host_uuid", hostUUID, "err", err) return ctxerr.Wrapf(ctx, err, "build host config for %s", hostUUID) diff --git a/server/mock/datastore_mock.go b/server/mock/datastore_mock.go index 5a379b5672..0ad7a40aec 100644 --- a/server/mock/datastore_mock.go +++ b/server/mock/datastore_mock.go @@ -1705,6 +1705,8 @@ type RevertHostCertificateTemplatesToPendingFunc func(ctx context.Context, hostU type SetHostCertificateTemplatesToPendingRemoveFunc func(ctx context.Context, certificateTemplateID uint) error +type SetHostCertificateTemplatesToPendingRemoveForHostFunc func(ctx context.Context, hostUUID string) error + type GetCurrentTimeFunc func(ctx context.Context) (time.Time, error) type UpdateOrDeleteHostMDMWindowsProfileFunc func(ctx context.Context, profile *fleet.HostMDMWindowsProfile) error @@ -4237,6 +4239,9 @@ type DataStore struct { SetHostCertificateTemplatesToPendingRemoveFunc SetHostCertificateTemplatesToPendingRemoveFunc SetHostCertificateTemplatesToPendingRemoveFuncInvoked bool + SetHostCertificateTemplatesToPendingRemoveForHostFunc SetHostCertificateTemplatesToPendingRemoveForHostFunc + SetHostCertificateTemplatesToPendingRemoveForHostFuncInvoked bool + GetCurrentTimeFunc GetCurrentTimeFunc GetCurrentTimeFuncInvoked bool @@ -10139,6 +10144,13 @@ func (s *DataStore) SetHostCertificateTemplatesToPendingRemove(ctx context.Conte return s.SetHostCertificateTemplatesToPendingRemoveFunc(ctx, certificateTemplateID) } +func (s *DataStore) SetHostCertificateTemplatesToPendingRemoveForHost(ctx context.Context, hostUUID string) error { + s.mu.Lock() + s.SetHostCertificateTemplatesToPendingRemoveForHostFuncInvoked = true + s.mu.Unlock() + return s.SetHostCertificateTemplatesToPendingRemoveForHostFunc(ctx, hostUUID) +} + func (s *DataStore) GetCurrentTime(ctx context.Context) (time.Time, error) { s.mu.Lock() s.GetCurrentTimeFuncInvoked = true diff --git a/server/service/certificate_templates_test.go b/server/service/certificate_templates_test.go index 2c1513b51e..1d724b6470 100644 --- a/server/service/certificate_templates_test.go +++ b/server/service/certificate_templates_test.go @@ -3,6 +3,7 @@ package service import ( "context" "errors" + "strings" "testing" "time" @@ -96,10 +97,7 @@ func TestCreateCertificateTemplate(t *testing.T) { }) t.Run("Name too long", func(t *testing.T) { - longName := string(make([]byte, 256)) - for i := range longName { - longName = longName[:i] + "a" + longName[i+1:] - } + longName := strings.Repeat("a", 256) _, err := svc.CreateCertificateTemplate(ctx, longName, TeamID, uint(ValidCATypeID), "CN=$FLEET_VAR_HOST_UUID") require.Error(t, err) require.Contains(t, err.Error(), "Certificate template name is too long") @@ -356,10 +354,7 @@ func TestApplyCertificateTemplateSpecs(t *testing.T) { }) t.Run("Name too long", func(t *testing.T) { - longName := string(make([]byte, 256)) - for i := range longName { - longName = longName[:i] + "a" + longName[i+1:] - } + longName := strings.Repeat("a", 256) err := svc.ApplyCertificateTemplateSpecs(ctx, []*fleet.CertificateRequestSpec{ { Name: longName, diff --git a/server/service/certificates.go b/server/service/certificates.go index 3ca91998b9..e26bb255c6 100644 --- a/server/service/certificates.go +++ b/server/service/certificates.go @@ -627,20 +627,22 @@ func (svc *Service) UpdateCertificateStatus( return err } - if record.Status != fleet.CertificateTemplateDelivered { - level.Info(svc.logger).Log("msg", "ignoring certificate status update for non-delivered certificate", "host_uuid", host.UUID, "certificate_template_id", certificateTemplateID, "current_status", record.Status, "new_status", status) - return nil - } - if record.OperationType != opType { level.Info(svc.logger).Log("msg", "ignoring certificate status update for different operation type", "host_uuid", host.UUID, "certificate_template_id", certificateTemplateID, "current_operation_type", record.OperationType, "new_operation_type", opType) return nil } - // If operation_type is "remove" and status is "verified", delete the host_certificate_template row + // If operation_type is "remove" and status is "verified", delete the host_certificate_template row. + // This allows deletions even when there are race conditions or status sync issues + // (e.g., device reports removal before server transitions status). if opType == fleet.MDMOperationTypeRemove && status == fleet.MDMDeliveryVerified { return svc.ds.DeleteHostCertificateTemplate(ctx, host.UUID, certificateTemplateID) } + if record.Status != fleet.CertificateTemplateDelivered { + level.Info(svc.logger).Log("msg", "ignoring certificate status update for non-delivered certificate", "host_uuid", host.UUID, "certificate_template_id", certificateTemplateID, "current_status", record.Status, "new_status", status) + return nil + } + return svc.ds.UpsertCertificateStatus(ctx, host.UUID, certificateTemplateID, status, detail, opType) } diff --git a/server/service/integration_android_certificate_templates_test.go b/server/service/integration_android_certificate_templates_test.go index 5f668de650..0381ed39ea 100644 --- a/server/service/integration_android_certificate_templates_test.go +++ b/server/service/integration_android_certificate_templates_test.go @@ -116,6 +116,61 @@ func (s *integrationMDMTestSuite) verifyCertificateStatusWithSubject( } } +// createTestCertificateAuthority creates a test certificate authority for use in tests. +func (s *integrationMDMTestSuite) createTestCertificateAuthority(t *testing.T, ctx context.Context) (uint, *fleet.CertificateAuthority) { + ca, err := s.ds.NewCertificateAuthority(ctx, &fleet.CertificateAuthority{ + Type: string(fleet.CATypeCustomSCEPProxy), + Name: ptr.String(t.Name() + "-CA"), + URL: ptr.String("http://localhost:8080/scep"), + Challenge: ptr.String("test-challenge"), + }) + require.NoError(t, err) + return ca.ID, ca +} + +// createEnrolledAndroidHost creates an enrolled Android host in a team and returns the host and orbit node key. +func (s *integrationMDMTestSuite) createEnrolledAndroidHost(t *testing.T, ctx context.Context, enterpriseID string, teamID *uint, suffix string) (*fleet.Host, string) { + hostUUID := uuid.NewString() + androidHostInput := &fleet.AndroidHost{ + Host: &fleet.Host{ + Hostname: t.Name() + "-host-" + suffix, + ComputerName: t.Name() + "-device-" + suffix, + Platform: "android", + OSVersion: "Android 14", + Build: "build1", + Memory: 1024, + TeamID: teamID, + HardwareSerial: uuid.NewString(), + UUID: hostUUID, + }, + Device: &android.Device{ + DeviceID: strings.ReplaceAll(uuid.NewString(), "-", ""), + EnterpriseSpecificID: ptr.String(enterpriseID), + AppliedPolicyID: ptr.String("1"), + }, + } + androidHostInput.SetNodeKey(enterpriseID) + createdAndroidHost, err := s.ds.NewAndroidHost(ctx, androidHostInput) + require.NoError(t, err) + + host := createdAndroidHost.Host + orbitNodeKey := *host.NodeKey + host.OrbitNodeKey = &orbitNodeKey + require.NoError(t, s.ds.UpdateHost(ctx, host)) + + // Mark host as enrolled in host_mdm + mysql.ExecAdhocSQL(t, s.ds, func(q sqlx.ExtContext) error { + _, err := q.ExecContext(ctx, ` + INSERT INTO host_mdm (host_id, enrolled, server_url, installed_from_dep, is_server) + VALUES (?, 1, 'https://example.com', 0, 0) + ON DUPLICATE KEY UPDATE enrolled = 1 + `, host.ID) + return err + }) + + return host, orbitNodeKey +} + // TestCertificateTemplateLifecycle tests the full Android certificate template lifecycle. func (s *integrationMDMTestSuite) TestCertificateTemplateLifecycle() { t := s.T() @@ -653,3 +708,347 @@ func (s *integrationMDMTestSuite) TestCertificateTemplateUnenrollReenroll() { s.verifyCertificateStatusWithSubject(t, host, orbitNodeKey, certTemplateID2, certTemplateName2, caID, fleet.CertificateTemplatePending, "", "CN="+host.UUID) } + +// TestCertificateTemplateTeamTransfer tests certificate template behavior when Android hosts transfer between teams: +// 1. Host with certs in various statuses (pending, delivering, delivered, verified, failed, remove) transfers teams -> all certs marked for removal +// 2. Host without any certs transfers to a team with certs -> gets new pending certs +// 3. Host with certs transfers to another team with different certs -> old certs removed, new certs added +func (s *integrationMDMTestSuite) TestCertificateTemplateTeamTransfer() { + t := s.T() + ctx := t.Context() + setupAMAPIEnvVars(t) + + enterpriseID := s.enableAndroidMDM(t) + + // Create two teams with different certificate templates + teamAName := t.Name() + "-teamA" + var createTeamResp teamResponse + s.DoJSON("POST", "/api/latest/fleet/teams", createTeamRequest{ + TeamPayload: fleet.TeamPayload{ + Name: ptr.String(teamAName), + }, + }, http.StatusOK, &createTeamResp) + teamAID := createTeamResp.Team.ID + + teamBName := t.Name() + "-teamB" + s.DoJSON("POST", "/api/latest/fleet/teams", createTeamRequest{ + TeamPayload: fleet.TeamPayload{ + Name: ptr.String(teamBName), + }, + }, http.StatusOK, &createTeamResp) + teamBID := createTeamResp.Team.ID + + // Create enroll secrets for both teams + s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/teams/%d/secrets", teamAID), modifyTeamEnrollSecretsRequest{ + Secrets: []fleet.EnrollSecret{{Secret: "teamA-secret"}}, + }, http.StatusOK, &teamEnrollSecretsResponse{}) + s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/teams/%d/secrets", teamBID), modifyTeamEnrollSecretsRequest{ + Secrets: []fleet.EnrollSecret{{Secret: "teamB-secret"}}, + }, http.StatusOK, &teamEnrollSecretsResponse{}) + + // Create a test certificate authority + caID, _ := s.createTestCertificateAuthority(t, ctx) + + // Create certificate templates for Team A + certTemplateA1Name := strings.ReplaceAll(t.Name(), "/", "-") + "-TeamA-Cert1" + var createCertResp createCertificateTemplateResponse + s.DoJSON("POST", "/api/latest/fleet/certificates", createCertificateTemplateRequest{ + Name: certTemplateA1Name, + TeamID: teamAID, + CertificateAuthorityId: caID, + SubjectName: "CN=$FLEET_VAR_HOST_HARDWARE_SERIAL", + }, http.StatusOK, &createCertResp) + certTemplateA1ID := createCertResp.ID + + certTemplateA2Name := strings.ReplaceAll(t.Name(), "/", "-") + "-TeamA-Cert2" + s.DoJSON("POST", "/api/latest/fleet/certificates", createCertificateTemplateRequest{ + Name: certTemplateA2Name, + TeamID: teamAID, + CertificateAuthorityId: caID, + SubjectName: "CN=$FLEET_VAR_HOST_UUID", + }, http.StatusOK, &createCertResp) + certTemplateA2ID := createCertResp.ID + + // Create certificate templates for Team B + certTemplateB1Name := strings.ReplaceAll(t.Name(), "/", "-") + "-TeamB-Cert1" + s.DoJSON("POST", "/api/latest/fleet/certificates", createCertificateTemplateRequest{ + Name: certTemplateB1Name, + TeamID: teamBID, + CertificateAuthorityId: caID, + SubjectName: "CN=$FLEET_VAR_HOST_HARDWARE_SERIAL", + }, http.StatusOK, &createCertResp) + certTemplateB1ID := createCertResp.ID + + // Helper to get certificate template statuses for a host from the database + getCertTemplateStatuses := func(hostUUID string) map[uint]struct { + Status fleet.CertificateTemplateStatus + OperationType fleet.MDMOperationType + } { + result := make(map[uint]struct { + Status fleet.CertificateTemplateStatus + OperationType fleet.MDMOperationType + }) + var rows []struct { + CertificateTemplateID uint `db:"certificate_template_id"` + Status fleet.CertificateTemplateStatus `db:"status"` + OperationType fleet.MDMOperationType `db:"operation_type"` + } + mysql.ExecAdhocSQL(t, s.ds, func(q sqlx.ExtContext) error { + return sqlx.SelectContext(ctx, q, &rows, ` + SELECT certificate_template_id, status, operation_type + FROM host_certificate_templates + WHERE host_uuid = ? + `, hostUUID) + }) + for _, r := range rows { + result[r.CertificateTemplateID] = struct { + Status fleet.CertificateTemplateStatus + OperationType fleet.MDMOperationType + }{r.Status, r.OperationType} + } + return result + } + + // Set up AMAPI mock to succeed + s.androidAPIClient.EnterprisesPoliciesModifyPolicyApplicationsFunc = func(_ context.Context, _ string, _ []*androidmanagement.ApplicationPolicy) (*androidmanagement.Policy, error) { + return &androidmanagement.Policy{}, nil + } + + t.Run("host with certs in all status/operation combinations transfers to team without certs", func(t *testing.T) { + // Create a team with 9 certificate templates to test all status/operation combinations + teamEName := t.Name() + "-teamE" + s.DoJSON("POST", "/api/latest/fleet/teams", createTeamRequest{ + TeamPayload: fleet.TeamPayload{ + Name: ptr.String(teamEName), + }, + }, http.StatusOK, &createTeamResp) + teamEID := createTeamResp.Team.ID + + // Create enroll secret for team E + s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/teams/%d/secrets", teamEID), modifyTeamEnrollSecretsRequest{ + Secrets: []fleet.EnrollSecret{{Secret: "teamE-secret"}}, + }, http.StatusOK, &teamEnrollSecretsResponse{}) + + // Create a team without certificate templates for transfer target + teamFName := t.Name() + "-teamF" + s.DoJSON("POST", "/api/latest/fleet/teams", createTeamRequest{ + TeamPayload: fleet.TeamPayload{ + Name: ptr.String(teamFName), + }, + }, http.StatusOK, &createTeamResp) + teamFID := createTeamResp.Team.ID + + // Create enroll secret for team F + s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/teams/%d/secrets", teamFID), modifyTeamEnrollSecretsRequest{ + Secrets: []fleet.EnrollSecret{{Secret: "teamF-secret"}}, + }, http.StatusOK, &teamEnrollSecretsResponse{}) + + // Define all status/operation combinations to test + type certTestCase struct { + status fleet.CertificateTemplateStatus + operation fleet.MDMOperationType + shouldDelete bool // true if record should be deleted during transfer + expectedStatus fleet.CertificateTemplateStatus // expected status after transfer (if not deleted) + expectedOp fleet.MDMOperationType // expected operation after transfer (if not deleted) + templateID uint + templateName string + } + + testCases := []certTestCase{ + // Install operations: pending/failed are deleted, others are marked for removal + {fleet.CertificateTemplatePending, fleet.MDMOperationTypeInstall, true, "", "", 0, ""}, + {fleet.CertificateTemplateDelivering, fleet.MDMOperationTypeInstall, false, fleet.CertificateTemplatePending, fleet.MDMOperationTypeRemove, 0, ""}, + {fleet.CertificateTemplateDelivered, fleet.MDMOperationTypeInstall, false, fleet.CertificateTemplatePending, fleet.MDMOperationTypeRemove, 0, ""}, + {fleet.CertificateTemplateVerified, fleet.MDMOperationTypeInstall, false, fleet.CertificateTemplatePending, fleet.MDMOperationTypeRemove, 0, ""}, + {fleet.CertificateTemplateFailed, fleet.MDMOperationTypeInstall, true, "", "", 0, ""}, + // Remove operations: all stay unchanged (removal already in progress) + {fleet.CertificateTemplatePending, fleet.MDMOperationTypeRemove, false, fleet.CertificateTemplatePending, fleet.MDMOperationTypeRemove, 0, ""}, + {fleet.CertificateTemplateDelivering, fleet.MDMOperationTypeRemove, false, fleet.CertificateTemplateDelivering, fleet.MDMOperationTypeRemove, 0, ""}, + {fleet.CertificateTemplateDelivered, fleet.MDMOperationTypeRemove, false, fleet.CertificateTemplateDelivered, fleet.MDMOperationTypeRemove, 0, ""}, + {fleet.CertificateTemplateFailed, fleet.MDMOperationTypeRemove, false, fleet.CertificateTemplateFailed, fleet.MDMOperationTypeRemove, 0, ""}, + } + + // Create certificate templates for team E + for i := range testCases { + name := fmt.Sprintf("%s-Cert-%s-%s", strings.ReplaceAll(t.Name(), "/", "-"), testCases[i].status, testCases[i].operation) + s.DoJSON("POST", "/api/latest/fleet/certificates", createCertificateTemplateRequest{ + Name: name, + TeamID: teamEID, + CertificateAuthorityId: caID, + SubjectName: fmt.Sprintf("CN=Test-%d", i), + }, http.StatusOK, &createCertResp) + testCases[i].templateID = createCertResp.ID + testCases[i].templateName = name + } + + // Create host in Team E + host, _ := s.createEnrolledAndroidHost(t, ctx, enterpriseID, &teamEID, "all-statuses") + + // Insert certificate template records with all status/operation combinations + mysql.ExecAdhocSQL(t, s.ds, func(q sqlx.ExtContext) error { + for _, tc := range testCases { + challenge := "challenge" + if tc.status == fleet.CertificateTemplatePending || tc.status == fleet.CertificateTemplateFailed { + challenge = "" // No challenge for pending/failed + } + _, err := q.ExecContext(ctx, ` + INSERT INTO host_certificate_templates (host_uuid, certificate_template_id, status, operation_type, fleet_challenge, name) + VALUES (?, ?, ?, ?, NULLIF(?, ''), ?) + `, host.UUID, tc.templateID, tc.status, tc.operation, challenge, tc.templateName) + if err != nil { + return err + } + } + return nil + }) + + // Verify initial state - should have 9 certificate template records + statuses := getCertTemplateStatuses(host.UUID) + require.Len(t, statuses, 9, "Should have 9 certificate template records") + + // Transfer host to Team F (no certs) + s.DoJSON("POST", "/api/latest/fleet/hosts/transfer", addHostsToTeamRequest{ + TeamID: &teamFID, + HostIDs: []uint{host.ID}, + }, http.StatusOK, &addHostsToTeamResponse{}) + + // Run the worker to process the transfer + s.runWorker() + + // Verify results + statuses = getCertTemplateStatuses(host.UUID) + + // Count expected remaining records (those not deleted) + expectedRemaining := 0 + for _, tc := range testCases { + if !tc.shouldDelete { + expectedRemaining++ + } + } + require.Len(t, statuses, expectedRemaining, "Should have %d certificate template records after transfer", expectedRemaining) + + // Verify each test case + for _, tc := range testCases { + status, exists := statuses[tc.templateID] + if tc.shouldDelete { + require.False(t, exists, "Record for %s/%s should be deleted", tc.status, tc.operation) + } else { + require.True(t, exists, "Record for %s/%s should exist", tc.status, tc.operation) + require.Equal(t, tc.expectedStatus, status.Status, + "Record for %s/%s should have status=%s", tc.status, tc.operation, tc.expectedStatus) + require.Equal(t, tc.expectedOp, status.OperationType, + "Record for %s/%s should have operation_type=%s", tc.status, tc.operation, tc.expectedOp) + } + } + }) + + t.Run("host without certs transfers to team with certs", func(t *testing.T) { + // Create a team without certificate templates + teamDName := t.Name() + "-teamD" + s.DoJSON("POST", "/api/latest/fleet/teams", createTeamRequest{ + TeamPayload: fleet.TeamPayload{ + Name: ptr.String(teamDName), + }, + }, http.StatusOK, &createTeamResp) + teamDID := createTeamResp.Team.ID + + // Create enroll secret for team D + s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/teams/%d/secrets", teamDID), modifyTeamEnrollSecretsRequest{ + Secrets: []fleet.EnrollSecret{{Secret: "teamD-secret"}}, + }, http.StatusOK, &teamEnrollSecretsResponse{}) + + // Create host in Team D (no certs) + host, _ := s.createEnrolledAndroidHost(t, ctx, enterpriseID, &teamDID, "no-certs") + + // Verify no certificate templates for this host + statuses := getCertTemplateStatuses(host.UUID) + require.Empty(t, statuses, "Host should have no certificate templates initially") + + // Transfer host to Team B (has certs) + s.DoJSON("POST", "/api/latest/fleet/hosts/transfer", addHostsToTeamRequest{ + TeamID: &teamBID, + HostIDs: []uint{host.ID}, + }, http.StatusOK, &addHostsToTeamResponse{}) + + // Run the worker to process the transfer + s.runWorker() + + // Verify host now has Team B's certificate template as pending install + statuses = getCertTemplateStatuses(host.UUID) + require.Len(t, statuses, 1, "Host should have Team B's certificate template") + require.Equal(t, fleet.CertificateTemplatePending, statuses[certTemplateB1ID].Status) + require.Equal(t, fleet.MDMOperationTypeInstall, statuses[certTemplateB1ID].OperationType) + }) + + t.Run("host with certs transfers to team with different certs", func(t *testing.T) { + // Create host in Team A + host, orbitNodeKey := s.createEnrolledAndroidHost(t, ctx, enterpriseID, &teamAID, "transfer-certs") + + // Create pending certificate templates for this host (Team A certs) + _, err := s.ds.CreatePendingCertificateTemplatesForNewHost(ctx, host.UUID, teamAID) + require.NoError(t, err) + + // Set both certs to verified status (simulating they were both installed on device) + mysql.ExecAdhocSQL(t, s.ds, func(q sqlx.ExtContext) error { + _, err := q.ExecContext(ctx, ` + UPDATE host_certificate_templates + SET status = ?, fleet_challenge = 'challenge' + WHERE host_uuid = ? + `, fleet.CertificateTemplateVerified, host.UUID) + return err + }) + + // Verify initial state - host has Team A's certs (both verified) + statuses := getCertTemplateStatuses(host.UUID) + require.Len(t, statuses, 2) + require.Contains(t, statuses, certTemplateA1ID) + require.Contains(t, statuses, certTemplateA2ID) + require.Equal(t, fleet.CertificateTemplateVerified, statuses[certTemplateA1ID].Status) + require.Equal(t, fleet.CertificateTemplateVerified, statuses[certTemplateA2ID].Status) + + // Transfer host to Team B + s.DoJSON("POST", "/api/latest/fleet/hosts/transfer", addHostsToTeamRequest{ + TeamID: &teamBID, + HostIDs: []uint{host.ID}, + }, http.StatusOK, &addHostsToTeamResponse{}) + + // Run the worker to process the transfer + s.runWorker() + + // Verify: + // - Team A's certs (both verified/installed) are marked as pending remove + // - Team B's cert is added as pending install + statuses = getCertTemplateStatuses(host.UUID) + require.Len(t, statuses, 3, "Should have 2 old certs (pending remove) + 1 new cert (pending install)") + + // Team A certs should be pending remove + require.Equal(t, fleet.CertificateTemplatePending, statuses[certTemplateA1ID].Status) + require.Equal(t, fleet.MDMOperationTypeRemove, statuses[certTemplateA1ID].OperationType) + require.Equal(t, fleet.CertificateTemplatePending, statuses[certTemplateA2ID].Status) + require.Equal(t, fleet.MDMOperationTypeRemove, statuses[certTemplateA2ID].OperationType) + + // Team B cert should be pending install + require.Equal(t, fleet.CertificateTemplatePending, statuses[certTemplateB1ID].Status) + require.Equal(t, fleet.MDMOperationTypeInstall, statuses[certTemplateB1ID].OperationType) + + // Test that device can report "verified" for a pending removal and the record gets deleted. + // This handles race conditions where the device processes the removal before the server + // transitions the status through the full state machine. + updateReq, err := json.Marshal(updateCertificateStatusRequest{ + Status: string(fleet.CertificateTemplateVerified), + OperationType: ptr.String(string(fleet.MDMOperationTypeRemove)), + }) + require.NoError(t, err) + + resp := s.DoRawWithHeaders("PUT", fmt.Sprintf("/api/fleetd/certificates/%d/status", certTemplateA1ID), updateReq, http.StatusOK, map[string]string{ + "Authorization": fmt.Sprintf("Node key %s", orbitNodeKey), + }) + _ = resp.Body.Close() + + // Verify the record was deleted + statuses = getCertTemplateStatuses(host.UUID) + require.Len(t, statuses, 2, "Should have 1 pending remove + 1 pending install after removal confirmed") + _, exists := statuses[certTemplateA1ID] + require.False(t, exists, "certTemplateA1 should be deleted after verified removal") + }) +} diff --git a/server/service/integration_core_test.go b/server/service/integration_core_test.go index 83ebccd413..bda5f1df48 100644 --- a/server/service/integration_core_test.go +++ b/server/service/integration_core_test.go @@ -15163,7 +15163,7 @@ func (s *integrationTestSuite) TestDeleteCertificateTemplate() { // Verified status - should be updated to pending/remove _, err = q.ExecContext(ctx, insertSQL, hostVerified.UUID, certificateTemplateID, "verified", "install", "challenge2", certTemplateName) require.NoError(t, err) - // Failed status - should be updated to pending/remove + // Failed status - should be deleted (never successfully installed) _, err = q.ExecContext(ctx, insertSQL, hostFailed.UUID, certificateTemplateID, "failed", "install", "challenge3", certTemplateName) require.NoError(t, err) return nil @@ -15224,23 +15224,28 @@ func (s *integrationTestSuite) TestDeleteCertificateTemplate() { s.DoJSON("DELETE", fmt.Sprintf("/api/latest/fleet/certificates/%d", certificateTemplateID), nil, http.StatusOK, &deleteResp) // After deletion: - // - hostPending (pending/install) should have NO profile (record was deleted) - // - hostDelivered, hostVerified, hostFailed should have pending/remove profiles + // - hostPending (pending/install) should have NO profile (record was deleted - never installed) + // - hostFailed (failed/install) should have NO profile (record was deleted - never successfully installed) + // - hostDelivered, hostVerified should have pending/remove profiles // (kept for cron job to process removal from devices) - // Verify hostPending has no profile after deletion + // Verify hostPending has no profile after deletion (was pending/install, never installed) s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/hosts/%d", hostPending.ID), nil, http.StatusOK, &getHostResp) profile := findProfile(getHostResp.Host.MDM.Profiles, certTemplateName) require.Nil(t, profile, "hostPending should not have certificate template profile after deletion") - // Verify hosts that had delivered/verified/failed status now have pending/remove profiles + // Verify hostFailed has no profile after deletion (was failed/install, never successfully installed) + s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/hosts/%d", hostFailed.ID), nil, http.StatusOK, &getHostResp) + profile = findProfile(getHostResp.Host.MDM.Profiles, certTemplateName) + require.Nil(t, profile, "hostFailed should not have certificate template profile after deletion") + + // Verify hosts that had delivered/verified status now have pending/remove profiles for _, tc := range []struct { host *fleet.Host hostName string }{ {hostDelivered, "hostDelivered"}, {hostVerified, "hostVerified"}, - {hostFailed, "hostFailed"}, } { s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/hosts/%d", tc.host.ID), nil, http.StatusOK, &getHostResp) profile := findProfile(getHostResp.Host.MDM.Profiles, certTemplateName) @@ -15416,16 +15421,22 @@ func (s *integrationTestSuite) TestDeleteCertificateTemplateSpec() { require.True(t, fleet.IsNotFound(err), "certificate template 2 should be deleted") // After deletion: - // - hostPending (pending/install) should have NO profile (record was deleted) - // - hostDelivered, hostVerified, hostFailed should have pending/remove profiles + // - hostPending (pending/install) should have NO profile (record was deleted - never installed) + // - hostFailed (failed/install) should have NO profile (record was deleted - never successfully installed) + // - hostDelivered, hostVerified should have pending/remove profiles // (kept for cron job to process removal from devices) - // Verify hostPending has no profile after deletion + // Verify hostPending has no profile after deletion (was pending/install, never installed) s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/hosts/%d", hostPending.ID), nil, http.StatusOK, &getHostResp) profile := findProfile(getHostResp.Host.MDM.Profiles, certTemplateName1) require.Nil(t, profile, "hostPending should not have certificate template profile after deletion") - // Verify hosts that had delivered/verified/failed status now have pending/remove profiles + // Verify hostFailed has no profile after deletion (was failed/install, never successfully installed) + s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/hosts/%d", hostFailed.ID), nil, http.StatusOK, &getHostResp) + profile = findProfile(getHostResp.Host.MDM.Profiles, certTemplateName2) + require.Nil(t, profile, "hostFailed should not have certificate template profile after deletion") + + // Verify hosts that had delivered/verified status now have pending/remove profiles for _, tc := range []struct { host *fleet.Host hostName string @@ -15433,7 +15444,6 @@ func (s *integrationTestSuite) TestDeleteCertificateTemplateSpec() { }{ {hostDelivered, "hostDelivered", certTemplateName1}, {hostVerified, "hostVerified", certTemplateName2}, - {hostFailed, "hostFailed", certTemplateName2}, } { s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/hosts/%d", tc.host.ID), nil, http.StatusOK, &getHostResp) profile := findProfile(getHostResp.Host.MDM.Profiles, tc.templateName) diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go index cb8e35a10a..c0480d1022 100644 --- a/server/service/integration_enterprise_test.go +++ b/server/service/integration_enterprise_test.go @@ -23391,8 +23391,8 @@ func (s *integrationEnterpriseTestSuite) TestDeleteTeamCertificateTemplates() { // Verified status - should be updated to pending/remove _, err = q.ExecContext(ctx, insertSQL, hostVerified.UUID, certificateTemplateID2, "verified", "install", "challenge2", certTemplateName2) require.NoError(t, err) - // Failed status - should be updated to pending/remove - _, err = q.ExecContext(ctx, insertSQL, hostFailed.UUID, certificateTemplateID2, "failed", "install", "challenge3", certTemplateName2) + // Failed status - should be deleted (never successfully installed) + _, err = q.ExecContext(ctx, insertSQL, hostFailed.UUID, certificateTemplateID2, "failed", "install", nil, certTemplateName2) require.NoError(t, err) return nil }) @@ -23464,16 +23464,22 @@ func (s *integrationEnterpriseTestSuite) TestDeleteTeamCertificateTemplates() { require.True(t, fleet.IsNotFound(err), "certificate template 2 should be deleted") // After team deletion: - // - hostPending (pending/install) should have NO profile (record was deleted) - // - hostDelivered, hostVerified, hostFailed should have pending/remove profiles + // - hostPending (pending/install) should have NO profile (record was deleted - never installed) + // - hostFailed (failed/install) should have NO profile (record was deleted - never successfully installed) + // - hostDelivered, hostVerified should have pending/remove profiles // (kept for cron job to process removal from devices) - // Verify hostPending has no profile after deletion + // Verify hostPending has no profile after deletion (was pending/install, never installed) s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/hosts/%d", hostPending.ID), nil, http.StatusOK, &getHostResp) profile := findProfile(getHostResp.Host.MDM.Profiles, certTemplateName1) require.Nil(t, profile, "hostPending should not have certificate template profile after deletion") - // Verify hosts that had delivered/verified/failed status now have pending/remove profiles + // Verify hostFailed has no profile after deletion (was failed/install, never successfully installed) + s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/hosts/%d", hostFailed.ID), nil, http.StatusOK, &getHostResp) + profile = findProfile(getHostResp.Host.MDM.Profiles, certTemplateName2) + require.Nil(t, profile, "hostFailed should not have certificate template profile after deletion") + + // Verify hosts that had delivered/verified status now have pending/remove profiles for _, tc := range []struct { host *fleet.Host hostName string @@ -23481,7 +23487,6 @@ func (s *integrationEnterpriseTestSuite) TestDeleteTeamCertificateTemplates() { }{ {hostDelivered, "hostDelivered", certTemplateName1}, {hostVerified, "hostVerified", certTemplateName2}, - {hostFailed, "hostFailed", certTemplateName2}, } { s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/hosts/%d", tc.host.ID), nil, http.StatusOK, &getHostResp) profile := findProfile(getHostResp.Host.MDM.Profiles, tc.templateName) diff --git a/server/worker/software_worker.go b/server/worker/software_worker.go index bf4ff8e365..9997f73e58 100644 --- a/server/worker/software_worker.go +++ b/server/worker/software_worker.go @@ -476,12 +476,25 @@ func (v *SoftwareWorker) bulkSetAndroidAppsAvailableForHosts(ctx context.Context return ctxerr.Wrapf(ctx, err, "get android host by host UUID %s", uuid) } + teamID := ptr.ValOrZero(androidHost.TeamID) + + // Update certificate templates for team transfer: + // 1. Mark old templates as pending removal + // 2. Create new pending templates for the new team + // This must happen before building the managed config, which includes certificate template IDs. + if err := v.Datastore.SetHostCertificateTemplatesToPendingRemoveForHost(ctx, uuid); err != nil { + return ctxerr.Wrap(ctx, err, "set host certificate templates to pending remove for host") + } + if _, err := v.Datastore.CreatePendingCertificateTemplatesForNewHost(ctx, uuid, teamID); err != nil { + return ctxerr.Wrap(ctx, err, "create pending certificate templates for new host") + } + appIDs, err := v.Datastore.GetAndroidAppsInScopeForHost(ctx, hostID) if err != nil { return ctxerr.WrapWithData(ctx, err, "get android apps in scope for host", map[string]any{"host_id": hostID}) } - configsByAppID, err := v.Datastore.BulkGetAndroidAppConfigurations(ctx, appIDs, ptr.ValOrZero(androidHost.TeamID)) + configsByAppID, err := v.Datastore.BulkGetAndroidAppConfigurations(ctx, appIDs, teamID) if err != nil { return ctxerr.Wrap(ctx, err, "bulk get android app configurations") } diff --git a/server/worker/software_worker_test.go b/server/worker/software_worker_test.go index cc9cdeebdd..e4cd16363e 100644 --- a/server/worker/software_worker_test.go +++ b/server/worker/software_worker_test.go @@ -65,6 +65,12 @@ func TestBulkSetAndroidAppsAvailableForHostsPreservesFleetAgent(t *testing.T) { }, }, nil } + ds.SetHostCertificateTemplatesToPendingRemoveForHostFunc = func(ctx context.Context, hostUUID string) error { + return nil + } + ds.CreatePendingCertificateTemplatesForNewHostFunc = func(ctx context.Context, hostUUID string, teamID uint) (int64, error) { + return 0, nil + } ds.GetAndroidAppsInScopeForHostFunc = func(ctx context.Context, hostID uint) ([]string, error) { return []string{"com.example.teamapp"}, nil }