Merge remote-tracking branch 'origin/master' into advanced-permissions-phase-2

14 files changed, 143 insertions, 48 deletions

diff --git a/api4/api.go b/api4/api.go
index 88526e4d3..d36c3e3ee 100644
--- a/api4/api.go
+++ b/api4/api.go
@@ -4,11 +4,12 @@
 package api4
 
 import (
+	"fmt"
 	"net/http"
 
-	l4g "github.com/alecthomas/log4go"
 	"github.com/gorilla/mux"
 	"github.com/mattermost/mattermost-server/app"
+	"github.com/mattermost/mattermost-server/mlog"
 	"github.com/mattermost/mattermost-server/model"
 	"github.com/mattermost/mattermost-server/utils"
 
@@ -243,7 +244,7 @@ func Init(a *app.App, root *mux.Router, full bool) *API {
 func Handle404(w http.ResponseWriter, r *http.Request) {
 	err := model.NewAppError("Handle404", "api.context.404.app_error", nil, "", http.StatusNotFound)
 
-	l4g.Debug("%v: code=404 ip=%v", r.URL.Path, utils.GetIpAddress(r))
+	mlog.Debug(fmt.Sprintf("%v: code=404 ip=%v", r.URL.Path, utils.GetIpAddress(r)))
 
 	w.WriteHeader(err.StatusCode)
 	err.DetailedError = "There doesn't appear to be an api call for the url='" + r.URL.Path + "'."
diff --git a/api4/api_test.go b/api4/api_test.go
index fd804b70d..2efd21f22 100644
--- a/api4/api_test.go
+++ b/api4/api_test.go
@@ -8,20 +8,29 @@ import (
 	"os"
 	"testing"
 
-	l4g "github.com/alecthomas/log4go"
-
+	"github.com/mattermost/mattermost-server/mlog"
 	"github.com/mattermost/mattermost-server/store/storetest"
 	"github.com/mattermost/mattermost-server/utils"
 )
 
 func TestMain(m *testing.M) {
 	flag.Parse()
+
+	// Setup a global logger to catch tests logging outside of app context
+	// The global logger will be stomped by apps initalizing but that's fine for testing. Ideally this won't happen.
+	mlog.InitGlobalLogger(mlog.NewLogger(&mlog.LoggerConfiguration{
+		EnableConsole: true,
+		ConsoleJson:   true,
+		ConsoleLevel:  "error",
+		EnableFile:    false,
+	}))
+
 	utils.TranslationsPreInit()
 
 	// In the case where a dev just wants to run a single test, it's faster to just use the default
 	// store.
 	if filter := flag.Lookup("test.run").Value.String(); filter != "" && filter != "." {
-		l4g.Info("-test.run used, not creating temporary containers")
+		mlog.Info("-test.run used, not creating temporary containers")
 		os.Exit(m.Run())
 	}
 
diff --git a/api4/apitestlib.go b/api4/apitestlib.go
index b56934c0a..abee5de02 100644
--- a/api4/apitestlib.go
+++ b/api4/apitestlib.go
@@ -19,8 +19,8 @@ import (
 	"testing"
 	"time"
 
-	l4g "github.com/alecthomas/log4go"
 	"github.com/mattermost/mattermost-server/app"
+	"github.com/mattermost/mattermost-server/mlog"
 	"github.com/mattermost/mattermost-server/model"
 	"github.com/mattermost/mattermost-server/store"
 	"github.com/mattermost/mattermost-server/store/sqlstore"
@@ -156,13 +156,13 @@ func (me *TestHelper) TearDown() {
 		options := map[string]bool{}
 		options[store.USER_SEARCH_OPTION_NAMES_ONLY_NO_FULL_NAME] = true
 		if result := <-me.App.Srv.Store.User().Search("", "fakeuser", options); result.Err != nil {
-			l4g.Error("Error tearing down test users")
+			mlog.Error("Error tearing down test users")
 		} else {
 			users := result.Data.([]*model.User)
 
 			for _, u := range users {
 				if err := me.App.PermanentDeleteUser(u); err != nil {
-					l4g.Error(err.Error())
+					mlog.Error(err.Error())
 				}
 			}
 		}
@@ -171,13 +171,13 @@ func (me *TestHelper) TearDown() {
 	go func() {
 		defer wg.Done()
 		if result := <-me.App.Srv.Store.Team().SearchByName("faketeam"); result.Err != nil {
-			l4g.Error("Error tearing down test teams")
+			mlog.Error("Error tearing down test teams")
 		} else {
 			teams := result.Data.([]*model.Team)
 
 			for _, t := range teams {
 				if err := me.App.PermanentDeleteTeam(t); err != nil {
-					l4g.Error(err.Error())
+					mlog.Error(err.Error())
 				}
 			}
 		}
@@ -186,7 +186,7 @@ func (me *TestHelper) TearDown() {
 	go func() {
 		defer wg.Done()
 		if result := <-me.App.Srv.Store.OAuth().GetApps(0, 1000); result.Err != nil {
-			l4g.Error("Error tearing down test oauth apps")
+			mlog.Error("Error tearing down test oauth apps")
 		} else {
 			apps := result.Data.([]*model.OAuthApp)
 
@@ -450,8 +450,8 @@ func (me *TestHelper) UpdateActiveUser(user *model.User, active bool) {
 
 	_, err := me.App.UpdateActive(user, active)
 	if err != nil {
-		l4g.Error(err.Error())
-		l4g.Close()
+		mlog.Error(err.Error())
+
 		time.Sleep(time.Second)
 		panic(err)
 	}
@@ -464,8 +464,8 @@ func (me *TestHelper) LinkUserToTeam(user *model.User, team *model.Team) {
 
 	err := me.App.JoinUserToTeam(team, user, "")
 	if err != nil {
-		l4g.Error(err.Error())
-		l4g.Close()
+		mlog.Error(err.Error())
+
 		time.Sleep(time.Second)
 		panic(err)
 	}
@@ -478,8 +478,8 @@ func (me *TestHelper) AddUserToChannel(user *model.User, channel *model.Channel)
 
 	member, err := me.App.AddUserToChannel(user, channel)
 	if err != nil {
-		l4g.Error(err.Error())
-		l4g.Close()
+		mlog.Error(err.Error())
+
 		time.Sleep(time.Second)
 		panic(err)
 	}
@@ -790,6 +790,9 @@ func (me *TestHelper) UpdateUserToTeamAdmin(user *model.User, team *model.Team)
 		}
 	} else {
 		utils.EnableDebugLogForTest()
+		mlog.Error(tmr.Err.Error())
+
+		time.Sleep(time.Second)
 		panic(tmr.Err)
 	}
 
@@ -808,6 +811,9 @@ func (me *TestHelper) UpdateUserToNonTeamAdmin(user *model.User, team *model.Tea
 		}
 	} else {
 		utils.EnableDebugLogForTest()
+		mlog.Error(tmr.Err.Error())
+
+		time.Sleep(time.Second)
 		panic(tmr.Err)
 	}
 
diff --git a/api4/channel.go b/api4/channel.go
index 685c188bc..83fa8eb18 100644
--- a/api4/channel.go
+++ b/api4/channel.go
@@ -6,8 +6,7 @@ package api4
 import (
 	"net/http"
 
-	l4g "github.com/alecthomas/log4go"
-
+	"github.com/mattermost/mattermost-server/mlog"
 	"github.com/mattermost/mattermost-server/model"
 )
 
@@ -139,7 +138,7 @@ func updateChannel(c *Context, w http.ResponseWriter, r *http.Request) {
 	} else {
 		if oldChannelDisplayName != channel.DisplayName {
 			if err := c.App.PostUpdateChannelDisplayNameMessage(c.Session.UserId, channel, oldChannelDisplayName, channel.DisplayName); err != nil {
-				l4g.Error(err.Error())
+				mlog.Error(err.Error())
 			}
 		}
 
diff --git a/api4/context.go b/api4/context.go
index 9f3822633..c965e1d80 100644
--- a/api4/context.go
+++ b/api4/context.go
@@ -10,10 +10,10 @@ import (
 	"strings"
 	"time"
 
-	l4g "github.com/alecthomas/log4go"
 	goi18n "github.com/nicksnyder/go-i18n/i18n"
 
 	"github.com/mattermost/mattermost-server/app"
+	"github.com/mattermost/mattermost-server/mlog"
 	"github.com/mattermost/mattermost-server/model"
 	"github.com/mattermost/mattermost-server/utils"
 )
@@ -90,7 +90,7 @@ type handler struct {
 
 func (h handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	now := time.Now()
-	l4g.Debug("%v - %v", r.Method, r.URL.Path)
+	mlog.Debug(fmt.Sprintf("%v - %v", r.Method, r.URL.Path))
 
 	c := &Context{}
 	c.App = h.app
@@ -124,7 +124,7 @@ func (h handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		session, err := c.App.GetSession(token)
 
 		if err != nil {
-			l4g.Info(utils.T("api.context.invalid_session.error"), err.Error())
+			mlog.Info(fmt.Sprintf("Invalid session err=%v", err.Error()))
 			if err.StatusCode == http.StatusInternalServerError {
 				c.Err = err
 			} else if h.requireSession {
@@ -220,19 +220,19 @@ func (c *Context) LogError(err *model.AppError) {
 		err.Id == "web.check_browser_compatibility.app_error" {
 		c.LogDebug(err)
 	} else {
-		l4g.Error(utils.TDefault("api.context.log.error"), c.Path, err.Where, err.StatusCode,
-			c.RequestId, c.Session.UserId, c.IpAddress, err.SystemMessage(utils.TDefault), err.DetailedError)
+		mlog.Error(fmt.Sprintf("%v:%v code=%v rid=%v uid=%v ip=%v %v [details: %v]", c.Path, err.Where, err.StatusCode,
+			c.RequestId, c.Session.UserId, c.IpAddress, err.SystemMessage(utils.TDefault), err.DetailedError), mlog.String("user_id", c.Session.UserId))
 	}
 }
 
 func (c *Context) LogInfo(err *model.AppError) {
-	l4g.Info(utils.TDefault("api.context.log.error"), c.Path, err.Where, err.StatusCode,
-		c.RequestId, c.Session.UserId, c.IpAddress, err.SystemMessage(utils.TDefault), err.DetailedError)
+	mlog.Info(fmt.Sprintf("%v:%v code=%v rid=%v uid=%v ip=%v %v [details: %v]", c.Path, err.Where, err.StatusCode,
+		c.RequestId, c.Session.UserId, c.IpAddress, err.SystemMessage(utils.TDefault), err.DetailedError), mlog.String("user_id", c.Session.UserId))
 }
 
 func (c *Context) LogDebug(err *model.AppError) {
-	l4g.Debug(utils.TDefault("api.context.log.error"), c.Path, err.Where, err.StatusCode,
-		c.RequestId, c.Session.UserId, c.IpAddress, err.SystemMessage(utils.TDefault), err.DetailedError)
+	mlog.Debug(fmt.Sprintf("%v:%v code=%v rid=%v uid=%v ip=%v %v [details: %v]", c.Path, err.Where, err.StatusCode,
+		c.RequestId, c.Session.UserId, c.IpAddress, err.SystemMessage(utils.TDefault), err.DetailedError), mlog.String("user_id", c.Session.UserId))
 }
 
 func (c *Context) IsSystemAdmin() bool {
diff --git a/api4/oauth.go b/api4/oauth.go
index a173159b6..fa120ebbf 100644
--- a/api4/oauth.go
+++ b/api4/oauth.go
@@ -9,8 +9,8 @@ import (
 	"path/filepath"
 	"strings"
 
-	l4g "github.com/alecthomas/log4go"
 	"github.com/mattermost/mattermost-server/app"
+	"github.com/mattermost/mattermost-server/mlog"
 	"github.com/mattermost/mattermost-server/model"
 	"github.com/mattermost/mattermost-server/utils"
 )
@@ -463,7 +463,7 @@ func completeOAuth(c *Context, w http.ResponseWriter, r *http.Request) {
 
 	if err != nil {
 		err.Translate(c.T)
-		l4g.Error(err.Error())
+		mlog.Error(err.Error())
 		if action == model.OAUTH_ACTION_MOBILE {
 			w.Write([]byte(err.ToJson()))
 		} else {
@@ -475,7 +475,7 @@ func completeOAuth(c *Context, w http.ResponseWriter, r *http.Request) {
 	user, err := c.App.CompleteOAuth(service, body, teamId, props)
 	if err != nil {
 		err.Translate(c.T)
-		l4g.Error(err.Error())
+		mlog.Error(err.Error())
 		if action == model.OAUTH_ACTION_MOBILE {
 			w.Write([]byte(err.ToJson()))
 		} else {
diff --git a/api4/plugin.go b/api4/plugin.go
index ee7121ffb..37fbf12cd 100644
--- a/api4/plugin.go
+++ b/api4/plugin.go
@@ -8,7 +8,7 @@ package api4
 import (
 	"net/http"
 
-	l4g "github.com/alecthomas/log4go"
+	"github.com/mattermost/mattermost-server/mlog"
 	"github.com/mattermost/mattermost-server/model"
 )
 
@@ -17,7 +17,7 @@ const (
 )
 
 func (api *API) InitPlugin() {
-	l4g.Debug("EXPERIMENTAL: Initializing plugin api")
+	mlog.Debug("EXPERIMENTAL: Initializing plugin api")
 
 	api.BaseRoutes.Plugins.Handle("", api.ApiSessionRequired(uploadPlugin)).Methods("POST")
 	api.BaseRoutes.Plugins.Handle("", api.ApiSessionRequired(getPlugins)).Methods("GET")
diff --git a/api4/system.go b/api4/system.go
index c307a39b7..acb02bc3e 100644
--- a/api4/system.go
+++ b/api4/system.go
@@ -5,11 +5,12 @@ package api4
 
 import (
 	"bytes"
+	"fmt"
 	"io"
 	"net/http"
 	"runtime"
 
-	l4g "github.com/alecthomas/log4go"
+	"github.com/mattermost/mattermost-server/mlog"
 	"github.com/mattermost/mattermost-server/model"
 	"github.com/mattermost/mattermost-server/utils"
 )
@@ -61,7 +62,7 @@ func getSystemPing(c *Context, w http.ResponseWriter, r *http.Request) {
 		rdata := map[string]string{}
 		rdata["status"] = "unhealthy"
 
-		l4g.Warn(utils.T("api.system.go_routines"), actualGoroutines, *c.App.Config().ServiceSettings.GoroutineHealthThreshold)
+		mlog.Warn(fmt.Sprintf("The number of running goroutines is over the health threshold %v of %v", actualGoroutines, *c.App.Config().ServiceSettings.GoroutineHealthThreshold))
 
 		w.WriteHeader(http.StatusInternalServerError)
 		w.Write([]byte(model.MapToJson(rdata)))
@@ -229,7 +230,7 @@ func postLog(c *Context, w http.ResponseWriter, r *http.Request) {
 		err.Where = "client"
 		c.LogError(err)
 	} else {
-		l4g.Debug(msg)
+		mlog.Debug(fmt.Sprint(msg))
 	}
 
 	m["message"] = msg
diff --git a/api4/system_test.go b/api4/system_test.go
index f74d91563..c0fde6c39 100644
--- a/api4/system_test.go
+++ b/api4/system_test.go
@@ -7,7 +7,7 @@ import (
 	"strings"
 	"testing"
 
-	l4g "github.com/alecthomas/log4go"
+	"github.com/mattermost/mattermost-server/mlog"
 	"github.com/mattermost/mattermost-server/model"
 	"github.com/stretchr/testify/assert"
 )
@@ -392,7 +392,7 @@ func TestGetLogs(t *testing.T) {
 	Client := th.Client
 
 	for i := 0; i < 20; i++ {
-		l4g.Info(i)
+		mlog.Info(fmt.Sprint(i))
 	}
 
 	logs, resp := th.SystemAdminClient.GetLogs(0, 10)
@@ -534,6 +534,28 @@ func TestGetAnalyticsOld(t *testing.T) {
 	_, resp = th.SystemAdminClient.GetAnalyticsOld("", th.BasicTeam.Id)
 	CheckNoError(t, resp)
 
+	rows2, resp2 := th.SystemAdminClient.GetAnalyticsOld("standard", "")
+	CheckNoError(t, resp2)
+	assert.Equal(t, "total_websocket_connections", rows2[5].Name)
+	assert.Equal(t, float64(0), rows2[5].Value)
+
+	WebSocketClient, err := th.CreateWebSocketClient()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	rows2, resp2 = th.SystemAdminClient.GetAnalyticsOld("standard", "")
+	CheckNoError(t, resp2)
+	assert.Equal(t, "total_websocket_connections", rows2[5].Name)
+	assert.Equal(t, float64(1), rows2[5].Value)
+
+	WebSocketClient.Close()
+
+	rows2, resp2 = th.SystemAdminClient.GetAnalyticsOld("standard", "")
+	CheckNoError(t, resp2)
+	assert.Equal(t, "total_websocket_connections", rows2[5].Name)
+	assert.Equal(t, float64(0), rows2[5].Value)
+
 	Client.Logout()
 	_, resp = Client.GetAnalyticsOld("", th.BasicTeam.Id)
 	CheckUnauthorizedStatus(t, resp)
diff --git a/api4/team.go b/api4/team.go
index 94035a770..023289579 100644
--- a/api4/team.go
+++ b/api4/team.go
@@ -32,6 +32,7 @@ func (api *API) InitTeam() {
 
 	api.BaseRoutes.Team.Handle("/image", api.ApiSessionRequiredTrustRequester(getTeamIcon)).Methods("GET")
 	api.BaseRoutes.Team.Handle("/image", api.ApiSessionRequired(setTeamIcon)).Methods("POST")
+	api.BaseRoutes.Team.Handle("/image", api.ApiSessionRequired(removeTeamIcon)).Methods("DELETE")
 
 	api.BaseRoutes.TeamMembers.Handle("", api.ApiSessionRequired(getTeamMembers)).Methods("GET")
 	api.BaseRoutes.TeamMembers.Handle("/ids", api.ApiSessionRequired(getTeamMembersByIds)).Methods("POST")
@@ -812,3 +813,23 @@ func setTeamIcon(c *Context, w http.ResponseWriter, r *http.Request) {
 	c.LogAudit("")
 	ReturnStatusOK(w)
 }
+
+func removeTeamIcon(c *Context, w http.ResponseWriter, r *http.Request) {
+	c.RequireTeamId()
+	if c.Err != nil {
+		return
+	}
+
+	if !c.App.SessionHasPermissionToTeam(c.Session, c.Params.TeamId, model.PERMISSION_MANAGE_TEAM) {
+		c.SetPermissionError(model.PERMISSION_MANAGE_TEAM)
+		return
+	}
+
+	if err := c.App.RemoveTeamIcon(c.Params.TeamId); err != nil {
+		c.Err = err
+		return
+	}
+
+	c.LogAudit("")
+	ReturnStatusOK(w)
+}
diff --git a/api4/team_test.go b/api4/team_test.go
index 3cd9d7d93..6540457b0 100644
--- a/api4/team_test.go
+++ b/api4/team_test.go
@@ -2015,3 +2015,40 @@ func TestGetTeamIcon(t *testing.T) {
 	_, resp = Client.GetTeamIcon(team.Id, "")
 	CheckUnauthorizedStatus(t, resp)
 }
+
+func TestRemoveTeamIcon(t *testing.T) {
+	th := Setup().InitBasic().InitSystemAdmin()
+	defer th.TearDown()
+	Client := th.Client
+	team := th.BasicTeam
+
+	th.LoginTeamAdmin()
+	data, _ := readTestFile("test.png")
+	Client.SetTeamIcon(team.Id, data)
+
+	_, resp := Client.RemoveTeamIcon(team.Id)
+	CheckNoError(t, resp)
+	teamAfter, _ := th.App.GetTeam(team.Id)
+	if teamAfter.LastTeamIconUpdate != 0 {
+		t.Fatal("should update LastTeamIconUpdate to 0")
+	}
+
+	Client.SetTeamIcon(team.Id, data)
+
+	_, resp = th.SystemAdminClient.RemoveTeamIcon(team.Id)
+	CheckNoError(t, resp)
+	teamAfter, _ = th.App.GetTeam(team.Id)
+	if teamAfter.LastTeamIconUpdate != 0 {
+		t.Fatal("should update LastTeamIconUpdate to 0")
+	}
+
+	Client.SetTeamIcon(team.Id, data)
+	Client.Logout()
+
+	_, resp = Client.RemoveTeamIcon(team.Id)
+	CheckUnauthorizedStatus(t, resp)
+
+	th.LoginBasic()
+	_, resp = Client.RemoveTeamIcon(team.Id)
+	CheckForbiddenStatus(t, resp)
+}
diff --git a/api4/user.go b/api4/user.go
index e13bf9448..897c49ad1 100644
--- a/api4/user.go
+++ b/api4/user.go
@@ -9,8 +9,8 @@ import (
 	"strconv"
 	"time"
 
-	l4g "github.com/alecthomas/log4go"
 	"github.com/mattermost/mattermost-server/app"
+	"github.com/mattermost/mattermost-server/mlog"
 	"github.com/mattermost/mattermost-server/model"
 	"github.com/mattermost/mattermost-server/store"
 )
@@ -1177,7 +1177,7 @@ func sendVerificationEmail(c *Context, w http.ResponseWriter, r *http.Request) {
 	err = c.App.SendEmailVerification(user)
 	if err != nil {
 		// Don't want to leak whether the email is valid or not
-		l4g.Error(err.Error())
+		mlog.Error(err.Error())
 		ReturnStatusOK(w)
 		return
 	}
diff --git a/api4/webhook.go b/api4/webhook.go
index a0e7b5785..fadc3fbf3 100644
--- a/api4/webhook.go
+++ b/api4/webhook.go
@@ -4,16 +4,15 @@
 package api4
 
 import (
+	"fmt"
 	"io"
 	"net/http"
 	"strings"
 
-	l4g "github.com/alecthomas/log4go"
-
 	"github.com/gorilla/mux"
 	"github.com/gorilla/schema"
+	"github.com/mattermost/mattermost-server/mlog"
 	"github.com/mattermost/mattermost-server/model"
-	"github.com/mattermost/mattermost-server/utils"
 )
 
 func (api *API) InitWebhook() {
@@ -492,7 +491,7 @@ func incomingWebhook(c *Context, w http.ResponseWriter, r *http.Request) {
 	}
 
 	if c.App.Config().LogSettings.EnableWebhookDebugging {
-		l4g.Debug(utils.T("api.webhook.incoming.debug"), incomingWebhookPayload.ToJson())
+		mlog.Debug(fmt.Sprint("Incoming webhook received. Content=", incomingWebhookPayload.ToJson()))
 	}
 
 	err = c.App.HandleIncomingWebhook(id, incomingWebhookPayload)
diff --git a/api4/websocket.go b/api4/websocket.go
index 7ea19224b..68125621a 100644
--- a/api4/websocket.go
+++ b/api4/websocket.go
@@ -4,12 +4,12 @@
 package api4
 
 import (
+	"fmt"
 	"net/http"
 
-	l4g "github.com/alecthomas/log4go"
 	"github.com/gorilla/websocket"
+	"github.com/mattermost/mattermost-server/mlog"
 	"github.com/mattermost/mattermost-server/model"
-	"github.com/mattermost/mattermost-server/utils"
 )
 
 func (api *API) InitWebSocket() {
@@ -25,7 +25,7 @@ func connectWebSocket(c *Context, w http.ResponseWriter, r *http.Request) {
 
 	ws, err := upgrader.Upgrade(w, r, nil)
 	if err != nil {
-		l4g.Error(utils.T("api.web_socket.connect.error"), err)
+		mlog.Error(fmt.Sprintf("websocket connect err: %v", err))
 		c.Err = model.NewAppError("connect", "api.web_socket.connect.upgrade.app_error", nil, "", http.StatusInternalServerError)
 		return
 	}