## 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 -->
310 lines
8.8 KiB
Go
310 lines
8.8 KiB
Go
package service
|
|
|
|
import (
|
|
"context"
|
|
"testing"
|
|
|
|
"github.com/fleetdm/fleet/v4/server/contexts/license"
|
|
"github.com/fleetdm/fleet/v4/server/contexts/viewer"
|
|
"github.com/fleetdm/fleet/v4/server/fleet"
|
|
"github.com/fleetdm/fleet/v4/server/mock"
|
|
"github.com/fleetdm/fleet/v4/server/ptr"
|
|
"github.com/stretchr/testify/require"
|
|
)
|
|
|
|
func TestServiceSoftwareTitlesAuth(t *testing.T) {
|
|
ds := new(mock.Store)
|
|
|
|
ds.ListSoftwareTitlesFunc = func(ctx context.Context, opt fleet.SoftwareTitleListOptions, tmf fleet.TeamFilter) ([]fleet.SoftwareTitleListResult, int, *fleet.PaginationMetadata, error) {
|
|
return []fleet.SoftwareTitleListResult{}, 0, &fleet.PaginationMetadata{}, nil
|
|
}
|
|
ds.SoftwareTitleByIDFunc = func(ctx context.Context, id uint, teamID *uint, tmFilter fleet.TeamFilter) (*fleet.SoftwareTitle, error) {
|
|
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)
|
|
|
|
for _, tc := range []struct {
|
|
name string
|
|
user *fleet.User
|
|
shouldFailGlobalRead bool
|
|
shouldFailTeamRead bool
|
|
shouldFailGetByID bool
|
|
shouldFailWrite bool
|
|
}{
|
|
{
|
|
name: "global-admin",
|
|
user: &fleet.User{
|
|
ID: 1,
|
|
GlobalRole: ptr.String(fleet.RoleAdmin),
|
|
},
|
|
shouldFailGlobalRead: false,
|
|
shouldFailTeamRead: false,
|
|
shouldFailGetByID: false,
|
|
shouldFailWrite: false,
|
|
},
|
|
{
|
|
name: "global-maintainer",
|
|
user: &fleet.User{
|
|
ID: 1,
|
|
GlobalRole: ptr.String(fleet.RoleMaintainer),
|
|
},
|
|
shouldFailGlobalRead: false,
|
|
shouldFailTeamRead: false,
|
|
shouldFailGetByID: false,
|
|
shouldFailWrite: true,
|
|
},
|
|
{
|
|
name: "global-observer",
|
|
user: &fleet.User{
|
|
ID: 1,
|
|
GlobalRole: ptr.String(fleet.RoleObserver),
|
|
},
|
|
shouldFailGlobalRead: false,
|
|
shouldFailTeamRead: false,
|
|
shouldFailGetByID: false,
|
|
shouldFailWrite: true,
|
|
},
|
|
{
|
|
name: "team-admin-belongs-to-team",
|
|
user: &fleet.User{
|
|
ID: 1,
|
|
Teams: []fleet.UserTeam{{
|
|
Team: fleet.Team{ID: 1},
|
|
Role: fleet.RoleAdmin,
|
|
}},
|
|
},
|
|
shouldFailGlobalRead: true,
|
|
shouldFailTeamRead: false,
|
|
shouldFailGetByID: false,
|
|
shouldFailWrite: true,
|
|
},
|
|
{
|
|
name: "team-maintainer-belongs-to-team",
|
|
user: &fleet.User{
|
|
ID: 1,
|
|
Teams: []fleet.UserTeam{{
|
|
Team: fleet.Team{ID: 1},
|
|
Role: fleet.RoleMaintainer,
|
|
}},
|
|
},
|
|
shouldFailGlobalRead: true,
|
|
shouldFailTeamRead: false,
|
|
shouldFailGetByID: false,
|
|
shouldFailWrite: true,
|
|
},
|
|
{
|
|
name: "team-observer-belongs-to-team",
|
|
user: &fleet.User{
|
|
ID: 1,
|
|
Teams: []fleet.UserTeam{{
|
|
Team: fleet.Team{ID: 1},
|
|
Role: fleet.RoleObserver,
|
|
}},
|
|
},
|
|
shouldFailGlobalRead: true,
|
|
shouldFailTeamRead: false,
|
|
shouldFailGetByID: false,
|
|
shouldFailWrite: true,
|
|
},
|
|
{
|
|
name: "team-admin-does-not-belong-to-team",
|
|
user: &fleet.User{
|
|
ID: 1,
|
|
Teams: []fleet.UserTeam{{
|
|
Team: fleet.Team{ID: 2},
|
|
Role: fleet.RoleAdmin,
|
|
}},
|
|
},
|
|
shouldFailGlobalRead: true,
|
|
shouldFailTeamRead: true,
|
|
shouldFailGetByID: true,
|
|
shouldFailWrite: true,
|
|
},
|
|
{
|
|
name: "team-maintainer-does-not-belong-to-team",
|
|
user: &fleet.User{
|
|
ID: 1,
|
|
Teams: []fleet.UserTeam{{
|
|
Team: fleet.Team{ID: 2},
|
|
Role: fleet.RoleMaintainer,
|
|
}},
|
|
},
|
|
shouldFailGlobalRead: true,
|
|
shouldFailTeamRead: true,
|
|
shouldFailGetByID: true,
|
|
shouldFailWrite: true,
|
|
},
|
|
{
|
|
name: "team-observer-does-not-belong-to-team",
|
|
user: &fleet.User{
|
|
ID: 1,
|
|
Teams: []fleet.UserTeam{{
|
|
Team: fleet.Team{ID: 2},
|
|
Role: fleet.RoleObserver,
|
|
}},
|
|
},
|
|
shouldFailGlobalRead: true,
|
|
shouldFailTeamRead: true,
|
|
shouldFailGetByID: true,
|
|
shouldFailWrite: true,
|
|
},
|
|
{
|
|
// GitOps can list software titles but cannot fetch a single title
|
|
// because SoftwareTitleByID also requires Host:list permission.
|
|
name: "global-gitops",
|
|
user: &fleet.User{
|
|
ID: 1,
|
|
GlobalRole: ptr.String(fleet.RoleGitOps),
|
|
},
|
|
shouldFailGlobalRead: false,
|
|
shouldFailTeamRead: false,
|
|
shouldFailGetByID: true,
|
|
shouldFailWrite: true,
|
|
},
|
|
{
|
|
name: "team-gitops-belongs-to-team",
|
|
user: &fleet.User{
|
|
ID: 1,
|
|
Teams: []fleet.UserTeam{{
|
|
Team: fleet.Team{ID: 1},
|
|
Role: fleet.RoleGitOps,
|
|
}},
|
|
},
|
|
shouldFailGlobalRead: true,
|
|
shouldFailTeamRead: false,
|
|
shouldFailGetByID: true,
|
|
shouldFailWrite: true,
|
|
},
|
|
{
|
|
name: "team-gitops-does-not-belong-to-team",
|
|
user: &fleet.User{
|
|
ID: 1,
|
|
Teams: []fleet.UserTeam{{
|
|
Team: fleet.Team{ID: 2},
|
|
Role: fleet.RoleGitOps,
|
|
}},
|
|
},
|
|
shouldFailGlobalRead: true,
|
|
shouldFailTeamRead: true,
|
|
shouldFailGetByID: true,
|
|
shouldFailWrite: true,
|
|
},
|
|
} {
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
ctx := viewer.NewContext(ctx, viewer.Viewer{User: tc.user})
|
|
premiumCtx := license.NewContext(ctx, &fleet.LicenseInfo{Tier: fleet.TierPremium})
|
|
|
|
// List all software titles.
|
|
_, _, _, err := svc.ListSoftwareTitles(ctx, fleet.SoftwareTitleListOptions{})
|
|
checkAuthErr(t, tc.shouldFailGlobalRead, err)
|
|
|
|
// List software for a team.
|
|
_, _, _, err = svc.ListSoftwareTitles(premiumCtx, fleet.SoftwareTitleListOptions{
|
|
TeamID: ptr.Uint(1),
|
|
})
|
|
checkAuthErr(t, tc.shouldFailTeamRead, err)
|
|
|
|
// List software for a team should fail no matter what
|
|
// with a non-premium context
|
|
if !tc.shouldFailTeamRead {
|
|
_, _, _, err = svc.ListSoftwareTitles(ctx, fleet.SoftwareTitleListOptions{
|
|
TeamID: ptr.Uint(1),
|
|
})
|
|
require.ErrorContains(t, err, "Requires Fleet Premium license")
|
|
}
|
|
|
|
// Get a software title for a team
|
|
_, err = svc.SoftwareTitleByID(ctx, 1, ptr.Uint(1))
|
|
checkAuthErr(t, tc.shouldFailGetByID, 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)
|
|
}
|
|
|
|
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)
|
|
}
|