MM-68501 - implement GetMaskedVisualAST and wire API handler (#36413)

* MM-68501 - implement GetMaskedVisualAST and wire API handler

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* add missing test and fix style issues

* fix styles

* implement coderabbit feedback

* MM-68501 - PR review: split masking file, model-level access mode, reject contradictory config

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* MM-68501 - apply shared_only filter to non-option field values (binary masking)

* MM-68501 - consolidate masking flag check and log corrupt text value during masking

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Mattermost Build <build@mattermost.com>
This commit is contained in:
Pablo Vélez
2026-05-12 18:26:08 +02:00
committed by GitHub
co-authored by Claude Opus 4.6 Mattermost Build
parent e3fbf8711f
commit 5661fe1941
9 changed files with 1126 additions and 33 deletions
+18 -1
View File
@@ -14,6 +14,14 @@ import (
"github.com/mattermost/mattermost/server/v8/channels/app"
)
// shouldRedactExpressions reports whether raw CEL expressions should be masked for this caller.
// Returns true when both ABAC and attribute-value masking are enabled. Callers reading raw expressions
// in a policy must also receive redacted raw expressions.
func shouldRedactExpressions(c *Context) bool {
return c.App.Config().FeatureFlags.AttributeBasedAccessControl &&
c.App.Config().FeatureFlags.AttributeValueMasking
}
func (api *API) InitAccessControlPolicy() {
if !api.srv.Config().FeatureFlags.AttributeBasedAccessControl {
return
@@ -995,7 +1003,16 @@ func convertToVisualAST(c *Context, w http.ResponseWriter, r *http.Request) {
}
}
}
visualAST, appErr := c.App.ExpressionToVisualAST(c.AppContext, cel.Expression)
var visualAST *model.VisualExpression
var appErr *model.AppError
// Masking is attribute-based, not permission-based: all admins receive a
// filtered AST based on what they themselves hold, regardless of role.
if shouldRedactExpressions(c) {
visualAST, appErr = c.App.GetMaskedVisualAST(c.AppContext, cel.Expression, c.AppContext.Session().UserId)
} else {
visualAST, appErr = c.App.ExpressionToVisualAST(c.AppContext, cel.Expression)
}
if appErr != nil {
c.Err = appErr
return
@@ -0,0 +1,247 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
package app
import (
"encoding/json"
"net/http"
"strings"
"github.com/mattermost/mattermost/server/public/model"
"github.com/mattermost/mattermost/server/public/shared/mlog"
"github.com/mattermost/mattermost/server/public/shared/request"
)
// GetMaskedVisualAST converts the given CEL expression to a VisualExpression and
// filters each condition's literal values to the subset visible to callerID.
//
// Masking is attribute-based, not role-based: every caller (including system
// admins) sees only values they themselves hold for shared_only fields, all
// values for public fields, and no values for source_only fields. Conditions
// whose values are partially or fully filtered get HasMaskedValues=true so the
// client can render the masked-chip UI.
func (a *App) GetMaskedVisualAST(rctx request.CTX, expression string, callerID string) (*model.VisualExpression, *model.AppError) {
visualAST, appErr := a.ExpressionToVisualAST(rctx, expression)
if appErr != nil {
return nil, appErr
}
if visualAST == nil || len(visualAST.Conditions) == 0 {
return visualAST, nil
}
cpaGroupID, appErr := a.CpaGroupID()
if appErr != nil {
return nil, model.NewAppError("GetMaskedVisualAST", "app.pap.get_masked_visual_ast.app_error", nil, "", http.StatusInternalServerError).Wrap(appErr)
}
// Embed callerID in context so GetPropertyFieldByName applies per-caller option filtering.
rctxWithCaller := RequestContextWithCallerID(rctx, callerID)
// Pre-fetch all referenced fields once to avoid N+1 DB queries across conditions.
fieldsByName := a.fetchConditionFields(rctxWithCaller, visualAST.Conditions, cpaGroupID)
for i := range visualAST.Conditions {
a.maskConditionValues(rctxWithCaller, callerID, &visualAST.Conditions[i], cpaGroupID, fieldsByName)
}
return visualAST, nil
}
// fetchConditionFields collects unique field names from conditions and fetches each once.
// Fields that fail lookup are omitted; maskConditionValues treats missing entries as fail-closed.
func (a *App) fetchConditionFields(rctx request.CTX, conditions []model.Condition, cpaGroupID string) map[string]*model.PropertyField {
seen := make(map[string]bool)
for _, c := range conditions {
if c.ValueType == model.AttrValue {
continue
}
if name := extractFieldName(c.Attribute); name != "" {
seen[name] = true
}
}
fields := make(map[string]*model.PropertyField, len(seen))
for name := range seen {
field, appErr := a.GetPropertyFieldByName(rctx, cpaGroupID, "", name)
if appErr != nil {
rctx.Logger().Warn("Failed to look up field for masking, failing closed",
mlog.String("field_name", name),
mlog.Err(appErr),
)
continue
}
fields[name] = field
}
return fields
}
// maskConditionValues applies masking to a single condition in place.
//
// Masking semantics differ by field type:
//
// - select / multiselect (partial masking): each value in a multi-value
// condition is independently masked or visible. A row may end up with some
// visible chips plus the masked-token covering the omitted values.
// - text (binary masking): a text condition is a single string comparison.
// The condition's value is either visible in full (the caller's stored
// text value matches it exactly) or fully masked. No partial chip behavior
// is possible because there's no multi-value list to filter.
func (a *App) maskConditionValues(rctx request.CTX, callerID string, condition *model.Condition, cpaGroupID string, fieldsByName map[string]*model.PropertyField) {
// AttrValue conditions compare two attributes (e.g. user.attr1 == user.attr2) — no literal values to mask.
if condition.ValueType == model.AttrValue {
return
}
fieldName := extractFieldName(condition.Attribute)
if fieldName == "" {
return
}
field, ok := fieldsByName[fieldName]
if !ok {
// Fail closed: field lookup failed at prefetch time.
condition.Value = nil
condition.HasMaskedValues = true
return
}
switch field.GetAccessMode() {
case model.PropertyAccessModePublic:
// no-op
case model.PropertyAccessModeSourceOnly:
condition.Value = nil
condition.HasMaskedValues = true
case model.PropertyAccessModeSharedOnly:
if field.Type == model.PropertyFieldTypeSelect || field.Type == model.PropertyFieldTypeMultiselect {
filterConditionValues(condition, extractVisibleOptionNames(field))
} else {
filterConditionValues(condition, a.getCallerTextValues(rctx, callerID, field, cpaGroupID))
}
default:
// Unknown access mode: fail closed.
condition.Value = nil
condition.HasMaskedValues = true
}
}
// extractFieldName strips the "user.attributes." prefix from a CEL attribute
// reference, returning just the property field name. Returns the empty string
// if the attribute is not a user-attribute reference.
func extractFieldName(attribute string) string {
const prefix = "user.attributes."
name := strings.TrimPrefix(attribute, prefix)
if name == attribute || name == "" {
return ""
}
return name
}
// extractVisibleOptionNames pulls option names from a pre-filtered PropertyField's
// Attrs["options"]. The field is expected to have already been filtered by
// PropertyAccessService.applyFieldReadAccessControl to the caller's holdings,
// so the names returned here are exactly what the caller can see.
func extractVisibleOptionNames(field *model.PropertyField) map[string]struct{} {
names := make(map[string]struct{})
if field.Attrs == nil {
return names
}
optionsRaw, ok := field.Attrs[model.PropertyFieldAttributeOptions]
if !ok {
return names
}
optionsSlice, ok := optionsRaw.([]any)
if !ok {
return names
}
for _, opt := range optionsSlice {
optMap, ok := opt.(map[string]any)
if !ok {
continue
}
name, ok := optMap["name"].(string)
if ok && name != "" {
names[name] = struct{}{}
}
}
return names
}
// getCallerTextValues returns the caller's stored text value(s) for the given
// text-type field, as the visible-names set used by filterConditionValues.
// A user has at most one text value per field, so this set has zero or one
// element. Empty values are treated as no value.
func (a *App) getCallerTextValues(rctx request.CTX, callerID string, field *model.PropertyField, cpaGroupID string) map[string]struct{} {
visible := make(map[string]struct{})
// Each (user, field) pair has at most one text value.
values, appErr := a.SearchPropertyValues(rctx, cpaGroupID, model.PropertyValueSearchOpts{
FieldID: field.ID,
TargetIDs: []string{callerID},
PerPage: 1,
})
if appErr != nil {
rctx.Logger().Warn("Failed to look up caller text value for masking, failing closed",
mlog.String("field_id", field.ID),
mlog.String("caller_id", callerID),
mlog.Err(appErr),
)
return visible
}
for _, pv := range values {
var textVal string
if err := json.Unmarshal(pv.Value, &textVal); err != nil {
rctx.Logger().Warn("Failed to unmarshal caller text value for masking, treating as no value",
mlog.String("field_id", field.ID),
mlog.String("caller_id", callerID),
mlog.String("value_id", pv.ID),
mlog.Err(err),
)
continue
}
if textVal != "" {
visible[textVal] = struct{}{}
}
}
return visible
}
// filterConditionValues drops any element of condition.Value that is not in the
// visibleNames set, setting HasMaskedValues=true if anything was dropped.
//
// For multi-value conditions ([]any), each string element is checked individually
// (partial masking). For single-value conditions (string), the whole value is
// either kept or replaced with nil (binary masking).
func filterConditionValues(condition *model.Condition, visibleNames map[string]struct{}) {
switch v := condition.Value.(type) {
case []any:
filtered := make([]any, 0, len(v))
totalStrings := 0
for _, val := range v {
strVal, ok := val.(string)
if !ok {
continue // non-string elements are not masking candidates
}
totalStrings++
if _, visible := visibleNames[strVal]; visible {
filtered = append(filtered, val)
}
}
if len(filtered) < totalStrings {
condition.HasMaskedValues = true
}
condition.Value = filtered
case string:
if _, visible := visibleNames[v]; !visible {
condition.Value = nil
condition.HasMaskedValues = true
}
}
}
@@ -0,0 +1,666 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
package app
import (
"encoding/json"
"testing"
"github.com/mattermost/mattermost/server/public/model"
"github.com/mattermost/mattermost/server/public/shared/request"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
"github.com/mattermost/mattermost/server/v8/einterfaces/mocks"
)
func TestExtractFieldName(t *testing.T) {
tests := []struct {
name string
attribute string
expected string
}{
{"standard attribute path", "user.attributes.Program", "Program"},
{"multi-word field", "user.attributes.Clearance Level", "Clearance Level"},
{"no prefix", "Program", ""},
{"partial prefix", "user.attributes.", ""},
{"empty string", "", ""},
{"different prefix", "team.attributes.Program", ""},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
result := extractFieldName(tc.attribute)
assert.Equal(t, tc.expected, result)
})
}
}
// Note: tests for field.GetAccessMode() live in model/property_access_test.go,
// where the method is defined (TestPropertyFieldGetAccessMode).
func TestExtractVisibleOptionNames(t *testing.T) {
t.Run("extracts names from valid options", func(t *testing.T) {
field := &model.PropertyField{
Attrs: model.StringInterface{
model.PropertyFieldAttributeOptions: []any{
map[string]any{"id": "id1", "name": "Alpha", "color": "red"},
map[string]any{"id": "id2", "name": "Bravo", "color": "blue"},
},
},
}
names := extractVisibleOptionNames(field)
assert.Len(t, names, 2)
assert.Contains(t, names, "Alpha")
assert.Contains(t, names, "Bravo")
})
t.Run("returns empty set for nil attrs", func(t *testing.T) {
field := &model.PropertyField{Attrs: nil}
names := extractVisibleOptionNames(field)
assert.Empty(t, names)
})
t.Run("returns empty set for empty options", func(t *testing.T) {
field := &model.PropertyField{
Attrs: model.StringInterface{
model.PropertyFieldAttributeOptions: []any{},
},
}
names := extractVisibleOptionNames(field)
assert.Empty(t, names)
})
t.Run("skips options without name field", func(t *testing.T) {
field := &model.PropertyField{
Attrs: model.StringInterface{
model.PropertyFieldAttributeOptions: []any{
map[string]any{"id": "id1", "name": "Alpha"},
map[string]any{"id": "id2"}, // no name
},
},
}
names := extractVisibleOptionNames(field)
assert.Len(t, names, 1)
assert.Contains(t, names, "Alpha")
})
t.Run("skips empty name", func(t *testing.T) {
field := &model.PropertyField{
Attrs: model.StringInterface{
model.PropertyFieldAttributeOptions: []any{
map[string]any{"id": "id1", "name": ""},
},
},
}
names := extractVisibleOptionNames(field)
assert.Empty(t, names)
})
}
func TestFilterConditionValues(t *testing.T) {
t.Run("multi-value: filters to visible only, sets HasMaskedValues", func(t *testing.T) {
condition := &model.Condition{
Attribute: "user.attributes.Program",
Operator: "in",
Value: []any{"Alpha", "Bravo", "Charlie"},
ValueType: model.LiteralValue,
AttributeType: "multiselect",
}
visibleNames := map[string]struct{}{"Alpha": {}}
filterConditionValues(condition, visibleNames)
values, ok := condition.Value.([]any)
require.True(t, ok)
assert.Equal(t, []any{"Alpha"}, values)
assert.True(t, condition.HasMaskedValues)
})
t.Run("multi-value: all visible, no masking", func(t *testing.T) {
condition := &model.Condition{
Attribute: "user.attributes.Program",
Operator: "in",
Value: []any{"Alpha", "Bravo"},
ValueType: model.LiteralValue,
AttributeType: "multiselect",
}
visibleNames := map[string]struct{}{"Alpha": {}, "Bravo": {}}
filterConditionValues(condition, visibleNames)
values, ok := condition.Value.([]any)
require.True(t, ok)
assert.Equal(t, []any{"Alpha", "Bravo"}, values)
assert.False(t, condition.HasMaskedValues)
})
t.Run("multi-value: none visible, all masked", func(t *testing.T) {
condition := &model.Condition{
Attribute: "user.attributes.Program",
Operator: "in",
Value: []any{"Alpha", "Bravo"},
ValueType: model.LiteralValue,
AttributeType: "multiselect",
}
visibleNames := map[string]struct{}{}
filterConditionValues(condition, visibleNames)
values, ok := condition.Value.([]any)
require.True(t, ok)
assert.Empty(t, values)
assert.True(t, condition.HasMaskedValues)
})
t.Run("single value: visible, no masking", func(t *testing.T) {
condition := &model.Condition{
Attribute: "user.attributes.Location",
Operator: "==",
Value: "Building 1",
ValueType: model.LiteralValue,
AttributeType: "select",
}
visibleNames := map[string]struct{}{"Building 1": {}}
filterConditionValues(condition, visibleNames)
assert.Equal(t, "Building 1", condition.Value)
assert.False(t, condition.HasMaskedValues)
})
t.Run("single value: not visible, masked", func(t *testing.T) {
condition := &model.Condition{
Attribute: "user.attributes.Location",
Operator: "==",
Value: "Building 7",
ValueType: model.LiteralValue,
AttributeType: "select",
}
visibleNames := map[string]struct{}{"Building 1": {}}
filterConditionValues(condition, visibleNames)
assert.Nil(t, condition.Value)
assert.True(t, condition.HasMaskedValues)
})
t.Run("non-string value: skipped without masking", func(t *testing.T) {
condition := &model.Condition{
Attribute: "user.attributes.Active",
Operator: "==",
Value: true,
ValueType: model.LiteralValue,
AttributeType: "text",
}
visibleNames := map[string]struct{}{}
filterConditionValues(condition, visibleNames)
assert.Equal(t, true, condition.Value)
assert.False(t, condition.HasMaskedValues)
})
t.Run("slice with non-string elements: non-strings excluded from masking count", func(t *testing.T) {
// A []any with non-string elements should not trigger HasMaskedValues —
// non-strings are not masking candidates, not masked values.
condition := &model.Condition{
Attribute: "user.attributes.Program",
Operator: "in",
Value: []any{true, 42, "Alpha"},
ValueType: model.LiteralValue,
AttributeType: "multiselect",
}
visibleNames := map[string]struct{}{"Alpha": {}}
filterConditionValues(condition, visibleNames)
values, ok := condition.Value.([]any)
require.True(t, ok)
assert.Equal(t, []any{"Alpha"}, values)
assert.False(t, condition.HasMaskedValues) // only string "Alpha" counted; it's visible
})
t.Run("nil value: skipped", func(t *testing.T) {
condition := &model.Condition{
Attribute: "user.attributes.Program",
Operator: "==",
Value: nil,
ValueType: model.LiteralValue,
}
visibleNames := map[string]struct{}{}
filterConditionValues(condition, visibleNames)
assert.Nil(t, condition.Value)
assert.False(t, condition.HasMaskedValues)
})
}
// TestAttrValueSkip_FilterConditionValuesNotCalled documents why the AttrValue early-return in
// maskConditionValues is necessary: without it, filterConditionValues would treat the attribute
// path string (e.g. "user.attributes.Department") as a literal value and mask it.
func TestAttrValueSkip_FilterConditionValuesNotCalled(t *testing.T) {
condition := &model.Condition{
Attribute: "user.attributes.Team",
Operator: "==",
Value: "user.attributes.Department", // attribute path, not a literal
ValueType: model.AttrValue,
}
// If filterConditionValues were called on an AttrValue condition, it would
// incorrectly mask the attribute path since it won't be in the visible set.
filterConditionValues(condition, map[string]struct{}{})
assert.Nil(t, condition.Value)
assert.True(t, condition.HasMaskedValues)
// This confirms maskConditionValues MUST return early for AttrValue before
// reaching filterConditionValues. The early-return path in maskConditionValues
// itself is not exercised here; this test only documents why that guard is required.
}
func TestFilterConditionValues_EmptySlice(t *testing.T) {
condition := &model.Condition{
Attribute: "user.attributes.Program",
Operator: "in",
Value: []any{},
ValueType: model.LiteralValue,
AttributeType: "multiselect",
}
visibleNames := map[string]struct{}{"Alpha": {}}
filterConditionValues(condition, visibleNames)
values, ok := condition.Value.([]any)
require.True(t, ok)
assert.Empty(t, values)
assert.False(t, condition.HasMaskedValues) // nothing was filtered
}
func TestFilterConditionValues_TextFieldMasking(t *testing.T) {
// Text field masking uses the same filterConditionValues function,
// but the visible set comes from the caller's actual text value
// instead of field options.
t.Run("text field with in operator: caller holds matching value", func(t *testing.T) {
condition := &model.Condition{
Attribute: "user.attributes.Clearance",
Operator: "in",
Value: []any{"Top Secret", "Secret", "Confidential"},
ValueType: model.LiteralValue,
AttributeType: "text",
}
// Caller holds "Top Secret" — only this value should be visible
callerTextValues := map[string]struct{}{"Top Secret": {}}
filterConditionValues(condition, callerTextValues)
values, ok := condition.Value.([]any)
require.True(t, ok)
assert.Equal(t, []any{"Top Secret"}, values)
assert.True(t, condition.HasMaskedValues)
})
t.Run("text field with in operator: caller holds no matching value", func(t *testing.T) {
condition := &model.Condition{
Attribute: "user.attributes.Clearance",
Operator: "in",
Value: []any{"Top Secret", "Secret"},
ValueType: model.LiteralValue,
AttributeType: "text",
}
// Caller holds "Unclassified" — none of the policy values match
callerTextValues := map[string]struct{}{"Unclassified": {}}
filterConditionValues(condition, callerTextValues)
values, ok := condition.Value.([]any)
require.True(t, ok)
assert.Empty(t, values)
assert.True(t, condition.HasMaskedValues)
})
t.Run("text field with == operator: caller holds matching value", func(t *testing.T) {
condition := &model.Condition{
Attribute: "user.attributes.Clearance",
Operator: "==",
Value: "Top Secret",
ValueType: model.LiteralValue,
AttributeType: "text",
}
callerTextValues := map[string]struct{}{"Top Secret": {}}
filterConditionValues(condition, callerTextValues)
assert.Equal(t, "Top Secret", condition.Value)
assert.False(t, condition.HasMaskedValues)
})
t.Run("text field with == operator: caller holds different value", func(t *testing.T) {
condition := &model.Condition{
Attribute: "user.attributes.Clearance",
Operator: "==",
Value: "Top Secret",
ValueType: model.LiteralValue,
AttributeType: "text",
}
callerTextValues := map[string]struct{}{"Secret": {}}
filterConditionValues(condition, callerTextValues)
assert.Nil(t, condition.Value)
assert.True(t, condition.HasMaskedValues)
})
t.Run("text field with is not operator: caller holds no value", func(t *testing.T) {
condition := &model.Condition{
Attribute: "user.attributes.Location",
Operator: "!=",
Value: "Building 7",
ValueType: model.LiteralValue,
AttributeType: "text",
}
// Caller has no value for Location — empty visible set
callerTextValues := map[string]struct{}{}
filterConditionValues(condition, callerTextValues)
assert.Nil(t, condition.Value)
assert.True(t, condition.HasMaskedValues)
})
}
func TestMaskConditionValues(t *testing.T) {
rctx := request.TestContext(t)
// nil App is safe for every branch that does not reach a.getCallerTextValues
// (i.e., everything except shared_only + text field, which needs a real store).
var a *App
makeField := func(accessMode string, fieldType model.PropertyFieldType, options []any) *model.PropertyField {
attrs := model.StringInterface{model.PropertyAttrsAccessMode: accessMode}
if options != nil {
attrs[model.PropertyFieldAttributeOptions] = options
}
return &model.PropertyField{Type: fieldType, Attrs: attrs}
}
options := []any{
map[string]any{"id": "id1", "name": "Alpha"},
map[string]any{"id": "id2", "name": "Bravo"},
}
t.Run("AttrValue condition: returns immediately, value untouched", func(t *testing.T) {
condition := &model.Condition{
Attribute: "user.attributes.Team",
Value: "user.attributes.Department",
ValueType: model.AttrValue,
}
a.maskConditionValues(rctx, "caller", condition, "", nil)
assert.Equal(t, "user.attributes.Department", condition.Value)
assert.False(t, condition.HasMaskedValues)
})
t.Run("non-user-attribute path: returns immediately, value untouched", func(t *testing.T) {
condition := &model.Condition{
Attribute: "team.attributes.Program",
Value: "Engineering",
ValueType: model.LiteralValue,
}
a.maskConditionValues(rctx, "caller", condition, "", map[string]*model.PropertyField{})
assert.Equal(t, "Engineering", condition.Value)
assert.False(t, condition.HasMaskedValues)
})
t.Run("field missing from prefetch map: fail-closed", func(t *testing.T) {
condition := &model.Condition{
Attribute: "user.attributes.Program",
Value: "Alpha",
ValueType: model.LiteralValue,
}
a.maskConditionValues(rctx, "caller", condition, "", map[string]*model.PropertyField{})
assert.Nil(t, condition.Value)
assert.True(t, condition.HasMaskedValues)
})
t.Run("public field: value passes through unchanged", func(t *testing.T) {
condition := &model.Condition{
Attribute: "user.attributes.Program",
Value: "Alpha",
ValueType: model.LiteralValue,
}
fields := map[string]*model.PropertyField{
"Program": makeField(model.PropertyAccessModePublic, model.PropertyFieldTypeSelect, options),
}
a.maskConditionValues(rctx, "caller", condition, "", fields)
assert.Equal(t, "Alpha", condition.Value)
assert.False(t, condition.HasMaskedValues)
})
t.Run("source_only field: value is nil'd and masked", func(t *testing.T) {
condition := &model.Condition{
Attribute: "user.attributes.Clearance",
Value: "Top Secret",
ValueType: model.LiteralValue,
}
fields := map[string]*model.PropertyField{
"Clearance": makeField(model.PropertyAccessModeSourceOnly, model.PropertyFieldTypeSelect, options),
}
a.maskConditionValues(rctx, "caller", condition, "", fields)
assert.Nil(t, condition.Value)
assert.True(t, condition.HasMaskedValues)
})
t.Run("shared_only select: visible option kept, hidden option masked", func(t *testing.T) {
condition := &model.Condition{
Attribute: "user.attributes.Location",
Value: "Alpha",
ValueType: model.LiteralValue,
}
fields := map[string]*model.PropertyField{
"Location": makeField(model.PropertyAccessModeSharedOnly, model.PropertyFieldTypeSelect, options),
}
a.maskConditionValues(rctx, "caller", condition, "", fields)
// "Alpha" is in the field options so it is visible
assert.Equal(t, "Alpha", condition.Value)
assert.False(t, condition.HasMaskedValues)
})
t.Run("shared_only select: value not in options is masked", func(t *testing.T) {
condition := &model.Condition{
Attribute: "user.attributes.Location",
Value: "Charlie",
ValueType: model.LiteralValue,
}
fields := map[string]*model.PropertyField{
"Location": makeField(model.PropertyAccessModeSharedOnly, model.PropertyFieldTypeSelect, options),
}
a.maskConditionValues(rctx, "caller", condition, "", fields)
assert.Nil(t, condition.Value)
assert.True(t, condition.HasMaskedValues)
})
t.Run("shared_only multiselect: visible values kept, hidden values masked", func(t *testing.T) {
condition := &model.Condition{
Attribute: "user.attributes.Programs",
Value: []any{"Alpha", "Charlie"},
ValueType: model.LiteralValue,
}
fields := map[string]*model.PropertyField{
"Programs": makeField(model.PropertyAccessModeSharedOnly, model.PropertyFieldTypeMultiselect, options),
}
a.maskConditionValues(rctx, "caller", condition, "", fields)
values, ok := condition.Value.([]any)
require.True(t, ok)
assert.Equal(t, []any{"Alpha"}, values)
assert.True(t, condition.HasMaskedValues)
})
t.Run("unknown access mode: fail-closed", func(t *testing.T) {
condition := &model.Condition{
Attribute: "user.attributes.Program",
Value: "Alpha",
ValueType: model.LiteralValue,
}
fields := map[string]*model.PropertyField{
"Program": {
Type: model.PropertyFieldTypeSelect,
Attrs: model.StringInterface{model.PropertyAttrsAccessMode: "future_unknown_mode"},
},
}
a.maskConditionValues(rctx, "caller", condition, "", fields)
assert.Nil(t, condition.Value)
assert.True(t, condition.HasMaskedValues)
})
}
// TestMaskConditionValues_SharedOnlyText covers the shared_only + text-field branch of
// maskConditionValues, which requires a real store to call getCallerTextValues →
// SearchPropertyValues. This branch is intentionally skipped in TestMaskConditionValues
// (which uses a nil App).
//
// A non-CPA V1 group is used so that field creation does not go through the CPA
// access-control layer (which requires a plugin caller for protected/shared_only fields).
// The group ID is passed explicitly to maskConditionValues so store lookups use the same group.
func TestMaskConditionValues_SharedOnlyText(t *testing.T) {
mainHelper.Parallel(t)
th := Setup(t).InitBasic(t)
rctx := request.TestContext(t)
callerID := model.NewId()
// Register a plain V1 group — no access-control overhead for this group.
group, appErr := th.App.RegisterPropertyGroup(rctx, &model.PropertyGroup{
Name: "masking_text_test_" + model.NewId(),
Version: model.PropertyGroupVersionV1,
})
require.Nil(t, appErr)
groupID := group.ID
// Create a text field and set shared_only in its Attrs.
// shared_only normally requires protected=true which is plugin-only in the CPA
// group; in this non-CPA group the access-control layer is not applied, so the
// field is written directly.
field := &model.PropertyField{
GroupID: groupID,
Name: "f_" + model.NewId(),
Type: model.PropertyFieldTypeText,
Attrs: model.StringInterface{model.PropertyAttrsAccessMode: model.PropertyAccessModeSharedOnly},
}
createdField, err := th.App.CreatePropertyField(rctx, field, false, "")
require.Nil(t, err)
// Store "Engineering" as the caller's value for this field.
_, appErr = th.App.CreatePropertyValue(rctx, &model.PropertyValue{
TargetID: callerID,
TargetType: model.PropertyValueTargetTypeUser,
GroupID: groupID,
FieldID: createdField.ID,
Value: json.RawMessage(`"Engineering"`),
})
require.Nil(t, appErr)
fieldsByName := map[string]*model.PropertyField{createdField.Name: createdField}
t.Run("caller's own value passes through", func(t *testing.T) {
condition := &model.Condition{
Attribute: "user.attributes." + createdField.Name,
Value: "Engineering",
ValueType: model.LiteralValue,
}
th.App.maskConditionValues(rctx, callerID, condition, groupID, fieldsByName)
assert.Equal(t, "Engineering", condition.Value)
assert.False(t, condition.HasMaskedValues)
})
t.Run("value the caller does not hold is masked", func(t *testing.T) {
condition := &model.Condition{
Attribute: "user.attributes." + createdField.Name,
Value: "Finance",
ValueType: model.LiteralValue,
}
th.App.maskConditionValues(rctx, callerID, condition, groupID, fieldsByName)
assert.Nil(t, condition.Value)
assert.True(t, condition.HasMaskedValues)
})
t.Run("caller with no stored value is fail-closed", func(t *testing.T) {
condition := &model.Condition{
Attribute: "user.attributes." + createdField.Name,
Value: "Engineering",
ValueType: model.LiteralValue,
}
th.App.maskConditionValues(rctx, model.NewId(), condition, groupID, fieldsByName)
assert.Nil(t, condition.Value)
assert.True(t, condition.HasMaskedValues)
})
}
// TestGetMaskedVisualAST_Wiring validates the orchestration inside GetMaskedVisualAST:
// ExpressionToVisualAST is mocked while field lookup and value fetching hit the real store.
//
// The shared_only + text path requires a plugin-owned CPA field and is covered by
// TestMaskConditionValues_SharedOnlyText. This test focuses on:
// - public field: value passes through unchanged (no masking)
// - unknown field: fail-closed (field absent from prefetch map → nil + HasMaskedValues)
func TestGetMaskedVisualAST_Wiring(t *testing.T) {
mainHelper.Parallel(t)
th := Setup(t).InitBasic(t)
cpaID, cErr := th.App.CpaGroupID()
require.Nil(t, cErr)
rctx := request.TestContext(t)
callerID := model.NewId()
// Create a plain public text field in the CPA group (no access mode = public).
// Non-protected fields are writable by any caller in the CPA group.
fieldName := "f_" + model.NewId()
field := &model.PropertyField{
GroupID: cpaID,
Name: fieldName,
Type: model.PropertyFieldTypeText,
}
_, appErr := th.App.CreatePropertyField(rctx, field, false, "")
require.Nil(t, appErr)
t.Run("public field value passes through unchanged", func(t *testing.T) {
visualAST := &model.VisualExpression{
Conditions: []model.Condition{
{Attribute: "user.attributes." + fieldName, Operator: "==", Value: "Engineering", ValueType: model.LiteralValue},
},
}
mockACS := &mocks.AccessControlServiceInterface{}
th.App.Srv().ch.AccessControl = mockACS
mockACS.On("ExpressionToVisualAST", mock.Anything, mock.Anything).Return(visualAST, nil).Once()
result, err := th.App.GetMaskedVisualAST(rctx, "irrelevant", callerID)
require.Nil(t, err)
require.Len(t, result.Conditions, 1)
assert.Equal(t, "Engineering", result.Conditions[0].Value)
assert.False(t, result.Conditions[0].HasMaskedValues)
mockACS.AssertExpectations(t)
})
t.Run("unknown field name fails closed", func(t *testing.T) {
visualAST := &model.VisualExpression{
Conditions: []model.Condition{
{Attribute: "user.attributes.f_" + model.NewId(), Operator: "==", Value: "SomeValue", ValueType: model.LiteralValue},
},
}
mockACS := &mocks.AccessControlServiceInterface{}
th.App.Srv().ch.AccessControl = mockACS
mockACS.On("ExpressionToVisualAST", mock.Anything, mock.Anything).Return(visualAST, nil).Once()
result, err := th.App.GetMaskedVisualAST(rctx, "irrelevant", callerID)
require.Nil(t, err)
require.Len(t, result.Conditions, 1)
// Field absent from prefetch map → fail-closed
assert.Nil(t, result.Conditions[0].Value)
assert.True(t, result.Conditions[0].HasMaskedValues)
mockACS.AssertExpectations(t)
})
}
@@ -19,6 +19,7 @@ package properties
// then Alice querying Bob's values would only see Bananas)
import (
"bytes"
"encoding/json"
"fmt"
"maps"
@@ -691,24 +692,11 @@ func (pas *PropertyAccessService) getSourcePluginID(field *model.PropertyField)
return sourcePluginID
}
// getAccessMode extracts the access_mode from a PropertyField's attrs.
// Returns empty string (public access mode) if not set (default).
func (pas *PropertyAccessService) getAccessMode(field *model.PropertyField) string {
if field.Attrs == nil {
return model.PropertyAccessModePublic
}
accessMode, ok := field.Attrs[model.PropertyAttrsAccessMode].(string)
if !ok {
return model.PropertyAccessModePublic
}
return accessMode
}
// checkUnrestrictedFieldReadAccess checks if the given caller can read a PropertyField without restrictions.
// Returns true if the caller has unrestricted read access (public field or source plugin).
// Returns an error if access requires filtering or should be denied entirely.
func (pas *PropertyAccessService) hasUnrestrictedFieldReadAccess(field *model.PropertyField, callerID string) bool {
accessMode := pas.getAccessMode(field)
accessMode := field.GetAccessMode()
// Public fields are readable by everyone without restrictions
if accessMode == model.PropertyAccessModePublic {
@@ -995,12 +983,16 @@ func (pas *PropertyAccessService) filterSharedOnlyFieldOptions(field *model.Prop
// filterSharedOnlyValue computes the intersection of caller and target values for shared_only fields.
// Returns the filtered value or nil if there's no intersection.
// For single-select: returns value only if both have the same value.
// For multi-select: returns the intersection of arrays.
// - select / multiselect: per-value intersection (a multi-value field may return a subset).
// - text / date / user / any other primitive type: binary — visible only if the caller's
// stored value equals the target's value exactly. Otherwise nil.
//
// The binary path is what protects scenarios like LDAP/SAML-synced text codenames whose
// existence is itself controlled information: a caller who doesn't hold the same value
// must not see the target's value through any read endpoint.
func (pas *PropertyAccessService) filterSharedOnlyValue(field *model.PropertyField, value *model.PropertyValue, callerID string) *model.PropertyValue {
// Only applies to select and multiselect fields
if field.Type != model.PropertyFieldTypeSelect && field.Type != model.PropertyFieldTypeMultiselect {
return value
return pas.filterSharedOnlyScalarValue(field, value, callerID)
}
// Get caller's option IDs for this field
@@ -1057,9 +1049,33 @@ func (pas *PropertyAccessService) filterSharedOnlyValue(field *model.PropertyFie
}
}
// filterSharedOnlyScalarValue applies binary masking to a non-option field's value:
// returns the value as-is if the caller's own stored value for the same field equals
// the target's value, otherwise nil. Caller and target may legitimately store nothing,
// in which case the value is hidden.
func (pas *PropertyAccessService) filterSharedOnlyScalarValue(field *model.PropertyField, value *model.PropertyValue, callerID string) *model.PropertyValue {
if value == nil || len(value.Value) == 0 {
return nil
}
callerValues, err := pas.getCallerValuesForField(field.GroupID, field.ID, callerID)
if err != nil || len(callerValues) == 0 {
return nil
}
for _, cv := range callerValues {
if bytes.Equal(cv.Value, value.Value) {
filtered := *value
return &filtered
}
}
return nil
}
// applyFieldReadAccessControl applies read access control to a single field.
// Returns the field with options filtered based on the caller's access permissions.
// - Public fields: returned as-is
// - User-editable fields (PermissionValues=member): returned as-is so users can see all choices
// - Source-only fields: returned with empty options if caller is not the source plugin
// - Shared-only fields: returned with options filtered using filterSharedOnlyFieldOptions
// - Unknown access modes: treated as source-only (secure default)
@@ -1071,7 +1087,7 @@ func (pas *PropertyAccessService) applyFieldReadAccessControl(field *model.Prope
}
// Access requires filtering
accessMode := pas.getAccessMode(field)
accessMode := field.GetAccessMode()
// Shared-only fields: use existing helper to filter options
if accessMode == model.PropertyAccessModeSharedOnly {
@@ -1163,7 +1179,7 @@ func (pas *PropertyAccessService) applyValueReadAccessControl(values []*model.Pr
return nil, fmt.Errorf("applyValueReadAccessControl: field not found for value %s", value.ID)
}
accessMode := pas.getAccessMode(field)
accessMode := field.GetAccessMode()
// Check if caller can read this value
if pas.hasUnrestrictedFieldReadAccess(field, callerID) {
@@ -1440,3 +1440,7 @@ func TestLinkedPropertyField_SecurityInheritance(t *testing.T) {
assert.False(t, model.IsPropertyFieldProtected(linked))
})
}
// The previous "member-writable shared_only" early-return in applyFieldReadAccessControl
// was removed in favor of rejecting that contradictory configuration at validation time
// (see TestValidatePropertyFieldAccessMode in server/public/model/property_access_test.go).
@@ -561,6 +561,76 @@ func TestGetPropertyValueReadAccess(t *testing.T) {
assert.Nil(t, retrieved)
})
t.Run("shared_only text - binary masking: caller sees value only if it matches their own", func(t *testing.T) {
// Create shared_only text field — the LDAP/SAML codename use case.
field := &model.PropertyField{
GroupID: th.CPAGroupID,
Name: "shared-only-text",
Type: model.PropertyFieldTypeText,
TargetType: "user",
Attrs: model.StringInterface{
model.PropertyAttrsAccessMode: model.PropertyAccessModeSharedOnly,
model.PropertyAttrsProtected: true,
},
}
field, err := th.service.CreatePropertyField(rctxTestPlugin, field)
require.NoError(t, err)
// User 1 has "TF-Zulu"
zuluValue, jsonErr := json.Marshal("TF-Zulu")
require.NoError(t, jsonErr)
value1 := &model.PropertyValue{
GroupID: th.CPAGroupID,
FieldID: field.ID,
TargetType: "user",
TargetID: userID1,
Value: zuluValue,
}
value1, err = th.service.CreatePropertyValue(rctxTestPlugin, value1)
require.NoError(t, err)
// User 2 also has "TF-Zulu" — should see user 1's value.
zuluValue2, jsonErr := json.Marshal("TF-Zulu")
require.NoError(t, jsonErr)
_, err = th.service.CreatePropertyValue(rctxTestPlugin, &model.PropertyValue{
GroupID: th.CPAGroupID,
FieldID: field.ID,
TargetType: "user",
TargetID: userID2,
Value: zuluValue2,
})
require.NoError(t, err)
retrieved, err := th.service.GetPropertyValue(rctxUser2, th.CPAGroupID, value1.ID)
require.NoError(t, err)
require.NotNil(t, retrieved, "caller holding the same text value must see the target's value")
assert.Equal(t, value1.ID, retrieved.ID)
assert.Equal(t, json.RawMessage(zuluValue), retrieved.Value)
// User 3 has "TF-Alpha" — must NOT see user 1's "TF-Zulu".
userID3 := model.NewId()
alphaValue, jsonErr := json.Marshal("TF-Alpha")
require.NoError(t, jsonErr)
_, err = th.service.CreatePropertyValue(rctxTestPlugin, &model.PropertyValue{
GroupID: th.CPAGroupID,
FieldID: field.ID,
TargetType: "user",
TargetID: userID3,
Value: alphaValue,
})
require.NoError(t, err)
retrieved, err = th.service.GetPropertyValue(RequestContextWithCallerID(th.Context, userID3), th.CPAGroupID, value1.ID)
require.NoError(t, err)
assert.Nil(t, retrieved, "caller with a different text value must not see the target's value (binary masking)")
// User 4 has no stored value — must NOT see user 1's value.
userID4 := model.NewId()
retrieved, err = th.service.GetPropertyValue(RequestContextWithCallerID(th.Context, userID4), th.CPAGroupID, value1.ID)
require.NoError(t, err)
assert.Nil(t, retrieved, "caller with no stored value must not see the target's value")
})
t.Run("shared_only value - caller with no values sees nothing", func(t *testing.T) {
// Create shared_only field
field := &model.PropertyField{
+4
View File
@@ -7576,6 +7576,10 @@
"id": "app.pap.get_channel_members_to_remove.app_error",
"translation": "Could not get channel members to remove."
},
{
"id": "app.pap.get_masked_visual_ast.app_error",
"translation": "Could not generate masked visual AST from expression."
},
{
"id": "app.pap.get_policies_for_field_ids.app_error",
"translation": "Could not get policies for the given field IDs."
+21 -7
View File
@@ -41,6 +41,19 @@ func IsPropertyFieldProtected(field *PropertyField) bool {
return ok && protected
}
// GetAccessMode returns the field's access mode. Returns the public mode (empty
// string) when no access_mode is configured or the field has no attrs at all.
func (f *PropertyField) GetAccessMode() string {
if f.Attrs == nil {
return PropertyAccessModePublic
}
accessMode, ok := f.Attrs[PropertyAttrsAccessMode].(string)
if !ok {
return PropertyAccessModePublic
}
return accessMode
}
// ValidatePropertyFieldAccessMode validates that the access_mode attribute is valid
// and compatible with the field type
func ValidatePropertyFieldAccessMode(field *PropertyField) error {
@@ -59,13 +72,6 @@ func ValidatePropertyFieldAccessMode(field *PropertyField) error {
return fmt.Errorf("invalid access mode '%s'", accessMode)
}
// Validate shared_only is only used with select/multiselect fields
if accessMode == PropertyAccessModeSharedOnly {
if field.Type != PropertyFieldTypeSelect && field.Type != PropertyFieldTypeMultiselect {
return fmt.Errorf("access mode 'shared_only' can only be used with select or multiselect field types, got '%s'", field.Type)
}
}
// Validate that non-public access modes require protected flag
if accessMode == PropertyAccessModeSourceOnly || accessMode == PropertyAccessModeSharedOnly {
if !IsPropertyFieldProtected(field) {
@@ -73,5 +79,13 @@ func ValidatePropertyFieldAccessMode(field *PropertyField) error {
}
}
// shared_only + member-writable is contradictory: shared_only filters what
// callers see to values they hold, but member-writable lets users self-assign
// any value. Reject the combination at validation time instead of working
// around it at the API/service layer.
if accessMode == PropertyAccessModeSharedOnly && field.PermissionValues != nil && *field.PermissionValues == PermissionLevelMember {
return fmt.Errorf("access mode 'shared_only' is incompatible with member-writable permission_values")
}
return nil
}
+60 -5
View File
@@ -89,6 +89,29 @@ func TestIsPropertyFieldProtected(t *testing.T) {
})
}
func TestPropertyFieldGetAccessMode(t *testing.T) {
t.Run("nil attrs returns public", func(t *testing.T) {
f := &PropertyField{Attrs: nil}
require.Equal(t, PropertyAccessModePublic, f.GetAccessMode())
})
t.Run("missing access_mode returns public", func(t *testing.T) {
f := &PropertyField{Attrs: StringInterface{}}
require.Equal(t, PropertyAccessModePublic, f.GetAccessMode())
})
t.Run("non-string access_mode returns public", func(t *testing.T) {
f := &PropertyField{Attrs: StringInterface{PropertyAttrsAccessMode: 123}}
require.Equal(t, PropertyAccessModePublic, f.GetAccessMode())
})
t.Run("shared_only returned as-is", func(t *testing.T) {
f := &PropertyField{Attrs: StringInterface{PropertyAttrsAccessMode: PropertyAccessModeSharedOnly}}
require.Equal(t, PropertyAccessModeSharedOnly, f.GetAccessMode())
})
t.Run("source_only returned as-is", func(t *testing.T) {
f := &PropertyField{Attrs: StringInterface{PropertyAttrsAccessMode: PropertyAccessModeSourceOnly}}
require.Equal(t, PropertyAccessModeSourceOnly, f.GetAccessMode())
})
}
func TestValidatePropertyFieldAccessMode(t *testing.T) {
tests := []struct {
name string
@@ -161,7 +184,7 @@ func TestValidatePropertyFieldAccessMode(t *testing.T) {
expectError: true,
},
{
name: "invalid shared_only access mode with text field",
name: "valid shared_only access mode with text field and protected",
field: &PropertyField{
Type: PropertyFieldTypeText,
Attrs: StringInterface{
@@ -169,10 +192,10 @@ func TestValidatePropertyFieldAccessMode(t *testing.T) {
PropertyAttrsProtected: true,
},
},
expectError: true,
expectError: false,
},
{
name: "invalid shared_only access mode with date field",
name: "valid shared_only access mode with date field and protected",
field: &PropertyField{
Type: PropertyFieldTypeDate,
Attrs: StringInterface{
@@ -180,10 +203,10 @@ func TestValidatePropertyFieldAccessMode(t *testing.T) {
PropertyAttrsProtected: true,
},
},
expectError: true,
expectError: false,
},
{
name: "invalid shared_only access mode with user field",
name: "valid shared_only access mode with user field and protected",
field: &PropertyField{
Type: PropertyFieldTypeUser,
Attrs: StringInterface{
@@ -191,6 +214,14 @@ func TestValidatePropertyFieldAccessMode(t *testing.T) {
PropertyAttrsProtected: true,
},
},
expectError: false,
},
{
name: "shared_only access mode with text field requires protected",
field: &PropertyField{
Type: PropertyFieldTypeText,
Attrs: StringInterface{PropertyAttrsAccessMode: PropertyAccessModeSharedOnly},
},
expectError: true,
},
{
@@ -201,6 +232,30 @@ func TestValidatePropertyFieldAccessMode(t *testing.T) {
},
expectError: true,
},
{
name: "shared_only rejected with member-writable permission_values",
field: &PropertyField{
Type: PropertyFieldTypeSelect,
Attrs: StringInterface{
PropertyAttrsAccessMode: PropertyAccessModeSharedOnly,
PropertyAttrsProtected: true,
},
PermissionValues: func() *PermissionLevel { p := PermissionLevelMember; return &p }(),
},
expectError: true,
},
{
name: "shared_only accepted with sysadmin permission_values",
field: &PropertyField{
Type: PropertyFieldTypeSelect,
Attrs: StringInterface{
PropertyAttrsAccessMode: PropertyAccessModeSharedOnly,
PropertyAttrsProtected: true,
},
PermissionValues: func() *PermissionLevel { p := PermissionLevelSysadmin; return &p }(),
},
expectError: false,
},
{
name: "nil attrs should not error",
field: &PropertyField{