From 9af01e53e9c09f5242661081253603839ae12918 Mon Sep 17 00:00:00 2001 From: Roberto Dip Date: Fri, 29 Mar 2024 14:43:28 -0300 Subject: [PATCH] add missing activity items, fix CLI error messages for #17954 --- .../ActivityItem/ActivityItem.tsx | 6 +- server/fleet/activities.go | 20 +++++++ server/mdm/mdm.go | 34 ++++++++--- server/mdm/mdm_test.go | 56 +++++++++++++++++++ server/service/integration_ddm_test.go | 11 ++++ server/service/integration_mdm_test.go | 48 +++++++++++++++- server/service/mdm.go | 15 +++-- 7 files changed, 169 insertions(+), 21 deletions(-) diff --git a/frontend/pages/DashboardPage/cards/ActivityFeed/ActivityItem/ActivityItem.tsx b/frontend/pages/DashboardPage/cards/ActivityFeed/ActivityItem/ActivityItem.tsx index a3941ac7a..999827869 100644 --- a/frontend/pages/DashboardPage/cards/ActivityFeed/ActivityItem/ActivityItem.tsx +++ b/frontend/pages/DashboardPage/cards/ActivityFeed/ActivityItem/ActivityItem.tsx @@ -799,10 +799,8 @@ const TAGGED_TEMPLATES = { return ( <> {" "} - edited declaration (DDM) profile - {activity.details?.profile_name} - {" "} - for{" "} + edited declaration (DDM) profiles{" "} + {activity.details?.profile_name} for{" "} {getProfileMessageSuffix( isPremiumTier, "darwin", diff --git a/server/fleet/activities.go b/server/fleet/activities.go index abd4835a4..f299222c2 100644 --- a/server/fleet/activities.go +++ b/server/fleet/activities.go @@ -87,6 +87,7 @@ var ActivityDetailsList = []ActivityDetails{ ActivityTypeCreatedDeclarationProfile{}, ActivityTypeDeletedDeclarationProfile{}, + ActivityTypeEditedDeclarationProfile{}, } type ActivityDetails interface { @@ -1370,6 +1371,25 @@ func (a ActivityTypeDeletedDeclarationProfile) Documentation() (activity string, }` } +type ActivityTypeEditedDeclarationProfile struct { + TeamID *uint `json:"team_id"` + TeamName *string `json:"team_name"` +} + +func (a ActivityTypeEditedDeclarationProfile) ActivityName() string { + return "edited_declaration_profile" +} + +func (a ActivityTypeEditedDeclarationProfile) Documentation() (activity string, details string, detailsExample string) { + return `Generated when a user edits the macOS declarations of a team (or no team) via the fleetctl CLI.`, + `This activity contains the following fields: +- "team_id": The ID of the team that the declarations apply to, ` + "`null`" + ` if they apply to devices that are not in a team. +- "team_name": The name of the team that the declarations apply to, ` + "`null`" + ` if they apply to devices that are not in a team.`, `{ + "team_id": 123, + "team_name": "Workstations" +}` +} + // LogRoleChangeActivities logs activities for each role change, globally and one for each change in teams. func LogRoleChangeActivities(ctx context.Context, ds Datastore, adminUser *User, oldGlobalRole *string, oldTeamRoles []UserTeam, user *User) error { if user.GlobalRole != nil && (oldGlobalRole == nil || *oldGlobalRole != *user.GlobalRole) { diff --git a/server/mdm/mdm.go b/server/mdm/mdm.go index 5a15dd8c0..e2888fbc5 100644 --- a/server/mdm/mdm.go +++ b/server/mdm/mdm.go @@ -30,6 +30,11 @@ func DecryptBase64CMS(p7Base64 string, cert *x509.Certificate, key crypto.Privat return p7.Decrypt(cert, key) } +func prefixMatches(val []byte, prefix string) bool { + return len(val) >= len(prefix) && + bytes.EqualFold([]byte(prefix), val[:len(prefix)]) +} + // GetRawProfilePlatform identifies the platform type of a profile bytes by // examining its initial content: // @@ -45,22 +50,37 @@ func GetRawProfilePlatform(profile []byte) string { return "" } - prefixMatches := func(prefix []byte) bool { - return len(trimmedProfile) >= len(prefix) && - bytes.EqualFold(prefix, trimmedProfile[:len(prefix)]) - } - - if prefixMatches([]byte(""), + expected: "xml", + }, + { + name: "XML with "), + expected: "xml", + }, + { + name: "XML with "), + expected: "xml", + }, + { + name: "JSON with { prefix", + profile: []byte("{ \"key\": \"value\" }"), + expected: "json", + }, + { + name: "Empty string", + profile: []byte(""), + expected: "", + }, + { + name: "Text with no recognizable prefix", + profile: []byte("This is just some text."), + expected: "", + }, + { + name: "XML with spaces before prefix", + profile: []byte(" "), + expected: "xml", + }, + { + name: "JSON with spaces before prefix", + profile: []byte(" { \"key\": \"value\" }"), + expected: "json", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + result := GuessProfileExtension(tc.profile) + require.Equal(t, tc.expected, result, "Expected result does not match actual result") + }) + } +} diff --git a/server/service/integration_ddm_test.go b/server/service/integration_ddm_test.go index cceafd1a8..122eec66e 100644 --- a/server/service/integration_ddm_test.go +++ b/server/service/integration_ddm_test.go @@ -944,3 +944,14 @@ func declarationForTest(identifier string) []byte { "Identifier": "%s" }`, identifier)) } + +func declarationForTestWithType(identifier string, dType string) []byte { + return []byte(fmt.Sprintf(` +{ + "Type": "%s", + "Payload": { + "Echo": "foo" + }, + "Identifier": "%s" +}`, dType, identifier)) +} diff --git a/server/service/integration_mdm_test.go b/server/service/integration_mdm_test.go index d026d849d..2ec8f3ca3 100644 --- a/server/service/integration_mdm_test.go +++ b/server/service/integration_mdm_test.go @@ -10846,6 +10846,11 @@ func (s *integrationMDMTestSuite) TestBatchSetMDMProfiles() { `{"team_id": null, "team_name": null}`, 0, ) + s.lastActivityOfTypeMatches( + fleet.ActivityTypeEditedDeclarationProfile{}.ActivityName(), + `{"team_id": null, "team_name": null}`, + 0, + ) // apply to both team id and name s.Do("POST", "/api/v1/fleet/mdm/profiles/batch", batchSetMDMProfilesRequest{Profiles: nil}, @@ -10860,6 +10865,7 @@ func (s *integrationMDMTestSuite) TestBatchSetMDMProfiles() { {Name: "N1", Contents: mobileconfigForTest("N1", "I1")}, {Name: "N2", Contents: mobileconfigForTest("N1", "I2")}, {Name: "N3", Contents: syncMLForTest("./Foo/Bar")}, + {Name: "N4", Contents: declarationForTest("D1")}, }}, http.StatusUnprocessableEntity, "team_id", strconv.Itoa(int(tm.ID))) // profiles with reserved macOS identifiers @@ -10868,6 +10874,7 @@ func (s *integrationMDMTestSuite) TestBatchSetMDMProfiles() { {Name: "N1", Contents: mobileconfigForTest("N1", "I1")}, {Name: p, Contents: mobileconfigForTest(p, p)}, {Name: "N3", Contents: syncMLForTest("./Foo/Bar")}, + {Name: "N4", Contents: declarationForTest("D1")}, }}, http.StatusUnprocessableEntity, "team_id", strconv.Itoa(int(tm.ID))) errMsg := extractServerErrorText(res.Body) require.Contains(t, errMsg, fmt.Sprintf("Validation Failed: payload identifier %s is not allowed", p)) @@ -10878,6 +10885,7 @@ func (s *integrationMDMTestSuite) TestBatchSetMDMProfiles() { res := s.Do("POST", "/api/v1/fleet/mdm/profiles/batch", batchSetMDMProfilesRequest{Profiles: []fleet.MDMProfileBatchPayload{ {Name: "N1", Contents: mobileconfigForTestWithContent("N1", "I1", "II1", p, "")}, {Name: "N3", Contents: syncMLForTest("./Foo/Bar")}, + {Name: "N4", Contents: declarationForTest("D1")}, }}, http.StatusUnprocessableEntity, "team_id", strconv.Itoa(int(tm.ID))) errMsg := extractServerErrorText(res.Body) require.Contains(t, errMsg, fmt.Sprintf("Validation Failed: unsupported PayloadType(s): %s", p)) @@ -10888,19 +10896,48 @@ func (s *integrationMDMTestSuite) TestBatchSetMDMProfiles() { res := s.Do("POST", "/api/v1/fleet/mdm/profiles/batch", batchSetMDMProfilesRequest{Profiles: []fleet.MDMProfileBatchPayload{ {Name: "N1", Contents: mobileconfigForTestWithContent("N1", "I1", p, "random", "")}, {Name: "N3", Contents: syncMLForTest("./Foo/Bar")}, + {Name: "N4", Contents: declarationForTest("D1")}, }}, http.StatusUnprocessableEntity, "team_id", strconv.Itoa(int(tm.ID))) errMsg := extractServerErrorText(res.Body) require.Contains(t, errMsg, fmt.Sprintf("Validation Failed: unsupported PayloadIdentifier(s): %s", p)) } + // profiles with forbidden declaration types + for dt := range fleet.ForbiddenDeclTypes { + res := s.Do("POST", "/api/v1/fleet/mdm/profiles/batch", batchSetMDMProfilesRequest{Profiles: []fleet.MDMProfileBatchPayload{ + {Name: "N1", Contents: mobileconfigForTest("N1", "I1")}, + {Name: "N3", Contents: syncMLForTest("./Foo/Bar")}, + {Name: "N4", Contents: declarationForTestWithType("D1", dt)}, + }}, http.StatusUnprocessableEntity, "team_id", strconv.Itoa(int(tm.ID))) + errMsg := extractServerErrorText(res.Body) + require.Contains(t, errMsg, "Only configuration declarations that don’t require an asset reference are supported", dt) + } + // and one more for the software update declaration + res := s.Do("POST", "/api/v1/fleet/mdm/profiles/batch", batchSetMDMProfilesRequest{Profiles: []fleet.MDMProfileBatchPayload{ + {Name: "N1", Contents: mobileconfigForTest("N1", "I1")}, + {Name: "N3", Contents: syncMLForTest("./Foo/Bar")}, + {Name: "N4", Contents: declarationForTestWithType("D1", "com.apple.configuration.softwareupdate.enforcement.specific")}, + }}, http.StatusUnprocessableEntity, "team_id", strconv.Itoa(int(tm.ID))) + errMsg := extractServerErrorText(res.Body) + require.Contains(t, errMsg, "Declaration profile can’t include OS updates settings. To control these settings, go to OS updates.") + + // invalid JSON + res = s.Do("POST", "/api/v1/fleet/mdm/profiles/batch", batchSetMDMProfilesRequest{Profiles: []fleet.MDMProfileBatchPayload{ + {Name: "N1", Contents: mobileconfigForTest("N1", "I1")}, + {Name: "N3", Contents: syncMLForTest("./Foo/Bar")}, + {Name: "N4", Contents: []byte(`{"foo":}`)}, + }}, http.StatusBadRequest, "team_id", strconv.Itoa(int(tm.ID))) + errMsg = extractServerErrorText(res.Body) + require.Contains(t, errMsg, "The file should include valid JSON") + // profiles with reserved Windows location URIs // bitlocker - res := s.Do("POST", "/api/v1/fleet/mdm/profiles/batch", batchSetMDMProfilesRequest{Profiles: []fleet.MDMProfileBatchPayload{ + res = s.Do("POST", "/api/v1/fleet/mdm/profiles/batch", batchSetMDMProfilesRequest{Profiles: []fleet.MDMProfileBatchPayload{ {Name: "N1", Contents: mobileconfigForTest("N1", "I1")}, {Name: syncml.FleetBitLockerTargetLocURI, Contents: syncMLForTest(fmt.Sprintf("%s/Foo", syncml.FleetBitLockerTargetLocURI))}, {Name: "N3", Contents: syncMLForTest("./Foo/Bar")}, }}, http.StatusUnprocessableEntity, "team_id", strconv.Itoa(int(tm.ID))) - errMsg := extractServerErrorText(res.Body) + errMsg = extractServerErrorText(res.Body) require.Contains(t, errMsg, "Custom configuration profiles can't include BitLocker settings. To control these settings, use the mdm.enable_disk_encryption option.") // os updates @@ -10930,6 +10967,7 @@ func (s *integrationMDMTestSuite) TestBatchSetMDMProfiles() { s.Do("POST", "/api/v1/fleet/mdm/profiles/batch", batchSetMDMProfilesRequest{Profiles: []fleet.MDMProfileBatchPayload{ {Name: "N1", Contents: mobileconfigForTest("N1", "I1")}, {Name: "N2", Contents: syncMLForTest("./Foo/Bar")}, + {Name: "N4", Contents: declarationForTest("D1")}, }}, http.StatusNoContent, "team_id", strconv.Itoa(int(tm.ID)), "dry_run", "true") s.assertConfigProfilesByIdentifier(&tm.ID, "I1", false) s.assertWindowsConfigProfilesByName(&tm.ID, "N1", false) @@ -10938,6 +10976,7 @@ func (s *integrationMDMTestSuite) TestBatchSetMDMProfiles() { s.Do("POST", "/api/v1/fleet/mdm/profiles/batch", batchSetMDMProfilesRequest{Profiles: []fleet.MDMProfileBatchPayload{ {Name: "N1", Contents: mobileconfigForTest("N1", "I1")}, {Name: "N2", Contents: syncMLForTest("./Foo/Bar")}, + {Name: "N4", Contents: declarationForTest("D1")}, }}, http.StatusNoContent, "team_id", strconv.Itoa(int(tm.ID))) s.assertConfigProfilesByIdentifier(&tm.ID, "I1", true) s.assertWindowsConfigProfilesByName(&tm.ID, "N2", true) @@ -10951,6 +10990,11 @@ func (s *integrationMDMTestSuite) TestBatchSetMDMProfiles() { fmt.Sprintf(`{"team_id": %d, "team_name": %q}`, tm.ID, tm.Name), 0, ) + s.lastActivityOfTypeMatches( + fleet.ActivityTypeEditedDeclarationProfile{}.ActivityName(), + fmt.Sprintf(`{"team_id": %d, "team_name": %q}`, tm.ID, tm.Name), + 0, + ) } func (s *integrationMDMTestSuite) TestBatchSetMDMProfilesBackwardsCompat() { diff --git a/server/service/mdm.go b/server/service/mdm.go index 63c71ba28..b6c932437 100644 --- a/server/service/mdm.go +++ b/server/service/mdm.go @@ -1568,6 +1568,12 @@ func (svc *Service) BatchSetMDMProfiles( }); err != nil { return ctxerr.Wrap(ctx, err, "logging activity for edited windows profile") } + if err := svc.ds.NewActivity(ctx, authz.UserFromContext(ctx), &fleet.ActivityTypeEditedDeclarationProfile{ + TeamID: tmID, + TeamName: tmName, + }); err != nil { + return ctxerr.Wrap(ctx, err, "logging activity for edited macos declarations") + } return nil } @@ -1631,14 +1637,7 @@ func getAppleProfiles( } // Check for DDM files - - isJSON := func(b []byte) bool { - var js json.RawMessage - return json.Unmarshal(b, &js) == nil - } - - // TODO(roberto): As a mini optimization, GetRawDeclarationValues could replace isJSON. - if isJSON(prof.Contents) { + if mdm.GuessProfileExtension(prof.Contents) == "json" { rawDecl, err := fleet.GetRawDeclarationValues(prof.Contents) if err != nil { return nil, nil, err