From 65234043f8a8c42f11c4de5a683526496da24824 Mon Sep 17 00:00:00 2001 From: Jahziel Villasana-Espinoza Date: Thu, 20 Nov 2025 18:55:46 -0500 Subject: [PATCH] add missing patch semantics (#36004) **Related issue:** Resolves #35998 # Checklist for submitter If some of the following don't apply, delete the relevant line. ## Testing - [x] Added/updated automated tests - [x] QA'd all new/changed functionality manually For unreleased bug fixes in a release candidate, one of: - [x] Confirmed that the fix is not expected to adversely impact load test results --- ee/server/service/software_installers.go | 28 ++++---- ee/server/service/vpp.go | 53 ++++++++------ frontend/services/entities/software.ts | 5 ++ server/fleet/service.go | 2 +- server/mock/service/service_mock.go | 4 +- server/service/integration_mdm_test.go | 16 ++--- .../integration_software_titles_test.go | 70 ++++++++++++++++--- .../service/integration_vpp_install_test.go | 4 +- server/service/vpp.go | 4 +- 9 files changed, 127 insertions(+), 59 deletions(-) diff --git a/ee/server/service/software_installers.go b/ee/server/service/software_installers.go index 2c86415dc2..748ce55312 100644 --- a/ee/server/service/software_installers.go +++ b/ee/server/service/software_installers.go @@ -327,21 +327,23 @@ func (svc *Service) UpdateSoftwareInstaller(ctx context.Context, payload *fleet. dirty := make(map[string]bool) - payload.Categories = server.RemoveDuplicatesFromSlice(payload.Categories) - catIDs, err := svc.ds.GetSoftwareCategoryIDs(ctx, payload.Categories) - if err != nil { - return nil, ctxerr.Wrap(ctx, err, "getting software category ids") - } - - if len(catIDs) != len(payload.Categories) { - return nil, &fleet.BadRequestError{ - Message: "some or all of the categories provided don't exist", - InternalErr: fmt.Errorf("categories provided: %v", payload.Categories), + if payload.Categories != nil { + payload.Categories = server.RemoveDuplicatesFromSlice(payload.Categories) + catIDs, err := svc.ds.GetSoftwareCategoryIDs(ctx, payload.Categories) + if err != nil { + return nil, ctxerr.Wrap(ctx, err, "getting software category ids") } - } - payload.CategoryIDs = catIDs - dirty["Categories"] = true + if len(catIDs) != len(payload.Categories) { + return nil, &fleet.BadRequestError{ + Message: "some or all of the categories provided don't exist", + InternalErr: fmt.Errorf("categories provided: %v", payload.Categories), + } + } + + payload.CategoryIDs = catIDs + dirty["Categories"] = true + } // Handle in house apps separately if software.InHouseAppCount == 1 { diff --git a/ee/server/service/vpp.go b/ee/server/service/vpp.go index a50e814874..a67a5794e3 100644 --- a/ee/server/service/vpp.go +++ b/ee/server/service/vpp.go @@ -603,7 +603,7 @@ func getVPPAppsMetadata(ctx context.Context, ids []fleet.VPPAppTeam) ([]*fleet.V return apps, nil } -func (svc *Service) UpdateAppStoreApp(ctx context.Context, titleID uint, teamID *uint, selfService bool, labelsIncludeAny, labelsExcludeAny, categories []string, displayName *string) (*fleet.VPPAppStoreApp, error) { +func (svc *Service) UpdateAppStoreApp(ctx context.Context, titleID uint, teamID *uint, selfService *bool, labelsIncludeAny, labelsExcludeAny, categories []string, displayName *string) (*fleet.VPPAppStoreApp, error) { if err := svc.authz.Authorize(ctx, &fleet.VPPApp{TeamID: teamID}, fleet.ActionWrite); err != nil { return nil, err } @@ -621,9 +621,13 @@ func (svc *Service) UpdateAppStoreApp(ctx context.Context, titleID uint, teamID teamName = tm.Name } - validatedLabels, err := ValidateSoftwareLabels(ctx, svc, labelsIncludeAny, labelsExcludeAny) - if err != nil { - return nil, ctxerr.Wrap(ctx, err, "UpdateAppStoreApp: validating software labels") + var validatedLabels *fleet.LabelIdentsWithScope + if labelsExcludeAny != nil || labelsIncludeAny != nil { + var err error + validatedLabels, err = ValidateSoftwareLabels(ctx, svc, labelsIncludeAny, labelsExcludeAny) + if err != nil { + return nil, ctxerr.Wrap(ctx, err, "UpdateAppStoreApp: validating software labels") + } } meta, err := svc.ds.GetVPPAppMetadataByTeamAndTitleID(ctx, teamID, titleID) @@ -631,12 +635,17 @@ func (svc *Service) UpdateAppStoreApp(ctx context.Context, titleID uint, teamID return nil, ctxerr.Wrap(ctx, err, "UpdateAppStoreApp: getting vpp app metadata") } + selfServiceVal := meta.SelfService + if selfService != nil { + selfServiceVal = *selfService + } + appToWrite := &fleet.VPPApp{ VPPAppTeam: fleet.VPPAppTeam{ VPPAppID: fleet.VPPAppID{ AdamID: meta.AdamID, Platform: meta.Platform, }, - SelfService: selfService, + SelfService: selfServiceVal, ValidatedLabels: validatedLabels, DisplayName: displayName, }, @@ -650,20 +659,22 @@ func (svc *Service) UpdateAppStoreApp(ctx context.Context, titleID uint, teamID appToWrite.IconURL = *meta.IconURL } - categories = server.RemoveDuplicatesFromSlice(categories) - catIDs, err := svc.ds.GetSoftwareCategoryIDs(ctx, categories) - if err != nil { - return nil, ctxerr.Wrap(ctx, err, "getting software category ids") - } - - if len(catIDs) != len(categories) { - return nil, &fleet.BadRequestError{ - Message: "some or all of the categories provided don't exist", - InternalErr: fmt.Errorf("categories provided: %v", categories), + if categories != nil { + categories = server.RemoveDuplicatesFromSlice(categories) + catIDs, err := svc.ds.GetSoftwareCategoryIDs(ctx, categories) + if err != nil { + return nil, ctxerr.Wrap(ctx, err, "getting software category ids") } - } - appToWrite.CategoryIDs = catIDs + if len(catIDs) != len(categories) { + return nil, &fleet.BadRequestError{ + Message: "some or all of the categories provided don't exist", + InternalErr: fmt.Errorf("categories provided: %v", categories), + } + } + + appToWrite.CategoryIDs = catIDs + } // check if labels have changed var existingLabels fleet.LabelIdentsWithScope @@ -682,8 +693,10 @@ func (svc *Service) UpdateAppStoreApp(ctx context.Context, titleID uint, teamID existingLabels.ByName[l.LabelName] = fleet.LabelIdent{LabelName: l.LabelName, LabelID: l.LabelID} } } - - labelsChanged := !validatedLabels.Equal(&existingLabels) + var labelsChanged bool + if validatedLabels != nil { + labelsChanged = !validatedLabels.Equal(&existingLabels) + } // Get the hosts that are NOT in label scope currently (before the update happens) var hostsNotInScope map[uint]struct{} @@ -727,7 +740,7 @@ func (svc *Service) UpdateAppStoreApp(ctx context.Context, titleID uint, teamID act := fleet.ActivityEditedAppStoreApp{ TeamName: &teamName, TeamID: teamID, - SelfService: selfService, + SelfService: selfServiceVal, SoftwareTitleID: titleID, SoftwareTitle: meta.Name, AppStoreID: meta.AdamID, diff --git a/frontend/services/entities/software.ts b/frontend/services/entities/software.ts index 48bcea0a0e..abd528e24a 100644 --- a/frontend/services/entities/software.ts +++ b/frontend/services/entities/software.ts @@ -311,6 +311,8 @@ const handleEditAppStoreAppForm = ( if (formData.categories && formData.categories.length > 0) { body.categories = formData.categories as SoftwareCategory[]; + } else { + body.categories = []; } if (formData.targetType === "Custom") { @@ -320,6 +322,9 @@ const handleEditAppStoreAppForm = ( } else { body.labels_exclude_any = selectedLabels; } + } else { + body.labels_exclude_any = []; + body.labels_include_any = []; } }; diff --git a/server/fleet/service.go b/server/fleet/service.go index f32f40d56a..44224b578e 100644 --- a/server/fleet/service.go +++ b/server/fleet/service.go @@ -717,7 +717,7 @@ type Service interface { // AddAppStoreApp persists a VPP app onto a team and returns the resulting title ID AddAppStoreApp(ctx context.Context, teamID *uint, appTeam VPPAppTeam) (uint, error) - UpdateAppStoreApp(ctx context.Context, titleID uint, teamID *uint, selfService bool, labelsIncludeAny, labelsExcludeAny, categories []string, displayName *string) (*VPPAppStoreApp, error) + UpdateAppStoreApp(ctx context.Context, titleID uint, teamID *uint, selfService *bool, labelsIncludeAny, labelsExcludeAny, categories []string, displayName *string) (*VPPAppStoreApp, error) // GetInHouseAppManifest returns a manifest XML file that points at the download URL for the given in-house app. GetInHouseAppManifest(ctx context.Context, titleID uint, teamID *uint) ([]byte, error) diff --git a/server/mock/service/service_mock.go b/server/mock/service/service_mock.go index 5a0245c520..629f296c5e 100644 --- a/server/mock/service/service_mock.go +++ b/server/mock/service/service_mock.go @@ -456,7 +456,7 @@ type GetAppStoreAppsFunc func(ctx context.Context, teamID *uint) ([]*fleet.VPPAp type AddAppStoreAppFunc func(ctx context.Context, teamID *uint, appTeam fleet.VPPAppTeam) (uint, error) -type UpdateAppStoreAppFunc func(ctx context.Context, titleID uint, teamID *uint, selfService bool, labelsIncludeAny []string, labelsExcludeAny []string, categories []string, displayName *string) (*fleet.VPPAppStoreApp, error) +type UpdateAppStoreAppFunc func(ctx context.Context, titleID uint, teamID *uint, selfService *bool, labelsIncludeAny []string, labelsExcludeAny []string, categories []string, displayName *string) (*fleet.VPPAppStoreApp, error) type GetInHouseAppManifestFunc func(ctx context.Context, titleID uint, teamID *uint) ([]byte, error) @@ -3635,7 +3635,7 @@ func (s *Service) AddAppStoreApp(ctx context.Context, teamID *uint, appTeam flee return s.AddAppStoreAppFunc(ctx, teamID, appTeam) } -func (s *Service) UpdateAppStoreApp(ctx context.Context, titleID uint, teamID *uint, selfService bool, labelsIncludeAny []string, labelsExcludeAny []string, categories []string, displayName *string) (*fleet.VPPAppStoreApp, error) { +func (s *Service) UpdateAppStoreApp(ctx context.Context, titleID uint, teamID *uint, selfService *bool, labelsIncludeAny []string, labelsExcludeAny []string, categories []string, displayName *string) (*fleet.VPPAppStoreApp, error) { s.mu.Lock() s.UpdateAppStoreAppFuncInvoked = true s.mu.Unlock() diff --git a/server/service/integration_mdm_test.go b/server/service/integration_mdm_test.go index 56e253fe4c..2bb901c714 100644 --- a/server/service/integration_mdm_test.go +++ b/server/service/integration_mdm_test.go @@ -11786,7 +11786,7 @@ func (s *integrationMDMTestSuite) TestVPPApps() { require.Len(t, resp.SoftwareTitles, 1) nonVPPTitleID := resp.SoftwareTitles[0].ID - updateAppReq := &updateAppStoreAppRequest{TeamID: &team.ID, SelfService: false} + updateAppReq := &updateAppStoreAppRequest{TeamID: &team.ID, SelfService: ptr.Bool(false)} // Attempt to update the non-VPP software using the VPP path. Should fail. s.Do("PATCH", fmt.Sprintf("/api/latest/fleet/software/titles/%d/app_store_app", nonVPPTitleID), updateAppReq, http.StatusNotFound) @@ -13288,7 +13288,7 @@ func (s *integrationMDMTestSuite) TestVPPAppPolicyAutomation() { // Update the app to exclude any with l2. We should not enqueue an install here because mdmHost2 // has l2 (but it will re-enqueue the script for execution, immediately "activated") - updateAppReq := &updateAppStoreAppRequest{TeamID: &team.ID, SelfService: false, LabelsExcludeAny: []string{l2.Name}} + updateAppReq := &updateAppStoreAppRequest{TeamID: &team.ID, SelfService: ptr.Bool(false), LabelsExcludeAny: []string{l2.Name}} s.Do("PATCH", fmt.Sprintf("/api/latest/fleet/software/titles/%d/app_store_app", macOSTitleID), updateAppReq, http.StatusOK) s.DoJSONWithoutAuth("POST", "/api/osquery/distributed/write", genDistributedReqWithPolicyResults( @@ -13357,7 +13357,7 @@ func (s *integrationMDMTestSuite) TestVPPAppPolicyAutomation() { // Update the app to include any with l1. We should now enqueue an install as the app is in scope // (in addition to the script execution, which will be the only one "activated"). - updateAppReq = &updateAppStoreAppRequest{TeamID: &team.ID, SelfService: false, LabelsIncludeAny: []string{l1.Name, l2.Name}} + updateAppReq = &updateAppStoreAppRequest{TeamID: &team.ID, SelfService: ptr.Bool(false), LabelsIncludeAny: []string{l1.Name, l2.Name}} s.Do("PATCH", fmt.Sprintf("/api/latest/fleet/software/titles/%d/app_store_app", macOSTitleID), updateAppReq, http.StatusOK) s.DoJSONWithoutAuth("POST", "/api/osquery/distributed/write", genDistributedReqWithPolicyResults( @@ -16431,7 +16431,7 @@ func (s *integrationMDMTestSuite) TestVPPPolicyAutomationLabelScopingRetrigger() // Update the include any labels. The host has label2, so this means that the software // moved in scope. - updateAppReq := &updateAppStoreAppRequest{TeamID: &team.ID, SelfService: false, LabelsIncludeAny: []string{label2.Name}} + updateAppReq := &updateAppStoreAppRequest{TeamID: &team.ID, SelfService: ptr.Bool(false), LabelsIncludeAny: []string{label2.Name}} s.Do("PATCH", fmt.Sprintf("/api/latest/fleet/software/titles/%d/app_store_app", vppAppTitleID), updateAppReq, http.StatusOK) // Send back a failed result for the policy. @@ -16453,7 +16453,7 @@ func (s *integrationMDMTestSuite) TestVPPPolicyAutomationLabelScopingRetrigger() require.Equal(t, uint(1), policy1.FailingHostCount) // Update to exclude_any: label 2. This moves the software out of scope. The policy is still failing. - updateAppReq = &updateAppStoreAppRequest{TeamID: &team.ID, SelfService: false, LabelsExcludeAny: []string{label2.Name}} + updateAppReq = &updateAppStoreAppRequest{TeamID: &team.ID, SelfService: ptr.Bool(false), LabelsExcludeAny: []string{label2.Name}} s.Do("PATCH", fmt.Sprintf("/api/latest/fleet/software/titles/%d/app_store_app", vppAppTitleID), updateAppReq, http.StatusOK) err = s.ds.UpdateHostPolicyCounts(ctx) @@ -16495,7 +16495,7 @@ func (s *integrationMDMTestSuite) TestVPPPolicyAutomationLabelScopingRetrigger() require.Equal(t, uint(1), policy1.FailingHostCount) // Update to exclude_any: label 2. This moves the software out of scope. The policy is still failing. - updateAppReq = &updateAppStoreAppRequest{TeamID: &team.ID, SelfService: false, LabelsExcludeAny: []string{label2.Name}} + updateAppReq = &updateAppStoreAppRequest{TeamID: &team.ID, SelfService: ptr.Bool(false), LabelsExcludeAny: []string{label2.Name}} s.Do("PATCH", fmt.Sprintf("/api/latest/fleet/software/titles/%d/app_store_app", vppAppTitleID), updateAppReq, http.StatusOK) err = s.ds.UpdateHostPolicyCounts(ctx) @@ -16507,7 +16507,7 @@ func (s *integrationMDMTestSuite) TestVPPPolicyAutomationLabelScopingRetrigger() // Update to exclude_any: label3. Host has label1, label2, so the app is in scope again and // status should clear. - updateAppReq = &updateAppStoreAppRequest{TeamID: &team.ID, SelfService: false, LabelsExcludeAny: []string{label3.Name}} + updateAppReq = &updateAppStoreAppRequest{TeamID: &team.ID, SelfService: ptr.Bool(false), LabelsExcludeAny: []string{label3.Name}} s.Do("PATCH", fmt.Sprintf("/api/latest/fleet/software/titles/%d/app_store_app", vppAppTitleID), updateAppReq, http.StatusOK) err = s.ds.UpdateHostPolicyCounts(ctx) @@ -17748,7 +17748,7 @@ func (s *integrationMDMTestSuite) TestSoftwareCategories() { require.Contains(t, titleResponse.SoftwareTitle.AppStoreApp.Categories, cat3.Name) // update the app and change categories - updateAppReq := &updateAppStoreAppRequest{TeamID: nil, SelfService: true} + updateAppReq := &updateAppStoreAppRequest{TeamID: nil, SelfService: ptr.Bool(true)} updateAppReq.Categories = []string{cat1.Name, cat3.Name} var updateAppResp updateAppStoreAppResponse s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/software/titles/%d/app_store_app", vppAppTitleID), updateAppReq, http.StatusOK, &updateAppResp) diff --git a/server/service/integration_software_titles_test.go b/server/service/integration_software_titles_test.go index 6f8399f3d7..74a03a2c90 100644 --- a/server/service/integration_software_titles_test.go +++ b/server/service/integration_software_titles_test.go @@ -104,6 +104,7 @@ func (s *integrationMDMTestSuite) TestSoftwareTitleDisplayNames() { TeamID: &team.ID, DisplayName: ptr.String("RubyUpdate1"), SelfService: ptr.Bool(true), + Categories: []string{"Developer tools", "Browsers"}, }, http.StatusOK, "") activityData = fmt.Sprintf(` @@ -131,7 +132,6 @@ func (s *integrationMDMTestSuite) TestSoftwareTitleDisplayNames() { // Set display name to be empty s.updateSoftwareInstaller(t, &fleet.UpdateSoftwareInstallerPayload{ - SelfService: ptr.Bool(true), InstallScript: ptr.String("some install script"), PreInstallQuery: ptr.String("some pre install query"), PostInstallScript: ptr.String("some post install script"), @@ -144,6 +144,9 @@ func (s *integrationMDMTestSuite) TestSoftwareTitleDisplayNames() { // Entity display name is empty s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/software/titles/%d", titleID), getSoftwareTitleRequest{}, http.StatusOK, &stResp, "team_id", fmt.Sprint(team.ID)) s.Assert().Empty(stResp.SoftwareTitle.DisplayName) + // PATCH semantics, so we shouldn't overwrite self service + s.Assert().True(stResp.SoftwareTitle.SoftwarePackage.SelfService) + s.Assert().ElementsMatch([]string{"Developer tools", "Browsers"}, stResp.SoftwareTitle.SoftwarePackage.Categories) // List software titles display name is empty s.DoJSON("GET", "/api/latest/fleet/software/titles", listSoftwareTitlesRequest{}, http.StatusOK, &resp, "team_id", fmt.Sprint(team.ID)) @@ -173,11 +176,31 @@ func (s *integrationMDMTestSuite) TestSoftwareTitleDisplayNames() { LatestVersion: "1.0.0", } + // Create a label + clr := createLabelResponse{} + s.DoJSON("POST", "/api/latest/fleet/labels", createLabelRequest{ + LabelPayload: fleet.LabelPayload{ + Name: "foo", + }, + }, http.StatusOK, &clr) + + lbl1Name := clr.Label.Name + + clr = createLabelResponse{} + s.DoJSON("POST", "/api/latest/fleet/labels", createLabelRequest{ + LabelPayload: fleet.LabelPayload{ + Name: "bar", + }, + }, http.StatusOK, &clr) + + lbl2Name := clr.Label.Name + var addAppResp addAppStoreAppResponse addAppReq := &addAppStoreAppRequest{ - TeamID: &team.ID, - AppStoreID: includeAnyApp.AdamID, - SelfService: true, + TeamID: &team.ID, + AppStoreID: includeAnyApp.AdamID, + SelfService: true, + LabelsIncludeAny: []string{lbl1Name, lbl2Name}, } // Now add it for real @@ -185,7 +208,7 @@ func (s *integrationMDMTestSuite) TestSoftwareTitleDisplayNames() { macOSTitleID := addAppResp.TitleID - updateAppReq := &updateAppStoreAppRequest{TeamID: &team.ID, SelfService: false, DisplayName: ptr.String("MacOSAppStoreAppUpdated1")} + updateAppReq := &updateAppStoreAppRequest{TeamID: &team.ID, SelfService: ptr.Bool(false), DisplayName: ptr.String("MacOSAppStoreAppUpdated1")} var updateAppResp updateAppStoreAppResponse s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/software/titles/%d/app_store_app", macOSTitleID), updateAppReq, http.StatusOK, &updateAppResp) @@ -203,7 +226,7 @@ func (s *integrationMDMTestSuite) TestSoftwareTitleDisplayNames() { } } - updateAppReq = &updateAppStoreAppRequest{TeamID: &team.ID, SelfService: false, DisplayName: ptr.String("MacOSAppStoreAppUpdated2")} + updateAppReq = &updateAppStoreAppRequest{TeamID: &team.ID, SelfService: ptr.Bool(false), DisplayName: ptr.String("MacOSAppStoreAppUpdated2")} s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/software/titles/%d/app_store_app", macOSTitleID), updateAppReq, http.StatusOK, &updateAppResp) s.Assert().Equal(*updateAppReq.DisplayName, updateAppResp.AppStoreApp.DisplayName) @@ -223,7 +246,11 @@ func (s *integrationMDMTestSuite) TestSoftwareTitleDisplayNames() { existingDisplayName := *updateAppReq.DisplayName // Omitting the field is a no-op - updateAppReq = &updateAppStoreAppRequest{TeamID: &team.ID, SelfService: true} + updateAppReq = &updateAppStoreAppRequest{ + TeamID: &team.ID, + SelfService: ptr.Bool(true), + Categories: []string{"Developer tools", "Browsers"}, + } s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/software/titles/%d/app_store_app", macOSTitleID), updateAppReq, http.StatusOK, &updateAppResp) s.Assert().Equal(existingDisplayName, updateAppResp.AppStoreApp.DisplayName) @@ -240,7 +267,10 @@ func (s *integrationMDMTestSuite) TestSoftwareTitleDisplayNames() { } } - updateAppReq = &updateAppStoreAppRequest{TeamID: &team.ID, SelfService: false, DisplayName: ptr.String("")} + updateAppReq = &updateAppStoreAppRequest{ + TeamID: &team.ID, + DisplayName: ptr.String(""), + } s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/software/titles/%d/app_store_app", macOSTitleID), updateAppReq, http.StatusOK, &updateAppResp) s.Assert().Equal(*updateAppReq.DisplayName, updateAppResp.AppStoreApp.DisplayName) @@ -248,12 +278,24 @@ func (s *integrationMDMTestSuite) TestSoftwareTitleDisplayNames() { stResp = getSoftwareTitleResponse{} s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/software/titles/%d", macOSTitleID), getSoftwareTitleRequest{}, http.StatusOK, &stResp, "team_id", fmt.Sprint(team.ID)) s.Assert().Empty(stResp.SoftwareTitle.DisplayName) + // PATCH semantics, so we shouldn't overwrite self service or categories or labels + s.Assert().True(stResp.SoftwareTitle.AppStoreApp.SelfService) + s.Assert().ElementsMatch([]string{"Developer tools", "Browsers"}, stResp.SoftwareTitle.AppStoreApp.Categories) + s.Assert().ElementsMatch([]string{lbl1Name, lbl2Name}, func() []string { + var ret []string + for _, l := range stResp.SoftwareTitle.AppStoreApp.LabelsIncludeAny { + ret = append(ret, l.LabelName) + } + return ret + }()) // List software titles has display name s.DoJSON("GET", "/api/latest/fleet/software/titles", listSoftwareTitlesRequest{}, http.StatusOK, &resp, "team_id", fmt.Sprint(team.ID), "query", includeAnyApp.Name) for _, a := range resp.SoftwareTitles { if a.ID == macOSTitleID { s.Assert().Empty(a.DisplayName) + // PATCH semantics, so we shouldn't overwrite self service + s.Assert().True(*a.AppStoreApp.SelfService) } } @@ -289,6 +331,8 @@ func (s *integrationMDMTestSuite) TestSoftwareTitleDisplayNames() { TitleID: titleID, TeamID: &team.ID, DisplayName: ptr.String("InHouseAppUpdate2"), + SelfService: ptr.Bool(true), + Categories: []string{"Developer tools", "Browsers"}, }, http.StatusOK, "") // Entity has display name @@ -306,14 +350,16 @@ func (s *integrationMDMTestSuite) TestSoftwareTitleDisplayNames() { // Omitting the field is a no-op s.updateSoftwareInstaller(t, &fleet.UpdateSoftwareInstallerPayload{ - TitleID: titleID, - TeamID: &team.ID, - SelfService: ptr.Bool(true), + TitleID: titleID, + TeamID: &team.ID, }, http.StatusOK, "") // Entity has display name s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/software/titles/%d", titleID), getSoftwareTitleRequest{}, http.StatusOK, &stResp, "team_id", fmt.Sprint(team.ID)) s.Assert().Equal("InHouseAppUpdate2", stResp.SoftwareTitle.DisplayName) + // PATCH semantics, so we shouldn't overwrite self service or categories + s.Assert().True(stResp.SoftwareTitle.SoftwarePackage.SelfService) + s.Assert().ElementsMatch([]string{"Developer tools", "Browsers"}, stResp.SoftwareTitle.SoftwarePackage.Categories) // List software titles has display name s.DoJSON("GET", "/api/latest/fleet/software/titles", listSoftwareTitlesRequest{}, http.StatusOK, &resp, "team_id", fmt.Sprint(team.ID)) @@ -321,6 +367,8 @@ func (s *integrationMDMTestSuite) TestSoftwareTitleDisplayNames() { for _, t := range resp.SoftwareTitles { if t.ID == titleID { s.Assert().Equal("InHouseAppUpdate2", t.DisplayName) + // PATCH semantics, so we shouldn't overwrite self service + s.Assert().True(*t.SoftwarePackage.SelfService) } } diff --git a/server/service/integration_vpp_install_test.go b/server/service/integration_vpp_install_test.go index 2dbbf87d0c..3a5a09e849 100644 --- a/server/service/integration_vpp_install_test.go +++ b/server/service/integration_vpp_install_test.go @@ -966,7 +966,7 @@ func (s *integrationMDMTestSuite) TestVPPAppInstallVerification() { // Enable self-service for vpp app updateAppResp := updateAppStoreAppResponse{} s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/software/titles/%d/app_store_app", data.titleID), - &updateAppStoreAppRequest{TitleID: data.titleID, TeamID: &team.ID, SelfService: true}, http.StatusOK, &updateAppResp) + &updateAppStoreAppRequest{TitleID: data.titleID, TeamID: &team.ID, SelfService: ptr.Bool(true)}, http.StatusOK, &updateAppResp) // Install self-service app correctly s.DoRawWithHeaders("POST", fmt.Sprintf("/api/latest/fleet/device/%s/software/install/%d", data.host.UUID, data.titleID), nil, http.StatusAccepted, headers) @@ -1669,7 +1669,7 @@ func (s *integrationMDMTestSuite) TestInHouseAppSelfInstall() { } // installed activity is now created - activityData = fmt.Sprintf(`{"host_id": %d, "host_display_name": %q, "command_uuid": %q, "install_uuid": "", + activityData = fmt.Sprintf(`{"host_id": %d, "host_display_name": %q, "command_uuid": %q, "install_uuid": "", "software_title": "ipa_test", "software_package": "", "self_service": true, "status": "installed", "policy_id": null, "policy_name": null}`, iosHost.ID, iosHost.DisplayName(), installCmdUUID) s.lastActivityMatches(fleet.ActivityTypeInstalledSoftware{}.ActivityName(), activityData, 0) diff --git a/server/service/vpp.go b/server/service/vpp.go index 5fd08919d6..779bf6379a 100644 --- a/server/service/vpp.go +++ b/server/service/vpp.go @@ -99,7 +99,7 @@ func (svc *Service) AddAppStoreApp(ctx context.Context, _ *uint, _ fleet.VPPAppT type updateAppStoreAppRequest struct { TitleID uint `url:"title_id"` TeamID *uint `json:"team_id"` - SelfService bool `json:"self_service"` + SelfService *bool `json:"self_service"` LabelsIncludeAny []string `json:"labels_include_any"` LabelsExcludeAny []string `json:"labels_exclude_any"` Categories []string `json:"categories"` @@ -124,7 +124,7 @@ func updateAppStoreAppEndpoint(ctx context.Context, request interface{}, svc fle return updateAppStoreAppResponse{AppStoreApp: updatedApp}, nil } -func (svc *Service) UpdateAppStoreApp(ctx context.Context, titleID uint, teamID *uint, selfService bool, labelsIncludeAny, labelsExcludeAny, categories []string, displayName *string) (*fleet.VPPAppStoreApp, error) { +func (svc *Service) UpdateAppStoreApp(ctx context.Context, titleID uint, teamID *uint, selfService *bool, labelsIncludeAny, labelsExcludeAny, categories []string, displayName *string) (*fleet.VPPAppStoreApp, error) { // skipauth: No authorization check needed due to implementation returning // only license error. svc.authz.SkipAuthorization(ctx)