MM-53092 Fix for Updating SysAdmin user. (#23750)

* check retrieved user for role and permissions

* add unit tests

---------

Co-authored-by: Mattermost Build <build@mattermost.com>
This commit is contained in:
Scott Bishel 2023-06-22 10:40:21 -06:00 committed by GitHub
parent f333085f8a
commit 644381b35e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 46 additions and 6 deletions

View File

@ -1249,12 +1249,6 @@ func updateUser(c *Context, w http.ResponseWriter, r *http.Request) {
return
}
// Cannot update a system admin unless user making request is a systemadmin also.
if user.IsSystemAdmin() && !c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionManageSystem) {
c.SetPermissionError(model.PermissionManageSystem)
return
}
if !c.App.SessionHasPermissionToUser(*c.AppContext.Session(), user.Id) {
c.SetPermissionError(model.PermissionEditOtherUsers)
return
@ -1265,6 +1259,12 @@ func updateUser(c *Context, w http.ResponseWriter, r *http.Request) {
c.Err = err
return
}
// Cannot update a system admin unless user making request is a systemadmin also.
if ouser.IsSystemAdmin() && !c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionManageSystem) {
c.SetPermissionError(model.PermissionManageSystem)
return
}
auditRec.AddEventPriorState(ouser)
auditRec.AddEventObjectType("user")

View File

@ -1910,6 +1910,26 @@ func TestUpdateUser(t *testing.T) {
})
}
func TestUpdateAdminUser(t *testing.T) {
th := Setup(t).InitBasic()
defer th.TearDown()
user := th.CreateUser()
th.App.UpdateUserRoles(th.Context, user.Id, model.SystemUserRoleId+" "+model.SystemAdminRoleId, false)
user.Email = th.GenerateTestEmail()
th.AddPermissionToRole(model.PermissionEditOtherUsers.Id, model.SystemUserManagerRoleId)
th.App.UpdateUserRoles(th.Context, th.BasicUser.Id, model.SystemUserManagerRoleId+" "+model.SystemUserAccessTokenRoleId, false)
_, resp, err := th.Client.UpdateUser(context.Background(), user)
require.Error(t, err)
CheckForbiddenStatus(t, resp)
u2, _, err := th.SystemAdminClient.UpdateUser(context.Background(), user)
require.NoError(t, err)
require.Equal(t, user.Email, u2.Email)
}
func TestPatchUser(t *testing.T) {
th := Setup(t).InitBasic()
defer th.TearDown()
@ -2021,6 +2041,26 @@ func TestPatchUser(t *testing.T) {
require.NoError(t, err)
}
func TestPatchAdminUser(t *testing.T) {
th := Setup(t).InitBasic()
defer th.TearDown()
user := th.CreateUser()
th.App.UpdateUserRoles(th.Context, user.Id, model.SystemUserRoleId+" "+model.SystemAdminRoleId, false)
patch := &model.UserPatch{}
patch.Email = model.NewString(th.GenerateTestEmail())
th.AddPermissionToRole(model.PermissionEditOtherUsers.Id, model.SystemUserManagerRoleId)
th.App.UpdateUserRoles(th.Context, th.BasicUser.Id, model.SystemUserManagerRoleId+" "+model.SystemUserAccessTokenRoleId, false)
_, resp, err := th.Client.PatchUser(context.Background(), user.Id, patch)
require.Error(t, err)
CheckForbiddenStatus(t, resp)
_, _, err = th.SystemAdminClient.PatchUser(context.Background(), user.Id, patch)
require.NoError(t, err)
}
func TestUserUnicodeNames(t *testing.T) {
th := Setup(t)
defer th.TearDown()