Fix unreleased bugs with labels_include_any feature (#23734)

This commit is contained in:
Sarah Gillespie
2024-11-14 08:30:32 -06:00
committed by GitHub
parent aed3c3f775
commit 16d0d8d58f
12 changed files with 454 additions and 77 deletions
@@ -171,6 +171,7 @@ const CustomSettings = ({
const hasLabels =
!!profileLabelsModalData?.labels_include_all?.length ||
!!profileLabelsModalData?.labels_include_any?.length ||
!!profileLabelsModalData?.labels_exclude_any?.length;
return (
@@ -70,7 +70,7 @@ const ProfileLabelsModal = ({
if (labels_include_all) {
targetTypeText = <b>have all</b>;
} else if (labels_include_any) {
targetTypeText = <b>have all</b>;
targetTypeText = <b>have any</b>;
} else {
targetTypeText = <b>don&apos;t have any</b>;
}
+45 -4
View File
@@ -247,9 +247,19 @@ WHERE
return nil, err
}
for _, lbl := range labels {
if lbl.Exclude {
switch {
case lbl.Exclude && lbl.RequireAll:
// this should never happen so log it for debugging
level.Debug(ds.logger).Log("msg", "unsupported profile label: cannot be both exclude and require all",
"profile_uuid", lbl.ProfileUUID,
"label_name", lbl.LabelName,
)
case lbl.Exclude && !lbl.RequireAll:
res.LabelsExcludeAny = append(res.LabelsExcludeAny, lbl)
} else {
case !lbl.Exclude && !lbl.RequireAll:
res.LabelsIncludeAny = append(res.LabelsIncludeAny, lbl)
default:
// default include all
res.LabelsIncludeAll = append(res.LabelsIncludeAll, lbl)
}
}
@@ -289,9 +299,19 @@ WHERE
return nil, err
}
for _, lbl := range labels {
if lbl.Exclude {
switch {
case lbl.Exclude && lbl.RequireAll:
// this should never happen so log it for debugging
level.Debug(ds.logger).Log("msg", "unsupported profile label: cannot be both exclude and require all",
"profile_uuid", lbl.ProfileUUID,
"label_name", lbl.LabelName,
)
case lbl.Exclude && !lbl.RequireAll:
res.LabelsExcludeAny = append(res.LabelsExcludeAny, lbl)
} else {
case !lbl.Exclude && !lbl.RequireAll:
res.LabelsIncludeAny = append(res.LabelsIncludeAny, lbl)
default:
// default include all
res.LabelsIncludeAll = append(res.LabelsIncludeAll, lbl)
}
}
@@ -1828,16 +1848,28 @@ ON DUPLICATE KEY UPDATE
for _, label := range incomingProf.LabelsIncludeAll {
label.ProfileUUID = newlyInsertedProf.ProfileUUID
label.Exclude = false
label.RequireAll = true
incomingLabels = append(incomingLabels, label)
}
for _, label := range incomingProf.LabelsIncludeAny {
label.ProfileUUID = newlyInsertedProf.ProfileUUID
label.Exclude = false
label.RequireAll = false
incomingLabels = append(incomingLabels, label)
}
for _, label := range incomingProf.LabelsExcludeAny {
label.ProfileUUID = newlyInsertedProf.ProfileUUID
label.Exclude = true
label.RequireAll = false
incomingLabels = append(incomingLabels, label)
}
}
}
// FIXME: At what point are we deleting label associations for existing profiles (e.g. if the user
// removes all labels from a profile in gitops, shouldn't we remove the old associations)?
// insert label associations
var updatedLabels bool
if updatedLabels, err = batchSetProfileLabelAssociationsDB(ctx, tx, incomingLabels,
@@ -4289,11 +4321,20 @@ WHERE
for _, label := range incomingDecl.LabelsIncludeAll {
label.ProfileUUID = newlyInsertedDecl.DeclarationUUID
label.Exclude = false
label.RequireAll = true
incomingLabels = append(incomingLabels, label)
}
for _, label := range incomingDecl.LabelsIncludeAny {
label.ProfileUUID = newlyInsertedDecl.DeclarationUUID
label.Exclude = false
label.RequireAll = false
incomingLabels = append(incomingLabels, label)
}
for _, label := range incomingDecl.LabelsExcludeAny {
label.ProfileUUID = newlyInsertedDecl.DeclarationUUID
label.Exclude = true
label.RequireAll = false
incomingLabels = append(incomingLabels, label)
}
}
+18 -4
View File
@@ -274,9 +274,19 @@ FROM (
}
for _, label := range labels {
if prof, ok := profMap[label.ProfileUUID]; ok {
if label.Exclude {
switch {
case label.Exclude && label.RequireAll:
// this should never happen so log it for debugging
level.Debug(ds.logger).Log("msg", "unsupported profile label: cannot be both exclude and require all",
"profile_uuid", label.ProfileUUID,
"label_name", label.LabelName,
)
case label.Exclude && !label.RequireAll:
prof.LabelsExcludeAny = append(prof.LabelsExcludeAny, label)
} else {
case !label.Exclude && !label.RequireAll:
prof.LabelsIncludeAny = append(prof.LabelsIncludeAny, label)
default:
// default include all
prof.LabelsIncludeAll = append(prof.LabelsIncludeAll, label)
}
}
@@ -293,7 +303,8 @@ SELECT
label_name,
COALESCE(label_id, 0) as label_id,
IF(label_id IS NULL, 1, 0) as broken,
exclude
exclude,
require_all
FROM
mdm_configuration_profile_labels mcpl
WHERE
@@ -305,7 +316,8 @@ SELECT
label_name,
COALESCE(label_id, 0) as label_id,
IF(label_id IS NULL, 1, 0) as broken,
exclude
exclude,
require_all
FROM
mdm_declaration_labels mdl
WHERE
@@ -997,6 +1009,8 @@ func batchSetProfileLabelAssociationsDB(
platform string,
) (updatedDB bool, err error) {
if len(profileLabels) == 0 {
// FIXME: At what point are we deleting all labels for a profile (e.g., the user might
// remove all labels from an existing profile)?
return false, nil
}
+8 -8
View File
@@ -699,20 +699,20 @@ func testListMDMConfigProfiles(t *testing.T, ds *Datastore) {
require.NoError(t, ds.DeleteLabel(ctx, labels[4].Name))
profLabels := map[string][]fleet.ConfigurationProfileLabel{
"C": {
{LabelName: labels[0].Name, LabelID: labels[0].ID},
{LabelName: labels[1].Name, LabelID: labels[1].ID},
{LabelName: labels[0].Name, LabelID: labels[0].ID, RequireAll: true},
{LabelName: labels[1].Name, LabelID: labels[1].ID, RequireAll: true},
},
"D": {
{LabelName: labels[2].Name, LabelID: labels[2].ID},
{LabelName: labels[3].Name, LabelID: 0, Broken: true},
{LabelName: labels[2].Name, LabelID: labels[2].ID, RequireAll: true},
{LabelName: labels[3].Name, LabelID: 0, Broken: true, RequireAll: true},
},
"E": {
{LabelName: labels[4].Name, LabelID: 0, Broken: true},
{LabelName: labels[5].Name, LabelID: labels[5].ID},
{LabelName: labels[4].Name, LabelID: 0, Broken: true, RequireAll: true},
{LabelName: labels[5].Name, LabelID: labels[5].ID, RequireAll: true},
},
"F": {
{LabelName: labels[6].Name, LabelID: labels[6].ID},
{LabelName: labels[7].Name, LabelID: labels[7].ID},
{LabelName: labels[6].Name, LabelID: labels[6].ID, RequireAll: true},
{LabelName: labels[7].Name, LabelID: labels[7].ID, RequireAll: true},
},
}
+21 -2
View File
@@ -745,9 +745,19 @@ WHERE
return nil, err
}
for _, lbl := range labels {
if lbl.Exclude {
switch {
case lbl.Exclude && lbl.RequireAll:
// this should never happen so log it for debugging
level.Debug(ds.logger).Log("msg", "unsupported profile label: cannot be both exclude and require all",
"profile_uuid", lbl.ProfileUUID,
"label_name", lbl.LabelName,
)
case lbl.Exclude && !lbl.RequireAll:
res.LabelsExcludeAny = append(res.LabelsExcludeAny, lbl)
} else {
case !lbl.Exclude && !lbl.RequireAll:
res.LabelsIncludeAny = append(res.LabelsIncludeAny, lbl)
default:
// default include all
res.LabelsIncludeAll = append(res.LabelsIncludeAll, lbl)
}
}
@@ -1872,11 +1882,20 @@ ON DUPLICATE KEY UPDATE
for _, label := range incomingProf.LabelsIncludeAll {
label.ProfileUUID = newlyInsertedProf.ProfileUUID
label.Exclude = false
label.RequireAll = true
incomingLabels = append(incomingLabels, label)
}
for _, label := range incomingProf.LabelsIncludeAny {
label.ProfileUUID = newlyInsertedProf.ProfileUUID
label.Exclude = false
label.RequireAll = false
incomingLabels = append(incomingLabels, label)
}
for _, label := range incomingProf.LabelsExcludeAny {
label.ProfileUUID = newlyInsertedProf.ProfileUUID
label.Exclude = true
label.RequireAll = false
incomingLabels = append(incomingLabels, label)
}
}
+1
View File
@@ -385,6 +385,7 @@ func (s *MacOSSettings) FromMap(m map[string]interface{}) (map[string]bool, erro
spec.Labels = extractLabelField(m, "labels")
spec.LabelsIncludeAll = extractLabelField(m, "labels_include_all")
spec.LabelsExcludeAny = extractLabelField(m, "labels_exclude_any")
spec.LabelsIncludeAny = extractLabelField(m, "labels_include_any")
csSpecs = append(csSpecs, spec)
} else if m, ok := v.(string); ok { // for backwards compatibility with the old way to define profiles
+10 -6
View File
@@ -525,26 +525,30 @@ func (p *MDMProfileSpec) UnmarshalJSON(data []byte) error {
if len(data) == 0 {
return nil
}
if lookAhead := bytes.TrimSpace(data); len(lookAhead) > 0 && lookAhead[0] == '"' {
var backwardsCompat string
if err := json.Unmarshal(data, &backwardsCompat); err != nil {
return fmt.Errorf("unmarshal profile spec. Error using old format: %w", err)
}
p.Path = backwardsCompat
// FIXME: equivalent of no label condition, should clear all labels slice?
// p.Labels = nil
// p.LabelsIncludeAll = nil
// p.LabelsIncludeAny = nil
// p.LabelsExcludeAny = nil
return nil
}
// use an alias type to avoid recursively calling this function forever.
type Alias MDMProfileSpec
aliasData := struct {
*Alias
}{
Alias: (*Alias)(p),
}
var aliasData Alias
if err := json.Unmarshal(data, &aliasData); err != nil {
return fmt.Errorf("unmarshal profile spec. Error using new format: %w", err)
}
// NOTE: we always want the newly unmarshaled profile spec to completely replace the old one
// (rather than merging the new data into the old one).
*p = MDMProfileSpec(aliasData)
return nil
}
+130 -34
View File
@@ -2,6 +2,7 @@ package fleet_test
import (
"context"
"encoding/json"
"errors"
"net/http"
"net/http/httptest"
@@ -122,72 +123,106 @@ func TestDEPClient(t *testing.T) {
wantToksTermsFlags map[string]bool
}{
// use a valid token, appconfig should not be updated (already unflagged)
{token: validToken, orgName: "org1", wantErr: false, readInvoked: true, writeTokInvoked: false,
writeAppCfgInvoked: false, wantAppCfgTermsFlag: false, wantToksTermsFlags: map[string]bool{"org1": false, "org2": false}},
{
token: validToken, orgName: "org1", wantErr: false, readInvoked: true, writeTokInvoked: false,
writeAppCfgInvoked: false, wantAppCfgTermsFlag: false, wantToksTermsFlags: map[string]bool{"org1": false, "org2": false},
},
// use a valid token without org, nothing is checked
{token: validToken, orgName: "", wantErr: false, readInvoked: false, writeTokInvoked: false,
writeAppCfgInvoked: false, wantAppCfgTermsFlag: false, wantToksTermsFlags: map[string]bool{"org1": false, "org2": false}},
{
token: validToken, orgName: "", wantErr: false, readInvoked: false, writeTokInvoked: false,
writeAppCfgInvoked: false, wantAppCfgTermsFlag: false, wantToksTermsFlags: map[string]bool{"org1": false, "org2": false},
},
// use an invalid token without org, call fails but nothing is checked because this is an unsaved token
{token: invalidToken, orgName: "", wantErr: true, readInvoked: false, writeTokInvoked: false,
writeAppCfgInvoked: false, wantAppCfgTermsFlag: false, wantToksTermsFlags: map[string]bool{"org1": false, "org2": false}},
{
token: invalidToken, orgName: "", wantErr: true, readInvoked: false, writeTokInvoked: false,
writeAppCfgInvoked: false, wantAppCfgTermsFlag: false, wantToksTermsFlags: map[string]bool{"org1": false, "org2": false},
},
// use an invalid token, appconfig should not even be read (not a terms error)
{token: invalidToken, orgName: "org1", wantErr: true, readInvoked: false, writeTokInvoked: false,
writeAppCfgInvoked: false, wantAppCfgTermsFlag: false, wantToksTermsFlags: map[string]bool{"org1": false, "org2": false}},
{
token: invalidToken, orgName: "org1", wantErr: true, readInvoked: false, writeTokInvoked: false,
writeAppCfgInvoked: false, wantAppCfgTermsFlag: false, wantToksTermsFlags: map[string]bool{"org1": false, "org2": false},
},
// terms changed for org1 during the auth request
{token: termsChangedToken, orgName: "org1", wantErr: true, readInvoked: true, writeTokInvoked: true,
writeAppCfgInvoked: true, wantAppCfgTermsFlag: true, wantToksTermsFlags: map[string]bool{"org1": true, "org2": false}},
{
token: termsChangedToken, orgName: "org1", wantErr: true, readInvoked: true, writeTokInvoked: true,
writeAppCfgInvoked: true, wantAppCfgTermsFlag: true, wantToksTermsFlags: map[string]bool{"org1": true, "org2": false},
},
// use of an invalid token does not update the flag
{token: invalidToken, orgName: "org1", wantErr: true, readInvoked: false, writeTokInvoked: false,
writeAppCfgInvoked: false, wantAppCfgTermsFlag: true, wantToksTermsFlags: map[string]bool{"org1": true, "org2": false}},
{
token: invalidToken, orgName: "org1", wantErr: true, readInvoked: false, writeTokInvoked: false,
writeAppCfgInvoked: false, wantAppCfgTermsFlag: true, wantToksTermsFlags: map[string]bool{"org1": true, "org2": false},
},
// use of a valid token for org1 resets the flags
{token: validToken, orgName: "org1", wantErr: false, readInvoked: true, writeTokInvoked: true,
writeAppCfgInvoked: true, wantAppCfgTermsFlag: false, wantToksTermsFlags: map[string]bool{"org1": false, "org2": false}},
{
token: validToken, orgName: "org1", wantErr: false, readInvoked: true, writeTokInvoked: true,
writeAppCfgInvoked: true, wantAppCfgTermsFlag: false, wantToksTermsFlags: map[string]bool{"org1": false, "org2": false},
},
// use of a valid token again with org2 does not update anything
{token: validToken, orgName: "org2", wantErr: false, readInvoked: true, writeTokInvoked: false,
writeAppCfgInvoked: false, wantAppCfgTermsFlag: false, wantToksTermsFlags: map[string]bool{"org1": false, "org2": false}},
{
token: validToken, orgName: "org2", wantErr: false, readInvoked: true, writeTokInvoked: false,
writeAppCfgInvoked: false, wantAppCfgTermsFlag: false, wantToksTermsFlags: map[string]bool{"org1": false, "org2": false},
},
// terms changed for org2 during the actual account request, after auth
{token: termsChangedAfterAuthToken, orgName: "org2", wantErr: true, readInvoked: true, writeTokInvoked: true,
writeAppCfgInvoked: true, wantAppCfgTermsFlag: true, wantToksTermsFlags: map[string]bool{"org1": false, "org2": true}},
{
token: termsChangedAfterAuthToken, orgName: "org2", wantErr: true, readInvoked: true, writeTokInvoked: true,
writeAppCfgInvoked: true, wantAppCfgTermsFlag: true, wantToksTermsFlags: map[string]bool{"org1": false, "org2": true},
},
// again terms changed after auth for org2, doesn't update appConfig
{token: termsChangedAfterAuthToken, orgName: "org2", wantErr: true, readInvoked: true, writeTokInvoked: true,
writeAppCfgInvoked: false, wantAppCfgTermsFlag: true, wantToksTermsFlags: map[string]bool{"org1": false, "org2": true}},
{
token: termsChangedAfterAuthToken, orgName: "org2", wantErr: true, readInvoked: true, writeTokInvoked: true,
writeAppCfgInvoked: false, wantAppCfgTermsFlag: true, wantToksTermsFlags: map[string]bool{"org1": false, "org2": true},
},
// terms changed during auth for org2, doesn't update appConfig
{token: termsChangedToken, orgName: "org2", wantErr: true, readInvoked: true, writeTokInvoked: true,
writeAppCfgInvoked: false, wantAppCfgTermsFlag: true, wantToksTermsFlags: map[string]bool{"org1": false, "org2": true}},
{
token: termsChangedToken, orgName: "org2", wantErr: true, readInvoked: true, writeTokInvoked: true,
writeAppCfgInvoked: false, wantAppCfgTermsFlag: true, wantToksTermsFlags: map[string]bool{"org1": false, "org2": true},
},
// terms changed during auth for org1, now both tokens have the flag, doesn't update appConfig
{token: termsChangedToken, orgName: "org1", wantErr: true, readInvoked: true, writeTokInvoked: true,
writeAppCfgInvoked: false, wantAppCfgTermsFlag: true, wantToksTermsFlags: map[string]bool{"org1": true, "org2": true}},
{
token: termsChangedToken, orgName: "org1", wantErr: true, readInvoked: true, writeTokInvoked: true,
writeAppCfgInvoked: false, wantAppCfgTermsFlag: true, wantToksTermsFlags: map[string]bool{"org1": true, "org2": true},
},
// use a valid token without org, nothing is checked
{token: validToken, orgName: "", wantErr: false, readInvoked: false, writeTokInvoked: false,
writeAppCfgInvoked: false, wantAppCfgTermsFlag: true, wantToksTermsFlags: map[string]bool{"org1": true, "org2": true}},
{
token: validToken, orgName: "", wantErr: false, readInvoked: false, writeTokInvoked: false,
writeAppCfgInvoked: false, wantAppCfgTermsFlag: true, wantToksTermsFlags: map[string]bool{"org1": true, "org2": true},
},
// use an invalid token without org, call fails but nothing is checked because this is an unsaved token
{token: invalidToken, orgName: "", wantErr: true, readInvoked: false, writeTokInvoked: false,
writeAppCfgInvoked: false, wantAppCfgTermsFlag: true, wantToksTermsFlags: map[string]bool{"org1": true, "org2": true}},
{
token: invalidToken, orgName: "", wantErr: true, readInvoked: false, writeTokInvoked: false,
writeAppCfgInvoked: false, wantAppCfgTermsFlag: true, wantToksTermsFlags: map[string]bool{"org1": true, "org2": true},
},
// valid token for org1, resets that token's flag but not appConfig
{token: validToken, orgName: "org1", wantErr: false, readInvoked: true, writeTokInvoked: true,
writeAppCfgInvoked: false, wantAppCfgTermsFlag: true, wantToksTermsFlags: map[string]bool{"org1": false, "org2": true}},
{
token: validToken, orgName: "org1", wantErr: false, readInvoked: true, writeTokInvoked: true,
writeAppCfgInvoked: false, wantAppCfgTermsFlag: true, wantToksTermsFlags: map[string]bool{"org1": false, "org2": true},
},
// valid token again for org1, still no write to appConfig
{token: validToken, orgName: "org1", wantErr: false, readInvoked: true, writeTokInvoked: true,
writeAppCfgInvoked: false, wantAppCfgTermsFlag: true, wantToksTermsFlags: map[string]bool{"org1": false, "org2": true}},
{
token: validToken, orgName: "org1", wantErr: false, readInvoked: true, writeTokInvoked: true,
writeAppCfgInvoked: false, wantAppCfgTermsFlag: true, wantToksTermsFlags: map[string]bool{"org1": false, "org2": true},
},
// valid token again for org2, this time resets appConfig
{token: validToken, orgName: "org2", wantErr: false, readInvoked: true, writeTokInvoked: true,
writeAppCfgInvoked: true, wantAppCfgTermsFlag: false, wantToksTermsFlags: map[string]bool{"org1": false, "org2": false}},
{
token: validToken, orgName: "org2", wantErr: false, readInvoked: true, writeTokInvoked: true,
writeAppCfgInvoked: true, wantAppCfgTermsFlag: false, wantToksTermsFlags: map[string]bool{"org1": false, "org2": false},
},
}
// order of calls is important, and test must not be parallelized as it would
@@ -333,6 +368,67 @@ func TestMDMProfileSpecUnmarshalJSON(t *testing.T) {
require.Equal(t, "oldpath", p.Path)
require.Empty(t, p.Labels)
})
t.Run("changing labels", func(t *testing.T) {
// When updating AppConfig, we unmarshal the incoming JSON into the existing AppConfig
// struct, see
// https://github.com/fleetdm/fleet/blob/d1144df1318b50482cbd9eb996b863443975f138/server/service/appconfig.go#L334-L335
//
// But we found there were issues unmarshaling the slice of profile specs where if a key is present in an old
// element but not in the new element (e.g. element[0] of the old slice and element[0] of the
// new slice), both keys were preserved. This test is designed to cover that issue, which
// was addressed in the unmarshal function, see
// https://github.com/fleetdm/fleet/blob/1042702def54f095335d8b42ed5fdcc90468fa0d/server/fleet/mdm.go#L551-L552
storedConfig := fleet.AppConfig{
OrgInfo: fleet.OrgInfo{
OrgName: "Test",
},
MDM: fleet.MDM{
MacOSSettings: fleet.MacOSSettings{
CustomSettings: []fleet.MDMProfileSpec{
{
Path: "some-profile-2",
LabelsExcludeAny: []string{"bar"},
},
{
Path: "some-profile-1",
LabelsIncludeAll: []string{"foo"},
},
},
},
},
}
incomingConfig := fleet.AppConfig{
OrgInfo: fleet.OrgInfo{
OrgName: "Test",
},
MDM: fleet.MDM{
MacOSSettings: fleet.MacOSSettings{
CustomSettings: []fleet.MDMProfileSpec{
{
Path: "some-profile-1",
LabelsIncludeAll: []string{"foo"},
},
{
Path: "some-profile-2",
LabelsIncludeAny: []string{"bar"},
},
},
},
},
}
b, err := json.Marshal(incomingConfig)
require.NoError(t, err)
err = json.Unmarshal(b, &storedConfig)
require.NoError(t, err)
require.Equal(t, storedConfig.MDM.MacOSSettings.CustomSettings, incomingConfig.MDM.MacOSSettings.CustomSettings)
require.Nil(t, storedConfig.MDM.MacOSSettings.CustomSettings[0].LabelsExcludeAny) // old key should be removed
require.Nil(t, storedConfig.MDM.MacOSSettings.CustomSettings[1].LabelsIncludeAll) // old key should be removed
})
}
func TestMDMProfileSpecsMatch(t *testing.T) {
+6 -4
View File
@@ -1511,7 +1511,8 @@ func TestModifyAppConfigForNDESSCEPProxy(t *testing.T) {
svc, ctx = newTestServiceWithConfig(t, ds, fleetConfig, nil, nil, &TestServerOpts{License: &fleet.LicenseInfo{Tier: fleet.TierPremium}})
ctx = viewer.NewContext(ctx, viewer.Viewer{User: admin})
ds.NewActivityFunc = func(ctx context.Context, user *fleet.User, activity fleet.ActivityDetails, details []byte,
createdAt time.Time) error {
createdAt time.Time,
) error {
assert.IsType(t, fleet.ActivityAddedNDESSCEPProxy{}, activity)
return nil
}
@@ -1560,7 +1561,8 @@ func TestModifyAppConfigForNDESSCEPProxy(t *testing.T) {
scepURL = "https://new.com/mscep/mscep.dll"
jsonPayload = fmt.Sprintf(jsonPayloadBase, scepURL, adminURL, username, "")
ds.NewActivityFunc = func(ctx context.Context, user *fleet.User, activity fleet.ActivityDetails, details []byte,
createdAt time.Time) error {
createdAt time.Time,
) error {
assert.IsType(t, fleet.ActivityEditedNDESSCEPProxy{}, activity)
return nil
}
@@ -1644,7 +1646,8 @@ func TestModifyAppConfigForNDESSCEPProxy(t *testing.T) {
// Second, real run.
appConfig.Integrations.NDESSCEPProxy.Valid = true
ds.NewActivityFunc = func(ctx context.Context, user *fleet.User, activity fleet.ActivityDetails, details []byte,
createdAt time.Time) error {
createdAt time.Time,
) error {
assert.IsType(t, fleet.ActivityDeletedNDESSCEPProxy{}, activity)
return nil
}
@@ -1682,5 +1685,4 @@ func TestModifyAppConfigForNDESSCEPProxy(t *testing.T) {
ctx = viewer.NewContext(ctx, viewer.Viewer{User: admin})
_, err = svc.ModifyAppConfig(ctx, []byte(jsonPayload), fleet.ApplySpecOptions{})
assert.ErrorContains(t, err, "private key")
}
+209 -9
View File
@@ -2650,15 +2650,20 @@ func (s *integrationMDMTestSuite) TestMDMConfigProfileCRUD() {
// NOTE: label names starting with "-" are sent as "labels_excluding_any"
// (and the leading "-" is removed from the name). Names starting with
// "!" are sent as the deprecated "labels" field (and the "!" is removed).
// Names starting with a "~" prefix are sent as "labels_include_any"
// (and the leading "~" is removed.
addLabelsFields := func(labelNames []string) map[string][]string {
var deprLabels, inclLabels, exclLabels []string
var deprLabels, inclAllLabels, inclAnyLabels, exclLabels []string
for _, lbl := range labelNames {
if strings.HasPrefix(lbl, "-") { //nolint:gocritic // ignore ifElseChain
switch {
case strings.HasPrefix(lbl, "~"):
inclAnyLabels = append(inclAnyLabels, strings.TrimPrefix(lbl, "~"))
case strings.HasPrefix(lbl, "-"):
exclLabels = append(exclLabels, strings.TrimPrefix(lbl, "-"))
} else if strings.HasPrefix(lbl, "!") {
case strings.HasPrefix(lbl, "!"):
deprLabels = append(deprLabels, strings.TrimPrefix(lbl, "!"))
} else {
inclLabels = append(inclLabels, lbl)
default:
inclAllLabels = append(inclAllLabels, lbl)
}
}
@@ -2666,12 +2671,15 @@ func (s *integrationMDMTestSuite) TestMDMConfigProfileCRUD() {
if len(deprLabels) > 0 {
fields["labels"] = deprLabels
}
if len(inclLabels) > 0 {
fields["labels_include_all"] = inclLabels
if len(inclAllLabels) > 0 {
fields["labels_include_all"] = inclAllLabels
}
if len(exclLabels) > 0 {
fields["labels_exclude_any"] = exclLabels
}
if len(inclAnyLabels) > 0 {
fields["labels_include_any"] = inclAnyLabels
}
return fields
}
@@ -2875,15 +2883,20 @@ func (s *integrationMDMTestSuite) TestMDMConfigProfileCRUD() {
// profiles with invalid mix of labels
assertAppleProfile("apple-invalid-profile-with-labels.mobileconfig", "apple-invalid-profile-with-labels", "ident-with-labels", 0, []string{"foo", "!bar"}, http.StatusBadRequest, `Only one of "labels_exclude_any", "labels_include_all", "labels_include_any", or "labels" can be included.`)
assertAppleProfile("apple-invalid-profile-with-labels.mobileconfig", "apple-invalid-profile-with-labels", "ident-with-labels", 0, []string{"foo", "~bar"}, http.StatusBadRequest, `Only one of "labels_exclude_any", "labels_include_all", "labels_include_any", or "labels" can be included.`)
assertAppleDeclaration("apple-invalid-decl-with-labels.json", "ident-decl-with-labels", 0, []string{"foo", "-bar"}, http.StatusBadRequest, `Only one of "labels_exclude_any", "labels_include_all", "labels_include_any", or "labels" can be included.`)
assertAppleDeclaration("apple-invalid-decl-with-labels.json", "ident-decl-with-labels", 0, []string{"foo", "~bar"}, http.StatusBadRequest, `Only one of "labels_exclude_any", "labels_include_all", "labels_include_any", or "labels" can be included.`)
assertWindowsProfile("win-invalid-profile-with-labels.xml", "./Test", 0, []string{"-foo", "!bar"}, http.StatusBadRequest, `Only one of "labels_exclude_any", "labels_include_all", "labels_include_any", or "labels" can be included.`)
assertWindowsProfile("win-invalid-profile-with-labels.xml", "./Test", 0, []string{"-foo", "~bar"}, http.StatusBadRequest, `Only one of "labels_exclude_any", "labels_include_all", "labels_include_any", or "labels" can be included.`)
// profiles with valid labels
uuidAppleWithLabel := assertAppleProfile("apple-profile-with-labels.mobileconfig", "apple-profile-with-labels", "ident-with-labels", 0, []string{"!foo"}, http.StatusOK, "")
uuidAppleWithInclAnyLabel := assertAppleProfile("apple-profile-with-incl-any-labels.mobileconfig", "apple-profile-with-incl-any-labels", "ident-with-incl-any-labels", 0, []string{"~foo", "~bar"}, http.StatusOK, "")
uuidAppleDDMWithLabel := createAppleDeclaration("apple-decl-with-labels", "ident-decl-with-labels", 0, []string{"foo"})
uuidWindowsWithLabel := assertWindowsProfile("win-profile-with-labels.xml", "./Test", 0, []string{"-foo", "-bar"}, http.StatusOK, "")
uuidAppleDDMTeamWithLabel := createAppleDeclaration("apple-team-decl-with-labels", "ident-team-decl-with-labels", testTeam.ID, []string{"-foo"})
uuidWindowsTeamWithLabel := assertWindowsProfile("win-team-profile-with-labels.xml", "./Test", testTeam.ID, []string{"foo", "bar"}, http.StatusOK, "")
uuidWindowsTeamWithInclAnyLabel := assertWindowsProfile("win-team-profile-with-incl-any-labels.xml", "./Test", testTeam.ID, []string{"foo", "bar"}, http.StatusOK, "")
// Windows invalid content
body, headers := generateNewProfileMultipartRequest(t, "win.xml", []byte("\x00\x01\x02"), s.token, nil)
@@ -2923,6 +2936,13 @@ func (s *integrationMDMTestSuite) TestMDMConfigProfileCRUD() {
{LabelID: labelFoo.ID, LabelName: labelFoo.Name},
},
},
{
ProfileUUID: uuidAppleWithInclAnyLabel, Platform: "darwin", Name: "apple-profile-with-incl-any-labels", Identifier: "ident-with-incl-any-labels", TeamID: nil,
LabelsIncludeAny: []fleet.ConfigurationProfileLabel{
{LabelID: labelBar.ID, LabelName: labelBar.Name},
{LabelID: labelFoo.ID, LabelName: labelFoo.Name},
},
},
{
ProfileUUID: uuidWindowsWithLabel, Platform: "windows", Name: "win-profile-with-labels", TeamID: nil,
LabelsExcludeAny: []fleet.ConfigurationProfileLabel{
@@ -2943,6 +2963,13 @@ func (s *integrationMDMTestSuite) TestMDMConfigProfileCRUD() {
{LabelID: labelFoo.ID, LabelName: labelFoo.Name},
},
},
{
ProfileUUID: uuidWindowsTeamWithInclAnyLabel, Platform: "windows", Name: "win-team-profile-with-incl-any-labels", TeamID: &testTeam.ID,
LabelsIncludeAll: []fleet.ConfigurationProfileLabel{
{LabelID: labelBar.ID, LabelName: labelBar.Name},
{LabelID: labelFoo.ID, LabelName: labelFoo.Name},
},
},
}
for _, prof := range expectedProfiles {
var getResp getMDMConfigProfileResponse
@@ -2963,6 +2990,9 @@ func (s *integrationMDMTestSuite) TestMDMConfigProfileCRUD() {
sort.Slice(getResp.LabelsExcludeAny, func(i, j int) bool {
return getResp.LabelsExcludeAny[i].LabelName < getResp.LabelsExcludeAny[j].LabelName
})
sort.Slice(getResp.LabelsIncludeAny, func(i, j int) bool {
return getResp.LabelsIncludeAny[i].LabelName < getResp.LabelsIncludeAny[j].LabelName
})
require.Equal(t, prof, *getResp.MDMConfigProfilePayload)
resp := s.Do("GET", fmt.Sprintf("/api/latest/fleet/configuration_profiles/%s", prof.ProfileUUID), nil, http.StatusOK, "alt", "media")
@@ -3104,6 +3134,8 @@ func (s *integrationMDMTestSuite) TestListMDMConfigProfiles() {
require.NoError(t, err)
lblBar, err := s.ds.NewLabel(ctx, &fleet.Label{Name: "bar", Query: "select 1"})
require.NoError(t, err)
lblBaz, err := s.ds.NewLabel(ctx, &fleet.Label{Name: "baz", Query: "select 1"})
require.NoError(t, err)
// create a couple profiles (Win and mac) for team 2, and none for team 3
tprof, err := fleet.NewMDMAppleConfigProfile(mcBytesForTest("tF", "tF.identifier", "tF.uuid"), nil)
@@ -3131,19 +3163,33 @@ func (s *integrationMDMTestSuite) TestListMDMConfigProfiles() {
},
})
require.NoError(t, err)
// make tm2ProfH a "include-any" label-based profile
tm2ProfH, err := s.ds.NewMDMWindowsConfigProfile(ctx, fleet.MDMWindowsConfigProfile{
Name: "tH",
TeamID: &tm2.ID,
SyncML: []byte(`<Add></Add>`),
LabelsIncludeAny: []fleet.ConfigurationProfileLabel{
{LabelID: lblBar.ID, LabelName: lblBar.Name},
{LabelID: lblBaz.ID, LabelName: lblBaz.Name},
},
})
require.NoError(t, err)
// break lblFoo by deleting it
require.NoError(t, s.ds.DeleteLabel(ctx, lblFoo.Name))
// test that all fields are correctly returned with team 2
var listResp listMDMConfigProfilesResponse
s.DoJSON("GET", "/api/latest/fleet/configuration_profiles", nil, http.StatusOK, &listResp, "team_id", fmt.Sprint(tm2.ID))
require.Len(t, listResp.Profiles, 2)
require.Len(t, listResp.Profiles, 3)
require.NotZero(t, listResp.Profiles[0].CreatedAt)
require.NotZero(t, listResp.Profiles[0].UploadedAt)
require.NotZero(t, listResp.Profiles[1].CreatedAt)
require.NotZero(t, listResp.Profiles[1].UploadedAt)
listResp.Profiles[0].CreatedAt, listResp.Profiles[0].UploadedAt = time.Time{}, time.Time{}
listResp.Profiles[1].CreatedAt, listResp.Profiles[1].UploadedAt = time.Time{}, time.Time{}
listResp.Profiles[2].CreatedAt, listResp.Profiles[2].UploadedAt = time.Time{}, time.Time{}
require.Equal(t, &fleet.MDMConfigProfilePayload{
ProfileUUID: tm2ProfF.ProfileUUID,
TeamID: tm2ProfF.TeamID,
@@ -3168,6 +3214,17 @@ func (s *integrationMDMTestSuite) TestListMDMConfigProfiles() {
{LabelID: 0, LabelName: lblFoo.Name, Broken: true},
},
}, listResp.Profiles[1])
require.Equal(t, &fleet.MDMConfigProfilePayload{
ProfileUUID: tm2ProfH.ProfileUUID,
TeamID: tm2ProfH.TeamID,
Name: tm2ProfH.Name,
Platform: "windows",
// labels are ordered by name
LabelsIncludeAny: []fleet.ConfigurationProfileLabel{
{LabelID: lblBar.ID, LabelName: lblBar.Name},
{LabelID: lblBaz.ID, LabelName: lblBaz.Name},
},
}, listResp.Profiles[2])
// get the specific include-all label-based profile returns the information
var getProfResp getMDMConfigProfileResponse
@@ -3203,6 +3260,21 @@ func (s *integrationMDMTestSuite) TestListMDMConfigProfiles() {
},
}, getProfResp.MDMConfigProfilePayload)
// get the specific include-any label-based profile returns the information
getProfResp = getMDMConfigProfileResponse{}
s.DoJSON("GET", "/api/latest/fleet/mdm/profiles/"+tm2ProfH.ProfileUUID, nil, http.StatusOK, &getProfResp)
getProfResp.CreatedAt, getProfResp.UploadedAt = time.Time{}, time.Time{}
require.Equal(t, &fleet.MDMConfigProfilePayload{
ProfileUUID: tm2ProfH.ProfileUUID,
TeamID: tm2ProfH.TeamID,
Name: tm2ProfH.Name,
Platform: "windows",
// labels are ordered by name
LabelsIncludeAny: []fleet.ConfigurationProfileLabel{
{LabelID: lblBar.ID, LabelName: lblBar.Name},
{LabelID: lblBaz.ID, LabelName: lblBaz.Name},
},
}, getProfResp.MDMConfigProfilePayload)
// list for a non-existing team returns 404
s.DoJSON("GET", "/api/latest/fleet/configuration_profiles", nil, http.StatusNotFound, &listResp, "team_id", "99999")
@@ -3252,7 +3324,7 @@ func (s *integrationMDMTestSuite) TestListMDMConfigProfiles() {
{
queries: []string{"per_page", "3"},
teamID: &tm2.ID,
wantNames: []string{"tF", "tG"},
wantNames: []string{"tF", "tG", "tH"},
wantMeta: &fleet.PaginationMetadata{HasNextResults: false, HasPreviousResults: false},
},
{
@@ -4909,6 +4981,134 @@ func (s *integrationMDMTestSuite) TestHostMDMProfilesExcludeLabels() {
})
}
func (s *integrationMDMTestSuite) TestMDMProfilesIncludeAnyLabels() {
t := s.T()
ctx := context.Background()
triggerReconcileProfiles := func() {
s.awaitTriggerProfileSchedule(t)
// this will only mark them as "pending", as the response to confirm
// profile deployment is asynchronous, so we simulate it here by
// updating any "pending" (not NULL) profiles to "verifying"
mysql.ExecAdhocSQL(t, s.ds, func(q sqlx.ExtContext) error {
if _, err := q.ExecContext(ctx, `UPDATE host_mdm_apple_profiles SET status = ? WHERE status = ?`, fleet.OSSettingsVerifying, fleet.OSSettingsPending); err != nil {
return err
}
if _, err := q.ExecContext(ctx, `UPDATE host_mdm_apple_declarations SET status = ? WHERE status = ?`, fleet.OSSettingsVerifying, fleet.OSSettingsPending); err != nil {
return err
}
if _, err := q.ExecContext(ctx, `UPDATE host_mdm_windows_profiles SET status = ? WHERE status = ?`, fleet.OSSettingsVerifying, fleet.OSSettingsPending); err != nil {
return err
}
return nil
})
}
// run the crons immediately, will create the Fleet-controlled profiles that
// will then be expected to be applied (e.g. com.fleetdm.fleetd.config and
// com.fleetdm.caroot)
// first create the no-team enroll secret (required to create the fleet profiles)
var applyResp applyEnrollSecretSpecResponse
s.DoJSON("POST", "/api/latest/fleet/spec/enroll_secret",
applyEnrollSecretSpecRequest{
Spec: &fleet.EnrollSecretSpec{Secrets: []*fleet.EnrollSecret{{Secret: "super-global-secret"}}},
}, http.StatusOK, &applyResp)
s.awaitTriggerProfileSchedule(t)
// create an Apple and a Windows host
appleHost, _ := createHostThenEnrollMDM(s.ds, s.server.URL, t)
windowsHost, _ := createWindowsHostThenEnrollMDM(s.ds, s.server.URL, t)
// create a few labels, we'll use the first five for "exclude any" profiles and the remaining for "include any"
labels := make([]*fleet.Label, 10)
for i := 0; i < len(labels); i++ {
label, err := s.ds.NewLabel(ctx, &fleet.Label{Name: fmt.Sprintf("label-%d", i), Query: "select 1;"})
require.NoError(t, err)
labels[i] = label
}
// simulate reporting label results for those hosts
appleHost.LabelUpdatedAt = time.Now()
windowsHost.LabelUpdatedAt = time.Now()
err := s.ds.UpdateHost(ctx, appleHost)
require.NoError(t, err)
err = s.ds.UpdateHost(ctx, windowsHost)
require.NoError(t, err)
// set up some Apple profiles and declarations and Windows profiles
s.Do("POST", "/api/v1/fleet/mdm/profiles/batch", batchSetMDMProfilesRequest{Profiles: []fleet.MDMProfileBatchPayload{
{Name: "A1", Contents: mobileconfigForTest("A1", "A1"), LabelsIncludeAny: []string{labels[0].Name, labels[1].Name}},
{Name: "W2", Contents: syncMLForTest("./Foo/W2"), LabelsIncludeAny: []string{labels[2].Name, labels[3].Name}},
{Name: "D3", Contents: declarationForTest("D3"), LabelsIncludeAny: []string{labels[4].Name}},
}}, http.StatusNoContent)
// hosts are not members of any label yet, so running the cron applies no labels
s.awaitTriggerProfileSchedule(t)
s.assertHostAppleConfigProfiles(map[*fleet.Host][]fleet.HostMDMAppleProfile{
appleHost: {
{Identifier: mobileconfig.FleetdConfigPayloadIdentifier, OperationType: fleet.MDMOperationTypeInstall, Status: &fleet.MDMDeliveryPending},
{Identifier: mobileconfig.FleetCARootConfigPayloadIdentifier, OperationType: fleet.MDMOperationTypeInstall, Status: &fleet.MDMDeliveryPending},
},
})
s.assertHostWindowsConfigProfiles(map[*fleet.Host][]fleet.HostMDMWindowsProfile{
windowsHost: {},
})
// make hosts members of labels [1], [2], [3] and [4], meaning that each of the "include any"
// labels will now match at least one host
err = s.ds.AsyncBatchInsertLabelMembership(ctx, [][2]uint{
{labels[0].ID, appleHost.ID},
{labels[1].ID, appleHost.ID},
{labels[2].ID, appleHost.ID},
{labels[3].ID, appleHost.ID},
{labels[4].ID, appleHost.ID},
{labels[1].ID, windowsHost.ID},
{labels[2].ID, windowsHost.ID},
{labels[3].ID, windowsHost.ID},
{labels[4].ID, windowsHost.ID},
})
require.NoError(t, err)
triggerReconcileProfiles()
s.assertHostAppleConfigProfiles(map[*fleet.Host][]fleet.HostMDMAppleProfile{
appleHost: {
{Identifier: "A1", OperationType: fleet.MDMOperationTypeInstall, Status: &fleet.MDMDeliveryVerifying},
{Identifier: "D3", OperationType: fleet.MDMOperationTypeInstall, Status: &fleet.MDMDeliveryVerifying},
{Identifier: mobileconfig.FleetdConfigPayloadIdentifier, OperationType: fleet.MDMOperationTypeInstall, Status: &fleet.MDMDeliveryVerifying},
{Identifier: mobileconfig.FleetCARootConfigPayloadIdentifier, OperationType: fleet.MDMOperationTypeInstall, Status: &fleet.MDMDeliveryVerifying},
},
})
s.assertHostWindowsConfigProfiles(map[*fleet.Host][]fleet.HostMDMWindowsProfile{
windowsHost: {
{Name: "W2", OperationType: fleet.MDMOperationTypeInstall, Status: &fleet.MDMDeliveryVerifying},
},
})
// remove membership of labels [2] for Windows, and [1] and [4] for Apple, meaning
// that D3 will be removed on Apple, A1 will remain on Apple because the host is still a member
// of [0], and W2 will remain on Windows because the host is still a member of [3]
err = s.ds.AsyncBatchDeleteLabelMembership(ctx, [][2]uint{
{labels[1].ID, appleHost.ID},
{labels[4].ID, appleHost.ID},
{labels[2].ID, windowsHost.ID},
})
require.NoError(t, err)
s.awaitTriggerProfileSchedule(t)
s.assertHostAppleConfigProfiles(map[*fleet.Host][]fleet.HostMDMAppleProfile{
appleHost: {
{Identifier: "A1", OperationType: fleet.MDMOperationTypeInstall, Status: &fleet.MDMDeliveryVerifying},
{Identifier: "D3", OperationType: fleet.MDMOperationTypeRemove, Status: &fleet.MDMDeliveryPending},
{Identifier: mobileconfig.FleetdConfigPayloadIdentifier, OperationType: fleet.MDMOperationTypeInstall, Status: &fleet.MDMDeliveryVerifying},
{Identifier: mobileconfig.FleetCARootConfigPayloadIdentifier, OperationType: fleet.MDMOperationTypeInstall, Status: &fleet.MDMDeliveryVerifying},
},
})
s.assertHostWindowsConfigProfiles(map[*fleet.Host][]fleet.HostMDMWindowsProfile{
windowsHost: {
{Name: "W2", OperationType: fleet.MDMOperationTypeInstall, Status: &fleet.MDMDeliveryVerifying},
},
})
}
func (s *integrationMDMTestSuite) TestOTAProfile() {
t := s.T()
ctx := context.Background()
+4 -5
View File
@@ -1304,7 +1304,7 @@ func newMDMConfigProfileEndpoint(ctx context.Context, request interface{}, svc f
labels = req.LabelsExcludeAny
labelsMode = fleet.LabelsExcludeAny
default:
// TODO: should this be the default?
// default include all
labels = req.LabelsIncludeAll
labelsMode = fleet.LabelsIncludeAll
}
@@ -1407,14 +1407,13 @@ func (svc *Service) NewMDMWindowsConfigProfile(ctx context.Context, teamID uint,
return nil, ctxerr.Wrap(ctx, err, "validating labels")
}
switch labelsMembershipMode {
case fleet.LabelsIncludeAll:
cp.LabelsIncludeAll = labelMap
case fleet.LabelsIncludeAny:
cp.LabelsIncludeAny = labelMap
case fleet.LabelsExcludeAny:
cp.LabelsExcludeAny = labelMap
default:
// TODO what happens if mode is not set?s
// default include all
cp.LabelsIncludeAll = labelMap
}
err = validateWindowsProfileFleetVariables(string(cp.SyncML))
@@ -1593,7 +1592,7 @@ func (svc *Service) BatchSetMDMProfiles(
labels := []string{}
for i := range profiles {
// from this point on (after this condition), only LabelsIncludeAll or
// from this point on (after this condition), only LabelsIncludeAll, LabelsIncludeAny or
// LabelsExcludeAny need to be checked.
if len(profiles[i].Labels) > 0 {
// must update the struct in the slice directly, because we don't have a