Improves validation in getRolesByName endpoint (#25215)

* Improves validation in getRolesByName endpoint

* Updates the max constant and fixes linter

* Adds a mechanism to split roles in chunks in the webapp client

---------

Co-authored-by: Mattermost Build <build@mattermost.com>
This commit is contained in:
Miguel de la Cruz
2023-12-22 10:29:51 +01:00
committed by GitHub
parent be24f108e1
commit 0e60f3d542
5 changed files with 45 additions and 1 deletions

View File

@@ -12,6 +12,8 @@ import (
"github.com/mattermost/mattermost/server/v8/channels/audit"
)
const GetRolesByNamesMax = 100
var notAllowedPermissions = []string{
model.PermissionSysconsoleWriteUserManagementSystemRoles.Id,
model.PermissionSysconsoleReadUserManagementSystemRoles.Id,
@@ -89,6 +91,13 @@ func getRolesByNames(c *Context, w http.ResponseWriter, r *http.Request) {
return
}
if len(rolenames) > GetRolesByNamesMax {
c.Err = model.NewAppError("getRolesByNames", "api.roles.get_multiple_by_name_too_many.request_error", map[string]any{
"MaxNames": GetRolesByNamesMax,
}, "", http.StatusBadRequest)
return
}
cleanedRoleNames, valid := model.CleanRoleNames(rolenames)
if !valid {
c.SetInvalidParam("rolename")

View File

@@ -184,6 +184,18 @@ func TestGetRolesByNames(t *testing.T) {
_, _, err = client.GetRolesByNames(context.Background(), []string{model.NewId(), model.NewId(), "", " "})
require.NoError(t, err)
})
th.TestForAllClients(t, func(t *testing.T, client *model.Client4) {
// too many roles should error with bad request
roles := []string{}
for i := 0; i < GetRolesByNamesMax+10; i++ {
roles = append(roles, role1.Name)
}
_, resp, err := client.GetRolesByNames(context.Background(), roles)
require.Error(t, err)
CheckBadRequestStatus(t, resp)
})
}
func TestPatchRole(t *testing.T) {

View File

@@ -2682,6 +2682,10 @@
"id": "api.restricted_system_admin",
"translation": "This action is forbidden to a restricted system admin."
},
{
"id": "api.roles.get_multiple_by_name_too_many.request_error",
"translation": "Unable to get that many roles by name. Only {{.MaxNames}} roles can be requested at once."
},
{
"id": "api.roles.patch_roles.license.error",
"translation": "Your license does not support advanced permissions."

View File

@@ -10,6 +10,8 @@ import type {DispatchFunc, GetStateFunc, ActionFunc} from 'mattermost-redux/type
import {bindClientFunc} from './helpers';
import {General} from '../constants';
export function getRolesByNames(rolesNames: string[]) {
return bindClientFunc({
clientFunc: Client4.getRolesByNames,
@@ -98,8 +100,24 @@ export function loadRolesIfNeeded(roles: Iterable<string>): ActionFunc {
if (state.entities.roles.pending) {
await dispatch(setPendingRoles([]));
}
if (newRoles.size > 0) {
return getRolesByNames(Array.from(newRoles))(dispatch, getState);
const newRolesArray = Array.from(newRoles);
const getRolesRequests = [];
for (let i = 0; i < newRolesArray.length; i += General.MAX_GET_ROLES_BY_NAMES) {
const chunk = newRolesArray.slice(i, i + General.MAX_GET_ROLES_BY_NAMES);
getRolesRequests.push(getRolesByNames(chunk)(dispatch, getState));
}
const result = await Promise.all(getRolesRequests);
return result.reduce(
(acc: Record<string, any>, val: Record<string, any>): Record<string, any> => {
acc.data = acc.data.concat(val.data);
return acc;
},
{data: []},
);
}
return {data: state.entities.roles.roles};
};

View File

@@ -76,4 +76,5 @@ export default {
ALWAYS_ON: 'always_on',
DEFAULT_GROUP: 'board',
CUSTOM_GROUP_USER_ROLE: 'custom_group_user',
MAX_GET_ROLES_BY_NAMES: 100,
};