[MM-68109] Introduce new policy version v0.3 (#35904)

This commit is contained in:
Ibrahim Serdar Acikgoz
2026-04-15 11:22:41 +02:00
committed by GitHub
parent 9fa8c8c0c8
commit c66bb0ecdb
19 changed files with 946 additions and 56 deletions
+10 -1
View File
@@ -79,6 +79,15 @@ func (a *App) CreateOrUpdateAccessControlPolicy(rctx request.CTX, policy *model.
policy.ID = model.NewId()
}
policy.Version = model.AccessControlPolicyVersionV0_3
for i, rule := range policy.Rules {
for j, action := range rule.Actions {
if action == "*" {
policy.Rules[i].Actions[j] = model.AccessControlPolicyActionMembership
}
}
}
var appErr *model.AppError
policy, appErr = acs.SavePolicy(rctx, policy)
if appErr != nil {
@@ -173,7 +182,7 @@ func (a *App) AssignAccessControlPolicyToChannels(rctx request.CTX, parentID str
Props: map[string]any{},
}
}
child.Version = model.AccessControlPolicyVersionV0_2
child.Version = model.AccessControlPolicyVersionV0_3
appErr := child.Inherit(policy)
if appErr != nil {
@@ -17,6 +17,96 @@ import (
"github.com/mattermost/mattermost/server/v8/einterfaces/mocks"
)
func TestCreateOrUpdateAccessControlPolicy(t *testing.T) {
th := Setup(t).InitBasic(t)
t.Run("Feature not enabled", func(t *testing.T) {
th.App.Srv().ch.AccessControl = nil
policy := &model.AccessControlPolicy{
Type: model.AccessControlPolicyTypeParent,
Name: "test-policy",
Rules: []model.AccessControlPolicyRule{
{Actions: []string{"*"}, Expression: "true"},
},
}
result, err := th.App.CreateOrUpdateAccessControlPolicy(th.Context, policy)
require.NotNil(t, err)
require.Nil(t, result)
})
t.Run("Wildcard actions rewritten to membership and version set to v0.3", func(t *testing.T) {
mockAccessControl := &mocks.AccessControlServiceInterface{}
th.App.Srv().ch.AccessControl = mockAccessControl
policy := &model.AccessControlPolicy{
Type: model.AccessControlPolicyTypeParent,
Name: "wildcard-rewrite",
Rules: []model.AccessControlPolicyRule{
{Actions: []string{"*"}, Expression: "user.attributes.team == \"eng\""},
},
}
mockAccessControl.On("SavePolicy", th.Context, mock.MatchedBy(func(p *model.AccessControlPolicy) bool {
return p.Version == model.AccessControlPolicyVersionV0_3 &&
len(p.Rules) == 1 &&
len(p.Rules[0].Actions) == 1 &&
p.Rules[0].Actions[0] == model.AccessControlPolicyActionMembership
})).Return(policy, nil).Once()
result, err := th.App.CreateOrUpdateAccessControlPolicy(th.Context, policy)
require.Nil(t, err)
require.NotNil(t, result)
mockAccessControl.AssertExpectations(t)
})
t.Run("Multiple rules with mixed actions", func(t *testing.T) {
mockAccessControl := &mocks.AccessControlServiceInterface{}
th.App.Srv().ch.AccessControl = mockAccessControl
policy := &model.AccessControlPolicy{
Type: model.AccessControlPolicyTypeParent,
Name: "mixed-actions",
Rules: []model.AccessControlPolicyRule{
{Actions: []string{"*"}, Expression: "expr1"},
{Actions: []string{model.AccessControlPolicyActionUploadFileAttachment}, Expression: "expr2"},
},
}
mockAccessControl.On("SavePolicy", th.Context, mock.MatchedBy(func(p *model.AccessControlPolicy) bool {
return p.Rules[0].Actions[0] == model.AccessControlPolicyActionMembership &&
p.Rules[1].Actions[0] == model.AccessControlPolicyActionUploadFileAttachment
})).Return(policy, nil).Once()
result, err := th.App.CreateOrUpdateAccessControlPolicy(th.Context, policy)
require.Nil(t, err)
require.NotNil(t, result)
mockAccessControl.AssertExpectations(t)
})
t.Run("Generates ID when empty", func(t *testing.T) {
mockAccessControl := &mocks.AccessControlServiceInterface{}
th.App.Srv().ch.AccessControl = mockAccessControl
policy := &model.AccessControlPolicy{
Type: model.AccessControlPolicyTypeParent,
Name: "no-id",
Rules: []model.AccessControlPolicyRule{
{Actions: []string{model.AccessControlPolicyActionMembership}, Expression: "true"},
},
}
mockAccessControl.On("SavePolicy", th.Context, mock.MatchedBy(func(p *model.AccessControlPolicy) bool {
return p.ID != "" && model.IsValidId(p.ID)
})).Return(policy, nil).Once()
result, err := th.App.CreateOrUpdateAccessControlPolicy(th.Context, policy)
require.Nil(t, err)
require.NotNil(t, result)
mockAccessControl.AssertExpectations(t)
})
}
func TestGetChannelsForPolicy(t *testing.T) {
th := Setup(t).InitBasic(t)
+1 -1
View File
@@ -1771,7 +1771,7 @@ func (a *App) addUserToChannel(rctx request.CTX, user *model.User, channel *mode
Type: model.AccessControlPolicyTypeChannel,
ID: channel.Id,
},
Action: "join_channel",
Action: "membership",
})
if evalErr != nil {
return nil, evalErr
+69
View File
@@ -14,6 +14,7 @@ import (
"github.com/mattermost/mattermost/server/public/model"
"github.com/mattermost/mattermost/server/public/shared/mlog"
"github.com/mattermost/mattermost/server/public/shared/request"
"github.com/mattermost/mattermost/server/public/utils"
"github.com/mattermost/mattermost/server/v8/channels/store"
)
@@ -907,6 +908,73 @@ func (s *Server) doDeleteDmsPreferencesMigration(rctx request.CTX) error {
return nil
}
func (s *Server) doAccessControlPolicyV0_3Migration(rctx request.CTX) error {
var nfErr *store.ErrNotFound
if _, err := s.Store().System().GetByName(model.MigrationKeyAccessControlPolicyV0_3); err == nil {
return nil
} else if !errors.As(err, &nfErr) {
return fmt.Errorf("could not query migration: %w", err)
}
policyTypes := []string{model.AccessControlPolicyTypeParent, model.AccessControlPolicyTypeChannel}
const pageSize = 100
for _, policyType := range policyTypes {
cursor := model.AccessControlPolicyCursor{}
policies, err := utils.Pager(func(_ int) ([]*model.AccessControlPolicy, error) {
results, _, err := s.Store().AccessControlPolicy().SearchPolicies(rctx, model.AccessControlPolicySearch{
Type: policyType,
Cursor: cursor,
Limit: pageSize,
})
if err != nil {
return nil, err
}
if len(results) > 0 {
cursor = model.AccessControlPolicyCursor{ID: results[len(results)-1].ID}
}
return results, nil
}, pageSize)
if err != nil {
return fmt.Errorf("failed to search access control policies: %w", err)
}
for _, policy := range policies {
if policy.Version != model.AccessControlPolicyVersionV0_2 {
continue
}
policy.Version = model.AccessControlPolicyVersionV0_3
for i, rule := range policy.Rules {
for j, action := range rule.Actions {
if action == "*" {
policy.Rules[i].Actions[j] = model.AccessControlPolicyActionMembership
}
}
}
if _, err := s.Store().AccessControlPolicy().Save(rctx, policy); err != nil {
return fmt.Errorf("failed to save migrated access control policy id=%s: %w", policy.ID, err)
}
if policy.Type == model.AccessControlPolicyTypeChannel {
s.Store().Channel().InvalidateChannel(policy.ID)
}
}
}
system := model.System{
Name: model.MigrationKeyAccessControlPolicyV0_3,
Value: "true",
}
if err := s.Store().System().Save(&system); err != nil {
return fmt.Errorf("failed to mark access control policy v0.3 migration as completed: %w", err)
}
return nil
}
func (a *App) DoAppMigrations() {
a.Srv().doAppMigrations()
}
@@ -953,6 +1021,7 @@ func (s *Server) doAppMigrations() {
{"Delete Empty Drafts Migration", s.doDeleteEmptyDraftsMigration},
{"Delete Orphan Drafts Migration", s.doDeleteOrphanDraftsMigration},
{"Delete Invalid Dms Preferences Migration", s.doDeleteDmsPreferencesMigration},
{"Access Control Policy V0.3 Migration", s.doAccessControlPolicyV0_3Migration},
}
rctx := request.EmptyContext(s.Log())
@@ -54,16 +54,19 @@ func (s *storeAccessControlPolicy) toModel() (*model.AccessControlPolicy, error)
Version: s.Version,
}
var p accessControlPolicyV0_1
if err := json.Unmarshal(s.Data, &p); err != nil {
return nil, err
if len(s.Data) > 0 {
var p accessControlPolicyV0_1
if err := json.Unmarshal(s.Data, &p); err != nil {
return nil, err
}
policy.Imports = p.Imports
policy.Rules = p.Rules
}
policy.Imports = p.Imports
policy.Rules = p.Rules
if err := json.Unmarshal(s.Props, &policy.Props); err != nil {
return nil, err
if len(s.Props) > 0 {
if err := json.Unmarshal(s.Props, &policy.Props); err != nil {
return nil, err
}
}
return policy, nil
@@ -640,6 +643,18 @@ func (s *SqlAccessControlPolicyStore) SearchPolicies(rctx request.CTX, opts mode
count = count.Where(condition)
}
if len(opts.Actions) > 0 {
or := sq.Or{}
for _, action := range opts.Actions {
or = append(or, sq.Expr(
"EXISTS (SELECT 1 FROM jsonb_array_elements(CASE WHEN jsonb_typeof(Data->'rules') = 'array' THEN Data->'rules' ELSE '[]'::jsonb END) AS rule WHERE rule->'actions' @> ?::jsonb)",
fmt.Sprintf("%q", action),
))
}
query = query.Where(or)
count = count.Where(or)
}
if opts.Active {
query = query.Where(sq.Eq{"Active": true})
count = count.Where(sq.Eq{"Active": true})
@@ -21,6 +21,7 @@ func TestAccessControlPolicyStore(t *testing.T, rctx request.CTX, ss store.Store
t.Run("SetActiveMultiple", func(t *testing.T) { testAccessControlPolicyStoreSetActiveMultiple(t, rctx, ss) })
t.Run("GetAll", func(t *testing.T) { testAccessControlPolicyStoreGetAll(t, rctx, ss) })
t.Run("Search", func(t *testing.T) { testAccessControlPolicyStoreSearch(t, rctx, ss) })
t.Run("SearchByActions", func(t *testing.T) { testAccessControlPolicyStoreSearchByActions(t, rctx, ss) })
t.Run("GetPoliciesByFieldID", func(t *testing.T) { testAccessControlPolicyStoreGetPoliciesByFieldID(t, rctx, ss) })
}
@@ -707,6 +708,134 @@ func testAccessControlPolicyStoreSearch(t *testing.T, rctx request.CTX, ss store
})
}
func testAccessControlPolicyStoreSearchByActions(t *testing.T, rctx request.CTX, ss store.Store) {
membershipOnly := &model.AccessControlPolicy{
ID: model.NewId(),
Name: "MembershipOnly " + model.NewId(),
Type: model.AccessControlPolicyTypeParent,
Active: true,
Revision: 1,
Version: model.AccessControlPolicyVersionV0_3,
Rules: []model.AccessControlPolicyRule{{
Actions: []string{model.AccessControlPolicyActionMembership},
Expression: "true",
}},
}
uploadOnly := &model.AccessControlPolicy{
ID: model.NewId(),
Name: "UploadOnly " + model.NewId(),
Type: model.AccessControlPolicyTypeParent,
Active: true,
Revision: 1,
Version: model.AccessControlPolicyVersionV0_3,
Rules: []model.AccessControlPolicyRule{{
Actions: []string{model.AccessControlPolicyActionUploadFileAttachment},
Expression: "true",
}},
}
multiAction := &model.AccessControlPolicy{
ID: model.NewId(),
Name: "Multi " + model.NewId(),
Type: model.AccessControlPolicyTypeParent,
Active: true,
Revision: 1,
Version: model.AccessControlPolicyVersionV0_3,
Rules: []model.AccessControlPolicyRule{
{
Actions: []string{model.AccessControlPolicyActionMembership},
Expression: "true",
},
{
Actions: []string{model.AccessControlPolicyActionDownloadFileAttachment},
Expression: "true",
},
},
}
multiActionSingleRule := &model.AccessControlPolicy{
ID: model.NewId(),
Name: "MultiSingleRule " + model.NewId(),
Type: model.AccessControlPolicyTypeParent,
Active: true,
Revision: 1,
Version: model.AccessControlPolicyVersionV0_3,
Rules: []model.AccessControlPolicyRule{{
Actions: []string{
model.AccessControlPolicyActionMembership,
model.AccessControlPolicyActionDownloadFileAttachment,
},
Expression: "true",
}},
}
for _, p := range []*model.AccessControlPolicy{membershipOnly, uploadOnly, multiAction, multiActionSingleRule} {
saved, err := ss.AccessControlPolicy().Save(rctx, p)
require.NoError(t, err)
require.NotNil(t, saved)
}
t.Cleanup(func() {
for _, p := range []*model.AccessControlPolicy{membershipOnly, uploadOnly, multiAction, multiActionSingleRule} {
_ = ss.AccessControlPolicy().Delete(rctx, p.ID)
}
})
t.Run("single action filter returns matching policies", func(t *testing.T) {
policies, _, err := ss.AccessControlPolicy().SearchPolicies(rctx, model.AccessControlPolicySearch{
Actions: []string{model.AccessControlPolicyActionMembership},
Limit: 10,
})
require.NoError(t, err)
ids := make([]string, len(policies))
for i, p := range policies {
ids[i] = p.ID
}
require.Contains(t, ids, membershipOnly.ID)
require.Contains(t, ids, multiAction.ID)
require.Contains(t, ids, multiActionSingleRule.ID)
require.NotContains(t, ids, uploadOnly.ID)
})
t.Run("upload action filter", func(t *testing.T) {
policies, _, err := ss.AccessControlPolicy().SearchPolicies(rctx, model.AccessControlPolicySearch{
Actions: []string{model.AccessControlPolicyActionUploadFileAttachment},
Limit: 10,
})
require.NoError(t, err)
ids := make([]string, len(policies))
for i, p := range policies {
ids[i] = p.ID
}
require.Contains(t, ids, uploadOnly.ID)
require.NotContains(t, ids, membershipOnly.ID)
require.NotContains(t, ids, multiAction.ID)
require.NotContains(t, ids, multiActionSingleRule.ID)
})
t.Run("multiple actions OR semantics", func(t *testing.T) {
policies, _, err := ss.AccessControlPolicy().SearchPolicies(rctx, model.AccessControlPolicySearch{
Actions: []string{model.AccessControlPolicyActionUploadFileAttachment, model.AccessControlPolicyActionDownloadFileAttachment},
Limit: 10,
})
require.NoError(t, err)
ids := make([]string, len(policies))
for i, p := range policies {
ids[i] = p.ID
}
require.Contains(t, ids, uploadOnly.ID)
require.Contains(t, ids, multiAction.ID)
require.Contains(t, ids, multiActionSingleRule.ID)
require.NotContains(t, ids, membershipOnly.ID)
})
t.Run("non-existent action returns nothing from scoped set", func(t *testing.T) {
policies, _, err := ss.AccessControlPolicy().SearchPolicies(rctx, model.AccessControlPolicySearch{
Actions: []string{"nonexistent_action"},
Limit: 10,
})
require.NoError(t, err)
require.Len(t, policies, 0)
})
}
func testAccessControlPolicyStoreSetActiveMultiple(t *testing.T, rctx request.CTX, ss store.Store) {
t.Run("Set active status for multiple policies", func(t *testing.T) {
policy1 := &model.AccessControlPolicy{
+1
View File
@@ -95,6 +95,7 @@ func GetMockStoreForSetupFunctions() *mocks.Store {
systemStore.On("GetByName", model.MigrationKeyAddChannelAccessRulesPermission).Return(&model.System{Name: model.MigrationKeyAddChannelAccessRulesPermission, Value: "true"}, nil)
systemStore.On("GetByName", model.MigrationKeyAddChannelAutoTranslationPermissions).Return(&model.System{Name: model.MigrationKeyAddChannelAutoTranslationPermissions, Value: "true"}, nil)
systemStore.On("GetByName", model.MigrationKeyRestoreManageOAuthPermission).Return(&model.System{Name: model.MigrationKeyRestoreManageOAuthPermission, Value: "true"}, nil)
systemStore.On("GetByName", model.MigrationKeyAccessControlPolicyV0_3).Return(&model.System{Name: model.MigrationKeyAccessControlPolicyV0_3, Value: "true"}, nil)
systemStore.On("InsertIfExists", mock.AnythingOfType("*model.System")).Return(&model.System{}, nil).Once()
systemStore.On("Save", mock.AnythingOfType("*model.System")).Return(nil)
+12
View File
@@ -10084,10 +10084,18 @@
"id": "model.access_policy.inherit.already_imported.app_error",
"translation": "The parent is already imported."
},
{
"id": "model.access_policy.inherit.permission.app_error",
"translation": "Permission policies cannot inherit from other policies."
},
{
"id": "model.access_policy.inherit.version.app_error",
"translation": "Could not inherit access control policy."
},
{
"id": "model.access_policy.is_valid.actions.app_error",
"translation": "Action(s) is not valid."
},
{
"id": "model.access_policy.is_valid.id.app_error",
"translation": "Invalid policy id."
@@ -10104,6 +10112,10 @@
"id": "model.access_policy.is_valid.revision.app_error",
"translation": "Invalid policy revision."
},
{
"id": "model.access_policy.is_valid.roles.app_error",
"translation": "Permission policies must be applied to exactly one role."
},
{
"id": "model.access_policy.is_valid.rules.app_error",
"translation": "Rule(s) is not valid."
+99 -3
View File
@@ -6,21 +6,34 @@ package model
import (
"fmt"
"slices"
"strings"
"github.com/pkg/errors"
"golang.org/x/mod/semver"
)
const (
AccessControlPolicyTypeParent = "parent"
AccessControlPolicyTypeChannel = "channel"
AccessControlPolicyTypeParent = "parent"
AccessControlPolicyTypeChannel = "channel"
AccessControlPolicyTypePermission = "permission"
MaxPolicyNameLength = 128
AccessControlPolicyVersionV0_1 = "v0.1"
AccessControlPolicyVersionV0_2 = "v0.2"
AccessControlPolicyVersionV0_3 = "v0.3"
AccessControlPolicyActionMembership = "membership"
AccessControlPolicyActionUploadFileAttachment = "upload_file_attachment"
AccessControlPolicyActionDownloadFileAttachment = "download_file_attachment"
)
var allowedActionsV0_3 = map[string]bool{
AccessControlPolicyActionMembership: true,
AccessControlPolicyActionUploadFileAttachment: true,
AccessControlPolicyActionDownloadFileAttachment: true,
}
// AccessControlAttribute represents a user attribute with its name and possible values
type AccessControlAttribute struct {
Attribute PropertyField `json:"attribute"`
@@ -48,6 +61,7 @@ type AccessControlPolicySearch struct {
Limit int `json:"limit"`
IncludeChildren bool `json:"include_children"`
Active bool `json:"active"`
Actions []string `json:"actions"`
}
type AccessControlPolicyCursor struct {
@@ -69,6 +83,7 @@ type AccessControlPolicy struct {
Revision int `json:"revision"`
Version string `json:"version"`
Roles []string `json:"roles"`
Imports []string `json:"imports"`
Rules []AccessControlPolicyRule `json:"rules"`
@@ -120,6 +135,8 @@ func (p *AccessControlPolicy) IsValid() *AppError {
return p.accessPolicyVersionV0_1()
case AccessControlPolicyVersionV0_2:
return p.accessPolicyVersionV0_2()
case AccessControlPolicyVersionV0_3:
return p.accessPolicyVersionV0_3()
default:
return NewAppError("AccessControlPolicy.IsValid", "model.access_policy.is_valid.version.app_error", nil, "", 400)
}
@@ -211,6 +228,75 @@ func (p *AccessControlPolicy) accessPolicyVersionV0_2() *AppError {
return nil
}
func (p *AccessControlPolicy) accessPolicyVersionV0_3() *AppError {
if !slices.Contains([]string{AccessControlPolicyTypeParent, AccessControlPolicyTypeChannel, AccessControlPolicyTypePermission}, p.Type) {
return NewAppError("AccessControlPolicy.IsValid", "model.access_policy.is_valid.type.app_error", nil, "", 400)
}
if !IsValidId(p.ID) {
return NewAppError("AccessControlPolicy.IsValid", "model.access_policy.is_valid.id.app_error", nil, "", 400)
}
if p.Type == AccessControlPolicyTypeParent && (p.Name == "" || len(p.Name) > MaxPolicyNameLength) {
return NewAppError("AccessControlPolicy.IsValid", "model.access_policy.is_valid.name.app_error", nil, "", 400)
}
if p.Revision < 0 {
return NewAppError("AccessControlPolicy.IsValid", "model.access_policy.is_valid.revision.app_error", nil, "", 400)
}
if !semver.IsValid(p.Version) {
return NewAppError("AccessControlPolicy.IsValid", "model.access_policy.is_valid.version.app_error", nil, "", 400)
}
switch p.Type {
case AccessControlPolicyTypeParent:
if len(p.Rules) == 0 {
return NewAppError("AccessControlPolicy.IsValid", "model.access_policy.is_valid.rules.app_error", nil, "", 400)
}
if len(p.Imports) > 0 {
return NewAppError("AccessControlPolicy.IsValid", "model.access_policy.is_valid.imports.app_error", nil, "", 400)
}
case AccessControlPolicyTypeChannel:
if len(p.Rules) == 0 && len(p.Imports) == 0 {
return NewAppError("AccessControlPolicy.IsValid", "model.access_policy.is_valid.rules_imports.app_error", nil, "", 400)
}
case AccessControlPolicyTypePermission:
if len(p.Rules) == 0 && len(p.Imports) == 0 {
return NewAppError("AccessControlPolicy.IsValid", "model.access_policy.is_valid.rules_imports.app_error", nil, "", 400)
}
// Permissions are only allowed to be applied to a single role as of v0.3
// role hierarchy is resolved at the PDP
if len(p.Roles) != 1 {
return NewAppError("AccessControlPolicy.IsValid", "model.access_policy.is_valid.roles.app_error", nil, "", 400)
}
for _, role := range p.Roles {
if strings.TrimSpace(role) == "" {
return NewAppError("AccessControlPolicy.IsValid", "model.access_policy.is_valid.roles.app_error", nil, "", 400)
}
}
if len(p.Imports) > 0 {
return NewAppError("AccessControlPolicy.IsValid", "model.access_policy.is_valid.imports.app_error", nil, "", 400)
}
}
for _, rule := range p.Rules {
if len(rule.Actions) == 0 {
return NewAppError("AccessControlPolicy.IsValid", "model.access_policy.is_valid.actions.app_error", nil, "actions must not be empty", 400)
}
for _, action := range rule.Actions {
if !allowedActionsV0_3[action] {
return NewAppError("AccessControlPolicy.IsValid", "model.access_policy.is_valid.actions.app_error", nil, fmt.Sprintf("unrecognized action: %s", action), 400)
}
}
}
return nil
}
func (p *AccessControlPolicy) Inherit(parent *AccessControlPolicy) *AppError {
rules := make([]AccessControlPolicyRule, len(p.Rules))
@@ -230,7 +316,17 @@ func (p *AccessControlPolicy) Inherit(parent *AccessControlPolicy) *AppError {
return NewAppError("AccessControlPolicy.Inherit", "model.access_policy.inherit.already_imported.app_error", nil, "", 400)
}
p.Imports = append(p.Imports, parent.ID)
case AccessControlPolicyVersionV0_3:
if p.Type == AccessControlPolicyTypePermission || parent.Type == AccessControlPolicyTypePermission {
return NewAppError("AccessControlPolicy.Inherit", "model.access_policy.inherit.permission.app_error", nil, "", 400)
}
if parent.Version != AccessControlPolicyVersionV0_3 {
return NewAppError("AccessControlPolicy.Inherit", "model.access_policy.inherit.version.app_error", nil, "", 400)
}
if slices.Contains(p.Imports, parent.ID) {
return NewAppError("AccessControlPolicy.Inherit", "model.access_policy.inherit.already_imported.app_error", nil, "", 400)
}
p.Imports = append(p.Imports, parent.ID)
default:
return NewAppError("AccessControlPolicy.Inherit", "model.access_policy.inherit.version.app_error", nil, "", 400)
}
+342
View File
@@ -213,3 +213,345 @@ func TestAccessPolicyVersionV0_1(t *testing.T) {
require.Nil(t, err, "Should not return error for valid channel policy")
})
}
func TestAccessPolicyVersionV0_3(t *testing.T) {
validRule := AccessControlPolicyRule{
Actions: []string{AccessControlPolicyActionMembership},
Expression: "user.properties.dept == \"eng\"",
}
t.Run("valid parent type", func(t *testing.T) {
policy := &AccessControlPolicy{
ID: NewId(),
Type: AccessControlPolicyTypeParent,
Name: "Parent",
Revision: 0,
Version: AccessControlPolicyVersionV0_3,
Rules: []AccessControlPolicyRule{{
Actions: []string{
AccessControlPolicyActionMembership,
AccessControlPolicyActionUploadFileAttachment,
AccessControlPolicyActionDownloadFileAttachment,
},
Expression: "user.properties.dept == \"eng\"",
}},
}
require.Nil(t, policy.accessPolicyVersionV0_3())
})
t.Run("valid channel type", func(t *testing.T) {
policy := &AccessControlPolicy{
ID: NewId(),
Type: AccessControlPolicyTypeChannel,
Revision: 0,
Version: AccessControlPolicyVersionV0_3,
Imports: []string{NewId()},
Rules: []AccessControlPolicyRule{validRule},
}
require.Nil(t, policy.accessPolicyVersionV0_3())
})
t.Run("valid permission type", func(t *testing.T) {
policy := &AccessControlPolicy{
ID: NewId(),
Type: AccessControlPolicyTypePermission,
Revision: 0,
Version: AccessControlPolicyVersionV0_3,
Roles: []string{"system_admin"},
Rules: []AccessControlPolicyRule{validRule},
}
require.Nil(t, policy.accessPolicyVersionV0_3())
})
t.Run("invalid type", func(t *testing.T) {
policy := &AccessControlPolicy{
ID: NewId(),
Type: "unknown",
Revision: 0,
Version: AccessControlPolicyVersionV0_3,
Rules: []AccessControlPolicyRule{validRule},
}
err := policy.accessPolicyVersionV0_3()
require.NotNil(t, err)
require.Equal(t, "model.access_policy.is_valid.type.app_error", err.Id)
})
t.Run("parent with no rules", func(t *testing.T) {
policy := &AccessControlPolicy{
ID: NewId(),
Type: AccessControlPolicyTypeParent,
Name: "Parent",
Revision: 0,
Version: AccessControlPolicyVersionV0_3,
Rules: []AccessControlPolicyRule{},
}
err := policy.accessPolicyVersionV0_3()
require.NotNil(t, err)
require.Equal(t, "model.access_policy.is_valid.rules.app_error", err.Id)
})
t.Run("parent with non-empty imports", func(t *testing.T) {
policy := &AccessControlPolicy{
ID: NewId(),
Type: AccessControlPolicyTypeParent,
Name: "Parent",
Revision: 0,
Version: AccessControlPolicyVersionV0_3,
Rules: []AccessControlPolicyRule{validRule},
Imports: []string{NewId()},
}
err := policy.accessPolicyVersionV0_3()
require.NotNil(t, err)
require.Equal(t, "model.access_policy.is_valid.imports.app_error", err.Id)
})
t.Run("permission with empty roles", func(t *testing.T) {
policy := &AccessControlPolicy{
ID: NewId(),
Type: AccessControlPolicyTypePermission,
Revision: 0,
Version: AccessControlPolicyVersionV0_3,
Roles: []string{},
Rules: []AccessControlPolicyRule{validRule},
}
err := policy.accessPolicyVersionV0_3()
require.NotNil(t, err)
require.Equal(t, "model.access_policy.is_valid.roles.app_error", err.Id)
})
t.Run("permission with blank role string", func(t *testing.T) {
policy := &AccessControlPolicy{
ID: NewId(),
Type: AccessControlPolicyTypePermission,
Revision: 0,
Version: AccessControlPolicyVersionV0_3,
Roles: []string{""},
Rules: []AccessControlPolicyRule{validRule},
}
err := policy.accessPolicyVersionV0_3()
require.NotNil(t, err)
require.Equal(t, "model.access_policy.is_valid.roles.app_error", err.Id)
})
t.Run("permission with multiple roles", func(t *testing.T) {
policy := &AccessControlPolicy{
ID: NewId(),
Type: AccessControlPolicyTypePermission,
Revision: 0,
Version: AccessControlPolicyVersionV0_3,
Roles: []string{"system_admin", "system_user"},
Rules: []AccessControlPolicyRule{validRule},
}
err := policy.accessPolicyVersionV0_3()
require.NotNil(t, err)
require.Equal(t, "model.access_policy.is_valid.roles.app_error", err.Id)
})
t.Run("permission with imports", func(t *testing.T) {
policy := &AccessControlPolicy{
ID: NewId(),
Type: AccessControlPolicyTypePermission,
Revision: 0,
Version: AccessControlPolicyVersionV0_3,
Roles: []string{"system_admin"},
Rules: []AccessControlPolicyRule{validRule},
Imports: []string{NewId()},
}
err := policy.accessPolicyVersionV0_3()
require.NotNil(t, err)
require.Equal(t, "model.access_policy.is_valid.imports.app_error", err.Id)
})
t.Run("unrecognized action", func(t *testing.T) {
policy := &AccessControlPolicy{
ID: NewId(),
Type: AccessControlPolicyTypeParent,
Name: "Parent",
Revision: 0,
Version: AccessControlPolicyVersionV0_3,
Rules: []AccessControlPolicyRule{{
Actions: []string{"not_a_real_action"},
Expression: "true",
}},
}
err := policy.accessPolicyVersionV0_3()
require.NotNil(t, err)
require.Equal(t, "model.access_policy.is_valid.actions.app_error", err.Id)
require.Contains(t, err.DetailedError, "not_a_real_action")
})
t.Run("empty actions slice", func(t *testing.T) {
policy := &AccessControlPolicy{
ID: NewId(),
Type: AccessControlPolicyTypeParent,
Name: "Parent",
Revision: 0,
Version: AccessControlPolicyVersionV0_3,
Rules: []AccessControlPolicyRule{{
Actions: []string{},
Expression: "true",
}},
}
err := policy.accessPolicyVersionV0_3()
require.NotNil(t, err)
require.Equal(t, "model.access_policy.is_valid.actions.app_error", err.Id)
})
t.Run("channel with no rules and no imports", func(t *testing.T) {
policy := &AccessControlPolicy{
ID: NewId(),
Type: AccessControlPolicyTypeChannel,
Revision: 0,
Version: AccessControlPolicyVersionV0_3,
}
err := policy.accessPolicyVersionV0_3()
require.NotNil(t, err)
require.Equal(t, "model.access_policy.is_valid.rules_imports.app_error", err.Id)
})
}
func TestInheritV0_3(t *testing.T) {
t.Run("successful inherit", func(t *testing.T) {
parentID := NewId()
parent := &AccessControlPolicy{
ID: parentID,
Type: AccessControlPolicyTypeParent,
Name: "Parent",
Revision: 0,
Version: AccessControlPolicyVersionV0_3,
Rules: []AccessControlPolicyRule{{
Actions: []string{AccessControlPolicyActionMembership},
Expression: "true",
}},
}
child := &AccessControlPolicy{
ID: NewId(),
Type: AccessControlPolicyTypeChannel,
Revision: 0,
Version: AccessControlPolicyVersionV0_3,
Rules: []AccessControlPolicyRule{{
Actions: []string{AccessControlPolicyActionMembership},
Expression: "true",
}},
}
err := child.Inherit(parent)
require.Nil(t, err)
require.Contains(t, child.Imports, parentID)
})
t.Run("duplicate import guard", func(t *testing.T) {
parentID := NewId()
parent := &AccessControlPolicy{
ID: parentID,
Type: AccessControlPolicyTypeParent,
Name: "Parent",
Revision: 0,
Version: AccessControlPolicyVersionV0_3,
Rules: []AccessControlPolicyRule{{
Actions: []string{AccessControlPolicyActionMembership},
Expression: "true",
}},
}
child := &AccessControlPolicy{
ID: NewId(),
Type: AccessControlPolicyTypeChannel,
Revision: 0,
Version: AccessControlPolicyVersionV0_3,
Imports: []string{parentID},
Rules: []AccessControlPolicyRule{{
Actions: []string{AccessControlPolicyActionMembership},
Expression: "true",
}},
}
err := child.Inherit(parent)
require.NotNil(t, err)
require.Equal(t, "model.access_policy.inherit.already_imported.app_error", err.Id)
})
t.Run("permission type child rejected", func(t *testing.T) {
parent := &AccessControlPolicy{
ID: NewId(),
Type: AccessControlPolicyTypeParent,
Name: "Parent",
Revision: 0,
Version: AccessControlPolicyVersionV0_3,
Rules: []AccessControlPolicyRule{{
Actions: []string{AccessControlPolicyActionMembership},
Expression: "true",
}},
}
child := &AccessControlPolicy{
ID: NewId(),
Type: AccessControlPolicyTypePermission,
Revision: 0,
Version: AccessControlPolicyVersionV0_3,
Roles: []string{"system_admin"},
Rules: []AccessControlPolicyRule{{
Actions: []string{AccessControlPolicyActionMembership},
Expression: "true",
}},
}
err := child.Inherit(parent)
require.NotNil(t, err)
require.Equal(t, "model.access_policy.inherit.permission.app_error", err.Id)
})
t.Run("permission type parent rejected", func(t *testing.T) {
parent := &AccessControlPolicy{
ID: NewId(),
Type: AccessControlPolicyTypePermission,
Revision: 0,
Version: AccessControlPolicyVersionV0_3,
Roles: []string{"system_admin"},
Rules: []AccessControlPolicyRule{{
Actions: []string{AccessControlPolicyActionMembership},
Expression: "true",
}},
}
child := &AccessControlPolicy{
ID: NewId(),
Type: AccessControlPolicyTypeChannel,
Revision: 0,
Version: AccessControlPolicyVersionV0_3,
Rules: []AccessControlPolicyRule{{
Actions: []string{AccessControlPolicyActionMembership},
Expression: "true",
}},
}
err := child.Inherit(parent)
require.NotNil(t, err)
require.Equal(t, "model.access_policy.inherit.permission.app_error", err.Id)
})
t.Run("non-v0.3 parent version rejected", func(t *testing.T) {
parent := &AccessControlPolicy{
ID: NewId(),
Type: AccessControlPolicyTypeParent,
Name: "V01 Parent",
Revision: 0,
Version: AccessControlPolicyVersionV0_1,
Rules: []AccessControlPolicyRule{{
Actions: []string{"read"},
Expression: "true",
}},
}
child := &AccessControlPolicy{
ID: NewId(),
Type: AccessControlPolicyTypeChannel,
Revision: 0,
Version: AccessControlPolicyVersionV0_3,
Rules: []AccessControlPolicyRule{{
Actions: []string{AccessControlPolicyActionMembership},
Expression: "true",
}},
}
err := child.Inherit(parent)
require.NotNil(t, err)
require.Equal(t, "model.access_policy.inherit.version.app_error", err.Id)
})
}
+1
View File
@@ -60,4 +60,5 @@ const (
MigrationKeyAddChannelAutoTranslationPermissions = "add_channel_auto_translation_permissions"
MigrationKeyAddSharedChannelManagerPermissions = "system_shared_channel_manager_permissions"
MigrationKeyRestoreManageOAuthPermission = "restore_manage_oauth_permission"
MigrationKeyAccessControlPolicyV0_3 = "access_control_policy_v0_3_migration"
)
@@ -7,6 +7,7 @@ import {FormattedMessage, useIntl} from 'react-intl';
import {GenericModal} from '@mattermost/components';
import type {AccessControlPolicy, AccessControlPolicyActiveUpdate, AccessControlPolicyRule} from '@mattermost/types/access_control';
import {getMembershipRule, buildRulesWithMembership} from '@mattermost/types/access_control';
import type {ChannelSearchOpts, ChannelWithTeamData} from '@mattermost/types/channels';
import type {AccessControlSettings} from '@mattermost/types/config';
import type {JobTypeBase} from '@mattermost/types/jobs';
@@ -74,7 +75,8 @@ function PolicyDetails({
accessControlSettings,
}: PolicyDetailsProps): JSX.Element {
const [policyName, setPolicyName] = useState(policy?.name || '');
const [expression, setExpression] = useState(policy?.rules?.[0]?.expression || '');
const [expression, setExpression] = useState(getMembershipRule(policy?.rules)?.expression || '');
const [existingRules, setExistingRules] = useState<AccessControlPolicyRule[]>(policy?.rules || []);
const [autoSyncMembership, setAutoSyncMembership] = useState(policy?.active || false);
const [serverError, setServerError] = useState<string | undefined>(undefined);
const [addChannelOpen, setAddChannelOpen] = useState(false);
@@ -151,7 +153,8 @@ function PolicyDetails({
// For existing policies, fetch policy details and channels
const policyPromise = actions.fetchPolicy(policyId).then((result) => {
setPolicyName(result.data?.name || '');
setExpression(result.data?.rules?.[0]?.expression || '');
setExpression(getMembershipRule(result.data?.rules)?.expression || '');
setExistingRules(result.data?.rules || []);
setAutoSyncMembership(result.data?.active || false);
});
@@ -197,9 +200,8 @@ function PolicyDetails({
await actions.createPolicy({
id: currentPolicyId || '',
name: policyName,
rules: [{expression, actions: ['*']}] as AccessControlPolicyRule[],
rules: buildRulesWithMembership(existingRules, expression),
type: 'parent',
version: 'v0.2',
}).then((result) => {
if (result.error) {
if (result.error.server_error_id === 'app.pap.save_policy.name_exists.app_error') {
@@ -213,7 +215,8 @@ function PolicyDetails({
}
currentPolicyId = result.data?.id;
setPolicyName(result.data?.name || '');
setExpression(result.data?.rules?.[0]?.expression || '');
setExpression(getMembershipRule(result.data?.rules)?.expression || '');
setExistingRules(result.data?.rules || []);
setAutoSyncMembership(result.data?.active || false);
});
@@ -5,7 +5,8 @@ import cloneDeep from 'lodash/cloneDeep';
import React from 'react';
import {FormattedMessage} from 'react-intl';
import type {AccessControlPolicy, AccessControlPolicyActiveUpdate} from '@mattermost/types/access_control';
import type {AccessControlPolicy, AccessControlPolicyActiveUpdate, AccessControlPolicyRule} from '@mattermost/types/access_control';
import {getMembershipRule, buildRulesWithMembership} from '@mattermost/types/access_control';
import type {Channel, ChannelModeration as ChannelPermissions, ChannelModerationPatch} from '@mattermost/types/channels';
import {SyncableType} from '@mattermost/types/groups';
import type {SyncablePatch, Group} from '@mattermost/types/groups';
@@ -95,6 +96,7 @@ interface ChannelDetailsState {
// Channel-level access rules state
channelRulesExpression: string;
channelRulesOriginalExpression: string;
channelRulesExistingRules: AccessControlPolicyRule[];
channelRulesAutoSync: boolean;
channelRulesOriginalAutoSync: boolean;
channelRulesHaveChanges: boolean;
@@ -179,6 +181,7 @@ export default class ChannelDetails extends React.PureComponent<ChannelDetailsPr
// Channel-level access rules state
channelRulesExpression: '',
channelRulesOriginalExpression: '',
channelRulesExistingRules: [],
channelRulesAutoSync: false,
channelRulesOriginalAutoSync: false,
channelRulesHaveChanges: false,
@@ -778,7 +781,6 @@ export default class ChannelDetails extends React.PureComponent<ChannelDetailsPr
id: channelID, // Channel-level policies use the channel ID as policy ID
name: accessControlPolicy?.name || `Channel Rules for ${channel.display_name}`,
type: 'channel',
version: accessControlPolicy?.version || 'v0.2',
revision: accessControlPolicy ? (accessControlPolicy.revision || 1) + 1 : 1,
created_at: accessControlPolicy?.created_at || Date.now(),
active: false, // Always save as false initially, then update separately
@@ -786,11 +788,7 @@ export default class ChannelDetails extends React.PureComponent<ChannelDetailsPr
// Include parent policies as imports
imports: this.state.accessControlPolicies.map((p) => p.id),
// Add/update channel-level rules
rules: [{
actions: ['*'],
expression: channelRulesExpression,
}],
rules: buildRulesWithMembership(this.state.channelRulesExistingRules, channelRulesExpression),
};
// Save the channel-level policy using the existing action
@@ -848,11 +846,10 @@ export default class ChannelDetails extends React.PureComponent<ChannelDetailsPr
id: accessControlPolicy?.id || channelID,
name: accessControlPolicy?.name || channel.display_name,
type: 'channel',
version: accessControlPolicy?.version || 'v0.2',
created_at: accessControlPolicy?.created_at || Date.now(),
revision: (accessControlPolicy?.revision || 1) + 1,
active: channelRulesAutoSync,
rules: [], // Remove channel-level rules
rules: buildRulesWithMembership(this.state.channelRulesExistingRules, ''),
imports: this.state.accessControlPolicies.map((p) => p.id), // SAME LOGIC as Channel Settings Modal
};
@@ -1165,11 +1162,12 @@ export default class ChannelDetails extends React.PureComponent<ChannelDetailsPr
// Check if this is a channel-level policy (not a parent policy)
if (policy.type === 'channel' && policy.rules && policy.rules.length > 0) {
const rule = policy.rules[0];
const rule = getMembershipRule(policy.rules);
const autoSyncValue = policy.active === true; // Explicitly check for true
this.setState({
channelRulesExpression: rule.expression || '',
channelRulesOriginalExpression: rule.expression || '',
channelRulesExpression: rule?.expression || '',
channelRulesOriginalExpression: rule?.expression || '',
channelRulesExistingRules: policy.rules,
channelRulesAutoSync: autoSyncValue,
channelRulesOriginalAutoSync: autoSyncValue,
channelRulesHaveChanges: false,
@@ -1188,7 +1186,7 @@ export default class ChannelDetails extends React.PureComponent<ChannelDetailsPr
private combineParentAndChannelExpressions = (channelExpression: string): string => {
// Get expressions from parent policies
const parentExpressions = this.state.accessControlPolicies.
map((policy) => policy.rules?.[0]?.expression).
map((policy) => getMembershipRule(policy.rules)?.expression).
filter((expr) => expr && expr.trim());
// Combine channel expression with parent expressions
@@ -1208,7 +1206,7 @@ export default class ChannelDetails extends React.PureComponent<ChannelDetailsPr
if (allExpressions.length === 0) {
return '';
} else if (allExpressions.length === 1) {
return allExpressions[0];
return allExpressions[0]!;
}
// Wrap each expression in parentheses and combine with &&
@@ -199,7 +199,7 @@ describe('ChannelSettingsAccessRulesTab - Activity Warning Integration', () => {
mockActions.getChannelPolicy.mockResolvedValue({
data: {
id: 'channel_id',
rules: [{expression: 'user.department == "Engineering"'}],
rules: [{actions: ['membership'], expression: 'user.department == "Engineering"'}],
active: true,
},
});
@@ -274,7 +274,7 @@ describe('ChannelSettingsAccessRulesTab - Activity Warning Integration', () => {
mockActions.getChannelPolicy.mockResolvedValue({
data: {
id: 'channel_id',
rules: [{expression: 'user.department == "Engineering"'}],
rules: [{actions: ['membership'], expression: 'user.department == "Engineering"'}],
active: true,
},
});
@@ -909,12 +909,11 @@ describe('components/channel_settings_modal/ChannelSettingsAccessRulesTab', () =
id: 'channel_id',
name: 'Test Channel',
type: 'channel',
version: 'v0.2',
active: false, // Policy starts as inactive until job completes
revision: 1,
created_at: expect.any(Number),
rules: [{
actions: ['*'],
actions: ['membership'],
expression: 'user.attributes.department == "Engineering"',
}],
imports: [],
@@ -1192,13 +1191,13 @@ describe('components/channel_settings_modal/ChannelSettingsAccessRulesTab', () =
id: 'system_policy_1',
name: 'System Policy 1',
type: 'parent',
version: 'v0.2',
version: 'v0.3',
revision: 1,
active: false,
createAt: 1234567890,
rules: [
{
actions: ['join_channel'],
actions: ['membership'],
expression: 'user.attributes.Program == "test"',
},
],
@@ -1208,13 +1207,13 @@ describe('components/channel_settings_modal/ChannelSettingsAccessRulesTab', () =
id: 'system_policy_2',
name: 'System Policy 2',
type: 'parent',
version: 'v0.2',
version: 'v0.3',
revision: 1,
active: false,
createAt: 1234567891,
rules: [
{
actions: ['join_channel'],
actions: ['membership'],
expression: 'user.attributes.Department == "Engineering"',
},
],
@@ -1775,7 +1774,7 @@ describe('components/channel_settings_modal/ChannelSettingsAccessRulesTab', () =
mockActions.getChannelPolicy.mockResolvedValue({
data: {
id: 'channel_id',
rules: [{expression: 'user.department == "Engineering"'}],
rules: [{actions: ['membership'], expression: 'user.department == "Engineering"'}],
active: false,
},
});
@@ -1818,7 +1817,7 @@ describe('components/channel_settings_modal/ChannelSettingsAccessRulesTab', () =
mockActions.getChannelPolicy.mockResolvedValue({
data: {
id: 'channel_id',
rules: [{expression: 'user.department == "Engineering"'}],
rules: [{actions: ['membership'], expression: 'user.department == "Engineering"'}],
active: false,
},
});
@@ -1877,7 +1876,7 @@ describe('components/channel_settings_modal/ChannelSettingsAccessRulesTab', () =
mockActions.getChannelPolicy.mockResolvedValue({
data: {
id: 'channel_id',
rules: [{expression: 'user.department == "Engineering"'}],
rules: [{actions: ['membership'], expression: 'user.department == "Engineering"'}],
active: false,
},
});
@@ -1938,7 +1937,7 @@ describe('components/channel_settings_modal/ChannelSettingsAccessRulesTab', () =
mockActions.getChannelPolicy.mockResolvedValue({
data: {
id: 'channel_id',
rules: [{expression: 'user.department == "Engineering"'}],
rules: [{actions: ['membership'], expression: 'user.department == "Engineering"'}],
active: true,
},
});
@@ -2008,7 +2007,7 @@ describe('components/channel_settings_modal/ChannelSettingsAccessRulesTab', () =
mockActions.getChannelPolicy.mockResolvedValue({
data: {
id: 'channel_id',
rules: [{expression: 'user.department == "Engineering"'}],
rules: [{actions: ['membership'], expression: 'user.department == "Engineering"'}],
active: true,
},
});
@@ -2062,7 +2061,7 @@ describe('components/channel_settings_modal/ChannelSettingsAccessRulesTab', () =
mockActions.getChannelPolicy.mockResolvedValue({
data: {
id: 'channel_id',
rules: [{expression: 'user.department == "Engineering"'}],
rules: [{actions: ['membership'], expression: 'user.department == "Engineering"'}],
active: true,
},
});
@@ -5,6 +5,8 @@ import React, {useState, useEffect, useCallback, useMemo, useRef} from 'react';
import {FormattedMessage, useIntl} from 'react-intl';
import {useSelector} from 'react-redux';
import type {AccessControlPolicyRule} from '@mattermost/types/access_control';
import {getMembershipRule, buildRulesWithMembership} from '@mattermost/types/access_control';
import type {Channel} from '@mattermost/types/channels';
import type {UserPropertyField} from '@mattermost/types/properties';
@@ -59,6 +61,7 @@ function ChannelSettingsAccessRulesTab({
// State for the access control expression and user attributes
const [expression, setExpression] = useState('');
const [originalExpression, setOriginalExpression] = useState('');
const [existingRules, setExistingRules] = useState<AccessControlPolicyRule[]>([]);
const [userAttributes, setUserAttributes] = useState<UserPropertyField[]>([]);
const [attributesLoaded, setAttributesLoaded] = useState(false);
@@ -121,12 +124,12 @@ function ChannelSettingsAccessRulesTab({
try {
const result = await actions.getChannelPolicy(channel.id);
if (result.data) {
// Extract expression from the policy rules
const existingExpression = result.data.rules?.[0]?.expression || '';
const existingExpression = getMembershipRule(result.data.rules)?.expression || '';
const existingAutoSync = result.data.active || false;
setExpression(existingExpression);
setOriginalExpression(existingExpression);
setExistingRules(result.data.rules || []);
setAutoSyncMembers(existingAutoSync);
setOriginalAutoSyncMembers(existingAutoSync);
}
@@ -205,7 +208,7 @@ function ChannelSettingsAccessRulesTab({
const combineSystemAndChannelExpressions = useCallback((channelExpression: string): string => {
// Get expressions from system policies
const systemExpressions = systemPolicies.
map((policy) => policy.rules?.[0]?.expression).
map((policy) => getMembershipRule(policy.rules)?.expression).
filter((expr) => expr && expr.trim());
// Combine channel expression with system expressions
@@ -225,7 +228,7 @@ function ChannelSettingsAccessRulesTab({
if (allExpressions.length === 0) {
return '';
} else if (allExpressions.length === 1) {
return allExpressions[0];
return allExpressions[0]!;
}
// Wrap each expression in parentheses and combine with &&
@@ -400,14 +403,10 @@ function ChannelSettingsAccessRulesTab({
id: channel.id,
name: channel.display_name,
type: 'channel',
version: 'v0.2',
active: false, // Always save as false initially, then update separately
revision: 1,
created_at: Date.now(),
rules: expression.trim() ? [{
actions: ['*'],
expression: expression.trim(),
}] : [],
rules: buildRulesWithMembership(existingRules, expression),
imports: systemPolicies.map((p) => p.id), // Include existing parent policies
};
+1 -1
View File
@@ -4716,7 +4716,7 @@ export default class Client4 {
getAccessControlPolicies = (after: string, limit: number) => {
return this.doFetch<AccessControlPoliciesResult>(
`${this.getBaseRoute()}/access_control_policies/search`,
{method: 'post', body: JSON.stringify({type: 'parent', cursor: {id: after}, limit})},
{method: 'post', body: JSON.stringify({type: 'parent', cursor: {id: after}, limit, actions: ['membership']})},
);
};
@@ -0,0 +1,98 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
import {getMembershipRule, buildRulesWithMembership} from './access_control';
import type {AccessControlPolicyRule} from './access_control';
describe('getMembershipRule', () => {
test('returns the membership rule when present', () => {
const rules: AccessControlPolicyRule[] = [
{actions: ['file_upload'], expression: 'upload_expr'},
{actions: ['membership'], expression: 'membership_expr'},
];
expect(getMembershipRule(rules)).toEqual({actions: ['membership'], expression: 'membership_expr'});
});
test('falls back to rules[0] for legacy v0.2 single-rule policy with wildcard action', () => {
const rules: AccessControlPolicyRule[] = [
{actions: ['*'], expression: 'legacy_expr'},
];
expect(getMembershipRule(rules)).toEqual({actions: ['*'], expression: 'legacy_expr'});
});
test('returns undefined when rules contain only non-membership, non-wildcard actions', () => {
const rules: AccessControlPolicyRule[] = [
{actions: ['file_upload'], expression: 'upload_expr'},
{actions: ['file_download'], expression: 'download_expr'},
];
expect(getMembershipRule(rules)).toBeUndefined();
});
test('returns undefined for empty rules array', () => {
expect(getMembershipRule([])).toBeUndefined();
});
test('returns undefined for undefined input', () => {
expect(getMembershipRule(undefined)).toBeUndefined();
});
});
describe('buildRulesWithMembership', () => {
test('inserts membership rule and preserves non-membership rules', () => {
const existing: AccessControlPolicyRule[] = [
{actions: ['file_upload'], expression: 'upload_expr'},
{actions: ['file_download'], expression: 'download_expr'},
];
const result = buildRulesWithMembership(existing, 'new_membership_expr');
expect(result).toEqual([
{actions: ['membership'], expression: 'new_membership_expr'},
{actions: ['file_upload'], expression: 'upload_expr'},
{actions: ['file_download'], expression: 'download_expr'},
]);
});
test('replaces existing membership rule while preserving others', () => {
const existing: AccessControlPolicyRule[] = [
{actions: ['membership'], expression: 'old_expr'},
{actions: ['file_upload'], expression: 'upload_expr'},
];
const result = buildRulesWithMembership(existing, 'new_expr');
expect(result).toEqual([
{actions: ['membership'], expression: 'new_expr'},
{actions: ['file_upload'], expression: 'upload_expr'},
]);
});
test('empty expression removes membership rule', () => {
const existing: AccessControlPolicyRule[] = [
{actions: ['membership'], expression: 'old_expr'},
{actions: ['file_upload'], expression: 'upload_expr'},
];
const result = buildRulesWithMembership(existing, '');
expect(result).toEqual([
{actions: ['file_upload'], expression: 'upload_expr'},
]);
});
test('whitespace-only expression removes membership rule', () => {
const existing: AccessControlPolicyRule[] = [
{actions: ['membership'], expression: 'old_expr'},
];
const result = buildRulesWithMembership(existing, ' ');
expect(result).toEqual([]);
});
test('trims whitespace from expression', () => {
const result = buildRulesWithMembership([], ' some_expr ');
expect(result).toEqual([
{actions: ['membership'], expression: 'some_expr'},
]);
});
test('empty existing rules with valid expression creates membership-only array', () => {
const result = buildRulesWithMembership([], 'expr');
expect(result).toEqual([
{actions: ['membership'], expression: 'expr'},
]);
});
});
@@ -42,6 +42,35 @@ export type AccessControlPolicyRule = {
expression: string;
}
/**
* Returns the first rule with a "membership" action, falling back to rules[0]
* only when it carries a wildcard action (legacy v0.2 policies).
*/
export function getMembershipRule(rules?: AccessControlPolicyRule[]): AccessControlPolicyRule | undefined {
const membership = rules?.find((r) => r.actions?.includes('membership'));
if (membership) {
return membership;
}
const first = rules?.[0];
if (first?.actions?.includes('*')) {
return first;
}
return undefined;
}
/**
* Replaces or inserts the membership rule in an existing rules array while
* preserving all non-membership rules (e.g. file_upload, file_download).
* If expression is empty the membership rule is removed.
*/
export function buildRulesWithMembership(existingRules: AccessControlPolicyRule[], expression: string): AccessControlPolicyRule[] {
const otherRules = existingRules.filter((r) => !r.actions?.includes('membership'));
if (!expression.trim()) {
return otherRules;
}
return [{actions: ['membership'], expression: expression.trim()}, ...otherRules];
}
export type CELExpressionError = {
message: string;
line: number;