[MM-47378] Respond with bad requests for wrong query parameters 'roles' in getUsers (#21569)

* Respond with bad requests for wrong query parameters in roles

* Revert "Respond with bad requests for wrong query parameters in roles"

This reverts commit d8374d94e0.

* Add GetUser client function to query with channel_id and roles

* Return bad parameters error on invalid roles

* Make client function generic, lint fixes

* i18n strings addition

* Validate 'role', add stricter check for comma separated roles
This commit is contained in:
Shivashis Padhi
2022-12-09 11:43:13 +05:30
committed by GitHub
parent fc31b1713e
commit 0193bfd7de
5 changed files with 149 additions and 0 deletions

View File

@@ -693,14 +693,44 @@ func getUsers(c *Context, w http.ResponseWriter, r *http.Request) {
c.SetInvalidURLParam("inactive")
}
roleNamesAll := []string{}
// MM-47378: validate 'role' related parameters
if role != "" || rolesString != "" || channelRolesString != "" || teamRolesString != "" {
// fetch all role names
rolesAll, err := c.App.GetAllRoles()
if err != nil {
c.Err = model.NewAppError("Api4.getUsers", "api.user.get_users.validation.app_error", nil, "Error fetching roles during validation.", http.StatusBadRequest)
return
}
for _, role := range rolesAll {
roleNamesAll = append(roleNamesAll, role.Name)
}
}
roles := []string{}
var rolesValid bool
if role != "" {
roles, rolesValid = model.CleanRoleNames([]string{role})
if !rolesValid {
c.SetInvalidParam("role")
return
}
roleValid := utils.StringInSlice(role, roleNamesAll)
if !roleValid {
c.SetInvalidParam("role")
return
}
}
if rolesString != "" {
roles, rolesValid = model.CleanRoleNames(strings.Split(rolesString, ","))
if !rolesValid {
c.SetInvalidParam("roles")
return
}
validRoleNames := utils.StringArrayIntersection(roleNamesAll, roles)
if len(validRoleNames) != len(roles) {
c.SetInvalidParam("roles")
return
}
}
channelRoles := []string{}
if channelRolesString != "" && inChannelId != "" {
@@ -709,6 +739,11 @@ func getUsers(c *Context, w http.ResponseWriter, r *http.Request) {
c.SetInvalidParam("channelRoles")
return
}
validRoleNames := utils.StringArrayIntersection(roleNamesAll, channelRoles)
if len(validRoleNames) != len(channelRoles) {
c.SetInvalidParam("channelRoles")
return
}
}
teamRoles := []string{}
if teamRolesString != "" && inTeamId != "" {
@@ -717,6 +752,11 @@ func getUsers(c *Context, w http.ResponseWriter, r *http.Request) {
c.SetInvalidParam("teamRoles")
return
}
validRoleNames := utils.StringArrayIntersection(roleNamesAll, teamRoles)
if len(validRoleNames) != len(teamRoles) {
c.SetInvalidParam("teamRoles")
return
}
}
restrictions, appErr := c.App.GetViewUsersRestrictions(c.AppContext.Session().UserId)

View File

@@ -7,11 +7,13 @@ import (
"encoding/json"
"net/http"
"strconv"
"strings"
"github.com/mattermost/mattermost-server/v6/audit"
"github.com/mattermost/mattermost-server/v6/model"
"github.com/mattermost/mattermost-server/v6/shared/mlog"
"github.com/mattermost/mattermost-server/v6/store"
"github.com/mattermost/mattermost-server/v6/utils"
)
func (api *API) InitUserLocal() {
@@ -56,7 +58,78 @@ func localGetUsers(c *Context, w http.ResponseWriter, r *http.Request) {
active := r.URL.Query().Get("active")
inactive := r.URL.Query().Get("inactive")
role := r.URL.Query().Get("role")
rolesString := r.URL.Query().Get("roles")
channelRolesString := r.URL.Query().Get("channel_roles")
teamRolesString := r.URL.Query().Get("team_roles")
sort := r.URL.Query().Get("sort")
roleNamesAll := []string{}
// MM-47378: validate 'role' related parameters
if role != "" || rolesString != "" || channelRolesString != "" || teamRolesString != "" {
// fetch all role names
rolesAll, err := c.App.GetAllRoles()
if err != nil {
c.Err = model.NewAppError("Api4.getUsers", "api.user.get_users.validation.app_error", nil, "Error fetching roles during validation.", http.StatusBadRequest)
return
}
for _, role := range rolesAll {
roleNamesAll = append(roleNamesAll, role.Name)
}
}
var roles []string
var rolesValid bool
if role != "" {
_, rolesValid = model.CleanRoleNames([]string{role})
if !rolesValid {
c.SetInvalidParam("role")
return
}
roleValid := utils.StringInSlice(role, roleNamesAll)
if !roleValid {
c.SetInvalidParam("role")
return
}
}
if rolesString != "" {
roles, rolesValid = model.CleanRoleNames(strings.Split(rolesString, ","))
if !rolesValid {
c.SetInvalidParam("roles")
return
}
validRoleNames := utils.StringArrayIntersection(roleNamesAll, roles)
if len(validRoleNames) != len(roles) {
c.SetInvalidParam("roles")
return
}
}
var channelRoles []string
if channelRolesString != "" && inChannelId != "" {
channelRoles, rolesValid = model.CleanRoleNames(strings.Split(channelRolesString, ","))
if !rolesValid {
c.SetInvalidParam("channelRoles")
return
}
validRoleNames := utils.StringArrayIntersection(roleNamesAll, channelRoles)
if len(validRoleNames) != len(channelRoles) {
c.SetInvalidParam("channelRoles")
return
}
}
var teamRoles []string
if teamRolesString != "" && inTeamId != "" {
teamRoles, rolesValid = model.CleanRoleNames(strings.Split(teamRolesString, ","))
if !rolesValid {
c.SetInvalidParam("teamRoles")
return
}
validRoleNames := utils.StringArrayIntersection(roleNamesAll, teamRoles)
if len(validRoleNames) != len(teamRoles) {
c.SetInvalidParam("teamRoles")
return
}
}
if notInChannelId != "" && inTeamId == "" {
c.SetInvalidURLParam("team_id")

View File

@@ -2410,6 +2410,20 @@ func TestGetUsers(t *testing.T) {
// Check default params for page and per_page
_, err = client.DoAPIGet("/users", "")
require.NoError(t, err)
// Check role params validity
_, _, err = client.GetUsersWithCustomQueryParameters(0, 5, "in_channel=random_channel_id&channel_roles=random_role_doesnt_exist", "")
require.Error(t, err)
require.Equal(t, err.Error(), ": Invalid or missing channelRoles in request body.")
_, _, err = client.GetUsersWithCustomQueryParameters(0, 5, "in_team=random_channel_id&team_roles=random_role_doesnt_exist", "")
require.Error(t, err)
require.Equal(t, err.Error(), ": Invalid or missing teamRoles in request body.")
_, _, err = client.GetUsersWithCustomQueryParameters(0, 5, "roles=random_role_doesnt_exist%2Csystem_user", "")
require.Error(t, err)
require.Equal(t, err.Error(), ": Invalid or missing roles in request body.")
_, _, err = client.GetUsersWithCustomQueryParameters(0, 5, "role=random_role_doesnt_exist", "")
require.Error(t, err)
require.Equal(t, err.Error(), ": Invalid or missing role in request body.")
})
th.Client.Logout()

View File

@@ -4131,6 +4131,10 @@
"id": "api.user.get_user_by_email.permissions.app_error",
"translation": "Unable to get user by email."
},
{
"id": "api.user.get_users.validation.app_error",
"translation": "Error fetching roles during validation."
},
{
"id": "api.user.invalidate_verify_email_tokens.error",
"translation": "Unable to get tokens by type when invalidating email verification tokens"

View File

@@ -1060,6 +1060,24 @@ func (c *Client4) GetUsers(page int, perPage int, etag string) ([]*User, *Respon
return list, BuildResponse(r), nil
}
// GetUsersWithChannelRoles returns a page of users on the system. Page counting starts at 0.
func (c *Client4) GetUsersWithCustomQueryParameters(page int, perPage int, queryParameters, etag string) ([]*User, *Response, error) {
query := fmt.Sprintf("?page=%v&per_page=%v&%v", page, perPage, queryParameters)
r, err := c.DoAPIGet(c.usersRoute()+query, etag)
if err != nil {
return nil, BuildResponse(r), err
}
defer closeBody(r)
var list []*User
if r.StatusCode == http.StatusNotModified {
return list, BuildResponse(r), nil
}
if err := json.NewDecoder(r.Body).Decode(&list); err != nil {
return nil, nil, NewAppError("GetUsers", "api.unmarshal_error", nil, "", http.StatusInternalServerError).Wrap(err)
}
return list, BuildResponse(r), nil
}
// GetUsersInTeam returns a page of users on a team. Page counting starts at 0.
func (c *Client4) GetUsersInTeam(teamId string, page int, perPage int, etag string) ([]*User, *Response, error) {
query := fmt.Sprintf("?in_team=%v&page=%v&per_page=%v", teamId, page, perPage)