From 5f313c8972f20f2896e31d27120b879a089e33e5 Mon Sep 17 00:00:00 2001
From: Roberto Dip <me@roperzh.com>
Date: Wed, 22 Nov 2023 10:48:28 -0300
Subject: [PATCH] fix various bugs after testing windows MDM profiles (#15264)

for #13281
---
 .../DeleteProfileModal/DeleteProfileModal.tsx |  2 +-
 .../OSSettingsTable/OSSettingsTableConfig.tsx | 33 +++++++++------
 pkg/spec/spec.go                              | 11 +++--
 pkg/spec/spec_test.go                         | 42 +++++++++++++++++++
 server/datastore/mysql/microsoft_mdm.go       | 10 ++---
 server/fleet/windows_mdm.go                   |  3 +-
 server/fleet/windows_mdm_test.go              |  7 ++++
 server/mdm/microsoft/microsoft_mdm.go         |  4 +-
 server/service/mdm.go                         |  6 ++-
 9 files changed, 89 insertions(+), 29 deletions(-)

diff --git a/frontend/pages/ManageControlsPage/OSSettings/cards/CustomSettings/components/DeleteProfileModal/DeleteProfileModal.tsx b/frontend/pages/ManageControlsPage/OSSettings/cards/CustomSettings/components/DeleteProfileModal/DeleteProfileModal.tsx
index 1d25dec162..09f584bb27 100644
--- a/frontend/pages/ManageControlsPage/OSSettings/cards/CustomSettings/components/DeleteProfileModal/DeleteProfileModal.tsx
+++ b/frontend/pages/ManageControlsPage/OSSettings/cards/CustomSettings/components/DeleteProfileModal/DeleteProfileModal.tsx
@@ -43,7 +43,7 @@ const DeleteProfileModal = ({
         <p>
           This action will delete configuration profile{" "}
           <span className={`${baseClass}__profile-name`}>{profileName}</span>{" "}
-          from all macOS hosts{messageSuffix}.
+          from all hosts{messageSuffix}.
         </p>
         <div className="modal-cta-wrap">
           <Button
diff --git a/frontend/pages/hosts/details/OSSettingsModal/OSSettingsTable/OSSettingsTableConfig.tsx b/frontend/pages/hosts/details/OSSettingsModal/OSSettingsTable/OSSettingsTableConfig.tsx
index 634c03b49e..dfeeb5a523 100644
--- a/frontend/pages/hosts/details/OSSettingsModal/OSSettingsTable/OSSettingsTableConfig.tsx
+++ b/frontend/pages/hosts/details/OSSettingsModal/OSSettingsTable/OSSettingsTableConfig.tsx
@@ -98,21 +98,28 @@ const tableHeaders: IDataColumn[] = [
   },
 ];
 
-const makeWindowsRows = ({ os_settings }: IHostMdmData) => {
-  if (
-    !os_settings?.disk_encryption?.status ||
-    !isWindowsDiskEncryptionStatus(os_settings.disk_encryption.status)
-  ) {
-    return null;
+const makeWindowsRows = ({ profiles, os_settings }: IHostMdmData) => {
+  const rows: ITableRowOsSettings[] = [];
+
+  if (profiles) {
+    rows.push(...profiles);
   }
 
-  const rows: ITableRowOsSettings[] = [];
-  rows.push(
-    generateWinDiskEncryptionProfile(
-      os_settings.disk_encryption.status,
-      os_settings.disk_encryption.detail
-    )
-  );
+  if (
+    os_settings?.disk_encryption?.status &&
+    isWindowsDiskEncryptionStatus(os_settings.disk_encryption.status)
+  ) {
+    rows.push(
+      generateWinDiskEncryptionProfile(
+        os_settings.disk_encryption.status,
+        os_settings.disk_encryption.detail
+      )
+    );
+  }
+
+  if (rows.length === 0 && !profiles) {
+    return null;
+  }
 
   return rows;
 };
diff --git a/pkg/spec/spec.go b/pkg/spec/spec.go
index 0d9808d73e..34a7d10d62 100644
--- a/pkg/spec/spec.go
+++ b/pkg/spec/spec.go
@@ -45,12 +45,15 @@ func GroupFromBytes(b []byte) (*Group, error) {
 			return nil, fmt.Errorf("failed to unmarshal spec item %w: \n%s", err, specItem)
 		}
 
-		if s.Spec == nil {
-			return nil, fmt.Errorf("no spec field on %q document", s.Kind)
-		}
-
 		kind := strings.ToLower(s.Kind)
 
+		if s.Spec == nil {
+			if kind == "" {
+				return nil, errors.New(`Missing required fields ("spec", "kind") on provided configuration.`)
+			}
+			return nil, fmt.Errorf(`Missing required fields ("spec") on provided %q configuration.`, s.Kind)
+		}
+
 		switch kind {
 		case fleet.QueryKind:
 			var querySpec *fleet.QuerySpec
diff --git a/pkg/spec/spec_test.go b/pkg/spec/spec_test.go
index 59a8eb06fe..46d0cd6e02 100644
--- a/pkg/spec/spec_test.go
+++ b/pkg/spec/spec_test.go
@@ -69,3 +69,45 @@ func TestGroupFromBytesWithWin10CISQueries(t *testing.T) {
 	require.NoError(t, err)
 	require.NotEmpty(t, g.Policies)
 }
+
+func TestGroupFromBytesMissingFields(t *testing.T) {
+	tests := []struct {
+		name string
+		in   []byte
+		want string
+	}{
+		{
+			"missing spec",
+			[]byte(`
+---
+apiVersion: v1
+kind: team
+			`),
+			`Missing required fields ("spec") on provided "team" configuration.`,
+		},
+		{
+			"missing spec and kind",
+			[]byte(`
+---
+apiVersion: v1
+			`),
+			`Missing required fields ("spec", "kind") on provided configuration`,
+		},
+		{
+			"missing spec and empty string kind",
+			[]byte(`
+---
+apiVersion: v1
+kind: ""
+			`),
+			`Missing required fields ("spec", "kind") on provided configuration`,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			_, err := GroupFromBytes(tt.in)
+			require.ErrorContains(t, err, tt.want)
+		})
+	}
+}
diff --git a/server/datastore/mysql/microsoft_mdm.go b/server/datastore/mysql/microsoft_mdm.go
index b027bbb99f..ea93dd7bab 100644
--- a/server/datastore/mysql/microsoft_mdm.go
+++ b/server/datastore/mysql/microsoft_mdm.go
@@ -371,7 +371,7 @@ func updateMDMWindowsHostProfileStatusFromResponseDB(
 	// update their detail and status.
 	const updateHostProfilesStmt = `
 		INSERT INTO host_mdm_windows_profiles
-			(host_uuid, profile_uuid, detail, status)
+			(host_uuid, profile_uuid, detail, status, command_uuid)
 		VALUES %s
 		ON DUPLICATE KEY UPDATE
 			detail = VALUES(detail),
@@ -402,11 +402,11 @@ func updateMDMWindowsHostProfileStatusFromResponseDB(
 	// find the matching entries for the given host_uuid, command_uuid combinations.
 	stmt, args, err := sqlx.In(getMatchingHostProfilesStmt, hostUUID, commandUUIDs)
 	if err != nil {
-		return err
+		return ctxerr.Wrap(ctx, err, "building sqlx.In query")
 	}
 	var matchingHostProfiles []fleet.MDMWindowsProfilePayload
 	if err := sqlx.SelectContext(ctx, tx, &matchingHostProfiles, stmt, args...); err != nil {
-		return err
+		return ctxerr.Wrap(ctx, err, "running query to get matching profiles")
 	}
 
 	// batch-update the matching entries with the desired detail and status>
@@ -415,12 +415,12 @@ func updateMDMWindowsHostProfileStatusFromResponseDB(
 	for _, hp := range matchingHostProfiles {
 		payload := uuidsToPayloads[hp.CommandUUID]
 		args = append(args, hp.HostUUID, hp.ProfileUUID, payload.Detail, payload.Status)
-		sb.WriteString("(?, ?, ?, ?),")
+		sb.WriteString("(?, ?, ?, ?, command_uuid),")
 	}
 
 	stmt = fmt.Sprintf(updateHostProfilesStmt, strings.TrimSuffix(sb.String(), ","))
 	_, err = tx.ExecContext(ctx, stmt, args...)
-	return err
+	return ctxerr.Wrap(ctx, err, "updating host profiles")
 }
 
 func (ds *Datastore) GetMDMWindowsCommandResults(ctx context.Context, commandUUID string) ([]*fleet.MDMCommandResult, error) {
diff --git a/server/fleet/windows_mdm.go b/server/fleet/windows_mdm.go
index 6bbf448a14..e471d44e22 100644
--- a/server/fleet/windows_mdm.go
+++ b/server/fleet/windows_mdm.go
@@ -68,8 +68,7 @@ func (m *MDMWindowsConfigProfile) ValidateUserProvided() error {
 			return errors.New("Only <Replace> supported as a top level element. Make sure you don't have other top level elements.")
 		}
 
-		for _, target := range element.FindElements("Target") {
-			locURI := target.FindElement("LocURI")
+		for _, locURI := range element.FindElements("//Target/LocURI") {
 			if locURI != nil {
 				if err := validateFleetProvidedLocURI(locURI.Text()); err != nil {
 					return err
diff --git a/server/fleet/windows_mdm_test.go b/server/fleet/windows_mdm_test.go
index a74aeb86af..0eecdabcc9 100644
--- a/server/fleet/windows_mdm_test.go
+++ b/server/fleet/windows_mdm_test.go
@@ -40,6 +40,13 @@ func TestValidateUserProvided(t *testing.T) {
 			},
 			wantErr: true,
 		},
+		{
+			name: "Reserved LocURI with implicit ./Device prefix",
+			profile: MDMWindowsConfigProfile{
+				SyncML: []byte(`<Replace><Target><LocURI>./Vendor/MSFT/BitLocker/Foo</LocURI></Target></Replace>`),
+			},
+			wantErr: true,
+		},
 		{
 			name: "XML with Multiple Replace Elements",
 			profile: MDMWindowsConfigProfile{
diff --git a/server/mdm/microsoft/microsoft_mdm.go b/server/mdm/microsoft/microsoft_mdm.go
index 0235c4d852..6bd63a4f29 100644
--- a/server/mdm/microsoft/microsoft_mdm.go
+++ b/server/mdm/microsoft/microsoft_mdm.go
@@ -396,8 +396,8 @@ const (
 )
 
 const (
-	FleetBitLockerTargetLocURI = "./Device/Vendor/MSFT/BitLocker"
-	FleetOSUpdateTargetLocURI  = "./Device/Vendor/MSFT/Policy/Config/Update"
+	FleetBitLockerTargetLocURI = "/Vendor/MSFT/BitLocker"
+	FleetOSUpdateTargetLocURI  = "/Vendor/MSFT/Policy/Config/Update"
 )
 
 func ResolveWindowsMDMDiscovery(serverURL string) (string, error) {
diff --git a/server/service/mdm.go b/server/service/mdm.go
index 39dd2f56ef..7585d1dda9 100644
--- a/server/service/mdm.go
+++ b/server/service/mdm.go
@@ -1120,8 +1120,10 @@ func (svc *Service) DeleteMDMWindowsConfigProfile(ctx context.Context, profileUU
 		return ctxerr.Wrap(ctx, err)
 	}
 
-	// TODO: integrate the call to bulk-update host profiles affected by this deletion (see Apple's implementation)
-	// (part of https://github.com/fleetdm/fleet/issues/14364)
+	// cannot use the profile ID as it is now deleted
+	if err := svc.ds.BulkSetPendingMDMHostProfiles(ctx, nil, []uint{teamID}, nil, nil, nil); err != nil {
+		return ctxerr.Wrap(ctx, err, "bulk set pending host profiles")
+	}
 
 	var (
 		actTeamID   *uint