diff --git a/changes/16853-show-all-mdm-errs b/changes/16853-show-all-mdm-errs new file mode 100644 index 000000000..772fb1f62 --- /dev/null +++ b/changes/16853-show-all-mdm-errs @@ -0,0 +1 @@ +- Fixes an issue where some MDM profile installation errors would not be shown in Fleet. \ No newline at end of file diff --git a/server/datastore/mysql/apple_mdm.go b/server/datastore/mysql/apple_mdm.go index c06c6e7c4..dc57ff9f6 100644 --- a/server/datastore/mysql/apple_mdm.go +++ b/server/datastore/mysql/apple_mdm.go @@ -1938,7 +1938,7 @@ func (ds *Datastore) BulkUpsertMDMAppleHostProfiles(ctx context.Context, payload func (ds *Datastore) UpdateOrDeleteHostMDMAppleProfile(ctx context.Context, profile *fleet.HostMDMAppleProfile) error { if profile.OperationType == fleet.MDMOperationTypeRemove && profile.Status != nil && - (*profile.Status == fleet.MDMDeliveryVerifying || *profile.Status == fleet.MDMDeliveryVerified || profile.IgnoreMDMClientError()) { + (*profile.Status == fleet.MDMDeliveryVerifying || *profile.Status == fleet.MDMDeliveryVerified) { _, err := ds.writer(ctx).ExecContext(ctx, ` DELETE FROM host_mdm_apple_profiles WHERE host_uuid = ? AND command_uuid = ? @@ -1946,11 +1946,17 @@ func (ds *Datastore) UpdateOrDeleteHostMDMAppleProfile(ctx context.Context, prof return err } + detail := profile.Detail + + if profile.OperationType == fleet.MDMOperationTypeRemove && profile.Status != nil && *profile.Status == fleet.MDMDeliveryFailed { + detail = fmt.Sprintf("Failed to remove: %s", detail) + } + _, err := ds.writer(ctx).ExecContext(ctx, ` UPDATE host_mdm_apple_profiles SET status = ?, operation_type = ?, detail = ? WHERE host_uuid = ? AND command_uuid = ? - `, profile.Status, profile.OperationType, profile.Detail, profile.HostUUID, profile.CommandUUID) + `, profile.Status, profile.OperationType, detail, profile.HostUUID, profile.CommandUUID) return err } diff --git a/server/datastore/mysql/apple_mdm_test.go b/server/datastore/mysql/apple_mdm_test.go index 6b8f73c3d..2b979cfa6 100644 --- a/server/datastore/mysql/apple_mdm_test.go +++ b/server/datastore/mysql/apple_mdm_test.go @@ -53,7 +53,7 @@ func TestMDMApple(t *testing.T) { {"TestAggregateMacOSSettingsStatusWithFileVault", testAggregateMacOSSettingsStatusWithFileVault}, {"TestMDMAppleHostsProfilesStatus", testMDMAppleHostsProfilesStatus}, {"TestMDMAppleIdPAccount", testMDMAppleIdPAccount}, - {"TestIgnoreMDMClientError", testIgnoreMDMClientError}, + {"TestIgnoreMDMClientError", testDoNotIgnoreMDMClientError}, {"TestDeleteMDMAppleProfilesForHost", testDeleteMDMAppleProfilesForHost}, {"TestGetMDMAppleCommandResults", testGetMDMAppleCommandResults}, {"TestBulkUpsertMDMAppleConfigProfiles", testBulkUpsertMDMAppleConfigProfile}, @@ -2294,7 +2294,7 @@ func testMDMAppleIdPAccount(t *testing.T, ds *Datastore) { require.Nil(t, out) } -func testIgnoreMDMClientError(t *testing.T, ds *Datastore) { +func testDoNotIgnoreMDMClientError(t *testing.T, ds *Datastore) { ctx := context.Background() // create new record for remove pending @@ -2326,7 +2326,12 @@ func testIgnoreMDMClientError(t *testing.T, ds *Datastore) { })) cps, err = ds.GetHostMDMAppleProfiles(ctx, "h1") require.NoError(t, err) - require.Len(t, cps, 0) // we ignore error code 89 and delete the pending record as well + require.Len(t, cps, 1) // we no longer ignore error code 89 + require.Equal(t, "name1", cps[0].Name) + require.Equal(t, fleet.MDMOperationTypeRemove, cps[0].OperationType) + require.NotNil(t, cps[0].Status) + require.Equal(t, fleet.MDMDeliveryFailed, *cps[0].Status) + require.Equal(t, "Failed to remove: MDMClientError (89): Profile with identifier 'p1' not found.", cps[0].Detail) // create another new record require.NoError(t, ds.BulkUpsertMDMAppleHostProfiles(ctx, []*fleet.MDMAppleBulkUpsertHostProfilePayload{{ @@ -2362,7 +2367,7 @@ func testIgnoreMDMClientError(t *testing.T, ds *Datastore) { require.Equal(t, fleet.MDMOperationTypeRemove, cps[0].OperationType) require.NotNil(t, cps[0].Status) require.Equal(t, fleet.MDMDeliveryFailed, *cps[0].Status) - require.Equal(t, "MDMClientError (96): Cannot replace profile 'p2' because it was not installed by the MDM server.", cps[0].Detail) + require.Equal(t, "Failed to remove: MDMClientError (96): Cannot replace profile 'p2' because it was not installed by the MDM server.", cps[0].Detail) } func testDeleteMDMAppleProfilesForHost(t *testing.T, ds *Datastore) { @@ -3751,7 +3756,7 @@ func testSetVerifiedMacOSProfiles(t *testing.T, ds *Datastore) { InstallDate: time.Now(), }, }))) - expectedHostMDMStatus[hosts[2].ID][cp1.Identifier] = fleet.MDMDeliveryVerified //cp1 can go from pending to verified + 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() diff --git a/server/fleet/apple_mdm.go b/server/fleet/apple_mdm.go index f1c01e89b..9dd95fd62 100644 --- a/server/fleet/apple_mdm.go +++ b/server/fleet/apple_mdm.go @@ -277,17 +277,6 @@ func (p HostMDMAppleProfile) ToHostMDMProfile() HostMDMProfile { } } -func (p HostMDMAppleProfile) IgnoreMDMClientError() bool { - switch p.OperationType { - case MDMOperationTypeRemove: - switch { - case strings.Contains(p.Detail, "MDMClientError (89)"): - return true - } - } - return false -} - type HostMDMProfileDetail string const ( diff --git a/server/fleet/apple_mdm_test.go b/server/fleet/apple_mdm_test.go index 44f8ce1b9..5230ee452 100644 --- a/server/fleet/apple_mdm_test.go +++ b/server/fleet/apple_mdm_test.go @@ -318,24 +318,6 @@ func mcPayloadContentForTest(refs []string) string { return formatted } -func TestHostMDMAppleProfileIgnoreClientError(t *testing.T) { - require.True(t, HostMDMAppleProfile{ - CommandUUID: "c1", - HostUUID: "h1", - Status: &MDMDeliveryFailed, - Detail: "MDMClientError (89): Profile with identifier 'p1' not found.", - OperationType: MDMOperationTypeRemove, - }.IgnoreMDMClientError()) - - require.False(t, HostMDMAppleProfile{ - CommandUUID: "c1", - HostUUID: "h1", - Status: &MDMDeliveryFailed, - Detail: "MDMClientError (96): Cannot replace profile 'p2' because it was not installed by the MDM server.", - OperationType: MDMOperationTypeRemove, - }.IgnoreMDMClientError()) -} - func TestHostDEPAssignment(t *testing.T) { cases := []struct { testName string diff --git a/server/service/integration_mdm_test.go b/server/service/integration_mdm_test.go index cc9eb17a8..d2129133d 100644 --- a/server/service/integration_mdm_test.go +++ b/server/service/integration_mdm_test.go @@ -11333,6 +11333,88 @@ func (s *integrationMDMTestSuite) TestGetManualEnrollmentProfile() { s.downloadAndVerifyEnrollmentProfile("/api/latest/fleet/mdm/manual_enrollment_profile") } +func (s *integrationMDMTestSuite) TestDontIgnoreAnyProfileErrors() { + t := s.T() + ctx := context.Background() + + // Create a host and a couple of profiles + host, mdmDevice := createHostThenEnrollMDM(s.ds, s.server.URL, t) + + globalProfiles := [][]byte{ + mobileconfigForTest("N1", "I1"), + mobileconfigForTest("N2", "I2"), + } + + s.Do("POST", "/api/v1/fleet/mdm/apple/profiles/batch", batchSetMDMAppleProfilesRequest{Profiles: globalProfiles}, http.StatusNoContent) + s.awaitTriggerProfileSchedule(t) + + // The profiles should be associated with the host we made + the standard fleet config + profs, err := s.ds.GetHostMDMAppleProfiles(ctx, host.UUID) + require.NoError(t, err) + require.Len(t, profs, 3) + + // Acknowledge the profiles so we can mark them as verified + cmd, err := mdmDevice.Idle() + require.NoError(t, err) + for cmd != nil { + cmd, err = mdmDevice.Acknowledge(cmd.CommandUUID) + require.NoError(t, err) + } + + require.NoError(t, apple_mdm.VerifyHostMDMProfiles(context.Background(), s.ds, host, map[string]*fleet.HostMacOSProfile{ + "I1": {Identifier: "I1", DisplayName: "I1", InstallDate: time.Now()}, + "I2": {Identifier: "I2", DisplayName: "I2", InstallDate: time.Now()}, + mobileconfig.FleetdConfigPayloadIdentifier: {Identifier: mobileconfig.FleetdConfigPayloadIdentifier, DisplayName: "I2", InstallDate: time.Now()}, + })) + + // Check that the profile is marked as verified when fetching the host + getHostResp := getHostResponse{} + s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/hosts/%d", host.ID), nil, http.StatusOK, &getHostResp) + require.NotNil(t, getHostResp.Host.MDM.Profiles) + for _, hm := range *getHostResp.Host.MDM.Profiles { + require.Equal(t, fleet.MDMDeliveryVerified, *hm.Status) + } + + // remove the profiles + s.Do("POST", "/api/v1/fleet/mdm/apple/profiles/batch", batchSetMDMAppleProfilesRequest{}, http.StatusNoContent) + s.awaitTriggerProfileSchedule(t) + + // On the host side, return errors for the two profile removal actions + cmd, err = mdmDevice.Idle() + require.NoError(t, err) + for cmd != nil { + if cmd.Command.RequestType == "RemoveProfile" { + var errChain []mdm.ErrorChain + if cmd.Command.RemoveProfile.Identifier == "I1" { + errChain = append(errChain, mdm.ErrorChain{ErrorCode: 89, ErrorDomain: "MDMClientError", USEnglishDescription: "Profile with identifier 'I1' not found."}) + } else { + errChain = append(errChain, mdm.ErrorChain{ErrorCode: 96, ErrorDomain: "MDMClientError", USEnglishDescription: "Cannot replace profile 'I2' because it was not installed by the MDM server."}) + } + cmd, err = mdmDevice.Err(cmd.CommandUUID, errChain) + require.NoError(t, err) + continue + } + cmd, err = mdmDevice.Acknowledge(cmd.CommandUUID) + require.NoError(t, err) + } + + // get that host - it should report "failed" for the profiles and include the error message detail + expectedErrs := map[string]string{ + "N1": "Failed to remove: MDMClientError (89): Profile with identifier 'I1' not found.\n", + "N2": "Failed to remove: MDMClientError (96): Cannot replace profile 'I2' because it was not installed by the MDM server.\n", + } + getHostResp = getHostResponse{} + s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/hosts/%d", host.ID), nil, http.StatusOK, &getHostResp) + for _, hm := range *getHostResp.Host.MDM.Profiles { + if wantErr, ok := expectedErrs[hm.Name]; ok { + require.Equal(t, fleet.MDMDeliveryFailed, *hm.Status) + require.Equal(t, wantErr, hm.Detail) + continue + } + require.Equal(t, fleet.MDMDeliveryVerified, *hm.Status) + } +} + func (s *integrationMDMTestSuite) TestSCEPCertExpiration() { t := s.T() ctx := context.Background()