diff --git a/api/v4/source/access_control.yaml b/api/v4/source/access_control.yaml index 832efbaf594..7bd8ead80ad 100644 --- a/api/v4/source/access_control.yaml +++ b/api/v4/source/access_control.yaml @@ -569,3 +569,37 @@ $ref: "#/components/responses/Forbidden" "500": $ref: "#/components/responses/InternalServerError" + /api/v4/access_control_policies/activate: + put: + tags: + - access control + summary: Activate or deactivate access control policies + description: | + Updates the active status of access control policies. + + ##### Permissions + Must have the `manage_system` permission. OR be a channel admin with manage_channel_access_rules permission for the specified channels. + operationId: UpdateAccessControlPoliciesActive + requestBody: + required: true + content: + application/json: + schema: + $ref: "#/components/schemas/AccessControlPolicyActiveUpdateRequest" + responses: + "200": + description: Access control policies active status updated successfully. + content: + application/json: + schema: + $ref: "#/components/schemas/AccessControlPolicy" + "400": + $ref: "#/components/responses/BadRequest" + "401": + $ref: "#/components/responses/Unauthorized" + "403": + $ref: "#/components/responses/Forbidden" + "500": + $ref: "#/components/responses/InternalServerError" + "501": + $ref: "#/components/responses/NotImplemented" diff --git a/api/v4/source/definitions.yaml b/api/v4/source/definitions.yaml index ff50ec2d650..bbebb7b3eba 100644 --- a/api/v4/source/definitions.yaml +++ b/api/v4/source/definitions.yaml @@ -4581,6 +4581,22 @@ components: required: - Enabled - ReviewerIds + AccessControlPolicyActiveUpdateRequest: + type: object + properties: + entries: + type: array + items: + $ref: "#/components/schemas/AccessControlPolicyActiveUpdate" + AccessControlPolicyActiveUpdate: + type: object + properties: + id: + type: string + description: The ID of the policy. + active: + type: boolean + description: The active status of the policy. externalDocs: description: Find out more about Mattermost url: 'https://about.mattermost.com' diff --git a/server/channels/api4/access_control.go b/server/channels/api4/access_control.go index 83ca6e6b374..d0faf8c4a6f 100644 --- a/server/channels/api4/access_control.go +++ b/server/channels/api4/access_control.go @@ -19,6 +19,7 @@ func (api *API) InitAccessControlPolicy() { } api.BaseRoutes.AccessControlPolicies.Handle("", api.APISessionRequired(createAccessControlPolicy)).Methods(http.MethodPut) api.BaseRoutes.AccessControlPolicies.Handle("/search", api.APISessionRequired(searchAccessControlPolicies)).Methods(http.MethodPost) + api.BaseRoutes.AccessControlPolicies.Handle("/activate", api.APISessionRequired(setActiveStatus)).Methods(http.MethodPut) api.BaseRoutes.AccessControlPolicies.Handle("/cel/check", api.APISessionRequired(checkExpression)).Methods(http.MethodPost) api.BaseRoutes.AccessControlPolicies.Handle("/cel/test", api.APISessionRequired(testExpression)).Methods(http.MethodPost) @@ -434,6 +435,42 @@ func updateActiveStatus(c *Context, w http.ResponseWriter, r *http.Request) { } } +func setActiveStatus(c *Context, w http.ResponseWriter, r *http.Request) { + var list model.AccessControlPolicyActiveUpdateRequest + if jsonErr := json.NewDecoder(r.Body).Decode(&list); jsonErr != nil { + c.SetInvalidParamWithErr("request", jsonErr) + return + } + + auditRec := c.MakeAuditRecord(model.AuditEventSetActiveStatus, model.AuditStatusFail) + defer c.LogAuditRec(auditRec) + model.AddEventParameterAuditableToAuditRec(auditRec, "requested", &list) + + // Check if user has system admin permission OR channel-specific permission for this policy + hasManageSystemPermission := c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionManageSystem) + if !hasManageSystemPermission { + for _, entry := range list.Entries { + hasChannelPermission := c.App.HasPermissionToChannel(c.AppContext, c.AppContext.Session().UserId, entry.ID, model.PermissionManageChannelAccessRules) + if !hasChannelPermission { + c.SetPermissionError(model.PermissionManageChannelAccessRules) + return + } + } + } + + policies, appErr := c.App.UpdateAccessControlPoliciesActive(c.AppContext, list.Entries) + if appErr != nil { + c.Err = appErr + return + } + auditRec.Success() + + w.Header().Set("Content-Type", "application/json") + if err := json.NewEncoder(w).Encode(policies); err != nil { + c.Logger.Warn("Error while writing response", mlog.Err(err)) + } +} + func assignAccessPolicy(c *Context, w http.ResponseWriter, r *http.Request) { if !c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionManageSystem) { c.SetPermissionError(model.PermissionManageSystem) diff --git a/server/channels/api4/access_control_local.go b/server/channels/api4/access_control_local.go index a91628f46f5..2acc143da3c 100644 --- a/server/channels/api4/access_control_local.go +++ b/server/channels/api4/access_control_local.go @@ -11,6 +11,7 @@ func (api *API) InitAccessControlPolicyLocal() { } api.BaseRoutes.AccessControlPolicies.Handle("", api.APILocal(createAccessControlPolicy)).Methods(http.MethodPut) api.BaseRoutes.AccessControlPolicies.Handle("/search", api.APILocal(searchAccessControlPolicies)).Methods(http.MethodPost) + api.BaseRoutes.AccessControlPolicies.Handle("/activate", api.APILocal(setActiveStatus)).Methods(http.MethodPut) api.BaseRoutes.AccessControlPolicies.Handle("/cel/check", api.APILocal(checkExpression)).Methods(http.MethodPost) api.BaseRoutes.AccessControlPolicies.Handle("/cel/test", api.APILocal(testExpression)).Methods(http.MethodPost) diff --git a/server/channels/api4/access_control_test.go b/server/channels/api4/access_control_test.go index 640e980fbf5..e7d11a1750c 100644 --- a/server/channels/api4/access_control_test.go +++ b/server/channels/api4/access_control_test.go @@ -855,3 +855,123 @@ func TestSearchChannelsForAccessControlPolicy(t *testing.T) { CheckForbiddenStatus(t, resp) }) } + +func TestSetActiveStatus(t *testing.T) { + th := Setup(t).InitBasic(t) + + samplePolicy := &model.AccessControlPolicy{ + ID: th.BasicChannel.Id, + Type: model.AccessControlPolicyTypeChannel, + Version: model.AccessControlPolicyVersionV0_2, + Revision: 1, + Rules: []model.AccessControlPolicyRule{ + { + Expression: "user.attributes.team == 'engineering'", + Actions: []string{"*"}, + }, + }, + } + var err error + samplePolicy, err = th.App.Srv().Store().AccessControlPolicy().Save(th.Context, samplePolicy) + require.NoError(t, err) + + // Sample update request + updateReq := model.AccessControlPolicyActiveUpdateRequest{ + Entries: []model.AccessControlPolicyActiveUpdate{ + {ID: samplePolicy.ID, Active: true}, + }, + } + + t.Run("SetActiveStatus without license", func(t *testing.T) { + _, resp, err := th.SystemAdminClient.SetAccessControlPolicyActive(context.Background(), updateReq) + require.Error(t, err) + CheckNotImplementedStatus(t, resp) + }) + + t.Run("SetActiveStatus with regular user", func(t *testing.T) { + ok := th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuEnterpriseAdvanced)) + require.True(t, ok, "SetLicense should return true") + + // Remove permission from regular user + _, resp, err := th.Client.SetAccessControlPolicyActive(context.Background(), updateReq) + require.Error(t, err) + CheckForbiddenStatus(t, resp) + }) + + th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) { + ok := th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuEnterpriseAdvanced)) + require.True(t, ok, "SetLicense should return true") + + th.App.UpdateConfig(func(cfg *model.Config) { + cfg.AccessControlSettings.EnableAttributeBasedAccessControl = model.NewPointer(true) + }) + + mockAccessControlService := &mocks.AccessControlServiceInterface{} + th.App.Srv().Channels().AccessControl = mockAccessControlService + + policies, resp, err := client.SetAccessControlPolicyActive(context.Background(), updateReq) + require.NoError(t, err) + CheckOKStatus(t, resp) + require.NotNil(t, policies, "expected policies in response") + require.Len(t, policies, 1, "expected one policy in response") + require.Equal(t, samplePolicy.ID, policies[0].ID, "expected policy ID to match") + require.True(t, policies[0].Active, "expected policy to be active") + }, "SetActiveStatus with system admin") + + t.Run("SetActiveStatus with channel admin for their channel", func(t *testing.T) { + th.App.UpdateConfig(func(cfg *model.Config) { + cfg.AccessControlSettings.EnableAttributeBasedAccessControl = model.NewPointer(true) + }) + + ok := th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuEnterpriseAdvanced)) + require.True(t, ok, "SetLicense should return true") + + // Add permission to channel admin role + th.AddPermissionToRole(t, model.PermissionManageChannelAccessRules.Id, model.ChannelAdminRoleId) + // Create private channel and make user channel admin + privateChannel := th.CreatePrivateChannel(t) + channelAdmin := th.CreateUser(t) + th.LinkUserToTeam(t, channelAdmin, th.BasicTeam) + th.AddUserToChannel(t, channelAdmin, privateChannel) + th.MakeUserChannelAdmin(t, channelAdmin, privateChannel) + + channelPolicy := &model.AccessControlPolicy{ + ID: privateChannel.Id, + Type: model.AccessControlPolicyTypeChannel, + Version: model.AccessControlPolicyVersionV0_2, + Revision: 1, + Rules: []model.AccessControlPolicyRule{ + { + Expression: "user.attributes.team == 'engineering'", + Actions: []string{"*"}, + }, + }, + } + var err error + channelPolicy, err = th.App.Srv().Store().AccessControlPolicy().Save(th.Context, channelPolicy) + require.NoError(t, err) + + channelAdminClient := th.CreateClient() + _, _, err = channelAdminClient.Login(context.Background(), channelAdmin.Email, channelAdmin.Password) + require.NoError(t, err) + + // Update request for the channel admin's channel + channelUpdateReq := model.AccessControlPolicyActiveUpdateRequest{ + Entries: []model.AccessControlPolicyActiveUpdate{ + {ID: privateChannel.Id, Active: true}, + }, + } + + mockAccessControlService := &mocks.AccessControlServiceInterface{} + th.App.Srv().Channels().AccessControl = mockAccessControlService + + // Channel admin should be able to set active status for their channel + policies, resp, err := channelAdminClient.SetAccessControlPolicyActive(context.Background(), channelUpdateReq) + require.NoError(t, err) + CheckOKStatus(t, resp) + require.NotNil(t, policies, "expected policies in response") + require.Len(t, policies, 1, "expected one policy in response") + require.Equal(t, channelPolicy.ID, policies[0].ID, "expected policy ID to match") + require.True(t, policies[0].Active, "expected policy to be active") + }) +} diff --git a/server/channels/app/access_control.go b/server/channels/app/access_control.go index 59e3b60dc4d..1475c4690ea 100644 --- a/server/channels/app/access_control.go +++ b/server/channels/app/access_control.go @@ -311,6 +311,19 @@ func (a *App) UpdateAccessControlPolicyActive(rctx request.CTX, policyID string, return nil } +func (a *App) UpdateAccessControlPoliciesActive(rctx request.CTX, updates []model.AccessControlPolicyActiveUpdate) ([]*model.AccessControlPolicy, *model.AppError) { + acs := a.Srv().ch.AccessControl + if acs == nil { + return nil, model.NewAppError("ExpressionToVisualAST", "app.pap.update_access_control_policies_active.app_error", nil, "Policy Administration Point is not initialized", http.StatusNotImplemented) + } + + policies, err := a.Srv().Store().AccessControlPolicy().SetActiveStatusMultiple(rctx, updates) + if err != nil { + return nil, model.NewAppError("UpdateAccessControlPoliciesActive", "app.pap.update_access_control_policies_active.app_error", nil, err.Error(), http.StatusInternalServerError) + } + return policies, nil +} + func (a *App) ExpressionToVisualAST(rctx request.CTX, expression string) (*model.VisualExpression, *model.AppError) { acs := a.Srv().ch.AccessControl if acs == nil { diff --git a/server/channels/store/retrylayer/retrylayer.go b/server/channels/store/retrylayer/retrylayer.go index 087914aa656..a817700f91a 100644 --- a/server/channels/store/retrylayer/retrylayer.go +++ b/server/channels/store/retrylayer/retrylayer.go @@ -658,6 +658,27 @@ func (s *RetryLayerAccessControlPolicyStore) SetActiveStatus(rctx request.CTX, i } +func (s *RetryLayerAccessControlPolicyStore) SetActiveStatusMultiple(rctx request.CTX, list []model.AccessControlPolicyActiveUpdate) ([]*model.AccessControlPolicy, error) { + + tries := 0 + for { + result, err := s.AccessControlPolicyStore.SetActiveStatusMultiple(rctx, list) + if err == nil { + return result, nil + } + if !isRepeatableError(err) { + return result, err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return result, err + } + timepkg.Sleep(100 * timepkg.Millisecond) + } + +} + func (s *RetryLayerAttributesStore) GetChannelMembersToRemove(rctx request.CTX, channelID string, opts model.SubjectSearchOptions) ([]*model.ChannelMember, error) { tries := 0 diff --git a/server/channels/store/sqlstore/access_control_policy_store.go b/server/channels/store/sqlstore/access_control_policy_store.go index de00d5c7189..9e9828b8726 100644 --- a/server/channels/store/sqlstore/access_control_policy_store.go +++ b/server/channels/store/sqlstore/access_control_policy_store.go @@ -381,6 +381,85 @@ func (s *SqlAccessControlPolicyStore) SetActiveStatus(rctx request.CTX, id strin return existingPolicy, nil } +func (s *SqlAccessControlPolicyStore) SetActiveStatusMultiple(rctx request.CTX, list []model.AccessControlPolicyActiveUpdate) ([]*model.AccessControlPolicy, error) { + tx, err := s.GetMaster().Beginx() + if err != nil { + return nil, errors.Wrap(err, "failed to start transaction") + } + defer finalizeTransactionX(tx, &err) + + // Group by active status for batch updates + activeTrue := []string{} + activeFalse := []string{} + ids := make([]any, 0, len(list)) + for _, entry := range list { + ids = append(ids, entry.ID) + if entry.Active { + activeTrue = append(activeTrue, entry.ID) + continue + } + activeFalse = append(activeFalse, entry.ID) + } + + // Update active=true policies + if len(activeTrue) > 0 { + query, args, qbErr := s.getQueryBuilder(). + Update("AccessControlPolicies"). + Set("Active", true). + Where(sq.Eq{"ID": activeTrue}). + ToSql() + + if qbErr != nil { + return nil, errors.Wrap(qbErr, "failed to build active=true update query") + } + + _, err = tx.Exec(query, args...) + if err != nil { + return nil, errors.Wrap(err, "failed to update active=true policies") + } + } + + // Update active=false policies + if len(activeFalse) > 0 { + query, args, qbErr := s.getQueryBuilder(). + Update("AccessControlPolicies"). + Set("Active", false). + Where(sq.Eq{"ID": activeFalse}). + ToSql() + + if qbErr != nil { + return nil, errors.Wrap(qbErr, "failed to build active=false update query") + } + + _, err = tx.Exec(query, args...) + if err != nil { + return nil, errors.Wrap(err, "failed to update active=false policies") + } + } + + p := []storeAccessControlPolicy{} + query := s.selectQueryBuilder.Where(sq.Eq{"ID": ids}) + + err = tx.SelectBuilder(&p, query) + if err != nil { + return nil, errors.Wrapf(err, "failed to find policies with ids=%v", ids) + } + + policies := make([]*model.AccessControlPolicy, len(p)) + for i := range p { + policies[i], err = p[i].toModel() + if err != nil { + return nil, errors.Wrapf(err, "failed to parse policy with id=%s", p[i].ID) + } + } + + if err = tx.Commit(); err != nil { + return nil, errors.Wrap(err, "commit_transaction") + } + + return policies, nil +} + func (s *SqlAccessControlPolicyStore) Get(_ request.CTX, id string) (*model.AccessControlPolicy, error) { p := storeAccessControlPolicy{} query := s.selectQueryBuilder.Where(sq.Eq{"ID": id}) diff --git a/server/channels/store/sqlstore/channel_store.go b/server/channels/store/sqlstore/channel_store.go index de2776f08a3..983fe0685c6 100644 --- a/server/channels/store/sqlstore/channel_store.go +++ b/server/channels/store/sqlstore/channel_store.go @@ -145,6 +145,7 @@ func channelSliceColumns(isSelect bool, prefix ...string) []string { } columns = append(columns, fmt.Sprintf("EXISTS (SELECT 1 FROM AccessControlPolicies acp WHERE acp.ID = %sId) AS PolicyEnforced", p)) + columns = append(columns, fmt.Sprintf("COALESCE((SELECT acp.Active FROM AccessControlPolicies acp WHERE acp.ID = %sId AND acp.Active = TRUE LIMIT 1), false) AS PolicyIsActive", p)) } return columns diff --git a/server/channels/store/store.go b/server/channels/store/store.go index fe5dbcaaee7..da0e29180e6 100644 --- a/server/channels/store/store.go +++ b/server/channels/store/store.go @@ -1137,6 +1137,7 @@ type AccessControlPolicyStore interface { Save(rctx request.CTX, policy *model.AccessControlPolicy) (*model.AccessControlPolicy, error) Delete(rctx request.CTX, id string) error SetActiveStatus(rctx request.CTX, id string, active bool) (*model.AccessControlPolicy, error) + SetActiveStatusMultiple(rctx request.CTX, list []model.AccessControlPolicyActiveUpdate) ([]*model.AccessControlPolicy, error) Get(rctx request.CTX, id string) (*model.AccessControlPolicy, error) SearchPolicies(rctx request.CTX, opts model.AccessControlPolicySearch) ([]*model.AccessControlPolicy, int64, error) } diff --git a/server/channels/store/storetest/access_control_policy_store.go b/server/channels/store/storetest/access_control_policy_store.go index fcf00b67168..9635cc0b397 100644 --- a/server/channels/store/storetest/access_control_policy_store.go +++ b/server/channels/store/storetest/access_control_policy_store.go @@ -16,6 +16,7 @@ func TestAccessControlPolicyStore(t *testing.T, rctx request.CTX, ss store.Store t.Run("Save", func(t *testing.T) { testAccessControlPolicyStoreSaveAndGet(t, rctx, ss) }) t.Run("Delete", func(t *testing.T) { testAccessControlPolicyStoreDelete(t, rctx, ss) }) t.Run("SetActive", func(t *testing.T) { testAccessControlPolicyStoreSetActive(t, rctx, ss) }) + 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) }) } @@ -468,3 +469,77 @@ func testAccessControlPolicyStoreSearch(t *testing.T, rctx request.CTX, ss store require.Len(t, policies, 15) }) } + +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{ + ID: model.NewId(), + Name: "Policy1", + Type: model.AccessControlPolicyTypeChannel, + Active: false, + Revision: 1, + Version: model.AccessControlPolicyVersionV0_2, + Imports: []string{}, + Rules: []model.AccessControlPolicyRule{ + { + Actions: []string{"action1"}, + Expression: "user.properties.program == \"engineering\"", + }, + }, + } + + policy2 := &model.AccessControlPolicy{ + ID: model.NewId(), + Name: "Policy2", + Type: model.AccessControlPolicyTypeParent, + Active: false, + Revision: 1, + Version: model.AccessControlPolicyVersionV0_2, + Imports: []string{}, + Rules: []model.AccessControlPolicyRule{ + { + Actions: []string{"action2"}, + Expression: "user.properties.department == \"sales\"", + }, + }, + } + + policy1, err := ss.AccessControlPolicy().Save(rctx, policy1) + require.NoError(t, err) + require.NotNil(t, policy1) + + policy2, err = ss.AccessControlPolicy().Save(rctx, policy2) + require.NoError(t, err) + require.NotNil(t, policy2) + + t.Cleanup(func() { + err = ss.AccessControlPolicy().Delete(rctx, policy1.ID) + require.NoError(t, err) + err = ss.AccessControlPolicy().Delete(rctx, policy2.ID) + require.NoError(t, err) + }) + + updates := []model.AccessControlPolicyActiveUpdate{ + {ID: policy1.ID, Active: true}, + {ID: policy2.ID, Active: true}, + } + + updatedPolicies, err := ss.AccessControlPolicy().SetActiveStatusMultiple(rctx, updates) + require.NoError(t, err) + require.Len(t, updatedPolicies, 2) + + for _, p := range updatedPolicies { + require.True(t, p.Active) + } + + p1, err := ss.AccessControlPolicy().Get(rctx, policy1.ID) + require.NoError(t, err) + require.NotNil(t, p1) + require.True(t, p1.Active) + + p2, err := ss.AccessControlPolicy().Get(rctx, policy2.ID) + require.NoError(t, err) + require.NotNil(t, p2) + require.True(t, p2.Active) + }) +} diff --git a/server/channels/store/storetest/mocks/AccessControlPolicyStore.go b/server/channels/store/storetest/mocks/AccessControlPolicyStore.go index 1ee3850f00c..e63426de240 100644 --- a/server/channels/store/storetest/mocks/AccessControlPolicyStore.go +++ b/server/channels/store/storetest/mocks/AccessControlPolicyStore.go @@ -160,6 +160,36 @@ func (_m *AccessControlPolicyStore) SetActiveStatus(rctx request.CTX, id string, return r0, r1 } +// SetActiveStatusMultiple provides a mock function with given fields: rctx, list +func (_m *AccessControlPolicyStore) SetActiveStatusMultiple(rctx request.CTX, list []model.AccessControlPolicyActiveUpdate) ([]*model.AccessControlPolicy, error) { + ret := _m.Called(rctx, list) + + if len(ret) == 0 { + panic("no return value specified for SetActiveStatusMultiple") + } + + var r0 []*model.AccessControlPolicy + var r1 error + if rf, ok := ret.Get(0).(func(request.CTX, []model.AccessControlPolicyActiveUpdate) ([]*model.AccessControlPolicy, error)); ok { + return rf(rctx, list) + } + if rf, ok := ret.Get(0).(func(request.CTX, []model.AccessControlPolicyActiveUpdate) []*model.AccessControlPolicy); ok { + r0 = rf(rctx, list) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]*model.AccessControlPolicy) + } + } + + if rf, ok := ret.Get(1).(func(request.CTX, []model.AccessControlPolicyActiveUpdate) error); ok { + r1 = rf(rctx, list) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // NewAccessControlPolicyStore creates a new instance of AccessControlPolicyStore. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. // The first argument is typically a *testing.T value. func NewAccessControlPolicyStore(t interface { diff --git a/server/channels/store/timerlayer/timerlayer.go b/server/channels/store/timerlayer/timerlayer.go index 008313b4264..c61e28a82f1 100644 --- a/server/channels/store/timerlayer/timerlayer.go +++ b/server/channels/store/timerlayer/timerlayer.go @@ -621,6 +621,22 @@ func (s *TimerLayerAccessControlPolicyStore) SetActiveStatus(rctx request.CTX, i return result, err } +func (s *TimerLayerAccessControlPolicyStore) SetActiveStatusMultiple(rctx request.CTX, list []model.AccessControlPolicyActiveUpdate) ([]*model.AccessControlPolicy, error) { + start := time.Now() + + result, err := s.AccessControlPolicyStore.SetActiveStatusMultiple(rctx, list) + + elapsed := float64(time.Since(start)) / float64(time.Second) + if s.Root.Metrics != nil { + success := "false" + if err == nil { + success = "true" + } + s.Root.Metrics.ObserveStoreMethodDuration("AccessControlPolicyStore.SetActiveStatusMultiple", success, elapsed) + } + return result, err +} + func (s *TimerLayerAttributesStore) GetChannelMembersToRemove(rctx request.CTX, channelID string, opts model.SubjectSearchOptions) ([]*model.ChannelMember, error) { start := time.Now() diff --git a/server/i18n/en.json b/server/i18n/en.json index 29c9fad658b..cca97a3b546 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -6758,6 +6758,10 @@ "id": "app.pap.unassign_access_control_policy_from_channels.app_error", "translation": "Could not unassign access control policy from channels." }, + { + "id": "app.pap.update_access_control_policies_active.app_error", + "translation": "Could not update active status of access control policies." + }, { "id": "app.pap.update_access_control_policy_active.app_error", "translation": "Could not change active status of access control policy." diff --git a/server/public/model/access_policy.go b/server/public/model/access_policy.go index abce3383823..e3d2e2ca1e6 100644 --- a/server/public/model/access_policy.go +++ b/server/public/model/access_policy.go @@ -90,6 +90,30 @@ type AccessControlQueryResult struct { MatchedSubjectIDs []string `json:"matched_subject_ids"` } +// AccessControlPolicyActiveUpdate represents a single policy's active status update. +type AccessControlPolicyActiveUpdate struct { + ID string `json:"id"` + Active bool `json:"active"` +} + +// AccessControlPolicyActiveUpdateRequest is used in the API to update active status for multiple policies. +type AccessControlPolicyActiveUpdateRequest struct { + Entries []AccessControlPolicyActiveUpdate `json:"entries"` +} + +func (r *AccessControlPolicyActiveUpdateRequest) Auditable() map[string]any { + entries := make([]map[string]any, 0, len(r.Entries)) + for _, entry := range r.Entries { + entries = append(entries, map[string]any{ + "id": entry.ID, + "active": entry.Active, + }) + } + return map[string]any{ + "entries": entries, + } +} + func (p *AccessControlPolicy) IsValid() *AppError { switch p.Version { case AccessControlPolicyVersionV0_1: diff --git a/server/public/model/audit_events.go b/server/public/model/audit_events.go index 3633b470044..06efacce91f 100644 --- a/server/public/model/audit_events.go +++ b/server/public/model/audit_events.go @@ -11,6 +11,7 @@ const ( AuditEventDeleteAccessControlPolicy = "deleteAccessControlPolicy" // delete access control policy AuditEventUnassignAccessPolicy = "unassignAccessPolicy" // remove access control policy from channels AuditEventUpdateActiveStatus = "updateActiveStatus" // update active/inactive status of access control policy + AuditEventSetActiveStatus = "setActiveStatus" // set active/inactive status of multiple access control policies ) // Audit & Certificates diff --git a/server/public/model/channel.go b/server/public/model/channel.go index 615bbc75cfa..9ed4f016e44 100644 --- a/server/public/model/channel.go +++ b/server/public/model/channel.go @@ -100,6 +100,7 @@ type Channel struct { LastRootPostAt int64 `json:"last_root_post_at"` BannerInfo *ChannelBannerInfo `json:"banner_info"` PolicyEnforced bool `json:"policy_enforced"` + PolicyIsActive bool `json:"policy_is_active"` DefaultCategoryName string `json:"default_category_name"` } @@ -122,6 +123,7 @@ func (o *Channel) Auditable() map[string]any { "type": o.Type, "update_at": o.UpdateAt, "policy_enforced": o.PolicyEnforced, + "policy_is_active": o.PolicyIsActive, // this field is only for logging purposes } } diff --git a/server/public/model/client4.go b/server/public/model/client4.go index df47093805e..56b63dd9631 100644 --- a/server/public/model/client4.go +++ b/server/public/model/client4.go @@ -7757,3 +7757,18 @@ func (c *Client4) SearchChannelsForAccessControlPolicy(ctx context.Context, poli defer closeBody(r) return DecodeJSONFromResponse[*ChannelsWithCount](r) } + +func (c *Client4) SetAccessControlPolicyActive(ctx context.Context, update AccessControlPolicyActiveUpdateRequest) ([]*AccessControlPolicy, *Response, error) { + r, err := c.DoAPIPutJSON(ctx, c.accessControlPoliciesRoute()+"/activate", update) + if err != nil { + return nil, BuildResponse(r), err + } + defer closeBody(r) + + var policies []*AccessControlPolicy + if err := json.NewDecoder(r.Body).Decode(&policies); err != nil { + return nil, nil, NewAppError("SetAccessControlPolicyActive", "api.unmarshal_error", nil, "", http.StatusInternalServerError).Wrap(err) + } + + return policies, BuildResponse(r), nil +} diff --git a/webapp/channels/src/components/admin_console/access_control/policy_details/__snapshots__/policy_details.test.tsx.snap b/webapp/channels/src/components/admin_console/access_control/policy_details/__snapshots__/policy_details.test.tsx.snap index 4d8d778247c..1778f1f3bcd 100644 --- a/webapp/channels/src/components/admin_console/access_control/policy_details/__snapshots__/policy_details.test.tsx.snap +++ b/webapp/channels/src/components/admin_console/access_control/policy_details/__snapshots__/policy_details.test.tsx.snap @@ -42,28 +42,6 @@ exports[`components/admin_console/access_control/policy_details/PolicyDetails sh placeholder="Add a unique policy name" value="" /> - - } - id="admin.access_control.policy.edit_policy.autoSyncMembership" - label={ -
- -
- } - onChange={[Function]} - setByEnv={false} - value={false} - /> - @@ -201,6 +182,7 @@ exports[`components/admin_console/access_control/policy_details/PolicyDetails sh } disabled={true} onClick={[Function]} + saving={false} /> - - } - id="admin.access_control.policy.edit_policy.autoSyncMembership" - label={ -
- -
- } - onChange={[Function]} - setByEnv={false} - value={false} - /> - @@ -387,6 +350,7 @@ exports[`components/admin_console/access_control/policy_details/PolicyDetails sh } disabled={true} onClick={[Function]} + saving={false} /> - , - }, - Object { - "field": "team", - "fixed": true, - "name": , - }, - Object { - "field": "remove", - "fixed": true, - "name": "", - "textAlign": "right", - }, - ] - } - endCount={2} - filterProps={ - Object { - "keys": Array [ - "teams", - ], - "onFilter": [Function], - "options": Object { - "teams": Object { - "keys": Array [ - "team_ids", - ], - "name": "Teams", - "type": Object { - "$$typeof": Symbol(react.memo), - "WrappedComponent": [Function], - "compare": null, - "type": [Function], - }, - "values": Object { - "team_ids": Object { - "name": , - "value": Array [], - }, - }, - }, - }, - } - } - loading={true} - nextPage={[Function]} - onSearch={[Function]} - page={0} - previousPage={[Function]} - rows={Array []} - startCount={1} - term="" - total={2} - /> - + + + `; exports[`components/admin_console/access_control/channel_list should match snapshot with channels to add 1`] = ` -
- , - }, - Object { - "field": "team", - "fixed": true, - "name": , - }, - Object { - "field": "remove", - "fixed": true, - "name": "", - "textAlign": "right", - }, - ] - } - endCount={3} - filterProps={ - Object { - "keys": Array [ - "teams", - ], - "onFilter": [Function], - "options": Object { - "teams": Object { - "keys": Array [ - "team_ids", - ], - "name": "Teams", - "type": Object { - "$$typeof": Symbol(react.memo), - "WrappedComponent": [Function], - "compare": null, - "type": [Function], - }, - "values": Object { - "team_ids": Object { - "name": , - "value": Array [], - }, - }, - }, - }, - } - } - loading={true} - nextPage={[Function]} - onSearch={[Function]} - page={0} - previousPage={[Function]} - rows={ - Array [ - Object { - "cells": Object { - "id": "channel3", - "name":
- -
- - Channel 3 - -
-
, - "remove": - - , - "team": "Team 1", - }, - }, - ] - } - startCount={1} - term="" - total={3} - /> -
+ + + `; exports[`components/admin_console/access_control/channel_list should match snapshot with channels to remove 1`] = ` -
- , - }, - Object { - "field": "team", - "fixed": true, - "name": , - }, - Object { - "field": "remove", - "fixed": true, - "name": "", - "textAlign": "right", - }, - ] - } - endCount={2} - filterProps={ - Object { - "keys": Array [ - "teams", - ], - "onFilter": [Function], - "options": Object { - "teams": Object { - "keys": Array [ - "team_ids", - ], - "name": "Teams", - "type": Object { - "$$typeof": Symbol(react.memo), - "WrappedComponent": [Function], - "compare": null, - "type": [Function], - }, - "values": Object { - "team_ids": Object { - "name": , - "value": Array [], - }, - }, - }, - }, - } - } - loading={true} - nextPage={[Function]} - onSearch={[Function]} - page={0} - previousPage={[Function]} - rows={Array []} - startCount={1} - term="" - total={2} - /> -
+ + + `; exports[`components/admin_console/access_control/channel_list should match snapshot with no channels 1`] = ` -
- , - }, - Object { - "field": "team", - "fixed": true, - "name": , - }, - Object { - "field": "remove", - "fixed": true, - "name": "", - "textAlign": "right", - }, - ] - } - endCount={0} - filterProps={ - Object { - "keys": Array [ - "teams", - ], - "onFilter": [Function], - "options": Object { - "teams": Object { - "keys": Array [ - "team_ids", - ], - "name": "Teams", - "type": Object { - "$$typeof": Symbol(react.memo), - "WrappedComponent": [Function], - "compare": null, - "type": [Function], - }, - "values": Object { - "team_ids": Object { - "name": , - "value": Array [], - }, - }, - }, - }, - } - } - loading={false} - nextPage={[Function]} - onSearch={[Function]} - page={0} - previousPage={[Function]} - rows={Array []} - startCount={1} - term="" - total={0} - /> -
+ + + `; diff --git a/webapp/channels/src/components/admin_console/access_control/policy_details/channel_list/channel_list.scss b/webapp/channels/src/components/admin_console/access_control/policy_details/channel_list/channel_list.scss index 7bc6c632dde..5188a4915a5 100644 --- a/webapp/channels/src/components/admin_console/access_control/policy_details/channel_list/channel_list.scss +++ b/webapp/channels/src/components/admin_console/access_control/policy_details/channel_list/channel_list.scss @@ -16,6 +16,156 @@ } } + .ChannelList__autoAddHeader { + display: flex; + align-items: center; + justify-content: flex-start; + padding-left: 16px; + gap: 8px; + + .header-checkbox { + position: relative; + display: inline-block; + width: 16px; + height: 16px; + padding: 0; + border: 1px solid var(--center-channel-color); + border-radius: 2px; + appearance: none; + background: none; + cursor: pointer; + transition: all 0.15s ease; + + > * { + pointer-events: none; + } + + &:checked { + border: 0; + background-color: var(--button-bg); + + &::after { + position: absolute; + top: 1px; + left: 5px; + width: 6px; + height: 10px; + border: solid white; + border-width: 0 1.5px 1.5px 0; + content: ''; + transform: rotate(45deg); + } + } + + &:indeterminate { + border: 0; + background-color: var(--button-bg); + + &::after { + position: absolute; + top: 7px; + left: 3px; + width: 10px; + height: 2px; + background-color: white; + content: ''; + } + } + + &:focus { + box-shadow: 0 0 0 2px rgba(var(--button-bg-rgb), 0.2); + outline: none; + } + + &:hover:not(:disabled) { + border-color: var(--button-bg); + } + + &:disabled { + border-color: rgba(var(--sys-center-channel-color-rgb), 0.16); + cursor: not-allowed; + opacity: 0.48; + } + } + + .header-text { + letter-spacing: 0.02em; + } + + .ChannelList__autoAddInfoIcon { + margin-left: -6px; + color: rgba(var(--sys-center-channel-color-rgb), 0.56); + font-size: 16px; + + &:hover { + color: rgba(var(--sys-center-channel-color-rgb), 0.72); + } + } + } + + .ChannelList__autoAddColumn { + display: flex; + align-items: center; + justify-content: flex-start; + padding-left: 16px; + gap: 8px; + + .channel-checkbox { + position: relative; + display: inline-block; + width: 16px; + height: 16px; + padding: 0; + border: 1px solid var(--center-channel-color); + border-radius: 2px; + appearance: none; + background: none; + cursor: pointer; + transition: all 0.15s ease; + + > * { + pointer-events: none; + } + + &:checked { + border: 0; + background-color: var(--button-bg); + + &::after { + position: absolute; + top: 1px; + left: 5px; + width: 6px; + height: 10px; + border: solid white; + border-width: 0 1.5px 1.5px 0; + content: ''; + transform: rotate(45deg); + } + } + + &:focus { + box-shadow: 0 0 0 2px rgba(var(--button-bg-rgb), 0.2); + outline: none; + } + + &:hover:not(:disabled) { + border-color: var(--button-bg); + } + + &:disabled { + border-color: rgba(var(--sys-center-channel-color-rgb), 0.16); + cursor: not-allowed; + opacity: 0.48; + } + } + + .checkbox-label { + line-height: 16px; + user-select: none; + } + } + .DataGrid { padding: 0; background-color: rgba(var(--sys-white-rgb), 0.04); diff --git a/webapp/channels/src/components/admin_console/access_control/policy_details/channel_list/channel_list.tsx b/webapp/channels/src/components/admin_console/access_control/policy_details/channel_list/channel_list.tsx index 2d052eb7a76..044c8cae2e6 100644 --- a/webapp/channels/src/components/admin_console/access_control/policy_details/channel_list/channel_list.tsx +++ b/webapp/channels/src/components/admin_console/access_control/policy_details/channel_list/channel_list.tsx @@ -4,7 +4,8 @@ import debounce from 'lodash/debounce'; import isEqual from 'lodash/isEqual'; import React from 'react'; -import {FormattedMessage} from 'react-intl'; +import {FormattedMessage, injectIntl} from 'react-intl'; +import type {WrappedComponentProps} from 'react-intl'; import type {ChannelSearchOpts, ChannelWithTeamData} from '@mattermost/types/channels'; @@ -17,13 +18,19 @@ import TeamFilterDropdown from 'components/admin_console/filter/team_filter_drop import ArchiveIcon from 'components/widgets/icons/archive_icon'; import GlobeIcon from 'components/widgets/icons/globe_icon'; import LockIcon from 'components/widgets/icons/lock_icon'; +import WithTooltip from 'components/with_tooltip'; import {isArchivedChannel} from 'utils/channel_utils'; import {Constants} from 'utils/constants'; import './channel_list.scss'; -type Props = { +type PolicyActiveStatus = { + id: string; + active: boolean; +} + +type Props = WrappedComponentProps & { channels: ChannelWithTeamData[]; totalCount: number; searchTerm: string; @@ -32,6 +39,9 @@ type Props = { onRemoveCallback: (channel: ChannelWithTeamData) => void; channelsToRemove: Record; channelsToAdd: Record; + policyActiveStatusChanges?: PolicyActiveStatus[]; + onPolicyActiveStatusChange?: (changes: PolicyActiveStatus[]) => void; + saving?: boolean; actions: { searchChannels: (id: string, term: string, opts: ChannelSearchOpts) => Promise; setChannelListSearch: (term: string) => void; @@ -48,7 +58,7 @@ type State = { const PAGE_SIZE = 10; -export default class ChannelList extends React.PureComponent { +class ChannelList extends React.PureComponent { private mounted = false; private searchDebounced; @@ -207,6 +217,119 @@ export default class ChannelList extends React.PureComponent { } }; + private handleAutoAddToggle = (channelId: string, currentStatus: boolean) => { + const {policyActiveStatusChanges = [], onPolicyActiveStatusChange, saving} = this.props; + + if (!onPolicyActiveStatusChange || saving) { + return; + } + + const newStatus = !currentStatus; + const existingChangeIndex = policyActiveStatusChanges.findIndex((change) => change.id === channelId); + const updatedChanges = [...policyActiveStatusChanges]; + + if (existingChangeIndex >= 0) { + // Update existing change + updatedChanges[existingChangeIndex] = { + id: channelId, + active: newStatus, + }; + } else { + // Add new change + updatedChanges.push({ + id: channelId, + active: newStatus, + }); + } + + onPolicyActiveStatusChange(updatedChanges); + }; + + private getChannelAutoAddStatus = (channelId: string): boolean => { + const {policyActiveStatusChanges = [], channels, channelsToAdd} = this.props; + const change = policyActiveStatusChanges.find((change) => change.id === channelId); + + // If there's a pending change, use that status + if (change) { + return change.active; + } + + // Find the channel to get its current policy_is_active status + const allChannels = [...channels, ...Object.values(channelsToAdd)]; + const channel = allChannels.find((ch) => ch.id === channelId); + + // Use the channel's policy_is_active value, defaulting to false if undefined + return channel?.policy_is_active ?? false; + }; + + private getAllChannelsAutoAddStatus = (): {allActive: boolean; allInactive: boolean; mixed: boolean} => { + const {channels, channelsToAdd, channelsToRemove} = this.props; + const {startCount, endCount} = this.getPaginationProps(); + + // Get all visible channels + const channelsToDisplay = [ + ...Object.values(channelsToAdd), + ...channels.filter((channel) => !channelsToRemove[channel.id]), + ].slice(startCount - 1, endCount); + + if (channelsToDisplay.length === 0) { + return {allActive: false, allInactive: false, mixed: false}; + } + + let activeCount = 0; + channelsToDisplay.forEach((channel) => { + if (this.getChannelAutoAddStatus(channel.id)) { + activeCount++; + } + }); + + const allActive = activeCount === channelsToDisplay.length; + const allInactive = activeCount === 0; + const mixed = !allActive && !allInactive; + + return {allActive, allInactive, mixed}; + }; + + private handleBulkAutoAddToggle = () => { + const {channels, channelsToAdd, channelsToRemove, policyActiveStatusChanges = [], onPolicyActiveStatusChange, saving} = this.props; + const {startCount, endCount} = this.getPaginationProps(); + + if (!onPolicyActiveStatusChange || saving) { + return; + } + + // Get all visible channels + const channelsToDisplay = [ + ...Object.values(channelsToAdd), + ...channels.filter((channel) => !channelsToRemove[channel.id]), + ].slice(startCount - 1, endCount); + + const {allActive} = this.getAllChannelsAutoAddStatus(); + const newStatus = !allActive; // If all are active, make them inactive; otherwise make them all active + + const updatedChanges = [...policyActiveStatusChanges]; + + channelsToDisplay.forEach((channel) => { + const existingChangeIndex = updatedChanges.findIndex((change) => change.id === channel.id); + + if (existingChangeIndex >= 0) { + // Update existing change + updatedChanges[existingChangeIndex] = { + id: channel.id, + active: newStatus, + }; + } else { + // Add new change + updatedChanges.push({ + id: channel.id, + active: newStatus, + }); + } + }); + + onPolicyActiveStatusChange(updatedChanges); + }; + getColumns = (): Column[] => { return [ { @@ -218,6 +341,7 @@ export default class ChannelList extends React.PureComponent { ), field: 'name', fixed: true, + width: 7, }, { name: ( @@ -228,12 +352,60 @@ export default class ChannelList extends React.PureComponent { ), field: 'team', fixed: true, + width: 7, + }, + { + name: ( +
+ { + if (input) { + const {mixed} = this.getAllChannelsAutoAddStatus(); + input.indeterminate = mixed; + } + }} + onChange={this.handleBulkAutoAddToggle} + /> + + + + + + +
+ ), + field: 'autoAdd', + textAlign: 'center', + fixed: true, + width: 8, }, { name: '', field: 'remove', textAlign: 'right', fixed: true, + width: 3, }, ]; }; @@ -271,6 +443,8 @@ export default class ChannelList extends React.PureComponent { /> ); + const autoAddStatus = this.getChannelAutoAddStatus(channel.id); + return { cells: { id: channel.id, @@ -288,6 +462,31 @@ export default class ChannelList extends React.PureComponent { ), team: channel.team_display_name, + autoAdd: ( +
+ this.handleAutoAddToggle(channel.id, autoAddStatus)} + /> + + {autoAddStatus ? ( + + ) : ( + + )} + +
+ ), remove: ( { ); } } + +export default injectIntl(ChannelList); diff --git a/webapp/channels/src/components/admin_console/access_control/policy_details/index.ts b/webapp/channels/src/components/admin_console/access_control/policy_details/index.ts index 4ec9d9c546c..9271d7f9c83 100644 --- a/webapp/channels/src/components/admin_console/access_control/policy_details/index.ts +++ b/webapp/channels/src/components/admin_console/access_control/policy_details/index.ts @@ -5,7 +5,7 @@ import {connect} from 'react-redux'; import {bindActionCreators} from 'redux'; import type {Dispatch} from 'redux'; -import {getAccessControlPolicy as fetchPolicy, createAccessControlPolicy as createPolicy, deleteAccessControlPolicy as deletePolicy, searchAccessControlPolicyChannels as searchChannels, assignChannelsToAccessControlPolicy, unassignChannelsFromAccessControlPolicy, updateAccessControlPolicyActive} from 'mattermost-redux/actions/access_control'; +import {getAccessControlPolicy as fetchPolicy, createAccessControlPolicy as createPolicy, deleteAccessControlPolicy as deletePolicy, searchAccessControlPolicyChannels as searchChannels, assignChannelsToAccessControlPolicy, unassignChannelsFromAccessControlPolicy, updateAccessControlPoliciesActive} from 'mattermost-redux/actions/access_control'; import {createJob} from 'mattermost-redux/actions/jobs'; import {getAccessControlSettings, getAccessControlPolicy as getPolicy} from 'mattermost-redux/selectors/entities/access_control'; @@ -45,7 +45,7 @@ function mapDispatchToProps(dispatch: Dispatch) { unassignChannelsFromAccessControlPolicy, setNavigationBlocked, createJob, - updateAccessControlPolicyActive, + updateAccessControlPoliciesActive, }, dispatch), }; } diff --git a/webapp/channels/src/components/admin_console/access_control/policy_details/policy_details.test.tsx b/webapp/channels/src/components/admin_console/access_control/policy_details/policy_details.test.tsx index d6acc8b55de..c1afeab5fef 100644 --- a/webapp/channels/src/components/admin_console/access_control/policy_details/policy_details.test.tsx +++ b/webapp/channels/src/components/admin_console/access_control/policy_details/policy_details.test.tsx @@ -34,6 +34,7 @@ describe('components/admin_console/access_control/policy_details/PolicyDetails', const mockOnRemoveCallback = jest.fn(); const mockOnUndoRemoveCallback = jest.fn(); const mockOnAddCallback = jest.fn(); + const mockOnPoliciesActiveStatusChange = jest.fn(); const mockFetchPolicy = jest.fn(); const mockSetNavigationBlocked = jest.fn(); const mockAssignChannelsToAccessControlPolicy = jest.fn(); @@ -41,7 +42,7 @@ describe('components/admin_console/access_control/policy_details/PolicyDetails', const mockGetAccessControlExpressionAutocomplete = jest.fn(); const mockGetAccessControlFields = jest.fn(); const mockCreateJob = jest.fn(); - const mockUpdateAccessControlPolicyActive = jest.fn(); + const mockUpdateAccessControlPoliciesActive = jest.fn(); const mockGetVisualAST = jest.fn(); const defaultProps = { policyId: 'policy1', @@ -60,7 +61,9 @@ describe('components/admin_console/access_control/policy_details/PolicyDetails', onRemoveCallback: mockOnRemoveCallback, onUndoRemoveCallback: mockOnUndoRemoveCallback, onAddCallback: mockOnAddCallback, + onPolicyActiveStatusChange: mockOnPoliciesActiveStatusChange, channelsToRemove: {}, + policyActiveStatusChanges: [], channelsToAdd: {}, autocompleteResult: {entities: {}}, actions: { @@ -77,8 +80,8 @@ describe('components/admin_console/access_control/policy_details/PolicyDetails', getAccessControlExpressionAutocomplete: mockGetAccessControlExpressionAutocomplete, getAccessControlFields: mockGetAccessControlFields, createJob: mockCreateJob, - updateAccessControlPolicyActive: mockUpdateAccessControlPolicyActive, getVisualAST: mockGetVisualAST, + updateAccessControlPoliciesActive: mockUpdateAccessControlPoliciesActive, }, }; @@ -94,8 +97,8 @@ describe('components/admin_console/access_control/policy_details/PolicyDetails', getChannelMembers: jest.fn(), createJob: jest.fn(), createAccessControlSyncJob: jest.fn(), - updateAccessControlPolicyActive: jest.fn(), validateExpressionAgainstRequester: jest.fn(), + updateAccessControlPoliciesActive: mockUpdateAccessControlPoliciesActive, }); mockCreatePolicy.mockReset(); @@ -107,6 +110,7 @@ describe('components/admin_console/access_control/policy_details/PolicyDetails', mockOnRemoveCallback.mockReset(); mockOnUndoRemoveCallback.mockReset(); mockOnAddCallback.mockReset(); + mockOnPoliciesActiveStatusChange.mockReset(); mockFetchPolicy.mockReset(); mockSetNavigationBlocked.mockReset(); mockAssignChannelsToAccessControlPolicy.mockReset(); @@ -114,7 +118,7 @@ describe('components/admin_console/access_control/policy_details/PolicyDetails', mockGetAccessControlExpressionAutocomplete.mockReset(); mockGetAccessControlFields.mockReset(); mockCreateJob.mockReset(); - mockUpdateAccessControlPolicyActive.mockReset(); + mockUpdateAccessControlPoliciesActive.mockReset(); mockGetVisualAST.mockReset(); }); diff --git a/webapp/channels/src/components/admin_console/access_control/policy_details/policy_details.tsx b/webapp/channels/src/components/admin_console/access_control/policy_details/policy_details.tsx index 36ce3587fc8..e6767b8dc74 100644 --- a/webapp/channels/src/components/admin_console/access_control/policy_details/policy_details.tsx +++ b/webapp/channels/src/components/admin_console/access_control/policy_details/policy_details.tsx @@ -6,7 +6,7 @@ import React, {useState, useEffect} from 'react'; import {FormattedMessage, useIntl} from 'react-intl'; import {GenericModal} from '@mattermost/components'; -import type {AccessControlPolicy, AccessControlPolicyRule} from '@mattermost/types/access_control'; +import type {AccessControlPolicy, AccessControlPolicyActiveUpdate, AccessControlPolicyRule} 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'; @@ -15,7 +15,6 @@ import type {UserPropertyField} from '@mattermost/types/properties'; import type {ActionResult} from 'mattermost-redux/types/actions'; import BlockableLink from 'components/admin_console/blockable_link'; -import BooleanSetting from 'components/admin_console/boolean_setting'; import Card from 'components/card/card'; import TitleAndButtonCardHeader from 'components/card/title_and_button_card_header/title_and_button_card_header'; import ChannelSelectorModal from 'components/channel_selector_modal'; @@ -47,7 +46,7 @@ interface PolicyActions { assignChannelsToAccessControlPolicy: (policyId: string, channelIds: string[]) => Promise; unassignChannelsFromAccessControlPolicy: (policyId: string, channelIds: string[]) => Promise; createJob: (job: JobTypeBase & { data: any }) => Promise; - updateAccessControlPolicyActive: (policyId: string, active: boolean) => Promise; + updateAccessControlPoliciesActive: (states: AccessControlPolicyActiveUpdate[]) => Promise; } export interface PolicyDetailsProps { @@ -63,6 +62,11 @@ interface ChannelChanges { removedCount: number; } +interface PolicyActiveStatus { + id: string; + active: boolean; +} + function PolicyDetails({ policy, policyId, @@ -80,7 +84,9 @@ function PolicyDetails({ added: {}, removedCount: 0, }); + const [policyActiveStatusChanges, setPolicyActiveStatusChanges] = useState([]); const [saveNeeded, setSaveNeeded] = useState(false); + const [saving, setSaving] = useState(false); const [channelsCount, setChannelsCount] = useState(0); const [autocompleteResult, setAutocompleteResult] = useState([]); const [attributesLoaded, setAttributesLoaded] = useState(false); @@ -167,91 +173,98 @@ function PolicyDetails({ }; const handleSubmit = async (apply = false) => { - let success = true; - let currentPolicyId = policyId; - - // --- Step 1: Create/Update Policy --- - await actions.createPolicy({ - id: currentPolicyId || '', - name: policyName, - rules: [{expression, actions: ['*']}] as AccessControlPolicyRule[], - type: 'parent', - version: 'v0.2', - }).then((result) => { - if (result.error) { - setServerError(result.error.message); - setShowConfirmationModal(false); - success = false; - return; - } - currentPolicyId = result.data?.id; - setPolicyName(result.data?.name || ''); - setExpression(result.data?.rules?.[0]?.expression || ''); - setAutoSyncMembership(result.data?.active || false); - }); - - if (!currentPolicyId || !success) { - setShowConfirmationModal(false); - return; - } - - // --- Step 2: Update Policy Active --- + setSaving(true); try { - await actions.updateAccessControlPolicyActive(currentPolicyId, autoSyncMembership); - } catch (error) { - setServerError(formatMessage({ - id: 'admin.access_control.policy.edit_policy.error.update_active_status', - defaultMessage: 'Error updating policy active status: {error}', - }, {error: error.message})); + let success = true; + let currentPolicyId = policyId; + + // --- Step 1: Create/Update Policy --- + await actions.createPolicy({ + id: currentPolicyId || '', + name: policyName, + rules: [{expression, actions: ['*']}] as AccessControlPolicyRule[], + type: 'parent', + version: 'v0.2', + }).then((result) => { + if (result.error) { + setServerError(result.error.message); + setShowConfirmationModal(false); + success = false; + return; + } + currentPolicyId = result.data?.id; + setPolicyName(result.data?.name || ''); + setExpression(result.data?.rules?.[0]?.expression || ''); + setAutoSyncMembership(result.data?.active || false); + }); + + if (!currentPolicyId || !success) { + setShowConfirmationModal(false); + return; + } + + // --- Step 2: Assign Channels --- + if (success) { + try { + if (channelChanges.removedCount > 0) { + await actions.unassignChannelsFromAccessControlPolicy(currentPolicyId, Object.keys(channelChanges.removed)); + } + if (Object.keys(channelChanges.added).length > 0) { + await actions.assignChannelsToAccessControlPolicy(currentPolicyId, Object.keys(channelChanges.added)); + } + + setChannelChanges({removed: {}, added: {}, removedCount: 0}); + } catch (error) { + setServerError(formatMessage({ + id: 'admin.access_control.policy.edit_policy.error.assign_channels', + defaultMessage: 'Error assigning channels: {error}', + }, {error: error.message})); + setShowConfirmationModal(false); + success = false; + return; + } + } + + // --- Step 3: Handle Policy Active Status Changes --- + if (success && policyActiveStatusChanges.length > 0) { + try { + await actions.updateAccessControlPoliciesActive(policyActiveStatusChanges); + } catch (error) { + setServerError(formatMessage({ + id: 'admin.access_control.policy.edit_policy.error.update_active_status', + defaultMessage: 'Error updating policy active status: {error}', + }, {error: error.message})); + success = false; + return; + } + setPolicyActiveStatusChanges([]); + } + + // --- Step 4: Create Job if necessary --- + if (apply) { + try { + await abacActions.createAccessControlSyncJob({ + policy_id: currentPolicyId, + }); + } catch (error) { + setServerError(formatMessage({ + id: 'admin.access_control.policy.edit_policy.error.create_job', + defaultMessage: 'Error creating job: {error}', + }, {error: error.message})); + setShowConfirmationModal(false); + success = false; + return; + } + } + + // --- Step 5: Navigate lastly --- + setSaveNeeded(false); setShowConfirmationModal(false); - success = false; - return; + actions.setNavigationBlocked(false); + getHistory().push('/admin_console/system_attributes/attribute_based_access_control'); + } finally { + setSaving(false); } - - // --- Step 3: Assign Channels --- - if (success) { - try { - if (channelChanges.removedCount > 0) { - await actions.unassignChannelsFromAccessControlPolicy(currentPolicyId, Object.keys(channelChanges.removed)); - } - if (Object.keys(channelChanges.added).length > 0) { - await actions.assignChannelsToAccessControlPolicy(currentPolicyId, Object.keys(channelChanges.added)); - } - - setChannelChanges({removed: {}, added: {}, removedCount: 0}); - } catch (error) { - setServerError(formatMessage({ - id: 'admin.access_control.policy.edit_policy.error.assign_channels', - defaultMessage: 'Error assigning channels: {error}', - }, {error: error.message})); - setShowConfirmationModal(false); - success = false; - return; - } - } - - // --- Step 4: Create Job if necessary --- - if (apply) { - try { - await abacActions.createAccessControlSyncJob({ - policy_id: currentPolicyId, - }); - } catch (error) { - setServerError(formatMessage({ - id: 'admin.access_control.policy.edit_policy.error.create_job', - defaultMessage: 'Error creating job: {error}', - }, {error: error.message})); - setShowConfirmationModal(false); - success = false; - return; - } - } - - // --- Step 5: Navigate lastly --- - setSaveNeeded(false); - setShowConfirmationModal(false); - actions.setNavigationBlocked(false); - getHistory().push('/admin_console/system_attributes/attribute_based_access_control'); }; const handleDelete = async () => { @@ -323,6 +336,12 @@ function PolicyDetails({ actions.setNavigationBlocked(true); }; + const handlePolicyActiveStatusChange = (changes: PolicyActiveStatus[]) => { + setPolicyActiveStatusChanges(changes); + setSaveNeeded(true); + actions.setNavigationBlocked(true); + }; + const hasChannels = () => { // If there are channels on the server (minus any pending removals) or newly added channels return ( @@ -370,30 +389,6 @@ function PolicyDetails({ inputClassName='col-sm-8' autoFocus={policyId === undefined} /> - - - - } - value={autoSyncMembership} - onChange={(_, value) => { - setAutoSyncMembership(value); - setSaveNeeded(true); - actions.setNavigationBlocked(true); - }} - setByEnv={false} - helpText={ - - } - /> {noUsableAttributes && (
@@ -647,6 +645,7 @@ function PolicyDetails({
{ if (!preSaveCheck()) { return; diff --git a/webapp/channels/src/components/channel_settings_modal/__snapshots__/channel_settings_access_rules_tab.test.tsx.snap b/webapp/channels/src/components/channel_settings_modal/__snapshots__/channel_settings_access_rules_tab.test.tsx.snap index f4645289c62..824855c51da 100644 --- a/webapp/channels/src/components/channel_settings_modal/__snapshots__/channel_settings_access_rules_tab.test.tsx.snap +++ b/webapp/channels/src/components/channel_settings_modal/__snapshots__/channel_settings_access_rules_tab.test.tsx.snap @@ -50,7 +50,7 @@ exports[`components/channel_settings_modal/ChannelSettingsAccessRulesTab should

- Auto-add is disabled because no access rules are defined. Channel will use standard Mattermost access controls. + Access rules will prevent unauthorized users from joining, but will not automatically add qualifying members.

diff --git a/webapp/channels/src/components/channel_settings_modal/channel_settings_access_rules_activity_warning.test.tsx b/webapp/channels/src/components/channel_settings_modal/channel_settings_access_rules_activity_warning.test.tsx index 033268ea63d..841c1debb15 100644 --- a/webapp/channels/src/components/channel_settings_modal/channel_settings_access_rules_activity_warning.test.tsx +++ b/webapp/channels/src/components/channel_settings_modal/channel_settings_access_rules_activity_warning.test.tsx @@ -149,6 +149,7 @@ describe('ChannelSettingsAccessRulesTab - Activity Warning Integration', () => { updateAccessControlPolicyActive: jest.fn().mockResolvedValue({data: {}}), validateExpressionAgainstRequester: jest.fn().mockResolvedValue({data: {requester_matches: true}}), savePreferences: jest.fn().mockResolvedValue({data: {}}), + updateAccessControlPoliciesActive: jest.fn().mockResolvedValue({data: {}}), }; const defaultProps = { diff --git a/webapp/channels/src/components/channel_settings_modal/channel_settings_access_rules_tab.test.tsx b/webapp/channels/src/components/channel_settings_modal/channel_settings_access_rules_tab.test.tsx index 4fc25082b3e..98d7cc33562 100644 --- a/webapp/channels/src/components/channel_settings_modal/channel_settings_access_rules_tab.test.tsx +++ b/webapp/channels/src/components/channel_settings_modal/channel_settings_access_rules_tab.test.tsx @@ -39,7 +39,7 @@ describe('components/channel_settings_modal/ChannelSettingsAccessRulesTab', () = getChannelMembers: jest.fn(), createJob: jest.fn(), createAccessControlSyncJob: jest.fn(), - updateAccessControlPolicyActive: jest.fn(), + updateAccessControlPoliciesActive: jest.fn(), validateExpressionAgainstRequester: jest.fn(), }; @@ -487,7 +487,7 @@ describe('components/channel_settings_modal/ChannelSettingsAccessRulesTab', () = ); expect(screen.getByText('Auto-add members based on access rules')).toBeInTheDocument(); - expect(screen.getByText('Auto-add is disabled because no access rules are defined. Channel will use standard Mattermost access controls.')).toBeInTheDocument(); + expect(screen.getByText('Access rules will prevent unauthorized users from joining, but will not automatically add qualifying members.')).toBeInTheDocument(); }); test('should show system policy applied message when policies exist but not forcing auto-sync', () => { @@ -512,119 +512,7 @@ describe('components/channel_settings_modal/ChannelSettingsAccessRulesTab', () = ); expect(screen.getByText('Auto-add members based on access rules')).toBeInTheDocument(); - expect(screen.getByText('Auto-add is disabled because no channel-level access rules are defined. Channel access will still be restricted by the applied system policy in addition to standard Mattermost access controls.')).toBeInTheDocument(); - }); - - test('should show system policy forced message when policies force auto-sync', () => { - // Mock system policies that force auto-sync (active: true) - mockUseChannelSystemPolicies.mockReturnValue({ - policies: [ - { - id: 'policy1', - name: 'Test Policy', - type: 'parent', - active: true, - rules: [{expression: 'user.attributes.Department == "Engineering"'}], - }, - ], - loading: false, - error: null, - }); - - renderWithContext( - , - initialState, - ); - - expect(screen.getByText('Auto-add members based on access rules')).toBeInTheDocument(); - expect(screen.getByText('Auto-add is enabled by system policy. Users who match the configured attribute values will be automatically added as members and those who no longer match will be removed.')).toBeInTheDocument(); - }); - - test('should disable auto-sync toggle when system policies force it', () => { - // Mock system policies that force auto-sync - mockUseChannelSystemPolicies.mockReturnValue({ - policies: [ - { - id: 'policy1', - name: 'Test Policy', - type: 'parent', - active: true, - rules: [{expression: 'user.attributes.Department == "Engineering"'}], - }, - ], - loading: false, - error: null, - }); - - renderWithContext( - , - initialState, - ); - - const checkbox = screen.getByRole('checkbox'); - expect(checkbox).toBeChecked(); // Should be auto-enabled - expect(checkbox).toBeDisabled(); // Should be disabled (can't uncheck) - }); - - test('should show correct tooltip when system policy forces auto-sync', () => { - // Mock system policies that force auto-sync - mockUseChannelSystemPolicies.mockReturnValue({ - policies: [ - { - id: 'policy1', - name: 'Test Policy', - type: 'parent', - active: true, - rules: [{expression: 'user.attributes.Department == "Engineering"'}], - }, - ], - loading: false, - error: null, - }); - - renderWithContext( - , - initialState, - ); - - const label = document.querySelector('label[for="autoSyncMembersCheckbox"]'); - expect(label).toHaveAttribute('title', 'Auto-add is enabled by system policy and cannot be disabled'); - }); - - test('should handle mixed system policies (some active, some not)', () => { - // Mock mixed system policies - mockUseChannelSystemPolicies.mockReturnValue({ - policies: [ - { - id: 'policy1', - name: 'Active Policy', - type: 'parent', - active: true, - rules: [{expression: 'user.attributes.Department == "Engineering"'}], - }, - { - id: 'policy2', - name: 'Inactive Policy', - type: 'parent', - active: false, - rules: [{expression: 'user.attributes.Team == "Backend"'}], - }, - ], - loading: false, - error: null, - }); - - renderWithContext( - , - initialState, - ); - - const checkbox = screen.getByRole('checkbox'); - - // Should be forced enabled because ANY policy is active - expect(checkbox).toBeChecked(); - expect(checkbox).toBeDisabled(); - expect(screen.getByText('Auto-add is enabled by system policy. Users who match the configured attribute values will be automatically added as members and those who no longer match will be removed.')).toBeInTheDocument(); + expect(screen.getByText('Access rules will prevent unauthorized users from joining, but will not automatically add qualifying members.')).toBeInTheDocument(); }); test('should toggle auto-sync checkbox when clicked', async () => { @@ -708,39 +596,6 @@ describe('components/channel_settings_modal/ChannelSettingsAccessRulesTab', () = // Should be disabled in empty state expect(checkbox).not.toBeChecked(); expect(checkbox).toBeDisabled(); - - // Should show empty state message - expect(screen.getByText('Auto-add is disabled because no access rules are defined. Channel will use standard Mattermost access controls.')).toBeInTheDocument(); - - // Should have empty state tooltip - const label = document.querySelector('label[for="autoSyncMembersCheckbox"]'); - expect(label).toHaveAttribute('title', 'Auto-add is disabled because no access rules are defined'); - }); - - test('should differentiate between empty state and system policies applied', () => { - // Mock inactive system policies (applied but not forcing) - mockUseChannelSystemPolicies.mockReturnValue({ - policies: [ - { - id: 'policy1', - name: 'Test Policy', - type: 'parent', - active: false, - rules: [{expression: 'user.attributes.Department == "Engineering"'}], - }, - ], - loading: false, - error: null, - }); - - renderWithContext( - , - initialState, - ); - - // Should show system policy applied message, not empty state message - expect(screen.queryByText('Auto-add is disabled because no access rules are defined. Channel will use standard Mattermost access controls.')).not.toBeInTheDocument(); - expect(screen.getByText('Auto-add is disabled because no channel-level access rules are defined. Channel access will still be restricted by the applied system policy in addition to standard Mattermost access controls.')).toBeInTheDocument(); }); test('should handle system policy loading state', () => { @@ -756,25 +611,11 @@ describe('components/channel_settings_modal/ChannelSettingsAccessRulesTab', () = initialState, ); + const checkbox = screen.getByRole('checkbox'); + // Should still render component without crashing expect(screen.getByText('Auto-add members based on access rules')).toBeInTheDocument(); - }); - - test('should handle system policy error state', () => { - // Mock system policy error - mockUseChannelSystemPolicies.mockReturnValue({ - policies: [], - loading: false, - error: 'Failed to load policies', - }); - - renderWithContext( - , - initialState, - ); - - // Should still render component and treat as empty state - expect(screen.getByText('Auto-add is disabled because no access rules are defined. Channel will use standard Mattermost access controls.')).toBeInTheDocument(); + expect(checkbox).not.toBeChecked(); }); test('should auto-disable sync when entering empty state', async () => { @@ -815,7 +656,9 @@ describe('components/channel_settings_modal/ChannelSettingsAccessRulesTab', () = // Now enable auto-sync const checkbox = screen.getByRole('checkbox'); await userEvent.click(checkbox); - expect(checkbox).toBeChecked(); + await waitFor(() => { + expect(checkbox).toBeChecked(); + }); // Now simulate removing all policies and channel rules (empty state) mockUseChannelSystemPolicies.mockReturnValue({ @@ -828,11 +671,98 @@ describe('components/channel_settings_modal/ChannelSettingsAccessRulesTab', () = onChangeCallback(''); await waitFor(() => { - // Auto-sync should be auto-disabled in empty state + // Auto-sync should be auto-disabled and unchecked in empty state expect(checkbox).not.toBeChecked(); expect(checkbox).toBeDisabled(); }); }); + + test('should auto-disable sync when loading with empty state and autoSyncMembers is true', async () => { + // Mock loading a policy with autoSyncMembers=true but no rules + mockActions.getChannelPolicy.mockResolvedValue({ + data: { + id: 'channel_id', + name: 'Test Channel', + type: 'channel', + active: true, // Server has auto-sync enabled + rules: [], // But no rules + }, + }); + + // No system policies + mockUseChannelSystemPolicies.mockReturnValue({ + policies: [], + loading: false, + error: null, + }); + + renderWithContext( + , + initialState, + ); + + // Wait for component to load + await waitFor(() => { + expect(screen.getByTestId('table-editor')).toBeInTheDocument(); + }); + + // Auto-sync should be automatically set to false even though server had it as true + await waitFor(() => { + const checkbox = screen.getByRole('checkbox'); + expect(checkbox).not.toBeChecked(); + expect(checkbox).toBeDisabled(); + }); + }); + + test('should not auto-disable sync when system policies exist even without channel rules', async () => { + // Mock system policies exist (inactive) + mockUseChannelSystemPolicies.mockReturnValue({ + policies: [ + { + id: 'policy1', + name: 'Test Policy', + type: 'parent', + active: false, + rules: [{expression: 'user.attributes.Department == "Engineering"'}], + }, + ], + loading: false, + error: null, + }); + + // Mock loading a policy with autoSyncMembers=true but no channel rules + mockActions.getChannelPolicy.mockResolvedValue({ + data: { + id: 'channel_id', + name: 'Test Channel', + type: 'channel', + active: true, // Server has auto-sync enabled + rules: [], // But no channel rules + }, + }); + + renderWithContext( + , + initialState, + ); + + // Wait for component to load + await waitFor(() => { + expect(screen.getByTestId('table-editor')).toBeInTheDocument(); + }); + + // Auto-sync should remain true because system policies exist (not empty state) + // The useEffect won't trigger because isEmptyRulesState is false + await waitFor(() => { + const checkbox = screen.getByRole('checkbox'); + + // Since system policies exist, isEmptyRulesState is false + // So the auto-disable useEffect won't trigger and autoSyncMembers should remain true + // Checkbox should be enabled (not disabled) because isEmptyRulesState is false + expect(checkbox).not.toBeDisabled(); + expect(checkbox).toBeChecked(); // Should remain checked because autoSyncMembers is true + }); + }); }); describe('SaveChangesPanel integration', () => { @@ -1515,9 +1445,10 @@ describe('components/channel_settings_modal/ChannelSettingsAccessRulesTab', () = }); // Don't set any channel expression, just enable auto-sync + const checkbox = screen.getByRole('checkbox'); + await userEvent.click(checkbox); await waitFor(() => { - const checkbox = screen.getByRole('checkbox'); - expect(checkbox).toBeDisabled(); // Should be disabled without expression + expect(checkbox).toBeChecked(); }); // System policies exist but no channel expression, so should use system expressions only @@ -1574,6 +1505,13 @@ describe('components/channel_settings_modal/ChannelSettingsAccessRulesTab', () = }); test('should handle empty or whitespace-only expressions', async () => { + // Override the beforeEach to have no system policies for this test + mockUseChannelSystemPolicies.mockReturnValue({ + policies: [], + loading: false, + error: null, + }); + mockActions.searchUsers.mockResolvedValue({data: {users: []}}); renderWithContext( @@ -1589,7 +1527,7 @@ describe('components/channel_settings_modal/ChannelSettingsAccessRulesTab', () = const onChangeCallback = MockedTableEditor.mock.calls[0][0].onChange; onChangeCallback(' '); // Just whitespace - // Checkbox should be disabled for empty expression + // Checkbox should be disabled for empty expression (no system policies + no rules = empty state) await waitFor(() => { const checkbox = screen.getByRole('checkbox'); expect(checkbox).toBeDisabled(); diff --git a/webapp/channels/src/components/channel_settings_modal/channel_settings_access_rules_tab.tsx b/webapp/channels/src/components/channel_settings_modal/channel_settings_access_rules_tab.tsx index a2e97ef3855..c84009fd8e7 100644 --- a/webapp/channels/src/components/channel_settings_modal/channel_settings_access_rules_tab.tsx +++ b/webapp/channels/src/components/channel_settings_modal/channel_settings_access_rules_tab.tsx @@ -65,7 +65,6 @@ function ChannelSettingsAccessRulesTab({ // Auto-sync members toggle state const [autoSyncMembers, setAutoSyncMembers] = useState(false); const [originalAutoSyncMembers, setOriginalAutoSyncMembers] = useState(false); - const [systemPolicyForcesAutoSync, setSystemPolicyForcesAutoSync] = useState(false); // SaveChangesPanel state const [saveChangesPanelState, setSaveChangesPanelState] = useState(); @@ -90,17 +89,6 @@ function ChannelSettingsAccessRulesTab({ // Fetch system policies applied to this channel const {policies: systemPolicies, loading: policiesLoading} = useChannelSystemPolicies(channel); - // Check if system policies force auto-sync to be enabled - useEffect(() => { - if (systemPolicies && systemPolicies.length > 0) { - // System policies force auto-sync when they are active - const hasActivePolicies = systemPolicies.some((policy) => policy.active === true); - setSystemPolicyForcesAutoSync(hasActivePolicies); - } else { - setSystemPolicyForcesAutoSync(false); - } - }, [systemPolicies]); - // Load user attributes on component mount useEffect(() => { const loadAttributes = async () => { @@ -135,12 +123,7 @@ function ChannelSettingsAccessRulesTab({ if (result.data) { // Extract expression from the policy rules const existingExpression = result.data.rules?.[0]?.expression || ''; - let existingAutoSync = result.data.active || false; - - // If system policies force auto-sync, override the channel setting - if (systemPolicyForcesAutoSync) { - existingAutoSync = true; - } + const existingAutoSync = result.data.active || false; setExpression(existingExpression); setOriginalExpression(existingExpression); @@ -151,16 +134,11 @@ function ChannelSettingsAccessRulesTab({ // If no policy exists (404), that's fine - use defaults setExpression(''); setOriginalExpression(''); - - // If system policies force auto-sync, enable it even without a channel policy - const defaultAutoSync = systemPolicyForcesAutoSync; - setAutoSyncMembers(defaultAutoSync); - setOriginalAutoSyncMembers(defaultAutoSync); } }; loadChannelPolicy(); - }, [channel.id, actions, systemPolicyForcesAutoSync]); + }, [channel.id, actions]); // Update parent component when changes occur useEffect(() => { @@ -198,24 +176,21 @@ function ChannelSettingsAccessRulesTab({ const hasChannelRules = expression && expression.trim().length > 0; const hasSystemPolicies = systemPolicies && systemPolicies.length > 0; - // Edge case: No channel rules AND no system policies at all (applied or not) - return !hasChannelRules && !hasSystemPolicies; + // Return true if there are no channel rules or system policies + return !(hasChannelRules || hasSystemPolicies); }, [expression, systemPolicies]); - // Auto-sync members toggle logic + // Auto-disable auto-sync when there are no system policies and no rules useEffect(() => { - // Priority 1: System policy forcing (highest priority) - // When system policies require auto-sync, it must be enabled - if (systemPolicyForcesAutoSync && !autoSyncMembers) { - setAutoSyncMembers(true); - setOriginalAutoSyncMembers(true); // Update original to prevent "unsaved changes" detection - } else if (isEmptyRulesState && autoSyncMembers && !systemPolicyForcesAutoSync) { - // Priority 2: Auto-disable when entering empty state (if not forced by system) - // When no rules exist and system doesn't force, auto-sync must be disabled - setAutoSyncMembers(false); - setOriginalAutoSyncMembers(false); // Update original to prevent "unsaved changes" detection + if (policiesLoading) { + return; } - }, [systemPolicyForcesAutoSync, isEmptyRulesState, autoSyncMembers]); + + // Only auto-disable if there are no system policies and no rules (empty state) + if (isEmptyRulesState && autoSyncMembers) { + setAutoSyncMembers(false); + } + }, [isEmptyRulesState, autoSyncMembers]); const handleAutoSyncToggle = useCallback(() => { // Don't allow toggling if in empty rules state @@ -223,18 +198,8 @@ function ChannelSettingsAccessRulesTab({ return; } - // Don't allow toggling if no expression - if (!expression.trim()) { - return; - } - - // Don't allow disabling if system policies force auto-sync - if (systemPolicyForcesAutoSync && autoSyncMembers) { - return; - } - setAutoSyncMembers((prev) => !prev); - }, [expression, isEmptyRulesState, systemPolicyForcesAutoSync, autoSyncMembers]); + }, [isEmptyRulesState]); // Helper function to combine system policy expressions with channel expression const combineSystemAndChannelExpressions = useCallback((channelExpression: string): string => { @@ -454,12 +419,11 @@ function ChannelSettingsAccessRulesTab({ // Step 2: Update the active status separately (like System Console does) try { - await actions.updateAccessControlPolicyActive(channel.id, autoSyncMembers); + await actions.updateAccessControlPoliciesActive([{id: channel.id, active: autoSyncMembers}]); } catch (activeError) { + // Don't fail the entire save operation for this, but log it // eslint-disable-next-line no-console console.error('Failed to update policy active status:', activeError); - - // Don't fail the entire save operation for this, but log it } // Step 3: Create a job to immediately sync channel membership when rules exist @@ -525,7 +489,7 @@ function ChannelSettingsAccessRulesTab({ } // Validate expression if auto-sync is enabled - if (autoSyncMembers && !expression.trim()) { + if (autoSyncMembers && isEmptyRulesState) { setFormError(formatMessage({ id: 'channel_settings.access_rules.expression_required_for_autosync', defaultMessage: 'Access rules are required when auto-add members is enabled', @@ -757,7 +721,7 @@ function ChannelSettingsAccessRulesTab({ className='ChannelSettingsModal__autoSyncCheckbox' checked={autoSyncMembers} onChange={handleAutoSyncToggle} - disabled={isEmptyRulesState || !expression.trim() || (systemPolicyForcesAutoSync && autoSyncMembers)} + disabled={isEmptyRulesState} id='autoSyncMembersCheckbox' name='autoSyncMembers' /> @@ -772,14 +736,6 @@ function ChannelSettingsAccessRulesTab({ }); } - // Show "forced by parent" when system policies force auto-sync (regardless of channel rules) - if (systemPolicyForcesAutoSync) { - return formatMessage({ - id: 'channel_settings.access_rules.auto_sync_forced_by_parent', - defaultMessage: 'Auto-add is enabled by system policy and cannot be disabled', - }); - } - if (!expression.trim()) { return formatMessage({ id: 'channel_settings.access_rules.auto_sync_requires_expression', @@ -789,7 +745,7 @@ function ChannelSettingsAccessRulesTab({ return undefined; })()} > - + {formatMessage({ id: 'channel_settings.access_rules.auto_sync', defaultMessage: 'Auto-add members based on access rules', @@ -799,40 +755,6 @@ function ChannelSettingsAccessRulesTab({

{(() => { - // Check for empty state first (no channel rules AND no system policies) - if (isEmptyRulesState) { - return formatMessage({ - id: 'channel_settings.access_rules.auto_sync_empty_state_description', - defaultMessage: 'Auto-add is disabled because no access rules are defined. Channel will use standard Mattermost access controls.', - }); - } - - // Show system policy forced description when policies force auto-sync - if (systemPolicyForcesAutoSync) { - return formatMessage({ - id: 'channel_settings.access_rules.auto_sync_forced_description', - defaultMessage: 'Auto-add is enabled by system policy. Users who match the configured attribute values will be automatically added as members and those who no longer match will be removed.', - }); - } - - // If there are no channel rules (and no system policies forcing) - if (!expression.trim()) { - // Check if system policies are applied (but not forcing auto-sync) - if (systemPolicies && systemPolicies.length > 0) { - return formatMessage({ - id: 'channel_settings.access_rules.auto_sync_system_policy_applied_description', - defaultMessage: 'Auto-add is disabled because no channel-level access rules are defined. Channel access will still be restricted by the applied system policy in addition to standard Mattermost access controls.', - }); - } - - // True empty state - no system policies at all - return formatMessage({ - id: 'channel_settings.access_rules.auto_sync_no_rules_description', - defaultMessage: 'Define access rules above to enable automatic member synchronization.', - }); - } - - // There are channel rules - show normal behavior if (autoSyncMembers) { return formatMessage({ id: 'channel_settings.access_rules.auto_sync_enabled_description', diff --git a/webapp/channels/src/hooks/useChannelAccessControlActions.ts b/webapp/channels/src/hooks/useChannelAccessControlActions.ts index 61f8d2d96f9..ce739022603 100644 --- a/webapp/channels/src/hooks/useChannelAccessControlActions.ts +++ b/webapp/channels/src/hooks/useChannelAccessControlActions.ts @@ -4,7 +4,7 @@ import {useMemo} from 'react'; import {useDispatch} from 'react-redux'; -import type {AccessControlVisualAST, AccessControlTestResult, AccessControlPolicy} from '@mattermost/types/access_control'; +import type {AccessControlVisualAST, AccessControlTestResult, AccessControlPolicy, AccessControlPolicyActiveUpdate} from '@mattermost/types/access_control'; import type {ChannelMembership} from '@mattermost/types/channels'; import type {JobTypeBase} from '@mattermost/types/jobs'; import type {UserPropertyField} from '@mattermost/types/properties'; @@ -15,10 +15,10 @@ import { searchUsersForExpression, getAccessControlPolicy, createAccessControlPolicy, - updateAccessControlPolicyActive, deleteAccessControlPolicy, validateExpressionAgainstRequester, createAccessControlSyncJob, + updateAccessControlPoliciesActive, } from 'mattermost-redux/actions/access_control'; import {getChannelMembers} from 'mattermost-redux/actions/channels'; import {createJob} from 'mattermost-redux/actions/jobs'; @@ -33,7 +33,7 @@ export interface ChannelAccessControlActions { deleteChannelPolicy: (policyId: string) => Promise; getChannelMembers: (channelId: string, page?: number, perPage?: number) => Promise>; createJob: (job: JobTypeBase & { data: any }) => Promise; - updateAccessControlPolicyActive: (policyId: string, active: boolean) => Promise; + updateAccessControlPoliciesActive: (statuses: AccessControlPolicyActiveUpdate[]) => Promise; validateExpressionAgainstRequester: (expression: string) => Promise>; createAccessControlSyncJob: (jobData: {policy_id: string}) => Promise; } @@ -113,13 +113,6 @@ export const useChannelAccessControlActions = (channelId?: string): ChannelAcces return dispatch(createJob(job)); }, - /** - * Update the active status of an access control policy - */ - updateAccessControlPolicyActive: (policyId: string, active: boolean) => { - return dispatch(updateAccessControlPolicyActive(policyId, active)); - }, - /** * Validate if the current user (requester) matches an access control expression */ @@ -133,6 +126,13 @@ export const useChannelAccessControlActions = (channelId?: string): ChannelAcces createAccessControlSyncJob: (jobData: {policy_id: string}) => { return dispatch(createAccessControlSyncJob(jobData)); }, + + /** + * Update the active statuses of access control policies + */ + updateAccessControlPoliciesActive: (statuses: AccessControlPolicyActiveUpdate[]) => { + return dispatch(updateAccessControlPoliciesActive(statuses)); + }, }), [dispatch, channelId]); }; diff --git a/webapp/channels/src/i18n/en.json b/webapp/channels/src/i18n/en.json index e48c082ec87..09953391a2b 100644 --- a/webapp/channels/src/i18n/en.json +++ b/webapp/channels/src/i18n/en.json @@ -283,11 +283,14 @@ "admin.access_control.policies.resources.channels": "{count, number} {count, plural, one {channel} other {channels}}", "admin.access_control.policies.resources.none": "None", "admin.access_control.policies.title": "Access Control Policies", + "admin.access_control.policy.channel_list.autoAddHeader": "Auto-add members", + "admin.access_control.policy.channel_list.autoAddTooltip.line1": "Toggle to auto-add members who meet all access requirements", + "admin.access_control.policy.channel_list.autoAddTooltip.line2": "Channel administrators can modify this setting", + "admin.access_control.policy.channel_list.off": "Off", + "admin.access_control.policy.channel_list.on": "On", "admin.access_control.policy.channels_affected": "Are you sure you want to save and apply the access control policy?", "admin.access_control.policy.edit_policy.access_rules.subtitle": "Select user attributes and values as rules to restrict channel membership.", "admin.access_control.policy.edit_policy.access_rules.title": "Attribute-based access rules", - "admin.access_control.policy.edit_policy.autoSyncMembership": "Auto-add members based on access rules:", - "admin.access_control.policy.edit_policy.autoSyncMembership.description": "Users who match the attribute values configured below will be automatically added as new members. Regardless of this setting, users who later no longer match the configured attribute values will be removed from the channel after the next sync.", "admin.access_control.policy.edit_policy.channel_selector.addChannels": "Add channels", "admin.access_control.policy.edit_policy.channel_selector.remove": "Remove", "admin.access_control.policy.edit_policy.channel_selector.subtitle": "Add channels that this attribute-based access policy will apply to.", @@ -3775,13 +3778,8 @@ "channel_settings.access_rules.auto_sync": "Auto-add members based on access rules", "channel_settings.access_rules.auto_sync_disabled_description": "Access rules will prevent unauthorized users from joining, but will not automatically add qualifying members.", "channel_settings.access_rules.auto_sync_disabled_empty_state": "Auto-add is disabled because no access rules are defined", - "channel_settings.access_rules.auto_sync_empty_state_description": "Auto-add is disabled because no access rules are defined. Channel will use standard Mattermost access controls.", "channel_settings.access_rules.auto_sync_enabled_description": "Users who match the configured attribute values will be automatically added as members and those who no longer match will be removed.", - "channel_settings.access_rules.auto_sync_forced_by_parent": "Auto-add is enabled by system policy and cannot be disabled", - "channel_settings.access_rules.auto_sync_forced_description": "Auto-add is enabled by system policy. Users who match the configured attribute values will be automatically added as members and those who no longer match will be removed.", - "channel_settings.access_rules.auto_sync_no_rules_description": "Define access rules above to enable automatic member synchronization.", "channel_settings.access_rules.auto_sync_requires_expression": "Define access rules to enable auto-add members", - "channel_settings.access_rules.auto_sync_system_policy_applied_description": "Auto-add is disabled because no channel-level access rules are defined. Channel access will still be restricted by the applied system policy in addition to standard Mattermost access controls.", "channel_settings.access_rules.confirm_modal.allowed_tab": "Allowed ({count})", "channel_settings.access_rules.confirm_modal.cancel": "Cancel", "channel_settings.access_rules.confirm_modal.continue": "Continue", diff --git a/webapp/channels/src/packages/mattermost-redux/src/actions/access_control.ts b/webapp/channels/src/packages/mattermost-redux/src/actions/access_control.ts index 1821a67fb42..89d3d482044 100644 --- a/webapp/channels/src/packages/mattermost-redux/src/actions/access_control.ts +++ b/webapp/channels/src/packages/mattermost-redux/src/actions/access_control.ts @@ -3,7 +3,7 @@ import {batchActions} from 'redux-batched-actions'; -import type {AccessControlPoliciesResult, AccessControlPolicy, AccessControlTestResult} from '@mattermost/types/access_control'; +import type {AccessControlPoliciesResult, AccessControlPolicy, AccessControlPolicyActiveUpdate, AccessControlTestResult} from '@mattermost/types/access_control'; import type {ChannelSearchOpts, ChannelsWithTotalCount} from '@mattermost/types/channels'; import type {ServerError} from '@mattermost/types/errors'; @@ -185,3 +185,10 @@ export function createAccessControlSyncJob(jobData: {policy_id: string}): Action return {data}; }; } + +export function updateAccessControlPoliciesActive(states: AccessControlPolicyActiveUpdate[]) { + return bindClientFunc({ + clientFunc: Client4.updateAccessControlPoliciesActive, + params: [states], + }); +} diff --git a/webapp/platform/client/src/client4.ts b/webapp/platform/client/src/client4.ts index 9c7178733dc..92912a20662 100644 --- a/webapp/platform/client/src/client4.ts +++ b/webapp/platform/client/src/client4.ts @@ -3,7 +3,7 @@ /* eslint-disable max-lines */ -import type {AccessControlPolicy, CELExpressionError, AccessControlTestResult, AccessControlPoliciesResult, AccessControlPolicyChannelsResult, AccessControlVisualAST, AccessControlAttributes} from '@mattermost/types/access_control'; +import type {AccessControlPolicy, CELExpressionError, AccessControlTestResult, AccessControlPoliciesResult, AccessControlPolicyChannelsResult, AccessControlVisualAST, AccessControlAttributes, AccessControlPolicyActiveUpdate} from '@mattermost/types/access_control'; import type {ClusterInfo, AnalyticsRow, SchemaMigration, LogFilterQuery} from '@mattermost/types/admin'; import type {Agent} from '@mattermost/types/agents'; import type {AppBinding, AppCallRequest, AppCallResponse} from '@mattermost/types/apps'; @@ -4685,6 +4685,13 @@ export default class Client4 { ); }; + updateAccessControlPoliciesActive = (states: AccessControlPolicyActiveUpdate[]) => { + return this.doFetch( + `${this.getBaseRoute()}/access_control_policies/activate`, + {method: 'put', body: JSON.stringify({entries: states})}, + ); + }; + getTeamContentFlaggingStatus = (teamId: string) => { return this.doFetch<{enabled: boolean}>( `${this.getContentFlaggingRoute()}/team/${teamId}/status`, diff --git a/webapp/platform/types/src/access_control.ts b/webapp/platform/types/src/access_control.ts index 3d0b0e8e209..bb159bfaba0 100644 --- a/webapp/platform/types/src/access_control.ts +++ b/webapp/platform/types/src/access_control.ts @@ -85,3 +85,8 @@ export interface AccessControlled { */ access_control_enforced?: boolean; } + +export type AccessControlPolicyActiveUpdate = { + id: string; + active: boolean; +} diff --git a/webapp/platform/types/src/channels.ts b/webapp/platform/types/src/channels.ts index 835680063be..e016a715bff 100644 --- a/webapp/platform/types/src/channels.ts +++ b/webapp/platform/types/src/channels.ts @@ -69,6 +69,7 @@ export type Channel = { policy_id?: string | null; banner_info?: ChannelBanner; policy_enforced?: boolean; + policy_is_active?: boolean; default_category_name?: string; };