add missing activity items, fix CLI error messages

for #17954

frontend/pages/DashboardPage/cards/ActivityFeed/ActivityItem/ActivityItem.tsx

@@ -799,10 +799,8 @@ const TAGGED_TEMPLATES = {
     return (
       <>
         {" "}
-        edited declaration (DDM) profile <b>
-          {activity.details?.profile_name}
-        </b>{" "}
-        for{" "}
+        edited declaration (DDM) profiles{" "}
+        <b>{activity.details?.profile_name}</b> for{" "}
         {getProfileMessageSuffix(
           isPremiumTier,
           "darwin",

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) {

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("<?xml")) || prefixMatches([]byte(`{`)) {
+	if prefixMatches(trimmedProfile, "<?xml") || prefixMatches(trimmedProfile, `{`) {
 		return "darwin"
 	}
 
-	if prefixMatches([]byte("<replace")) || prefixMatches([]byte("<add")) {
+	if prefixMatches(trimmedProfile, "<replace") || prefixMatches(trimmedProfile, "<add") {
 		return "windows"
 	}
 
 	return ""
 }
 
+// GuessProfileExtension determines the likely file extension of a profile
+// based on its content.
+//
+// It returns a string representing the determined file extension ("xml",
+// "json", or "") based on the profile's content.
+func GuessProfileExtension(profile []byte) string {
+	trimmedProfile := bytes.TrimSpace(profile)
+
+	switch {
+	case prefixMatches(trimmedProfile, "<?xml"),
+		prefixMatches(trimmedProfile, "<replace"),
+		prefixMatches(trimmedProfile, "<add"):
+		return "xml"
+	case prefixMatches(trimmedProfile, "{"):
+		return "json"
+	default:
+		return ""
+	}
+}
+
 const (
 
 	// FleetdConfigProfileName is the value for the PayloadDisplayName used by

server/mdm/mdm_test.go

@@ -198,3 +198,59 @@ func TestGetRawProfilePlatform(t *testing.T) {
 		})
 	}
 }
+
+func TestGuessProfileExtension(t *testing.T) {
+	testCases := []struct {
+		name     string
+		profile  []byte
+		expected string
+	}{
+		{
+			name:     "XML with <?xml prefix",
+			profile:  []byte("<?xml version=\"1.0\" encoding=\"UTF-8\"?>"),
+			expected: "xml",
+		},
+		{
+			name:     "XML with <replace prefix",
+			profile:  []byte("<replace value=\"something\"/>"),
+			expected: "xml",
+		},
+		{
+			name:     "XML with <add prefix",
+			profile:  []byte("<add key=\"somekey\" value=\"somevalue\"/>"),
+			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("   <?xml version=\"1.0\" encoding=\"UTF-8\"?>"),
+			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")
+		})
+	}
+}

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))
+}

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() {

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