mirror of
https://github.com/fleetdm/fleet
synced 2026-04-21 13:37:30 +00:00
Windows profile delete fixes (#42495)
<!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> **Related issue:** Resolves #42452 - Editing a Windows profile to remove LocURIs now deletes those LocURIs - Removing a shared LocURI from one profile would NOT delete it even though another profile still uses it. - Loadtest fixes (batching, etc.) - Ordering commands by created to make sure a new profile AFTER a delete doesn't get deleted. # Checklist for submitter ## Testing - [x] Added/updated automated tests - [x] QA'd all new/changed functionality manually <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added profile change detection to identify and remove LocURIs when Windows profiles are edited. * **Bug Fixes** * Improved error logging when profile payload operations fail. * Enhanced pending command ordering for consistent processing. * Optimized profile deletion to prevent orphaned configurations across multiple profiles. * **Tests** * Added integration tests validating Windows profile edits with multi-part removals and shared LocURI protection. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
parent
e54ea7b3ad
commit
d84beaa43f
5 changed files with 472 additions and 134 deletions
|
|
@ -1,6 +1,7 @@
|
|||
package mysql
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"database/sql"
|
||||
"encoding/xml"
|
||||
|
|
@ -18,6 +19,11 @@ import (
|
|||
"github.com/jmoiron/sqlx"
|
||||
)
|
||||
|
||||
// windowsMDMProfileDeleteBatchSize is the number of hosts to process per
|
||||
// batch when enqueuing <Delete> commands and updating host profile rows
|
||||
// during profile deletion.
|
||||
const windowsMDMProfileDeleteBatchSize = 5000
|
||||
|
||||
func isWindowsHostConnectedToFleetMDM(ctx context.Context, q sqlx.QueryerContext, h *fleet.Host) (bool, error) {
|
||||
var unused string
|
||||
|
||||
|
|
@ -378,12 +384,32 @@ func (ds *Datastore) MDMWindowsInsertCommandForHosts(ctx context.Context, hostUU
|
|||
})
|
||||
}
|
||||
|
||||
func (ds *Datastore) mdmWindowsInsertCommandForHostsDB(ctx context.Context, tx sqlx.ExecerContext, hostUUIDsOrDeviceIDs []string, cmd *fleet.MDMWindowsCommand) error {
|
||||
// first, create the command entry
|
||||
stmt := `
|
||||
INSERT INTO windows_mdm_commands (command_uuid, raw_command, target_loc_uri)
|
||||
VALUES (?, ?, ?)
|
||||
`
|
||||
func (ds *Datastore) mdmWindowsInsertCommandForHostsDB(ctx context.Context, tx sqlx.ExtContext, hostUUIDsOrDeviceIDs []string, cmd *fleet.MDMWindowsCommand) error {
|
||||
// Resolve host UUIDs / device IDs to enrollment IDs using the general-purpose
|
||||
// lookup (supports both host_uuid and mdm_device_id via subquery).
|
||||
enrollmentIDs, err := ds.getEnrollmentIDsByHostUUIDOrDeviceIDDB(ctx, tx, hostUUIDsOrDeviceIDs)
|
||||
if err != nil {
|
||||
return ctxerr.Wrap(ctx, err, "fetching enrollment IDs for command queue")
|
||||
}
|
||||
return ds.mdmWindowsInsertCommandForEnrollmentIDsDB(ctx, tx, enrollmentIDs, cmd)
|
||||
}
|
||||
|
||||
// mdmWindowsInsertCommandForHostUUIDsDB is the fast path for bulk operations
|
||||
// that always have host UUIDs (not device IDs). Uses an indexed batch SELECT
|
||||
// instead of per-row subqueries.
|
||||
func (ds *Datastore) mdmWindowsInsertCommandForHostUUIDsDB(ctx context.Context, tx sqlx.ExtContext, hostUUIDs []string, cmd *fleet.MDMWindowsCommand) error {
|
||||
enrollmentIDs, err := ds.getEnrollmentIDsByHostUUIDDB(ctx, tx, hostUUIDs)
|
||||
if err != nil {
|
||||
return ctxerr.Wrap(ctx, err, "fetching enrollment IDs by host UUID")
|
||||
}
|
||||
return ds.mdmWindowsInsertCommandForEnrollmentIDsDB(ctx, tx, enrollmentIDs, cmd)
|
||||
}
|
||||
|
||||
// mdmWindowsInsertCommandForEnrollmentIDsDB inserts the command and queues it
|
||||
// for the given enrollment IDs.
|
||||
func (ds *Datastore) mdmWindowsInsertCommandForEnrollmentIDsDB(ctx context.Context, tx sqlx.ExtContext, enrollmentIDs []uint, cmd *fleet.MDMWindowsCommand) error {
|
||||
// Create the command entry.
|
||||
stmt := `INSERT INTO windows_mdm_commands (command_uuid, raw_command, target_loc_uri) VALUES (?, ?, ?)`
|
||||
if _, err := tx.ExecContext(ctx, stmt, cmd.CommandUUID, cmd.RawCommand, cmd.TargetLocURI); err != nil {
|
||||
if IsDuplicate(err) {
|
||||
return ctxerr.Wrap(ctx, alreadyExists("MDMWindowsCommand", cmd.CommandUUID))
|
||||
|
|
@ -391,29 +417,68 @@ func (ds *Datastore) mdmWindowsInsertCommandForHostsDB(ctx context.Context, tx s
|
|||
return ctxerr.Wrap(ctx, err, "inserting MDMWindowsCommand")
|
||||
}
|
||||
|
||||
// create the command execution queue entries, one per host
|
||||
for _, hostUUIDOrDeviceID := range hostUUIDsOrDeviceIDs {
|
||||
if err := ds.mdmWindowsInsertHostCommandDB(ctx, tx, hostUUIDOrDeviceID, cmd.CommandUUID); err != nil {
|
||||
return err
|
||||
}
|
||||
if len(enrollmentIDs) == 0 {
|
||||
return nil
|
||||
}
|
||||
return nil
|
||||
|
||||
// Batch insert into command queue.
|
||||
return common_mysql.BatchProcessSimple(enrollmentIDs, windowsMDMProfileDeleteBatchSize, func(batch []uint) error {
|
||||
valuesPart := strings.Repeat("(?, ?),", len(batch))
|
||||
valuesPart = strings.TrimSuffix(valuesPart, ",")
|
||||
|
||||
args := make([]any, 0, len(batch)*2)
|
||||
for _, eid := range batch {
|
||||
args = append(args, eid, cmd.CommandUUID)
|
||||
}
|
||||
|
||||
batchStmt := `INSERT INTO windows_mdm_command_queue (enrollment_id, command_uuid) VALUES ` + valuesPart
|
||||
if _, err := tx.ExecContext(ctx, batchStmt, args...); err != nil {
|
||||
return ctxerr.Wrap(ctx, err, "batch inserting MDMWindowsCommandQueue")
|
||||
}
|
||||
return nil
|
||||
})
|
||||
}
|
||||
|
||||
func (ds *Datastore) mdmWindowsInsertHostCommandDB(ctx context.Context, tx sqlx.ExecerContext, hostUUIDOrDeviceID, commandUUID string) error {
|
||||
stmt := `
|
||||
INSERT INTO windows_mdm_command_queue (enrollment_id, command_uuid)
|
||||
VALUES ((SELECT id FROM mdm_windows_enrollments WHERE host_uuid = ? OR mdm_device_id = ? ORDER BY created_at DESC LIMIT 1), ?)
|
||||
`
|
||||
|
||||
if _, err := tx.ExecContext(ctx, stmt, hostUUIDOrDeviceID, hostUUIDOrDeviceID, commandUUID); err != nil {
|
||||
if IsDuplicate(err) {
|
||||
return ctxerr.Wrap(ctx, alreadyExists("MDMWindowsCommandQueue", commandUUID))
|
||||
// getEnrollmentIDsByHostUUIDDB fetches enrollment IDs for a list of host UUIDs
|
||||
// using an indexed batch query. Returns the most recent enrollment per host.
|
||||
func (ds *Datastore) getEnrollmentIDsByHostUUIDDB(ctx context.Context, tx sqlx.ExtContext, hostUUIDs []string) ([]uint, error) {
|
||||
var allIDs []uint
|
||||
err := common_mysql.BatchProcessSimple(hostUUIDs, windowsMDMProfileDeleteBatchSize, func(batch []string) error {
|
||||
stmt, args, err := sqlx.In(
|
||||
`SELECT MAX(id) FROM mdm_windows_enrollments WHERE host_uuid IN (?) GROUP BY host_uuid`,
|
||||
batch)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
return ctxerr.Wrap(ctx, err, "inserting MDMWindowsCommandQueue", "host_uuid_or_device_id", hostUUIDOrDeviceID, "command_uuid", commandUUID)
|
||||
}
|
||||
var ids []uint
|
||||
if err := sqlx.SelectContext(ctx, tx, &ids, stmt, args...); err != nil {
|
||||
return err
|
||||
}
|
||||
allIDs = append(allIDs, ids...)
|
||||
return nil
|
||||
})
|
||||
return allIDs, err
|
||||
}
|
||||
|
||||
return nil
|
||||
// getEnrollmentIDsByHostUUIDOrDeviceIDDB fetches enrollment IDs using a
|
||||
// per-row SELECT that supports both host_uuid and mdm_device_id lookups.
|
||||
// Used by the general-purpose command insertion path (typically 1-2 IDs).
|
||||
func (ds *Datastore) getEnrollmentIDsByHostUUIDOrDeviceIDDB(ctx context.Context, tx sqlx.ExtContext, hostUUIDsOrDeviceIDs []string) ([]uint, error) {
|
||||
var allIDs []uint
|
||||
for _, id := range hostUUIDsOrDeviceIDs {
|
||||
var eid uint
|
||||
err := sqlx.GetContext(ctx, tx, &eid,
|
||||
`SELECT id FROM mdm_windows_enrollments WHERE host_uuid = ? OR mdm_device_id = ? ORDER BY created_at DESC LIMIT 1`,
|
||||
id, id)
|
||||
if err != nil {
|
||||
if errors.Is(err, sql.ErrNoRows) {
|
||||
continue // host not enrolled, skip
|
||||
}
|
||||
return nil, ctxerr.Wrap(ctx, err, "looking up enrollment ID")
|
||||
}
|
||||
allIDs = append(allIDs, eid)
|
||||
}
|
||||
return allIDs, nil
|
||||
}
|
||||
|
||||
// MDMWindowsGetPendingCommands retrieves all commands awaiting execution for a
|
||||
|
|
@ -448,6 +513,8 @@ WHERE
|
|||
wmcr.enrollment_id = wmcq.enrollment_id AND
|
||||
wmcr.command_uuid = wmcq.command_uuid
|
||||
)
|
||||
ORDER BY
|
||||
wmc.created_at ASC
|
||||
`
|
||||
|
||||
if err := sqlx.SelectContext(ctx, ds.reader(ctx), &commands, query, deviceID); err != nil {
|
||||
|
|
@ -559,7 +626,7 @@ func (ds *Datastore) MDMWindowsSaveResponse(ctx context.Context, deviceID string
|
|||
pp, err := fleet.BuildMDMWindowsProfilePayloadFromMDMResponse(cmdWithSecret, enrichedSyncML.CmdRefUUIDToStatus,
|
||||
enrolledDevice.HostUUID, cmdOperationTypes[cmd.CommandUUID] == fleet.MDMOperationTypeRemove)
|
||||
if err != nil {
|
||||
return err
|
||||
return ctxerr.Wrap(ctx, err, "building profile payload from MDM response")
|
||||
}
|
||||
potentialProfilePayloads = append(potentialProfilePayloads, pp)
|
||||
}
|
||||
|
|
@ -1201,15 +1268,18 @@ func (ds *Datastore) cancelWindowsHostInstallsForDeletedMDMProfiles(
|
|||
// This prevents deleting one profile from undoing settings in another.
|
||||
activeLocURIs := make(map[string]bool)
|
||||
if len(profileUUIDs) > 0 {
|
||||
// Get team IDs for the profiles being deleted (from host assignments).
|
||||
// Get team IDs for the profiles being deleted. Use the hosts table
|
||||
// with a single query instead of one per host.
|
||||
teamIDStmt, teamIDArgs, err := sqlx.In(
|
||||
`SELECT DISTINCT COALESCE(h.team_id, 0) FROM hosts h
|
||||
JOIN host_mdm_windows_profiles hwp ON hwp.host_uuid = h.uuid
|
||||
WHERE hwp.profile_uuid IN (?)`, profileUUIDs)
|
||||
if err != nil {
|
||||
return ctxerr.Wrap(ctx, err, "building IN for team ID lookup")
|
||||
}
|
||||
var teamIDs []uint
|
||||
for _, row := range rowsToRemove {
|
||||
// Look up team from the host.
|
||||
var teamID uint
|
||||
if err := sqlx.GetContext(ctx, tx, &teamID,
|
||||
`SELECT COALESCE(team_id, 0) FROM hosts WHERE uuid = ?`, row.HostUUID); err == nil {
|
||||
teamIDs = append(teamIDs, teamID)
|
||||
}
|
||||
if err := sqlx.SelectContext(ctx, tx, &teamIDs, teamIDStmt, teamIDArgs...); err != nil {
|
||||
return ctxerr.Wrap(ctx, err, "selecting team IDs for LocURI protection")
|
||||
}
|
||||
if len(teamIDs) > 0 {
|
||||
// Query all SyncML from profiles NOT being deleted in the same team(s).
|
||||
|
|
@ -1221,14 +1291,8 @@ func (ds *Datastore) cancelWindowsHostInstallsForDeletedMDMProfiles(
|
|||
var activeProfiles [][]byte
|
||||
if err := sqlx.SelectContext(ctx, tx, &activeProfiles, apStmt, apArgs...); err == nil {
|
||||
for _, syncML := range activeProfiles {
|
||||
cmds, err := fleet.UnmarshallMultiTopLevelXMLProfile(syncML)
|
||||
if err != nil {
|
||||
continue
|
||||
}
|
||||
for _, cmd := range cmds {
|
||||
if uri := cmd.GetTargetURI(); uri != "" {
|
||||
activeLocURIs[uri] = true
|
||||
}
|
||||
for _, uri := range fleet.ExtractLocURIsFromProfileBytes(syncML) {
|
||||
activeLocURIs[uri] = true
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -1268,7 +1332,7 @@ func (ds *Datastore) cancelWindowsHostInstallsForDeletedMDMProfiles(
|
|||
continue
|
||||
}
|
||||
|
||||
if err := ds.mdmWindowsInsertCommandForHostsDB(ctx, tx, target.hostUUIDs, deleteCmd); err != nil {
|
||||
if err := ds.mdmWindowsInsertCommandForHostUUIDsDB(ctx, tx, target.hostUUIDs, deleteCmd); err != nil {
|
||||
return ctxerr.Wrap(ctx, err, "inserting delete commands for hosts")
|
||||
}
|
||||
enqueuedTargets[profUUID] = target
|
||||
|
|
@ -1278,18 +1342,23 @@ func (ds *Datastore) cancelWindowsHostInstallsForDeletedMDMProfiles(
|
|||
// This covers both install rows (being flipped to remove) and remove+NULL rows
|
||||
// (being given a command_uuid and set to pending).
|
||||
for profUUID, target := range enqueuedTargets {
|
||||
upStmt, upArgs, err := sqlx.In(
|
||||
`UPDATE host_mdm_windows_profiles
|
||||
SET operation_type = ?, status = ?, command_uuid = ?, detail = ''
|
||||
WHERE profile_uuid = ? AND host_uuid IN (?)`,
|
||||
fleet.MDMOperationTypeRemove, fleet.MDMDeliveryPending, target.cmdUUID,
|
||||
profUUID, target.hostUUIDs,
|
||||
)
|
||||
if err != nil {
|
||||
return ctxerr.Wrap(ctx, err, "building IN for phase 2 update")
|
||||
}
|
||||
if _, err := tx.ExecContext(ctx, upStmt, upArgs...); err != nil {
|
||||
return ctxerr.Wrap(ctx, err, "updating host profiles to remove")
|
||||
if err := common_mysql.BatchProcessSimple(target.hostUUIDs, windowsMDMProfileDeleteBatchSize, func(batch []string) error {
|
||||
upStmt, upArgs, err := sqlx.In(
|
||||
`UPDATE host_mdm_windows_profiles
|
||||
SET operation_type = ?, status = ?, command_uuid = ?, detail = ''
|
||||
WHERE profile_uuid = ? AND host_uuid IN (?)`,
|
||||
fleet.MDMOperationTypeRemove, fleet.MDMDeliveryPending, target.cmdUUID,
|
||||
profUUID, batch,
|
||||
)
|
||||
if err != nil {
|
||||
return ctxerr.Wrap(ctx, err, "building IN for phase 2 update")
|
||||
}
|
||||
if _, err := tx.ExecContext(ctx, upStmt, upArgs...); err != nil {
|
||||
return ctxerr.Wrap(ctx, err, "updating host profiles to remove")
|
||||
}
|
||||
return nil
|
||||
}); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -2560,6 +2629,82 @@ ON DUPLICATE KEY UPDATE
|
|||
}
|
||||
}
|
||||
|
||||
// For profiles being updated (same name, different content), diff the old
|
||||
// and new LocURIs. Generate <Delete> commands for LocURIs that were removed
|
||||
// so the device reverts those settings.
|
||||
//
|
||||
// This is an edge case (most edits change values, not remove LocURIs).
|
||||
// The delete commands are best-effort and currently not visible to the
|
||||
// IT admin in the UI or API. They are fire-and-forget MDM commands
|
||||
// with no corresponding host_mdm_windows_profiles status entry.
|
||||
// Collect all LocURIs across incoming profiles AND reserved profiles that
|
||||
// are kept even when not in the incoming list (e.g. "Windows OS Updates").
|
||||
// This prevents deleting LocURIs that are still enforced by any profile
|
||||
// that will remain after the batch-set.
|
||||
allRetainedURIs := make(map[string]bool)
|
||||
for _, p := range incomingProfs {
|
||||
for _, uri := range fleet.ExtractLocURIsFromProfileBytes(p.SyncML) {
|
||||
allRetainedURIs[uri] = true
|
||||
}
|
||||
}
|
||||
// Include LocURIs from reserved profiles that are always kept.
|
||||
for _, ep := range existingProfiles {
|
||||
if _, isReserved := mdm.FleetReservedProfileNames()[ep.Name]; isReserved {
|
||||
for _, uri := range fleet.ExtractLocURIsFromProfileBytes(ep.SyncML) {
|
||||
allRetainedURIs[uri] = true
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
for _, existing := range existingProfiles {
|
||||
incoming := incomingProfs[existing.Name]
|
||||
if incoming == nil || bytes.Equal(existing.SyncML, incoming.SyncML) {
|
||||
continue
|
||||
}
|
||||
|
||||
oldURIs := fleet.ExtractLocURIsFromProfileBytes(existing.SyncML)
|
||||
newURIs := fleet.ExtractLocURIsFromProfileBytes(incoming.SyncML)
|
||||
|
||||
newSet := make(map[string]bool, len(newURIs))
|
||||
for _, u := range newURIs {
|
||||
newSet[u] = true
|
||||
}
|
||||
|
||||
var removedURIs []string
|
||||
for _, u := range oldURIs {
|
||||
// Skip if the LocURI is still in the updated version of this profile,
|
||||
// or if another profile in the batch still targets it.
|
||||
if !newSet[u] && !allRetainedURIs[u] {
|
||||
removedURIs = append(removedURIs, u)
|
||||
}
|
||||
}
|
||||
|
||||
if len(removedURIs) == 0 {
|
||||
continue
|
||||
}
|
||||
|
||||
cmdUUID := uuid.NewString()
|
||||
deleteCmd, err := fleet.BuildDeleteCommandFromLocURIs(removedURIs, cmdUUID)
|
||||
if err != nil || deleteCmd == nil {
|
||||
continue
|
||||
}
|
||||
|
||||
// Find hosts that have this profile installed (not pending removal).
|
||||
var hostUUIDs []string
|
||||
if err := sqlx.SelectContext(ctx, tx, &hostUUIDs,
|
||||
`SELECT host_uuid FROM host_mdm_windows_profiles WHERE profile_uuid = ? AND operation_type = ? AND status IS NOT NULL`,
|
||||
existing.ProfileUUID, fleet.MDMOperationTypeInstall); err != nil {
|
||||
return false, ctxerr.Wrap(ctx, err, "selecting hosts for edited profile LocURI cleanup")
|
||||
}
|
||||
if len(hostUUIDs) > 0 {
|
||||
ds.logger.InfoContext(ctx, "sending delete commands for LocURIs removed from edited profile",
|
||||
"profile.name", existing.Name, "profile.uuid", existing.ProfileUUID, "removed_loc_uris", len(removedURIs))
|
||||
if err := ds.mdmWindowsInsertCommandForHostUUIDsDB(ctx, tx, hostUUIDs, deleteCmd); err != nil {
|
||||
return false, ctxerr.Wrap(ctx, err, "inserting delete commands for removed LocURIs")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// insert the new profiles and the ones that have changed
|
||||
for _, p := range incomingProfs {
|
||||
if result, err = tx.ExecContext(ctx, insertNewOrEditedProfile, profTeamID, p.Name,
|
||||
|
|
|
|||
|
|
@ -1754,90 +1754,49 @@ func BuildDeleteCommandFromProfileBytes(profileBytes []byte, commandUUID string,
|
|||
normalized = fmt.Appendf([]byte{}, "<Atomic>%s</Atomic>", normalized)
|
||||
}
|
||||
|
||||
cmds, err := UnmarshallMultiTopLevelXMLProfile(normalized)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("unmarshalling profile bytes for delete: %w", err)
|
||||
}
|
||||
if len(cmds) == 0 {
|
||||
return nil, errors.New("no commands found in profile")
|
||||
allURIs := ExtractLocURIsFromProfileBytes(normalized)
|
||||
if len(allURIs) == 0 {
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
// extractLocURIs collects all Target LocURIs from Replace and Add commands. Exec commands are intentionally excluded.
|
||||
extractLocURIs := func(cmd *SyncMLCmd) []string {
|
||||
var uris []string
|
||||
for _, nested := range cmd.ReplaceCommands {
|
||||
if uri := nested.GetTargetURI(); uri != "" {
|
||||
uris = append(uris, uri)
|
||||
}
|
||||
}
|
||||
for _, nested := range cmd.AddCommands {
|
||||
if uri := nested.GetTargetURI(); uri != "" {
|
||||
uris = append(uris, uri)
|
||||
}
|
||||
}
|
||||
return uris
|
||||
}
|
||||
|
||||
// makeDeleteCmd creates a single <Delete> SyncMLCmd for the given LocURI.
|
||||
makeDeleteCmd := func(locURI string, cmdID string) SyncMLCmd {
|
||||
target := locURI
|
||||
return SyncMLCmd{
|
||||
XMLName: xml.Name{Local: CmdDelete},
|
||||
CmdID: CmdID{Value: cmdID, IncludeFleetComment: true},
|
||||
Items: []CmdItem{{Target: &target}},
|
||||
}
|
||||
}
|
||||
|
||||
// Build the set of protected LocURIs from the variadic parameter.
|
||||
// Filter out LocURIs that are still targeted by other active profiles.
|
||||
inUse := make(map[string]bool)
|
||||
if len(locURIsInUseByOtherProfiles) > 0 && locURIsInUseByOtherProfiles[0] != nil {
|
||||
inUse = locURIsInUseByOtherProfiles[0]
|
||||
}
|
||||
|
||||
// safeToDelete returns true if the LocURI is safe to delete (no other
|
||||
// active profile targets it).
|
||||
safeToDelete := func(uri string) bool {
|
||||
return !inUse[uri]
|
||||
}
|
||||
|
||||
var rawCommand []byte
|
||||
|
||||
// Collect all LocURIs to delete. For Atomic profiles, extract from nested
|
||||
// commands; for non-atomic, extract from top-level commands.
|
||||
var allURIs []string
|
||||
switch {
|
||||
case len(cmds) == 1 && cmds[0].XMLName.Local == CmdAtomic:
|
||||
allURIs = extractLocURIs(&cmds[0])
|
||||
default:
|
||||
for _, cmd := range cmds {
|
||||
if cmd.XMLName.Local == CmdExec {
|
||||
continue
|
||||
}
|
||||
if uri := cmd.GetTargetURI(); uri != "" {
|
||||
allURIs = append(allURIs, uri)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Filter out protected URIs and build individual <Delete> commands.
|
||||
// Never wrap in <Atomic> — removal is best-effort, and <Atomic> would
|
||||
// cause all deletes to roll back (507) if any single one fails.
|
||||
var deleteCmds []SyncMLCmd
|
||||
var safeURIs []string
|
||||
for _, uri := range allURIs {
|
||||
if !safeToDelete(uri) {
|
||||
continue
|
||||
if !inUse[uri] {
|
||||
safeURIs = append(safeURIs, uri)
|
||||
}
|
||||
}
|
||||
|
||||
return BuildDeleteCommandFromLocURIs(safeURIs, commandUUID)
|
||||
}
|
||||
|
||||
// BuildDeleteCommandFromLocURIs generates individual <Delete> commands for
|
||||
// the given LocURIs. Returns nil if locURIs is empty.
|
||||
func BuildDeleteCommandFromLocURIs(locURIs []string, commandUUID string) (*MDMWindowsCommand, error) {
|
||||
if len(locURIs) == 0 {
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
var deleteCmds []SyncMLCmd
|
||||
for _, uri := range locURIs {
|
||||
cmdID := uuid.NewString()
|
||||
if len(deleteCmds) == 0 {
|
||||
cmdID = commandUUID
|
||||
}
|
||||
deleteCmds = append(deleteCmds, makeDeleteCmd(uri, cmdID))
|
||||
}
|
||||
if len(deleteCmds) == 0 {
|
||||
return nil, nil
|
||||
target := uri
|
||||
deleteCmds = append(deleteCmds, SyncMLCmd{
|
||||
XMLName: xml.Name{Local: CmdDelete},
|
||||
CmdID: CmdID{Value: cmdID, IncludeFleetComment: true},
|
||||
Items: []CmdItem{{Target: &target}},
|
||||
})
|
||||
}
|
||||
|
||||
rawCommand, err = xml.Marshal(deleteCmds)
|
||||
rawCommand, err := xml.Marshal(deleteCmds)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("marshalling delete commands: %w", err)
|
||||
}
|
||||
|
|
@ -1870,3 +1829,41 @@ func UnmarshallMultiTopLevelXMLProfile(profileBytes []byte) ([]SyncMLCmd, error)
|
|||
|
||||
return root.Commands, nil
|
||||
}
|
||||
|
||||
// ExtractLocURIsFromProfileBytes returns all Target LocURIs found in the
|
||||
// profile's Replace and Add commands. Exec commands are excluded (they
|
||||
// trigger one-time actions, not persistent settings). For Atomic profiles,
|
||||
// nested commands are inspected.
|
||||
func ExtractLocURIsFromProfileBytes(profileBytes []byte) []string {
|
||||
// Mirror the install-side SCEP normalization.
|
||||
normalized := profileBytes
|
||||
if strings.Contains(string(normalized), WINDOWS_SCEP_LOC_URI_PART) && !strings.Contains(string(normalized), "<Atomic>") {
|
||||
normalized = fmt.Appendf([]byte{}, "<Atomic>%s</Atomic>", normalized)
|
||||
}
|
||||
|
||||
cmds, err := UnmarshallMultiTopLevelXMLProfile(normalized)
|
||||
if err != nil || len(cmds) == 0 {
|
||||
return nil
|
||||
}
|
||||
|
||||
var uris []string
|
||||
for _, cmd := range cmds {
|
||||
if cmd.XMLName.Local == CmdAtomic {
|
||||
for _, nested := range cmd.ReplaceCommands {
|
||||
if uri := nested.GetTargetURI(); uri != "" {
|
||||
uris = append(uris, uri)
|
||||
}
|
||||
}
|
||||
for _, nested := range cmd.AddCommands {
|
||||
if uri := nested.GetTargetURI(); uri != "" {
|
||||
uris = append(uris, uri)
|
||||
}
|
||||
}
|
||||
} else if cmd.XMLName.Local == CmdReplace || cmd.XMLName.Local == CmdAdd {
|
||||
if uri := cmd.GetTargetURI(); uri != "" {
|
||||
uris = append(uris, uri)
|
||||
}
|
||||
}
|
||||
}
|
||||
return uris
|
||||
}
|
||||
|
|
|
|||
|
|
@ -540,9 +540,9 @@ func TestBuildDeleteCommandFromProfileBytes(t *testing.T) {
|
|||
expectNil: true,
|
||||
},
|
||||
{
|
||||
name: "empty profile",
|
||||
profileXML: "",
|
||||
expectError: "no commands found in profile",
|
||||
name: "empty profile returns nil",
|
||||
profileXML: "",
|
||||
expectNil: true,
|
||||
},
|
||||
}
|
||||
|
||||
|
|
@ -780,3 +780,35 @@ func TestBuildMDMWindowsProfilePayloadFromMDMResponseRemoveOperation(t *testing.
|
|||
require.Equal(t, MDMDeliveryVerified, *payload.Status)
|
||||
})
|
||||
}
|
||||
|
||||
func TestExtractLocURIsFromProfileBytes(t *testing.T) {
|
||||
t.Parallel()
|
||||
t.Run("atomic profile", func(t *testing.T) {
|
||||
xml := `<Atomic><Replace><Item><Target><LocURI>./Device/A</LocURI></Target></Item></Replace><Replace><Item><Target><LocURI>./Device/B</LocURI></Target></Item></Replace></Atomic>`
|
||||
uris := ExtractLocURIsFromProfileBytes([]byte(xml))
|
||||
require.Equal(t, []string{"./Device/A", "./Device/B"}, uris)
|
||||
})
|
||||
|
||||
t.Run("non-atomic profile", func(t *testing.T) {
|
||||
xml := `<Replace><Item><Target><LocURI>./Device/X</LocURI></Target></Item></Replace><Replace><Item><Target><LocURI>./Device/Y</LocURI></Target></Item></Replace>`
|
||||
uris := ExtractLocURIsFromProfileBytes([]byte(xml))
|
||||
require.Equal(t, []string{"./Device/X", "./Device/Y"}, uris)
|
||||
})
|
||||
|
||||
t.Run("exec commands excluded", func(t *testing.T) {
|
||||
xml := `<Replace><Item><Target><LocURI>./Device/A</LocURI></Target></Item></Replace><Exec><Item><Target><LocURI>./Device/Enroll</LocURI></Target></Item></Exec>`
|
||||
uris := ExtractLocURIsFromProfileBytes([]byte(xml))
|
||||
require.Equal(t, []string{"./Device/A"}, uris)
|
||||
})
|
||||
|
||||
t.Run("empty profile", func(t *testing.T) {
|
||||
uris := ExtractLocURIsFromProfileBytes([]byte(""))
|
||||
require.Nil(t, uris)
|
||||
})
|
||||
|
||||
t.Run("delete commands excluded", func(t *testing.T) {
|
||||
xml := `<Replace><Item><Target><LocURI>./Device/A</LocURI></Target></Item></Replace><Delete><Item><Target><LocURI>./Device/B</LocURI></Target></Item></Delete>`
|
||||
uris := ExtractLocURIsFromProfileBytes([]byte(xml))
|
||||
require.Equal(t, []string{"./Device/A"}, uris)
|
||||
})
|
||||
}
|
||||
|
|
|
|||
|
|
@ -4423,6 +4423,176 @@ func (s *integrationMDMTestSuite) TestWindowsProfileManagement() {
|
|||
res = s.DoRaw("POST", fmt.Sprintf("/api/latest/fleet/hosts/%d/configuration_profiles/%s/resend", host.ID, mcUUID), nil, http.StatusUnprocessableEntity)
|
||||
errMsg = extractServerErrorText(res.Body)
|
||||
require.Contains(t, errMsg, "Profile is not compatible with host platform")
|
||||
|
||||
t.Run("edit_profile_removes_locuri_sends_delete", func(t *testing.T) {
|
||||
// Editing a profile to remove a LocURI should send a <Delete> for the removed LocURI.
|
||||
// Create a profile with two LocURIs via batch-set, then edit it to have only one.
|
||||
// The device should receive a <Delete> for the removed LocURI plus an install for the updated profile.
|
||||
|
||||
// First, clean up: use batch-set with only our new test profile to remove all
|
||||
// existing team profiles cleanly (this goes through the proper deletion path).
|
||||
twoLocURIProfile := `<Atomic><Replace><Item><Target><LocURI>./Device/Vendor/MSFT/Policy/Config/Camera/AllowCamera</LocURI></Target><Meta><Format xmlns="syncml:metinf">int</Format></Meta><Data>0</Data></Item></Replace><Replace><Item><Target><LocURI>./Device/Vendor/MSFT/Policy/Config/Experience/AllowCortana</LocURI></Target><Meta><Format xmlns="syncml:metinf">int</Format></Meta><Data>0</Data></Item></Replace></Atomic>`
|
||||
// Also disable OS updates to simplify
|
||||
tmResp := teamResponse{}
|
||||
s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/teams/%d", tm.ID), json.RawMessage(`{"mdm": { "windows_updates": {"deadline_days": null, "grace_period_days": null} }}`), http.StatusOK, &tmResp)
|
||||
s.Do("POST", "/api/v1/fleet/mdm/profiles/batch",
|
||||
batchSetMDMProfilesRequest{Profiles: []fleet.MDMProfileBatchPayload{
|
||||
{Name: "edit-locuri-test", Contents: []byte(twoLocURIProfile)},
|
||||
}}, http.StatusNoContent, "team_id", fmt.Sprint(tm.ID))
|
||||
|
||||
// Trigger profile sync: device gets the new profile + delete commands for old profiles.
|
||||
// Drain all commands from the device without asserting exact counts (cleanup varies).
|
||||
s.awaitTriggerProfileSchedule(t)
|
||||
cmds, err := mdmDevice.StartManagementSession()
|
||||
require.NoError(t, err)
|
||||
msgID, err := mdmDevice.GetCurrentMsgID()
|
||||
require.NoError(t, err)
|
||||
for _, c := range cmds {
|
||||
cmdID := c.Cmd.CmdID
|
||||
status := syncml.CmdStatusOK
|
||||
mdmDevice.AppendResponse(fleet.SyncMLCmd{
|
||||
XMLName: xml.Name{Local: fleet.CmdStatus},
|
||||
MsgRef: &msgID,
|
||||
CmdRef: &cmdID.Value,
|
||||
Cmd: ptr.String(c.Verb),
|
||||
Data: &status,
|
||||
CmdID: fleet.CmdID{Value: uuid.NewString()},
|
||||
})
|
||||
}
|
||||
_, err = mdmDevice.SendResponse()
|
||||
require.NoError(t, err)
|
||||
|
||||
// Drain any remaining commands until the device is clean.
|
||||
verifyProfiles(mdmDevice, 0, false)
|
||||
|
||||
// Now edit the profile to remove AllowCortana, keeping only AllowCamera.
|
||||
oneLocURIProfile := `<Atomic><Replace><Item><Target><LocURI>./Device/Vendor/MSFT/Policy/Config/Camera/AllowCamera</LocURI></Target><Meta><Format xmlns="syncml:metinf">int</Format></Meta><Data>0</Data></Item></Replace></Atomic>`
|
||||
s.Do("POST", "/api/v1/fleet/mdm/profiles/batch",
|
||||
batchSetMDMProfilesRequest{Profiles: []fleet.MDMProfileBatchPayload{
|
||||
{Name: "edit-locuri-test", Contents: []byte(oneLocURIProfile)},
|
||||
}}, http.StatusNoContent, "team_id", fmt.Sprint(tm.ID))
|
||||
|
||||
// Trigger profile sync: device should get:
|
||||
// - 1 <Delete> for the removed AllowCortana LocURI (from batchSet edit diff)
|
||||
// - 1 Atomic install for the updated profile (from reconciler checksum mismatch)
|
||||
// Total: 2 Status + 1 Delete + 1 Atomic = 4 commands
|
||||
s.awaitTriggerProfileSchedule(t)
|
||||
cmds, err = mdmDevice.StartManagementSession()
|
||||
require.NoError(t, err)
|
||||
require.Len(t, cmds, 4, "expected 2 status + 1 delete + 1 install")
|
||||
|
||||
var gotDelete, gotInstall bool
|
||||
msgID, err = mdmDevice.GetCurrentMsgID()
|
||||
require.NoError(t, err)
|
||||
for _, c := range cmds {
|
||||
cmdID := c.Cmd.CmdID
|
||||
status := syncml.CmdStatusOK
|
||||
switch c.Verb {
|
||||
case "Delete":
|
||||
gotDelete = true
|
||||
require.Contains(t, c.Cmd.GetTargetURI(), "AllowCortana",
|
||||
"Delete should target the removed LocURI")
|
||||
case "Atomic":
|
||||
if len(c.Cmd.ReplaceCommands) > 0 {
|
||||
gotInstall = true
|
||||
// Verify the install only has AllowCamera, not AllowCortana
|
||||
var installURIs []string
|
||||
for _, rc := range c.Cmd.ReplaceCommands {
|
||||
installURIs = append(installURIs, rc.GetTargetURI())
|
||||
}
|
||||
require.Len(t, installURIs, 1)
|
||||
require.Contains(t, installURIs[0], "AllowCamera")
|
||||
}
|
||||
}
|
||||
mdmDevice.AppendResponse(fleet.SyncMLCmd{
|
||||
XMLName: xml.Name{Local: fleet.CmdStatus},
|
||||
MsgRef: &msgID,
|
||||
CmdRef: &cmdID.Value,
|
||||
Cmd: ptr.String(c.Verb),
|
||||
Data: &status,
|
||||
Items: nil,
|
||||
CmdID: fleet.CmdID{Value: uuid.NewString()},
|
||||
})
|
||||
}
|
||||
require.True(t, gotDelete, "expected a Delete command for the removed LocURI")
|
||||
require.True(t, gotInstall, "expected an install command for the updated profile")
|
||||
|
||||
cmds, err = mdmDevice.SendResponse()
|
||||
require.NoError(t, err)
|
||||
require.Len(t, cmds, 1) // just the ack
|
||||
})
|
||||
|
||||
t.Run("edit_profile_does_not_delete_locuri_used_by_another_profile", func(t *testing.T) {
|
||||
// If profile A has LocURIs X, Y and profile B has LocURI Y,
|
||||
// editing A to remove Y should NOT send a <Delete> for Y
|
||||
// because profile B still enforces it.
|
||||
|
||||
// Set up two profiles that share a LocURI (AllowCamera).
|
||||
profileA := `<Atomic><Replace><Item><Target><LocURI>./Device/Vendor/MSFT/Policy/Config/Camera/AllowCamera</LocURI></Target><Meta><Format xmlns="syncml:metinf">int</Format></Meta><Data>0</Data></Item></Replace><Replace><Item><Target><LocURI>./Device/Vendor/MSFT/Policy/Config/Experience/AllowCortana</LocURI></Target><Meta><Format xmlns="syncml:metinf">int</Format></Meta><Data>0</Data></Item></Replace></Atomic>`
|
||||
profileB := `<Replace><Item><Target><LocURI>./Device/Vendor/MSFT/Policy/Config/Camera/AllowCamera</LocURI></Target><Meta><Format xmlns="syncml:metinf">int</Format></Meta><Data>1</Data></Item></Replace>`
|
||||
s.Do("POST", "/api/v1/fleet/mdm/profiles/batch",
|
||||
batchSetMDMProfilesRequest{Profiles: []fleet.MDMProfileBatchPayload{
|
||||
{Name: "shared-locuri-A", Contents: []byte(profileA)},
|
||||
{Name: "shared-locuri-B", Contents: []byte(profileB)},
|
||||
}}, http.StatusNoContent, "team_id", fmt.Sprint(tm.ID))
|
||||
|
||||
// Drain install commands.
|
||||
s.awaitTriggerProfileSchedule(t)
|
||||
cmds, err := mdmDevice.StartManagementSession()
|
||||
require.NoError(t, err)
|
||||
msgID, err := mdmDevice.GetCurrentMsgID()
|
||||
require.NoError(t, err)
|
||||
for _, c := range cmds {
|
||||
cmdID := c.Cmd.CmdID
|
||||
status := syncml.CmdStatusOK
|
||||
mdmDevice.AppendResponse(fleet.SyncMLCmd{
|
||||
XMLName: xml.Name{Local: fleet.CmdStatus},
|
||||
MsgRef: &msgID,
|
||||
CmdRef: &cmdID.Value,
|
||||
Cmd: ptr.String(c.Verb),
|
||||
Data: &status,
|
||||
CmdID: fleet.CmdID{Value: uuid.NewString()},
|
||||
})
|
||||
}
|
||||
_, err = mdmDevice.SendResponse()
|
||||
require.NoError(t, err)
|
||||
verifyProfiles(mdmDevice, 0, false)
|
||||
|
||||
// Edit profile A to remove AllowCamera (shared with B), keep only AllowCortana.
|
||||
profileAEdited := `<Replace><Item><Target><LocURI>./Device/Vendor/MSFT/Policy/Config/Experience/AllowCortana</LocURI></Target><Meta><Format xmlns="syncml:metinf">int</Format></Meta><Data>0</Data></Item></Replace>`
|
||||
s.Do("POST", "/api/v1/fleet/mdm/profiles/batch",
|
||||
batchSetMDMProfilesRequest{Profiles: []fleet.MDMProfileBatchPayload{
|
||||
{Name: "shared-locuri-A", Contents: []byte(profileAEdited)},
|
||||
{Name: "shared-locuri-B", Contents: []byte(profileB)},
|
||||
}}, http.StatusNoContent, "team_id", fmt.Sprint(tm.ID))
|
||||
|
||||
// Trigger sync: should get an install for the updated A, but NO delete
|
||||
// for AllowCamera since profile B still uses it.
|
||||
s.awaitTriggerProfileSchedule(t)
|
||||
cmds, err = mdmDevice.StartManagementSession()
|
||||
require.NoError(t, err)
|
||||
|
||||
msgID, err = mdmDevice.GetCurrentMsgID()
|
||||
require.NoError(t, err)
|
||||
for _, c := range cmds {
|
||||
if c.Verb == "Delete" {
|
||||
require.NotContains(t, c.Cmd.GetTargetURI(), "AllowCamera",
|
||||
"should NOT delete AllowCamera because profile B still uses it")
|
||||
}
|
||||
cmdID := c.Cmd.CmdID
|
||||
status := syncml.CmdStatusOK
|
||||
mdmDevice.AppendResponse(fleet.SyncMLCmd{
|
||||
XMLName: xml.Name{Local: fleet.CmdStatus},
|
||||
MsgRef: &msgID,
|
||||
CmdRef: &cmdID.Value,
|
||||
Cmd: ptr.String(c.Verb),
|
||||
Data: &status,
|
||||
CmdID: fleet.CmdID{Value: uuid.NewString()},
|
||||
})
|
||||
}
|
||||
_, err = mdmDevice.SendResponse()
|
||||
require.NoError(t, err)
|
||||
})
|
||||
}
|
||||
|
||||
func (s *integrationMDMTestSuite) TestApplyTeamsMDMWindowsProfiles() {
|
||||
|
|
|
|||
|
|
@ -2754,14 +2754,8 @@ func ReconcileWindowsProfiles(ctx context.Context, ds fleet.Datastore, logger *s
|
|||
if _, isBeingRemoved := removeTargets[otherUUID]; isBeingRemoved {
|
||||
continue // also being removed, don't protect its URIs
|
||||
}
|
||||
otherCmds, err := fleet.UnmarshallMultiTopLevelXMLProfile(otherContent.SyncML)
|
||||
if err != nil {
|
||||
continue
|
||||
}
|
||||
for _, cmd := range otherCmds {
|
||||
if uri := cmd.GetTargetURI(); uri != "" {
|
||||
activeLocURIs[uri] = true
|
||||
}
|
||||
for _, uri := range fleet.ExtractLocURIsFromProfileBytes(otherContent.SyncML) {
|
||||
activeLocURIs[uri] = true
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue