Scrub device policy responses in Fleet Desktop (#50094)
- [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 ## fleetd/orbit/Fleet Desktop - [X] Verified compatibility with the latest released version of Fleet (see [Must rule](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/workflows/fleetd-development-and-release-strategy.md)) - [X] Verified auto-update works from the released version of component to the new version (see [tools/tuf/test](../tools/tuf/test/README.md)) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Security Improvements** * Updated device-authenticated policy and host-detail responses to omit policy author identity fields and any raw SQL/query data. * Device policy endpoints now return a device-safe policy representation consistently. * **Bug Fixes** * Prevented administrative policy information from appearing in device-authenticated host details and policy listings. * **Tests** * Strengthened integration coverage to verify device-safe responses (required user-facing fields present; sensitive fields absent). <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
Lucas Manuel Rodriguez
co-authored by
Copilot Autofix powered by AI
Copilot Autofix powered by AI
parent
451319b384
commit
9c2ef14947
@@ -0,0 +1 @@
|
||||
- Fixed device-authenticated ("My device") endpoints so that host policies are returned in a device-safe representation that no longer exposes the policy author's name and email or the policy's raw SQL query.
|
||||
@@ -188,13 +188,13 @@ func (dc *DeviceClient) Ping() error {
|
||||
// listDevicePoliciesResponse is a local response type for deserializing the device policies response.
|
||||
// Definition duplicated for now (orbit should not depend server/service).
|
||||
type listDevicePoliciesResponse struct {
|
||||
Err error `json:"error,omitempty"`
|
||||
Policies []*fleet.HostPolicy `json:"policies"`
|
||||
Err error `json:"error,omitempty"`
|
||||
Policies []*fleet.DevicePolicy `json:"policies"`
|
||||
}
|
||||
|
||||
func (r listDevicePoliciesResponse) Error() error { return r.Err }
|
||||
|
||||
func (dc *DeviceClient) getListDevicePolicies(token string) ([]*fleet.HostPolicy, error) {
|
||||
func (dc *DeviceClient) getListDevicePolicies(token string) ([]*fleet.DevicePolicy, error) {
|
||||
verb, path := "GET", "/api/latest/fleet/device/%s/policies"
|
||||
var responseBody listDevicePoliciesResponse
|
||||
err := dc.request(verb, path, token, "", nil, &responseBody)
|
||||
|
||||
@@ -15,8 +15,14 @@ import (
|
||||
"github.com/fleetdm/fleet/v4/server/ptr"
|
||||
)
|
||||
|
||||
func (svc *Service) ListDevicePolicies(ctx context.Context, host *fleet.Host) ([]*fleet.HostPolicy, error) {
|
||||
return svc.ds.ListPoliciesForHost(ctx, host)
|
||||
func (svc *Service) ListDevicePolicies(ctx context.Context, host *fleet.Host) ([]*fleet.DevicePolicy, error) {
|
||||
policies, err := svc.ds.ListPoliciesForHost(ctx, host)
|
||||
if err != nil {
|
||||
return nil, ctxerr.Wrap(ctx, err, "list policies for host")
|
||||
}
|
||||
// return the device-safe representation of the policies, which excludes
|
||||
// the policy author's identity and the raw SQL query.
|
||||
return fleet.HostPoliciesToDevicePolicies(policies), nil
|
||||
}
|
||||
|
||||
// TriggerMigrateMDMDevice triggers the webhook associated with the MDM
|
||||
|
||||
@@ -0,0 +1 @@
|
||||
* Updated client response type for the `GET /api/latest/fleet/device/{token}/policies` endpoint for consistency with the server response type (`fleet.DevicePolicy`).
|
||||
@@ -554,6 +554,65 @@ type HostPolicy struct {
|
||||
Response string `json:"response" db:"response"`
|
||||
}
|
||||
|
||||
// DevicePolicy is a device-safe representation of a policy in the context of
|
||||
// a host, for device-authenticated ("My device") endpoints. It intentionally
|
||||
// omits fields that must not be exposed to end users holding only a device
|
||||
// token, such as the policy author's name and email and the raw SQL query.
|
||||
type DevicePolicy struct {
|
||||
// ID is the unique ID of the policy.
|
||||
ID uint `json:"id"`
|
||||
// Name is the name of the policy.
|
||||
Name string `json:"name"`
|
||||
// Description describes the policy.
|
||||
Description string `json:"description"`
|
||||
// Resolution describes how to solve a failing policy.
|
||||
Resolution *string `json:"resolution,omitempty"`
|
||||
// Platform is a comma-separated string to indicate the target platforms.
|
||||
//
|
||||
// Empty string targets all platforms.
|
||||
Platform string `json:"platform"`
|
||||
// Critical marks the policy as high impact.
|
||||
Critical bool `json:"critical"`
|
||||
// ConditionalAccessEnabled indicates whether this is a policy used for
|
||||
// conditional access.
|
||||
ConditionalAccessEnabled bool `json:"conditional_access_enabled"`
|
||||
// Response can be one of the following values:
|
||||
// - "pass": if the policy was executed and passed.
|
||||
// - "fail": if the policy was executed and did not pass.
|
||||
// - "": if the policy did not run yet.
|
||||
Response string `json:"response"`
|
||||
}
|
||||
|
||||
// ToDevicePolicy returns the device-safe representation of the host policy.
|
||||
func (p *HostPolicy) ToDevicePolicy() *DevicePolicy {
|
||||
return &DevicePolicy{
|
||||
ID: p.ID,
|
||||
Name: p.Name,
|
||||
Description: p.Description,
|
||||
Resolution: p.Resolution,
|
||||
Platform: p.Platform,
|
||||
Critical: p.Critical,
|
||||
ConditionalAccessEnabled: p.ConditionalAccessEnabled,
|
||||
Response: p.Response,
|
||||
}
|
||||
}
|
||||
|
||||
// HostPoliciesToDevicePolicies converts host policies to their device-safe
|
||||
// representation for device-authenticated endpoints.
|
||||
func HostPoliciesToDevicePolicies(policies []*HostPolicy) []*DevicePolicy {
|
||||
if policies == nil {
|
||||
return nil
|
||||
}
|
||||
devicePolicies := make([]*DevicePolicy, 0, len(policies))
|
||||
for _, p := range policies {
|
||||
if p == nil {
|
||||
continue
|
||||
}
|
||||
devicePolicies = append(devicePolicies, p.ToDevicePolicy())
|
||||
}
|
||||
return devicePolicies
|
||||
}
|
||||
|
||||
// PolicySpec is used to hold policy data to apply policy specs.
|
||||
//
|
||||
// Policies are currently identified by name (unique).
|
||||
|
||||
@@ -461,8 +461,10 @@ type Service interface {
|
||||
// HostLiteByIdentifier returns a host and a subset of its fields from its id.
|
||||
HostLiteByID(ctx context.Context, id uint) (*HostLite, error)
|
||||
|
||||
// ListDevicePolicies lists all policies for the given host, including passing / failing summaries
|
||||
ListDevicePolicies(ctx context.Context, host *Host) ([]*HostPolicy, error)
|
||||
// ListDevicePolicies lists all policies for the given host in their
|
||||
// device-safe representation (which excludes the policy author's identity
|
||||
// and the raw SQL query), including passing / failing responses.
|
||||
ListDevicePolicies(ctx context.Context, host *Host) ([]*DevicePolicy, error)
|
||||
|
||||
// BypassConditionalAccess lets a host skip conditional access checks for one check
|
||||
BypassConditionalAccess(ctx context.Context, host *Host) error
|
||||
|
||||
@@ -264,7 +264,7 @@ type HostLiteByIdentifierFunc func(ctx context.Context, identifier string) (*fle
|
||||
|
||||
type HostLiteByIDFunc func(ctx context.Context, id uint) (*fleet.HostLite, error)
|
||||
|
||||
type ListDevicePoliciesFunc func(ctx context.Context, host *fleet.Host) ([]*fleet.HostPolicy, error)
|
||||
type ListDevicePoliciesFunc func(ctx context.Context, host *fleet.Host) ([]*fleet.DevicePolicy, error)
|
||||
|
||||
type BypassConditionalAccessFunc func(ctx context.Context, host *fleet.Host) error
|
||||
|
||||
@@ -3288,7 +3288,7 @@ func (s *Service) HostLiteByID(ctx context.Context, id uint) (*fleet.HostLite, e
|
||||
return s.HostLiteByIDFunc(ctx, id)
|
||||
}
|
||||
|
||||
func (s *Service) ListDevicePolicies(ctx context.Context, host *fleet.Host) ([]*fleet.HostPolicy, error) {
|
||||
func (s *Service) ListDevicePolicies(ctx context.Context, host *fleet.Host) ([]*fleet.DevicePolicy, error) {
|
||||
s.mu.Lock()
|
||||
s.ListDevicePoliciesFuncInvoked = true
|
||||
s.mu.Unlock()
|
||||
|
||||
@@ -107,8 +107,17 @@ func (r *getDeviceHostRequest) deviceAuthToken() string {
|
||||
return r.Token
|
||||
}
|
||||
|
||||
// deviceHostDetailResponse wraps the host detail response to shadow the
|
||||
// host's policies with their device-safe representation, which excludes the
|
||||
// policy author's identity and the raw SQL query (this is a device-authenticated
|
||||
// endpoint, so it must not expose admin-only data).
|
||||
type deviceHostDetailResponse struct {
|
||||
*fleet.HostDetailResponse
|
||||
Policies *[]*fleet.DevicePolicy `json:"policies,omitempty"`
|
||||
}
|
||||
|
||||
type getDeviceHostResponse struct {
|
||||
Host *fleet.HostDetailResponse `json:"host"`
|
||||
Host *deviceHostDetailResponse `json:"host"`
|
||||
// Deprecated: use OrgLogoURLDarkMode.
|
||||
OrgLogoURL string `json:"org_logo_url"`
|
||||
// Deprecated: use OrgLogoURLLightMode.
|
||||
@@ -237,8 +246,19 @@ func getDeviceHostEndpoint(ctx context.Context, request interface{}, svc fleet.S
|
||||
},
|
||||
}
|
||||
|
||||
deviceHost := &deviceHostDetailResponse{HostDetailResponse: resp}
|
||||
if resp.Policies != nil {
|
||||
devicePolicies := fleet.HostPoliciesToDevicePolicies(*resp.Policies)
|
||||
deviceHost.Policies = &devicePolicies
|
||||
// defense-in-depth: the shadow field above already wins over the
|
||||
// embedded policies when marshaling, but clear the admin-facing
|
||||
// policies anyway so they cannot leak if the wrapped response is ever
|
||||
// marshaled directly.
|
||||
resp.Policies = nil
|
||||
}
|
||||
|
||||
return getDeviceHostResponse{
|
||||
Host: resp,
|
||||
Host: deviceHost,
|
||||
OrgLogoURL: ac.OrgInfo.OrgLogoURL,
|
||||
OrgLogoURLLightBackground: ac.OrgInfo.OrgLogoURLLightBackground,
|
||||
OrgLogoURLDarkMode: ac.OrgInfo.OrgLogoURLDarkMode,
|
||||
@@ -463,8 +483,8 @@ func (r *listDevicePoliciesRequest) deviceAuthToken() string {
|
||||
}
|
||||
|
||||
type listDevicePoliciesResponse struct {
|
||||
Err error `json:"error,omitempty"`
|
||||
Policies []*fleet.HostPolicy `json:"policies"`
|
||||
Err error `json:"error,omitempty"`
|
||||
Policies []*fleet.DevicePolicy `json:"policies"`
|
||||
}
|
||||
|
||||
func (r listDevicePoliciesResponse) Error() error { return r.Err }
|
||||
@@ -484,7 +504,7 @@ func listDevicePoliciesEndpoint(ctx context.Context, request interface{}, svc fl
|
||||
return listDevicePoliciesResponse{Policies: data}, nil
|
||||
}
|
||||
|
||||
func (svc *Service) ListDevicePolicies(ctx context.Context, host *fleet.Host) ([]*fleet.HostPolicy, error) {
|
||||
func (svc *Service) ListDevicePolicies(ctx context.Context, host *fleet.Host) ([]*fleet.DevicePolicy, error) {
|
||||
// skipauth: No authorization check needed due to implementation returning
|
||||
// only license error.
|
||||
svc.authz.SkipAuthorization(ctx)
|
||||
|
||||
@@ -4439,23 +4439,49 @@ func (s *integrationEnterpriseTestSuite) TestListDevicePolicies() {
|
||||
err = res.Body.Close()
|
||||
require.NoError(t, err)
|
||||
|
||||
// asserts that a JSON-decoded policy from a device-authenticated endpoint
|
||||
// only contains device-safe fields, i.e. it never exposes the policy
|
||||
// author's identity nor the raw SQL query.
|
||||
assertDeviceSafePolicy := func(policy map[string]any) {
|
||||
require.NotContains(t, policy, "query")
|
||||
require.NotContains(t, policy, "author_id")
|
||||
require.NotContains(t, policy, "author_name")
|
||||
require.NotContains(t, policy, "author_email")
|
||||
require.Contains(t, policy, "name")
|
||||
require.Contains(t, policy, "response")
|
||||
}
|
||||
|
||||
// GET `/api/_version_/fleet/device/{token}/policies`
|
||||
listDevicePoliciesResp := listDevicePoliciesResponse{}
|
||||
res = s.DoRawNoAuth("GET", "/api/latest/fleet/device/"+token+"/policies", nil, http.StatusOK)
|
||||
err = json.NewDecoder(res.Body).Decode(&listDevicePoliciesResp)
|
||||
rawBody, err := io.ReadAll(res.Body)
|
||||
require.NoError(t, err)
|
||||
err = res.Body.Close()
|
||||
require.NoError(t, err)
|
||||
err = json.Unmarshal(rawBody, &listDevicePoliciesResp)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, listDevicePoliciesResp.Policies, 2)
|
||||
require.NoError(t, listDevicePoliciesResp.Err)
|
||||
// the response must not leak the policy author's identity nor the raw SQL query
|
||||
var rawPoliciesResp struct {
|
||||
Policies []map[string]any `json:"policies"`
|
||||
}
|
||||
err = json.Unmarshal(rawBody, &rawPoliciesResp)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, rawPoliciesResp.Policies, 2)
|
||||
for _, policy := range rawPoliciesResp.Policies {
|
||||
assertDeviceSafePolicy(policy)
|
||||
}
|
||||
|
||||
// GET `/api/_version_/fleet/device/{token}`
|
||||
getDeviceHostResp := getDeviceHostResponse{}
|
||||
res = s.DoRawNoAuth("GET", "/api/latest/fleet/device/"+token, nil, http.StatusOK)
|
||||
err = json.NewDecoder(res.Body).Decode(&getDeviceHostResp)
|
||||
rawBody, err = io.ReadAll(res.Body)
|
||||
require.NoError(t, err)
|
||||
err = res.Body.Close()
|
||||
require.NoError(t, err)
|
||||
err = json.Unmarshal(rawBody, &getDeviceHostResp)
|
||||
require.NoError(t, err)
|
||||
require.NoError(t, getDeviceHostResp.Err)
|
||||
require.Equal(t, host.ID, getDeviceHostResp.Host.ID)
|
||||
require.False(t, getDeviceHostResp.Host.RefetchRequested)
|
||||
@@ -4463,6 +4489,19 @@ func (s *integrationEnterpriseTestSuite) TestListDevicePolicies() {
|
||||
require.Equal(t, "http://example.com/contact", getDeviceHostResp.OrgContactURL)
|
||||
require.Len(t, *getDeviceHostResp.Host.Policies, 2)
|
||||
require.False(t, getDeviceHostResp.GlobalConfig.Features.EnableSoftwareInventory)
|
||||
// the host's policies must not leak the policy author's identity nor the
|
||||
// raw SQL query
|
||||
var rawHostResp struct {
|
||||
Host struct {
|
||||
Policies []map[string]any `json:"policies"`
|
||||
} `json:"host"`
|
||||
}
|
||||
err = json.Unmarshal(rawBody, &rawHostResp)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, rawHostResp.Host.Policies, 2)
|
||||
for _, policy := range rawHostResp.Host.Policies {
|
||||
assertDeviceSafePolicy(policy)
|
||||
}
|
||||
|
||||
// GET `/api/_version_/fleet/device/{token}/desktop`
|
||||
getDesktopResp := fleetDesktopResponse{}
|
||||
|
||||
Reference in New Issue
Block a user