Skip to content

Commit

Permalink
add missing activity items, fix CLI error messages (#17961)
Browse files Browse the repository at this point in the history
for #17954
  • Loading branch information
Roberto Dip authored Mar 29, 2024
2 parents 7973a32 + 9af01e5 commit 140dde1
Show file tree
Hide file tree
Showing 7 changed files with 169 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
20 changes: 20 additions & 0 deletions server/fleet/activities.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ var ActivityDetailsList = []ActivityDetails{

ActivityTypeCreatedDeclarationProfile{},
ActivityTypeDeletedDeclarationProfile{},
ActivityTypeEditedDeclarationProfile{},
}

type ActivityDetails interface {
Expand Down Expand Up @@ -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) {
Expand Down
34 changes: 27 additions & 7 deletions server/mdm/mdm.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
//
Expand All @@ -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
Expand Down
56 changes: 56 additions & 0 deletions server/mdm/mdm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
})
}
}
11 changes: 11 additions & 0 deletions server/service/integration_ddm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1018,3 +1018,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))
}
48 changes: 46 additions & 2 deletions server/service/integration_mdm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand All @@ -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
Expand All @@ -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))
Expand All @@ -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))
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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() {
Expand Down
15 changes: 7 additions & 8 deletions server/service/mdm.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 140dde1

Please sign in to comment.