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
2017-03-22 11:13:44 -04:00
--- 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()

--- 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 {
--- 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)
 }
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -3111,6 +3111,14 @@
    "id": "authentication.permissions.create_group_channel.name",
    "translation": "Create Group Message"
  },
+  {
+    "id": "authentication.permissions.read_public_channel.description",
+    "translation": "Ability to read public channels"
+  },
+  {
+    "id": "authentication.permissions.read_public_channel.name",
+    "translation": "Read Public Channels"
+  },
  {
    "id": "authentication.permissions.create_team_roles.description",
    "translation": "Ability to create new teams"
--- a/model/authorization.go
+++ b/model/authorization.go
@@ -39,6 +39,7 @@ var PERMISSION_DELETE_PUBLIC_CHANNEL *Permission
 var PERMISSION_DELETE_PRIVATE_CHANNEL *Permission
 var PERMISSION_EDIT_OTHER_USERS *Permission
 var PERMISSION_READ_CHANNEL *Permission
+var PERMISSION_READ_PUBLIC_CHANNEL *Permission
 var PERMISSION_PERMANENT_DELETE_USER *Permission
 var PERMISSION_UPLOAD_FILE *Permission
 var PERMISSION_GET_PUBLIC_LINK *Permission
@@ -195,6 +196,11 @@ func InitalizePermissions() {
 		"authentication.permissions.read_channel.name",
 		"authentication.permissions.read_channel.description",
 	}
+	PERMISSION_READ_PUBLIC_CHANNEL = &Permission{
+		"read_public_channel",
+		"authentication.permissions.read_public_channel.name",
+		"authentication.permissions.read_public_channel.description",
+	}
 	PERMISSION_PERMANENT_DELETE_USER = &Permission{
 		"permanent_delete_user",
 		"authentication.permissions.permanent_delete_user.name",
@@ -326,6 +332,7 @@ func InitalizeRoles() {
 		[]string{
 			PERMISSION_LIST_TEAM_CHANNELS.Id,
 			PERMISSION_JOIN_PUBLIC_CHANNELS.Id,
+			PERMISSION_READ_PUBLIC_CHANNEL.Id,
 			PERMISSION_VIEW_TEAM.Id,
 		},
 	}
@@ -378,6 +385,8 @@ func InitalizeRoles() {
 							PERMISSION_MANAGE_SYSTEM.Id,
 							PERMISSION_MANAGE_ROLES.Id,
 							PERMISSION_MANAGE_PUBLIC_CHANNEL_PROPERTIES.Id,
+							PERMISSION_MANAGE_PUBLIC_CHANNEL_MEMBERS.Id,
+							PERMISSION_MANAGE_PRIVATE_CHANNEL_MEMBERS.Id,
 							PERMISSION_DELETE_PUBLIC_CHANNEL.Id,
 							PERMISSION_CREATE_PUBLIC_CHANNEL.Id,
 							PERMISSION_MANAGE_PRIVATE_CHANNEL_PROPERTIES.Id,