diff --git a/server/channels/api4/access_control.go b/server/channels/api4/access_control.go index 8d31d855789..c8bb59038b2 100644 --- a/server/channels/api4/access_control.go +++ b/server/channels/api4/access_control.go @@ -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 diff --git a/server/channels/app/access_control_masking.go b/server/channels/app/access_control_masking.go new file mode 100644 index 00000000000..b3d9bd315f7 --- /dev/null +++ b/server/channels/app/access_control_masking.go @@ -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 + } + } +} diff --git a/server/channels/app/access_control_masking_test.go b/server/channels/app/access_control_masking_test.go new file mode 100644 index 00000000000..96242489e0e --- /dev/null +++ b/server/channels/app/access_control_masking_test.go @@ -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) + }) +} diff --git a/server/channels/app/properties/access_control.go b/server/channels/app/properties/access_control.go index 3bc65e57cde..63fd0f6b608 100644 --- a/server/channels/app/properties/access_control.go +++ b/server/channels/app/properties/access_control.go @@ -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) { diff --git a/server/channels/app/properties/access_control_field_test.go b/server/channels/app/properties/access_control_field_test.go index e573f3facf7..6ceec86ce78 100644 --- a/server/channels/app/properties/access_control_field_test.go +++ b/server/channels/app/properties/access_control_field_test.go @@ -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). diff --git a/server/channels/app/properties/access_control_value_test.go b/server/channels/app/properties/access_control_value_test.go index 0914485d8d3..8b40e179b03 100644 --- a/server/channels/app/properties/access_control_value_test.go +++ b/server/channels/app/properties/access_control_value_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{ diff --git a/server/i18n/en.json b/server/i18n/en.json index 83d18553233..b0abeee2be5 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -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." diff --git a/server/public/model/property_access.go b/server/public/model/property_access.go index c877622f762..483ac02e01f 100644 --- a/server/public/model/property_access.go +++ b/server/public/model/property_access.go @@ -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 } diff --git a/server/public/model/property_access_test.go b/server/public/model/property_access_test.go index c59922cf9fb..75bb668c812 100644 --- a/server/public/model/property_access_test.go +++ b/server/public/model/property_access_test.go @@ -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{