From ab95a0f107191de4ad3c5247de09ae050b4241ba Mon Sep 17 00:00:00 2001
From: Victor Lyuboslavsky <victor.lyuboslavsky@gmail.com>
Date: Fri, 20 Dec 2024 17:34:43 -0600
Subject: [PATCH] Fix issue deleting DDM profiles with secret variables.
 (#24978)

#24548
Fix issue deleting DDM profiles with secret variables.

# Checklist for submitter
- [x] Added/updated tests
- [x] Manual QA for all new/changed functionality
---
 server/service/apple_mdm.go                | 33 +++++++++++++---------
 server/service/integration_mdm_ddm_test.go | 11 ++++++++
 2 files changed, 30 insertions(+), 14 deletions(-)

diff --git a/server/service/apple_mdm.go b/server/service/apple_mdm.go
index e636b7264c..ddb9e94d97 100644
--- a/server/service/apple_mdm.go
+++ b/server/service/apple_mdm.go
@@ -903,22 +903,27 @@ func (svc *Service) DeleteMDMAppleDeclaration(ctx context.Context, declUUID stri
 		return ctxerr.Wrap(ctx, err)
 	}
 
-	if _, ok := mdm_types.FleetReservedProfileNames()[decl.Name]; ok {
-		return &fleet.BadRequestError{
-			Message:     "profiles managed by Fleet can't be deleted using this endpoint.",
-			InternalErr: fmt.Errorf("deleting profile %s is not allowed because it's managed by Fleet", decl.Name),
+	// Check if the declaration contains a secret variable. If it does, this means that the declaration
+	// has been provided by the user and can be deleted. We don't need to validate that it is a Fleet declaration.
+	hasSecretVariable := len(fleet.ContainsPrefixVars(string(decl.RawJSON), fleet.ServerSecretPrefix)) > 0
+	if !hasSecretVariable {
+		if _, ok := mdm_types.FleetReservedProfileNames()[decl.Name]; ok {
+			return &fleet.BadRequestError{
+				Message:     "profiles managed by Fleet can't be deleted using this endpoint.",
+				InternalErr: fmt.Errorf("deleting profile %s is not allowed because it's managed by Fleet", decl.Name),
+			}
 		}
-	}
 
-	// TODO: refine our approach to deleting restricted/forbidden types of declarations so that we
-	// can check that Fleet-managed aren't being deleted; this can be addressed once we add support
-	// for more types of declarations
-	var d fleet.MDMAppleRawDeclaration
-	if err := json.Unmarshal(decl.RawJSON, &d); err != nil {
-		return ctxerr.Wrap(ctx, err, "unmarshalling declaration")
-	}
-	if err := d.ValidateUserProvided(); err != nil {
-		return ctxerr.Wrap(ctx, &fleet.BadRequestError{Message: err.Error()})
+		// TODO: refine our approach to deleting restricted/forbidden types of declarations so that we
+		// can check that Fleet-managed aren't being deleted; this can be addressed once we add support
+		// for more types of declarations
+		var d fleet.MDMAppleRawDeclaration
+		if err := json.Unmarshal(decl.RawJSON, &d); err != nil {
+			return ctxerr.Wrap(ctx, err, "unmarshalling declaration")
+		}
+		if err := d.ValidateUserProvided(); err != nil {
+			return ctxerr.Wrap(ctx, &fleet.BadRequestError{Message: err.Error()})
+		}
 	}
 
 	var teamName string
diff --git a/server/service/integration_mdm_ddm_test.go b/server/service/integration_mdm_ddm_test.go
index 220b77cb6e..91b23f2a46 100644
--- a/server/service/integration_mdm_ddm_test.go
+++ b/server/service/integration_mdm_ddm_test.go
@@ -598,15 +598,19 @@ WHERE name = ?`
 		return decl
 	}
 	nameToIdentifier := make(map[string]string, 3)
+	nameToUUID := make(map[string]string, 3)
 	decl := getDeclaration(t, "N0")
 	nameToIdentifier["N0"] = decl.Identifier
+	nameToUUID["N0"] = decl.DeclarationUUID
 	decl = getDeclaration(t, "N1")
 	assert.NotContains(t, string(decl.RawJSON), myBash)
 	assert.Contains(t, string(decl.RawJSON), "$"+fleet.ServerSecretPrefix+"BASH")
 	nameToIdentifier["N1"] = decl.Identifier
+	nameToUUID["N1"] = decl.DeclarationUUID
 	decl = getDeclaration(t, "N2")
 	assert.Equal(t, string(decl.RawJSON), "${"+fleet.ServerSecretPrefix+"PROFILE}")
 	nameToIdentifier["N2"] = decl.Identifier
+	nameToUUID["N2"] = decl.DeclarationUUID
 
 	// trigger a profile sync
 	s.awaitTriggerProfileSchedule(t)
@@ -641,6 +645,13 @@ WHERE name = ?`
 	require.NoError(t, err)
 	require.NoError(t, json.NewDecoder(r.Body).Decode(&gotParsed))
 	assert.EqualValues(t, `{"DataAssetReference":"com.fleet.asset.bash","ServiceType":"com.apple.bash2"}`, gotParsed.Payload)
+
+	// Delete the profiles
+	s.Do("DELETE", "/api/latest/fleet/configuration_profiles/"+nameToUUID["N0"], nil, http.StatusOK)
+	s.Do("DELETE", "/api/latest/fleet/configuration_profiles/"+nameToUUID["N1"], nil, http.StatusOK)
+	s.Do("DELETE", "/api/latest/fleet/configuration_profiles/"+nameToUUID["N2"], nil, http.StatusOK)
+	s.DoJSON("GET", "/api/latest/fleet/mdm/profiles", &listMDMConfigProfilesRequest{}, http.StatusOK, &resp)
+	require.Empty(t, resp.Profiles)
 }
 
 func (s *integrationMDMTestSuite) TestAppleDDMReconciliation() {