improve validation error messages for custom activations (#50753)
<!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> **Related issue:** Resolves nothing, just something I caught while doing test plan and PR review. # Checklist for submitter If some of the following don't apply, delete the relevant line. - [ ] 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/guides/committing-changes.md#changes-files) for more information. part of a bigger story. - [x] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters. - [x] Timeouts are implemented and retries are limited to avoid infinite loops - [x] If paths of existing endpoints are modified without backwards compatibility, checked the frontend/CLI for any necessary changes ## Testing - [x] Added/updated automated tests - [x] QA'd all new/changed functionality manually <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved validation messages for custom activations. * Clarified that standard configurations must reference exactly one configuration profile. * Added a documentation link for the single-profile reference requirement. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -1129,14 +1129,14 @@ func (r *MDMAppleRawActivation) ValidateUserProvided(configurationIdentifier str
|
||||
invalid := &InvalidArgumentError{}
|
||||
|
||||
if strings.TrimSpace(r.Type) == "" {
|
||||
invalid.Append("Type", "Activation must include a Type.")
|
||||
invalid.Append("Type", "The custom activation must include a Type.")
|
||||
} else if !strings.HasPrefix(r.Type, MDMAppleActivationTypePrefix) {
|
||||
invalid.Append("Type", fmt.Sprintf("Only activation declarations (%s) are supported.", MDMAppleActivationTypePrefix))
|
||||
}
|
||||
|
||||
switch {
|
||||
case strings.TrimSpace(r.Identifier) == "":
|
||||
invalid.Append("Identifier", "Activation must include an Identifier.")
|
||||
invalid.Append("Identifier", "The custom activation must include an Identifier.")
|
||||
case len(r.Identifier) > MDMAppleDeclarationIdentifierMaxLen:
|
||||
invalid.Append("Identifier", fmt.Sprintf(
|
||||
"Identifier must be %d bytes or fewer.", MDMAppleDeclarationIdentifierMaxLen))
|
||||
@@ -1144,12 +1144,12 @@ func (r *MDMAppleRawActivation) ValidateUserProvided(configurationIdentifier str
|
||||
|
||||
switch configs := r.Payload.StandardConfigurations; {
|
||||
case len(configs) == 0:
|
||||
invalid.Append("StandardConfigurations", "Activation must reference the configuration profile it's uploaded with.")
|
||||
invalid.Append("StandardConfigurations", "The custom activation must reference the identifier of the configuration profile used to upload it.")
|
||||
case len(configs) > 1:
|
||||
invalid.Append("StandardConfigurations", "Activation can only reference one configuration profile.")
|
||||
invalid.Append("StandardConfigurations", "The custom activation can only have one referenced configuration profile. Learn more: https://fleetdm.com/learn-more-about/ddm-activations")
|
||||
case configs[0] != configurationIdentifier:
|
||||
invalid.Append("StandardConfigurations", fmt.Sprintf(
|
||||
"Activation must reference the configuration profile it's uploaded with. Expected %q, got %q.",
|
||||
"The custom activation must reference the identifier of the configuration profile used to upload it. Expected %q, got %q.",
|
||||
configurationIdentifier, configs[0]))
|
||||
}
|
||||
|
||||
|
||||
@@ -1235,13 +1235,13 @@ func TestMDMAppleRawActivationValidateUserProvided(t *testing.T) {
|
||||
name: "missing type",
|
||||
activation: rawActivation("", "com.fleet.act.passcode", configIdentifier),
|
||||
wantErr: true,
|
||||
errContains: "Activation must include a Type.",
|
||||
errContains: "The custom activation must include a Type.",
|
||||
},
|
||||
{
|
||||
name: "whitespace type",
|
||||
activation: rawActivation(" ", "com.fleet.act.passcode", configIdentifier),
|
||||
wantErr: true,
|
||||
errContains: "Activation must include a Type.",
|
||||
errContains: "The custom activation must include a Type.",
|
||||
},
|
||||
{
|
||||
name: "configuration type is not an activation",
|
||||
@@ -1254,7 +1254,7 @@ func TestMDMAppleRawActivationValidateUserProvided(t *testing.T) {
|
||||
name: "missing identifier",
|
||||
activation: rawActivation("com.apple.activation.simple", " ", configIdentifier),
|
||||
wantErr: true,
|
||||
errContains: "Activation must include an Identifier.",
|
||||
errContains: "The custom activation must include an Identifier.",
|
||||
},
|
||||
{
|
||||
name: "identifier over Apple's 64 octet limit",
|
||||
@@ -1266,14 +1266,14 @@ func TestMDMAppleRawActivationValidateUserProvided(t *testing.T) {
|
||||
name: "no configurations referenced",
|
||||
activation: rawActivation("com.apple.activation.simple", "com.fleet.act.passcode"),
|
||||
wantErr: true,
|
||||
errContains: "Activation must reference the configuration profile it's uploaded with.",
|
||||
errContains: "The custom activation must reference the identifier of the configuration profile used to upload it.",
|
||||
},
|
||||
{
|
||||
name: "more than one configuration referenced",
|
||||
activation: rawActivation("com.apple.activation.simple", "com.fleet.act.passcode",
|
||||
configIdentifier, "com.fleet.cfg.firewall"),
|
||||
wantErr: true,
|
||||
errContains: "Activation can only reference one configuration profile.",
|
||||
errContains: "The custom activation can only have one referenced configuration profile.",
|
||||
},
|
||||
{
|
||||
name: "references a different configuration",
|
||||
|
||||
@@ -9993,7 +9993,7 @@ func TestNewMDMAppleDeclarationWithActivation(t *testing.T) {
|
||||
activation := activationBytesForTest("com.fleet.actD1", "com.fleet.configOther")
|
||||
|
||||
_, err := svc.NewMDMAppleDeclaration(ctx, 0, decl, nil, "name", fleet.LabelsIncludeAll, nil, activation)
|
||||
require.ErrorContains(t, err, "Activation must reference the configuration profile it's uploaded with")
|
||||
require.ErrorContains(t, err, "The custom activation must reference the identifier of the configuration profile used to upload it")
|
||||
})
|
||||
|
||||
t.Run("malformed activation is rejected", func(t *testing.T) {
|
||||
|
||||
Reference in New Issue
Block a user