diff --git a/server/datastore/mysql/microsoft_mdm.go b/server/datastore/mysql/microsoft_mdm.go index fb128f612c..8174b0aae9 100644 --- a/server/datastore/mysql/microsoft_mdm.go +++ b/server/datastore/mysql/microsoft_mdm.go @@ -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 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 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, diff --git a/server/fleet/microsoft_mdm.go b/server/fleet/microsoft_mdm.go index 4f400e401b..6bf002266a 100644 --- a/server/fleet/microsoft_mdm.go +++ b/server/fleet/microsoft_mdm.go @@ -1754,90 +1754,49 @@ func BuildDeleteCommandFromProfileBytes(profileBytes []byte, commandUUID string, normalized = fmt.Appendf([]byte{}, "%s", 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 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 commands. - // Never wrap in — removal is best-effort, and 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 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), "") { + normalized = fmt.Appendf([]byte{}, "%s", 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 +} diff --git a/server/fleet/microsoft_mdm_test.go b/server/fleet/microsoft_mdm_test.go index e5c2b7cdf8..f84fb563e9 100644 --- a/server/fleet/microsoft_mdm_test.go +++ b/server/fleet/microsoft_mdm_test.go @@ -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 := `./Device/A./Device/B` + uris := ExtractLocURIsFromProfileBytes([]byte(xml)) + require.Equal(t, []string{"./Device/A", "./Device/B"}, uris) + }) + + t.Run("non-atomic profile", func(t *testing.T) { + xml := `./Device/X./Device/Y` + uris := ExtractLocURIsFromProfileBytes([]byte(xml)) + require.Equal(t, []string{"./Device/X", "./Device/Y"}, uris) + }) + + t.Run("exec commands excluded", func(t *testing.T) { + xml := `./Device/A./Device/Enroll` + 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 := `./Device/A./Device/B` + uris := ExtractLocURIsFromProfileBytes([]byte(xml)) + require.Equal(t, []string{"./Device/A"}, uris) + }) +} diff --git a/server/service/integration_mdm_profiles_test.go b/server/service/integration_mdm_profiles_test.go index 93aeb97028..70e7d498b6 100644 --- a/server/service/integration_mdm_profiles_test.go +++ b/server/service/integration_mdm_profiles_test.go @@ -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 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 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 := `./Device/Vendor/MSFT/Policy/Config/Camera/AllowCameraint0./Device/Vendor/MSFT/Policy/Config/Experience/AllowCortanaint0` + // 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 := `./Device/Vendor/MSFT/Policy/Config/Camera/AllowCameraint0` + 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 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 for Y + // because profile B still enforces it. + + // Set up two profiles that share a LocURI (AllowCamera). + profileA := `./Device/Vendor/MSFT/Policy/Config/Camera/AllowCameraint0./Device/Vendor/MSFT/Policy/Config/Experience/AllowCortanaint0` + profileB := `./Device/Vendor/MSFT/Policy/Config/Camera/AllowCameraint1` + 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 := `./Device/Vendor/MSFT/Policy/Config/Experience/AllowCortanaint0` + 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() { diff --git a/server/service/microsoft_mdm.go b/server/service/microsoft_mdm.go index 72dc03925a..ac855120a7 100644 --- a/server/service/microsoft_mdm.go +++ b/server/service/microsoft_mdm.go @@ -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 } }