From 014f10fb46a5a6ce4e5be8dc2e6b28342e71f428 Mon Sep 17 00:00:00 2001 From: Ian Littman Date: Fri, 7 Mar 2025 11:36:17 -0600 Subject: [PATCH] Add experimental software title name update endpoint for titles with a bundle ID (#26938) For #26933. # Checklist for submitter If some of the following don't apply, delete the relevant line. - [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] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements) - [x] Added/updated automated tests - [x] A detailed QA plan exists on the associated ticket (if it isn't there, work with the product group's QA engineer to add it) - [x] Manual QA for all new/changed functionality --------- Co-authored-by: Rachael Shaw --- changes/26933-software-name-edit | 1 + docs/Contributing/API-for-contributors.md | 31 ++++++++++ server/authz/policy.rego | 7 +++ server/datastore/mysql/software_titles.go | 8 +++ .../datastore/mysql/software_titles_test.go | 44 +++++++++++++ server/fleet/datastore.go | 1 + server/fleet/service.go | 4 ++ server/mock/datastore_mock.go | 12 ++++ server/service/handler.go | 1 + server/service/software_titles.go | 47 ++++++++++++++ server/service/software_titles_test.go | 61 +++++++++++++++++++ 11 files changed, 217 insertions(+) create mode 100644 changes/26933-software-name-edit diff --git a/changes/26933-software-name-edit b/changes/26933-software-name-edit new file mode 100644 index 000000000000..4eb0aba93c22 --- /dev/null +++ b/changes/26933-software-name-edit @@ -0,0 +1 @@ +* Added `PATCH /api/latest/fleet/software/titles/:id/name` endpoint for cleaning up incorrect software titles for software that has a bundle ID. diff --git a/docs/Contributing/API-for-contributors.md b/docs/Contributing/API-for-contributors.md index fa549b729f34..450ea02f12e6 100644 --- a/docs/Contributing/API-for-contributors.md +++ b/docs/Contributing/API-for-contributors.md @@ -4005,6 +4005,37 @@ Run a live script and get results back (5 minute timeout). Live scripts only run ``` ## Software +### Update software title name + +`PATCH /api/v1/fleet/software/titles/:software_title_id/name` + +Only available for software titles that have a non-empty bundle ID, as titles without a bundle +ID will be added back as new rows on the next software ingest with the same name. Endpoint authorization limited +to global admins as this changes the software title's name across all teams. + +> **Experimental endpoint**. This endpoint is not guaranteed to continue to exist on future minor releases of Fleet. + +#### Parameters + +| Name | Type | In | Description | +|-------------------|---------|------|----------------------------------------------------| +| software_title_id | integer | path | **Required**. The ID of the software title to modify. | +| name | string | body | **Required**. The new name of the title. | + +#### Example + +`PATCH /api/v1/fleet/software/titles/1/name` + +```json +{ + "name": "2 Chrome 2 Furious.app" +} +``` + +##### Default response + +`Status: 205` + ### Batch-apply software _Available in Fleet Premium._ diff --git a/server/authz/policy.rego b/server/authz/policy.rego index 55096ded0057..5e802920cfd2 100644 --- a/server/authz/policy.rego +++ b/server/authz/policy.rego @@ -635,6 +635,13 @@ allow { action == read } +# Only global admins can modify software inventory (specifically software title names) +allow { + object.type == "software_inventory" + subject.global_role == admin + action == write +} + # Team admins, maintainers, observers and observer_plus can read all software in their teams. allow { not is_null(object.team_id) diff --git a/server/datastore/mysql/software_titles.go b/server/datastore/mysql/software_titles.go index 9778474ba149..2374fb62d46a 100644 --- a/server/datastore/mysql/software_titles.go +++ b/server/datastore/mysql/software_titles.go @@ -79,6 +79,14 @@ GROUP BY return &title, nil } +func (ds *Datastore) UpdateSoftwareTitleName(ctx context.Context, titleID uint, name string) error { + if _, err := ds.writer(ctx).ExecContext(ctx, "UPDATE software_titles SET name = ? WHERE id = ? AND bundle_identifier != ''", name, titleID); err != nil { + return ctxerr.Wrap(ctx, err, "update software title name") + } + + return nil +} + func (ds *Datastore) ListSoftwareTitles( ctx context.Context, opt fleet.SoftwareTitleListOptions, diff --git a/server/datastore/mysql/software_titles_test.go b/server/datastore/mysql/software_titles_test.go index 58b652ca680a..d5a93f30b9bd 100644 --- a/server/datastore/mysql/software_titles_test.go +++ b/server/datastore/mysql/software_titles_test.go @@ -31,6 +31,7 @@ func TestSoftwareTitles(t *testing.T) { {"ListSoftwareTitlesAllTeams", testListSoftwareTitlesAllTeams}, {"UploadedSoftwareExists", testUploadedSoftwareExists}, {"ListSoftwareTitlesVulnerabilityFilters", testListSoftwareTitlesVulnerabilityFilters}, + {"UpdateSoftwareTitleName", testUpdateSoftwareTitleName}, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { @@ -1700,3 +1701,46 @@ func testListSoftwareTitlesVulnerabilityFilters(t *testing.T, ds *Datastore) { }) } } + +func testUpdateSoftwareTitleName(t *testing.T, ds *Datastore) { + ctx := context.Background() + + tm, err := ds.NewTeam(ctx, &fleet.Team{Name: "Team Foo"}) + require.NoError(t, err) + user1 := test.NewUser(t, ds, "Alice", "alice@example.com", true) + + installer1, _, err := ds.MatchOrCreateSoftwareInstaller(ctx, &fleet.UploadSoftwareInstallerPayload{ + Title: "installer1", + Source: "apps", + InstallScript: "echo", + Filename: "installer1.pkg", + BundleIdentifier: "com.foo.installer1", + UserID: user1.ID, + ValidatedLabels: &fleet.LabelIdentsWithScope{}, + }) + require.NoError(t, err) + require.NotZero(t, installer1) + installer2, _, err := ds.MatchOrCreateSoftwareInstaller(ctx, &fleet.UploadSoftwareInstallerPayload{ + Title: "installer2", + Source: "programs", + InstallScript: "echo", + Filename: "installer2.msi", + TeamID: &tm.ID, + UserID: user1.ID, + ValidatedLabels: &fleet.LabelIdentsWithScope{}, + }) + require.NoError(t, err) + require.NotZero(t, installer2) + + // Changes name with bundle ID + require.NoError(t, ds.UpdateSoftwareTitleName(ctx, installer1, "A new name")) + title1, err := ds.SoftwareTitleByID(ctx, installer1, nil, fleet.TeamFilter{User: user1}) + require.NoError(t, err) + require.Equal(t, "A new name", title1.Name) + + // Doesn't change name with no bundle ID + require.NoError(t, ds.UpdateSoftwareTitleName(ctx, installer2, "A newer name")) + title2, err := ds.SoftwareTitleByID(ctx, installer2, &tm.ID, fleet.TeamFilter{User: user1}) + require.NoError(t, err) + require.Equal(t, "installer2", title2.Name) +} diff --git a/server/fleet/datastore.go b/server/fleet/datastore.go index 7fb159c530c7..b3c8e07374fa 100644 --- a/server/fleet/datastore.go +++ b/server/fleet/datastore.go @@ -566,6 +566,7 @@ type Datastore interface { ListSoftwareTitles(ctx context.Context, opt SoftwareTitleListOptions, tmFilter TeamFilter) ([]SoftwareTitleListResult, int, *PaginationMetadata, error) SoftwareTitleByID(ctx context.Context, id uint, teamID *uint, tmFilter TeamFilter) (*SoftwareTitle, error) + UpdateSoftwareTitleName(ctx context.Context, id uint, name string) error // InsertSoftwareInstallRequest tracks a new request to install the provided // software installer in the host. It returns the auto-generated installation diff --git a/server/fleet/service.go b/server/fleet/service.go index a05f7684509b..5f959b08dc04 100644 --- a/server/fleet/service.go +++ b/server/fleet/service.go @@ -435,6 +435,10 @@ type Service interface { // the specified host. ListHostSoftware(ctx context.Context, hostID uint, opts HostSoftwareTitleListOptions) ([]*HostSoftwareWithInstaller, *PaginationMetadata, error) + // UpdateSoftwareName updates the name of a software title if the title is uniquely identified by bundle ID + // rather than by name + UpdateSoftwareName(ctx context.Context, titleID uint, name string) error + // ListHostCertificates lists the certificates installed on the specified host. ListHostCertificates(ctx context.Context, hostID uint, opts ListOptions) ([]*HostCertificatePayload, *PaginationMetadata, error) diff --git a/server/mock/datastore_mock.go b/server/mock/datastore_mock.go index 328c0c791def..6d8ee47022e3 100644 --- a/server/mock/datastore_mock.go +++ b/server/mock/datastore_mock.go @@ -432,6 +432,8 @@ type ListSoftwareTitlesFunc func(ctx context.Context, opt fleet.SoftwareTitleLis type SoftwareTitleByIDFunc func(ctx context.Context, id uint, teamID *uint, tmFilter fleet.TeamFilter) (*fleet.SoftwareTitle, error) +type UpdateSoftwareTitleNameFunc func(ctx context.Context, id uint, name string) error + type InsertSoftwareInstallRequestFunc func(ctx context.Context, hostID uint, softwareInstallerID uint, opts fleet.HostSoftwareInstallOptions) (string, error) type InsertSoftwareUninstallRequestFunc func(ctx context.Context, executionID string, hostID uint, softwareInstallerID uint) error @@ -1872,6 +1874,9 @@ type DataStore struct { SoftwareTitleByIDFunc SoftwareTitleByIDFunc SoftwareTitleByIDFuncInvoked bool + UpdateSoftwareTitleNameFunc UpdateSoftwareTitleNameFunc + UpdateSoftwareTitleNameFuncInvoked bool + InsertSoftwareInstallRequestFunc InsertSoftwareInstallRequestFunc InsertSoftwareInstallRequestFuncInvoked bool @@ -4546,6 +4551,13 @@ func (s *DataStore) SoftwareTitleByID(ctx context.Context, id uint, teamID *uint return s.SoftwareTitleByIDFunc(ctx, id, teamID, tmFilter) } +func (s *DataStore) UpdateSoftwareTitleName(ctx context.Context, id uint, name string) error { + s.mu.Lock() + s.UpdateSoftwareTitleNameFuncInvoked = true + s.mu.Unlock() + return s.UpdateSoftwareTitleNameFunc(ctx, id, name) +} + func (s *DataStore) InsertSoftwareInstallRequest(ctx context.Context, hostID uint, softwareInstallerID uint, opts fleet.HostSoftwareInstallOptions) (string, error) { s.mu.Lock() s.InsertSoftwareInstallRequestFuncInvoked = true diff --git a/server/service/handler.go b/server/service/handler.go index 4c139f83dc00..f89c497e0d1b 100644 --- a/server/service/handler.go +++ b/server/service/handler.go @@ -342,6 +342,7 @@ func attachFleetAPIRoutes(r *mux.Router, svc fleet.Service, config config.FleetC ue.POST("/api/_version_/fleet/software/titles/{title_id:[0-9]+}/package/token", getSoftwareInstallerTokenEndpoint, getSoftwareInstallerRequest{}) ue.POST("/api/_version_/fleet/software/package", uploadSoftwareInstallerEndpoint, uploadSoftwareInstallerRequest{}) + ue.PATCH("/api/_version_/fleet/software/titles/{id:[0-9]+}/name", updateSoftwareNameEndpoint, updateSoftwareNameRequest{}) ue.PATCH("/api/_version_/fleet/software/titles/{id:[0-9]+}/package", updateSoftwareInstallerEndpoint, updateSoftwareInstallerRequest{}) ue.DELETE("/api/_version_/fleet/software/titles/{title_id:[0-9]+}/available_for_install", deleteSoftwareInstallerEndpoint, deleteSoftwareInstallerRequest{}) ue.GET("/api/_version_/fleet/software/install/{install_uuid}/results", getSoftwareInstallResultsEndpoint, diff --git a/server/service/software_titles.go b/server/service/software_titles.go index 0b07ffd25719..da6c9b2472b4 100644 --- a/server/service/software_titles.go +++ b/server/service/software_titles.go @@ -220,3 +220,50 @@ func (svc *Service) SoftwareTitleByID(ctx context.Context, id uint, teamID *uint return software, nil } + +///////////////////////////////////////////////////////////////////////////////// +// Update a software title's name +///////////////////////////////////////////////////////////////////////////////// + +type updateSoftwareNameRequest struct { + ID uint `url:"id"` + Name string `json:"name"` +} + +type updateSoftwareNameResponse struct { + Err error `json:"error,omitempty"` +} + +func (r updateSoftwareNameResponse) Error() error { return r.Err } +func (r updateSoftwareNameResponse) Status() int { return http.StatusResetContent } + +func updateSoftwareNameEndpoint(ctx context.Context, request interface{}, svc fleet.Service) (fleet.Errorer, error) { + req := request.(*updateSoftwareNameRequest) + return updateSoftwareNameResponse{Err: svc.UpdateSoftwareName(ctx, req.ID, req.Name)}, nil +} + +func (svc *Service) UpdateSoftwareName(ctx context.Context, titleID uint, name string) error { + if err := svc.authz.Authorize(ctx, &fleet.AuthzSoftwareInventory{}, fleet.ActionWrite); err != nil { + return err + } + vc, ok := viewer.FromContext(ctx) + if !ok { + return fleet.ErrNoContext + } + + // get software by id including team_id data from software_title_host_counts + software, err := svc.ds.SoftwareTitleByID(ctx, titleID, nil, fleet.TeamFilter{ + User: vc.User, + }) + if err != nil { + return ctxerr.Wrap(ctx, err, "getting software title by id") + } + if software.BundleIdentifier == nil || *software.BundleIdentifier == "" { + return fleet.NewInvalidArgumentError("id", "only titles with a bundle ID can have their name modified") + } + if name == "" { + return fleet.NewInvalidArgumentError("name", "cannot be empty") + } + + return svc.ds.UpdateSoftwareTitleName(ctx, titleID, name) +} diff --git a/server/service/software_titles_test.go b/server/service/software_titles_test.go index 599cba177471..d16f58941821 100644 --- a/server/service/software_titles_test.go +++ b/server/service/software_titles_test.go @@ -22,6 +22,12 @@ func TestServiceSoftwareTitlesAuth(t *testing.T) { return &fleet.SoftwareTitle{}, nil } ds.TeamExistsFunc = func(ctx context.Context, teamID uint) (bool, error) { return true, nil } + ds.SoftwareTitleByIDFunc = func(ctx context.Context, id uint, teamID *uint, tmFilter fleet.TeamFilter) (*fleet.SoftwareTitle, error) { + return &fleet.SoftwareTitle{BundleIdentifier: ptr.String("foo")}, nil + } + ds.UpdateSoftwareTitleNameFunc = func(ctx context.Context, id uint, name string) error { + return nil + } svc, ctx := newTestService(t, ds, nil, nil) @@ -30,6 +36,7 @@ func TestServiceSoftwareTitlesAuth(t *testing.T) { user *fleet.User shouldFailGlobalRead bool shouldFailTeamRead bool + shouldFailWrite bool }{ { name: "global-admin", @@ -39,6 +46,7 @@ func TestServiceSoftwareTitlesAuth(t *testing.T) { }, shouldFailGlobalRead: false, shouldFailTeamRead: false, + shouldFailWrite: false, }, { name: "global-maintainer", @@ -48,6 +56,7 @@ func TestServiceSoftwareTitlesAuth(t *testing.T) { }, shouldFailGlobalRead: false, shouldFailTeamRead: false, + shouldFailWrite: true, }, { name: "global-observer", @@ -57,6 +66,7 @@ func TestServiceSoftwareTitlesAuth(t *testing.T) { }, shouldFailGlobalRead: false, shouldFailTeamRead: false, + shouldFailWrite: true, }, { name: "team-admin-belongs-to-team", @@ -69,6 +79,7 @@ func TestServiceSoftwareTitlesAuth(t *testing.T) { }, shouldFailGlobalRead: true, shouldFailTeamRead: false, + shouldFailWrite: true, }, { name: "team-maintainer-belongs-to-team", @@ -81,6 +92,7 @@ func TestServiceSoftwareTitlesAuth(t *testing.T) { }, shouldFailGlobalRead: true, shouldFailTeamRead: false, + shouldFailWrite: true, }, { name: "team-observer-belongs-to-team", @@ -93,6 +105,7 @@ func TestServiceSoftwareTitlesAuth(t *testing.T) { }, shouldFailGlobalRead: true, shouldFailTeamRead: false, + shouldFailWrite: true, }, { name: "team-admin-does-not-belong-to-team", @@ -105,6 +118,7 @@ func TestServiceSoftwareTitlesAuth(t *testing.T) { }, shouldFailGlobalRead: true, shouldFailTeamRead: true, + shouldFailWrite: true, }, { name: "team-maintainer-does-not-belong-to-team", @@ -117,6 +131,7 @@ func TestServiceSoftwareTitlesAuth(t *testing.T) { }, shouldFailGlobalRead: true, shouldFailTeamRead: true, + shouldFailWrite: true, }, { name: "team-observer-does-not-belong-to-team", @@ -129,6 +144,7 @@ func TestServiceSoftwareTitlesAuth(t *testing.T) { }, shouldFailGlobalRead: true, shouldFailTeamRead: true, + shouldFailWrite: true, }, } { t.Run(tc.name, func(t *testing.T) { @@ -157,6 +173,51 @@ func TestServiceSoftwareTitlesAuth(t *testing.T) { // Get a software title for a team _, err = svc.SoftwareTitleByID(ctx, 1, ptr.Uint(1)) checkAuthErr(t, tc.shouldFailTeamRead, err) + + // Update a software title's name + err = svc.UpdateSoftwareName(ctx, 1, "2 Chrome 2 Furious") + checkAuthErr(t, tc.shouldFailWrite, err) }) } } + +func TestSoftwareNameUpdate(t *testing.T) { + ds := new(mock.Store) + ds.SoftwareTitleByIDFunc = func(ctx context.Context, id uint, teamID *uint, tmFilter fleet.TeamFilter) (*fleet.SoftwareTitle, error) { + return nil, ¬FoundError{} + } + + svc, ctx := newTestService(t, ds, nil, nil) + ctx = viewer.NewContext(ctx, viewer.Viewer{User: &fleet.User{ + ID: 1, + GlobalRole: ptr.String(fleet.RoleAdmin), + }}) + + // Title not found + err := svc.UpdateSoftwareName(ctx, 1, "2 Chrome 2 Furious") + require.ErrorContains(t, err, "not found") + require.False(t, ds.UpdateHostSoftwareFuncInvoked) + + // Title found but doesn't have a bundle ID + title := &fleet.SoftwareTitle{} + ds.SoftwareTitleByIDFunc = func(ctx context.Context, id uint, teamID *uint, tmFilter fleet.TeamFilter) (*fleet.SoftwareTitle, error) { + return title, nil + } + err = svc.UpdateSoftwareName(ctx, 1, "2 Chrome 2 Furious") + require.ErrorContains(t, err, "bundle") + require.False(t, ds.UpdateHostSoftwareFuncInvoked) + + // Title found with bundle ID but user didn't provide a name + title = &fleet.SoftwareTitle{BundleIdentifier: ptr.String("foo")} + err = svc.UpdateSoftwareName(ctx, 1, "") + require.ErrorContains(t, err, "name") + require.False(t, ds.UpdateHostSoftwareFuncInvoked) + + // Success case + ds.UpdateSoftwareTitleNameFunc = func(ctx context.Context, id uint, name string) error { + return nil + } + err = svc.UpdateSoftwareName(ctx, 1, "2 Chrome 2 Furious") + require.NoError(t, err) + require.True(t, ds.UpdateSoftwareTitleNameFuncInvoked) +}