From 8484f2726b321b606e174e66907870484b660932 Mon Sep 17 00:00:00 2001 From: Oleksandr Didukh Date: Tue, 18 Aug 2026 14:48:47 +0200 Subject: [PATCH] fix(multitenancy): let the global policy modify endpoint accept own-team policies Creation re-homes a tenant's "global" policies to the pinned team, but modifyPolicy still required team NULL on the global endpoint, so every policy update from the tenant UI failed with 400 "policy does not belong to team/global". Apply the same own-pinned-team allowance as DeleteGlobalPolicies. Co-Authored-By: Claude Opus 5 --- openframe/docs/mysql-multitenancy-feature.md | 6 +- .../service/global_policies_openframe_test.go | 59 +++++++++++++++++++ server/service/team_policies.go | 14 ++++- 3 files changed, 76 insertions(+), 3 deletions(-) diff --git a/openframe/docs/mysql-multitenancy-feature.md b/openframe/docs/mysql-multitenancy-feature.md index ddc7dcb21b5..36eea6223aa 100644 --- a/openframe/docs/mysql-multitenancy-feature.md +++ b/openframe/docs/mysql-multitenancy-feature.md @@ -66,8 +66,10 @@ role-authz grouping upstream; here it is a hard boundary regardless of token rol the minor getters `HostLiteByIdentifier`/`HostLiteByID`, `ListHostsLiteByIDs`, `HostIDsByIdentifier`. Deliberately **unfenced**: `HostByUUID` (pre-auth iDevice identity lookup — no pin yet). - **policies** — list/count/by-id (`Policy`, `PolicyLite`, `PoliciesByID`), create→pinned, save - (verify-on-primary), delete (filter foreign ids); service-layer `DeleteGlobalPolicies` treats an - own-pinned-team policy as deletable (creation re-homes policies to the team). + (verify-on-primary), delete (filter foreign ids); service-layer `DeleteGlobalPolicies` and + `modifyPolicy` (the global modify endpoint) treat an own-pinned-team policy as global — creation + re-homes "global" policies to the pinned team, so the tenant UI's global endpoints must accept + them back. - **queries** — list/by-id/name, create→pinned, save-verify, delete, `ApplyQueries` re-home. - **enroll_secrets** (`app_configs.go`) — `GetEnrollSecrets`/`ApplyEnrollSecrets` force `teamID = pinned`; `VerifyEnrollSecret` only accepts a secret whose `team_id = pinned` (agent boundary). diff --git a/server/service/global_policies_openframe_test.go b/server/service/global_policies_openframe_test.go index a222f2f9894..b739a505912 100644 --- a/server/service/global_policies_openframe_test.go +++ b/server/service/global_policies_openframe_test.go @@ -69,3 +69,62 @@ func TestOpenframeDeleteGlobalPoliciesPinnedTeam(t *testing.T) { require.NoError(t, err) }) } + +// TestOpenframeModifyGlobalPolicyPinnedTeam verifies the OPENFRAME(mysql-multitenancy) adjustment +// in modifyPolicy: the tenant UI edits policies through the global endpoint, but creation re-homed +// them to the pinned team, so under a pin the upstream "policy does not belong to team/global" +// check must let own-team policies through. Foreign-team policies still reject, and unpinned +// behavior is unchanged. +func TestOpenframeModifyGlobalPolicyPinnedTeam(t *testing.T) { + const pinnedTeam = uint(7) + + newSvc := func(policyTeamID *uint) (fleet.Service, context.Context) { + ds := new(mock.Store) + ds.PolicyFunc = func(ctx context.Context, id uint) (*fleet.Policy, error) { + return &fleet.Policy{PolicyData: fleet.PolicyData{ID: id, TeamID: policyTeamID, Query: "SELECT 1"}}, nil + } + ds.SavePolicyFunc = func(ctx context.Context, p *fleet.Policy, shouldRemoveAllPolicyMemberships bool, removePolicyStats bool) error { + return nil + } + ds.AppConfigFunc = func(ctx context.Context) (*fleet.AppConfig, error) { + return &fleet.AppConfig{}, nil + } + svc, ctx := newTestService(t, ds, nil, nil) + ctx = viewer.NewContext(ctx, viewer.Viewer{User: &fleet.User{GlobalRole: ptr.String(fleet.RoleAdmin)}}) + return svc, ctx + } + + payload := fleet.ModifyPolicyPayload{Name: ptr.String("renamed")} + + t.Run("pinned: own-team policy modifies through the global endpoint", func(t *testing.T) { + svc, ctx := newSvc(ptr.Uint(pinnedTeam)) + ctx = fleet.NewOpenframeTeamContext(ctx, pinnedTeam) + + policy, err := svc.ModifyGlobalPolicy(ctx, 1, payload) + require.NoError(t, err) + require.Equal(t, "renamed", policy.Name) + }) + + t.Run("pinned: foreign-team policy still rejects", func(t *testing.T) { + svc, ctx := newSvc(ptr.Uint(pinnedTeam + 1)) + ctx = fleet.NewOpenframeTeamContext(ctx, pinnedTeam) + + _, err := svc.ModifyGlobalPolicy(ctx, 1, payload) + require.Error(t, err, "foreign-team policy must keep the bad-request reject") + }) + + t.Run("unpinned: team policy rejects (upstream behavior unchanged)", func(t *testing.T) { + svc, ctx := newSvc(ptr.Uint(pinnedTeam)) + + _, err := svc.ModifyGlobalPolicy(ctx, 1, payload) + require.Error(t, err, "unpinned team-policy modify must keep upstream's reject") + }) + + t.Run("unpinned: global (nil-team) policy modifies fine", func(t *testing.T) { + svc, ctx := newSvc(nil) + + policy, err := svc.ModifyGlobalPolicy(ctx, 1, payload) + require.NoError(t, err) + require.Equal(t, "renamed", policy.Name) + }) +} diff --git a/server/service/team_policies.go b/server/service/team_policies.go index 76c08178319..3177d3cda56 100644 --- a/server/service/team_policies.go +++ b/server/service/team_policies.go @@ -602,7 +602,19 @@ func (svc *Service) modifyPolicy(ctx context.Context, teamID *uint, id uint, p f return nil, err } - if ok := checkTeamID(teamID, policy); !ok { + ok := checkTeamID(teamID, policy) + // >>> OPENFRAME(mysql-multitenancy): under a per-request tenant pin the tenant's own policies + // carry the pinned team id (creation re-homes "global" policies to the pinned team), so from + // the tenant's perspective an own-team policy IS a global policy — the global modify endpoint + // must accept it. Same allowance as DeleteGlobalPolicies. Foreign policies never reach here: + // the fenced ds.Policy above already returned NotFound for them. Unpinned (flag off) keeps + // upstream's exact check. + if !ok && teamID == nil && policy.TeamID != nil { + pinned, pinnedOK := fleet.OpenframeTeamID(ctx) + ok = pinnedOK && *policy.TeamID == pinned + } + // <<< OPENFRAME(mysql-multitenancy) + if !ok { return nil, ctxerr.Wrap(ctx, &fleet.BadRequestError{ Message: "policy does not belong to team/global", InternalErr: fmt.Errorf("teamID: %+v, policy: %+v", teamID, policy),