update error message for same name profile (#25673)

For #17700

improve error message when a custom profile is edited or uploaded with
one that has the same name.

updates the error message for `POST /fleet/mdm/profiles` and in fleetctl
when using gitops


- [x] Changes file added for user-visible changes in `changes/`,
`orbit/changes/` or `ee/fleetd-chrome/changes`.
See [Changes
files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/Committing-Changes.md#changes-files)
for more information.
- [x] Added/updated automated tests
- [x] Manual QA for all new/changed functionality
This commit is contained in:
Gabriel Hernandez 2025-01-30 11:17:36 +00:00 committed by GitHub
parent e55c664b13
commit 8bcdb82d55
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 52 additions and 75 deletions

View file

@ -63,6 +63,10 @@ const (
FleetVarHostEndUserEmailIDP = "HOST_END_USER_EMAIL_IDP"
)
const (
SameProfileNameUploadErrorMsg = "Couldn't add. A configuration profile with this name already exists (PayloadDisplayName for .mobileconfig and file name for .json and .xml)."
)
var (
profileVariableRegex = regexp.MustCompile(`(\$FLEET_VAR_(?P<name1>\w+))|(\${FLEET_VAR_(?P<name2>\w+)})`)
fleetVarNDESSCEPChallengeRegexp = regexp.MustCompile(fmt.Sprintf(`(\$FLEET_VAR_%s)|(\${FLEET_VAR_%s})`, FleetVarNDESSCEPChallenge,
@ -424,7 +428,7 @@ func (svc *Service) NewMDMAppleConfigProfile(ctx context.Context, teamID uint, r
if err != nil {
var existsErr existsErrorInterface
if errors.As(err, &existsErr) {
msg := "Couldn't upload. A configuration profile with this name already exists."
msg := SameProfileNameUploadErrorMsg
if re, ok := existsErr.(interface{ Resource() string }); ok {
if re.Resource() == "MDMAppleConfigProfile.PayloadIdentifier" {
msg = "Couldn't upload. A configuration profile with this identifier (PayloadIdentifier) already exists."
@ -2015,14 +2019,14 @@ func (svc *Service) BatchSetMDMAppleProfiles(ctx context.Context, tmID *uint, tm
if byName[mdmProf.Name] {
return ctxerr.Wrap(ctx,
fleet.NewInvalidArgumentError(fmt.Sprintf("profiles[%d]", i), fmt.Sprintf("Couldnt edit custom_settings. More than one configuration profile have the same name (PayloadDisplayName): %q", mdmProf.Name)),
fleet.NewInvalidArgumentError(fmt.Sprintf("profiles[%d]", i), fmt.Sprintf("Couldn't edit custom_settings. More than one configuration profile have the same name (PayloadDisplayName): %q", mdmProf.Name)),
"duplicate mobileconfig profile by name")
}
byName[mdmProf.Name] = true
if byIdent[mdmProf.Identifier] {
return ctxerr.Wrap(ctx,
fleet.NewInvalidArgumentError(fmt.Sprintf("profiles[%d]", i), fmt.Sprintf("Couldnt edit custom_settings. More than one configuration profile have the same identifier (PayloadIdentifier): %q", mdmProf.Identifier)),
fleet.NewInvalidArgumentError(fmt.Sprintf("profiles[%d]", i), fmt.Sprintf("Couldn't edit custom_settings. More than one configuration profile have the same identifier (PayloadIdentifier): %q", mdmProf.Identifier)),
"duplicate mobileconfig profile by identifier")
}
byIdent[mdmProf.Identifier] = true
@ -2046,11 +2050,11 @@ func (svc *Service) BatchSetMDMAppleProfiles(ctx context.Context, tmID *uint, tm
continue
case strings.HasPrefix(p.ProfileUUID, "w"):
err := fleet.NewInvalidArgumentError("PayloadDisplayName", fmt.Sprintf(
"Couldnt edit custom_settings. A Windows configuration profile shares the same name as a macOS configuration profile (PayloadDisplayName): %q", p.Name))
"Couldn't edit custom_settings. A Windows configuration profile shares the same name as a macOS configuration profile (PayloadDisplayName): %q", p.Name))
return ctxerr.Wrap(ctx, err, "duplicate xml and mobileconfig by name")
default:
err := fleet.NewInvalidArgumentError("PayloadDisplayName", fmt.Sprintf(
"Couldnt edit custom_settings. More than one configuration profile have the same name (PayloadDisplayName): %q", p.Name))
"Couldn't edit custom_settings. More than one configuration profile have the same name (PayloadDisplayName): %q", p.Name))
return ctxerr.Wrap(ctx, err, "duplicate json and mobileconfig by name")
}
}

View file

@ -3713,7 +3713,8 @@ func TestRenewSCEPCertificatesBranches(t *testing.T) {
}
appleStore.EnqueueCommandFunc = func(ctx context.Context, id []string, cmd *mdm.CommandWithSubtype) (map[string]error,
error) {
error,
) {
require.Equal(t, "InstallProfile", cmd.Command.Command.RequestType)
wantCommandUUID = cmd.CommandUUID
return map[string]error{}, nil
@ -3740,7 +3741,8 @@ func TestRenewSCEPCertificatesBranches(t *testing.T) {
}
appleStore.EnqueueCommandFunc = func(ctx context.Context, id []string, cmd *mdm.CommandWithSubtype) (map[string]error,
error) {
error,
) {
return map[string]error{}, errors.New("foo")
}
},
@ -3754,7 +3756,8 @@ func TestRenewSCEPCertificatesBranches(t *testing.T) {
return []fleet.SCEPIdentityAssociation{{HostUUID: "hostUUID2", EnrollReference: "ref1"}}, nil
}
appleStore.EnqueueCommandFunc = func(ctx context.Context, id []string, cmd *mdm.CommandWithSubtype) (map[string]error,
error) {
error,
) {
require.Equal(t, "InstallProfile", cmd.Command.Command.RequestType)
wantCommandUUID = cmd.CommandUUID
return map[string]error{}, nil
@ -3780,7 +3783,8 @@ func TestRenewSCEPCertificatesBranches(t *testing.T) {
}
appleStore.EnqueueCommandFunc = func(ctx context.Context, id []string, cmd *mdm.CommandWithSubtype) (map[string]error,
error) {
error,
) {
return map[string]error{}, errors.New("foo")
}
},

View file

@ -372,8 +372,8 @@ func getProfilesContents(baseDir string, macProfiles []fleet.MDMProfileSpec, win
}
// check for duplicate names across all profiles
if e, isDuplicate := extByName[name]; isDuplicate {
return nil, errors.New(fmtDuplicateNameErrMsg(name, e, ext))
if _, isDuplicate := extByName[name]; isDuplicate {
return nil, errors.New(fmtDuplicateNameErrMsg(name))
}
extByName[name] = ext

View file

@ -83,7 +83,7 @@ func (s *integrationMDMTestSuite) TestAppleProfileManagement() {
s.Do("POST", "/api/v1/fleet/mdm/apple/profiles/batch", batchSetMDMAppleProfilesRequest{Profiles: globalProfiles}, http.StatusNoContent)
// invalid secrets
var invalidSecretsProfile = []byte(`
invalidSecretsProfile := []byte(`
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
@ -1351,8 +1351,10 @@ func (s *integrationMDMTestSuite) TestPuppetMatchPreassignProfiles() {
{Identifier: mobileconfig.FleetCARootConfigPayloadIdentifier, OperationType: fleet.MDMOperationTypeInstall, Status: &fleet.MDMDeliveryPending},
// Profiles from previous team being deleted
{Identifier: "i2", OperationType: fleet.MDMOperationTypeRemove, Status: &fleet.MDMDeliveryPending},
{Identifier: mobileconfig.FleetFileVaultPayloadIdentifier, OperationType: fleet.MDMOperationTypeRemove,
Status: &fleet.MDMDeliveryPending},
{
Identifier: mobileconfig.FleetFileVaultPayloadIdentifier, OperationType: fleet.MDMOperationTypeRemove,
Status: &fleet.MDMDeliveryPending,
},
},
})
@ -2994,19 +2996,19 @@ func (s *integrationMDMTestSuite) TestMDMConfigProfileCRUD() {
teamWinProfUUID := createWindowsProfile("win-team-profile", testTeam.ID, nil)
// Windows profile name conflicts with Apple's for no team
assertWindowsProfile("apple-global-profile.xml", "./Test", 0, nil, http.StatusConflict, "Couldn't upload. A configuration profile with this name already exists.")
assertWindowsProfile("apple-global-profile.xml", "./Test", 0, nil, http.StatusConflict, SameProfileNameUploadErrorMsg)
// but no conflict for team 1
assertWindowsProfile("apple-global-profile.xml", "./Test", testTeam.ID, nil, http.StatusOK, "")
// Apple profile name conflicts with Windows' for no team
assertAppleProfile("win-global-profile.mobileconfig", "win-global-profile", "test-global-ident-2", 0, nil, http.StatusConflict, "Couldn't upload. A configuration profile with this name already exists.")
assertAppleProfile("win-global-profile.mobileconfig", "win-global-profile", "test-global-ident-2", 0, nil, http.StatusConflict, SameProfileNameUploadErrorMsg)
// but no conflict for team 1
assertAppleProfile("win-global-profile.mobileconfig", "win-global-profile", "test-global-ident-2", testTeam.ID, nil, http.StatusOK, "")
// Windows profile name conflicts with Apple's for team 1
assertWindowsProfile("apple-team-profile.xml", "./Test", testTeam.ID, nil, http.StatusConflict, "Couldn't upload. A configuration profile with this name already exists.")
assertWindowsProfile("apple-team-profile.xml", "./Test", testTeam.ID, nil, http.StatusConflict, SameProfileNameUploadErrorMsg)
// but no conflict for no-team
assertWindowsProfile("apple-team-profile.xml", "./Test", 0, nil, http.StatusOK, "")
// Apple profile name conflicts with Windows' for team 1
assertAppleProfile("win-team-profile.mobileconfig", "win-team-profile", "test-team-ident-2", testTeam.ID, nil, http.StatusConflict, "Couldn't upload. A configuration profile with this name already exists.")
assertAppleProfile("win-team-profile.mobileconfig", "win-team-profile", "test-team-ident-2", testTeam.ID, nil, http.StatusConflict, SameProfileNameUploadErrorMsg)
// but no conflict for no-team
assertAppleProfile("win-team-profile.mobileconfig", "win-team-profile", "test-team-ident-2", 0, nil, http.StatusOK, "")
@ -3023,9 +3025,9 @@ func (s *integrationMDMTestSuite) TestMDMConfigProfileCRUD() {
// name is pulled from filename, it conflicts with existing macOS config profile
assertAppleDeclaration("win-global-profile.json", "test-declaration-ident-2", 0, nil, http.StatusConflict, "win-global-profile already exists")
// windows profile name conflicts with existing declaration
assertWindowsProfile("apple-declaration.xml", "./Test", 0, nil, http.StatusConflict, "Couldn't upload. A configuration profile with this name already exists.")
assertWindowsProfile("apple-declaration.xml", "./Test", 0, nil, http.StatusConflict, SameProfileNameUploadErrorMsg)
// macOS profile name conflicts with existing declaration
assertAppleProfile("apple-declaration.mobileconfig", "apple-declaration", "test-declaration-ident", 0, nil, http.StatusConflict, "Couldn't upload. A configuration profile with this name already exists.")
assertAppleProfile("apple-declaration.mobileconfig", "apple-declaration", "test-declaration-ident", 0, nil, http.StatusConflict, SameProfileNameUploadErrorMsg)
// not an xml nor mobileconfig file
assertWindowsProfile("foo.txt", "./Test", 0, nil, http.StatusBadRequest, "Couldn't add profile. The file should be a .mobileconfig, XML, or JSON file.")
@ -4330,15 +4332,15 @@ func (s *integrationMDMTestSuite) TestBatchSetMDMProfiles() {
}{
{
payload: []fleet.MDMProfileBatchPayload{{Name: "N1", Contents: mcBytes}, {Name: "N1", Contents: winBytes}},
expectErr: "More than one configuration profile have the same name 'N1' (Windows .xml file name or macOS .mobileconfig PayloadDisplayName).",
expectErr: "More than one configuration profile have the same name 'N1'",
},
{
payload: []fleet.MDMProfileBatchPayload{{Name: "N1", Contents: declBytes}, {Name: "N1", Contents: winBytes}},
expectErr: "More than one configuration profile have the same name 'N1' (macOS .json file name or Windows .xml file name).",
expectErr: "More than one configuration profile have the same name 'N1'",
},
{
payload: []fleet.MDMProfileBatchPayload{{Name: "N1", Contents: mcBytes}, {Name: "N1", Contents: declBytes}},
expectErr: "More than one configuration profile have the same name 'N1' (macOS .json file name or macOS .mobileconfig PayloadDisplayName).",
expectErr: "More than one configuration profile have the same name 'N1'",
},
} {
// team profiles
@ -5382,7 +5384,8 @@ func (s *integrationMDMTestSuite) TestAppleDDMSecretVariablesUpload() {
}
func (s *integrationMDMTestSuite) testSecretVariablesUpload(newProfileBytes func(i int) []byte,
getProfileContents func(profileUUID string) string, fileExtension string, platform string) {
getProfileContents func(profileUUID string) string, fileExtension string, platform string,
) {
t := s.T()
const numProfiles = 2
var profiles [][]byte
@ -5459,7 +5462,6 @@ func (s *integrationMDMTestSuite) testSecretVariablesUpload(newProfileBytes func
}
s.DoJSON("GET", "/api/latest/fleet/mdm/profiles", &listMDMConfigProfilesRequest{}, http.StatusOK, &listResp)
require.Empty(t, listResp.Profiles)
}
// TestAppleConfigSecretVariablesUpload tests uploading Apple config profiles with secrets via the /configuration_profiles endpoint
@ -5513,7 +5515,6 @@ func (s *integrationMDMTestSuite) TestAppleConfigSecretVariablesUpload() {
}
s.testSecretVariablesUpload(newProfileBytes, getProfileContents, "mobileconfig", "darwin")
}
// TestWindowsConfigSecretVariablesUpload tests uploading Windows profiles with secrets via the /configuration_profiles endpoint
@ -5543,7 +5544,6 @@ func (s *integrationMDMTestSuite) TestWindowsConfigSecretVariablesUpload() {
}
s.testSecretVariablesUpload(newProfileBytes, getProfileContents, "xml", "windows")
}
func (s *integrationMDMTestSuite) TestAppleProfileDeletion() {
@ -5698,5 +5698,4 @@ func (s *integrationMDMTestSuite) TestAppleProfileDeletion() {
profiles, err = s.ds.GetHostMDMAppleProfiles(ctx, host2.UUID)
require.NoError(t, err)
assert.Len(t, profiles, 3)
}

View file

@ -1435,7 +1435,7 @@ func (svc *Service) NewMDMWindowsConfigProfile(ctx context.Context, teamID uint,
if err != nil {
var existsErr existsErrorInterface
if errors.As(err, &existsErr) {
err = fleet.NewInvalidArgumentError("profile", "Couldn't upload. A configuration profile with this name already exists.").
err = fleet.NewInvalidArgumentError("profile", SameProfileNameUploadErrorMsg).
WithStatus(http.StatusConflict)
}
return nil, ctxerr.Wrap(ctx, err)
@ -1776,27 +1776,28 @@ func validateFleetVariables(ctx context.Context, appleProfiles map[int]*fleet.MD
}
func (svc *Service) validateCrossPlatformProfileNames(ctx context.Context, appleProfiles map[int]*fleet.MDMAppleConfigProfile,
windowsProfiles map[int]*fleet.MDMWindowsConfigProfile, appleDecls map[int]*fleet.MDMAppleDeclaration) error {
windowsProfiles map[int]*fleet.MDMWindowsConfigProfile, appleDecls map[int]*fleet.MDMAppleDeclaration,
) error {
// map all profile names to check for duplicates, regardless of platform; key is name, value is one of
// ".mobileconfig" or ".json" or ".xml"
extByName := make(map[string]string, len(appleProfiles)+len(windowsProfiles)+len(appleDecls))
for i, p := range appleProfiles {
if v, ok := extByName[p.Name]; ok {
err := fleet.NewInvalidArgumentError(fmt.Sprintf("appleProfiles[%d]", i), fmtDuplicateNameErrMsg(p.Name, ".mobileconfig", v))
if _, ok := extByName[p.Name]; ok {
err := fleet.NewInvalidArgumentError(fmt.Sprintf("appleProfiles[%d]", i), fmtDuplicateNameErrMsg(p.Name))
return ctxerr.Wrap(ctx, err, "duplicate mobileconfig profile by name")
}
extByName[p.Name] = ".mobileconfig"
}
for i, p := range windowsProfiles {
if v, ok := extByName[p.Name]; ok {
err := fleet.NewInvalidArgumentError(fmt.Sprintf("windowsProfiles[%d]", i), fmtDuplicateNameErrMsg(p.Name, ".xml", v))
if _, ok := extByName[p.Name]; ok {
err := fleet.NewInvalidArgumentError(fmt.Sprintf("windowsProfiles[%d]", i), fmtDuplicateNameErrMsg(p.Name))
return ctxerr.Wrap(ctx, err, "duplicate xml by name")
}
extByName[p.Name] = ".xml"
}
for i, p := range appleDecls {
if v, ok := extByName[p.Name]; ok {
err := fleet.NewInvalidArgumentError(fmt.Sprintf("appleDecls[%d]", i), fmtDuplicateNameErrMsg(p.Name, ".json", v))
if _, ok := extByName[p.Name]; ok {
err := fleet.NewInvalidArgumentError(fmt.Sprintf("appleDecls[%d]", i), fmtDuplicateNameErrMsg(p.Name))
return ctxerr.Wrap(ctx, err, "duplicate json by name")
}
extByName[p.Name] = ".json"
@ -1805,39 +1806,9 @@ func (svc *Service) validateCrossPlatformProfileNames(ctx context.Context, apple
return nil
}
func fmtDuplicateNameErrMsg(name, fileType1, fileType2 string) string {
var part1 string
switch fileType1 {
case ".xml":
part1 = "Windows .xml file name"
case ".mobileconfig":
part1 = "macOS .mobileconfig PayloadDisplayName"
case ".json":
part1 = "macOS .json file name"
}
var part2 string
switch fileType2 {
case ".xml":
part2 = "Windows .xml file name"
case ".mobileconfig":
part2 = "macOS .mobileconfig PayloadDisplayName"
case ".json":
part2 = "macOS .json file name"
}
base := fmt.Sprintf(`Couldnt edit custom_settings. More than one configuration profile have the same name '%s'`, name)
detail := ` (%s).`
switch {
case part1 == part2:
return fmt.Sprintf(base+detail, part1)
case part1 != "" && part2 != "":
return fmt.Sprintf(base+detail, fmt.Sprintf("%s or %s", part1, part2))
case part1 != "" || part2 != "":
return fmt.Sprintf(base+detail, part1+part2)
default:
return base + "." // should never happen
}
func fmtDuplicateNameErrMsg(name string) string {
const SameProfileNameErrorMsg = "Couldn't edit custom_settings. More than one configuration profile have the same name '%s' (PayloadDisplayName for .mobileconfig and file name for .json and .xml)."
return fmt.Sprintf(SameProfileNameErrorMsg, name)
}
func (svc *Service) authorizeBatchProfiles(ctx context.Context, tmID *uint, tmName *string) (*uint, *string, error) {
@ -2010,14 +1981,13 @@ func getAppleProfiles(
if mdmProf.Name != prof.Name {
return nil, nil, ctxerr.Wrap(ctx,
fleet.NewInvalidArgumentError(prof.Name, fmt.Sprintf("Couldnt edit custom_settings. The name provided for the profile must match the profile PayloadDisplayName: %q", mdmProf.Name)),
fleet.NewInvalidArgumentError(prof.Name, fmt.Sprintf("Couldn't edit custom_settings. The name provided for the profile must match the profile PayloadDisplayName: %q", mdmProf.Name)),
"duplicate mobileconfig profile by name")
}
// TODO: confirm error messages
if _, ok := byName[mdmProf.Name]; ok {
return nil, nil, ctxerr.Wrap(ctx,
fleet.NewInvalidArgumentError(prof.Name, fmt.Sprintf("Couldnt edit custom_settings. More than one configuration profile have the same name (PayloadDisplayName): %q", mdmProf.Name)),
fleet.NewInvalidArgumentError(prof.Name, fmt.Sprintf("Couldn't edit custom_settings. More than one configuration profile have the same name (PayloadDisplayName): %q", mdmProf.Name)),
"duplicate mobileconfig profile by name")
}
byName[mdmProf.Name] = "mobileconfig"
@ -2025,7 +1995,7 @@ func getAppleProfiles(
// TODO: confirm error messages
if _, ok := byIdent[mdmProf.Identifier]; ok {
return nil, nil, ctxerr.Wrap(ctx,
fleet.NewInvalidArgumentError(prof.Name, fmt.Sprintf("Couldnt edit custom_settings. More than one configuration profile have the same identifier (PayloadIdentifier): %q", mdmProf.Identifier)),
fleet.NewInvalidArgumentError(prof.Name, fmt.Sprintf("Couldn't edit custom_settings. More than one configuration profile have the same identifier (PayloadIdentifier): %q", mdmProf.Identifier)),
"duplicate mobileconfig profile by identifier")
}
byIdent[mdmProf.Identifier] = "mobileconfig"

View file

@ -1207,7 +1207,7 @@ func TestUploadWindowsMDMConfigProfileValidations(t *testing.T) {
{"random non-xml data", 0, "\x00\x01\x02", true, "The file should include valid XML:"},
{"valid windows profile", 0, `<Replace></Replace>`, true, ""},
{"mdm not enabled", 0, `<Replace></Replace>`, false, "Windows MDM isn't turned on."},
{"duplicate profile name", 0, `<Replace>duplicate</Replace>`, true, "configuration profile with this name already exists."},
{"duplicate profile name", 0, `<Replace>duplicate</Replace>`, true, "configuration profile with this name already exists"},
{"multiple Replace", 0, `<Replace>a</Replace><Replace>b</Replace>`, true, ""},
{"Replace and non-Replace", 0, `<Replace>a</Replace><Get>b</Get>`, true, "Windows configuration profiles can only have <Replace> or <Add> top level elements."},
{"BitLocker profile", 0, `<Replace><Item><Target><LocURI>./Device/Vendor/MSFT/BitLocker/AllowStandardUserEncryption</LocURI></Target></Item></Replace>`, true, "Custom configuration profiles can't include BitLocker settings."},
@ -1219,7 +1219,7 @@ func TestUploadWindowsMDMConfigProfileValidations(t *testing.T) {
{"team random non-xml data", 1, "\x00\x01\x02", true, "The file should include valid XML:"},
{"team valid windows profile", 1, `<Replace></Replace>`, true, ""},
{"team mdm not enabled", 1, `<Replace></Replace>`, false, "Windows MDM isn't turned on."},
{"team duplicate profile name", 1, `<Replace>duplicate</Replace>`, true, "configuration profile with this name already exists."},
{"team duplicate profile name", 1, `<Replace>duplicate</Replace>`, true, "configuration profile with this name already exists"},
{"team multiple Replace", 1, `<Replace>a</Replace><Replace>b</Replace>`, true, ""},
{"team Replace and non-Replace", 1, `<Replace>a</Replace><Get>b</Get>`, true, "Windows configuration profiles can only have <Replace> or <Add> top level elements."},
{"team BitLocker profile", 1, `<Replace><Item><Target><LocURI>./Device/Vendor/MSFT/BitLocker/AllowStandardUserEncryption</LocURI></Target></Item></Replace>`, true, "Custom configuration profiles can't include BitLocker settings."},