IntenseWebs/mattermost
3
0
Fork 0
mirror of https://github.com/mattermost/mattermost.git synced 2026-08-26 13:17:29 -05:00
Code Issues Packages Projects Releases Wiki Activity

[MM-69734] Add no session data, ensure session attributes are not leaked through evaluation trace to non sysadmins (#37600)

Browse Source
This commit is contained in:
Devin Binnie
2026-07-30 08:42:38 -04:00
committed by GitHub
parent 8a9aacb0fb
commit b021e5be06
10 changed files with 325 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/channels/api4/access_control.go
+5
View File
@@ -681,6 +681,11 @@ func simulatePolicyForUsers(c *Context, w http.ResponseWriter, r *http.Request)
// the Decision Details panel and per-leaf ActualValue strings.
c.App.RedactSimulationAttributesForCaller(c.AppContext, resp, hasSystemPermission)
// Sanitize evaluation traces for non-system-admin callers.
// Traces are sysadmin-only; channel/team admins get flat
// expressions and the frontend falls back when trees are absent.
c.App.SanitizeSimulationEvaluationTracesForCaller(resp, hasSystemPermission)
js, err := json.Marshal(resp)
if err != nil {
c.Err = model.NewAppError("simulatePolicyForUsers", "api.marshal_error", nil, "", http.StatusInternalServerError).Wrap(err)
server/channels/api4/access_control_test.go
+127
View File
@@ -3118,6 +3118,133 @@ func TestSimulatePolicyForUsers(t *testing.T) {
require.Equal(t, http.StatusForbidden, resp.StatusCode)
mockACS.AssertNotCalled(t, "SimulatePolicyForUsers", mock.Anything, mock.Anything)
})
t.Run("channel admin response strips evaluation trees", func(t *testing.T) {
ok := th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuEnterpriseAdvanced))
require.True(t, ok)
updateTestFeatureFlags(t, th, func(cfg *model.Config) {
cfg.FeatureFlags.PermissionPolicies = true
cfg.FeatureFlags.PolicySimulation = true
cfg.AccessControlSettings.EnableAttributeBasedAccessControl = model.NewPointer(true)
})
defer updateTestFeatureFlags(t, th, restoreABACFeatureFlagDefaults)
th.AddPermissionToRole(t, model.PermissionManageChannelAccessRules.Id, model.ChannelAdminRoleId)
privateChannel := th.CreatePrivateChannel(t)
channelAdmin := th.CreateUser(t)
th.LinkUserToTeam(t, channelAdmin, th.BasicTeam)
th.AddUserToChannel(t, channelAdmin, privateChannel)
th.MakeUserChannelAdmin(t, channelAdmin, privateChannel)
channelAdminClient := th.CreateClient()
th.LoginBasicWithClient(t, channelAdminClient)
_, _, err := channelAdminClient.Login(context.Background(), channelAdmin.Email, channelAdmin.Password)
require.NoError(t, err)
evalTree := &model.PolicySimulationEvaluationNode{
Kind: model.PolicySimulationEvaluationKindAnd,
Outcome: model.PolicySimulationEvaluationOutcomeFalse,
}
mockACS := &mocks.AccessControlServiceInterface{}
mockACS.On("SimulatePolicyForUsers", mock.Anything, mock.Anything).Return(
&model.PolicySimulationResponse{
Results: []model.PolicySimulationUserResult{{
User: th.BasicUser,
Decisions: map[string]model.PolicySimulationActionDecision{
model.AccessControlPolicyActionUploadFileAttachment: {
Decision: false,
Blame: []model.PolicySimulationBlame{{
Source: model.PolicySimulationBlameSourceThisRule,
RuleName: "rule1",
Expression: "user.attributes.clearance == 'il5'",
EvaluationTree: evalTree,
}},
},
},
}},
Total: 1,
},
(*model.AppError)(nil),
)
th.App.Srv().Channels().AccessControl = mockACS
th.AddUserToChannel(t, th.BasicUser, privateChannel)
body := mustMarshal(t, model.PolicySimulationByUsersParams{
Policy: &model.AccessControlPolicy{ID: privateChannel.Id, Type: model.AccessControlPolicyTypeChannel, Version: model.AccessControlPolicyVersionV0_4},
Actions: []string{model.AccessControlPolicyActionUploadFileAttachment},
Users: []model.PolicySimulationUserOverride{{UserID: th.BasicUser.Id}},
ChannelID: privateChannel.Id,
})
resp, err := channelAdminClient.DoAPIPost(context.Background(), "/access_control_policies/cel/simulate_users", string(body))
require.NoError(t, err)
defer resp.Body.Close()
require.Equal(t, http.StatusOK, resp.StatusCode)
var out model.PolicySimulationResponse
require.NoError(t, json.NewDecoder(resp.Body).Decode(&out))
require.Len(t, out.Results, 1)
blame := out.Results[0].Decisions[model.AccessControlPolicyActionUploadFileAttachment].Blame[0]
require.Nil(t, blame.EvaluationTree)
require.Equal(t, "user.attributes.clearance == 'il5'", blame.Expression)
})
t.Run("system admin response keeps evaluation trees", func(t *testing.T) {
ok := th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuEnterpriseAdvanced))
require.True(t, ok)
updateTestFeatureFlags(t, th, func(cfg *model.Config) {
cfg.FeatureFlags.PermissionPolicies = true
cfg.FeatureFlags.PolicySimulation = true
cfg.AccessControlSettings.EnableAttributeBasedAccessControl = model.NewPointer(true)
})
defer updateTestFeatureFlags(t, th, restoreABACFeatureFlagDefaults)
evalTree := &model.PolicySimulationEvaluationNode{
Kind: model.PolicySimulationEvaluationKindAnd,
Outcome: model.PolicySimulationEvaluationOutcomeFalse,
}
mockACS := &mocks.AccessControlServiceInterface{}
mockACS.On("SimulatePolicyForUsers", mock.Anything, mock.Anything).Return(
&model.PolicySimulationResponse{
Results: []model.PolicySimulationUserResult{{
User: th.BasicUser,
Decisions: map[string]model.PolicySimulationActionDecision{
model.AccessControlPolicyActionUploadFileAttachment: {
Decision: false,
Blame: []model.PolicySimulationBlame{{
Source: model.PolicySimulationBlameSourceThisRule,
RuleName: "rule1",
Expression: "user.attributes.clearance == 'il5'",
EvaluationTree: evalTree,
}},
},
},
}},
Total: 1,
},
(*model.AppError)(nil),
)
th.App.Srv().Channels().AccessControl = mockACS
body := mustMarshal(t, model.PolicySimulationByUsersParams{
Policy: &model.AccessControlPolicy{ID: model.NewId(), Type: model.AccessControlPolicyTypeChannel, Version: model.AccessControlPolicyVersionV0_4},
Actions: []string{model.AccessControlPolicyActionUploadFileAttachment},
Users: []model.PolicySimulationUserOverride{{UserID: th.BasicUser.Id}},
})
resp, err := th.SystemAdminClient.DoAPIPost(context.Background(), "/access_control_policies/cel/simulate_users", string(body))
require.NoError(t, err)
defer resp.Body.Close()
require.Equal(t, http.StatusOK, resp.StatusCode)
var out model.PolicySimulationResponse
require.NoError(t, json.NewDecoder(resp.Body).Decode(&out))
require.Len(t, out.Results, 1)
blame := out.Results[0].Decisions[model.AccessControlPolicyActionUploadFileAttachment].Blame[0]
require.NotNil(t, blame.EvaluationTree)
require.Equal(t, model.PolicySimulationEvaluationKindAnd, blame.EvaluationTree.Kind)
})
}
func mustMarshal(t *testing.T, v any) []byte {
server/channels/app/access_control.go
+35
View File
@@ -1043,6 +1043,41 @@ func clearActualValuesInTree(node *model.PolicySimulationEvaluationNode) {
}
}
// SanitizeSimulationEvaluationTracesForCaller strips evaluation trace
// trees from a PolicySimulationResponse before it is returned to
// non-system-admin callers. Evaluation traces are sysadmin-only;
// channel/team admins still receive flat blame expressions where
// the simulator attached them, and the frontend falls back to
// rendering those when evaluation_tree is absent.
func (a *App) SanitizeSimulationEvaluationTracesForCaller(resp *model.PolicySimulationResponse, callerIsSystemAdmin bool) {
if resp == nil || callerIsSystemAdmin {
return
}
for i := range resp.Results {
r := &resp.Results[i]
for action, dec := range r.Decisions {
stripEvaluationTracesFromDecision(&dec)
r.Decisions[action] = dec
}
for j := range r.Sessions {
for action, dec := range r.Sessions[j].Decisions {
stripEvaluationTracesFromDecision(&dec)
r.Sessions[j].Decisions[action] = dec
}
}
}
}
func stripEvaluationTracesFromDecision(dec *model.PolicySimulationActionDecision) {
for i := range dec.Blame {
b := &dec.Blame[i]
b.EvaluationTree = nil
for j := range b.MergedRules {
b.MergedRules[j].EvaluationTree = nil
}
}
}
// enrichBlameForDraftScope walks the simulator response and:
// - copies the failing rule's expression into draft-side blame entries
// (this_rule / sibling_rule / sibling_saved) using params.Policy.Rules
server/channels/app/access_control_test.go
+83
View File
@@ -3949,6 +3949,89 @@ func TestRedactSimulationAttributesForCallerSystemAdminBypass(t *testing.T) {
mockValueStore.AssertExpectations(t)
}
func TestSanitizeSimulationEvaluationTracesForCaller(t *testing.T) {
mainHelper.Parallel(t)
th := Setup(t).InitBasic(t)
topLevelTree := &model.PolicySimulationEvaluationNode{
Kind: model.PolicySimulationEvaluationKindAnd,
Outcome: model.PolicySimulationEvaluationOutcomeFalse,
}
mergedRuleTree := &model.PolicySimulationEvaluationNode{
Kind: model.PolicySimulationEvaluationKindCompare,
Attribute: "user.attributes.clearance",
ActualValue: "il5",
Outcome: model.PolicySimulationEvaluationOutcomeFalse,
}
mkResp := func() *model.PolicySimulationResponse {
return &model.PolicySimulationResponse{
Results: []model.PolicySimulationUserResult{{
User: &model.User{Id: model.NewId()},
Decisions: map[string]model.PolicySimulationActionDecision{
"upload_file_attachment": {
Decision: false,
Blame: []model.PolicySimulationBlame{{
Source: model.PolicySimulationBlameSourceThisRule,
RuleName: "rule1",
Expression: "user.attributes.clearance == 'il5'",
EvaluationTree: topLevelTree,
MergedRules: []model.PolicySimulationMergedRule{{
Name: "rule1",
Expression: "user.attributes.clearance == 'il5'",
EvaluationTree: mergedRuleTree,
}},
}},
},
},
Sessions: []model.PolicySimulationSession{{
ID: "s1",
Decisions: map[string]model.PolicySimulationActionDecision{
"upload_file_attachment": {
Decision: false,
Blame: []model.PolicySimulationBlame{{
Source: model.PolicySimulationBlameSourceThisRule,
EvaluationTree: topLevelTree,
}},
},
},
}},
}},
}
}
t.Run("system admins keep evaluation trees", func(t *testing.T) {
resp := mkResp()
th.App.SanitizeSimulationEvaluationTracesForCaller(resp, true)
blame := resp.Results[0].Decisions["upload_file_attachment"].Blame[0]
require.NotNil(t, blame.EvaluationTree)
require.NotNil(t, blame.MergedRules[0].EvaluationTree)
sessionBlame := resp.Results[0].Sessions[0].Decisions["upload_file_attachment"].Blame[0]
require.NotNil(t, sessionBlame.EvaluationTree)
})
t.Run("non-system-admin callers get trees stripped", func(t *testing.T) {
resp := mkResp()
th.App.SanitizeSimulationEvaluationTracesForCaller(resp, false)
blame := resp.Results[0].Decisions["upload_file_attachment"].Blame[0]
assert.Nil(t, blame.EvaluationTree)
assert.Nil(t, blame.MergedRules[0].EvaluationTree)
assert.Equal(t, "user.attributes.clearance == 'il5'", blame.Expression)
assert.Equal(t, "user.attributes.clearance == 'il5'", blame.MergedRules[0].Expression)
sessionBlame := resp.Results[0].Sessions[0].Decisions["upload_file_attachment"].Blame[0]
assert.Nil(t, sessionBlame.EvaluationTree)
})
t.Run("nil response is a safe no-op", func(t *testing.T) {
require.NotPanics(t, func() {
th.App.SanitizeSimulationEvaluationTracesForCaller(nil, false)
})
})
}
// TestValidatePolicySimulationUsersInScopeChannel covers the channel-
// scope branch of the delegated-simulate input validator. The
// channel-scope branch is reached when a non-system-admin author
server/public/model/access_request.go
+7
View File
@@ -262,6 +262,13 @@ const (
// "Policy doesn't apply" pill from this entry. Never produced by
// production evaluation — simulation-only.
PolicySimulationBlameSourceNoApplicablePolicy = "no_applicable_policy"
// PolicySimulationBlameSourceNoSessionData is a synthetic blame source
// emitted by the simulator when a picked user has no cached session
// attributes (and no explicit session_overrides) but the action's
// contributing rules reference user.session.*. The decision is recorded
// as a vacuous ALLOW so the picker renders a neutral "No recent
// session" pill instead of a misleading deny. Simulation-only.
PolicySimulationBlameSourceNoSessionData = "no_session_data"
// PolicySimulationBlameSourceSiblingSaved is attached to an ALLOW
// decision when the rule the author is editing alone would have DENIED
// the subject, but a sibling rule (same role + action, OR-combined at
webapp/channels/src/components/admin_console/access_control/modals/simulate_access/decision_aggregate.test.ts
+12
View File
@@ -135,6 +135,18 @@ describe('aggregateDecisions', () => {
)).toBe('denied');
});
test('not-applicable when every action is no_session_data', () => {
const inapplicable = {
decision: true,
blame: [{source: POLICY_SIMULATION_BLAME_SOURCES.NO_SESSION_DATA}],
};
expect(aggregateDecisions(
['a', 'b'],
{a: inapplicable, b: inapplicable},
false,
)).toBe('not-applicable');
});
test('mixed no_applicable_rule + no_applicable_policy still rolls up to not-applicable', () => {
// The two synthetic markers can co-occur on different
// actions of the same row (e.g. one action falls outside
webapp/channels/src/components/admin_console/access_control/modals/simulate_access/decision_aggregate.ts
+2 -1
View File
@@ -81,7 +81,8 @@ export function aggregateDecisions(
if (dec.blame?.some((b) =>
(
b.source === POLICY_SIMULATION_BLAME_SOURCES.NO_APPLICABLE_POLICY ||
b.source === POLICY_SIMULATION_BLAME_SOURCES.NO_APPLICABLE_RULE
b.source === POLICY_SIMULATION_BLAME_SOURCES.NO_APPLICABLE_RULE ||
b.source === POLICY_SIMULATION_BLAME_SOURCES.NO_SESSION_DATA
) &&
b.outcome !== 'allow',
)) {
webapp/channels/src/components/admin_console/access_control/modals/simulate_access/decision_chip.tsx
+52 -9
View File
@@ -47,6 +47,10 @@ const blameSourceMessages = defineMessages({
id: 'admin.access_control.simulate_access.blame.no_applicable_rule',
defaultMessage: "this rule doesn't apply to this user",
},
[POLICY_SIMULATION_BLAME_SOURCES.NO_SESSION_DATA]: {
id: 'admin.access_control.simulate_access.blame.no_session_data',
defaultMessage: 'No recent session',
},
[POLICY_SIMULATION_BLAME_SOURCES.SIBLING_SAVED]: {
id: 'admin.access_control.simulate_access.blame.sibling_saved',
defaultMessage: 'another rule',
@@ -150,6 +154,21 @@ export default function DecisionChip({decision, pending}: Props): JSX.Element {
);
}
if (hasBlame(decision.blame, POLICY_SIMULATION_BLAME_SOURCES.NO_SESSION_DATA)) {
return (
<span
className='SimulateAccessModal__rowChip SimulateAccessModal__rowChip--not-applicable'
data-testid='simulate-access-row-chip-no-session-data'
>
<MinusCircleOutlineIcon
size={ICON_SIZE}
className='SimulateAccessModal__rowChipIcon'
/>
<FormattedMessage {...blameSourceMessages[POLICY_SIMULATION_BLAME_SOURCES.NO_SESSION_DATA]}/>
</span>
);
}
// ALLOW with sibling_saved blame: the editing rule alone would
// have denied. Surface that inline so authors can spot
// "OR-saved" allows. Only reached when no_applicable_rule
@@ -196,23 +215,47 @@ export default function DecisionChip({decision, pending}: Props): JSX.Element {
// governed but the subject's role doesn't match. Render the same
// neutral "doesn't apply" pill rather than a hard "Denied" chip so
// the UX stays consistent regardless of evaluation scope.
if (hasBlame(decision.blame, POLICY_SIMULATION_BLAME_SOURCES.NO_APPLICABLE_RULE) ||
hasBlame(decision.blame, POLICY_SIMULATION_BLAME_SOURCES.NO_APPLICABLE_POLICY)) {
const source = hasBlame(decision.blame, POLICY_SIMULATION_BLAME_SOURCES.NO_APPLICABLE_RULE) ?
POLICY_SIMULATION_BLAME_SOURCES.NO_APPLICABLE_RULE :
POLICY_SIMULATION_BLAME_SOURCES.NO_APPLICABLE_POLICY;
if (hasBlame(decision.blame, POLICY_SIMULATION_BLAME_SOURCES.NO_APPLICABLE_RULE)) {
return (
<span
className='SimulateAccessModal__rowChip SimulateAccessModal__rowChip--not-applicable'
data-testid={source === POLICY_SIMULATION_BLAME_SOURCES.NO_APPLICABLE_RULE ?
'simulate-access-row-chip-not-applicable-rule' :
'simulate-access-row-chip-not-applicable'}
data-testid='simulate-access-row-chip-not-applicable-rule'
>
<MinusCircleOutlineIcon
size={ICON_SIZE}
className='SimulateAccessModal__rowChipIcon'
/>
<FormattedMessage {...blameSourceMessages[source]}/>
<FormattedMessage {...blameSourceMessages[POLICY_SIMULATION_BLAME_SOURCES.NO_APPLICABLE_RULE]}/>
</span>
);
}
if (hasBlame(decision.blame, POLICY_SIMULATION_BLAME_SOURCES.NO_SESSION_DATA)) {
return (
<span
className='SimulateAccessModal__rowChip SimulateAccessModal__rowChip--not-applicable'
data-testid='simulate-access-row-chip-no-session-data'
>
<MinusCircleOutlineIcon
size={ICON_SIZE}
className='SimulateAccessModal__rowChipIcon'
/>
<FormattedMessage {...blameSourceMessages[POLICY_SIMULATION_BLAME_SOURCES.NO_SESSION_DATA]}/>
</span>
);
}
if (hasBlame(decision.blame, POLICY_SIMULATION_BLAME_SOURCES.NO_APPLICABLE_POLICY)) {
return (
<span
className='SimulateAccessModal__rowChip SimulateAccessModal__rowChip--not-applicable'
data-testid='simulate-access-row-chip-not-applicable'
>
<MinusCircleOutlineIcon
size={ICON_SIZE}
className='SimulateAccessModal__rowChipIcon'
/>
<FormattedMessage {...blameSourceMessages[POLICY_SIMULATION_BLAME_SOURCES.NO_APPLICABLE_POLICY]}/>
</span>
);
}
webapp/channels/src/i18n/en.json
+1
View File
@@ -363,6 +363,7 @@
"admin.access_control.simulate_access.blame.channel_policy": "parent policy",
"admin.access_control.simulate_access.blame.no_applicable_policy": "policy doesn't apply to this user",
"admin.access_control.simulate_access.blame.no_applicable_rule": "this rule doesn't apply to this user",
"admin.access_control.simulate_access.blame.no_session_data": "No recent session",
"admin.access_control.simulate_access.blame.peer_policy": "another policy",
"admin.access_control.simulate_access.blame.sibling_rule": "another rule",
"admin.access_control.simulate_access.blame.sibling_saved": "another rule",
webapp/platform/types/src/access_control.ts
+1
View File
@@ -262,6 +262,7 @@ export const POLICY_SIMULATION_BLAME_SOURCES = {
PEER_POLICY: 'peer_policy',
NO_APPLICABLE_POLICY: 'no_applicable_policy',
NO_APPLICABLE_RULE: 'no_applicable_rule',
NO_SESSION_DATA: 'no_session_data',
SIBLING_SAVED: 'sibling_saved',
} as const;