Merge branch 'master' into advanced-permissions-phase-1

10 files changed, 492 insertions, 50 deletions

diff --git a/api4/apitestlib.go b/api4/apitestlib.go
index ac89a1f71..a788d4311 100644
--- a/api4/apitestlib.go
+++ b/api4/apitestlib.go
@@ -12,7 +12,6 @@ import (
 	"net/http"
 	"os"
 	"reflect"
-	"runtime/debug"
 	"strconv"
 	"strings"
 	"sync"
@@ -497,6 +496,8 @@ func GenerateTestId() string {
 }
 
 func CheckUserSanitization(t *testing.T, user *model.User) {
+	t.Helper()
+
 	if user.Password != "" {
 		t.Fatal("password wasn't blank")
 	}
@@ -511,6 +512,8 @@ func CheckUserSanitization(t *testing.T, user *model.User) {
 }
 
 func CheckTeamSanitization(t *testing.T, team *model.Team) {
+	t.Helper()
+
 	if team.Email != "" {
 		t.Fatal("email wasn't blank")
 	}
@@ -521,13 +524,13 @@ func CheckTeamSanitization(t *testing.T, team *model.Team) {
 }
 
 func CheckEtag(t *testing.T, data interface{}, resp *model.Response) {
+	t.Helper()
+
 	if !reflect.ValueOf(data).IsNil() {
-		debug.PrintStack()
 		t.Fatal("etag data was not nil")
 	}
 
 	if resp.StatusCode != http.StatusNotModified {
-		debug.PrintStack()
 		t.Log("actual: " + strconv.Itoa(resp.StatusCode))
 		t.Log("expected: " + strconv.Itoa(http.StatusNotModified))
 		t.Fatal("wrong status code for etag")
@@ -535,15 +538,17 @@ func CheckEtag(t *testing.T, data interface{}, resp *model.Response) {
 }
 
 func CheckNoError(t *testing.T, resp *model.Response) {
+	t.Helper()
+
 	if resp.Error != nil {
-		debug.PrintStack()
 		t.Fatal("Expected no error, got " + resp.Error.Error())
 	}
 }
 
 func CheckCreatedStatus(t *testing.T, resp *model.Response) {
+	t.Helper()
+
 	if resp.StatusCode != http.StatusCreated {
-		debug.PrintStack()
 		t.Log("actual: " + strconv.Itoa(resp.StatusCode))
 		t.Log("expected: " + strconv.Itoa(http.StatusCreated))
 		t.Fatal("wrong status code")
@@ -551,14 +556,14 @@ func CheckCreatedStatus(t *testing.T, resp *model.Response) {
 }
 
 func CheckForbiddenStatus(t *testing.T, resp *model.Response) {
+	t.Helper()
+
 	if resp.Error == nil {
-		debug.PrintStack()
 		t.Fatal("should have errored with status:" + strconv.Itoa(http.StatusForbidden))
 		return
 	}
 
 	if resp.StatusCode != http.StatusForbidden {
-		debug.PrintStack()
 		t.Log("actual: " + strconv.Itoa(resp.StatusCode))
 		t.Log("expected: " + strconv.Itoa(http.StatusForbidden))
 		t.Fatal("wrong status code")
@@ -566,14 +571,14 @@ func CheckForbiddenStatus(t *testing.T, resp *model.Response) {
 }
 
 func CheckUnauthorizedStatus(t *testing.T, resp *model.Response) {
+	t.Helper()
+
 	if resp.Error == nil {
-		debug.PrintStack()
 		t.Fatal("should have errored with status:" + strconv.Itoa(http.StatusUnauthorized))
 		return
 	}
 
 	if resp.StatusCode != http.StatusUnauthorized {
-		debug.PrintStack()
 		t.Log("actual: " + strconv.Itoa(resp.StatusCode))
 		t.Log("expected: " + strconv.Itoa(http.StatusUnauthorized))
 		t.Fatal("wrong status code")
@@ -581,14 +586,14 @@ func CheckUnauthorizedStatus(t *testing.T, resp *model.Response) {
 }
 
 func CheckNotFoundStatus(t *testing.T, resp *model.Response) {
+	t.Helper()
+
 	if resp.Error == nil {
-		debug.PrintStack()
 		t.Fatal("should have errored with status:" + strconv.Itoa(http.StatusNotFound))
 		return
 	}
 
 	if resp.StatusCode != http.StatusNotFound {
-		debug.PrintStack()
 		t.Log("actual: " + strconv.Itoa(resp.StatusCode))
 		t.Log("expected: " + strconv.Itoa(http.StatusNotFound))
 		t.Fatal("wrong status code")
@@ -596,14 +601,14 @@ func CheckNotFoundStatus(t *testing.T, resp *model.Response) {
 }
 
 func CheckBadRequestStatus(t *testing.T, resp *model.Response) {
+	t.Helper()
+
 	if resp.Error == nil {
-		debug.PrintStack()
 		t.Fatal("should have errored with status:" + strconv.Itoa(http.StatusBadRequest))
 		return
 	}
 
 	if resp.StatusCode != http.StatusBadRequest {
-		debug.PrintStack()
 		t.Log("actual: " + strconv.Itoa(resp.StatusCode))
 		t.Log("expected: " + strconv.Itoa(http.StatusBadRequest))
 		t.Fatal("wrong status code")
@@ -611,14 +616,14 @@ func CheckBadRequestStatus(t *testing.T, resp *model.Response) {
 }
 
 func CheckNotImplementedStatus(t *testing.T, resp *model.Response) {
+	t.Helper()
+
 	if resp.Error == nil {
-		debug.PrintStack()
 		t.Fatal("should have errored with status:" + strconv.Itoa(http.StatusNotImplemented))
 		return
 	}
 
 	if resp.StatusCode != http.StatusNotImplemented {
-		debug.PrintStack()
 		t.Log("actual: " + strconv.Itoa(resp.StatusCode))
 		t.Log("expected: " + strconv.Itoa(http.StatusNotImplemented))
 		t.Fatal("wrong status code")
@@ -626,6 +631,8 @@ func CheckNotImplementedStatus(t *testing.T, resp *model.Response) {
 }
 
 func CheckOKStatus(t *testing.T, resp *model.Response) {
+	t.Helper()
+
 	CheckNoError(t, resp)
 
 	if resp.StatusCode != http.StatusOK {
@@ -634,14 +641,14 @@ func CheckOKStatus(t *testing.T, resp *model.Response) {
 }
 
 func CheckErrorMessage(t *testing.T, resp *model.Response, errorId string) {
+	t.Helper()
+
 	if resp.Error == nil {
-		debug.PrintStack()
 		t.Fatal("should have errored with message:" + errorId)
 		return
 	}
 
 	if resp.Error.Id != errorId {
-		debug.PrintStack()
 		t.Log("actual: " + resp.Error.Id)
 		t.Log("expected: " + errorId)
 		t.Fatal("incorrect error message")
@@ -649,14 +656,14 @@ func CheckErrorMessage(t *testing.T, resp *model.Response, errorId string) {
 }
 
 func CheckInternalErrorStatus(t *testing.T, resp *model.Response) {
+	t.Helper()
+
 	if resp.Error == nil {
-		debug.PrintStack()
 		t.Fatal("should have errored with status:" + strconv.Itoa(http.StatusInternalServerError))
 		return
 	}
 
 	if resp.StatusCode != http.StatusInternalServerError {
-		debug.PrintStack()
 		t.Log("actual: " + strconv.Itoa(resp.StatusCode))
 		t.Log("expected: " + strconv.Itoa(http.StatusInternalServerError))
 		t.Fatal("wrong status code")
@@ -664,14 +671,14 @@ func CheckInternalErrorStatus(t *testing.T, resp *model.Response) {
 }
 
 func CheckPayLoadTooLargeStatus(t *testing.T, resp *model.Response) {
+	t.Helper()
+
 	if resp.Error == nil {
-		debug.PrintStack()
 		t.Fatal("should have errored with status:" + strconv.Itoa(http.StatusRequestEntityTooLarge))
 		return
 	}
 
 	if resp.StatusCode != http.StatusRequestEntityTooLarge {
-		debug.PrintStack()
 		t.Log("actual: " + strconv.Itoa(resp.StatusCode))
 		t.Log("expected: " + strconv.Itoa(http.StatusRequestEntityTooLarge))
 		t.Fatal("wrong status code")
diff --git a/api4/context.go b/api4/context.go
index 19778dda3..62fe55758 100644
--- a/api4/context.go
+++ b/api4/context.go
@@ -212,8 +212,10 @@ func (c *Context) LogAuditWithUserId(userId, extraInfo string) {
 
 func (c *Context) LogError(err *model.AppError) {
 
-	// filter out endless reconnects
-	if c.Path == "/api/v3/users/websocket" && err.StatusCode == 401 || err.Id == "web.check_browser_compatibility.app_error" {
+	// Filter out 404s, endless reconnects and browser compatibility errors
+	if err.StatusCode == http.StatusNotFound ||
+		(c.Path == "/api/v3/users/websocket" && err.StatusCode == 401) ||
+		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,
@@ -447,6 +449,18 @@ func (c *Context) RequireFileId() *Context {
 	return c
 }
 
+func (c *Context) RequireFilename() *Context {
+	if c.Err != nil {
+		return c
+	}
+
+	if len(c.Params.Filename) == 0 {
+		c.SetInvalidUrlParam("filename")
+	}
+
+	return c
+}
+
 func (c *Context) RequirePluginId() *Context {
 	if c.Err != nil {
 		return c
diff --git a/api4/file.go b/api4/file.go
index acc4c78e5..0b0973b30 100644
--- a/api4/file.go
+++ b/api4/file.go
@@ -4,6 +4,7 @@
 package api4
 
 import (
+	"io"
 	"net/http"
 	"net/url"
 	"strconv"
@@ -65,32 +66,62 @@ func uploadFile(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if err := r.ParseMultipartForm(*c.App.Config().FileSettings.MaxFileSize); err != nil {
+	var resStruct *model.FileUploadResponse
+	var appErr *model.AppError
+
+	if err := r.ParseMultipartForm(*c.App.Config().FileSettings.MaxFileSize); err != nil && err != http.ErrNotMultipart {
 		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
-	}
+	} else if err == http.ErrNotMultipart {
+		defer r.Body.Close()
 
-	m := r.MultipartForm
+		c.RequireChannelId()
+		c.RequireFilename()
 
-	props := m.Value
-	if len(props["channel_id"]) == 0 {
-		c.SetInvalidParam("channel_id")
-		return
-	}
-	channelId := props["channel_id"][0]
-	if len(channelId) == 0 {
-		c.SetInvalidParam("channel_id")
-		return
-	}
+		if c.Err != nil {
+			return
+		}
 
-	if !c.App.SessionHasPermissionToChannel(c.Session, channelId, model.PERMISSION_UPLOAD_FILE) {
-		c.SetPermissionError(model.PERMISSION_UPLOAD_FILE)
-		return
+		channelId := c.Params.ChannelId
+		filename := c.Params.Filename
+
+		if !c.App.SessionHasPermissionToChannel(c.Session, channelId, model.PERMISSION_UPLOAD_FILE) {
+			c.SetPermissionError(model.PERMISSION_UPLOAD_FILE)
+			return
+		}
+
+		resStruct, appErr = c.App.UploadFiles(
+			FILE_TEAM_ID,
+			channelId,
+			c.Session.UserId,
+			[]io.ReadCloser{r.Body},
+			[]string{filename},
+			[]string{},
+		)
+	} else {
+		m := r.MultipartForm
+
+		props := m.Value
+		if len(props["channel_id"]) == 0 {
+			c.SetInvalidParam("channel_id")
+			return
+		}
+		channelId := props["channel_id"][0]
+		if len(channelId) == 0 {
+			c.SetInvalidParam("channel_id")
+			return
+		}
+
+		if !c.App.SessionHasPermissionToChannel(c.Session, channelId, model.PERMISSION_UPLOAD_FILE) {
+			c.SetPermissionError(model.PERMISSION_UPLOAD_FILE)
+			return
+		}
+
+		resStruct, appErr = c.App.UploadMultipartFiles(FILE_TEAM_ID, channelId, c.Session.UserId, m.File["files"], m.Value["client_ids"])
 	}
 
-	resStruct, err := c.App.UploadFiles(FILE_TEAM_ID, channelId, c.Session.UserId, m.File["files"], m.Value["client_ids"])
-	if err != nil {
-		c.Err = err
+	if appErr != nil {
+		c.Err = appErr
 		return
 	}
 
diff --git a/api4/file_test.go b/api4/file_test.go
index 7010b3039..a28420c76 100644
--- a/api4/file_test.go
+++ b/api4/file_test.go
@@ -14,7 +14,7 @@ import (
 	"github.com/mattermost/mattermost-server/store"
 )
 
-func TestUploadFile(t *testing.T) {
+func TestUploadFileAsMultipart(t *testing.T) {
 	th := Setup().InitBasic().InitSystemAdmin()
 	defer th.TearDown()
 	Client := th.Client
@@ -119,7 +119,132 @@ func TestUploadFile(t *testing.T) {
 	th.App.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.EnableFileAttachments = false })
 
 	_, resp = th.SystemAdminClient.UploadFile(data, channel.Id, "test.png")
-	if resp.StatusCode != http.StatusNotImplemented && resp.StatusCode != 0 {
+	if resp.StatusCode == 0 {
+		t.Log("file upload request failed completely")
+	} else if resp.StatusCode != http.StatusNotImplemented {
+		// This should return an HTTP 501, but it occasionally causes the http client itself to error
+		t.Fatalf("should've returned HTTP 501 or failed completely, got %v instead", resp.StatusCode)
+	}
+}
+
+func TestUploadFileAsRequestBody(t *testing.T) {
+	th := Setup().InitBasic().InitSystemAdmin()
+	defer th.TearDown()
+	Client := th.Client
+
+	user := th.BasicUser
+	channel := th.BasicChannel
+
+	var uploadInfo *model.FileInfo
+	var data []byte
+	var err error
+	if data, err = readTestFile("test.png"); err != nil {
+		t.Fatal(err)
+	} else if fileResp, resp := Client.UploadFileAsRequestBody(data, channel.Id, "test.png"); resp.Error != nil {
+		t.Fatal(resp.Error)
+	} else if len(fileResp.FileInfos) != 1 {
+		t.Fatal("should've returned a single file infos")
+	} else {
+		uploadInfo = fileResp.FileInfos[0]
+	}
+
+	// The returned file info from the upload call will be missing some fields that will be stored in the database
+	if uploadInfo.CreatorId != user.Id {
+		t.Fatal("file should be assigned to user")
+	} else if uploadInfo.PostId != "" {
+		t.Fatal("file shouldn't have a post")
+	} else if uploadInfo.Path != "" {
+		t.Fatal("file path should not be set on returned info")
+	} else if uploadInfo.ThumbnailPath != "" {
+		t.Fatal("file thumbnail path should not be set on returned info")
+	} else if uploadInfo.PreviewPath != "" {
+		t.Fatal("file preview path should not be set on returned info")
+	}
+
+	var info *model.FileInfo
+	if result := <-th.App.Srv.Store.FileInfo().Get(uploadInfo.Id); result.Err != nil {
+		t.Fatal(result.Err)
+	} else {
+		info = result.Data.(*model.FileInfo)
+	}
+
+	if info.Id != uploadInfo.Id {
+		t.Fatal("file id from response should match one stored in database")
+	} else if info.CreatorId != user.Id {
+		t.Fatal("file should be assigned to user")
+	} else if info.PostId != "" {
+		t.Fatal("file shouldn't have a post")
+	} else if info.Path == "" {
+		t.Fatal("file path should be set in database")
+	} else if info.ThumbnailPath == "" {
+		t.Fatal("file thumbnail path should be set in database")
+	} else if info.PreviewPath == "" {
+		t.Fatal("file preview path should be set in database")
+	}
+
+	date := time.Now().Format("20060102")
+
+	// This also makes sure that the relative path provided above is sanitized out
+	expectedPath := fmt.Sprintf("%v/teams/%v/channels/%v/users/%v/%v/test.png", date, FILE_TEAM_ID, channel.Id, user.Id, info.Id)
+	if info.Path != expectedPath {
+		t.Logf("file is saved in %v", info.Path)
+		t.Fatalf("file should've been saved in %v", expectedPath)
+	}
+
+	expectedThumbnailPath := fmt.Sprintf("%v/teams/%v/channels/%v/users/%v/%v/test_thumb.jpg", date, FILE_TEAM_ID, channel.Id, user.Id, info.Id)
+	if info.ThumbnailPath != expectedThumbnailPath {
+		t.Logf("file thumbnail is saved in %v", info.ThumbnailPath)
+		t.Fatalf("file thumbnail should've been saved in %v", expectedThumbnailPath)
+	}
+
+	expectedPreviewPath := fmt.Sprintf("%v/teams/%v/channels/%v/users/%v/%v/test_preview.jpg", date, FILE_TEAM_ID, channel.Id, user.Id, info.Id)
+	if info.PreviewPath != expectedPreviewPath {
+		t.Logf("file preview is saved in %v", info.PreviewPath)
+		t.Fatalf("file preview should've been saved in %v", expectedPreviewPath)
+	}
+
+	// Wait a bit for files to ready
+	time.Sleep(2 * time.Second)
+
+	if err := th.cleanupTestFile(info); err != nil {
+		t.Fatal(err)
+	}
+
+	_, resp := Client.UploadFileAsRequestBody(data, model.NewId(), "test.png")
+	CheckForbiddenStatus(t, resp)
+
+	_, resp = Client.UploadFileAsRequestBody(data, "../../junk", "test.png")
+	if resp.StatusCode == 0 {
+		t.Log("file upload request failed completely")
+	} else if resp.StatusCode != http.StatusBadRequest {
+		// This should return an HTTP 400, but it occasionally causes the http client itself to error
+		t.Fatalf("should've returned HTTP 400 or failed completely, got %v instead", resp.StatusCode)
+	}
+
+	_, resp = th.SystemAdminClient.UploadFileAsRequestBody(data, model.NewId(), "test.png")
+	CheckForbiddenStatus(t, resp)
+
+	_, resp = th.SystemAdminClient.UploadFileAsRequestBody(data, "../../junk", "test.png")
+	if resp.StatusCode == 0 {
+		t.Log("file upload request failed completely")
+	} else if resp.StatusCode != http.StatusBadRequest {
+		// This should return an HTTP 400, but it occasionally causes the http client itself to error
+		t.Fatalf("should've returned HTTP 400 or failed completely, got %v instead", resp.StatusCode)
+	}
+
+	_, resp = th.SystemAdminClient.UploadFileAsRequestBody(data, channel.Id, "test.png")
+	CheckNoError(t, resp)
+
+	enableFileAttachments := *th.App.Config().FileSettings.EnableFileAttachments
+	defer func() {
+		th.App.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.EnableFileAttachments = enableFileAttachments })
+	}()
+	th.App.UpdateConfig(func(cfg *model.Config) { *cfg.FileSettings.EnableFileAttachments = false })
+
+	_, resp = th.SystemAdminClient.UploadFileAsRequestBody(data, channel.Id, "test.png")
+	if resp.StatusCode == 0 {
+		t.Log("file upload request failed completely")
+	} else if resp.StatusCode != http.StatusNotImplemented {
 		// This should return an HTTP 501, but it occasionally causes the http client itself to error
 		t.Fatalf("should've returned HTTP 501 or failed completely, got %v instead", resp.StatusCode)
 	}
diff --git a/api4/params.go b/api4/params.go
index 357036e80..e8e3f25e7 100644
--- a/api4/params.go
+++ b/api4/params.go
@@ -27,6 +27,7 @@ type ApiParams struct {
 	ChannelId      string
 	PostId         string
 	FileId         string
+	Filename       string
 	PluginId       string
 	CommandId      string
 	HookId         string
@@ -56,6 +57,7 @@ func ApiParamsFromRequest(r *http.Request) *ApiParams {
 	params := &ApiParams{}
 
 	props := mux.Vars(r)
+	query := r.URL.Query()
 
 	if val, ok := props["user_id"]; ok {
 		params.UserId = val
@@ -75,6 +77,8 @@ func ApiParamsFromRequest(r *http.Request) *ApiParams {
 
 	if val, ok := props["channel_id"]; ok {
 		params.ChannelId = val
+	} else {
+		params.ChannelId = query.Get("channel_id")
 	}
 
 	if val, ok := props["post_id"]; ok {
@@ -85,6 +89,8 @@ func ApiParamsFromRequest(r *http.Request) *ApiParams {
 		params.FileId = val
 	}
 
+	params.Filename = query.Get("filename")
+
 	if val, ok := props["plugin_id"]; ok {
 		params.PluginId = val
 	}
@@ -161,17 +167,17 @@ func ApiParamsFromRequest(r *http.Request) *ApiParams {
 		params.RoleName = val
 	}
 
-	if val, err := strconv.Atoi(r.URL.Query().Get("page")); err != nil || val < 0 {
+	if val, err := strconv.Atoi(query.Get("page")); err != nil || val < 0 {
 		params.Page = PAGE_DEFAULT
 	} else {
 		params.Page = val
 	}
 
-	if val, err := strconv.ParseBool(r.URL.Query().Get("permanent")); err != nil {
+	if val, err := strconv.ParseBool(query.Get("permanent")); err != nil {
 		params.Permanent = val
 	}
 
-	if val, err := strconv.Atoi(r.URL.Query().Get("per_page")); err != nil || val < 0 {
+	if val, err := strconv.Atoi(query.Get("per_page")); err != nil || val < 0 {
 		params.PerPage = PER_PAGE_DEFAULT
 	} else if val > PER_PAGE_MAXIMUM {
 		params.PerPage = PER_PAGE_MAXIMUM
@@ -179,7 +185,7 @@ func ApiParamsFromRequest(r *http.Request) *ApiParams {
 		params.PerPage = val
 	}
 
-	if val, err := strconv.Atoi(r.URL.Query().Get("logs_per_page")); err != nil || val < 0 {
+	if val, err := strconv.Atoi(query.Get("logs_per_page")); err != nil || val < 0 {
 		params.LogsPerPage = LOGS_PER_PAGE_DEFAULT
 	} else if val > LOGS_PER_PAGE_MAXIMUM {
 		params.LogsPerPage = LOGS_PER_PAGE_MAXIMUM
diff --git a/api4/system.go b/api4/system.go
index 2355cb476..aab65bf20 100644
--- a/api4/system.go
+++ b/api4/system.go
@@ -29,6 +29,7 @@ func (api *API) InitSystem() {
 
 	api.BaseRoutes.ApiRoot.Handle("/audits", api.ApiSessionRequired(getAudits)).Methods("GET")
 	api.BaseRoutes.ApiRoot.Handle("/email/test", api.ApiSessionRequired(testEmail)).Methods("POST")
+	api.BaseRoutes.ApiRoot.Handle("/file/s3_test", api.ApiSessionRequired(testS3)).Methods("POST")
 	api.BaseRoutes.ApiRoot.Handle("/database/recycle", api.ApiSessionRequired(databaseRecycle)).Methods("POST")
 	api.BaseRoutes.ApiRoot.Handle("/caches/invalidate", api.ApiSessionRequired(invalidateCaches)).Methods("POST")
 
@@ -384,3 +385,33 @@ func getAnalytics(c *Context, w http.ResponseWriter, r *http.Request) {
 
 	w.Write([]byte(rows.ToJson()))
 }
+
+func testS3(c *Context, w http.ResponseWriter, r *http.Request) {
+	cfg := model.ConfigFromJson(r.Body)
+	if cfg == nil {
+		cfg = c.App.Config()
+	}
+
+	if !c.App.SessionHasPermissionTo(c.Session, model.PERMISSION_MANAGE_SYSTEM) {
+		c.SetPermissionError(model.PERMISSION_MANAGE_SYSTEM)
+		return
+	}
+
+	err := utils.CheckMandatoryS3Fields(&cfg.FileSettings)
+	if err != nil {
+		c.Err = err
+		return
+	}
+
+	license := c.App.License()
+	backend, appErr := utils.NewFileBackend(&cfg.FileSettings, license != nil && *license.Features.Compliance)
+	if appErr == nil {
+		appErr = backend.TestConnection()
+	}
+	if appErr != nil {
+		c.Err = appErr
+		return
+	}
+
+	ReturnStatusOK(w)
+}
diff --git a/api4/system_test.go b/api4/system_test.go
index 01b4934ae..e39486b77 100644
--- a/api4/system_test.go
+++ b/api4/system_test.go
@@ -1,7 +1,9 @@
 package api4
 
 import (
+	"fmt"
 	"net/http"
+	"os"
 	"strings"
 	"testing"
 
@@ -466,3 +468,65 @@ func TestGetAnalyticsOld(t *testing.T) {
 	_, resp = Client.GetAnalyticsOld("", th.BasicTeam.Id)
 	CheckUnauthorizedStatus(t, resp)
 }
+
+func TestS3TestConnection(t *testing.T) {
+	th := Setup().InitBasic().InitSystemAdmin()
+	defer th.TearDown()
+	Client := th.Client
+
+	s3Host := os.Getenv("CI_HOST")
+	if s3Host == "" {
+		s3Host = "dockerhost"
+	}
+
+	s3Port := os.Getenv("CI_MINIO_PORT")
+	if s3Port == "" {
+		s3Port = "9001"
+	}
+
+	s3Endpoint := fmt.Sprintf("%s:%s", s3Host, s3Port)
+	config := model.Config{
+		FileSettings: model.FileSettings{
+			DriverName:              model.NewString(model.IMAGE_DRIVER_S3),
+			AmazonS3AccessKeyId:     model.MINIO_ACCESS_KEY,
+			AmazonS3SecretAccessKey: model.MINIO_SECRET_KEY,
+			AmazonS3Bucket:          "",
+			AmazonS3Endpoint:        "",
+			AmazonS3SSL:             model.NewBool(false),
+		},
+	}
+
+	_, resp := Client.TestS3Connection(&config)
+	CheckForbiddenStatus(t, resp)
+
+	_, resp = th.SystemAdminClient.TestS3Connection(&config)
+	CheckBadRequestStatus(t, resp)
+	if resp.Error.Message != "S3 Bucket is required" {
+		t.Fatal("should return error - missing s3 bucket")
+	}
+
+	config.FileSettings.AmazonS3Bucket = model.MINIO_BUCKET
+	_, resp = th.SystemAdminClient.TestS3Connection(&config)
+	CheckBadRequestStatus(t, resp)
+	if resp.Error.Message != "S3 Endpoint is required" {
+		t.Fatal("should return error - missing s3 endpoint")
+	}
+
+	config.FileSettings.AmazonS3Endpoint = s3Endpoint
+	_, resp = th.SystemAdminClient.TestS3Connection(&config)
+	CheckBadRequestStatus(t, resp)
+	if resp.Error.Message != "S3 Region is required" {
+		t.Fatal("should return error - missing s3 region")
+	}
+
+	config.FileSettings.AmazonS3Region = "us-east-1"
+	_, resp = th.SystemAdminClient.TestS3Connection(&config)
+	CheckOKStatus(t, resp)
+
+	config.FileSettings.AmazonS3Bucket = "Wrong_bucket"
+	_, resp = th.SystemAdminClient.TestS3Connection(&config)
+	CheckInternalErrorStatus(t, resp)
+	if resp.Error.Message != "Error checking if bucket exists." {
+		t.Fatal("should return error ")
+	}
+}
diff --git a/api4/team.go b/api4/team.go
index d770aee22..8e4c5c312 100644
--- a/api4/team.go
+++ b/api4/team.go
@@ -6,6 +6,7 @@ package api4
 import (
 	"bytes"
 	"encoding/base64"
+	"fmt"
 	"net/http"
 	"strconv"
 
@@ -28,6 +29,10 @@ func (api *API) InitTeam() {
 	api.BaseRoutes.Team.Handle("", api.ApiSessionRequired(deleteTeam)).Methods("DELETE")
 	api.BaseRoutes.Team.Handle("/patch", api.ApiSessionRequired(patchTeam)).Methods("PUT")
 	api.BaseRoutes.Team.Handle("/stats", api.ApiSessionRequired(getTeamStats)).Methods("GET")
+
+	api.BaseRoutes.Team.Handle("/image", api.ApiSessionRequiredTrustRequester(getTeamIcon)).Methods("GET")
+	api.BaseRoutes.Team.Handle("/image", api.ApiSessionRequired(setTeamIcon)).Methods("POST")
+
 	api.BaseRoutes.TeamMembers.Handle("", api.ApiSessionRequired(getTeamMembers)).Methods("GET")
 	api.BaseRoutes.TeamMembers.Handle("/ids", api.ApiSessionRequired(getTeamMembersByIds)).Methods("POST")
 	api.BaseRoutes.TeamMembersForUser.Handle("", api.ApiSessionRequired(getTeamMembersForUser)).Methods("GET")
@@ -729,3 +734,81 @@ func getInviteInfo(c *Context, w http.ResponseWriter, r *http.Request) {
 		w.Write([]byte(model.MapToJson(result)))
 	}
 }
+
+func getTeamIcon(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_VIEW_TEAM) {
+		c.SetPermissionError(model.PERMISSION_VIEW_TEAM)
+		return
+	}
+
+	if team, err := c.App.GetTeam(c.Params.TeamId); err != nil {
+		c.Err = err
+		return
+	} else {
+		etag := strconv.FormatInt(team.LastTeamIconUpdate, 10)
+
+		if c.HandleEtag(etag, "Get Team Icon", w, r) {
+			return
+		}
+
+		if img, err := c.App.GetTeamIcon(team); err != nil {
+			c.Err = err
+			return
+		} else {
+			w.Header().Set("Content-Type", "image/png")
+			w.Header().Set("Cache-Control", fmt.Sprintf("max-age=%v, public", 24*60*60)) // 24 hrs
+			w.Header().Set(model.HEADER_ETAG_SERVER, etag)
+			w.Write(img)
+		}
+	}
+}
+
+func setTeamIcon(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 r.ContentLength > *c.App.Config().FileSettings.MaxFileSize {
+		c.Err = model.NewAppError("setTeamIcon", "api.team.set_team_icon.too_large.app_error", nil, "", http.StatusBadRequest)
+		return
+	}
+
+	if err := r.ParseMultipartForm(*c.App.Config().FileSettings.MaxFileSize); err != nil {
+		c.Err = model.NewAppError("setTeamIcon", "api.team.set_team_icon.parse.app_error", nil, err.Error(), http.StatusBadRequest)
+		return
+	}
+
+	m := r.MultipartForm
+
+	imageArray, ok := m.File["image"]
+	if !ok {
+		c.Err = model.NewAppError("setTeamIcon", "api.team.set_team_icon.no_file.app_error", nil, "", http.StatusBadRequest)
+		return
+	}
+
+	if len(imageArray) <= 0 {
+		c.Err = model.NewAppError("setTeamIcon", "api.team.set_team_icon.array.app_error", nil, "", http.StatusBadRequest)
+		return
+	}
+
+	imageData := imageArray[0]
+
+	if err := c.App.SetTeamIcon(c.Params.TeamId, imageData); err != nil {
+		c.Err = err
+		return
+	}
+
+	c.LogAudit("")
+	ReturnStatusOK(w)
+}
diff --git a/api4/team_test.go b/api4/team_test.go
index fa139faae..faac81312 100644
--- a/api4/team_test.go
+++ b/api4/team_test.go
@@ -15,6 +15,8 @@ import (
 
 	"github.com/mattermost/mattermost-server/model"
 	"github.com/mattermost/mattermost-server/utils"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestCreateTeam(t *testing.T) {
@@ -1903,3 +1905,82 @@ func TestGetTeamInviteInfo(t *testing.T) {
 	_, resp = Client.GetTeamInviteInfo("junk")
 	CheckNotFoundStatus(t, resp)
 }
+
+func TestSetTeamIcon(t *testing.T) {
+	th := Setup().InitBasic().InitSystemAdmin()
+	defer th.TearDown()
+	Client := th.Client
+	team := th.BasicTeam
+
+	data, err := readTestFile("test.png")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	th.LoginTeamAdmin()
+
+	ok, resp := Client.SetTeamIcon(team.Id, data)
+	if !ok {
+		t.Fatal(resp.Error)
+	}
+	CheckNoError(t, resp)
+
+	ok, resp = Client.SetTeamIcon(model.NewId(), data)
+	if ok {
+		t.Fatal("Should return false, set team icon not allowed")
+	}
+	CheckForbiddenStatus(t, resp)
+
+	th.LoginBasic()
+
+	_, resp = Client.SetTeamIcon(team.Id, data)
+	if resp.StatusCode == http.StatusForbidden {
+		CheckForbiddenStatus(t, resp)
+	} else if resp.StatusCode == http.StatusUnauthorized {
+		CheckUnauthorizedStatus(t, resp)
+	} else {
+		t.Fatal("Should have failed either forbidden or unauthorized")
+	}
+
+	Client.Logout()
+
+	_, resp = Client.SetTeamIcon(team.Id, data)
+	if resp.StatusCode == http.StatusForbidden {
+		CheckForbiddenStatus(t, resp)
+	} else if resp.StatusCode == http.StatusUnauthorized {
+		CheckUnauthorizedStatus(t, resp)
+	} else {
+		t.Fatal("Should have failed either forbidden or unauthorized")
+	}
+
+	teamBefore, err := th.App.GetTeam(team.Id)
+	require.Nil(t, err)
+
+	_, resp = th.SystemAdminClient.SetTeamIcon(team.Id, data)
+	CheckNoError(t, resp)
+
+	teamAfter, err := th.App.GetTeam(team.Id)
+	require.Nil(t, err)
+	assert.True(t, teamBefore.LastTeamIconUpdate < teamAfter.LastTeamIconUpdate, "LastTeamIconUpdate should have been updated for team")
+
+	info := &model.FileInfo{Path: "teams/" + team.Id + "/teamIcon.png"}
+	if err := th.cleanupTestFile(info); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestGetTeamIcon(t *testing.T) {
+	th := Setup().InitBasic().InitSystemAdmin()
+	defer th.TearDown()
+	Client := th.Client
+	team := th.BasicTeam
+
+	// should always fail because no initial image and no auto creation
+	_, resp := Client.GetTeamIcon(team.Id, "")
+	CheckNotFoundStatus(t, resp)
+
+	Client.Logout()
+
+	_, resp = Client.GetTeamIcon(team.Id, "")
+	CheckUnauthorizedStatus(t, resp)
+}
diff --git a/api4/user.go b/api4/user.go
index cfb2a5b3f..f82a6e3d5 100644
--- a/api4/user.go
+++ b/api4/user.go
@@ -13,7 +13,6 @@ import (
 	"github.com/mattermost/mattermost-server/app"
 	"github.com/mattermost/mattermost-server/model"
 	"github.com/mattermost/mattermost-server/store"
-	"github.com/mattermost/mattermost-server/utils"
 )
 
 func (api *API) InitUser() {
@@ -894,7 +893,7 @@ func sendPasswordReset(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if sent, err := c.App.SendPasswordReset(email, utils.GetSiteURL()); err != nil {
+	if sent, err := c.App.SendPasswordReset(email, c.App.GetSiteURL()); err != nil {
 		c.Err = err
 		return
 	} else if sent {
@@ -1076,6 +1075,7 @@ func attachDeviceId(c *Context, w http.ResponseWriter, r *http.Request) {
 		MaxAge:   maxAge,
 		Expires:  expiresAt,
 		HttpOnly: true,
+		Domain:   c.App.GetCookieDomain(),
 		Secure:   secure,
 	}