allow to verify profiles that are pending (#15911)

for #15678

Author: Roberto Dip

changes/profile-verification

@@ -0,0 +1 @@
+* Improve profile verification logic to verify 'pending' profiles.

server/datastore/mysql/apple_mdm_test.go

@@ -3464,11 +3464,13 @@ func testSetVerifiedMacOSProfiles(t *testing.T, ds *Datastore) {
 	require.NoError(t, err)
 	cp3, err := ds.NewMDMAppleConfigProfile(ctx, *configProfileForTest(t, "name3", "cp3", "uuid3"))
 	require.NoError(t, err)
+	cp4, err := ds.NewMDMAppleConfigProfile(ctx, *configProfileForTest(t, "name4", "cp4", "uuid4"))
+	require.NoError(t, err)
 
 	// list config profiles for no team
 	cps, err := ds.ListMDMAppleConfigProfiles(ctx, nil)
 	require.NoError(t, err)
-	require.Len(t, cps, 3)
+	require.Len(t, cps, 4)
 	storedByIdentifier := make(map[string]*fleet.MDMAppleConfigProfile)
 	for _, cp := range cps {
 		storedByIdentifier[cp.Identifier] = cp
@@ -3484,6 +3486,7 @@ func testSetVerifiedMacOSProfiles(t *testing.T, ds *Datastore) {
 			cp1.Identifier: fleet.MDMDeliveryPending,
 			cp2.Identifier: fleet.MDMDeliveryVerifying,
 			cp3.Identifier: fleet.MDMDeliveryVerified,
+			cp4.Identifier: fleet.MDMDeliveryPending,
 		}
 	}
 
@@ -3497,12 +3500,12 @@ func testSetVerifiedMacOSProfiles(t *testing.T, ds *Datastore) {
 		for _, h := range hosts {
 			gotProfs, err := ds.GetHostMDMAppleProfiles(ctx, h.UUID)
 			require.NoError(t, err)
-			require.Len(t, gotProfs, 3)
+			require.Len(t, gotProfs, 4)
 			for _, p := range gotProfs {
 				s, ok := expectedHostMDMStatus[h.ID][p.Identifier]
 				require.True(t, ok)
 				require.NotNil(t, p.Status)
-				require.Equal(t, s, *p.Status)
+				require.Equalf(t, s, *p.Status, "profile identifier %s", p.Identifier)
 			}
 		}
 	}
@@ -3520,23 +3523,13 @@ func testSetVerifiedMacOSProfiles(t *testing.T, ds *Datastore) {
 	upsertHostCPs(hosts, []*fleet.MDMAppleConfigProfile{storedByIdentifier[cp1.Identifier]}, fleet.MDMOperationTypeInstall, &fleet.MDMDeliveryPending, ctx, ds, t)
 	upsertHostCPs(hosts, []*fleet.MDMAppleConfigProfile{storedByIdentifier[cp2.Identifier]}, fleet.MDMOperationTypeInstall, &fleet.MDMDeliveryVerifying, ctx, ds, t)
 	upsertHostCPs(hosts, []*fleet.MDMAppleConfigProfile{storedByIdentifier[cp3.Identifier]}, fleet.MDMOperationTypeInstall, &fleet.MDMDeliveryVerified, ctx, ds, t)
+	upsertHostCPs(hosts, []*fleet.MDMAppleConfigProfile{storedByIdentifier[cp4.Identifier]}, fleet.MDMOperationTypeInstall, &fleet.MDMDeliveryPending, ctx, ds, t)
 	checkHostMDMProfileStatuses()
 
 	// statuses don't change during the grace period if profiles are missing (i.e. not installed)
 	require.NoError(t, apple_mdm.VerifyHostMDMProfiles(ctx, ds, hosts[0], map[string]*fleet.HostMacOSProfile{}))
 	checkHostMDMProfileStatuses()
 
-	// only "verifying" status can change to "verified" so status of cp1 doesn't change (it
-	// remains "pending")
-	require.NoError(t, apple_mdm.VerifyHostMDMProfiles(ctx, ds, hosts[0], map[string]*fleet.HostMacOSProfile{
-		cp1.Identifier: {
-			Identifier:  cp1.Identifier,
-			DisplayName: cp1.Name,
-			InstallDate: time.Now(),
-		},
-	}))
-	checkHostMDMProfileStatuses()
-
 	// if install date is before the updated at timestamp of the profile, statuses don't change
 	// during the grace period
 	require.NoError(t, apple_mdm.VerifyHostMDMProfiles(ctx, ds, hosts[1], profilesByIdentifier([]*fleet.HostMacOSProfile{
@@ -3555,17 +3548,17 @@ func testSetVerifiedMacOSProfiles(t *testing.T, ds *Datastore) {
 			DisplayName: cp3.Name,
 			InstallDate: storedByIdentifier[cp3.Identifier].UpdatedAt.Add(-1 * time.Hour),
 		},
+		{
+			Identifier:  cp4.Identifier,
+			DisplayName: cp4.Name,
+			InstallDate: storedByIdentifier[cp4.Identifier].UpdatedAt.Add(-1 * time.Hour),
+		},
 	})))
 	checkHostMDMProfileStatuses()
 
-	// if install date is on or after the updated at timestamp of the profile, "verifying" status
-	// changes to "verified"
+	// if install date is on or after the updated at timestamp of the profile, "verifying" or "pending" status
+	// changes to "verified". Any "pending" profiles not reported are not changed
 	require.NoError(t, apple_mdm.VerifyHostMDMProfiles(ctx, ds, hosts[2], profilesByIdentifier([]*fleet.HostMacOSProfile{
-		{
-			Identifier:  cp1.Identifier,
-			DisplayName: cp1.Name,
-			InstallDate: storedByIdentifier[cp1.Identifier].UpdatedAt,
-		},
 		{
 			Identifier:  cp2.Identifier,
 			DisplayName: cp2.Name,
@@ -3576,17 +3569,18 @@ func testSetVerifiedMacOSProfiles(t *testing.T, ds *Datastore) {
 			DisplayName: cp3.Name,
 			InstallDate: storedByIdentifier[cp3.Identifier].UpdatedAt,
 		},
+		{
+			Identifier:  cp4.Identifier,
+			DisplayName: cp4.Name,
+			InstallDate: storedByIdentifier[cp4.Identifier].UpdatedAt,
+		},
 	})))
 	expectedHostMDMStatus[hosts[2].ID][cp2.Identifier] = fleet.MDMDeliveryVerified
+	expectedHostMDMStatus[hosts[2].ID][cp4.Identifier] = fleet.MDMDeliveryVerified
 	checkHostMDMProfileStatuses()
 
 	// repeated call doesn't change statuses
 	require.NoError(t, apple_mdm.VerifyHostMDMProfiles(ctx, ds, hosts[2], profilesByIdentifier([]*fleet.HostMacOSProfile{
-		{
-			Identifier:  cp1.Identifier,
-			DisplayName: cp1.Name,
-			InstallDate: storedByIdentifier[cp1.Identifier].UpdatedAt,
-		},
 		{
 			Identifier:  cp2.Identifier,
 			DisplayName: cp2.Name,
@@ -3597,15 +3591,20 @@ func testSetVerifiedMacOSProfiles(t *testing.T, ds *Datastore) {
 			DisplayName: cp3.Name,
 			InstallDate: storedByIdentifier[cp3.Identifier].UpdatedAt,
 		},
+		{
+			Identifier:  cp4.Identifier,
+			DisplayName: cp4.Name,
+			InstallDate: storedByIdentifier[cp4.Identifier].UpdatedAt,
+		},
 	})))
 	checkHostMDMProfileStatuses()
 
 	// simulate expired grace period by setting updated_at timestamp of profiles back by 24 hours
 	ExecAdhocSQL(t, ds, func(tx sqlx.ExtContext) error {
 		_, err := tx.ExecContext(ctx,
-			`UPDATE mdm_apple_configuration_profiles SET updated_at = ? WHERE profile_uuid IN(?, ?, ?)`,
+			`UPDATE mdm_apple_configuration_profiles SET updated_at = ? WHERE profile_uuid IN(?, ?, ?, ?)`,
 			time.Now().Add(-24*time.Hour),
-			cp1.ProfileUUID, cp2.ProfileUUID, cp3.ProfileUUID,
+			cp1.ProfileUUID, cp2.ProfileUUID, cp3.ProfileUUID, cp4.ProfileUUID,
 		)
 		return err
 	})
@@ -3623,11 +3622,14 @@ func testSetVerifiedMacOSProfiles(t *testing.T, ds *Datastore) {
 			InstallDate: time.Now(),
 		},
 	})))
-	expectedHostMDMStatus[hosts[2].ID][cp3.Identifier] = fleet.MDMDeliveryPending // first retry for cp3
+	expectedHostMDMStatus[hosts[2].ID][cp1.Identifier] = fleet.MDMDeliveryVerified //cp1 can go from pending to verified
+	expectedHostMDMStatus[hosts[2].ID][cp3.Identifier] = fleet.MDMDeliveryPending  // first retry for cp3
+	expectedHostMDMStatus[hosts[2].ID][cp4.Identifier] = fleet.MDMDeliveryPending  // first retry for cp4
 	checkHostMDMProfileStatuses()
 	// simulate retry command acknowledged by setting status to "verifying"
 	adHocSetVerifying(hosts[2].UUID, cp3.Identifier)
-	// report osquery results again with cp3 still missing
+	adHocSetVerifying(hosts[2].UUID, cp4.Identifier)
+	// report osquery results again with cp3 and cp4 still missing
 	require.NoError(t, apple_mdm.VerifyHostMDMProfiles(ctx, ds, hosts[2], profilesByIdentifier([]*fleet.HostMacOSProfile{
 		{
 			Identifier:  cp1.Identifier,
@@ -3641,6 +3643,7 @@ func testSetVerifiedMacOSProfiles(t *testing.T, ds *Datastore) {
 		},
 	})))
 	expectedHostMDMStatus[hosts[2].ID][cp3.Identifier] = fleet.MDMDeliveryFailed // still missing after retry so expect cp3 to fail
+	expectedHostMDMStatus[hosts[2].ID][cp4.Identifier] = fleet.MDMDeliveryFailed // still missing after retry so expect cp4 to fail
 	checkHostMDMProfileStatuses()
 
 	// after the grace period and one retry attempt, status changes to "failed" if a profile is outdated (i.e. installed
@@ -4471,7 +4474,7 @@ func TestMDMAppleProfileVerification(t *testing.T) {
 			{
 				name:           "PendingThenFoundExpected",
 				initialStatus:  fleet.MDMDeliveryPending,
-				expectedStatus: fleet.MDMDeliveryPending, // no change
+				expectedStatus: fleet.MDMDeliveryVerified, // pending can go to verified if found
 				expectedDetail: "",
 			},
 			{
@@ -4534,7 +4537,7 @@ func TestMDMAppleProfileVerification(t *testing.T) {
 			{
 				name:           "PendingThenFoundExpectedAndUnexpected",
 				initialStatus:  fleet.MDMDeliveryPending,
-				expectedStatus: fleet.MDMDeliveryPending, // no change
+				expectedStatus: fleet.MDMDeliveryVerified, // profile can go from pending to verified
 				expectedDetail: "",
 			},
 			{

server/datastore/mysql/mdm.go

@@ -435,7 +435,10 @@ WHERE
 		fleet.HostMDMProfileDetailFailedWasVerified,
 		fleet.MDMDeliveryFailed,
 		host.UUID,
-		[]interface{}{fleet.MDMDeliveryVerifying, fleet.MDMDeliveryVerified},
+		[]interface{}{
+			fleet.MDMDeliveryVerifying,
+			fleet.MDMDeliveryVerified,
+		},
 		fleet.MDMOperationTypeInstall,
 		identifiersOrNames,
 	}
@@ -482,7 +485,11 @@ WHERE
 	args := []interface{}{
 		fleet.MDMDeliveryVerified,
 		host.UUID,
-		[]interface{}{fleet.MDMDeliveryVerifying, fleet.MDMDeliveryFailed},
+		[]interface{}{
+			fleet.MDMDeliveryPending,
+			fleet.MDMDeliveryVerifying,
+			fleet.MDMDeliveryFailed,
+		},
 		fleet.MDMOperationTypeInstall,
 		identifiersOrNames,
 	}

server/mdm/apple/profile_verifier_test.go

@@ -0,0 +1,125 @@
+package apple_mdm
+
+import (
+	"context"
+	"errors"
+	"testing"
+	"time"
+
+	"github.com/fleetdm/fleet/v4/server/fleet"
+	"github.com/fleetdm/fleet/v4/server/mock"
+	"github.com/stretchr/testify/require"
+)
+
+func TestVerifyHostMDMProfiles(t *testing.T) {
+	ctx := context.Background()
+	host := &fleet.Host{
+		DetailUpdatedAt: time.Now(),
+	}
+	ds := new(mock.Store)
+
+	tests := []struct {
+		name             string
+		expectedProfiles map[string]*fleet.ExpectedMDMProfile
+		retryCounts      []fleet.HostMDMProfileRetryCount
+		installed        map[string]*fleet.HostMacOSProfile
+		toVerify         []string
+		toFail           []string
+		toRetry          []string
+		expectErr        bool
+	}{
+		{
+			name:      "error on getting expected profiles",
+			expectErr: true,
+		},
+		{
+			name:             "all profiles verified",
+			expectedProfiles: map[string]*fleet.ExpectedMDMProfile{"profile1": {}},
+			installed:        map[string]*fleet.HostMacOSProfile{"profile1": {}},
+			toVerify:         []string{"profile1"},
+		},
+		{
+			name: "profiles missing, not within grace period, no retries yet",
+			expectedProfiles: map[string]*fleet.ExpectedMDMProfile{
+				"profile1": {},
+				"profile2": {EarliestInstallDate: host.DetailUpdatedAt.Add(-24 * time.Hour)},
+			},
+			installed: map[string]*fleet.HostMacOSProfile{"profile1": {}},
+			toVerify:  []string{"profile1"},
+			toRetry:   []string{"profile2"},
+		},
+		{
+			name:             "profiles missing, with and without retries, not within grace period",
+			expectedProfiles: map[string]*fleet.ExpectedMDMProfile{"profile1": {}, "profile2": {}},
+			retryCounts: []fleet.HostMDMProfileRetryCount{
+				{ProfileIdentifier: "profile1", Retries: 0},
+				{ProfileIdentifier: "profile2", Retries: 1},
+			},
+			installed: map[string]*fleet.HostMacOSProfile{},
+			toRetry:   []string{"profile1"},
+			toFail:    []string{"profile2"},
+		},
+		{
+			name: "host profile installed prior to uploading profile to Fleet",
+			expectedProfiles: map[string]*fleet.ExpectedMDMProfile{
+				"profile1": {EarliestInstallDate: time.Now().Add(-2 * time.Hour)},
+			},
+			installed: map[string]*fleet.HostMacOSProfile{
+				"profile1": {InstallDate: time.Now().Add(-24 * time.Hour)},
+			},
+			toRetry: []string{"profile1"},
+		},
+		{
+			name: "host profile installed prior to uploading profile to Fleet with max retries",
+			expectedProfiles: map[string]*fleet.ExpectedMDMProfile{
+				"profile1": {EarliestInstallDate: time.Now().Add(-2 * time.Hour)},
+			},
+			installed: map[string]*fleet.HostMacOSProfile{
+				"profile1": {InstallDate: time.Now().Add(-24 * time.Hour)},
+			},
+			retryCounts: []fleet.HostMDMProfileRetryCount{
+				{ProfileIdentifier: "profile1", Retries: 1},
+			},
+			toFail: []string{"profile1"},
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			// setup mocks
+			ds.GetHostMDMProfilesExpectedForVerificationFunc = func(ctx context.Context, host *fleet.Host) (map[string]*fleet.ExpectedMDMProfile, error) {
+				if tc.expectErr {
+					return nil, errors.New("error")
+				}
+				return tc.expectedProfiles, nil
+			}
+
+			ds.GetHostMDMProfilesRetryCountsFunc = func(ctx context.Context, host *fleet.Host) ([]fleet.HostMDMProfileRetryCount, error) {
+				return tc.retryCounts, nil
+			}
+
+			ds.UpdateHostMDMProfilesVerificationFunc = func(ctx context.Context, host *fleet.Host, verified, toFail, toRetry []string) error {
+				require.ElementsMatch(t, tc.toVerify, verified, "verified profiles do not match")
+				require.ElementsMatch(t, tc.toFail, toFail, "failed profiles do not match")
+				require.ElementsMatch(t, tc.toRetry, toRetry, "retried profiles do not match")
+				return nil
+			}
+
+			// run the test
+			err := VerifyHostMDMProfiles(ctx, ds, host, tc.installed)
+			if tc.expectErr {
+				require.Error(t, err)
+				require.False(
+					t,
+					ds.UpdateHostMDMProfilesVerificationFuncInvoked,
+					"UpdateHostMDMProfilesVerificationFunc should not have been called",
+				)
+			} else {
+				require.NoError(t, err)
+				require.True(t, ds.UpdateHostMDMProfilesVerificationFuncInvoked, "UpdateHostMDMProfilesVerificationFunc should have been called")
+			}
+
+			ds.UpdateHostMDMProfilesVerificationFuncInvoked = false
+		})
+	}
+}