MM-11230: Make permissions checks in commands failsafe. (#9392)

Also add additional unit tests to make sure the permissions tests are
completely solid.

1 files changed, 30 insertions, 23 deletions

diff --git a/app/command_join.go b/app/command_join.go
index 61ed65ba6..b913014b8 100644
--- a/app/command_join.go
+++ b/app/command_join.go
@@ -4,9 +4,11 @@
 package app
 
 import (
-	"github.com/mattermost/mattermost-server/model"
-	goi18n "github.com/nicksnyder/go-i18n/i18n"
 	"strings"
+
+	goi18n "github.com/nicksnyder/go-i18n/i18n"
+
+	"github.com/mattermost/mattermost-server/model"
 )
 
 type JoinProvider struct {
@@ -41,33 +43,38 @@ func (me *JoinProvider) DoCommand(a *App, args *model.CommandArgs, message strin
 		channelName = message[1:]
 	}
 
-	if result := <-a.Srv.Store.Channel().GetByName(args.TeamId, channelName, true); result.Err != nil {
+	result := <-a.Srv.Store.Channel().GetByName(args.TeamId, channelName, true)
+	if result.Err != nil {
 		return &model.CommandResponse{Text: args.T("api.command_join.list.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL}
-	} else {
-		channel := result.Data.(*model.Channel)
+	}
 
-		if channel.Name == channelName {
-			allowed := false
-			if (channel.Type == model.CHANNEL_PRIVATE && a.SessionHasPermissionToChannel(args.Session, channel.Id, model.PERMISSION_READ_CHANNEL)) || channel.Type == model.CHANNEL_OPEN {
-				allowed = true
-			}
+	channel := result.Data.(*model.Channel)
 
-			if !allowed {
-				return &model.CommandResponse{Text: args.T("api.command_join.fail.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL}
-			}
+	if channel.Name != channelName {
+		return &model.CommandResponse{ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, Text: args.T("api.command_join.missing.app_error")}
+	}
 
-			if err := a.JoinChannel(channel, args.UserId); err != nil {
-				return &model.CommandResponse{Text: args.T("api.command_join.fail.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL}
-			}
+	switch channel.Type {
+	case model.CHANNEL_OPEN:
+		if !a.SessionHasPermissionToChannel(args.Session, channel.Id, model.PERMISSION_JOIN_PUBLIC_CHANNELS) {
+			return &model.CommandResponse{Text: args.T("api.command_join.fail.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL}
+		}
+	case model.CHANNEL_PRIVATE:
+		if !a.SessionHasPermissionToChannel(args.Session, channel.Id, model.PERMISSION_READ_CHANNEL) {
+			return &model.CommandResponse{Text: args.T("api.command_join.fail.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL}
+		}
+	default:
+		return &model.CommandResponse{Text: args.T("api.command_join.fail.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL}
+	}
 
-			team, err := a.GetTeam(channel.TeamId)
-			if err != nil {
-				return &model.CommandResponse{Text: args.T("api.command_join.fail.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL}
-			}
+	if err := a.JoinChannel(channel, args.UserId); err != nil {
+		return &model.CommandResponse{Text: args.T("api.command_join.fail.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL}
+	}
 
-			return &model.CommandResponse{GotoLocation: args.SiteURL + "/" + team.Name + "/channels/" + channel.Name}
-		}
+	team, err := a.GetTeam(channel.TeamId)
+	if err != nil {
+		return &model.CommandResponse{Text: args.T("api.command_join.fail.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL}
 	}
 
-	return &model.CommandResponse{ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL, Text: args.T("api.command_join.missing.app_error")}
+	return &model.CommandResponse{GotoLocation: args.SiteURL + "/" + team.Name + "/channels/" + channel.Name}
 }