From 4e11b3574c84e3cebbf4832b7671d536c2b0a107 Mon Sep 17 00:00:00 2001 From: Roberto Dip Date: Fri, 29 Mar 2024 12:40:47 -0300 Subject: [PATCH 1/2] missing table cleanups for DDM for #17953, this adds missing cleanups when: - teams are deleted - hosts are deleted also includes a few extra tests. --- server/datastore/mysql/apple_mdm.go | 4 +- server/datastore/mysql/hosts.go | 1 + server/datastore/mysql/hosts_test.go | 6 + server/datastore/mysql/teams.go | 8 +- server/datastore/mysql/teams_test.go | 10 ++ server/fleet/datastore.go | 2 +- server/mock/datastore_mock.go | 4 +- server/service/apple_mdm.go | 6 +- server/service/integration_ddm_test.go | 146 +++++++++++++++++++------ 9 files changed, 143 insertions(+), 44 deletions(-) diff --git a/server/datastore/mysql/apple_mdm.go b/server/datastore/mysql/apple_mdm.go index 1dac196ae2..8c9c3cc153 100644 --- a/server/datastore/mysql/apple_mdm.go +++ b/server/datastore/mysql/apple_mdm.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "database/sql" + "encoding/json" "errors" "fmt" "strings" @@ -323,7 +324,6 @@ func (ds *Datastore) DeleteMDMAppleConfigProfileByTeamAndIdentifier(ctx context. teamID = ptr.Uint(0) } - // TODO: add deletion of declarations here or separate method? res, err := ds.writer(ctx).ExecContext(ctx, `DELETE FROM mdm_apple_configuration_profiles WHERE team_id = ? AND identifier = ?`, teamID, profileIdentifier) if err != nil { return ctxerr.Wrap(ctx, err) @@ -3970,7 +3970,7 @@ func (ds *Datastore) MDMAppleSetPendingDeclarationsAs(ctx context.Context, hostU return ctxerr.Wrap(ctx, err, "updating host declaration status to verifying") } -func (ds *Datastore) InsertMDMAppleDDMRequest(ctx context.Context, hostUUID, messageType, rawJSON string) error { +func (ds *Datastore) InsertMDMAppleDDMRequest(ctx context.Context, hostUUID, messageType string, rawJSON json.RawMessage) error { const stmt = ` INSERT INTO mdm_apple_declarative_requests ( diff --git a/server/datastore/mysql/hosts.go b/server/datastore/mysql/hosts.go index 7d94e41c9e..28361f0a54 100644 --- a/server/datastore/mysql/hosts.go +++ b/server/datastore/mysql/hosts.go @@ -521,6 +521,7 @@ var additionalHostRefsByUUID = map[string]string{ "host_mdm_apple_profiles": "host_uuid", "host_mdm_apple_bootstrap_packages": "host_uuid", "host_mdm_windows_profiles": "host_uuid", + "host_mdm_apple_declarations": "host_uuid", } func (ds *Datastore) DeleteHost(ctx context.Context, hid uint) error { diff --git a/server/datastore/mysql/hosts_test.go b/server/datastore/mysql/hosts_test.go index b57784b1b3..32e5507887 100644 --- a/server/datastore/mysql/hosts_test.go +++ b/server/datastore/mysql/hosts_test.go @@ -6558,6 +6558,12 @@ func testHostsDeleteHosts(t *testing.T, ds *Datastore) { `, host.UUID) require.NoError(t, err) + _, err = ds.writer(context.Background()).Exec(` + INSERT INTO host_mdm_apple_declarations (host_uuid, declaration_uuid) + VALUES (?, uuid()) + `, host.UUID) + require.NoError(t, err) + err = ds.NewActivity( // automatically creates the host_activities entry context.Background(), user1, diff --git a/server/datastore/mysql/teams.go b/server/datastore/mysql/teams.go index be1a453a5a..6360540e63 100644 --- a/server/datastore/mysql/teams.go +++ b/server/datastore/mysql/teams.go @@ -5,9 +5,10 @@ import ( "database/sql" "encoding/json" "fmt" - "golang.org/x/text/unicode/norm" "strings" + "golang.org/x/text/unicode/norm" + "github.com/fleetdm/fleet/v4/server/contexts/ctxerr" "github.com/fleetdm/fleet/v4/server/fleet" "github.com/fleetdm/fleet/v4/server/ptr" @@ -116,6 +117,11 @@ func (ds *Datastore) DeleteTeam(ctx context.Context, tid uint) error { return ctxerr.Wrapf(ctx, err, "deleting mdm_windows_configuration_profiles for team %d", tid) } + _, err = tx.ExecContext(ctx, `DELETE FROM mdm_apple_declarations WHERE team_id=?`, tid) + if err != nil { + return ctxerr.Wrapf(ctx, err, "deleting mdm_apple_declarations for team %d", tid) + } + return nil }) } diff --git a/server/datastore/mysql/teams_test.go b/server/datastore/mysql/teams_test.go index bbaa8c7c5e..33126a4d17 100644 --- a/server/datastore/mysql/teams_test.go +++ b/server/datastore/mysql/teams_test.go @@ -97,6 +97,13 @@ func testTeamsGetSetDelete(t *testing.T, ds *Datastore) { }) require.NoError(t, err) + dec, err := ds.NewMDMAppleDeclaration(context.Background(), &fleet.MDMAppleDeclaration{ + Identifier: "decl-1", + Name: "decl-1", + TeamID: &team.ID, + }) + require.NoError(t, err) + err = ds.DeleteTeam(context.Background(), team.ID) require.NoError(t, err) @@ -114,6 +121,9 @@ func testTeamsGetSetDelete(t *testing.T, ds *Datastore) { _, err = ds.GetMDMWindowsConfigProfile(context.Background(), wcp.ProfileUUID) require.ErrorAs(t, err, &nfe) + _, err = ds.GetMDMAppleConfigProfile(context.Background(), dec.DeclarationUUID) + require.ErrorAs(t, err, &nfe) + require.NoError(t, ds.DeletePack(context.Background(), newP.Name)) }) } diff --git a/server/fleet/datastore.go b/server/fleet/datastore.go index 816f706f6a..7d0b112a42 100644 --- a/server/fleet/datastore.go +++ b/server/fleet/datastore.go @@ -1173,7 +1173,7 @@ type Datastore interface { UpdateDEPAssignProfileRetryPending(ctx context.Context, jobID uint, serials []string) error // InsertMDMAppleDDMRequest inserts a DDM request. - InsertMDMAppleDDMRequest(ctx context.Context, hostUUID, messageType, rawJSON string) error + InsertMDMAppleDDMRequest(ctx context.Context, hostUUID, messageType string, rawJSON json.RawMessage) error // MDMAppleDDMDeclarationsToken returns the token used to synchronize declarations for the // specified host UUID. diff --git a/server/mock/datastore_mock.go b/server/mock/datastore_mock.go index 26510aaac0..c42c08ee6c 100644 --- a/server/mock/datastore_mock.go +++ b/server/mock/datastore_mock.go @@ -780,7 +780,7 @@ type GetDEPAssignProfileExpiredCooldownsFunc func(ctx context.Context) (map[uint type UpdateDEPAssignProfileRetryPendingFunc func(ctx context.Context, jobID uint, serials []string) error -type InsertMDMAppleDDMRequestFunc func(ctx context.Context, hostUUID string, messageType string, rawJSON string) error +type InsertMDMAppleDDMRequestFunc func(ctx context.Context, hostUUID string, messageType string, rawJSON json.RawMessage) error type MDMAppleDDMDeclarationsTokenFunc func(ctx context.Context, hostUUID string) (*fleet.MDMAppleDDMDeclarationsToken, error) @@ -4889,7 +4889,7 @@ func (s *DataStore) UpdateDEPAssignProfileRetryPending(ctx context.Context, jobI return s.UpdateDEPAssignProfileRetryPendingFunc(ctx, jobID, serials) } -func (s *DataStore) InsertMDMAppleDDMRequest(ctx context.Context, hostUUID string, messageType string, rawJSON string) error { +func (s *DataStore) InsertMDMAppleDDMRequest(ctx context.Context, hostUUID string, messageType string, rawJSON json.RawMessage) error { s.mu.Lock() s.InsertMDMAppleDDMRequestFuncInvoked = true s.mu.Unlock() diff --git a/server/service/apple_mdm.go b/server/service/apple_mdm.go index 81e80a2653..66e4ba30c6 100644 --- a/server/service/apple_mdm.go +++ b/server/service/apple_mdm.go @@ -3245,7 +3245,7 @@ func (svc *MDMAppleDDMService) DeclarativeManagement(r *mdm.Request, dm *mdm.Dec } level.Debug(svc.logger).Log("msg", "ddm request received", "endpoint", dm.Endpoint) - if err := svc.ds.InsertMDMAppleDDMRequest(r.Context, dm.UDID, dm.Endpoint, string(dm.Data)); err != nil { + if err := svc.ds.InsertMDMAppleDDMRequest(r.Context, dm.UDID, dm.Endpoint, dm.Data); err != nil { return nil, ctxerr.Wrap(r.Context, err, "insert ddm request history") } @@ -3332,7 +3332,7 @@ func (svc *MDMAppleDDMService) handleDeclarationItems(ctx context.Context, hostU func (svc *MDMAppleDDMService) handleDeclarationsResponse(ctx context.Context, endpoint string, hostUUID string) ([]byte, error) { parts := strings.Split(endpoint, "/") if len(parts) != 3 { - return nil, nano_service.NewHTTPStatusError(http.StatusBadRequest, ctxerr.New(ctx, fmt.Sprintf("unrecognized declarations endpoint: %s", endpoint))) + return nil, nano_service.NewHTTPStatusError(http.StatusBadRequest, ctxerr.Errorf(ctx, "unrecognized declarations endpoint: %s", endpoint)) } level.Debug(svc.logger).Log("msg", "parsed declarations request", "type", parts[1], "identifier", parts[2]) @@ -3342,7 +3342,7 @@ func (svc *MDMAppleDDMService) handleDeclarationsResponse(ctx context.Context, e case "configuration": return svc.handleConfigurationDeclaration(ctx, parts, hostUUID) default: - return nil, newNotFoundError() + return nil, nano_service.NewHTTPStatusError(http.StatusBadRequest, ctxerr.Errorf(ctx, "declaration type not supported: %s", parts[1])) } } diff --git a/server/service/integration_ddm_test.go b/server/service/integration_ddm_test.go index cceafd1a83..b6d7e72e0f 100644 --- a/server/service/integration_ddm_test.go +++ b/server/service/integration_ddm_test.go @@ -346,30 +346,9 @@ INSERT INTO host_mdm_apple_declarations ( } } - checkRequestsDatabase := func(t *testing.T, messageType, enrollmentID string, expectedCount int) { - mysql.ExecAdhocSQL(t, s.ds, func(q sqlx.ExtContext) error { - var count int - if err := sqlx.GetContext( - context.Background(), - q, - &count, - "SELECT count(*) AS count FROM mdm_apple_declarative_requests WHERE enrollment_id = ? AND message_type = ?", - enrollmentID, - messageType, - ); err != nil { - return err - } - - require.Equal(t, expectedCount, count, "unexpected db row count for declaration requests") - - return nil - }) - } - var currDeclToken string // we'll use this to track the expected token across tests t.Run("Tokens", func(t *testing.T) { - checkRequestsDatabase(t, "tokens", mdmDevice.UUID, 0) // get tokens, timestamp should be the same as the declaration and token should be non-empty r, err := mdmDevice.DeclarativeManagement("tokens") require.NoError(t, err) @@ -393,7 +372,6 @@ INSERT INTO host_mdm_apple_declarations ( } insertDeclaration(t, noTeamDeclsByUUID["456"]) insertHostDeclaration(t, mdmDevice.UUID, noTeamDeclsByUUID["456"]) - checkRequestsDatabase(t, "tokens", mdmDevice.UUID, 1) // get tokens again, timestamp and token should have changed r, err = mdmDevice.DeclarativeManagement("tokens") @@ -401,11 +379,9 @@ INSERT INTO host_mdm_apple_declarations ( parsed = parseTokensResp(r) checkTokensResp(t, parsed, then.Add(1*time.Minute), currDeclToken) currDeclToken = parsed.SyncTokens.DeclarationsToken - checkRequestsDatabase(t, "tokens", mdmDevice.UUID, 2) }) t.Run("DeclarationItems", func(t *testing.T) { - checkRequestsDatabase(t, "declaration-items", mdmDevice.UUID, 0) r, err := mdmDevice.DeclarativeManagement("declaration-items") require.NoError(t, err) checkDeclarationItemsResp(t, parseDeclarationItemsResp(r), currDeclToken, mapDeclsByChecksum(noTeamDeclsByUUID)) @@ -426,7 +402,6 @@ INSERT INTO host_mdm_apple_declarations ( } insertDeclaration(t, noTeamDeclsByUUID["789"]) insertHostDeclaration(t, mdmDevice.UUID, noTeamDeclsByUUID["789"]) - checkRequestsDatabase(t, "declaration-items", mdmDevice.UUID, 1) // get tokens again, timestamp and token should have changed r, err = mdmDevice.DeclarativeManagement("tokens") @@ -438,20 +413,16 @@ INSERT INTO host_mdm_apple_declarations ( r, err = mdmDevice.DeclarativeManagement("declaration-items") require.NoError(t, err) checkDeclarationItemsResp(t, parseDeclarationItemsResp(r), currDeclToken, mapDeclsByChecksum(noTeamDeclsByUUID)) - checkRequestsDatabase(t, "declaration-items", mdmDevice.UUID, 2) }) t.Run("Status", func(t *testing.T) { - checkRequestsDatabase(t, "status", mdmDevice.UUID, 0) _, err := mdmDevice.DeclarativeManagement("status", fleet.MDMAppleDDMStatusReport{}) require.NoError(t, err) - checkRequestsDatabase(t, "status", mdmDevice.UUID, 1) }) t.Run("Declaration", func(t *testing.T) { want := noTeamDeclsByUUID["123"] declarationPath := fmt.Sprintf("declaration/%s/%s", "configuration", want.Identifier) - checkRequestsDatabase(t, declarationPath, mdmDevice.UUID, 0) r, err := mdmDevice.DeclarativeManagement(declarationPath) require.NoError(t, err) @@ -476,23 +447,26 @@ INSERT INTO host_mdm_apple_declarations ( want = noTeamDeclsByUUID["abc"] r, err = mdmDevice.DeclarativeManagement(fmt.Sprintf("declaration/%s/%s", "configuration", want.Identifier)) require.NoError(t, err) - checkRequestsDatabase(t, declarationPath, mdmDevice.UUID, 1) // try getting a non-existent declaration, should fail 404 nonExistantDeclarationPath := fmt.Sprintf("declaration/%s/%s", "configuration", "nonexistent") - checkRequestsDatabase(t, nonExistantDeclarationPath, mdmDevice.UUID, 0) - _, err = mdmDevice.DeclarativeManagement(nonExistantDeclarationPath) + res, err := mdmDevice.DeclarativeManagement(nonExistantDeclarationPath) require.Error(t, err) + require.Equal(t, http.StatusNotFound, res.StatusCode) + require.ErrorContains(t, err, "404 Not Found") + + // try getting an unsupported declaration, should fail 404 + unsupportedDeclarationPath := fmt.Sprintf("declaration/%s/%s", "asset", "nonexistent") + res, err = mdmDevice.DeclarativeManagement(unsupportedDeclarationPath) + require.Error(t, err) + require.Equal(t, http.StatusNotFound, res.StatusCode) require.ErrorContains(t, err, "404 Not Found") - checkRequestsDatabase(t, nonExistantDeclarationPath, mdmDevice.UUID, 1) // typo should fail as bad request typoDeclarationPath := fmt.Sprintf("declarations/%s/%s", "configurations", want.Identifier) - checkRequestsDatabase(t, typoDeclarationPath, mdmDevice.UUID, 0) _, err = mdmDevice.DeclarativeManagement(typoDeclarationPath) require.Error(t, err) require.ErrorContains(t, err, "400 Bad Request") - checkRequestsDatabase(t, typoDeclarationPath, mdmDevice.UUID, 1) assertDeclarationResponse(r, want) }) @@ -934,6 +908,108 @@ func (s *integrationMDMTestSuite) TestDDMNoDeclarationsLeft() { require.Empty(t, items.Declarations.Management) } +func (s *integrationMDMTestSuite) TestDDMTransactionRecording() { + t := s.T() + ctx := context.Background() + + type record struct { + EnrollmentID string `db:"enrollment_id"` + MessageType string `db:"message_type"` + RawJSON *json.RawMessage `db:"raw_json"` + } + verifyTransactionRecord := func(want record) { + var got record + mysql.ExecAdhocSQL(t, s.ds, func(q sqlx.ExtContext) error { + return sqlx.GetContext( + ctx, q, &got, + `SELECT + enrollment_id, message_type, raw_json + FROM mdm_apple_declarative_requests + ORDER BY id DESC + LIMIT 1`, + ) + }) + if got.RawJSON != nil { + fmt.Println(string(*got.RawJSON)) + } + require.Equal(t, want, got) + } + + declarations := []fleet.MDMProfileBatchPayload{ + {Name: "N1.json", Contents: declarationForTest("I1")}, + {Name: "N2.json", Contents: declarationForTest("I2")}, + } + // add global declarations + s.Do("POST", "/api/v1/fleet/mdm/profiles/batch", batchSetMDMProfilesRequest{Profiles: declarations}, http.StatusNoContent) + + // reconcile declarations + err := ReconcileAppleDeclarations(ctx, s.ds, s.mdmCommander, s.logger) + require.NoError(t, err) + + _, mdmDevice := createHostThenEnrollMDM(s.ds, s.server.URL, t) + _, err = mdmDevice.DeclarativeManagement("tokens") + require.NoError(t, err) + verifyTransactionRecord(record{ + MessageType: "tokens", + EnrollmentID: mdmDevice.UUID, + RawJSON: nil, + }) + + res, err := mdmDevice.DeclarativeManagement("declaration-items") + require.NoError(t, err) + verifyTransactionRecord(record{ + MessageType: "declaration-items", + EnrollmentID: mdmDevice.UUID, + RawJSON: nil, + }) + + var items fleet.MDMAppleDDMDeclarationItemsResponse + require.NoError(t, json.NewDecoder(res.Body).Decode(&items)) + var i1ServerToken string + for _, d := range items.Declarations.Configurations { + if d.Identifier == "I1" { + i1ServerToken = d.ServerToken + } + } + + // a second device requests tokens + _, mdmDeviceTwo := createHostThenEnrollMDM(s.ds, s.server.URL, t) + _, err = mdmDeviceTwo.DeclarativeManagement("tokens") + require.NoError(t, err) + verifyTransactionRecord(record{ + MessageType: "tokens", + EnrollmentID: mdmDeviceTwo.UUID, + RawJSON: nil, + }) + + _, err = mdmDevice.DeclarativeManagement("declaration/configuration/I1") + require.NoError(t, err) + verifyTransactionRecord(record{ + MessageType: "declaration/configuration/I1", + EnrollmentID: mdmDevice.UUID, + RawJSON: nil, + }) + + report := fleet.MDMAppleDDMStatusReport{} + report.StatusItems.Management.Declarations.Configurations = []fleet.MDMAppleDDMStatusDeclaration{ + {Active: true, Valid: fleet.MDMAppleDeclarationValid, Identifier: "I1", ServerToken: i1ServerToken}, + } + _, err = mdmDevice.DeclarativeManagement("status", report) + require.NoError(t, err) + verifyTransactionRecord(record{ + MessageType: "status", + EnrollmentID: mdmDevice.UUID, + RawJSON: ptr.RawMessage( + json.RawMessage( + fmt.Sprintf( + `{"StatusItems":{"management":{"declarations":{"activations":null,"configurations":[{"active":true,"identifier":"I1","valid":"valid","server-token":"%s"}],"assets":null,"management":null}}},"Errors":null}`, + i1ServerToken, + ), + ), + ), + }) +} + func declarationForTest(identifier string) []byte { return []byte(fmt.Sprintf(` { From e39d5d9341d0c6c4812bcc91acb6c9d07f2a0c38 Mon Sep 17 00:00:00 2001 From: Roberto Dip Date: Fri, 29 Mar 2024 14:18:26 -0300 Subject: [PATCH 2/2] fix tests --- server/service/apple_mdm.go | 2 +- server/service/integration_ddm_test.go | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/server/service/apple_mdm.go b/server/service/apple_mdm.go index 66e4ba30c6..c230ccbec7 100644 --- a/server/service/apple_mdm.go +++ b/server/service/apple_mdm.go @@ -3342,7 +3342,7 @@ func (svc *MDMAppleDDMService) handleDeclarationsResponse(ctx context.Context, e case "configuration": return svc.handleConfigurationDeclaration(ctx, parts, hostUUID) default: - return nil, nano_service.NewHTTPStatusError(http.StatusBadRequest, ctxerr.Errorf(ctx, "declaration type not supported: %s", parts[1])) + return nil, nano_service.NewHTTPStatusError(http.StatusNotFound, ctxerr.Errorf(ctx, "declaration type not supported: %s", parts[1])) } } diff --git a/server/service/integration_ddm_test.go b/server/service/integration_ddm_test.go index b6d7e72e0f..a2c5425ca3 100644 --- a/server/service/integration_ddm_test.go +++ b/server/service/integration_ddm_test.go @@ -450,16 +450,14 @@ INSERT INTO host_mdm_apple_declarations ( // try getting a non-existent declaration, should fail 404 nonExistantDeclarationPath := fmt.Sprintf("declaration/%s/%s", "configuration", "nonexistent") - res, err := mdmDevice.DeclarativeManagement(nonExistantDeclarationPath) + _, err = mdmDevice.DeclarativeManagement(nonExistantDeclarationPath) require.Error(t, err) - require.Equal(t, http.StatusNotFound, res.StatusCode) require.ErrorContains(t, err, "404 Not Found") // try getting an unsupported declaration, should fail 404 unsupportedDeclarationPath := fmt.Sprintf("declaration/%s/%s", "asset", "nonexistent") - res, err = mdmDevice.DeclarativeManagement(unsupportedDeclarationPath) + _, err = mdmDevice.DeclarativeManagement(unsupportedDeclarationPath) require.Error(t, err) - require.Equal(t, http.StatusNotFound, res.StatusCode) require.ErrorContains(t, err, "404 Not Found") // typo should fail as bad request