diff --git a/changes/software-title-scope-validation b/changes/software-title-scope-validation new file mode 100644 index 0000000000..6c7680301b --- /dev/null +++ b/changes/software-title-scope-validation @@ -0,0 +1 @@ +- Improved fleet scope validation for software title lookups. diff --git a/server/service/software_titles.go b/server/service/software_titles.go index 562223ca3c..2167c6088f 100644 --- a/server/service/software_titles.go +++ b/server/service/software_titles.go @@ -143,17 +143,19 @@ func (svc *Service) SoftwareTitleByID(ctx context.Context, id uint, teamID *uint return nil, err } - if teamID != nil && *teamID != 0 { - // This auth check ensures we return 403 if the user doesn't have access to the team + if teamID != nil { + // Verify the caller has permission for the requested scope (team or global). if err := svc.authz.Authorize(ctx, &fleet.AuthzSoftwareInventory{TeamID: teamID}, fleet.ActionRead); err != nil { return nil, err } - exists, err := svc.ds.TeamExists(ctx, *teamID) - if err != nil { - return nil, ctxerr.Wrap(ctx, err, "checking if team exists") - } else if !exists { - return nil, fleet.NewInvalidArgumentError("team_id", fmt.Sprintf("fleet %d does not exist", *teamID)). - WithStatus(http.StatusNotFound) + if *teamID != 0 { + exists, err := svc.ds.TeamExists(ctx, *teamID) + if err != nil { + return nil, ctxerr.Wrap(ctx, err, "checking if team exists") + } else if !exists { + return nil, fleet.NewInvalidArgumentError("team_id/fleet_id", fmt.Sprintf("fleet %d does not exist", *teamID)). + WithStatus(http.StatusNotFound) + } } } diff --git a/server/service/software_titles_test.go b/server/service/software_titles_test.go index 604baa18fc..f1381c2512 100644 --- a/server/service/software_titles_test.go +++ b/server/service/software_titles_test.go @@ -272,3 +272,38 @@ func TestSoftwareNameUpdate(t *testing.T) { require.NoError(t, err) require.True(t, ds.UpdateSoftwareTitleNameFuncInvoked) } + +func TestSoftwareTitleByIDTeamIDZero(t *testing.T) { + ds := new(mock.Store) + ds.SoftwareTitleByIDFunc = func(ctx context.Context, id uint, teamID *uint, tmFilter fleet.TeamFilter) (*fleet.SoftwareTitle, error) { + return &fleet.SoftwareTitle{BundleIdentifier: new("com.example.app")}, nil + } + ds.TeamExistsFunc = func(ctx context.Context, teamID uint) (bool, error) { return true, nil } + + svc, ctx := newTestService(t, ds, nil, nil) + + teamIDZero := new(uint) // *uint pointing to 0 + + // Team-scoped user on team 1 should not be able to access software with team_id=0 + teamUser := &fleet.User{ + ID: 1, + Teams: []fleet.UserTeam{{ + Team: fleet.Team{ID: 1}, + Role: fleet.RoleAdmin, + }}, + } + ctx = viewer.NewContext(ctx, viewer.Viewer{User: teamUser}) + + _, err := svc.SoftwareTitleByID(ctx, 1, teamIDZero) + checkAuthErr(t, true, err) + + // Global admin should still be able to access software with team_id=0 + globalAdmin := &fleet.User{ + ID: 2, + GlobalRole: new(fleet.RoleAdmin), + } + adminCtx := viewer.NewContext(ctx, viewer.Viewer{User: globalAdmin}) + + _, err = svc.SoftwareTitleByID(adminCtx, 1, teamIDZero) + checkAuthErr(t, false, err) +}