upstream/mattermost
1
0
Fork 0
mirror of https://github.com/mattermost/mattermost.git synced 2026-02-03 20:40:00 -05:00
Code Issues Projects Releases Packages Wiki Activity Actions 11

[MM-67126] Deprecate UpdateAccessControlPolicyActiveStatus API in favor of new one (#34940)

Browse source
This commit is contained in:
Ibrahim Serdar Acikgoz 2026-01-27 15:49:08 +01:00 committed by GitHub
parent 89a29ce3c2
commit ced9a56e39
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 98 additions and 45 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
api/v4/source/access_control.yaml
View file

@ -283,11 +283,16 @@
$ref: "#/components/responses/InternalServerError"
"/api/v4/access_control_policies/{policy_id}/activate":
get:
deprecated: true
tags:
- access control
summary: Activate or deactivate an access control policy
description: |
Updates the active status of an access control policy.
**Deprecated:** This endpoint will be removed in a future release. Use the dedicated access control policy update endpoint instead.
Link: </api/v4/access_control_policies/activate>; rel="successor-version"
##### Permissions
Must have the `manage_system` permission.
operationId: UpdateAccessControlPolicyActiveStatus

26
server/channels/api4/access_control.go
View file

@ -382,12 +382,22 @@ func searchAccessControlPolicies(c *Context, w http.ResponseWriter, r *http.Requ
}
}
// updateActiveStatus updates the active status of a single access control policy.
//
// Deprecated: This endpoint is deprecated and will be removed in a future release.
// Use PUT /api/v4/access_control/policies/activate instead, which supports batch updates.
func updateActiveStatus(c *Context, w http.ResponseWriter, r *http.Request) {
c.RequirePolicyId()
if c.Err != nil {
return
}
// CSRF barrier: only allow header-based auth (reject cookie-only sessions)
if r.Header.Get(model.HeaderAuth) == "" {
c.SetInvalidParam("Authorization")
return
}
policyID := c.Params.PolicyId
// Check if user has system admin permission OR channel-specific permission for this policy
@ -416,7 +426,11 @@ func updateActiveStatus(c *Context, w http.ResponseWriter, r *http.Request) {
}
model.AddEventParameterToAuditRec(auditRec, "active", activeBool)
appErr := c.App.UpdateAccessControlPolicyActive(c.AppContext, policyID, activeBool)
// Wrap single update in slice to use the batch update method
updates := []model.AccessControlPolicyActiveUpdate{
{ID: policyID, Active: activeBool},
}
_, appErr := c.App.UpdateAccessControlPoliciesActive(c.AppContext, updates)
if appErr != nil {
c.Err = appErr
return
@ -429,6 +443,9 @@ func updateActiveStatus(c *Context, w http.ResponseWriter, r *http.Request) {
"status": "OK",
}
// Set deprecation header to inform clients
w.Header().Set("Deprecation", "true")
w.Header().Set("Link", "</api/v4/access_control/policies/activate>; rel=\"successor-version\"")
w.Header().Set("Content-Type", "application/json")
if err := json.NewEncoder(w).Encode(response); err != nil {
c.Logger.Warn("Error while writing response", mlog.Err(err))
@ -446,12 +463,13 @@ func setActiveStatus(c *Context, w http.ResponseWriter, r *http.Request) {
defer c.LogAuditRec(auditRec)
model.AddEventParameterAuditableToAuditRec(auditRec, "requested", &list)
// Check if user has system admin permission OR channel-specific permission for this policy
// Check if user has system admin permission OR policy-specific permission
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 {
// Validate policy access permission - this fetches the policy first to verify it exists
// and is a channel-type policy before checking channel permissions
if appErr := c.App.ValidateAccessControlPolicyPermission(c.AppContext, c.AppContext.Session().UserId, entry.ID); appErr != nil {
c.SetPermissionError(model.PermissionManageChannelAccessRules)
return
}

1
server/channels/api4/access_control_local.go
View file

@ -21,7 +21,6 @@ func (api *API) InitAccessControlPolicyLocal() {
api.BaseRoutes.AccessControlPolicy.Handle("", api.APILocal(getAccessControlPolicy)).Methods(http.MethodGet)
api.BaseRoutes.AccessControlPolicy.Handle("", api.APILocal(deleteAccessControlPolicy)).Methods(http.MethodDelete)
api.BaseRoutes.AccessControlPolicy.Handle("/activate", api.APILocal(updateActiveStatus)).Methods(http.MethodGet)
api.BaseRoutes.AccessControlPolicy.Handle("/assign", api.APILocal(assignAccessPolicy)).Methods(http.MethodPost)
api.BaseRoutes.AccessControlPolicy.Handle("/unassign", api.APILocal(unassignAccessPolicy)).Methods(http.MethodDelete)
api.BaseRoutes.AccessControlPolicy.Handle("/resources/channels", api.APILocal(getChannelsForAccessControlPolicy)).Methods(http.MethodGet)

62
server/channels/api4/access_control_test.go
View file

@ -963,6 +963,7 @@ func TestSetActiveStatus(t *testing.T) {
}
mockAccessControlService := &mocks.AccessControlServiceInterface{}
mockAccessControlService.On("GetPolicy", mock.AnythingOfType("*request.Context"), privateChannel.Id).Return(channelPolicy, nil)
th.App.Srv().Channels().AccessControl = mockAccessControlService
// Channel admin should be able to set active status for their channel
@ -974,4 +975,65 @@ func TestSetActiveStatus(t *testing.T) {
require.Equal(t, channelPolicy.ID, policies[0].ID, "expected policy ID to match")
require.True(t, policies[0].Active, "expected policy to be active")
})
t.Run("SetActiveStatus with channel admin for another channel should fail", func(t *testing.T) {
// This test verifies the security fix: a channel admin cannot modify the active status
// of a policy for a channel they don't have permissions on, even if they attempt to
// use a policy ID that matches a channel they control.
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 two private channels
channelA := th.CreatePrivateChannel(t)
channelB := th.CreatePrivateChannel(t)
// Create a channel admin who only has access to channel A
channelAdmin := th.CreateUser(t)
th.LinkUserToTeam(t, channelAdmin, th.BasicTeam)
th.AddUserToChannel(t, channelAdmin, channelA)
th.MakeUserChannelAdmin(t, channelAdmin, channelA)
// Create a policy for channel B (which the channel admin does NOT have access to)
channelBPolicy := &model.AccessControlPolicy{
ID: channelB.Id,
Type: model.AccessControlPolicyTypeChannel,
Version: model.AccessControlPolicyVersionV0_2,
Revision: 1,
Rules: []model.AccessControlPolicyRule{
{
Expression: "user.attributes.team == 'engineering'",
Actions: []string{"*"},
},
},
}
_, err := th.App.Srv().Store().AccessControlPolicy().Save(th.Context, channelBPolicy)
require.NoError(t, err)
channelAdminClient := th.CreateClient()
_, _, err = channelAdminClient.Login(context.Background(), channelAdmin.Email, channelAdmin.Password)
require.NoError(t, err)
mockAccessControlService := &mocks.AccessControlServiceInterface{}
th.App.Srv().Channels().AccessControl = mockAccessControlService
mockAccessControlService.On("GetPolicy", mock.AnythingOfType("*request.Context"), channelB.Id).Return(channelBPolicy, nil)
// Attempt to update the policy for channel B (which the admin doesn't have access to)
maliciousUpdateReq := model.AccessControlPolicyActiveUpdateRequest{
Entries: []model.AccessControlPolicyActiveUpdate{
{ID: channelB.Id, Active: true},
},
}
// Channel admin should NOT be able to set active status for another channel's policy
_, resp, err := channelAdminClient.SetAccessControlPolicyActive(context.Background(), maliciousUpdateReq)
require.Error(t, err)
CheckForbiddenStatus(t, resp)
})
}

9
server/channels/app/access_control.go
View file

@ -302,15 +302,6 @@ func (a *App) GetAccessControlFieldsAutocomplete(rctx request.CTX, after string,
return fields, nil
}
func (a *App) UpdateAccessControlPolicyActive(rctx request.CTX, policyID string, active bool) *model.AppError {
_, err := a.Srv().Store().AccessControlPolicy().SetActiveStatus(rctx, policyID, active)
if err != nil {
return model.NewAppError("UpdateAccessControlPolicyActive", "app.pap.update_access_control_policy_active.app_error", nil, err.Error(), http.StatusInternalServerError)
}
return nil
}
func (a *App) UpdateAccessControlPoliciesActive(rctx request.CTX, updates []model.AccessControlPolicyActiveUpdate) ([]*model.AccessControlPolicy, *model.AppError) {
acs := a.Srv().ch.AccessControl
if acs == nil {

4
server/i18n/en.json
View file

@ -6918,10 +6918,6 @@
"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."
},
{
"id": "app.pdp.access_evaluation.app_error",
"translation": "Failed evaluate access control policy."

6
webapp/channels/src/components/admin_console/team_channel_settings/channel/details/channel_details.test.tsx
View file

@ -105,7 +105,7 @@ describe('admin_console/team_channel_settings/channel/ChannelDetails', () => {
saveChannelAccessPolicy: jest.fn().mockResolvedValue({data: {}}),
validateChannelExpression: jest.fn().mockResolvedValue({data: {}}),
createAccessControlSyncJob: jest.fn().mockResolvedValue({data: {}}),
updateAccessControlPolicyActive: jest.fn().mockResolvedValue({data: {}}),
updateAccessControlPoliciesActive: jest.fn().mockResolvedValue({data: {}}),
searchUsersForExpression: jest.fn().mockResolvedValue({data: {users: [], total: 0}}),
getChannelMembers: jest.fn().mockResolvedValue({data: []}),
getProfilesByIds: jest.fn().mockResolvedValue({data: []}),
@ -246,7 +246,7 @@ describe('admin_console/team_channel_settings/channel/ChannelDetails', () => {
saveChannelAccessPolicy: jest.fn().mockResolvedValue({data: {}}),
validateChannelExpression: jest.fn().mockResolvedValue({data: {}}),
createAccessControlSyncJob: jest.fn().mockResolvedValue({data: {}}),
updateAccessControlPolicyActive: jest.fn().mockResolvedValue({data: {}}),
updateAccessControlPoliciesActive: jest.fn().mockResolvedValue({data: {}}),
searchUsersForExpression: jest.fn().mockResolvedValue({data: {users: [], total: 0}}),
getChannelMembers: jest.fn().mockResolvedValue({data: []}),
getProfilesByIds: jest.fn().mockResolvedValue({data: []}),
@ -388,7 +388,7 @@ describe('admin_console/team_channel_settings/channel/ChannelDetails', () => {
saveChannelAccessPolicy: jest.fn().mockResolvedValue({data: {}}),
validateChannelExpression: jest.fn().mockResolvedValue({data: {}}),
createAccessControlSyncJob: jest.fn().mockResolvedValue({data: {}}),
updateAccessControlPolicyActive: jest.fn().mockResolvedValue({data: {}}),
updateAccessControlPoliciesActive: jest.fn().mockResolvedValue({data: {}}),
searchUsersForExpression: jest.fn().mockResolvedValue({data: {users: [], total: 0}}),
getChannelMembers: jest.fn().mockResolvedValue({data: []}),
getProfilesByIds: jest.fn().mockResolvedValue({data: []}),

6
webapp/channels/src/components/admin_console/team_channel_settings/channel/details/channel_details.tsx
View file

@ -5,7 +5,7 @@ import cloneDeep from 'lodash/cloneDeep';
import React from 'react';
import {FormattedMessage} from 'react-intl';
import type {AccessControlPolicy} from '@mattermost/types/access_control';
import type {AccessControlPolicy, AccessControlPolicyActiveUpdate} 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';
@ -139,7 +139,7 @@ export type ChannelDetailsActions = {
saveChannelAccessPolicy: (policy: AccessControlPolicy) => Promise<ActionResult>;
validateChannelExpression: (expression: string, channelId: string) => Promise<ActionResult>;
createAccessControlSyncJob: (job: JobTypeBase & { data: any }) => Promise<ActionResult>;
updateAccessControlPolicyActive: (policyId: string, active: boolean) => Promise<ActionResult>;
updateAccessControlPoliciesActive: (states: AccessControlPolicyActiveUpdate[]) => Promise<ActionResult>;
searchUsersForExpression: (expression: string, term: string, after: string, limit: number, channelId?: string) => Promise<ActionResult>;
getChannelMembers: (channelId: string, page?: number, perPage?: number) => Promise<ActionResult>;
getProfilesByIds: (userIds: string[]) => Promise<ActionResult>;
@ -802,7 +802,7 @@ export default class ChannelDetails extends React.PureComponent<ChannelDetailsPr
} else {
// Update the active status separately
try {
await actions.updateAccessControlPolicyActive(channelID, channelRulesAutoSync);
await actions.updateAccessControlPoliciesActive([{id: channelID, active: channelRulesAutoSync} as AccessControlPolicyActiveUpdate]);
} catch (activeError) {
// eslint-disable-next-line no-console
console.error('Failed to update policy active status:', activeError);

4
webapp/channels/src/components/admin_console/team_channel_settings/channel/details/index.ts
View file

@ -7,7 +7,7 @@ import type {Dispatch} from 'redux';
import type {GlobalState} from '@mattermost/types/store';
import {getAccessControlPolicy, deleteAccessControlPolicy, assignChannelsToAccessControlPolicy, searchAccessControlPolicies, unassignChannelsFromAccessControlPolicy, createAccessControlPolicy, getAccessControlFields, getVisualAST, validateExpressionAgainstRequester, updateAccessControlPolicyActive, searchUsersForExpression} from 'mattermost-redux/actions/access_control';
import {getAccessControlPolicy, deleteAccessControlPolicy, assignChannelsToAccessControlPolicy, searchAccessControlPolicies, unassignChannelsFromAccessControlPolicy, createAccessControlPolicy, getAccessControlFields, getVisualAST, validateExpressionAgainstRequester, updateAccessControlPoliciesActive, searchUsersForExpression} from 'mattermost-redux/actions/access_control';
import {
addChannelMember,
deleteChannel,
@ -127,7 +127,7 @@ function mapDispatchToProps(dispatch: Dispatch) {
saveChannelAccessPolicy: createAccessControlPolicy,
validateChannelExpression: validateExpressionAgainstRequester,
createAccessControlSyncJob: createJob,
updateAccessControlPolicyActive,
updateAccessControlPoliciesActive,
searchUsersForExpression,
getChannelMembers,
getProfilesByIds,

3
webapp/channels/src/components/channel_settings_modal/channel_settings_access_rules_activity_warning.test.tsx
View file

@ -146,10 +146,9 @@ describe('ChannelSettingsAccessRulesTab - Activity Warning Integration', () => {
getChannelMembers: jest.fn().mockResolvedValue({data: []}),
createJob: jest.fn().mockResolvedValue({data: {}}),
createAccessControlSyncJob: jest.fn().mockResolvedValue({data: {}}),
updateAccessControlPolicyActive: jest.fn().mockResolvedValue({data: {}}),
updateAccessControlPoliciesActive: jest.fn().mockResolvedValue({data: {}}),
validateExpressionAgainstRequester: jest.fn().mockResolvedValue({data: {requester_matches: true}}),
savePreferences: jest.fn().mockResolvedValue({data: {}}),
updateAccessControlPoliciesActive: jest.fn().mockResolvedValue({data: {}}),
};
const defaultProps = {

10
webapp/channels/src/packages/mattermost-redux/src/actions/access_control.ts
View file

@ -125,16 +125,6 @@ export function getAccessControlFields(after: string, limit: number, channelId?:
});
}
export function updateAccessControlPolicyActive(policyId: string, active: boolean) {
return bindClientFunc({
clientFunc: Client4.updateAccessControlPolicyActive,
params: [
policyId,
active,
],
});
}
export function searchUsersForExpression(expression: string, term: string, after: string, limit: number, channelId?: string): ActionFuncAsync<AccessControlTestResult> {
return async (dispatch, getState) => {
let data;

7
webapp/platform/client/src/client4.ts
View file

@ -4681,13 +4681,6 @@ export default class Client4 {
);
};
updateAccessControlPolicyActive = (policyId: string, active: boolean) => {
return this.doFetch<StatusOK>(
`${this.getBaseRoute()}/access_control_policies/${policyId}/activate?active=${active}`,
{method: 'get'},
);
};
assignChannelsToAccessControlPolicy = (policyId: string, channelIds: string[]) => {
return this.doFetch<StatusOK>(
`${this.getBaseRoute()}/access_control_policies/${policyId}/assign`,