strip query strings from MDM server_url during ingestion (#12107)

for #12106
2023-06-05 12:53:36 -03:00
parent b08881b0d9
commit 3fa809e167
6 changed files with 276 additions and 17 deletions
@@ -0,0 +1,112 @@
+package tables
+
+import (
+	"database/sql"
+	"fmt"
+	"net/url"
+	"strings"
+
+	"github.com/jmoiron/sqlx"
+	"github.com/jmoiron/sqlx/reflectx"
+)
+
+func init() {
+	MigrationClient.AddMigration(Up_20230602111827, Down_20230602111827)
+}
+
+func Up_20230602111827(tx *sql.Tx) error {
+	txx := sqlx.Tx{Tx: tx, Mapper: reflectx.NewMapperFunc("db", sqlx.NameMapper)}
+	type mdmSolution struct {
+		ID        uint   `db:"id"`
+		ServerURL string `db:"server_url"`
+		Name      string `db:"name"`
+	}
+
+	// first, find all the MDM solutions
+	var mdmSolutions []mdmSolution
+	err := txx.Select(
+		&mdmSolutions,
+		`SELECT id, server_url, name
+                 FROM mobile_device_management_solutions
+                 ORDER BY created_at DESC`,
+	)
+	if err != nil {
+		return fmt.Errorf("selecting mobile_device_management_solutions: %w", err)
+	}
+
+	// find all the dupes
+	uniqs := map[string]mdmSolution{}
+	dupes := []uint{}
+	for _, solution := range mdmSolutions {
+		serverURL, err := url.Parse(solution.ServerURL)
+		if err != nil {
+			logger.Warn.Printf("unable to parse server_url %s, skipping\n", serverURL)
+			continue
+		}
+		// strip any query parameters from the URL
+		serverURL.RawQuery = ""
+		cleanURL := serverURL.String()
+
+		uniqSolution, ok := uniqs[cleanURL]
+		if !ok {
+			uniqs[cleanURL] = solution
+			continue
+		}
+
+		dupes = append(dupes, solution.ID)
+
+		// update host_mdm entries to point to the new solution
+		_, err = txx.Exec(
+			`UPDATE host_mdm SET server_url = ?, mdm_id = ?
+                         WHERE mdm_id = ?`, cleanURL, uniqSolution.ID, solution.ID)
+		if err != nil {
+			return fmt.Errorf("updating host_mdm entries with new solution: %w", err)
+		}
+	}
+
+	// delete all duplicated solutions
+	if len(dupes) > 0 {
+		stmt, args, err := sqlx.In(`DELETE FROM mobile_device_management_solutions WHERE id IN (?)`, dupes)
+		if err != nil {
+			return fmt.Errorf("building SQL IN statement: %w", err)
+		}
+		_, err = txx.Exec(stmt, args...)
+		if err != nil {
+			return fmt.Errorf("deleting duplicated MDM solutions: %w", err)
+		}
+	}
+
+	// make sure all the new solutions have the right URL
+	if len(uniqs) > 0 {
+		inPart := ""
+		args := []interface{}{}
+		for serverURL, solution := range uniqs {
+			inPart += "(?, ?, ?),"
+			args = append(args, solution.ID, solution.Name, serverURL)
+
+			// and the related host_mdm rows as well
+			_, err = txx.Exec(`UPDATE host_mdm SET server_url = ? WHERE mdm_id = ?`, serverURL, solution.ID)
+			if err != nil {
+				return fmt.Errorf("updating host_mdm entries with new server_url: %w", err)
+			}
+		}
+		stmt := `
+	INSERT INTO mobile_device_management_solutions (id, name, server_url)
+          VALUES %s
+          ON DUPLICATE KEY UPDATE server_url = VALUES(server_url)
+	`
+		_, err = tx.Exec(
+			fmt.Sprintf(stmt, strings.TrimSuffix(inPart, ",")),
+			args...,
+		)
+		if err != nil {
+			return fmt.Errorf("updating mobile_device_management_solutions server_url: %w", err)
+		}
+	}
+
+	return nil
+}
+
+func Down_20230602111827(tx *sql.Tx) error {
+	return nil
+}
@@ -0,0 +1,60 @@
+package tables
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestUp_20230602111827(t *testing.T) {
+	db := applyUpToPrev(t)
+	insertMDMSolutionStmt := `INSERT INTO mobile_device_management_solutions (id, name, server_url) VALUES (?, ?, ?)`
+	_, err := db.Exec(insertMDMSolutionStmt, 1, "foo", "https://test.example.com?test=1")
+	require.NoError(t, err)
+	_, err = db.Exec(insertMDMSolutionStmt, 2, "foo", "https://test.example.com")
+	require.NoError(t, err)
+	_, err = db.Exec(insertMDMSolutionStmt, 3, "bar", "https://test.example.com/abc")
+	require.NoError(t, err)
+	_, err = db.Exec(insertMDMSolutionStmt, 4, "bar", "https://test.example.com/abc?test=1")
+	require.NoError(t, err)
+	_, err = db.Exec(insertMDMSolutionStmt, 5, "baz", "https://foo.bar.com")
+	require.NoError(t, err)
+
+	insertHostMDMStmt := `INSERT INTO host_mdm (host_id, server_url, mdm_id) VALUES (?, ?, ?)`
+	_, err = db.Exec(insertHostMDMStmt, 1, "https://test.example.com?test=1", 1)
+	require.NoError(t, err)
+	_, err = db.Exec(insertHostMDMStmt, 2, "https://test.example.com", 2)
+	require.NoError(t, err)
+	_, err = db.Exec(insertHostMDMStmt, 3, "https://test.example.com/abc", 3)
+	require.NoError(t, err)
+	_, err = db.Exec(insertHostMDMStmt, 4, "https://test.example.com/abc?test=1", 4)
+	require.NoError(t, err)
+	_, err = db.Exec(insertHostMDMStmt, 5, "https://foo.bar.com", 5)
+	require.NoError(t, err)
+	_, err = db.Exec(insertHostMDMStmt, 6, "https://test.example.com?test=1", 1)
+	require.NoError(t, err)
+	_, err = db.Exec(insertHostMDMStmt, 7, "https://test.example.com", 2)
+	require.NoError(t, err)
+
+	applyNext(t, db)
+
+	type hostMDM struct {
+		ServerURL string `db:"server_url"`
+		MDMID     uint   `db:"mdm_id"`
+	}
+	var hostMDMs []hostMDM
+	err = db.Select(&hostMDMs, "SELECT server_url, mdm_id FROM host_mdm GROUP BY server_url, mdm_id")
+	require.NoError(t, err)
+	require.Len(t, hostMDMs, 3)
+	require.ElementsMatch(t, []hostMDM{
+		{"https://test.example.com", 1},
+		{"https://test.example.com/abc", 3},
+		{"https://foo.bar.com", 5},
+	}, hostMDMs)
+
+	var mdmSolutions []string
+	err = db.Select(&mdmSolutions, "SELECT server_url FROM mobile_device_management_solutions")
+	require.NoError(t, err)
+	require.Len(t, mdmSolutions, 3)
+	require.ElementsMatch(t, []string{"https://test.example.com", "https://test.example.com/abc", "https://foo.bar.com"}, mdmSolutions)
+}
@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"net"
+	"net/url"
 	"regexp"
 	"strconv"
 	"strings"
@@ -1283,11 +1284,18 @@ func directIngestMDMMac(ctx context.Context, logger log.Logger, host *fleet.Host
 		host.RefetchCriticalQueriesUntil = nil
 	}

+	serverURL, err := url.Parse(rows[0]["server_url"])
+	if err != nil {
+		return ctxerr.Wrap(ctx, err, "parsing server_url")
+	}
+	// strip any query parameters from the URL
+	serverURL.RawQuery = ""
+
 	return ds.SetOrUpdateMDMData(ctx,
 		host.ID,
 		false,
 		enrolled,
-		rows[0]["server_url"],
+		serverURL.String(),
 		installedFromDep,
 		mdmSolutionName,
 	)
@@ -13,6 +13,7 @@ import (
 	"github.com/WatchBeam/clock"
 	"github.com/fleetdm/fleet/v4/server/config"
 	"github.com/fleetdm/fleet/v4/server/fleet"
+	apple_mdm "github.com/fleetdm/fleet/v4/server/mdm/apple"
 	"github.com/fleetdm/fleet/v4/server/mock"
 	"github.com/fleetdm/fleet/v4/server/ptr"
 	"github.com/fleetdm/fleet/v4/server/service/async"
@@ -446,24 +447,101 @@ func TestDetailQueriesOSVersionChrome(t *testing.T) {

 func TestDirectIngestMDMMac(t *testing.T) {
 	ds := new(mock.Store)
-	ds.SetOrUpdateMDMDataFunc = func(ctx context.Context, hostID uint, isServer, enrolled bool, serverURL string, installedFromDep bool, name string) error {
-		require.False(t, enrolled)
-		require.False(t, installedFromDep)
-		require.Empty(t, serverURL)
-		return nil
-	}
-
 	var host fleet.Host

-	err := directIngestMDMMac(context.Background(), log.NewNopLogger(), &host, ds, []map[string]string{
+	cases := []struct {
+		name       string
+		got        map[string]string
+		wantParams []any
+		wantErr    string
+	}{
 		{
-			"enrolled":           "false",
-			"installed_from_dep": "",
-			"server_url":         "",
+			"empty server URL",
+			map[string]string{
+				"enrolled":           "false",
+				"installed_from_dep": "",
+				"server_url":         "",
+			},
+			[]any{false, false, "", false, fleet.UnknownMDMName},
+			"",
 		},
-	})
-	require.NoError(t, err)
-	require.True(t, ds.SetOrUpdateMDMDataFuncInvoked)
+		{
+			"with Fleet payload identifier",
+			map[string]string{
+				"enrolled":           "true",
+				"installed_from_dep": "true",
+				"server_url":         "https://test.example.com",
+				"payload_identifier": apple_mdm.FleetPayloadIdentifier,
+			},
+			[]any{false, true, "https://test.example.com", true, fleet.WellKnownMDMFleet},
+			"",
+		},
+		{
+			"with a query string on the server URL",
+			map[string]string{
+				"enrolled":           "true",
+				"installed_from_dep": "true",
+				"server_url":         "https://jamf.com/1/some/path?one=1&two=2",
+			},
+			[]any{false, true, "https://jamf.com/1/some/path", true, fleet.WellKnownMDMJamf},
+			"",
+		},
+		{
+			"with invalid installed_from_dep",
+			map[string]string{
+				"enrolled":           "true",
+				"installed_from_dep": "invalid",
+				"server_url":         "https://jamf.com/1/some/path?one=1&two=2",
+			},
+			[]any{},
+			"parsing installed_from_dep",
+		},
+		{
+			"with invalid enrolled",
+			map[string]string{
+				"enrolled":           "invalid",
+				"installed_from_dep": "false",
+				"server_url":         "https://jamf.com/1/some/path?one=1&two=2",
+			},
+			[]any{},
+			"parsing enrolled",
+		},
+		{
+			"with invalid server_url",
+			map[string]string{
+				"enrolled":           "false",
+				"installed_from_dep": "false",
+				"server_url":         "ht tp://foo.com",
+			},
+			[]any{},
+			"parsing server_url",
+		},
+	}
+
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			ds.SetOrUpdateMDMDataFunc = func(ctx context.Context, hostID uint, isServer, enrolled bool, serverURL string, installedFromDep bool, name string) error {
+				require.Equal(t, isServer, c.wantParams[0])
+				require.Equal(t, enrolled, c.wantParams[1])
+				require.Equal(t, serverURL, c.wantParams[2])
+				require.Equal(t, installedFromDep, c.wantParams[3])
+				require.Equal(t, name, c.wantParams[4])
+				return nil
+			}
+
+			err := directIngestMDMMac(context.Background(), log.NewNopLogger(), &host, ds, []map[string]string{c.got})
+			if c.wantErr != "" {
+				require.ErrorContains(t, err, c.wantErr)
+				require.False(t, ds.SetOrUpdateMDMDataFuncInvoked)
+
+			} else {
+				require.True(t, ds.SetOrUpdateMDMDataFuncInvoked)
+				require.NoError(t, err)
+				ds.SetOrUpdateMDMDataFuncInvoked = false
+			}
+		})
+	}
+
 }

 func TestDirectIngestMDMWindows(t *testing.T) {