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-53098 Fix for checking bot and user permissions on shared endpoints (#23751)

Browse Source 
* temp commit

* update test to allow bot creation

* add bot check to updateUser and deleteUser

* add more unit tests

* lint fixes

* lint fix

* update based on doc

* add more unit tests

* lint fixes

* fix unit tests

* fix unit tests

---------

Co-authored-by: Mattermost Build <build@mattermost.com>
This commit is contained in:
Scott Bishel 2023-07-10 13:28:40 -06:00 committed by GitHub
parent 2abcdfe76a
commit 30140c0a27
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 251 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
server/channels/api4/bot_test.go
View File

@ -106,6 +106,7 @@ func TestCreateBot(t *testing.T) {
th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableUserAccessTokens = true })
th.AddPermissionToRole(model.PermissionCreateBot.Id, model.TeamUserRoleId)
th.AddPermissionToRole(model.PermissionManageBots.Id, model.TeamUserRoleId)
th.AddPermissionToRole(model.PermissionEditOtherUsers.Id, model.TeamUserRoleId)
th.App.UpdateUserRoles(th.Context, th.BasicUser.Id, model.TeamUserRoleId+" "+model.SystemUserAccessTokenRoleId, false)

12
server/channels/api4/user.go
View File

@ -428,7 +428,7 @@ func setProfileImage(c *Context, w http.ResponseWriter, r *http.Request) {
return
}
if !c.App.SessionHasPermissionToUser(*c.AppContext.Session(), c.Params.UserId) {
if !c.App.SessionHasPermissionToUserOrBot(*c.AppContext.Session(), c.Params.UserId) {
c.SetPermissionError(model.PermissionEditOtherUsers)
return
}
@ -499,7 +499,7 @@ func setDefaultProfileImage(c *Context, w http.ResponseWriter, r *http.Request)
return
}
if !c.App.SessionHasPermissionToUser(*c.AppContext.Session(), c.Params.UserId) {
if !c.App.SessionHasPermissionToUserOrBot(*c.AppContext.Session(), c.Params.UserId) {
c.SetPermissionError(model.PermissionEditOtherUsers)
return
}
@ -1249,7 +1249,7 @@ func updateUser(c *Context, w http.ResponseWriter, r *http.Request) {
return
}
if !c.App.SessionHasPermissionToUser(*c.AppContext.Session(), user.Id) {
if !c.App.SessionHasPermissionToUserOrBot(*c.AppContext.Session(), user.Id) {
c.SetPermissionError(model.PermissionEditOtherUsers)
return
}
@ -1259,6 +1259,7 @@ 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)
@ -1325,7 +1326,7 @@ func patchUser(c *Context, w http.ResponseWriter, r *http.Request) {
audit.AddEventParameterAuditable(auditRec, "user_patch", &patch)
defer c.LogAuditRec(auditRec)
if !c.App.SessionHasPermissionToUser(*c.AppContext.Session(), c.Params.UserId) {
if !c.App.SessionHasPermissionToUserOrBot(*c.AppContext.Session(), c.Params.UserId) {
c.SetPermissionError(model.PermissionEditOtherUsers)
return
}
@ -1335,6 +1336,7 @@ func patchUser(c *Context, w http.ResponseWriter, r *http.Request) {
c.SetInvalidParam("user_id")
return
}
auditRec.AddEventPriorState(ouser)
auditRec.AddEventObjectType("user")
@ -1402,7 +1404,7 @@ func deleteUser(c *Context, w http.ResponseWriter, r *http.Request) {
audit.AddEventParameter(auditRec, "user_id", c.Params.UserId)
defer c.LogAuditRec(auditRec)
if !c.App.SessionHasPermissionToUser(*c.AppContext.Session(), userId) {
if !c.App.SessionHasPermissionToUserOrBot(*c.AppContext.Session(), userId) {
c.SetPermissionError(model.PermissionEditOtherUsers)
return
}

58
server/channels/api4/user_test.go
View File

@ -1932,6 +1932,27 @@ func TestUpdateAdminUser(t *testing.T) {
require.Equal(t, user.Email, u2.Email)
}
func TestUpdateBotUser(t *testing.T) {
th := Setup(t).InitBasic()
defer th.TearDown()
th.App.UpdateConfig(func(c *model.Config) {
*c.ServiceSettings.EnableBotAccountCreation = true
})
bot := th.CreateBotWithSystemAdminClient()
botUser, _, err := th.SystemAdminClient.GetUser(context.Background(), bot.UserId, "")
require.NoError(t, err)
updateUser, _, err := th.SystemAdminClient.UpdateUser(context.Background(), botUser)
require.NoError(t, err)
require.Equal(t, botUser.Id, updateUser.Id)
_, resp, err := th.Client.UpdateUser(context.Background(), botUser)
require.Error(t, err)
CheckForbiddenStatus(t, resp)
}
func TestPatchUser(t *testing.T) {
th := Setup(t).InitBasic()
defer th.TearDown()
@ -2043,6 +2064,27 @@ func TestPatchUser(t *testing.T) {
require.NoError(t, err)
}
func TestPatchBotUser(t *testing.T) {
th := Setup(t).InitBasic()
defer th.TearDown()
th.App.UpdateConfig(func(c *model.Config) {
*c.ServiceSettings.EnableBotAccountCreation = true
})
bot := th.CreateBotWithSystemAdminClient()
patch := &model.UserPatch{}
patch.Email = model.NewString("newemail@test.com")
user, _, err := th.SystemAdminClient.PatchUser(context.Background(), bot.UserId, patch)
require.NoError(t, err)
require.Equal(t, bot.UserId, user.Id)
_, resp, err := th.Client.PatchUser(context.Background(), bot.UserId, patch)
require.Error(t, err)
CheckForbiddenStatus(t, resp)
}
func TestPatchAdminUser(t *testing.T) {
th := Setup(t).InitBasic()
defer th.TearDown()
@ -2063,6 +2105,7 @@ func TestPatchAdminUser(t *testing.T) {
_, _, err = th.SystemAdminClient.PatchUser(context.Background(), user.Id, patch)
require.NoError(t, err)
}
func TestUserUnicodeNames(t *testing.T) {
th := Setup(t)
defer th.TearDown()
@ -2229,6 +2272,21 @@ func TestDeleteUser(t *testing.T) {
require.NoError(t, err)
}
func TestDeleteBotUser(t *testing.T) {
th := Setup(t).InitBasic()
defer th.TearDown()
th.App.UpdateConfig(func(c *model.Config) {
*c.ServiceSettings.EnableBotAccountCreation = true
})
bot := th.CreateBotWithSystemAdminClient()
_, err := th.Client.DeleteUser(context.Background(), bot.UserId)
require.Error(t, err)
require.Equal(t, err.Error(), ": You do not have the appropriate permissions.")
}
func TestPermanentDeleteUser(t *testing.T) {
th := Setup(t).InitBasic()
defer th.TearDown()

10
server/channels/app/authorization.go
View File

@ -258,14 +258,16 @@ func (a *App) SessionHasPermissionToUserOrBot(session model.Session, userID stri
if session.IsUnrestricted() {
return true
}
err := a.SessionHasPermissionToManageBot(session, userID)
if err == nil {
return true
}
if err.Id == "store.sql_bot.get.missing.app_error" && err.Unwrap() != nil {
if a.SessionHasPermissionToUser(session, userID) {
return true
}
if err := a.SessionHasPermissionToManageBot(session, userID); err == nil {
return true
}
return false
}

178
server/channels/app/authorization_test.go
View File

@ -113,6 +113,184 @@ func TestSessionHasPermissionToChannel(t *testing.T) {
})
}
func TestHasPermissionToUser(t *testing.T) {
th := Setup(t).InitBasic()
defer th.TearDown()
assert.True(t, th.App.HasPermissionToUser(th.SystemAdminUser.Id, th.BasicUser.Id))
assert.True(t, th.App.HasPermissionToUser(th.BasicUser.Id, th.BasicUser.Id))
assert.False(t, th.App.HasPermissionToUser(th.BasicUser.Id, th.BasicUser2.Id))
}
func TestSessionHasPermissionToManageBot(t *testing.T) {
th := Setup(t).InitBasic()
defer th.TearDown()
bot, err := th.App.CreateBot(th.Context, &model.Bot{
Username: "username",
Description: "a bot",
OwnerId: th.BasicUser.Id,
})
require.Nil(t, err)
defer th.App.PermanentDeleteBot(bot.UserId)
assert.NotNil(t, bot)
t.Run("test my bot", func(t *testing.T) {
session := model.Session{
UserId: th.BasicUser.Id,
Roles: model.SystemUserRoleId,
}
err = th.App.SessionHasPermissionToManageBot(session, bot.UserId)
assert.NotNil(t, err)
assert.Equal(t, "store.sql_bot.get.missing.app_error", err.Id)
assert.NoError(t, err.Unwrap())
th.AddPermissionToRole(model.PermissionReadBots.Id, model.SystemUserRoleId)
err = th.App.SessionHasPermissionToManageBot(session, bot.UserId)
assert.NotNil(t, err)
assert.Equal(t, "api.context.permissions.app_error", err.Id)
assert.NoError(t, err.Unwrap())
th.AddPermissionToRole(model.PermissionManageBots.Id, model.SystemUserRoleId)
err = th.App.SessionHasPermissionToManageBot(session, bot.UserId)
assert.Nil(t, err)
th.RemovePermissionFromRole(model.PermissionReadBots.Id, model.SystemUserRoleId)
th.RemovePermissionFromRole(model.PermissionManageBots.Id, model.SystemUserRoleId)
})
t.Run("test others bot", func(t *testing.T) {
session := model.Session{
UserId: th.BasicUser2.Id,
Roles: model.SystemUserRoleId,
}
err = th.App.SessionHasPermissionToManageBot(session, bot.UserId)
assert.NotNil(t, err)
assert.Equal(t, "store.sql_bot.get.missing.app_error", err.Id)
assert.NoError(t, err.Unwrap())
th.AddPermissionToRole(model.PermissionReadOthersBots.Id, model.SystemUserRoleId)
err = th.App.SessionHasPermissionToManageBot(session, bot.UserId)
assert.NotNil(t, err)
assert.Equal(t, "api.context.permissions.app_error", err.Id)
assert.NoError(t, err.Unwrap())
th.AddPermissionToRole(model.PermissionManageOthersBots.Id, model.SystemUserRoleId)
err = th.App.SessionHasPermissionToManageBot(session, bot.UserId)
assert.Nil(t, err)
th.RemovePermissionFromRole(model.PermissionReadOthersBots.Id, model.SystemUserRoleId)
th.RemovePermissionFromRole(model.PermissionManageOthersBots.Id, model.SystemUserRoleId)
})
t.Run("test sysadmin role", func(t *testing.T) {
session := model.Session{
UserId: th.SystemAdminUser.Id,
Roles: model.SystemAdminRoleId,
}
err = th.App.SessionHasPermissionToManageBot(session, bot.UserId)
assert.Nil(t, err)
})
t.Run("test non bot ", func(t *testing.T) {
session := model.Session{
UserId: th.SystemAdminUser.Id,
Roles: model.SystemUserRoleId,
}
err = th.App.SessionHasPermissionToManageBot(session, "12345")
assert.NotNil(t, err)
assert.Equal(t, "store.sql_bot.get.missing.app_error", err.Id)
assert.Error(t, err.Unwrap())
})
}
func TestSessionHasPermissionToUser(t *testing.T) {
th := Setup(t).InitBasic()
defer th.TearDown()
t.Run("test my user access", func(t *testing.T) {
session := model.Session{
UserId: th.BasicUser.Id,
Roles: model.SystemUserRoleId,
}
assert.True(t, th.App.SessionHasPermissionToUser(session, th.BasicUser.Id))
assert.False(t, th.App.SessionHasPermissionToUser(session, th.BasicUser2.Id))
})
t.Run("test user manager access", func(t *testing.T) {
session := model.Session{
UserId: th.BasicUser.Id,
Roles: model.SystemUserManagerRoleId,
}
assert.False(t, th.App.SessionHasPermissionToUser(session, th.BasicUser2.Id))
th.AddPermissionToRole(model.PermissionEditOtherUsers.Id, model.SystemUserManagerRoleId)
assert.True(t, th.App.SessionHasPermissionToUser(session, th.BasicUser2.Id))
th.RemovePermissionFromRole(model.PermissionEditOtherUsers.Id, model.SystemUserManagerRoleId)
})
t.Run("test admin user access", func(t *testing.T) {
session := model.Session{
UserId: th.SystemAdminUser.Id,
Roles: model.SystemAdminRoleId,
}
assert.True(t, th.App.SessionHasPermissionToUser(session, th.BasicUser.Id))
assert.True(t, th.App.SessionHasPermissionToUser(session, th.BasicUser2.Id))
})
}
func TestSessionHasPermissionToManageUserOrBot(t *testing.T) {
th := Setup(t).InitBasic()
defer th.TearDown()
bot, err := th.App.CreateBot(th.Context, &model.Bot{
Username: "username",
Description: "a bot",
OwnerId: th.BasicUser.Id,
})
require.Nil(t, err)
defer th.App.PermanentDeleteBot(bot.UserId)
t.Run("test basic user access", func(t *testing.T) {
session := model.Session{
UserId: th.BasicUser.Id,
Roles: model.SystemUserRoleId,
}
assert.True(t, th.App.SessionHasPermissionToUserOrBot(session, th.BasicUser.Id))
assert.False(t, th.App.SessionHasPermissionToUserOrBot(session, bot.UserId))
assert.False(t, th.App.SessionHasPermissionToUserOrBot(session, th.BasicUser2.Id))
})
t.Run("test user manager access", func(t *testing.T) {
session := model.Session{
UserId: th.BasicUser2.Id,
Roles: model.SystemUserManagerRoleId,
}
assert.False(t, th.App.SessionHasPermissionToUserOrBot(session, th.BasicUser.Id))
assert.True(t, th.App.SessionHasPermissionToUserOrBot(session, th.BasicUser2.Id))
assert.False(t, th.App.SessionHasPermissionToUserOrBot(session, bot.UserId))
th.AddPermissionToRole(model.PermissionEditOtherUsers.Id, model.SystemUserManagerRoleId)
assert.True(t, th.App.SessionHasPermissionToUserOrBot(session, th.BasicUser.Id))
assert.False(t, th.App.SessionHasPermissionToUserOrBot(session, bot.UserId))
th.RemovePermissionFromRole(model.PermissionEditOtherUsers.Id, model.SystemUserManagerRoleId)
th.AddPermissionToRole(model.PermissionManageOthersBots.Id, model.SystemUserManagerRoleId)
assert.False(t, th.App.SessionHasPermissionToUserOrBot(session, th.BasicUser.Id))
assert.True(t, th.App.SessionHasPermissionToUserOrBot(session, bot.UserId))
th.RemovePermissionFromRole(model.PermissionManageOthersBots.Id, model.SystemUserManagerRoleId)
})
t.Run("test system admin access", func(t *testing.T) {
session := model.Session{
UserId: th.SystemAdminUser.Id,
Roles: model.SystemAdminRoleId,
}
assert.True(t, th.App.SessionHasPermissionToUserOrBot(session, bot.UserId))
assert.True(t, th.App.SessionHasPermissionToUserOrBot(session, th.BasicUser.Id))
})
}
func TestHasPermissionToCategory(t *testing.T) {
th := Setup(t).InitBasic()
defer th.TearDown()