From 61b1237c20bc71334acc4f96606a077a6b8c262a Mon Sep 17 00:00:00 2001 From: Joram Wilander Date: Wed, 22 Mar 2017 11:13:44 -0400 Subject: Update channel permissions for v4 endpoints (#5829) * Fix join channel permission for v4 endpoint * Allow regular users to get public channels they are not in * Fix unit test --- api4/apitestlib.go | 20 +++++++++------- api4/channel.go | 68 ++++++++++++++++++++++++++++++++++++---------------- api4/channel_test.go | 53 +++++++++++++++++++++++++++++++++++++--- 3 files changed, 109 insertions(+), 32 deletions(-) (limited to 'api4') diff --git a/api4/apitestlib.go b/api4/apitestlib.go index b3007ebfe..6d1822ae9 100644 --- a/api4/apitestlib.go +++ b/api4/apitestlib.go @@ -25,14 +25,15 @@ import ( ) type TestHelper struct { - Client *model.Client4 - BasicUser *model.User - BasicUser2 *model.User - TeamAdminUser *model.User - BasicTeam *model.Team - BasicChannel *model.Channel - BasicChannel2 *model.Channel - BasicPost *model.Post + Client *model.Client4 + BasicUser *model.User + BasicUser2 *model.User + TeamAdminUser *model.User + BasicTeam *model.Team + BasicChannel *model.Channel + BasicPrivateChannel *model.Channel + BasicChannel2 *model.Channel + BasicPost *model.Post SystemAdminClient *model.Client4 SystemAdminUser *model.User @@ -135,6 +136,7 @@ func (me *TestHelper) InitBasic() *TestHelper { me.LoginTeamAdmin() me.BasicTeam = me.CreateTeam() me.BasicChannel = me.CreatePublicChannel() + me.BasicPrivateChannel = me.CreatePrivateChannel() me.BasicChannel2 = me.CreatePublicChannel() me.BasicPost = me.CreatePost() me.BasicUser = me.CreateUser() @@ -145,6 +147,8 @@ func (me *TestHelper) InitBasic() *TestHelper { app.AddUserToChannel(me.BasicUser2, me.BasicChannel) app.AddUserToChannel(me.BasicUser, me.BasicChannel2) app.AddUserToChannel(me.BasicUser2, me.BasicChannel2) + app.AddUserToChannel(me.BasicUser, me.BasicPrivateChannel) + app.AddUserToChannel(me.BasicUser2, me.BasicPrivateChannel) app.UpdateUserRoles(me.BasicUser.Id, model.ROLE_SYSTEM_USER.Id) me.LoginBasic() diff --git a/api4/channel.go b/api4/channel.go index a4820d729..fd33eb882 100644 --- a/api4/channel.go +++ b/api4/channel.go @@ -199,18 +199,26 @@ func getChannel(c *Context, w http.ResponseWriter, r *http.Request) { return } - if !app.SessionHasPermissionToChannel(c.Session, c.Params.ChannelId, model.PERMISSION_READ_CHANNEL) { - c.SetPermissionError(model.PERMISSION_READ_CHANNEL) + channel, err := app.GetChannel(c.Params.ChannelId) + if err != nil { + c.Err = err return } - if channel, err := app.GetChannel(c.Params.ChannelId); err != nil { - c.Err = err - return + if channel.Type == model.CHANNEL_OPEN { + if !app.SessionHasPermissionToTeam(c.Session, channel.TeamId, model.PERMISSION_READ_PUBLIC_CHANNEL) { + c.SetPermissionError(model.PERMISSION_READ_PUBLIC_CHANNEL) + return + } } else { - w.Write([]byte(channel.ToJson())) - return + if !app.SessionHasPermissionToChannel(c.Session, c.Params.ChannelId, model.PERMISSION_READ_CHANNEL) { + c.SetPermissionError(model.PERMISSION_READ_CHANNEL) + return + } } + + w.Write([]byte(channel.ToJson())) + return } func getChannelUnread(c *Context, w http.ResponseWriter, r *http.Request) { @@ -328,13 +336,19 @@ func getChannelByName(c *Context, w http.ResponseWriter, r *http.Request) { return } - if !app.SessionHasPermissionToChannel(c.Session, channel.Id, model.PERMISSION_READ_CHANNEL) { - c.SetPermissionError(model.PERMISSION_READ_CHANNEL) - return + if channel.Type == model.CHANNEL_OPEN { + if !app.SessionHasPermissionToTeam(c.Session, channel.TeamId, model.PERMISSION_READ_PUBLIC_CHANNEL) { + c.SetPermissionError(model.PERMISSION_READ_PUBLIC_CHANNEL) + return + } + } else { + if !app.SessionHasPermissionToChannel(c.Session, channel.Id, model.PERMISSION_READ_CHANNEL) { + c.SetPermissionError(model.PERMISSION_READ_CHANNEL) + return + } } w.Write([]byte(channel.ToJson())) - return } func getChannelByNameForTeamName(c *Context, w http.ResponseWriter, r *http.Request) { @@ -525,9 +539,19 @@ func addChannelMember(c *Context, w http.ResponseWriter, r *http.Request) { return } - if channel.Type == model.CHANNEL_OPEN && !app.SessionHasPermissionToChannel(c.Session, channel.Id, model.PERMISSION_MANAGE_PUBLIC_CHANNEL_MEMBERS) { - c.SetPermissionError(model.PERMISSION_MANAGE_PUBLIC_CHANNEL_MEMBERS) - return + // Check join permission if adding yourself, otherwise check manage permission + if channel.Type == model.CHANNEL_OPEN { + if member.UserId == c.Session.UserId { + if !app.SessionHasPermissionToChannel(c.Session, channel.Id, model.PERMISSION_JOIN_PUBLIC_CHANNELS) { + c.SetPermissionError(model.PERMISSION_JOIN_PUBLIC_CHANNELS) + return + } + } else { + if !app.SessionHasPermissionToChannel(c.Session, channel.Id, model.PERMISSION_MANAGE_PUBLIC_CHANNEL_MEMBERS) { + c.SetPermissionError(model.PERMISSION_MANAGE_PUBLIC_CHANNEL_MEMBERS) + return + } + } } if channel.Type == model.CHANNEL_PRIVATE && !app.SessionHasPermissionToChannel(c.Session, channel.Id, model.PERMISSION_MANAGE_PRIVATE_CHANNEL_MEMBERS) { @@ -557,14 +581,16 @@ func removeChannelMember(c *Context, w http.ResponseWriter, r *http.Request) { return } - if channel.Type == model.CHANNEL_OPEN && !app.SessionHasPermissionToChannel(c.Session, channel.Id, model.PERMISSION_MANAGE_PUBLIC_CHANNEL_MEMBERS) { - c.SetPermissionError(model.PERMISSION_MANAGE_PUBLIC_CHANNEL_MEMBERS) - return - } + if c.Params.UserId != c.Session.UserId { + if channel.Type == model.CHANNEL_OPEN && !app.SessionHasPermissionToChannel(c.Session, channel.Id, model.PERMISSION_MANAGE_PUBLIC_CHANNEL_MEMBERS) { + c.SetPermissionError(model.PERMISSION_MANAGE_PUBLIC_CHANNEL_MEMBERS) + return + } - if channel.Type == model.CHANNEL_PRIVATE && !app.SessionHasPermissionToChannel(c.Session, channel.Id, model.PERMISSION_MANAGE_PRIVATE_CHANNEL_MEMBERS) { - c.SetPermissionError(model.PERMISSION_MANAGE_PRIVATE_CHANNEL_MEMBERS) - return + if channel.Type == model.CHANNEL_PRIVATE && !app.SessionHasPermissionToChannel(c.Session, channel.Id, model.PERMISSION_MANAGE_PRIVATE_CHANNEL_MEMBERS) { + c.SetPermissionError(model.PERMISSION_MANAGE_PRIVATE_CHANNEL_MEMBERS) + return + } } if err = app.RemoveUserFromChannel(c.Params.UserId, c.Session.UserId, channel, c.GetSiteURL()); err != nil { diff --git a/api4/channel_test.go b/api4/channel_test.go index 754413300..e8e79cebd 100644 --- a/api4/channel_test.go +++ b/api4/channel_test.go @@ -308,9 +308,24 @@ func TestGetChannel(t *testing.T) { t.Fatal("ids did not match") } - _, resp = Client.GetChannel(model.NewId(), "") + Client.RemoveUserFromChannel(th.BasicChannel.Id, th.BasicUser.Id) + _, resp = Client.GetChannel(th.BasicChannel.Id, "") + CheckNoError(t, resp) + + channel, resp = Client.GetChannel(th.BasicPrivateChannel.Id, "") + CheckNoError(t, resp) + + if channel.Id != th.BasicPrivateChannel.Id { + t.Fatal("ids did not match") + } + + Client.RemoveUserFromChannel(th.BasicPrivateChannel.Id, th.BasicUser.Id) + _, resp = Client.GetChannel(th.BasicPrivateChannel.Id, "") CheckForbiddenStatus(t, resp) + _, resp = Client.GetChannel(model.NewId(), "") + CheckNotFoundStatus(t, resp) + Client.Logout() _, resp = Client.GetChannel(th.BasicChannel.Id, "") CheckUnauthorizedStatus(t, resp) @@ -323,6 +338,9 @@ func TestGetChannel(t *testing.T) { _, resp = th.SystemAdminClient.GetChannel(th.BasicChannel.Id, "") CheckNoError(t, resp) + _, resp = th.SystemAdminClient.GetChannel(th.BasicPrivateChannel.Id, "") + CheckNoError(t, resp) + _, resp = th.SystemAdminClient.GetChannel(th.BasicUser.Id, "") CheckNotFoundStatus(t, resp) } @@ -657,9 +675,27 @@ func TestGetChannelByName(t *testing.T) { t.Fatal("names did not match") } + channel, resp = Client.GetChannelByName(th.BasicPrivateChannel.Name, th.BasicTeam.Id, "") + CheckNoError(t, resp) + + if channel.Name != th.BasicPrivateChannel.Name { + t.Fatal("names did not match") + } + + Client.RemoveUserFromChannel(th.BasicChannel.Id, th.BasicUser.Id) + _, resp = Client.GetChannelByName(th.BasicChannel.Name, th.BasicTeam.Id, "") + CheckNoError(t, resp) + + Client.RemoveUserFromChannel(th.BasicPrivateChannel.Id, th.BasicUser.Id) + _, resp = Client.GetChannelByName(th.BasicPrivateChannel.Name, th.BasicTeam.Id, "") + CheckForbiddenStatus(t, resp) + _, resp = Client.GetChannelByName(GenerateTestChannelName(), th.BasicTeam.Id, "") CheckNotFoundStatus(t, resp) + _, resp = Client.GetChannelByName(GenerateTestChannelName(), "junk", "") + CheckBadRequestStatus(t, resp) + Client.Logout() _, resp = Client.GetChannelByName(th.BasicChannel.Name, th.BasicTeam.Id, "") CheckUnauthorizedStatus(t, resp) @@ -861,8 +897,8 @@ func TestGetChannelMembersForUser(t *testing.T) { members, resp := Client.GetChannelMembersForUser(th.BasicUser.Id, th.BasicTeam.Id, "") CheckNoError(t, resp) - if len(*members) != 4 { - t.Fatal("should have 4 members on team") + if len(*members) != 5 { + t.Fatal("should have 5 members on team") } _, resp = Client.GetChannelMembersForUser("", th.BasicTeam.Id, "") @@ -1149,6 +1185,10 @@ func TestAddChannelMember(t *testing.T) { t.Fatal("should have returned exact user added to private channel") } + Client.RemoveUserFromChannel(publicChannel.Id, user.Id) + _, resp = Client.AddChannelMember(publicChannel.Id, user.Id) + CheckNoError(t, resp) + cm, resp = Client.AddChannelMember(publicChannel.Id, "junk") CheckBadRequestStatus(t, resp) @@ -1227,6 +1267,9 @@ func TestRemoveChannelMember(t *testing.T) { _, resp = Client.RemoveUserFromChannel(th.BasicChannel.Id, model.NewId()) CheckNotFoundStatus(t, resp) + _, resp = Client.RemoveUserFromChannel(model.NewId(), th.BasicUser2.Id) + CheckNotFoundStatus(t, resp) + th.LoginBasic2() _, resp = Client.RemoveUserFromChannel(th.BasicChannel.Id, th.BasicUser.Id) CheckForbiddenStatus(t, resp) @@ -1248,6 +1291,10 @@ func TestRemoveChannelMember(t *testing.T) { _, resp = Client.RemoveUserFromChannel(private.Id, th.BasicUser2.Id) CheckNoError(t, resp) + th.LoginBasic2() + _, resp = Client.RemoveUserFromChannel(private.Id, th.BasicUser.Id) + CheckForbiddenStatus(t, resp) + _, resp = th.SystemAdminClient.RemoveUserFromChannel(private.Id, th.BasicUser.Id) CheckNoError(t, resp) } -- cgit v1.2.3-1-g7c22