IntenseWebs/mattermost
3
0
Fork 0
mirror of https://github.com/mattermost/mattermost.git synced 2025-02-25 18:55:24 -06:00
Code Issues Packages Projects Releases Wiki Activity

[MM-28533] [MM-28532] [MM-28531] Fixes several bugs with sysconsole_write_usermanagement (#15559)

Browse Source 
* MM-28533 Fix incorrect permission check for reset password

* Allow write users to edit other users, promote and demote guests

* Update ancillary perms for PERMISSION_SYSCONSOLE_WRITE_USERMANAGEMENT_USER

* MM-28532

* Dont allow non sysadmin to update passwords / reset passwords / patch user on sysadmins

* MM-28532: Updates test.

* MM-28533: Merge fix.

* MM-28533: Adds ability for new roles to activate/deactivate non-system-admin users.

Co-authored-by: Martin Kraft <martin@upspin.org>
Co-authored-by: Mattermod <mattermod@users.noreply.github.com>
This commit is contained in:
Farhan Munshi
2020-10-07 19:41:46 -04:00
committed by GitHub
parent 7f85991009
commit 6766853f8a
4 changed files with 84 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

42
api4/user.go
View File

@@ -1065,6 +1065,12 @@ func updateUser(c *Context, w http.ResponseWriter, r *http.Request) {
auditRec := c.MakeAuditRecord("updateUser", audit.Fail)
defer c.LogAuditRec(auditRec)
// Cannot update a system admin unless user making request is a systemadmin also.
if user.IsSystemAdmin() && !c.App.SessionHasPermissionTo(*c.App.Session(), model.PERMISSION_MANAGE_SYSTEM) {
c.SetPermissionError(model.PERMISSION_MANAGE_SYSTEM)
return
}
if !c.App.SessionHasPermissionToUser(*c.App.Session(), user.Id) {
c.SetPermissionError(model.PERMISSION_EDIT_OTHER_USERS)
return
@@ -1134,6 +1140,12 @@ func patchUser(c *Context, w http.ResponseWriter, r *http.Request) {
}
auditRec.AddMeta("user", ouser)
// Cannot update a system admin unless user making request is a systemadmin also
if ouser.IsSystemAdmin() && !c.App.SessionHasPermissionTo(*c.App.Session(), model.PERMISSION_MANAGE_SYSTEM) {
c.SetPermissionError(model.PERMISSION_MANAGE_SYSTEM)
return
}
if c.App.Session().IsOAuth && patch.Email != nil {
if ouser.Email != *patch.Email {
c.SetPermissionError(model.PERMISSION_EDIT_OTHER_USERS)
@@ -1199,6 +1211,12 @@ func deleteUser(c *Context, w http.ResponseWriter, r *http.Request) {
}
auditRec.AddMeta("user", user)
// Cannot update a system admin unless user making request is a systemadmin also
if user.IsSystemAdmin() && !c.App.SessionHasPermissionTo(*c.App.Session(), model.PERMISSION_MANAGE_SYSTEM) {
c.SetPermissionError(model.PERMISSION_MANAGE_SYSTEM)
return
}
if c.Params.Permanent {
if *c.App.Config().ServiceSettings.EnableAPIUserDeletion {
err = c.App.PermanentDeleteUser(user)
@@ -1286,7 +1304,7 @@ func updateUserActive(c *Context, w http.ResponseWriter, r *http.Request) {
// true when you're trying to de-activate yourself
isSelfDeactive := !active && c.Params.UserId == c.App.Session().UserId
if !isSelfDeactive && !c.App.SessionHasPermissionTo(*c.App.Session(), model.PERMISSION_MANAGE_SYSTEM) {
if !isSelfDeactive && !c.App.SessionHasPermissionTo(*c.App.Session(), model.PERMISSION_SYSCONSOLE_WRITE_USERMANAGEMENT_USERS) {
c.Err = model.NewAppError("updateUserActive", "api.user.update_active.permissions.app_error", nil, "userId="+c.Params.UserId, http.StatusForbidden)
return
}
@@ -1304,6 +1322,11 @@ func updateUserActive(c *Context, w http.ResponseWriter, r *http.Request) {
}
auditRec.AddMeta("user", user)
if user.IsSystemAdmin() && !c.App.SessionHasPermissionTo(*c.App.Session(), model.PERMISSION_MANAGE_SYSTEM) {
c.SetPermissionError(model.PERMISSION_MANAGE_SYSTEM)
return
}
if active && user.IsGuest() && !*c.App.Config().GuestAccountsSettings.Enable {
c.Err = model.NewAppError("updateUserActive", "api.user.update_active.cannot_enable_guest_when_guest_feature_is_disabled.app_error", nil, "userId="+c.Params.UserId, http.StatusUnauthorized)
return
@@ -1493,8 +1516,15 @@ func updatePassword(c *Context, w http.ResponseWriter, r *http.Request) {
defer c.LogAuditRec(auditRec)
c.LogAudit("attempted")
var canUpdatePassword bool
if user, err := c.App.GetUser(c.Params.UserId); err == nil {
auditRec.AddMeta("user", user)
if user.IsSystemAdmin() {
canUpdatePassword = c.App.SessionHasPermissionTo(*c.App.Session(), model.PERMISSION_MANAGE_SYSTEM)
} else {
canUpdatePassword = c.App.SessionHasPermissionTo(*c.App.Session(), model.PERMISSION_SYSCONSOLE_WRITE_USERMANAGEMENT_USERS)
}
}
var err *model.AppError
@@ -1502,7 +1532,7 @@ func updatePassword(c *Context, w http.ResponseWriter, r *http.Request) {
// There are two main update flows depending on whether the provided password
// is already hashed or not.
if props["already_hashed"] == "true" {
if c.App.SessionHasPermissionTo(*c.App.Session(), model.PERMISSION_MANAGE_SYSTEM) {
if canUpdatePassword {
err = c.App.UpdateHashedPasswordByUserId(c.Params.UserId, newPassword)
} else if c.Params.UserId == c.App.Session().UserId {
err = model.NewAppError("updatePassword", "api.user.update_password.user_and_hashed.app_error", nil, "", http.StatusUnauthorized)
@@ -1518,7 +1548,7 @@ func updatePassword(c *Context, w http.ResponseWriter, r *http.Request) {
}
err = c.App.UpdatePasswordAsUser(c.Params.UserId, currentPassword, newPassword)
} else if c.App.SessionHasPermissionTo(*c.App.Session(), model.PERMISSION_MANAGE_SYSTEM) {
} else if canUpdatePassword {
err = c.App.UpdatePasswordByUserIdSendEmail(c.Params.UserId, newPassword, c.App.T("api.user.reset_password.method"))
} else {
err = model.NewAppError("updatePassword", "api.user.update_password.context.app_error", nil, "", http.StatusForbidden)
@@ -2492,6 +2522,12 @@ func demoteUserToGuest(c *Context, w http.ResponseWriter, r *http.Request) {
c.Err = err
return
}
if user.IsSystemAdmin() && !c.App.SessionHasPermissionTo(*c.App.Session(), model.PERMISSION_MANAGE_SYSTEM) {
c.SetPermissionError(model.PERMISSION_MANAGE_SYSTEM)
return
}
auditRec.AddMeta("user", user)
if user.IsGuest() {

27
api4/user_test.go
View File

@@ -1599,7 +1599,7 @@ func TestUpdateUser(t *testing.T) {
th.Client.Login(user.Email, user.Password)
user.Nickname = "Joram Wilander"
user.Roles = model.SYSTEM_ADMIN_ROLE_ID
user.Roles = model.SYSTEM_USER_ROLE_ID
user.LastPasswordUpdate = 123
ruser, resp := th.Client.UpdateUser(user)
@@ -5208,3 +5208,28 @@ func TestMigrateAuthToSAML(t *testing.T) {
CheckNotImplementedStatus(t, err)
})
}
func TestUpdatePassword(t *testing.T) {
th := Setup(t)
defer th.TearDown()
t.Run("Forbidden when request performed by system user on a system admin", func(t *testing.T) {
res := th.Client.UpdatePassword(th.SystemAdminUser.Id, "Pa$$word11", "foobar")
CheckForbiddenStatus(t, res)
})
t.Run("OK when request performed by system user with requisite system permission, except if requested user is system admin", func(t *testing.T) {
th.AddPermissionToRole(model.PERMISSION_SYSCONSOLE_WRITE_USERMANAGEMENT_USERS.Id, model.SYSTEM_USER_ROLE_ID)
defer th.RemovePermissionFromRole(model.PERMISSION_SYSCONSOLE_WRITE_USERMANAGEMENT_USERS.Id, model.SYSTEM_USER_ROLE_ID)
res := th.Client.UpdatePassword(th.TeamAdminUser.Id, "Pa$$word11", "foobar")
CheckOKStatus(t, res)
res = th.Client.UpdatePassword(th.SystemAdminUser.Id, "Pa$$word11", "foobar")
CheckForbiddenStatus(t, res)
})
t.Run("OK when request performed by system admin, even if requested user is system admin", func(t *testing.T) {
res := th.SystemAdminClient.UpdatePassword(th.SystemAdminUser.Id, "Pa$$word11", "foobar")
CheckOKStatus(t, res)
})
}

10
model/client4.go
View File

@@ -5611,6 +5611,16 @@ func (c *Client4) UploadData(uploadId string, data io.Reader) (*FileInfo, *Respo
return FileInfoFromJson(r.Body), BuildResponse(r)
}
func (c *Client4) UpdatePassword(userId, currentPassword, newPassword string) *Response {
requestBody := map[string]string{"current_password": currentPassword, "new_password": newPassword}
r, err := c.DoApiPut(c.GetUserRoute(userId)+"/password", MapToJson(requestBody))
if err != nil {
return BuildErrorResponse(r, err)
}
defer closeBody(r)
return BuildResponse(r)
}
// Cloud Section
func (c *Client4) GetCloudProducts() ([]*Product, *Response) {

9
model/role.go
View File

@@ -45,6 +45,7 @@ func init() {
CHANNEL_ADMIN_ROLE_ID,
}, NewSystemRoleIDs...)
// When updating the values here, the values in mattermost-redux must also be updated.
SysconsoleAncillaryPermissions = map[string][]*Permission{
PERMISSION_SYSCONSOLE_READ_USERMANAGEMENT_CHANNELS.Id: {
PERMISSION_READ_PUBLIC_CHANNEL,
@@ -69,10 +70,17 @@ func init() {
PERMISSION_SYSCONSOLE_READ_REPORTING.Id: {
PERMISSION_VIEW_TEAM,
},
PERMISSION_SYSCONSOLE_WRITE_USERMANAGEMENT_USERS.Id: {
PERMISSION_EDIT_OTHER_USERS,
PERMISSION_DEMOTE_TO_GUEST,
PERMISSION_PROMOTE_GUEST,
},
PERMISSION_SYSCONSOLE_WRITE_USERMANAGEMENT_CHANNELS.Id: {
PERMISSION_MANAGE_TEAM,
PERMISSION_MANAGE_PUBLIC_CHANNEL_PROPERTIES,
PERMISSION_MANAGE_PRIVATE_CHANNEL_PROPERTIES,
PERMISSION_MANAGE_PRIVATE_CHANNEL_MEMBERS,
PERMISSION_MANAGE_PUBLIC_CHANNEL_MEMBERS,
PERMISSION_DELETE_PRIVATE_CHANNEL,
PERMISSION_DELETE_PUBLIC_CHANNEL,
PERMISSION_MANAGE_CHANNEL_ROLES,
@@ -80,6 +88,7 @@ func init() {
PERMISSION_CONVERT_PRIVATE_CHANNEL_TO_PUBLIC,
},
PERMISSION_SYSCONSOLE_WRITE_USERMANAGEMENT_TEAMS.Id: {
PERMISSION_MANAGE_TEAM,
PERMISSION_MANAGE_TEAM_ROLES,
PERMISSION_REMOVE_USER_FROM_TEAM,
PERMISSION_JOIN_PRIVATE_TEAMS,