Discharge7443/fleet
1
0
Fork 0
Code Issues Pull Requests Actions 22 Packages Projects Releases Wiki Activity

Fix issue with permissions in host activity list for fleet-users (#46362)

Browse Source 
**Related issue:** Resolves #46009.

- [X] Changes file added for user-visible changes in `changes/`,
`orbit/changes/` or `ee/fleetd-chrome/changes`.

## 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

* **Bug Fixes**
* Resolved an authorization issue preventing users from viewing past
host activities on hosts that contained user-initiated operations such
as lock, wipe, run script, or install software.

<!-- review_stack_entry_start -->

[![Review Change
Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/fleetdm/fleet/pull/46362?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack)

<!-- review_stack_entry_end -->

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Lucas Manuel Rodriguez
2026-05-28 14:57:27 -03:00
committed by GitHub
parent e76f28937e
commit a1d91464ea
8 changed files with 101 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files
changes/46009-fix-host-activity-user-enrichment-authz
+1
View File
@@ -0,0 +1 @@
* Fixed Fleet-scoped users getting a 403 when viewing past activities on a host that has user-initiated activities (e.g. lock/wipe/run script/install software).
cmd/fleet/serve.go
+3 -3
View File
@@ -1084,7 +1084,7 @@ func runServeCmd(cmd *cobra.Command, configManager configpkg.Manager, debug, dev
logger.InfoContext(ctx, "instance info", "instanceID", instanceID)
// Bootstrap activity bounded context (needed for cron schedules and HTTP routes)
activitySvc, activityRoutes := createActivityBoundedContext(svc, dbConns, logger)
activitySvc, activityRoutes := createActivityBoundedContext(svc, ds, dbConns, logger)
// Inject the activity bounded context into the main service
svc.SetActivityService(activitySvc)
@@ -1978,13 +1978,13 @@ func initOrgLogoStore(ctx context.Context, s3Config configpkg.S3Config, logger *
return store
}
func createActivityBoundedContext(svc fleet.Service, dbConns *common_mysql.DBConnections, logger *slog.Logger) (activity_api.Service, endpointer.HandlerRoutesFunc) {
func createActivityBoundedContext(svc fleet.Service, ds fleet.Datastore, dbConns *common_mysql.DBConnections, logger *slog.Logger) (activity_api.Service, endpointer.HandlerRoutesFunc) {
legacyAuthorizer, err := authz.NewAuthorizer()
if err != nil {
initFatal(err, "initializing activity authorizer")
}
activityAuthorizer := authz.NewAuthorizerAdapter(legacyAuthorizer)
activityACLAdapter := activityacl.NewFleetServiceAdapter(svc)
activityACLAdapter := activityacl.NewFleetServiceAdapter(svc, ds)
activitySvc, activityRoutesFn := activity_bootstrap.New(
dbConns,
activityAuthorizer,
server/acl/activityacl/fleet_adapter.go
+21 -5
View File
@@ -13,15 +13,29 @@ import (
"github.com/fleetdm/fleet/v4/server/fleet"
)
// UsersByIDsLookup is the minimal interface needed to look up user summaries by ID
// without going through the service layer's authz. The activity bounded context
// performs its own authorization on the host before reaching the user-enrichment
// step, so a second authz check (which would reject team-scoped viewers since
// Service.UsersByIDs authorizes against an empty *fleet.User) is both incorrect
// and the cause of issue #46009 for hosts with user-initiated past activities.
type UsersByIDsLookup interface {
UsersByIDs(ctx context.Context, ids []uint) ([]*fleet.UserSummary, error)
}
// FleetServiceAdapter provides access to Fleet service methods
// for data that the activity bounded context doesn't own.
type FleetServiceAdapter struct {
svc fleet.ActivityLookupService
svc fleet.ActivityLookupService
usersByIDsDS UsersByIDsLookup
}
// NewFleetServiceAdapter creates a new adapter for the Fleet service.
func NewFleetServiceAdapter(svc fleet.ActivityLookupService) *FleetServiceAdapter {
return &FleetServiceAdapter{svc: svc}
// usersByIDsDS is a datastore-direct lookup used for activity user enrichment;
// it must bypass service-layer authz because the host authz check has already
// gated access to the activity list.
func NewFleetServiceAdapter(svc fleet.ActivityLookupService, usersByIDsDS UsersByIDsLookup) *FleetServiceAdapter {
return &FleetServiceAdapter{svc: svc, usersByIDsDS: usersByIDsDS}
}
// Ensure FleetServiceAdapter implements the required interfaces
@@ -32,14 +46,16 @@ var (
_ activity.UpcomingActivityActivator = (*FleetServiceAdapter)(nil)
)
// UsersByIDs fetches users by their IDs from the Fleet service.
// UsersByIDs fetches users by their IDs from the datastore directly,
// bypassing service-layer authz. The activity service has already authorized
// the caller against the host before this is called for enrichment.
func (a *FleetServiceAdapter) UsersByIDs(ctx context.Context, ids []uint) ([]*activity.User, error) {
if len(ids) == 0 {
return nil, nil
}
// Fetch only the requested users by their IDs
users, err := a.svc.UsersByIDs(ctx, ids)
users, err := a.usersByIDsDS.UsersByIDs(ctx, ids)
if err != nil {
return nil, err
}
server/datastore/mysql/mysqltest/mysqltest.go
+1 -1
View File
@@ -903,7 +903,7 @@ func NewTestActivityService(t testing.TB, ds *mysql.Datastore) activity_api.Serv
dbConns := TestDBConnections(t, ds)
lookupSvc := &testingLookupService{ds: ds}
aclAdapter := activityacl.NewFleetServiceAdapter(lookupSvc)
aclAdapter := activityacl.NewFleetServiceAdapter(lookupSvc, ds)
discardLogger := slog.New(slog.DiscardHandler)
svc, _ := activity_bootstrap.New(dbConns, &testingAuthorizer{}, aclAdapter, discardLogger)
server/datastore/mysql/testing_utils_test.go
+1 -1
View File
@@ -930,7 +930,7 @@ func NewTestActivityService(t testing.TB, ds *Datastore) activity_api.Service {
// Use the real ACL adapter with a testing lookup service
lookupSvc := &testingLookupService{ds: ds}
aclAdapter := activityacl.NewFleetServiceAdapter(lookupSvc)
aclAdapter := activityacl.NewFleetServiceAdapter(lookupSvc, ds)
// Create service via bootstrap (the public API for creating the bounded context)
discardLogger := slog.New(slog.DiscardHandler)
server/service/integration_enterprise_test.go
+72
View File
@@ -43,6 +43,7 @@ import (
"github.com/fleetdm/fleet/v4/pkg/optjson"
"github.com/fleetdm/fleet/v4/pkg/scripts"
"github.com/fleetdm/fleet/v4/server"
activity_api "github.com/fleetdm/fleet/v4/server/activity/api"
apiendpoints "github.com/fleetdm/fleet/v4/server/api_endpoints"
"github.com/fleetdm/fleet/v4/server/config"
"github.com/fleetdm/fleet/v4/server/contexts/installersize"
@@ -31677,3 +31678,74 @@ func (s *integrationEnterpriseTestSuite) TestOrbitEnrollWithEUAToken() {
require.Equal(t, idpEmail, dms[0].Email)
require.Equal(t, fleet.DeviceMappingMDMIdpAccounts, dms[0].Source)
}
// TestTeamAdminCanReadHostPastActivities is a regression test for issue #46009:
// after the OPA-tag fix, team-scoped users were still getting 403 on
// GET /api/_version_/fleet/hosts/{id}/activities for any host with a
// user-initiated past activity (locked_host being the common reproducer).
// The cause was Service.UsersByIDs gating user enrichment behind an authz
// check on an empty *fleet.User, which no team role can satisfy. The activity
// ACL adapter now bypasses that check by going directly to the datastore.
func (s *integrationEnterpriseTestSuite) TestTeamAdminCanReadHostPastActivities() {
t := s.T()
ctx := context.Background()
team, err := s.ds.NewTeam(ctx, &fleet.Team{
Name: t.Name(),
Description: t.Name(),
})
require.NoError(t, err)
t.Cleanup(func() {
require.NoError(t, s.ds.DeleteTeam(context.Background(), team.ID))
})
teamAdminEmail := t.Name() + "_admin@example.com"
teamAdmin := &fleet.User{
Name: teamAdminEmail,
Email: teamAdminEmail,
Teams: []fleet.UserTeam{
{Team: *team, Role: fleet.RoleAdmin},
},
}
require.NoError(t, teamAdmin.SetPassword(test.GoodPassword, 10, 10))
teamAdmin, err = s.ds.NewUser(ctx, teamAdmin)
require.NoError(t, err)
host, err := s.ds.NewHost(ctx, &fleet.Host{
DetailUpdatedAt: time.Now(),
LabelUpdatedAt: time.Now(),
PolicyUpdatedAt: time.Now(),
SeenTime: time.Now(),
OsqueryHostID: new(t.Name() + "_host"),
NodeKey: new(t.Name() + "_host"),
UUID: t.Name() + "_host",
Hostname: t.Name() + "_host.local",
Platform: "darwin",
TeamID: &team.ID,
})
require.NoError(t, err)
// Insert a host-linked past activity attributed to the team admin. This
// mirrors what happens when the team admin locks one of their hosts —
// the activity row has user_id set, which forces enrichWithUserData to
// call the user lookup. Before the fix, that lookup went through
// Service.UsersByIDs and 403'd the team admin viewing the activity feed.
activitySvc := mysqltest.NewTestActivityService(t, s.ds)
apiUser := &activity_api.User{ID: teamAdmin.ID, Name: teamAdmin.Name, Email: teamAdmin.Email}
require.NoError(t, activitySvc.NewActivity(ctx, apiUser, fleet.ActivityTypeLockedHost{
HostID: host.ID,
HostDisplayName: host.DisplayName(),
}))
t.Cleanup(func() { s.token = s.getTestAdminToken() })
s.token = s.getTestToken(teamAdmin.Email, test.GoodPassword)
var listResp listActivitiesResponse
s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/hosts/%d/activities", host.ID), nil, http.StatusOK, &listResp)
require.Len(t, listResp.Activities, 1)
require.Equal(t, fleet.ActivityTypeLockedHost{}.ActivityName(), listResp.Activities[0].Type)
require.NotNil(t, listResp.Activities[0].ActorEmail)
require.Equal(t, teamAdmin.Email, *listResp.Activities[0].ActorEmail)
require.NotNil(t, listResp.Activities[0].ActorFullName)
require.Equal(t, teamAdmin.Name, *listResp.Activities[0].ActorFullName)
}
server/service/svctest/server.go
+1 -1
View File
@@ -92,7 +92,7 @@ func RunServerForTestsWithServiceWithDS(t *testing.T, ctx context.Context, ds fl
legacyAuthorizer, err := authz.NewAuthorizer()
require.NoError(t, err)
activityAuthorizer := authz.NewAuthorizerAdapter(legacyAuthorizer)
activityACLAdapter := activityacl.NewFleetServiceAdapter(svc)
activityACLAdapter := activityacl.NewFleetServiceAdapter(svc, ds)
activitySvc, activityRoutesFn := activity_bootstrap.New(
opts[0].DBConns,
activityAuthorizer,
server/service/testing_utils_test.go
+1 -1
View File
@@ -441,7 +441,7 @@ func RunServerForTestsWithServiceWithDS(t *testing.T, ctx context.Context, ds fl
legacyAuthorizer, err := authz.NewAuthorizer()
require.NoError(t, err)
activityAuthorizer := authz.NewAuthorizerAdapter(legacyAuthorizer)
activityACLAdapter := activityacl.NewFleetServiceAdapter(svc)
activityACLAdapter := activityacl.NewFleetServiceAdapter(svc, ds)
activitySvc, activityRoutesFn := activity_bootstrap.New(
opts[0].DBConns,
activityAuthorizer,