Improve fleet scope validation for software title lookups (#48034)
## Summary Ensures that `SoftwareTitleByID` validates fleet scope for all non-nil `team_id` values, including zero. Previously the scope check was only applied when `team_id > 0`. ## Reproduction Added a unit test (`TestSoftwareTitleByIDTeamIDZero`) that sets up a fleet-scoped user on fleet 1, then calls `SoftwareTitleByID` with `team_id=0`. Before this change, the call succeeded. After, it correctly returns 403. Also confirmed that a global admin calling with `team_id=0` still succeeds, and that all existing `TestServiceSoftwareTitlesAuth` subtests continue to pass. # 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/guides/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), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters. ## Testing - [x] Added/updated automated tests - [x] QA'd all new/changed functionality manually <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Improvements** * Enhanced fleet scope validation for software title lookups, including correct handling when a team scope value is set to `0`. * **Tests** * Added unit test coverage for software title retrieval authorization behavior when the team scope value is `0`. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -0,0 +1 @@
|
||||
- Improved fleet scope validation for software title lookups.
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user