From 2d16a71af9bff88d89244279849f8129a326a0e1 Mon Sep 17 00:00:00 2001 From: George Goldberg Date: Tue, 10 Jul 2018 09:55:46 +0100 Subject: MM-11228: Fix channel update/patch API endpoints. (#9073) --- api4/channel.go | 63 +++++++++++++++++++++++++++++++++++----------------- api4/channel_test.go | 58 ++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 100 insertions(+), 21 deletions(-) (limited to 'api4') diff --git a/api4/channel.go b/api4/channel.go index 1afadf39b..7505d899b 100644 --- a/api4/channel.go +++ b/api4/channel.go @@ -96,12 +96,28 @@ func updateChannel(c *Context, w http.ResponseWriter, r *http.Request) { return } - if _, err = c.App.GetChannelMember(channel.Id, c.Session.UserId); err != nil { - c.Err = err - return - } + switch oldChannel.Type { + case model.CHANNEL_OPEN: + if !c.App.SessionHasPermissionToChannel(c.Session, c.Params.ChannelId, model.PERMISSION_MANAGE_PUBLIC_CHANNEL_PROPERTIES) { + c.SetPermissionError(model.PERMISSION_MANAGE_PUBLIC_CHANNEL_PROPERTIES) + return + } - if !CanManageChannel(c, channel) { + case model.CHANNEL_PRIVATE: + if !c.App.SessionHasPermissionToChannel(c.Session, c.Params.ChannelId, model.PERMISSION_MANAGE_PRIVATE_CHANNEL_PROPERTIES) { + c.SetPermissionError(model.PERMISSION_MANAGE_PRIVATE_CHANNEL_PROPERTIES) + return + } + + case model.CHANNEL_GROUP, model.CHANNEL_DIRECT: + // Modifying the header is not linked to any specific permission for group/dm channels, so just check for membership. + if _, err := c.App.GetChannelMember(channel.Id, c.Session.UserId); err != nil { + c.Err = model.NewAppError("updateChannel", "api.channel.patch_update_channel.forbidden.app_error", nil, "", http.StatusForbidden) + return + } + + default: + c.Err = model.NewAppError("updateChannel", "api.channel.patch_update_channel.forbidden.app_error", nil, "", http.StatusForbidden) return } @@ -205,7 +221,28 @@ func patchChannel(c *Context, w http.ResponseWriter, r *http.Request) { return } - if !CanManageChannel(c, oldChannel) { + switch oldChannel.Type { + case model.CHANNEL_OPEN: + if !c.App.SessionHasPermissionToChannel(c.Session, c.Params.ChannelId, model.PERMISSION_MANAGE_PUBLIC_CHANNEL_PROPERTIES) { + c.SetPermissionError(model.PERMISSION_MANAGE_PUBLIC_CHANNEL_PROPERTIES) + return + } + + case model.CHANNEL_PRIVATE: + if !c.App.SessionHasPermissionToChannel(c.Session, c.Params.ChannelId, model.PERMISSION_MANAGE_PRIVATE_CHANNEL_PROPERTIES) { + c.SetPermissionError(model.PERMISSION_MANAGE_PRIVATE_CHANNEL_PROPERTIES) + return + } + + case model.CHANNEL_GROUP, model.CHANNEL_DIRECT: + // Modifying the header is not linked to any specific permission for group/dm channels, so just check for membership. + if _, err := c.App.GetChannelMember(c.Params.ChannelId, c.Session.UserId); err != nil { + c.Err = model.NewAppError("patchChannel", "api.channel.patch_update_channel.forbidden.app_error", nil, "", http.StatusForbidden) + return + } + + default: + c.Err = model.NewAppError("patchChannel", "api.channel.patch_update_channel.forbidden.app_error", nil, "", http.StatusForbidden) return } @@ -255,20 +292,6 @@ func restoreChannel(c *Context, w http.ResponseWriter, r *http.Request) { } -func CanManageChannel(c *Context, channel *model.Channel) bool { - if channel.Type == model.CHANNEL_OPEN && !c.App.SessionHasPermissionToChannel(c.Session, channel.Id, model.PERMISSION_MANAGE_PUBLIC_CHANNEL_PROPERTIES) { - c.SetPermissionError(model.PERMISSION_MANAGE_PUBLIC_CHANNEL_PROPERTIES) - return false - } - - if channel.Type == model.CHANNEL_PRIVATE && !c.App.SessionHasPermissionToChannel(c.Session, channel.Id, model.PERMISSION_MANAGE_PRIVATE_CHANNEL_PROPERTIES) { - c.SetPermissionError(model.PERMISSION_MANAGE_PRIVATE_CHANNEL_PROPERTIES) - return false - } - - return true -} - func createDirectChannel(c *Context, w http.ResponseWriter, r *http.Request) { userIds := model.ArrayFromJson(r.Body) allowed := false diff --git a/api4/channel_test.go b/api4/channel_test.go index ab751f151..43223d060 100644 --- a/api4/channel_test.go +++ b/api4/channel_test.go @@ -209,8 +209,34 @@ func TestUpdateChannel(t *testing.T) { channel.DisplayName = "Should not update" _, resp = Client.UpdateChannel(channel) - CheckNotFoundStatus(t, resp) + CheckForbiddenStatus(t, resp) + + // Test updating the header of someone else's GM channel. + user1 := th.CreateUser() + user2 := th.CreateUser() + user3 := th.CreateUser() + + groupChannel, resp := Client.CreateGroupChannel([]string{user1.Id, user2.Id}) + CheckNoError(t, resp) + + groupChannel.Header = "lolololol" + Client.Logout() + Client.Login(user3.Email, user3.Password) + _, resp = Client.UpdateChannel(groupChannel) + CheckForbiddenStatus(t, resp) + // Test updating the header of someone else's GM channel. + Client.Logout() + Client.Login(user.Email, user.Password) + + directChannel, resp := Client.CreateDirectChannel(user.Id, user1.Id) + CheckNoError(t, resp) + + directChannel.Header = "lolololol" + Client.Logout() + Client.Login(user3.Email, user3.Password) + _, resp = Client.UpdateChannel(directChannel) + CheckForbiddenStatus(t, resp) } func TestPatchChannel(t *testing.T) { @@ -267,6 +293,36 @@ func TestPatchChannel(t *testing.T) { _, resp = th.SystemAdminClient.PatchChannel(th.BasicPrivateChannel.Id, patch) CheckNoError(t, resp) + + // Test updating the header of someone else's GM channel. + user1 := th.CreateUser() + user2 := th.CreateUser() + user3 := th.CreateUser() + + groupChannel, resp := Client.CreateGroupChannel([]string{user1.Id, user2.Id}) + CheckNoError(t, resp) + + Client.Logout() + Client.Login(user3.Email, user3.Password) + + channelPatch := &model.ChannelPatch{} + channelPatch.Header = new(string) + *channelPatch.Header = "lolololol" + + _, resp = Client.PatchChannel(groupChannel.Id, channelPatch) + CheckForbiddenStatus(t, resp) + + // Test updating the header of someone else's GM channel. + Client.Logout() + Client.Login(user.Email, user.Password) + + directChannel, resp := Client.CreateDirectChannel(user.Id, user1.Id) + CheckNoError(t, resp) + + Client.Logout() + Client.Login(user3.Email, user3.Password) + _, resp = Client.PatchChannel(directChannel.Id, channelPatch) + CheckForbiddenStatus(t, resp) } func TestCreateDirectChannel(t *testing.T) { -- cgit v1.2.3-1-g7c22