mirrors/mostlymatter
1
0
Форкнуть 0
Код Задачи Запросы на слияние Действия Пакеты Проекты Релизы Вики Активность

MM-16990 - Fix webhooks visible to users without viewing permissions (#11698)

Просмотр исходного кода 
* Filtered incoming webhooks for users wihtout PERMISSION_MANAGE_OTHERS_INCOMING_WEBHOOKS

* Filtered outgoing webhooks for users without PERMISSION_MANAGE_OTHERS_OUTGOING_WEBHOOKS

* Refactored GetOutgoingByTeamByUser to its own method in app and store

* Fixed paging condition for outgoing webhooks in store

* Separated test cases into separate t.run in WebhookStore

* Improved unit test. PR Feedback

* Filtered outgoing webhooks by channel for users without PERMISSION_MANAGE_OTHERS

* Filtered getting full list of outgoing webhooks for users without PERMISSION_MANAGE_OTHERS

* Added missing signature for GetOutgoingWebhooksPage in app

* Expanded permissions in test to SYSTEM_USER_ROLE

* Filtered getting full list of incoming webhooks for users without PERMISSION_MANAGE_OTHERS

* Removed unnecessary sq.and operator
Этот коммит содержится в:
Maria A Nunez
2019-07-29 12:32:26 -04:00
коммит произвёл GitHub
родитель 8f4dab0162
Коммит 3187907b67
7 изменённых файлов: 671 добавлений и 35 удалений
Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

37
api4/webhook.go
Просмотреть файл

@@ -137,6 +137,7 @@ func updateIncomingHook(c *Context, w http.ResponseWriter, r *http.Request) {
func getIncomingHooks(c *Context, w http.ResponseWriter, r *http.Request) {
teamId := r.URL.Query().Get("team_id")
userId := c.App.Session.UserId
var hooks []*model.IncomingWebhook
var err *model.AppError
@@ -147,14 +148,24 @@ func getIncomingHooks(c *Context, w http.ResponseWriter, r *http.Request) {
return
}
hooks, err = c.App.GetIncomingWebhooksForTeamPage(teamId, c.Params.Page, c.Params.PerPage)
// Remove userId as a filter if they have permission to manage others.
if c.App.SessionHasPermissionToTeam(c.App.Session, teamId, model.PERMISSION_MANAGE_OTHERS_INCOMING_WEBHOOKS) {
userId = ""
}
hooks, err = c.App.GetIncomingWebhooksForTeamPageByUser(teamId, userId, c.Params.Page, c.Params.PerPage)
} else {
if !c.App.SessionHasPermissionTo(c.App.Session, model.PERMISSION_MANAGE_INCOMING_WEBHOOKS) {
c.SetPermissionError(model.PERMISSION_MANAGE_INCOMING_WEBHOOKS)
return
}
hooks, err = c.App.GetIncomingWebhooksPage(c.Params.Page, c.Params.PerPage)
// Remove userId as a filter if they have permission to manage others.
if c.App.SessionHasPermissionTo(c.App.Session, model.PERMISSION_MANAGE_OTHERS_INCOMING_WEBHOOKS) {
userId = ""
}
hooks, err = c.App.GetIncomingWebhooksPageByUser(userId, c.Params.Page, c.Params.PerPage)
}
if err != nil {
@@ -339,6 +350,7 @@ func createOutgoingHook(c *Context, w http.ResponseWriter, r *http.Request) {
func getOutgoingHooks(c *Context, w http.ResponseWriter, r *http.Request) {
channelId := r.URL.Query().Get("channel_id")
teamId := r.URL.Query().Get("team_id")
userId := c.App.Session.UserId
var hooks []*model.OutgoingWebhook
var err *model.AppError
@@ -349,21 +361,36 @@ func getOutgoingHooks(c *Context, w http.ResponseWriter, r *http.Request) {
return
}
hooks, err = c.App.GetOutgoingWebhooksForChannelPage(channelId, c.Params.Page, c.Params.PerPage)
// Remove userId as a filter if they have permission to manage others.
if c.App.SessionHasPermissionToChannel(c.App.Session, channelId, model.PERMISSION_MANAGE_OTHERS_OUTGOING_WEBHOOKS) {
userId = ""
}
hooks, err = c.App.GetOutgoingWebhooksForChannelPageByUser(channelId, userId, c.Params.Page, c.Params.PerPage)
} else if len(teamId) > 0 {
if !c.App.SessionHasPermissionToTeam(c.App.Session, teamId, model.PERMISSION_MANAGE_OUTGOING_WEBHOOKS) {
c.SetPermissionError(model.PERMISSION_MANAGE_OUTGOING_WEBHOOKS)
return
}
hooks, err = c.App.GetOutgoingWebhooksForTeamPage(teamId, c.Params.Page, c.Params.PerPage)
// Remove userId as a filter if they have permission to manage others.
if c.App.SessionHasPermissionToTeam(c.App.Session, teamId, model.PERMISSION_MANAGE_OTHERS_OUTGOING_WEBHOOKS) {
userId = ""
}
hooks, err = c.App.GetOutgoingWebhooksForTeamPageByUser(teamId, userId, c.Params.Page, c.Params.PerPage)
} else {
if !c.App.SessionHasPermissionTo(c.App.Session, model.PERMISSION_MANAGE_OUTGOING_WEBHOOKS) {
c.SetPermissionError(model.PERMISSION_MANAGE_OUTGOING_WEBHOOKS)
return
}
hooks, err = c.App.GetOutgoingWebhooksPage(c.Params.Page, c.Params.PerPage)
// Remove userId as a filter if they have permission to manage others.
if c.App.SessionHasPermissionTo(c.App.Session, model.PERMISSION_MANAGE_OTHERS_OUTGOING_WEBHOOKS) {
userId = ""
}
hooks, err = c.App.GetOutgoingWebhooksPageByUser(userId, c.Params.Page, c.Params.PerPage)
}
if err != nil {

212
api4/webhook_test.go
Просмотреть файл

@@ -74,7 +74,6 @@ func TestCreateIncomingWebhook(t *testing.T) {
CheckNotImplementedStatus(t, resp)
}
func TestCreateIncomingWebhook_BypassTeamPermissions(t *testing.T) {
th := Setup().InitBasic()
defer th.TearDown()
@@ -96,7 +95,7 @@ func TestCreateIncomingWebhook_BypassTeamPermissions(t *testing.T) {
require.Equal(t, rhook.ChannelId, hook.ChannelId)
require.Equal(t, rhook.UserId, th.BasicUser.Id)
require.Equal(t, rhook.TeamId,th.BasicTeam.Id)
require.Equal(t, rhook.TeamId, th.BasicTeam.Id)
team := th.CreateTeam()
team.AllowOpenInvite = false
@@ -188,6 +187,88 @@ func TestGetIncomingWebhooks(t *testing.T) {
CheckUnauthorizedStatus(t, resp)
}
func TestGetIncomingWebhooksListByUser(t *testing.T) {
th := Setup().InitBasic()
defer th.TearDown()
BasicClient := th.Client
th.LoginBasic()
th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableIncomingWebhooks = true })
defaultRolePermissions := th.SaveDefaultRolePermissions()
defer func() {
th.RestoreDefaultRolePermissions(defaultRolePermissions)
}()
th.AddPermissionToRole(model.PERMISSION_MANAGE_INCOMING_WEBHOOKS.Id, model.TEAM_ADMIN_ROLE_ID)
th.AddPermissionToRole(model.PERMISSION_MANAGE_INCOMING_WEBHOOKS.Id, model.SYSTEM_USER_ROLE_ID)
// Basic user webhook
bHook := &model.IncomingWebhook{ChannelId: th.BasicChannel.Id, TeamId: th.BasicTeam.Id, UserId: th.BasicUser.Id}
basicHook, resp := BasicClient.CreateIncomingWebhook(bHook)
CheckNoError(t, resp)
basicHooks, resp := BasicClient.GetIncomingWebhooks(0, 1000, "")
CheckNoError(t, resp)
assert.Equal(t, 1, len(basicHooks))
assert.Equal(t, basicHook.Id, basicHooks[0].Id)
// Admin User webhook
aHook := &model.IncomingWebhook{ChannelId: th.BasicChannel.Id, TeamId: th.BasicTeam.Id, UserId: th.SystemAdminUser.Id}
_, resp = th.SystemAdminClient.CreateIncomingWebhook(aHook)
CheckNoError(t, resp)
adminHooks, resp := th.SystemAdminClient.GetIncomingWebhooks(0, 1000, "")
CheckNoError(t, resp)
assert.Equal(t, 2, len(adminHooks))
//Re-check basic user that has no MANAGE_OTHERS permission
filteredHooks, resp := BasicClient.GetIncomingWebhooks(0, 1000, "")
CheckNoError(t, resp)
assert.Equal(t, 1, len(filteredHooks))
assert.Equal(t, basicHook.Id, filteredHooks[0].Id)
}
func TestGetIncomingWebhooksByTeam(t *testing.T) {
th := Setup().InitBasic()
defer th.TearDown()
BasicClient := th.Client
th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableIncomingWebhooks = true })
defaultRolePermissions := th.SaveDefaultRolePermissions()
defer func() {
th.RestoreDefaultRolePermissions(defaultRolePermissions)
}()
th.AddPermissionToRole(model.PERMISSION_MANAGE_INCOMING_WEBHOOKS.Id, model.TEAM_ADMIN_ROLE_ID)
th.AddPermissionToRole(model.PERMISSION_MANAGE_INCOMING_WEBHOOKS.Id, model.TEAM_USER_ROLE_ID)
// Basic user webhook
bHook := &model.IncomingWebhook{ChannelId: th.BasicChannel.Id, TeamId: th.BasicTeam.Id, UserId: th.BasicUser.Id}
basicHook, resp := BasicClient.CreateIncomingWebhook(bHook)
CheckNoError(t, resp)
basicHooks, resp := BasicClient.GetIncomingWebhooksForTeam(th.BasicTeam.Id, 0, 1000, "")
CheckNoError(t, resp)
assert.Equal(t, 1, len(basicHooks))
assert.Equal(t, basicHook.Id, basicHooks[0].Id)
// Admin User webhook
aHook := &model.IncomingWebhook{ChannelId: th.BasicChannel.Id, TeamId: th.BasicTeam.Id, UserId: th.SystemAdminUser.Id}
_, resp = th.SystemAdminClient.CreateIncomingWebhook(aHook)
CheckNoError(t, resp)
adminHooks, resp := th.SystemAdminClient.GetIncomingWebhooksForTeam(th.BasicTeam.Id, 0, 1000, "")
CheckNoError(t, resp)
assert.Equal(t, 2, len(adminHooks))
//Re-check basic user that has no MANAGE_OTHERS permission
filteredHooks, resp := BasicClient.GetIncomingWebhooksForTeam(th.BasicTeam.Id, 0, 1000, "")
CheckNoError(t, resp)
assert.Equal(t, 1, len(filteredHooks))
assert.Equal(t, basicHook.Id, filteredHooks[0].Id)
}
func TestGetIncomingWebhook(t *testing.T) {
th := Setup().InitBasic()
defer th.TearDown()
@@ -429,6 +510,129 @@ func TestGetOutgoingWebhooks(t *testing.T) {
CheckUnauthorizedStatus(t, resp)
}
func TestGetOutgoingWebhooksByTeam(t *testing.T) {
th := Setup().InitBasic()
defer th.TearDown()
BasicClient := th.Client
th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableOutgoingWebhooks = true })
defaultRolePermissions := th.SaveDefaultRolePermissions()
defer func() {
th.RestoreDefaultRolePermissions(defaultRolePermissions)
}()
th.AddPermissionToRole(model.PERMISSION_MANAGE_OUTGOING_WEBHOOKS.Id, model.TEAM_ADMIN_ROLE_ID)
th.AddPermissionToRole(model.PERMISSION_MANAGE_OUTGOING_WEBHOOKS.Id, model.TEAM_USER_ROLE_ID)
// Basic user webhook
bHook := &model.OutgoingWebhook{ChannelId: th.BasicChannel.Id, TeamId: th.BasicChannel.TeamId, CallbackURLs: []string{"http://nowhere.com"}}
basicHook, resp := BasicClient.CreateOutgoingWebhook(bHook)
CheckNoError(t, resp)
basicHooks, resp := BasicClient.GetOutgoingWebhooksForTeam(th.BasicTeam.Id, 0, 1000, "")
CheckNoError(t, resp)
assert.Equal(t, 1, len(basicHooks))
assert.Equal(t, basicHook.Id, basicHooks[0].Id)
// Admin User webhook
aHook := &model.OutgoingWebhook{ChannelId: th.BasicChannel.Id, TeamId: th.BasicChannel.TeamId, CallbackURLs: []string{"http://nowhere.com"}}
_, resp = th.SystemAdminClient.CreateOutgoingWebhook(aHook)
CheckNoError(t, resp)
adminHooks, resp := th.SystemAdminClient.GetOutgoingWebhooksForTeam(th.BasicTeam.Id, 0, 1000, "")
CheckNoError(t, resp)
assert.Equal(t, 2, len(adminHooks))
//Re-check basic user that has no MANAGE_OTHERS permission
filteredHooks, resp := BasicClient.GetOutgoingWebhooksForTeam(th.BasicTeam.Id, 0, 1000, "")
CheckNoError(t, resp)
assert.Equal(t, 1, len(filteredHooks))
assert.Equal(t, basicHook.Id, filteredHooks[0].Id)
}
func TestGetOutgoingWebhooksByChannel(t *testing.T) {
th := Setup().InitBasic()
defer th.TearDown()
BasicClient := th.Client
th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableOutgoingWebhooks = true })
defaultRolePermissions := th.SaveDefaultRolePermissions()
defer func() {
th.RestoreDefaultRolePermissions(defaultRolePermissions)
}()
th.AddPermissionToRole(model.PERMISSION_MANAGE_OUTGOING_WEBHOOKS.Id, model.TEAM_ADMIN_ROLE_ID)
th.AddPermissionToRole(model.PERMISSION_MANAGE_OUTGOING_WEBHOOKS.Id, model.TEAM_USER_ROLE_ID)
// Basic user webhook
bHook := &model.OutgoingWebhook{ChannelId: th.BasicChannel.Id, TeamId: th.BasicChannel.TeamId, CallbackURLs: []string{"http://nowhere.com"}}
basicHook, resp := BasicClient.CreateOutgoingWebhook(bHook)
CheckNoError(t, resp)
basicHooks, resp := BasicClient.GetOutgoingWebhooksForChannel(th.BasicChannel.Id, 0, 1000, "")
CheckNoError(t, resp)
assert.Equal(t, 1, len(basicHooks))
assert.Equal(t, basicHook.Id, basicHooks[0].Id)
// Admin User webhook
aHook := &model.OutgoingWebhook{ChannelId: th.BasicChannel.Id, TeamId: th.BasicChannel.TeamId, CallbackURLs: []string{"http://nowhere.com"}}
_, resp = th.SystemAdminClient.CreateOutgoingWebhook(aHook)
CheckNoError(t, resp)
adminHooks, resp := th.SystemAdminClient.GetOutgoingWebhooksForChannel(th.BasicChannel.Id, 0, 1000, "")
CheckNoError(t, resp)
assert.Equal(t, 2, len(adminHooks))
//Re-check basic user that has no MANAGE_OTHERS permission
filteredHooks, resp := BasicClient.GetOutgoingWebhooksForChannel(th.BasicChannel.Id, 0, 1000, "")
CheckNoError(t, resp)
assert.Equal(t, 1, len(filteredHooks))
assert.Equal(t, basicHook.Id, filteredHooks[0].Id)
}
func TestGetOutgoingWebhooksListByUser(t *testing.T) {
th := Setup().InitBasic()
defer th.TearDown()
BasicClient := th.Client
th.LoginBasic()
th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableOutgoingWebhooks = true })
defaultRolePermissions := th.SaveDefaultRolePermissions()
defer func() {
th.RestoreDefaultRolePermissions(defaultRolePermissions)
}()
th.AddPermissionToRole(model.PERMISSION_MANAGE_OUTGOING_WEBHOOKS.Id, model.TEAM_ADMIN_ROLE_ID)
th.AddPermissionToRole(model.PERMISSION_MANAGE_OUTGOING_WEBHOOKS.Id, model.SYSTEM_USER_ROLE_ID)
// Basic user webhook
bHook := &model.OutgoingWebhook{ChannelId: th.BasicChannel.Id, TeamId: th.BasicChannel.TeamId, CallbackURLs: []string{"http://nowhere.com"}}
basicHook, resp := BasicClient.CreateOutgoingWebhook(bHook)
CheckNoError(t, resp)
basicHooks, resp := BasicClient.GetOutgoingWebhooks(0, 1000, "")
CheckNoError(t, resp)
assert.Equal(t, 1, len(basicHooks))
assert.Equal(t, basicHook.Id, basicHooks[0].Id)
// Admin User webhook
aHook := &model.OutgoingWebhook{ChannelId: th.BasicChannel.Id, TeamId: th.BasicChannel.TeamId, CallbackURLs: []string{"http://nowhere.com"}}
_, resp = th.SystemAdminClient.CreateOutgoingWebhook(aHook)
CheckNoError(t, resp)
adminHooks, resp := th.SystemAdminClient.GetOutgoingWebhooks(0, 1000, "")
CheckNoError(t, resp)
assert.Equal(t, 2, len(adminHooks))
//Re-check basic user that has no MANAGE_OTHERS permission
filteredHooks, resp := BasicClient.GetOutgoingWebhooks(0, 1000, "")
CheckNoError(t, resp)
assert.Equal(t, 1, len(filteredHooks))
assert.Equal(t, basicHook.Id, filteredHooks[0].Id)
}
func TestGetOutgoingWebhook(t *testing.T) {
th := Setup().InitBasic()
defer th.TearDown()
@@ -695,7 +899,7 @@ func TestUpdateIncomingWebhook_BypassTeamPermissions(t *testing.T) {
require.Equal(t, rhook.ChannelId, hook.ChannelId)
require.Equal(t, rhook.UserId, th.BasicUser.Id)
require.Equal(t, rhook.TeamId,th.BasicTeam.Id)
require.Equal(t, rhook.TeamId, th.BasicTeam.Id)
team := th.CreateTeam()
team.AllowOpenInvite = false
@@ -922,7 +1126,7 @@ func TestUpdateOutgoingWebhook_BypassTeamPermissions(t *testing.T) {
CheckNoError(t, resp)
require.Equal(t, rhook.ChannelId, hook.ChannelId)
require.Equal(t, rhook.TeamId,th.BasicTeam.Id)
require.Equal(t, rhook.TeamId, th.BasicTeam.Id)
team := th.CreateTeam()
team.AllowOpenInvite = false