diff --git a/changes/fix-hosts-software-title-filter-response b/changes/fix-hosts-software-title-filter-response new file mode 100644 index 0000000000..4f34155acc --- /dev/null +++ b/changes/fix-hosts-software-title-filter-response @@ -0,0 +1 @@ +* Fixed the hosts list endpoint sometimes returning software title details that didn't match the applied filter. diff --git a/server/datastore/mysql/software_titles.go b/server/datastore/mysql/software_titles.go index 050e7a4fbd..271f4a6a60 100644 --- a/server/datastore/mysql/software_titles.go +++ b/server/datastore/mysql/software_titles.go @@ -132,31 +132,62 @@ GROUP BY return &title, nil } -// SoftwareTitleNameForHostFilter returns the name and display_name -// of a software title by ID without applying team-scoped inventory auth. -// This intentionally allows callers to discover the title name and display_name -// even if the title is not present on their team. -// -// Only use this for host list filters and similar UX helpers where -// exposing the existence of a title is acceptable. Not for endpoints -// that return team-scoped inventory data. -func (ds *Datastore) SoftwareTitleNameForHostFilter( - ctx context.Context, - id uint, -) (name, displayName string, err error) { - const stmt = ` +// SoftwareTitleNameForHostFilter confirms a software title's presence via a +// live host/software join, instead of the software_titles_host_counts +// aggregate SoftwareTitleByID relies on. It returns either the team's +// display_name override or the title's name -- never both, the unused +// return is always "". A nil teamID is scoped to every team tmFilter's +// user can access (the same boundary whereFilterHostsByTeams applies +// elsewhere), never to any team at all, so it can't disclose a title +// outside that boundary; both branches return NotFound instead of +// revealing which team(s) hold the title. +func (ds *Datastore) SoftwareTitleNameForHostFilter(ctx context.Context, id uint, teamID *uint, tmFilter fleet.TeamFilter) (name, displayName string, err error) { + // "No team" hosts have hosts.team_id IS NULL, never a literal 0. + hostTeamFilter := "h.team_id IS NULL" + switch { + case teamID != nil && *teamID != 0: + hostTeamFilter = "h.team_id = ?" + case teamID == nil: + hostTeamFilter = ds.whereFilterHostsByTeams(tmFilter, "h") + } + + // Display name is per-team; skip it entirely when no team is given. + displayNameJoinCond := "FALSE" + if teamID != nil { + displayNameJoinCond = "stdn.team_id = ?" + } + + stmt := fmt.Sprintf(` SELECT - name, - display_name - FROM software_titles - LEFT JOIN software_title_display_names ON software_titles.id = software_title_display_names.software_title_id - WHERE software_titles.id = ? - ` + st.name, + stdn.display_name + FROM software_titles st + LEFT JOIN software_title_display_names stdn + ON stdn.software_title_id = st.id AND %s + WHERE st.id = ? + AND EXISTS ( + SELECT 1 + FROM host_software hs + INNER JOIN software sw ON sw.id = hs.software_id + INNER JOIN hosts h ON h.id = hs.host_id + WHERE sw.title_id = st.id AND %s + ) + `, displayNameJoinCond, hostTeamFilter) + + var allArgs []any + if teamID != nil { + allArgs = append(allArgs, *teamID) + } + allArgs = append(allArgs, id) + if teamID != nil && *teamID != 0 { + allArgs = append(allArgs, *teamID) + } + var results struct { Name string `db:"name"` DisplayName *string `db:"display_name"` } - if err := sqlx.GetContext(ctx, ds.reader(ctx), &results, stmt, id); err != nil { + if err := sqlx.GetContext(ctx, ds.reader(ctx), &results, stmt, allArgs...); err != nil { if err == sql.ErrNoRows { return "", "", notFound("SoftwareTitle").WithID(id) } diff --git a/server/datastore/mysql/software_titles_test.go b/server/datastore/mysql/software_titles_test.go index 117d2c9c3d..768a2eae4f 100644 --- a/server/datastore/mysql/software_titles_test.go +++ b/server/datastore/mysql/software_titles_test.go @@ -47,6 +47,7 @@ func TestSoftwareTitles(t *testing.T) { {"ListSoftwareTitlesSortByDisplayName", testListSoftwareTitlesSortByDisplayName}, {"ListSoftwareTitlesMultiplePackages", testListSoftwareTitlesMultiplePackages}, {"ListSoftwareTitlesPolicyDispatchPerInstaller", testListSoftwareTitlesPolicyDispatchPerInstaller}, + {"SoftwareTitleNameForHostFilter", testSoftwareTitleNameForHostFilter}, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { @@ -2343,6 +2344,96 @@ func testSoftwareTitleHostCount(t *testing.T, ds *Datastore) { require.Equal(t, ptr.Uint(1), title.Versions[0].HostsCount) } +// testSoftwareTitleNameForHostFilter verifies SoftwareTitleNameForHostFilter's +// live-join lookup and its team/tmFilter scoping. +func testSoftwareTitleNameForHostFilter(t *testing.T, ds *Datastore) { + ctx := t.Context() + + team1, err := ds.NewTeam(ctx, &fleet.Team{Name: "team1"}) + require.NoError(t, err) + team2, err := ds.NewTeam(ctx, &fleet.Team{Name: "team2"}) + require.NoError(t, err) + + host1 := test.NewHost(t, ds, "host1", "", "host1key", "host1uuid", time.Now(), test.WithTeamID(team1.ID)) + + testSw := fleet.Software{Name: "UniqueDSTitleApp", Version: "1.0", Source: "apps", BundleIdentifier: "com.unique.dstitleapp"} + _, err = ds.UpdateHostSoftware(ctx, host1.ID, []fleet.Software{testSw}) + require.NoError(t, err) + require.NoError(t, ds.LoadHostSoftware(ctx, host1, false)) + require.Len(t, host1.Software, 1) + require.NotNil(t, host1.Software[0].TitleID) + titleID := *host1.Software[0].TitleID + + // Scoped to team1 only: can't see team2 or "no team" hosts. + team1ScopedUser := &fleet.User{Teams: []fleet.UserTeam{{Team: fleet.Team{ID: team1.ID}, Role: fleet.RoleObserver}}} + team1Filter := fleet.TeamFilter{User: team1ScopedUser, IncludeObserver: true} + + globalAdminUser := &fleet.User{GlobalRole: new(fleet.RoleAdmin)} + globalAdminFilter := fleet.TeamFilter{User: globalAdminUser, IncludeObserver: true} + + // No SyncHostsSoftwareTitles call: this must find titles via a live + // join, not the aggregate table sync populates. + + // In-scope team: found immediately, pre-sync. + name, displayName, err := ds.SoftwareTitleNameForHostFilter(ctx, titleID, &team1.ID, team1Filter) + require.NoError(t, err) + assert.Equal(t, testSw.Name, name) + assert.Empty(t, displayName) + + // A team's display_name override takes precedence over the title name. + ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { + return updateSoftwareTitleDisplayName(ctx, q, &team1.ID, titleID, "Team1 Custom Name") + }) + name, displayName, err = ds.SoftwareTitleNameForHostFilter(ctx, titleID, &team1.ID, team1Filter) + require.NoError(t, err) + assert.Empty(t, name) + assert.Equal(t, "Team1 Custom Name", displayName) + + // Out-of-scope team: NotFound, no data disclosed. + _, _, err = ds.SoftwareTitleNameForHostFilter(ctx, titleID, &team2.ID, team1Filter) + require.Error(t, err) + assert.True(t, fleet.IsNotFound(err)) + + // Nonexistent title ID: NotFound. + _, _, err = ds.SoftwareTitleNameForHostFilter(ctx, titleID+999999, &team1.ID, team1Filter) + require.Error(t, err) + assert.True(t, fleet.IsNotFound(err)) + + // "No team" (team_id=0). + noTeamHost := test.NewHost(t, ds, "no-team-host", "", "no-team-hostkey", "no-team-hostuuid", time.Now()) + noTeamSw := fleet.Software{Name: "UniqueNoTeamTitleApp", Version: "1.0", Source: "apps", BundleIdentifier: "com.unique.noteamtitleapp"} + _, err = ds.UpdateHostSoftware(ctx, noTeamHost.ID, []fleet.Software{noTeamSw}) + require.NoError(t, err) + require.NoError(t, ds.LoadHostSoftware(ctx, noTeamHost, false)) + require.Len(t, noTeamHost.Software, 1) + require.NotNil(t, noTeamHost.Software[0].TitleID) + noTeamTitleID := *noTeamHost.Software[0].TitleID + + zero := uint(0) + name, displayName, err = ds.SoftwareTitleNameForHostFilter(ctx, noTeamTitleID, &zero, team1Filter) + require.NoError(t, err) + assert.Equal(t, noTeamSw.Name, name) + assert.Empty(t, displayName) + + // nil teamID: scoped to every team the caller can access, and ignores + // team1's display_name override (set above) since no single team is in scope. + name, displayName, err = ds.SoftwareTitleNameForHostFilter(ctx, titleID, nil, team1Filter) + require.NoError(t, err) + assert.Equal(t, testSw.Name, name) + assert.Empty(t, displayName) + + // ...but not "no team", which this caller can't see. + _, _, err = ds.SoftwareTitleNameForHostFilter(ctx, noTeamTitleID, nil, team1Filter) + require.Error(t, err) + assert.True(t, fleet.IsNotFound(err)) + + // A global admin can see "no team" too. + name, displayName, err = ds.SoftwareTitleNameForHostFilter(ctx, noTeamTitleID, nil, globalAdminFilter) + require.NoError(t, err) + assert.Equal(t, noTeamSw.Name, name) + assert.Empty(t, displayName) +} + func testListSoftwareTitlesInHouseApps(t *testing.T, ds *Datastore) { ctx := t.Context() diff --git a/server/fleet/datastore.go b/server/fleet/datastore.go index b16c162715..060ad0867b 100644 --- a/server/fleet/datastore.go +++ b/server/fleet/datastore.go @@ -723,7 +723,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) - SoftwareTitleNameForHostFilter(ctx context.Context, id uint) (name, displayName string, err error) + SoftwareTitleNameForHostFilter(ctx context.Context, id uint, teamID *uint, tmFilter TeamFilter) (name, displayName string, err error) UpdateSoftwareTitleName(ctx context.Context, id uint, name string) error UpdateSoftwareTitleAutoUpdateConfig(ctx context.Context, titleID uint, teamID uint, config SoftwareAutoUpdateConfig) error ListSoftwareAutoUpdateSchedules(ctx context.Context, teamID uint, source string, optionalFilter ...SoftwareAutoUpdateScheduleFilter) ([]SoftwareAutoUpdateSchedule, error) diff --git a/server/fleet/service.go b/server/fleet/service.go index 3a62c477aa..e782314f5a 100644 --- a/server/fleet/service.go +++ b/server/fleet/service.go @@ -774,7 +774,7 @@ type Service interface { ListSoftwareTitles(ctx context.Context, opt SoftwareTitleListOptions) ([]SoftwareTitleListResult, int, *PaginationMetadata, error) SoftwareTitleByID(ctx context.Context, id uint, teamID *uint) (*SoftwareTitle, error) - SoftwareTitleNameForHostFilter(ctx context.Context, id uint) (name, displayName string, err error) + SoftwareTitleNameForHostFilter(ctx context.Context, id uint, teamID *uint) (name, displayName string, err error) // InstallSoftwareTitle installs a software title in the given host. InstallSoftwareTitle(ctx context.Context, hostID uint, softwareTitleID uint) error diff --git a/server/mock/datastore_mock.go b/server/mock/datastore_mock.go index 45fb44f9fb..08bd5021db 100644 --- a/server/mock/datastore_mock.go +++ b/server/mock/datastore_mock.go @@ -520,7 +520,7 @@ 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 SoftwareTitleNameForHostFilterFunc func(ctx context.Context, id uint) (name string, displayName string, err error) +type SoftwareTitleNameForHostFilterFunc func(ctx context.Context, id uint, teamID *uint, tmFilter fleet.TeamFilter) (name string, displayName string, err error) type UpdateSoftwareTitleNameFunc func(ctx context.Context, id uint, name string) error @@ -7314,11 +7314,11 @@ func (s *DataStore) SoftwareTitleByID(ctx context.Context, id uint, teamID *uint return s.SoftwareTitleByIDFunc(ctx, id, teamID, tmFilter) } -func (s *DataStore) SoftwareTitleNameForHostFilter(ctx context.Context, id uint) (name string, displayName string, err error) { +func (s *DataStore) SoftwareTitleNameForHostFilter(ctx context.Context, id uint, teamID *uint, tmFilter fleet.TeamFilter) (name string, displayName string, err error) { s.mu.Lock() s.SoftwareTitleNameForHostFilterFuncInvoked = true s.mu.Unlock() - return s.SoftwareTitleNameForHostFilterFunc(ctx, id) + return s.SoftwareTitleNameForHostFilterFunc(ctx, id, teamID, tmFilter) } func (s *DataStore) UpdateSoftwareTitleName(ctx context.Context, id uint, name string) error { diff --git a/server/mock/service/service_mock.go b/server/mock/service/service_mock.go index 9b2968f1a3..f80d7871a5 100644 --- a/server/mock/service/service_mock.go +++ b/server/mock/service/service_mock.go @@ -486,7 +486,7 @@ type ListSoftwareTitlesFunc func(ctx context.Context, opt fleet.SoftwareTitleLis type SoftwareTitleByIDFunc func(ctx context.Context, id uint, teamID *uint) (*fleet.SoftwareTitle, error) -type SoftwareTitleNameForHostFilterFunc func(ctx context.Context, id uint) (name string, displayName string, err error) +type SoftwareTitleNameForHostFilterFunc func(ctx context.Context, id uint, teamID *uint) (name string, displayName string, err error) type InstallSoftwareTitleFunc func(ctx context.Context, hostID uint, softwareTitleID uint) error @@ -4065,11 +4065,11 @@ func (s *Service) SoftwareTitleByID(ctx context.Context, id uint, teamID *uint) return s.SoftwareTitleByIDFunc(ctx, id, teamID) } -func (s *Service) SoftwareTitleNameForHostFilter(ctx context.Context, id uint) (name string, displayName string, err error) { +func (s *Service) SoftwareTitleNameForHostFilter(ctx context.Context, id uint, teamID *uint) (name string, displayName string, err error) { s.mu.Lock() s.SoftwareTitleNameForHostFilterFuncInvoked = true s.mu.Unlock() - return s.SoftwareTitleNameForHostFilterFunc(ctx, id) + return s.SoftwareTitleNameForHostFilterFunc(ctx, id, teamID) } func (s *Service) InstallSoftwareTitle(ctx context.Context, hostID uint, softwareTitleID uint) error { diff --git a/server/service/hosts.go b/server/service/hosts.go index b9c82ad822..de3d2c33b1 100644 --- a/server/service/hosts.go +++ b/server/service/hosts.go @@ -301,21 +301,23 @@ func listHostsEndpoint(ctx context.Context, request interface{}, svc fleet.Servi titleID := *req.Opts.SoftwareTitleIDFilter // 1. Try full title for this team. - // Needed in order to grab display_name if it exists + // Needed in order to grab display_name if it exists. st, err := svc.SoftwareTitleByID(ctx, titleID, req.Opts.TeamFilter) switch { case err == nil: - fmt.Println("regular") softwareTitle = st case fleet.IsNotFound(err): - // Not found: only ID + Name as string from helper. - name, displayName, errName := svc.SoftwareTitleNameForHostFilter(ctx, titleID) + // SoftwareTitleByID depends on the software_titles_host_counts + // aggregate, populated only by the periodic + // SyncHostsSoftwareTitles job, so a title just installed on an + // in-scope host can be NotFound here until the next sync. Fall + // back to a live join instead of leaving softwareTitle unset. + name, displayName, errName := svc.SoftwareTitleNameForHostFilter(ctx, titleID, req.Opts.TeamFilter) if errName != nil && !fleet.IsNotFound(errName) { return listHostsResponse{Err: errName}, nil } if errName == nil { - fmt.Println("here") softwareTitle = &fleet.SoftwareTitle{ ID: titleID, } diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go index b8252e6418..2eb85dbb84 100644 --- a/server/service/integration_enterprise_test.go +++ b/server/service/integration_enterprise_test.go @@ -5929,6 +5929,70 @@ func (s *integrationEnterpriseTestSuite) TestListHostsSoftwareVersionOnDifferent assert.Empty(t, resp.Software) } +func (s *integrationEnterpriseTestSuite) TestListHostsSoftwareTitleOnDifferentTeam() { + t := s.T() + ctx := t.Context() + + // create 2 teams + team1, err := s.ds.NewTeam(ctx, &fleet.Team{Name: t.Name() + "_team1"}) + require.NoError(t, err) + team2, err := s.ds.NewTeam(ctx, &fleet.Team{Name: t.Name() + "_team2"}) + require.NoError(t, err) + + // create 1 host on team1 + h1, err := s.ds.NewHost(ctx, &fleet.Host{ + DetailUpdatedAt: time.Now(), + LabelUpdatedAt: time.Now(), + PolicyUpdatedAt: time.Now(), + SeenTime: time.Now(), + OsqueryHostID: new(t.Name() + "h1"), + NodeKey: new(t.Name() + "h1"), + UUID: uuid.New().String(), + Hostname: t.Name() + "h1.local", + Platform: "darwin", + TeamID: &team1.ID, + }) + require.NoError(t, err) + + // Install software only on h1 (team1). + testSw := fleet.Software{Name: "UniqueTitleApp", Version: "3.4.5", Source: "apps", BundleIdentifier: "com.unique.titleapp"} + _, err = s.ds.UpdateHostSoftware(ctx, h1.ID, []fleet.Software{testSw}) + require.NoError(t, err) + require.NoError(t, s.ds.LoadHostSoftware(ctx, h1, false)) + require.Len(t, h1.Software, 1) + require.NotNil(t, h1.Software[0].TitleID) + titleID := *h1.Software[0].TitleID + + // Deliberately do NOT call SyncHostsSoftwareTitles here: the title's + // entry in software_titles_host_counts (which SoftwareTitleByID relies + // on) is only populated by that periodic sync, so skipping it + // reproduces the up-to-~1h window between a host reporting new + // software and the next sync run. The in-scope enrichment below must + // still succeed immediately via a live (non-aggregated) fallback. + + // Filtering team1 (in-scope) by the software title returns the host and the title's name. + var resp listHostsResponse + s.DoJSON("GET", "/api/latest/fleet/hosts", nil, http.StatusOK, &resp, + "software_title_id", fmt.Sprint(titleID), + "team_id", fmt.Sprint(team1.ID), + ) + require.Len(t, resp.Hosts, 1) + assert.Equal(t, h1.ID, resp.Hosts[0].ID) + require.NotNil(t, resp.SoftwareTitle) + assert.Equal(t, testSw.Name, resp.SoftwareTitle.Name) + + // Filtering team2 (out-of-scope: the title isn't installed on any host on + // this team) must not leak the title's name/display_name — software_title + // should be omitted entirely, not backfilled from an unscoped lookup. + resp = listHostsResponse{} + s.DoJSON("GET", "/api/latest/fleet/hosts", nil, http.StatusOK, &resp, + "software_title_id", fmt.Sprint(titleID), + "team_id", fmt.Sprint(team2.ID), + ) + require.Empty(t, resp.Hosts) + assert.Nil(t, resp.SoftwareTitle) +} + func (s *integrationEnterpriseTestSuite) TestHostHealth() { t := s.T() @@ -11563,9 +11627,11 @@ func (s *integrationEnterpriseTestSuite) TestSoftwareAuth() { var resp listSoftwareVersionsResponse s.DoJSON("GET", "/api/latest/fleet/software/versions", listSoftwareTitlesRequest{}, http.StatusForbidden, &resp) - // Get a global software title + // Get a global software title (only on the "no team" host, which + // no team-scoped user can see): NotFound, not Forbidden, so its + // existence can't be inferred from the response. var getSoftwareTitleResp getSoftwareTitleResponse - s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/software/titles/%d", softwareBar.ID), getSoftwareTitleRequest{}, http.StatusForbidden, &getSoftwareTitleResp) + s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/software/titles/%d", softwareBar.ID), getSoftwareTitleRequest{}, http.StatusNotFound, &getSoftwareTitleResp) // Get a global software version var getSoftwareResp getSoftwareResponse @@ -11627,9 +11693,11 @@ func (s *integrationEnterpriseTestSuite) TestSoftwareAuth() { var resp listSoftwareTitlesResponse s.DoJSON("GET", "/api/latest/fleet/software/versions", listSoftwareRequest{SoftwareListOptions: fleet.SoftwareListOptions{TeamID: &team1.ID}}, http.StatusForbidden, &resp) - // Get a team software title + // Get a team software title (on team1 and "no team", neither + // visible to this team-2 user): NotFound, not Forbidden, so its + // existence can't be inferred from the response. var getSoftwareTitleResp getSoftwareTitleResponse - s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/software/titles/%d", softwareFoo.ID), getSoftwareTitleRequest{}, http.StatusForbidden, &getSoftwareTitleResp) + s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/software/titles/%d", softwareFoo.ID), getSoftwareTitleRequest{}, http.StatusNotFound, &getSoftwareTitleResp) // Get a team software version var getSoftwareResp getSoftwareResponse diff --git a/server/service/integration_software_titles_test.go b/server/service/integration_software_titles_test.go index a4fb91498c..9c67e65f34 100644 --- a/server/service/integration_software_titles_test.go +++ b/server/service/integration_software_titles_test.go @@ -10,7 +10,6 @@ import ( "net/http" "net/http/httptest" "os" - "reflect" "strings" "time" @@ -904,8 +903,9 @@ func (s *integrationMDMTestSuite) TestListHostsSoftwareTitleIDFilter() { s.DoJSON("POST", "/api/latest/fleet/teams", &createTeamRequest{TeamPayload: fleet.TeamPayload{Name: ptr.String("team_" + t.Name())}}, http.StatusOK, &newTeamResp) team := newTeamResp.Team - s.DoJSON("POST", "/api/latest/fleet/teams", &createTeamRequest{TeamPayload: fleet.TeamPayload{Name: ptr.String("team_2_" + t.Name())}}, http.StatusOK, &newTeamResp) - team2 := newTeamResp.Team + var newTeam2Resp teamResponse + s.DoJSON("POST", "/api/latest/fleet/teams", &createTeamRequest{TeamPayload: fleet.TeamPayload{Name: new("team_2_" + t.Name())}}, http.StatusOK, &newTeam2Resp) + team2 := newTeam2Resp.Team // Enroll a host token := "good_token" @@ -928,6 +928,7 @@ func (s *integrationMDMTestSuite) TestListHostsSoftwareTitleIDFilter() { s.Require().NoError(err) s.Require().NoError(s.ds.SyncHostsSoftware(ctx, time.Now())) + s.Require().NoError(s.ds.SyncHostsSoftwareTitles(ctx, time.Now())) sw, _, err := s.ds.ListHostSoftware(ctx, host, fleet.HostSoftwareTitleListOptions{}) s.Require().NoError(err) @@ -957,7 +958,9 @@ func (s *integrationMDMTestSuite) TestListHostsSoftwareTitleIDFilter() { s.Assert().Equal(titleID, listResp.SoftwareTitle.ID) s.Assert().Equal("bar", listResp.SoftwareTitle.Name) - // Use the other team ID, should still get a response with the name and title ID + // Use the other team ID: the title isn't installed on any host on this + // team, so no hosts should match and its name/display_name must not leak. + listResp = listHostsResponse{} s.DoJSON( "GET", "/api/latest/fleet/hosts", @@ -969,16 +972,8 @@ func (s *integrationMDMTestSuite) TestListHostsSoftwareTitleIDFilter() { "software_title_id", fmt.Sprint(titleID), ) - s.Require().Len(listResp.Hosts, 1) - s.Assert().NotNil(listResp.SoftwareTitle) - s.Assert().Equal(titleID, listResp.SoftwareTitle.ID) - s.Assert().Equal("bar", listResp.SoftwareTitle.Name) - v := reflect.ValueOf(*listResp.SoftwareTitle) - for i := 0; i < v.NumField(); i++ { - if v.Type().Field(i).Name != "ID" && v.Type().Field(i).Name != "Name" { - s.Assert().True(v.Field(i).IsZero()) - } - } + s.Require().Empty(listResp.Hosts) + s.Nil(listResp.SoftwareTitle) // Add a custom package and set a display name for the software title payload := &fleet.UploadSoftwareInstallerPayload{ @@ -1056,8 +1051,10 @@ func (s *integrationMDMTestSuite) TestListHostsSoftwareTitleIDFilter() { s.token = s.getTestToken(*params.Email, *params.Password) - // Use the other team ID, should still get a response with the display name and title ID - fmt.Println("before final call") + // The observer is scoped to their own team (team1): querying it still + // returns the full title details for the custom package, including the + // custom display name. + listResp = listHostsResponse{} s.DoJSON( "GET", "/api/latest/fleet/hosts", @@ -1065,7 +1062,7 @@ func (s *integrationMDMTestSuite) TestListHostsSoftwareTitleIDFilter() { http.StatusOK, &listResp, "team_id", - fmt.Sprint(team2.ID), + fmt.Sprint(team.ID), "software_title_id", fmt.Sprint(titleID), ) diff --git a/server/service/software_titles.go b/server/service/software_titles.go index 80b2db95a7..01be83ec36 100644 --- a/server/service/software_titles.go +++ b/server/service/software_titles.go @@ -12,7 +12,6 @@ import ( "github.com/fleetdm/fleet/v4/server/contexts/ctxerr" "github.com/fleetdm/fleet/v4/server/contexts/viewer" "github.com/fleetdm/fleet/v4/server/fleet" - "github.com/fleetdm/fleet/v4/server/ptr" ) ///////////////////////////////////////////////////////////////////////////////// @@ -172,16 +171,9 @@ func (svc *Service) SoftwareTitleByID(ctx context.Context, id uint, teamID *uint IncludeObserver: true, }) if err != nil { - if fleet.IsNotFound(err) && teamID == nil { - // here we use a global admin as filter because we want to check if the software exists - filter := fleet.TeamFilter{User: &fleet.User{GlobalRole: ptr.String(fleet.RoleAdmin)}} - _, err = svc.ds.SoftwareTitleByID(ctx, id, nil, filter) - if err != nil { - return nil, ctxerr.Wrap(ctx, err, "checked using a global admin") - } - - return nil, fleet.NewPermissionError("Error: You don't have permission to view specified software. It is installed on hosts that belong to a fleet you don't have permissions to view.") - } + // A title that exists only on a team outside the caller's visibility + // must return the same NotFound as a title that doesn't exist at + // all, so existence elsewhere can't be inferred from the response. return nil, ctxerr.Wrap(ctx, err, "getting software title by id") } @@ -334,21 +326,21 @@ func (svc *Service) SoftwareTitleByID(ctx context.Context, id uint, teamID *uint return software, nil } -func (svc *Service) SoftwareTitleNameForHostFilter( - ctx context.Context, - id uint, -) (name, displayName string, err error) { +func (svc *Service) SoftwareTitleNameForHostFilter(ctx context.Context, id uint, teamID *uint) (name, displayName string, err error) { // Intentionally skip team-scoped inventory auth: only minimal title name. - if err := svc.authz.Authorize(ctx, &fleet.Host{}, fleet.ActionList); err != nil { + if err := svc.authz.Authorize(ctx, &fleet.Host{TeamID: teamID}, fleet.ActionList); err != nil { return "", "", err } - name, displayName, err = svc.ds.SoftwareTitleNameForHostFilter(ctx, id) - if err != nil { - return "", "", err + vc, ok := viewer.FromContext(ctx) + if !ok { + return "", "", fleet.ErrNoContext } - return name, displayName, nil + return svc.ds.SoftwareTitleNameForHostFilter(ctx, id, teamID, fleet.TeamFilter{ + User: vc.User, + IncludeObserver: true, + }) } ///////////////////////////////////////////////////////////////////////////////// diff --git a/server/service/software_titles_test.go b/server/service/software_titles_test.go index f1381c2512..eaea71e1f7 100644 --- a/server/service/software_titles_test.go +++ b/server/service/software_titles_test.go @@ -307,3 +307,30 @@ func TestSoftwareTitleByIDTeamIDZero(t *testing.T) { _, err = svc.SoftwareTitleByID(adminCtx, 1, teamIDZero) checkAuthErr(t, false, err) } + +func TestSoftwareTitleByIDNilTeamIDExistsElsewhere(t *testing.T) { + ds := new(mock.Store) + var filtersUsed []fleet.TeamFilter + ds.SoftwareTitleByIDFunc = func(ctx context.Context, id uint, teamID *uint, tmFilter fleet.TeamFilter) (*fleet.SoftwareTitle, error) { + filtersUsed = append(filtersUsed, tmFilter) + return nil, newNotFoundError() + } + + svc, ctx := newTestService(t, ds, nil, nil) + teamUser := &fleet.User{ + ID: 1, + Teams: []fleet.UserTeam{{Team: fleet.Team{ID: 1}, Role: fleet.RoleObserver}}, + } + ctx = viewer.NewContext(ctx, viewer.Viewer{User: teamUser}) + + _, err := svc.SoftwareTitleByID(ctx, 1, nil) + require.Error(t, err) + // A title's existence on a team the caller can't see must not be + // distinguishable (via status code) from it not existing at all. + require.True(t, fleet.IsNotFound(err), "expected NotFound, got: %v", err) + + // Guard against reintroducing a secondary lookup: exactly one call, and + // never with a global-role filter standing in for the real caller. + require.Len(t, filtersUsed, 1) + require.Equal(t, teamUser, filtersUsed[0].User) +}