From a669f3938bb0aeb6d56d9bed334a7e0b37bd7a26 Mon Sep 17 00:00:00 2001 From: Gabriel Hernandez Date: Tue, 18 Feb 2025 20:08:06 +0000 Subject: [PATCH] fix issue with resetting abm token teams (#26259) For #24040 Add gitops option for the request to modify the app config. There was an issue with the abm token teams getting reset to default anytime the `PATCH /fleet/config` endpoint was called. @jahzielv and I discussed various options on how to solve this and agreed that the approach taken in this PR was the quickest but not the best. Ideally, we'd like the gitops client to send back the data to the endpoint that its going to update. This will allow the `PATCH /fleet/config` endpoint to work like a standard `PATCH` request and only update the options provided instead of updating the app config differently depending on the client calling the endpoint. - [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 --- .../issue-24040-fix-abm-token-team-resetting | 1 + ee/server/service/mdm.go | 1 - server/service/appconfig.go | 20 ++++++------ server/service/client.go | 6 ++++ server/service/integration_mdm_test.go | 31 +++++++++++++++++++ 5 files changed, 49 insertions(+), 10 deletions(-) create mode 100644 changes/issue-24040-fix-abm-token-team-resetting diff --git a/changes/issue-24040-fix-abm-token-team-resetting b/changes/issue-24040-fix-abm-token-team-resetting new file mode 100644 index 0000000000..e14b8542c3 --- /dev/null +++ b/changes/issue-24040-fix-abm-token-team-resetting @@ -0,0 +1 @@ +- fix an issue where the abm token teams were being reset when making updates to the app config. diff --git a/ee/server/service/mdm.go b/ee/server/service/mdm.go index 2de82b7080..c9b350b3aa 100644 --- a/ee/server/service/mdm.go +++ b/ee/server/service/mdm.go @@ -1301,7 +1301,6 @@ func (svc *Service) UpdateABMTokenTeams(ctx context.Context, tokenID uint, macOS } // validate the team IDs - token.MacOSTeam = fleet.ABMTokenTeam{Name: fleet.TeamNameNoTeam} token.MacOSDefaultTeamID = nil token.IOSTeam = fleet.ABMTokenTeam{Name: fleet.TeamNameNoTeam} diff --git a/server/service/appconfig.go b/server/service/appconfig.go index accdd0b6e4..a12ac23878 100644 --- a/server/service/appconfig.go +++ b/server/service/appconfig.go @@ -702,7 +702,6 @@ func (svc *Service) ModifyAppConfig(ctx context.Context, p []byte, applyOpts fle } } - // Reset teams for ABM tokens that exist in Fleet but aren't present in the config being passed tokensInCfg := make(map[string]struct{}) for _, t := range newAppConfig.MDM.AppleBusinessManager.Value { tokensInCfg[t.OrganizationName] = struct{}{} @@ -712,13 +711,16 @@ func (svc *Service) ModifyAppConfig(ctx context.Context, p []byte, applyOpts fle if err != nil { return nil, ctxerr.Wrap(ctx, err, "listing ABM tokens") } - for _, tok := range toks { - if _, ok := tokensInCfg[tok.OrganizationName]; !ok { - tok.MacOSDefaultTeamID = nil - tok.IOSDefaultTeamID = nil - tok.IPadOSDefaultTeamID = nil - if err := svc.ds.SaveABMToken(ctx, tok); err != nil { - return nil, ctxerr.Wrap(ctx, err, "saving ABM token assignments") + + if newAppConfig.MDM.AppleBusinessManager.Set && len(newAppConfig.MDM.AppleBusinessManager.Value) == 0 { + for _, tok := range toks { + if _, ok := tokensInCfg[tok.OrganizationName]; !ok { + tok.MacOSDefaultTeamID = nil + tok.IOSDefaultTeamID = nil + tok.IPadOSDefaultTeamID = nil + if err := svc.ds.SaveABMToken(ctx, tok); err != nil { + return nil, ctxerr.Wrap(ctx, err, "saving ABM token assignments") + } } } } @@ -1229,7 +1231,7 @@ func (svc *Service) validateABMAssignments( return []*fleet.ABMToken{tok}, nil } - if mdm.AppleBusinessManager.Set && mdm.AppleBusinessManager.Valid { + if mdm.AppleBusinessManager.Set && len(mdm.AppleBusinessManager.Value) > 0 { if !license.IsPremium() { invalid.Append("mdm.apple_business_manager", ErrMissingLicense.Error()) return nil, nil diff --git a/server/service/client.go b/server/service/client.go index 4938dd5167..895fb03a60 100644 --- a/server/service/client.go +++ b/server/service/client.go @@ -1613,6 +1613,12 @@ func (c *Client) DoGitOps( return nil, errors.New("org_settings.mdm config is not a map") } + if _, ok := mdmAppConfig["apple_bm_default_team"]; !ok && appConfig.License.IsPremium() { + if _, ok := mdmAppConfig["apple_business_manager"]; !ok { + mdmAppConfig["apple_business_manager"] = []interface{}{} + } + } + // Put in default value for volume_purchasing_program to clear the configuration if it's not set. if v, ok := mdmAppConfig["volume_purchasing_program"]; !ok || v == nil { mdmAppConfig["volume_purchasing_program"] = []interface{}{} diff --git a/server/service/integration_mdm_test.go b/server/service/integration_mdm_test.go index f1bf7b184e..39121c2fda 100644 --- a/server/service/integration_mdm_test.go +++ b/server/service/integration_mdm_test.go @@ -950,6 +950,37 @@ func (s *integrationMDMTestSuite) getABMTokenByName(orgName string, tokens []*fl return nil } +func (s *integrationMDMTestSuite) TestABMTeamPersistsOnConfigChange() { + t := s.T() + + s.enableABM(t.Name()) + + // Same tests, but for teams + team, err := s.ds.NewTeam(context.Background(), &fleet.Team{ + Name: "team1", + Description: "desc team1", + }) + require.NoError(t, err) + + tokensResp := listABMTokensResponse{} + s.DoJSON("GET", "/api/latest/fleet/abm_tokens", nil, http.StatusOK, &tokensResp) + tok := s.getABMTokenByName(t.Name(), tokensResp.Tokens) + + s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/abm_tokens/%d/teams", tok.ID), json.RawMessage(fmt.Sprintf(`{ + "ios_team_id": %d + }`, team.ID)), http.StatusOK, &updateABMTokenTeamsResponse{}) + + s.DoJSON("PATCH", "/api/latest/fleet/config", json.RawMessage(`{ + "org_info": {"org_name": "New name"} + }`), http.StatusOK, &appConfigResponse{}) + + s.DoJSON("GET", "/api/latest/fleet/abm_tokens", nil, http.StatusOK, &tokensResp) + + require.Len(t, tokensResp.Tokens, 1) + assert.Equal(t, team.Name, tokensResp.Tokens[0].IOSTeam.Name) + assert.Equal(t, team.ID, tokensResp.Tokens[0].IOSTeam.ID) +} + func (s *integrationMDMTestSuite) TestABMExpiredToken() { t := s.T()