diff --git a/changes/20515-delete-vpp-app b/changes/20515-delete-vpp-app new file mode 100644 index 0000000000..49599edf94 --- /dev/null +++ b/changes/20515-delete-vpp-app @@ -0,0 +1,2 @@ +* Added support to delete a VPP app from a team in `DELETE /software/titles/:software_title_id/available_for_install`. +* Fixed path that was incorrect for the download software installer package endpoint `GET /software/titles/:software_title_id/package`. diff --git a/ee/server/service/software_installers.go b/ee/server/service/software_installers.go index 988e2d4df5..f8f4faf32f 100644 --- a/ee/server/service/software_installers.go +++ b/ee/server/service/software_installers.go @@ -85,15 +85,60 @@ func (svc *Service) DeleteSoftwareInstaller(ctx context.Context, titleID uint, t return fleet.NewInvalidArgumentError("team_id", "is required and can't be zero") } + // we authorize with SoftwareInstaller here, but it uses the same AuthzType + // as VPPApp, so this is correct for both software installers and VPP apps. if err := svc.authz.Authorize(ctx, &fleet.SoftwareInstaller{TeamID: teamID}, fleet.ActionWrite); err != nil { return err } + // first, look for a software installer meta, err := svc.ds.GetSoftwareInstallerMetadataByTeamAndTitleID(ctx, teamID, titleID, false) if err != nil { + if fleet.IsNotFound(err) { + // no software installer, look for a VPP app + meta, err := svc.ds.GetVPPAppMetadataByTeamAndTitleID(ctx, teamID, titleID) + if err != nil { + return ctxerr.Wrap(ctx, err, "getting software app metadata") + } + return svc.deleteVPPApp(ctx, teamID, meta) + } return ctxerr.Wrap(ctx, err, "getting software installer metadata") } + return svc.deleteSoftwareInstaller(ctx, meta) +} +func (svc *Service) deleteVPPApp(ctx context.Context, teamID *uint, meta *fleet.VPPAppStoreApp) error { + vc, ok := viewer.FromContext(ctx) + if !ok { + return fleet.ErrNoContext + } + + if err := svc.ds.DeleteVPPAppFromTeam(ctx, teamID, meta.AppStoreID); err != nil { + return ctxerr.Wrap(ctx, err, "deleting VPP app") + } + + var teamName *string + if teamID != nil { + t, err := svc.ds.Team(ctx, *teamID) + if err != nil { + return ctxerr.Wrap(ctx, err, "getting team name for deleted vpp app") + } + teamName = &t.Name + } + + if err := svc.NewActivity(ctx, vc.User, fleet.ActivityDeletedAppStoreApp{ + AppStoreID: meta.AppStoreID, + SoftwareTitle: meta.Name, + TeamName: teamName, + TeamID: teamID, + }); err != nil { + return ctxerr.Wrap(ctx, err, "creating activity for deleted vpp app") + } + + return nil +} + +func (svc *Service) deleteSoftwareInstaller(ctx context.Context, meta *fleet.SoftwareInstaller) error { vc, ok := viewer.FromContext(ctx) if !ok { return fleet.ErrNoContext diff --git a/ee/server/service/vpp.go b/ee/server/service/vpp.go index eb3ba1ad1b..d02f89e711 100644 --- a/ee/server/service/vpp.go +++ b/ee/server/service/vpp.go @@ -155,6 +155,7 @@ func (svc *Service) AddAppStoreApp(ctx context.Context, teamID *uint, adamID str BundleIdentifier: assetMD.BundleID, IconURL: assetMD.ArtworkURL, Name: assetMD.TrackName, + LatestVersion: assetMD.Version, } if err := svc.ds.InsertVPPAppWithTeam(ctx, app, teamID); err != nil { return ctxerr.Wrap(ctx, err, "writing VPP app to db") diff --git a/server/datastore/mysql/vpp.go b/server/datastore/mysql/vpp.go index 38163c972b..88fc13ae27 100644 --- a/server/datastore/mysql/vpp.go +++ b/server/datastore/mysql/vpp.go @@ -141,6 +141,8 @@ func (ds *Datastore) BatchInsertVPPApps(ctx context.Context, apps []*fleet.VPPAp func (ds *Datastore) InsertVPPAppWithTeam(ctx context.Context, app *fleet.VPPApp, teamID *uint) error { return ds.withRetryTxx(ctx, func(tx sqlx.ExtContext) error { + // TODO: we should not just create a software title, I think we should try + // to match to an existing one (i.e. getOrCreate logic)? titleID, err := insertSoftwareTitleForVPPApp(ctx, tx, app) if err != nil { return err @@ -244,3 +246,22 @@ func insertSoftwareTitleForVPPApp(ctx context.Context, tx sqlx.ExtContext, app * return uint(id), nil } + +func (ds *Datastore) DeleteVPPAppFromTeam(ctx context.Context, teamID *uint, adamID string) error { + const stmt = `DELETE FROM vpp_apps_teams WHERE global_or_team_id = ? AND adam_id = ?` + + var globalOrTeamID uint + if teamID != nil { + globalOrTeamID = *teamID + } + res, err := ds.writer(ctx).ExecContext(ctx, stmt, globalOrTeamID, adamID) + if err != nil { + return ctxerr.Wrap(ctx, err, "delete VPP app from team") + } + + rows, _ := res.RowsAffected() + if rows == 0 { + return notFound("VPPApp").WithMessage(fmt.Sprintf("adam id %s for team id %d", adamID, globalOrTeamID)) + } + return nil +} diff --git a/server/datastore/mysql/vpp_test.go b/server/datastore/mysql/vpp_test.go index 75ac98f134..9a13374ded 100644 --- a/server/datastore/mysql/vpp_test.go +++ b/server/datastore/mysql/vpp_test.go @@ -96,6 +96,35 @@ func testVPPAppMetadata(t *testing.T, ds *Datastore) { require.NoError(t, err) require.Equal(t, &fleet.VPPAppStoreApp{Name: "vpp3", AppStoreID: vpp3}, meta) + // delete vpp1 + err = ds.DeleteVPPAppFromTeam(ctx, nil, vpp1) + require.NoError(t, err) + // it is now not found + _, err = ds.GetVPPAppMetadataByTeamAndTitleID(ctx, nil, titleID1) + require.Error(t, err) + require.ErrorAs(t, err, &nfe) + // vpp3 (also in no team) is left untouched + meta, err = ds.GetVPPAppMetadataByTeamAndTitleID(ctx, nil, titleID3) + require.NoError(t, err) + require.Equal(t, &fleet.VPPAppStoreApp{Name: "vpp3", AppStoreID: vpp3}, meta) + + // delete vpp2 for team1 + err = ds.DeleteVPPAppFromTeam(ctx, &team1.ID, vpp2) + require.NoError(t, err) + // it is now not found for team1 + _, err = ds.GetVPPAppMetadataByTeamAndTitleID(ctx, &team1.ID, titleID2) + require.Error(t, err) + require.ErrorAs(t, err, &nfe) + // but still found for team2 + meta, err = ds.GetVPPAppMetadataByTeamAndTitleID(ctx, &team2.ID, titleID2) + require.NoError(t, err) + require.Equal(t, &fleet.VPPAppStoreApp{Name: "vpp2", AppStoreID: vpp2}, meta) + + // delete vpp1 again fails, not found + err = ds.DeleteVPPAppFromTeam(ctx, nil, vpp1) + require.Error(t, err) + require.ErrorAs(t, err, &nfe) + // delete the software title ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { _, err := q.ExecContext(ctx, "DELETE FROM software_titles WHERE id = ?", titleID3) @@ -107,6 +136,7 @@ func testVPPAppMetadata(t *testing.T, ds *Datastore) { require.Error(t, err) require.ErrorAs(t, err, &nfe) require.Nil(t, meta) + } func testVPPAppStatus(t *testing.T, ds *Datastore) { diff --git a/server/fleet/activities.go b/server/fleet/activities.go index 849a78953b..1af7e9e41f 100644 --- a/server/fleet/activities.go +++ b/server/fleet/activities.go @@ -1649,9 +1649,10 @@ func (a ActivityAddedAppStoreApp) Documentation() (activity string, details stri } type ActivityDeletedAppStoreApp struct { - SoftwareTitle string `json:"software_title"` - AppStoreID string `json:"app_store_id"` - TeamName string `json:"team_name"` + SoftwareTitle string `json:"software_title"` + AppStoreID string `json:"app_store_id"` + TeamName *string `json:"team_name"` + TeamID *uint `json:"team_id"` } func (a ActivityDeletedAppStoreApp) ActivityName() string { diff --git a/server/fleet/datastore.go b/server/fleet/datastore.go index ca13c5ee86..f002479d91 100644 --- a/server/fleet/datastore.go +++ b/server/fleet/datastore.go @@ -1561,6 +1561,10 @@ type Datastore interface { // DeleteSoftwareInstaller deletes the software installer corresponding to the id. DeleteSoftwareInstaller(ctx context.Context, id uint) error + // DeleteVPPAppFromTeam deletes the VPP app corresponding to the adamID from + // the provided team. + DeleteVPPAppFromTeam(ctx context.Context, teamID *uint, adamID string) error + // GetSummaryHostSoftwareInstalls returns the software install summary for // the given software installer id. GetSummaryHostSoftwareInstalls(ctx context.Context, installerID uint) (*SoftwareInstallerStatusSummary, error) diff --git a/server/mock/datastore_mock.go b/server/mock/datastore_mock.go index 65527b990d..e137130ca5 100644 --- a/server/mock/datastore_mock.go +++ b/server/mock/datastore_mock.go @@ -981,6 +981,8 @@ type GetVPPAppMetadataByTeamAndTitleIDFunc func(ctx context.Context, teamID *uin type DeleteSoftwareInstallerFunc func(ctx context.Context, id uint) error +type DeleteVPPAppFromTeamFunc func(ctx context.Context, teamID *uint, adamID string) error + type GetSummaryHostSoftwareInstallsFunc func(ctx context.Context, installerID uint) (*fleet.SoftwareInstallerStatusSummary, error) type GetSummaryHostVPPAppInstallsFunc func(ctx context.Context, teamID *uint, adamID string) (*fleet.VPPAppStatusSummary, error) @@ -2443,6 +2445,9 @@ type DataStore struct { DeleteSoftwareInstallerFunc DeleteSoftwareInstallerFunc DeleteSoftwareInstallerFuncInvoked bool + DeleteVPPAppFromTeamFunc DeleteVPPAppFromTeamFunc + DeleteVPPAppFromTeamFuncInvoked bool + GetSummaryHostSoftwareInstallsFunc GetSummaryHostSoftwareInstallsFunc GetSummaryHostSoftwareInstallsFuncInvoked bool @@ -5840,6 +5845,13 @@ func (s *DataStore) DeleteSoftwareInstaller(ctx context.Context, id uint) error return s.DeleteSoftwareInstallerFunc(ctx, id) } +func (s *DataStore) DeleteVPPAppFromTeam(ctx context.Context, teamID *uint, adamID string) error { + s.mu.Lock() + s.DeleteVPPAppFromTeamFuncInvoked = true + s.mu.Unlock() + return s.DeleteVPPAppFromTeamFunc(ctx, teamID, adamID) +} + func (s *DataStore) GetSummaryHostSoftwareInstalls(ctx context.Context, installerID uint) (*fleet.SoftwareInstallerStatusSummary, error) { s.mu.Lock() s.GetSummaryHostSoftwareInstallsFuncInvoked = true diff --git a/server/service/handler.go b/server/service/handler.go index 2071390583..a9f4ee85ca 100644 --- a/server/service/handler.go +++ b/server/service/handler.go @@ -370,9 +370,9 @@ func attachFleetAPIRoutes(r *mux.Router, svc fleet.Service, config config.FleetC ue.POST("/api/_version_/fleet/hosts/{host_id:[0-9]+}/software/install/{software_title_id:[0-9]+}", installSoftwareTitleEndpoint, installSoftwareRequest{}) // Sofware installers - ue.GET("/api/_version_/fleet/software/{title_id:[0-9]+}/package", getSoftwareInstallerEndpoint, getSoftwareInstallerRequest{}) + ue.GET("/api/_version_/fleet/software/titles/{title_id:[0-9]+}/package", getSoftwareInstallerEndpoint, getSoftwareInstallerRequest{}) ue.POST("/api/_version_/fleet/software/package", uploadSoftwareInstallerEndpoint, uploadSoftwareInstallerRequest{}) - ue.DELETE("/api/_version_/fleet/software/{title_id:[0-9]+}/package", deleteSoftwareInstallerEndpoint, deleteSoftwareInstallerRequest{}) + ue.DELETE("/api/_version_/fleet/software/titles/{title_id:[0-9]+}/available_for_install", deleteSoftwareInstallerEndpoint, deleteSoftwareInstallerRequest{}) ue.GET("/api/_version_/fleet/software/install/results/{install_uuid}", getSoftwareInstallResultsEndpoint, getSoftwareInstallResultsRequest{}) ue.POST("/api/_version_/fleet/software/batch", batchSetSoftwareInstallersEndpoint, batchSetSoftwareInstallersRequest{}) diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go index 4d1217b95b..d0c259973a 100644 --- a/server/service/integration_enterprise_test.go +++ b/server/service/integration_enterprise_test.go @@ -9460,10 +9460,10 @@ func (s *integrationEnterpriseTestSuite) TestSoftwareInstallerUploadDownloadAndD }, http.StatusUnauthorized) // download the installer - s.Do("GET", fmt.Sprintf("/api/latest/fleet/software/%d/package?alt=media", titleID), nil, http.StatusBadRequest) + s.Do("GET", fmt.Sprintf("/api/latest/fleet/software/titles/%d/package?alt=media", titleID), nil, http.StatusBadRequest) // delete the installer - s.Do("DELETE", fmt.Sprintf("/api/latest/fleet/software/%d/package", titleID), nil, http.StatusBadRequest) + s.Do("DELETE", fmt.Sprintf("/api/latest/fleet/software/titles/%d/available_for_install", titleID), nil, http.StatusBadRequest) }) t.Run("create team software installer", func(t *testing.T) { @@ -9499,7 +9499,7 @@ func (s *integrationEnterpriseTestSuite) TestSoftwareInstallerUploadDownloadAndD s.uploadSoftwareInstaller(payload, http.StatusConflict, "already exists") // download the installer - r := s.Do("GET", fmt.Sprintf("/api/latest/fleet/software/%d/package?alt=media", titleID), nil, http.StatusOK, "team_id", fmt.Sprintf("%d", *payload.TeamID)) + r := s.Do("GET", fmt.Sprintf("/api/latest/fleet/software/titles/%d/package?alt=media", titleID), nil, http.StatusOK, "team_id", fmt.Sprintf("%d", *payload.TeamID)) checkDownloadResponse(t, r, payload.Filename) // create an orbit host that is not in the team @@ -9530,13 +9530,13 @@ func (s *integrationEnterpriseTestSuite) TestSoftwareInstallerUploadDownloadAndD checkDownloadResponse(t, r, payload.Filename) // delete the installer - s.Do("DELETE", fmt.Sprintf("/api/latest/fleet/software/%d/package", titleID), nil, http.StatusNoContent, "team_id", fmt.Sprintf("%d", *payload.TeamID)) + s.Do("DELETE", fmt.Sprintf("/api/latest/fleet/software/titles/%d/available_for_install", titleID), nil, http.StatusNoContent, "team_id", fmt.Sprintf("%d", *payload.TeamID)) // check activity s.lastActivityOfTypeMatches(fleet.ActivityTypeDeletedSoftware{}.ActivityName(), fmt.Sprintf(`{"software_title": "ruby", "software_package": "ruby.deb", "team_name": "%s", "team_id": %d, "self_service": true}`, createTeamResp.Team.Name, createTeamResp.Team.ID), 0) // download the installer, not found anymore - s.Do("GET", fmt.Sprintf("/api/latest/fleet/software/%d/package?alt=media", titleID), nil, http.StatusNotFound, "team_id", fmt.Sprintf("%d", *payload.TeamID)) + s.Do("GET", fmt.Sprintf("/api/latest/fleet/software/titles/%d/package?alt=media", titleID), nil, http.StatusNotFound, "team_id", fmt.Sprintf("%d", *payload.TeamID)) }) } @@ -10762,17 +10762,19 @@ func (s *integrationMDMTestSuite) TestVPPApps() { require.False(t, appResp.AppStoreApps[1].Added) // Add an app store app to team 1 + addedApp := appResp.AppStoreApps[0] var addAppResp addAppStoreAppResponse - s.DoJSON("POST", "/api/latest/fleet/software/app_store_apps", &addAppStoreAppRequest{TeamID: &team.ID, AppStoreID: appResp.AppStoreApps[0].AdamID}, http.StatusOK, &addAppResp) - s.lastActivityMatches(fleet.ActivityAddedAppStoreApp{}.ActivityName(), fmt.Sprintf(`{"team_name": "%s", "software_title": "%s", "app_store_id": "%s", "team_id": %d}`, team.Name, appResp.AppStoreApps[0].Name, appResp.AppStoreApps[0].AdamID, team.ID), 0) + s.DoJSON("POST", "/api/latest/fleet/software/app_store_apps", &addAppStoreAppRequest{TeamID: &team.ID, AppStoreID: addedApp.AdamID}, http.StatusOK, &addAppResp) + s.lastActivityMatches(fleet.ActivityAddedAppStoreApp{}.ActivityName(), fmt.Sprintf(`{"team_name": "%s", "software_title": "%s", "app_store_id": "%s", "team_id": %d}`, team.Name, addedApp.Name, addedApp.AdamID, team.ID), 0) // Add an app store app to non-existent team - s.DoJSON("POST", "/api/latest/fleet/software/app_store_apps", &addAppStoreAppRequest{TeamID: ptr.Uint(9999), AppStoreID: appResp.AppStoreApps[0].AdamID}, http.StatusNotFound, &addAppResp) + s.DoJSON("POST", "/api/latest/fleet/software/app_store_apps", &addAppStoreAppRequest{TeamID: ptr.Uint(9999), AppStoreID: addedApp.AdamID}, http.StatusNotFound, &addAppResp) // Add an installer // Verify that we are not able to add the VPP app for that same app whose installer we just added // Now we should be filtering out the app we added to team 1 + appResp = getAppStoreAppsResponse{} s.DoJSON("GET", "/api/latest/fleet/software/app_store_apps", &getAppStoreAppsRequest{}, http.StatusOK, &appResp, "team_id", strconv.Itoa(int(team.ID))) require.NoError(t, appResp.Err) require.Len(t, appResp.AppStoreApps, 1) @@ -10784,6 +10786,27 @@ func (s *integrationMDMTestSuite) TestVPPApps() { require.Equal(t, "2.0.0", appResp.AppStoreApps[0].LatestVersion) require.False(t, appResp.AppStoreApps[0].Added) + // list the software titles for that team, to get the title id of the VPP app + var listSw listSoftwareTitlesResponse + s.DoJSON("GET", "/api/latest/fleet/software/titles", nil, http.StatusOK, &listSw, "team_id", fmt.Sprint(team.ID), "available_for_install", "true") + require.Len(t, listSw.SoftwareTitles, 1) + titleID := listSw.SoftwareTitles[0].ID + + // delete the app store app for team 1 + s.Do("DELETE", fmt.Sprintf("/api/latest/fleet/software/titles/%d/available_for_install", titleID), nil, http.StatusNoContent, "team_id", fmt.Sprint(team.ID)) + s.lastActivityMatches(fleet.ActivityDeletedAppStoreApp{}.ActivityName(), fmt.Sprintf(`{"team_name": "%s", "software_title": "%s", "app_store_id": "%s", "team_id": %d}`, team.Name, addedApp.Name, addedApp.AdamID, team.ID), 0) + + // deleting it again fails, not found + s.Do("DELETE", fmt.Sprintf("/api/latest/fleet/software/titles/%d/available_for_install", titleID), nil, http.StatusNotFound, "team_id", fmt.Sprint(team.ID)) + + // get the list of available apps, returns both apps now + appResp = getAppStoreAppsResponse{} + s.DoJSON("GET", "/api/latest/fleet/software/app_store_apps", nil, http.StatusOK, &appResp, "team_id", fmt.Sprint(team.ID)) + require.NoError(t, appResp.Err) + require.Len(t, appResp.AppStoreApps, 2) + require.Equal(t, "App 1", appResp.AppStoreApps[0].Name) + require.Equal(t, "App 2", appResp.AppStoreApps[1].Name) + // Delete VPP token and check that it's not appearing anymore s.Do("DELETE", "/api/latest/fleet/mdm/apple/vpp_token", &deleteMDMAppleVPPTokenRequest{}, http.StatusNoContent) s.DoJSON("GET", "/api/latest/fleet/vpp", &getMDMAppleVPPTokenRequest{}, http.StatusNotFound, &resp)