From e07e114deffda800188249779037b4d8a6ccc87a Mon Sep 17 00:00:00 2001 From: Scott Bishel Date: Thu, 7 Nov 2019 12:12:37 -0700 Subject: [PATCH 01/18] don't clear AuthService/AuthData if user is System Admin (#12986) --- api4/user.go | 2 +- api4/user_test.go | 11 ++++++++--- model/user.go | 8 +++++--- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/api4/user.go b/api4/user.go index 25984727f5..0892624ed0 100644 --- a/api4/user.go +++ b/api4/user.go @@ -86,7 +86,7 @@ func createUser(c *Context, w http.ResponseWriter, r *http.Request) { return } - user.SanitizeInput() + user.SanitizeInput(c.IsSystemAdmin()) tokenId := r.URL.Query().Get("t") inviteId := r.URL.Query().Get("iid") diff --git a/api4/user_test.go b/api4/user_test.go index 452a045e21..003573fd09 100644 --- a/api4/user_test.go +++ b/api4/user_test.go @@ -110,10 +110,15 @@ func TestCreateUserInputFilter(t *testing.T) { _, resp := th.SystemAdminClient.CreateUser(user) CheckBadRequestStatus(t, resp) }) - - t.Run("AuthServiceFilter", func(t *testing.T) { - user := &model.User{Email: "foobar+testdomainrestriction@mattermost.org", Password: "Password1", Username: GenerateTestUsername(), AuthService: "ldap"} + t.Run("ValidAuthServiceFilter", func(t *testing.T) { + user := &model.User{Email: "foobar+testdomainrestriction@mattermost.org", Username: GenerateTestUsername(), AuthService: "ldap", AuthData: model.NewString("999099")} _, resp := th.SystemAdminClient.CreateUser(user) + CheckNoError(t, resp) + }) + + t.Run("InvalidAuthServiceFilter", func(t *testing.T) { + user := &model.User{Email: "foobar+testdomainrestriction@mattermost.org", Password: "Password1", Username: GenerateTestUsername(), AuthService: "ldap"} + _, resp := th.Client.CreateUser(user) CheckBadRequestStatus(t, resp) }) }) diff --git a/model/user.go b/model/user.go index 5a308775cc..4e04fc2954 100644 --- a/model/user.go +++ b/model/user.go @@ -497,9 +497,11 @@ func (u *User) Sanitize(options map[string]bool) { } // Remove any input data from the user object that is not user controlled -func (u *User) SanitizeInput() { - u.AuthData = NewString("") - u.AuthService = "" +func (u *User) SanitizeInput(isAdmin bool) { + if !isAdmin { + u.AuthData = NewString("") + u.AuthService = "" + } u.LastPasswordUpdate = 0 u.LastPictureUpdate = 0 u.FailedAttempts = 0 From c2fbcd71d1edc47622f4580848258c68e9d4dfe2 Mon Sep 17 00:00:00 2001 From: Carlos Tadeu Panato Junior Date: Fri, 8 Nov 2019 15:43:34 +0100 Subject: [PATCH 02/18] Update minio (#12979) * update minio docker image to latest stable available on 31/10/2019 * update minio-go dependencies --- build/docker-compose.common.yml | 2 +- go.mod | 2 +- go.sum | 4 +- .../github.com/minio/minio-go/v6/api-list.go | 42 +++++++++---------- .../minio/minio-go/v6/api-notification.go | 11 ++++- vendor/github.com/minio/minio-go/v6/api.go | 2 +- vendor/github.com/minio/minio-go/v6/go.sum | 3 ++ vendor/modules.txt | 2 +- 8 files changed, 38 insertions(+), 30 deletions(-) diff --git a/build/docker-compose.common.yml b/build/docker-compose.common.yml index ad8e9326cd..e6acef4eb3 100644 --- a/build/docker-compose.common.yml +++ b/build/docker-compose.common.yml @@ -21,7 +21,7 @@ services: POSTGRES_PASSWORD: mostest POSTGRES_DB: mattermost_test minio: - image: "minio/minio:RELEASE.2019-08-14T20-37-41Z" + image: "minio/minio:RELEASE.2019-10-11T00-38-09Z" command: "server /data" networks: - mm-test diff --git a/go.mod b/go.mod index 9f2699c127..98d401226b 100644 --- a/go.mod +++ b/go.mod @@ -50,7 +50,7 @@ require ( github.com/mattn/go-runewidth v0.0.4 // indirect github.com/mattn/go-sqlite3 v1.11.0 github.com/miekg/dns v1.1.19 // indirect - github.com/minio/minio-go/v6 v6.0.38 + github.com/minio/minio-go/v6 v6.0.40 github.com/mitchellh/go-testing-interface v1.0.0 // indirect github.com/muesli/smartcrop v0.3.0 // indirect github.com/olekukonko/tablewriter v0.0.1 // indirect diff --git a/go.sum b/go.sum index 9c3a83c5c7..ec930343d0 100644 --- a/go.sum +++ b/go.sum @@ -263,8 +263,8 @@ github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5 github.com/miekg/dns v1.0.14/go.mod h1:W1PPwlIAgtquWBMBEV9nkV9Cazfe8ScdGz/Lj7v3Nrg= github.com/miekg/dns v1.1.19 h1:0ymbfaLG1/utH2+BydNiF+dx1jSEmdr/nylOtkGHZZg= github.com/miekg/dns v1.1.19/go.mod h1:bPDLeHnStXmXAq1m/Ch/hvfNHr14JKNPMBo3VZKjuso= -github.com/minio/minio-go/v6 v6.0.38 h1:zd3yagckaBVAMJT+HsbpURx9ndqYQp/N/udc1UVS72E= -github.com/minio/minio-go/v6 v6.0.38/go.mod h1:qD0lajrGW49lKZLtXKtCB4X/qkMf0a5tBvN2PaZg7Gg= +github.com/minio/minio-go/v6 v6.0.40 h1:MlSCSXvItiu2jINMxYdhUU99KR4446Db+0iAU1IKaZ0= +github.com/minio/minio-go/v6 v6.0.40/go.mod h1:qD0lajrGW49lKZLtXKtCB4X/qkMf0a5tBvN2PaZg7Gg= github.com/minio/sha256-simd v0.1.1 h1:5QHSlgo3nt5yKOJrC7W8w7X+NFl8cMPZm96iu8kKUJU= github.com/minio/sha256-simd v0.1.1/go.mod h1:B5e1o+1/KgNmWrSQK08Y6Z1Vb5pwIktudl0J58iy0KM= github.com/mitchellh/cli v1.0.0/go.mod h1:hNIlj7HEI86fIcpObd7a0FcrxTWetlwJDGcceTlRvqc= diff --git a/vendor/github.com/minio/minio-go/v6/api-list.go b/vendor/github.com/minio/minio-go/v6/api-list.go index 2bd83fedad..b9b0bcccd8 100644 --- a/vendor/github.com/minio/minio-go/v6/api-list.go +++ b/vendor/github.com/minio/minio-go/v6/api-list.go @@ -1,6 +1,6 @@ /* * MinIO Go Library for Amazon S3 Compatible Cloud Storage - * Copyright 2015-2017 MinIO, Inc. + * Copyright 2015-2019 MinIO, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -208,12 +208,10 @@ func (c Client) listObjectsV2Query(bucketName, objectPrefix, continuationToken s urlValues.Set("fetch-owner", "true") } - // maxkeys should default to 1000 or less. - if maxkeys == 0 || maxkeys > 1000 { - maxkeys = 1000 - } // Set max keys. - urlValues.Set("max-keys", fmt.Sprintf("%d", maxkeys)) + if maxkeys > 0 { + urlValues.Set("max-keys", fmt.Sprintf("%d", maxkeys)) + } // Set start-after if startAfter != "" { @@ -248,15 +246,15 @@ func (c Client) listObjectsV2Query(bucketName, objectPrefix, continuationToken s return listBucketResult, errors.New("Truncated response should have continuation token set") } - for _, obj := range listBucketResult.Contents { - obj.Key, err = url.QueryUnescape(obj.Key) + for i, obj := range listBucketResult.Contents { + listBucketResult.Contents[i].Key, err = url.QueryUnescape(obj.Key) if err != nil { return listBucketResult, err } } - for _, obj := range listBucketResult.CommonPrefixes { - obj.Prefix, err = url.QueryUnescape(obj.Prefix) + for i, obj := range listBucketResult.CommonPrefixes { + listBucketResult.CommonPrefixes[i].Prefix, err = url.QueryUnescape(obj.Prefix) if err != nil { return listBucketResult, err } @@ -401,12 +399,10 @@ func (c Client) listObjectsQuery(bucketName, objectPrefix, objectMarker, delimit urlValues.Set("marker", objectMarker) } - // maxkeys should default to 1000 or less. - if maxkeys == 0 || maxkeys > 1000 { - maxkeys = 1000 - } // Set max keys. - urlValues.Set("max-keys", fmt.Sprintf("%d", maxkeys)) + if maxkeys > 0 { + urlValues.Set("max-keys", fmt.Sprintf("%d", maxkeys)) + } // Always set encoding-type urlValues.Set("encoding-type", "url") @@ -433,15 +429,15 @@ func (c Client) listObjectsQuery(bucketName, objectPrefix, objectMarker, delimit return listBucketResult, err } - for _, obj := range listBucketResult.Contents { - obj.Key, err = url.QueryUnescape(obj.Key) + for i, obj := range listBucketResult.Contents { + listBucketResult.Contents[i].Key, err = url.QueryUnescape(obj.Key) if err != nil { return listBucketResult, err } } - for _, obj := range listBucketResult.CommonPrefixes { - obj.Prefix, err = url.QueryUnescape(obj.Prefix) + for i, obj := range listBucketResult.CommonPrefixes { + listBucketResult.CommonPrefixes[i].Prefix, err = url.QueryUnescape(obj.Prefix) if err != nil { return listBucketResult, err } @@ -642,15 +638,15 @@ func (c Client) listMultipartUploadsQuery(bucketName, keyMarker, uploadIDMarker, return listMultipartUploadsResult, err } - for _, obj := range listMultipartUploadsResult.Uploads { - obj.Key, err = url.QueryUnescape(obj.Key) + for i, obj := range listMultipartUploadsResult.Uploads { + listMultipartUploadsResult.Uploads[i].Key, err = url.QueryUnescape(obj.Key) if err != nil { return listMultipartUploadsResult, err } } - for _, obj := range listMultipartUploadsResult.CommonPrefixes { - obj.Prefix, err = url.QueryUnescape(obj.Prefix) + for i, obj := range listMultipartUploadsResult.CommonPrefixes { + listMultipartUploadsResult.CommonPrefixes[i].Prefix, err = url.QueryUnescape(obj.Prefix) if err != nil { return listMultipartUploadsResult, err } diff --git a/vendor/github.com/minio/minio-go/v6/api-notification.go b/vendor/github.com/minio/minio-go/v6/api-notification.go index f35619541e..0480c21eb5 100644 --- a/vendor/github.com/minio/minio-go/v6/api-notification.go +++ b/vendor/github.com/minio/minio-go/v6/api-notification.go @@ -200,6 +200,11 @@ func (c Client) ListenBucketNotification(bucketName, prefix, suffix string, even for bio.Scan() { var notificationInfo NotificationInfo if err = json.Unmarshal(bio.Bytes(), ¬ificationInfo); err != nil { + // Unexpected error during json unmarshal, send + // the error to caller for actionable as needed. + notificationInfoCh <- NotificationInfo{ + Err: err, + } closeResponse(resp) continue } @@ -211,7 +216,11 @@ func (c Client) ListenBucketNotification(bucketName, prefix, suffix string, even return } } - + if err = bio.Err(); err != nil { + notificationInfoCh <- NotificationInfo{ + Err: err, + } + } // Close current connection before looping further. closeResponse(resp) } diff --git a/vendor/github.com/minio/minio-go/v6/api.go b/vendor/github.com/minio/minio-go/v6/api.go index 1daf055f23..71cbc1196a 100644 --- a/vendor/github.com/minio/minio-go/v6/api.go +++ b/vendor/github.com/minio/minio-go/v6/api.go @@ -104,7 +104,7 @@ type Options struct { // Global constants. const ( libraryName = "minio-go" - libraryVersion = "v6.0.38" + libraryVersion = "v6.0.40" ) // User Agent should always following the below style. diff --git a/vendor/github.com/minio/minio-go/v6/go.sum b/vendor/github.com/minio/minio-go/v6/go.sum index b3cacbda4d..cd02277ed3 100644 --- a/vendor/github.com/minio/minio-go/v6/go.sum +++ b/vendor/github.com/minio/minio-go/v6/go.sum @@ -1,3 +1,4 @@ +github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/dustin/go-humanize v1.0.0 h1:VSnTsYCnlFHaM2/igO1h6X3HA71jcobQuxemgkq4zYo= github.com/dustin/go-humanize v1.0.0/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk= @@ -10,6 +11,7 @@ github.com/minio/sha256-simd v0.1.1 h1:5QHSlgo3nt5yKOJrC7W8w7X+NFl8cMPZm96iu8kKU github.com/minio/sha256-simd v0.1.1/go.mod h1:B5e1o+1/KgNmWrSQK08Y6Z1Vb5pwIktudl0J58iy0KM= github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG+4E0Y= github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/sirupsen/logrus v1.4.2 h1:SPIRibHv4MatM3XXNO2BJeFLZwZ2LvZgfQ5+UNI2im4= github.com/sirupsen/logrus v1.4.2/go.mod h1:tLMulIdttU9McNUspp0xgXVQah82FyeX6MwdIuYE2rE= @@ -18,6 +20,7 @@ github.com/smartystreets/assertions v0.0.0-20180927180507-b2de0cb4f26d/go.mod h1 github.com/smartystreets/goconvey v0.0.0-20190330032615-68dc04aab96a h1:pa8hGb/2YqsZKovtsgrwcDH1RZhVbTKCjLp47XpqCDs= github.com/smartystreets/goconvey v0.0.0-20190330032615-68dc04aab96a/go.mod h1:syvi0/a8iFYH4r/RixwvyeAJjdLS9QV7WQ/tjFTllLA= github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/testify v1.2.2 h1:bSDNvY7ZPG5RlJ8otE/7V6gMiyenm9RtJ7IUVIAoJ1w= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20190513172903-22d7a77e9e5f h1:R423Cnkcp5JABoeemiGEPlt9tHXFfw5kvc0yqlxRPWo= diff --git a/vendor/modules.txt b/vendor/modules.txt index 489ab2f8dd..7a5d974bb1 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -132,7 +132,7 @@ github.com/mattn/go-sqlite3 github.com/matttproud/golang_protobuf_extensions/pbutil # github.com/miekg/dns v1.1.19 github.com/miekg/dns -# github.com/minio/minio-go/v6 v6.0.38 +# github.com/minio/minio-go/v6 v6.0.40 github.com/minio/minio-go/v6 github.com/minio/minio-go/v6/pkg/credentials github.com/minio/minio-go/v6/pkg/encrypt From 613a015cd07acedd870bb2628a394736723b0398 Mon Sep 17 00:00:00 2001 From: larkox Date: Fri, 8 Nov 2019 20:18:30 +0100 Subject: [PATCH 03/18] [GH-13031] Split tests into subtests or table testing for the file "https://github.com/mattermost/mattermost-server/blob/master/model/file_info_test.go" (#13032) * Split tests into subtests and table testing for 'model/file_info_test.go' * Improve error messages and fit testify function parameter order --- model/file_info_test.go | 231 ++++++++++++++++++++++++++-------------- 1 file changed, 152 insertions(+), 79 deletions(-) diff --git a/model/file_info_test.go b/model/file_info_test.go index bf2e449ac2..04920270e4 100644 --- a/model/file_info_test.go +++ b/model/file_info_test.go @@ -24,109 +24,182 @@ func TestFileInfoIsValid(t *testing.T) { Path: "fake/path.png", } - require.Nil(t, info.IsValid()) + t.Run("Valid File Info", func(t *testing.T) { + assert.Nil(t, info.IsValid()) + }) - info.Id = "" - require.NotNil(t, info.IsValid(), "empty Id isn't valid") + t.Run("Empty ID is not valid", func(t *testing.T) { + info.Id = "" + assert.NotNil(t, info.IsValid(), "empty Id isn't valid") + info.Id = NewId() + }) - info.Id = NewId() - info.CreateAt = 0 - require.NotNil(t, info.IsValid(), "empty CreateAt isn't valid") + t.Run("CreateAt 0 is not valid", func(t *testing.T) { + info.CreateAt = 0 + assert.NotNil(t, info.IsValid(), "empty CreateAt isn't valid") + info.CreateAt = 1234 + }) - info.CreateAt = 1234 - info.UpdateAt = 0 - require.NotNil(t, info.IsValid(), "empty UpdateAt isn't valid") + t.Run("UpdateAt 0 is not valid", func(t *testing.T) { + info.UpdateAt = 0 + assert.NotNil(t, info.IsValid(), "empty UpdateAt isn't valid") + info.UpdateAt = 1234 + }) - info.UpdateAt = 1234 - info.PostId = NewId() - require.Nil(t, info.IsValid()) + t.Run("New Post ID is valid", func(t *testing.T) { + info.PostId = NewId() + assert.Nil(t, info.IsValid()) + }) - info.Path = "" - require.NotNil(t, info.IsValid(), "empty Path isn't valid") - - info.Path = "fake/path.png" - require.Nil(t, info.IsValid()) + t.Run("Empty path is not valid", func(t *testing.T) { + info.Path = "" + assert.NotNil(t, info.IsValid(), "empty Path isn't valid") + info.Path = "fake/path.png" + }) } func TestFileInfoIsImage(t *testing.T) { - info := &FileInfo{MimeType: "image/png"} - assert.True(t, info.IsImage(), "file is an image") + info := &FileInfo{} + t.Run("MimeType set to image/png is considered an image", func(t *testing.T) { + info.MimeType = "image/png" + assert.True(t, info.IsImage(), "PNG file should be considered as an image") + }) - info.MimeType = "text/plain" - assert.False(t, info.IsImage(), "file is not an image") + t.Run("MimeType set to text/plain is not considered an image", func(t *testing.T) { + info.MimeType = "text/plain" + assert.False(t, info.IsImage(), "Text file should not be considered as an image") + }) } func TestGetInfoForFile(t *testing.T) { fakeFile := make([]byte, 1000) - info, errApp := GetInfoForBytes("file.txt", fakeFile) - require.Nil(t, errApp) - assert.Equalf(t, info.Name, "file.txt", "Got incorrect filename: %v", info.Name) - assert.Equalf(t, info.Extension, "txt", "Got incorrect extension: %v", info.Extension) - assert.EqualValuesf(t, info.Size, 1000, "Got incorrect size: %v", info.Size) - assert.Truef(t, strings.HasPrefix(info.MimeType, "text/plain"), "Got incorrect mime type: %v", info.MimeType) - assert.Equalf(t, info.Width, 0, "Got incorrect width: %v", info.Width) - assert.Equalf(t, info.Height, 0, "Got incorrect height: %v", info.Height) - assert.Falsef(t, info.HasPreviewImage, "Got incorrect has preview image: %v", info.HasPreviewImage) - pngFile, err := ioutil.ReadFile("../tests/test.png") require.Nilf(t, err, "Failed to load test.png") - info, err = GetInfoForBytes("test.png", pngFile) - require.Nil(t, err) - assert.Equalf(t, info.Name, "test.png", "Got incorrect filename: %v", info.Name) - assert.Equalf(t, info.Extension, "png", "Got incorrect extension: %v", info.Extension) - assert.EqualValues(t, info.Size, 279591, "Got incorrect size: %v", info.Size) - assert.Equalf(t, info.MimeType, "image/png", "Got incorrect mime type: %v", info.MimeType) - assert.Equalf(t, info.Width, 408, "Got incorrect width: %v", info.Width) - assert.Equalf(t, info.Height, 336, "Got incorrect height: %v", info.Height) - assert.Truef(t, info.HasPreviewImage, "Got incorrect has preview image: %v", info.HasPreviewImage) - // base 64 encoded version of handtinywhite.gif from http://probablyprogramming.com/2009/03/15/the-tiniest-gif-ever gifFile, _ := base64.StdEncoding.DecodeString("R0lGODlhAQABAIABAP///wAAACwAAAAAAQABAAACAkQBADs=") - info, err = GetInfoForBytes("handtinywhite.gif", gifFile) - require.Nil(t, err) - assert.Equalf(t, info.Name, "handtinywhite.gif", "Got incorrect filename: %v", info.Name) - assert.Equalf(t, info.Extension, "gif", "Got incorrect extension: %v", info.Extension) - assert.EqualValuesf(t, info.Size, 35, "Got incorrect size: %v", info.Size) - assert.Equalf(t, info.MimeType, "image/gif", "Got incorrect mime type: %v", info.MimeType) - assert.Equalf(t, info.Width, 1, "Got incorrect width: %v", info.Width) - assert.Equalf(t, info.Height, 1, "Got incorrect height: %v", info.Height) - assert.Truef(t, info.HasPreviewImage, "Got incorrect has preview image: %v", info.HasPreviewImage) animatedGifFile, err := ioutil.ReadFile("../tests/testgif.gif") require.Nilf(t, err, "Failed to load testgif.gif") - info, err = GetInfoForBytes("testgif.gif", animatedGifFile) - require.Nil(t, err) - assert.Equalf(t, info.Name, "testgif.gif", "Got incorrect filename: %v", info.Name) - assert.Equalf(t, info.Extension, "gif", "Got incorrect extension: %v", info.Extension) - assert.EqualValuesf(t, info.Size, 38689, "Got incorrect size: %v", info.Size) - assert.Equalf(t, info.MimeType, "image/gif", "Got incorrect mime type: %v", info.MimeType) - assert.Equalf(t, info.Width, 118, "Got incorrect width: %v", info.Width) - assert.Equalf(t, info.Height, 118, "Got incorrect height: %v", info.Height) - assert.Falsef(t, info.HasPreviewImage, "Got incorrect has preview image: %v", info.HasPreviewImage) + var ttc = []struct { + testName string + filename string + file []byte + usePrefixForMime bool + expectedExtension string + expectedSize int + expectedMime string + expectedWidth int + expectedHeight int + expectedHasPreviewImage bool + }{ + { + testName: "Text File", + filename: "file.txt", + file: fakeFile, + usePrefixForMime: true, + expectedExtension: "txt", + expectedSize: 1000, + expectedMime: "text/plain", + expectedWidth: 0, + expectedHeight: 0, + expectedHasPreviewImage: false, + }, + { + testName: "PNG file", + filename: "test.png", + file: pngFile, + usePrefixForMime: false, + expectedExtension: "png", + expectedSize: 279591, + expectedMime: "image/png", + expectedWidth: 408, + expectedHeight: 336, + expectedHasPreviewImage: true, + }, + { + testName: "Static Gif File", + filename: "handtinywhite.gif", + file: gifFile, + usePrefixForMime: false, + expectedExtension: "gif", + expectedSize: 35, + expectedMime: "image/gif", + expectedWidth: 1, + expectedHeight: 1, + expectedHasPreviewImage: true, + }, + { + testName: "Animated Gif File", + filename: "testgif.gif", + file: animatedGifFile, + usePrefixForMime: false, + expectedExtension: "gif", + expectedSize: 38689, + expectedMime: "image/gif", + expectedWidth: 118, + expectedHeight: 118, + expectedHasPreviewImage: false, + }, + { + testName: "No extension File", + filename: "filewithoutextension", + file: fakeFile, + usePrefixForMime: false, + expectedExtension: "", + expectedSize: 1000, + expectedMime: "", + expectedWidth: 0, + expectedHeight: 0, + expectedHasPreviewImage: false, + }, + { + // Always make the extension lower case to make it easier to use in other places + testName: "Uppercase extension File", + filename: "file.TXT", + file: fakeFile, + usePrefixForMime: true, + expectedExtension: "txt", + expectedSize: 1000, + expectedMime: "text/plain", + expectedWidth: 0, + expectedHeight: 0, + expectedHasPreviewImage: false, + }, + { + // Don't error out for image formats we don't support + testName: "Not supported File", + filename: "file.tif", + file: fakeFile, + usePrefixForMime: false, + expectedExtension: "tif", + expectedSize: 1000, + expectedMime: "image/tiff", + expectedWidth: 0, + expectedHeight: 0, + expectedHasPreviewImage: false, + }, + } - info, err = GetInfoForBytes("filewithoutextension", fakeFile) - require.Nil(t, err) - assert.Equalf(t, info.Name, "filewithoutextension", "Got incorrect filename: %v", info.Name) - assert.Equalf(t, info.Extension, "", "Got incorrect extension: %v", info.Extension) - assert.EqualValuesf(t, info.Size, 1000, "Got incorrect size: %v", info.Size) - assert.Equalf(t, info.MimeType, "", "Got incorrect mime type: %v", info.MimeType) - assert.Equalf(t, info.Width, 0, "Got incorrect width: %v", info.Width) - assert.Equalf(t, info.Height, 0, "Got incorrect height: %v", info.Height) - assert.Falsef(t, info.HasPreviewImage, "Got incorrect has preview image: %v", info.HasPreviewImage) + for _, tc := range ttc { + t.Run(tc.testName, func(t *testing.T) { + info, errApp := GetInfoForBytes(tc.filename, tc.file) + require.Nil(t, errApp) - // Always make the extension lower case to make it easier to use in other places - info, err = GetInfoForBytes("file.TXT", fakeFile) - require.Nil(t, err) - assert.Equalf(t, info.Name, "file.TXT", "Got incorrect filename: %v", info.Name) - assert.Equalf(t, info.Extension, "txt", "Got incorrect extension: %v", info.Extension) + assert.Equalf(t, tc.filename, info.Name, "Got incorrect filename: %v", info.Name) + assert.Equalf(t, tc.expectedExtension, info.Extension, "Got incorrect extension: %v", info.Extension) + assert.EqualValuesf(t, tc.expectedSize, info.Size, "Got incorrect size: %v", info.Size) + assert.Equalf(t, tc.expectedWidth, info.Width, "Got incorrect width: %v", info.Width) + assert.Equalf(t, tc.expectedHeight, info.Height, "Got incorrect height: %v", info.Height) + assert.Equalf(t, tc.expectedHasPreviewImage, info.HasPreviewImage, "Got incorrect has preview image: %v", info.HasPreviewImage) - // Don't error out for image formats we don't support - info, err = GetInfoForBytes("file.tif", fakeFile) - require.Nil(t, err) - assert.Equalf(t, info.Name, "file.tif", "Got incorrect filename: %v", info.Name) - assert.Equalf(t, info.Extension, "tif", "Got incorrect extension: %v", info.Extension) - assert.True(t, info.MimeType == "image/x-tiff" || info.MimeType == "image/tiff", "Got incorrect mime type: %v", info.MimeType) + if tc.usePrefixForMime { + assert.Truef(t, strings.HasPrefix(info.MimeType, tc.expectedMime), "Got incorrect mime type: %v", info.MimeType) + } else { + assert.Equalf(t, tc.expectedMime, info.MimeType, "Got incorrect mime type: %v", info.MimeType) + } + }) + } } From c0c47df3f0d49eeb0fbab4aea1d587bfb3aebebb Mon Sep 17 00:00:00 2001 From: Claudio Costa Date: Fri, 8 Nov 2019 20:37:15 +0100 Subject: [PATCH 04/18] Fix data race on server startup (#13006) --- cmd/mattermost/commands/server.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/mattermost/commands/server.go b/cmd/mattermost/commands/server.go index d8553e3167..9813f387c7 100644 --- a/cmd/mattermost/commands/server.go +++ b/cmd/mattermost/commands/server.go @@ -72,16 +72,16 @@ func runServer(configStore config.Store, disableConfigWatch bool, usedPlatform b mlog.Error("The platform binary has been deprecated, please switch to using the mattermost binary.") } + api := api4.Init(server, server.AppOptions, server.Router) + wsapi.Init(server.FakeApp(), server.WebSocketRouter) + web.New(server, server.AppOptions, server.Router) + serverErr := server.Start() if serverErr != nil { mlog.Critical(serverErr.Error()) return serverErr } - api := api4.Init(server, server.AppOptions, server.Router) - wsapi.Init(server.FakeApp(), server.WebSocketRouter) - web.New(server, server.AppOptions, server.Router) - // If we allow testing then listen for manual testing URL hits if *server.Config().ServiceSettings.EnableTesting { manualtesting.Init(api) From 7600d9c1699a0ce577fc2b1d494e7fc19824f8a0 Mon Sep 17 00:00:00 2001 From: Jeff Abshire Date: Fri, 8 Nov 2019 12:27:37 -0800 Subject: [PATCH 05/18] Remove a potential tabnabbing attack in a template. (#13024) My company's security scanner picked up on a potential problem in this template in our deployed Mattermost instance. https://www.owasp.org/index.php/Reverse_Tabnabbing --- templates/unsupported_browser.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/unsupported_browser.html b/templates/unsupported_browser.html index dc901b7e0a..53e81baed9 100644 --- a/templates/unsupported_browser.html +++ b/templates/unsupported_browser.html @@ -292,7 +292,7 @@ {{template "unsupported_browser-system_browser" .Props.SystemBrowser}} {{end}}
- {{.Props.LearnMoreString}} + {{.Props.LearnMoreString}}
From d9a154fc70b39acc7121bdd487f9a81131de1f8f Mon Sep 17 00:00:00 2001 From: Arjit Chaudhary Date: Sun, 10 Nov 2019 01:00:45 +0800 Subject: [PATCH 06/18] Refactor "app/notification_email.go" to use structured logging #12726 (#12731) * Refactor "app/notification_email.go" to use structured logging #12726 * Update notification_email.go * Update notification_email.go * Update notification_email.go * add back fmt as fmt.Sprintf calls are used for Year/Day/Minute * Update notification_email.go * Update notification_email.go * Update notification_email.go --- app/notification_email.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/notification_email.go b/app/notification_email.go index cdd7ae7ef9..20af2a7fe3 100644 --- a/app/notification_email.go +++ b/app/notification_email.go @@ -100,7 +100,7 @@ func (a *App) sendNotificationEmail(notification *postNotification, user *model. a.Srv.Go(func() { if err := a.SendNotificationMail(user.Email, html.UnescapeString(subjectText), bodyText); err != nil { - mlog.Error("Error while sending the email", mlog.String("email", user.Email), mlog.Err(err)) + mlog.Error("Error while sending the email", mlog.String("user_email", user.Email), mlog.Err(err)) } }) @@ -285,7 +285,7 @@ func getFormattedPostTime(user *model.User, post *model.Post, useMilitaryTime bo func (a *App) generateHyperlinkForChannels(postMessage, teamName, teamURL string) string { team, err := a.GetTeamByName(teamName) if err != nil { - mlog.Error("Encountered error while looking up team by name", mlog.String("Team Name", teamName), mlog.Err(err)) + mlog.Error("Encountered error while looking up team by name", mlog.String("team_name", teamName), mlog.Err(err)) return postMessage } From 53ee077cfdff7ceb4366cdeac15492b4bae2082d Mon Sep 17 00:00:00 2001 From: Scott Bishel Date: Sun, 10 Nov 2019 08:07:49 -0700 Subject: [PATCH 07/18] MM-18005: Improve display message when attempting to add an outsider to a group-synced team via new team invite flow (#12966) * Update group errors being returned * update based on merge, error already added * remove unnecessary translation * Revert "remove unnecessary translation" This reverts commit 5a4ae89618ddca5ea256009aab167f3e071f488c. * remove unnecessary translation --- app/command_invite_test.go | 71 ++++++++++++++++++++++++++++++++++++++ i18n/en.json | 4 +-- 2 files changed, 73 insertions(+), 2 deletions(-) diff --git a/app/command_invite_test.go b/app/command_invite_test.go index 354b373b77..f79117b5f5 100644 --- a/app/command_invite_test.go +++ b/app/command_invite_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/mattermost/mattermost-server/model" ) @@ -42,6 +43,15 @@ func TestInviteProvider(t *testing.T) { userAndInvalidPrivate := "@" + basicUser3.Username + " ~" + privateChannel2.Name deactivatedUserPublicChannel := "@" + deactivatedUser.Username + " ~" + channel.Name + groupChannel := th.createChannel(th.BasicTeam, model.CHANNEL_PRIVATE) + var err *model.AppError + _, err = th.App.AddChannelMember(th.BasicUser.Id, groupChannel, "", "") + require.Nil(t, err) + groupChannel.GroupConstrained = model.NewBool(true) + groupChannel, _ = th.App.UpdateChannel(groupChannel) + + groupChannelNonUser := "@" + th.BasicUser2.Username + " ~" + groupChannel.Name + tests := []struct { desc string expected string @@ -97,6 +107,11 @@ func TestInviteProvider(t *testing.T) { expected: "api.command_invite.user_not_in_team.app_error", msg: basicUser4.Username, }, + { + desc: "try to add a user not part of the group to a group channel", + expected: "api.command_invite.group_constrained_user_denied", + msg: groupChannelNonUser, + }, { desc: "try to add a user to a private channel with no permission", expected: "api.command_invite.private_channel.app_error", @@ -116,3 +131,59 @@ func TestInviteProvider(t *testing.T) { }) } } + +func TestInviteGroup(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + th.BasicTeam.GroupConstrained = model.NewBool(true) + var err *model.AppError + _, _ = th.App.AddTeamMember(th.BasicTeam.Id, th.BasicUser.Id) + _, err = th.App.AddTeamMember(th.BasicTeam.Id, th.BasicUser2.Id) + require.Nil(t, err) + th.BasicTeam, _ = th.App.UpdateTeam(th.BasicTeam) + + privateChannel := th.createChannel(th.BasicTeam, model.CHANNEL_PRIVATE) + + groupChannelUser1 := "@" + th.BasicUser.Username + " ~" + privateChannel.Name + groupChannelUser2 := "@" + th.BasicUser2.Username + " ~" + privateChannel.Name + basicUser3 := th.CreateUser() + groupChannelUser3 := "@" + basicUser3.Username + " ~" + privateChannel.Name + + InviteP := InviteProvider{} + args := &model.CommandArgs{ + T: func(s string, args ...interface{}) string { return s }, + ChannelId: th.BasicChannel.Id, + TeamId: th.BasicTeam.Id, + Session: model.Session{UserId: th.BasicUser.Id, TeamMembers: []*model.TeamMember{{TeamId: th.BasicTeam.Id, Roles: model.TEAM_USER_ROLE_ID}}}, + } + + tests := []struct { + desc string + expected string + msg string + }{ + { + desc: "try to add an existing user part of the group to a group channel", + expected: "api.command_invite.user_already_in_channel.app_error", + msg: groupChannelUser1, + }, + { + desc: "try to add a user part of the group to a group channel", + expected: "api.command_invite.success", + msg: groupChannelUser2, + }, + { + desc: "try to add a user NOT part of the group to a group channel", + expected: "api.command_invite.user_not_in_team.app_error", + msg: groupChannelUser3, + }, + } + + for _, test := range tests { + t.Run(test.desc, func(t *testing.T) { + actual := InviteP.DoCommand(th.App, args, test.msg).Text + assert.Equal(t, test.expected, actual) + }) + } +} diff --git a/i18n/en.json b/i18n/en.json index c20c213a7f..f565df6d60 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -764,7 +764,7 @@ }, { "id": "api.command_invite.group_constrained_user_denied", - "translation": "User cannot be added to this channel because it is constrained to group members only." + "translation": "This channel is managed by groups. This user is not part of a group that is synched to this channel." }, { "id": "api.command_invite.hint", @@ -1864,7 +1864,7 @@ }, { "id": "api.team.add_members.user_denied", - "translation": "Team membership denied to the following users because of group constraints: {{ .UserIDs }}" + "translation": "This team is managed by groups. This user is not part of a group that is synched to this team." }, { "id": "api.team.add_user_to_team.added", From 9557bcf329744da9c3d9ac923c8abec516df3387 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Espino?= Date: Mon, 11 Nov 2019 13:11:50 +0100 Subject: [PATCH 08/18] Making private the sqlstore upgrade functions because nobody needs to call them from the outside (#13023) --- store/sqlstore/supplier.go | 2 +- store/sqlstore/upgrade.go | 176 ++++++++++++++++----------------- store/sqlstore/upgrade_test.go | 18 ++-- 3 files changed, 98 insertions(+), 98 deletions(-) diff --git a/store/sqlstore/supplier.go b/store/sqlstore/supplier.go index 94791b3fc9..c799408471 100644 --- a/store/sqlstore/supplier.go +++ b/store/sqlstore/supplier.go @@ -162,7 +162,7 @@ func NewSqlSupplier(settings model.SqlSettings, metrics einterfaces.MetricsInter os.Exit(EXIT_CREATE_TABLE) } - err = UpgradeDatabase(supplier, model.CurrentVersion) + err = upgradeDatabase(supplier, model.CurrentVersion) if err != nil { mlog.Critical("Failed to upgrade database.", mlog.Err(err)) time.Sleep(time.Second) diff --git a/store/sqlstore/upgrade.go b/store/sqlstore/upgrade.go index fe29f711e7..2f923ad424 100644 --- a/store/sqlstore/upgrade.go +++ b/store/sqlstore/upgrade.go @@ -70,10 +70,10 @@ const ( EXIT_TEAM_INVITEID_MIGRATION_FAILED = 1006 ) -// UpgradeDatabase attempts to migrate the schema to the latest supported version. +// upgradeDatabase attempts to migrate the schema to the latest supported version. // The value of model.CurrentVersion is accepted as a parameter for unit testing, but it is not // used to stop migrations at that version. -func UpgradeDatabase(sqlStore SqlStore, currentModelVersionString string) error { +func upgradeDatabase(sqlStore SqlStore, currentModelVersionString string) error { currentModelVersion, err := semver.Parse(currentModelVersionString) if err != nil { return errors.Wrapf(err, "failed to parse current model version %s", currentModelVersionString) @@ -122,47 +122,47 @@ func UpgradeDatabase(sqlStore SqlStore, currentModelVersionString string) error // Otherwise, apply any necessary migrations. Note that these methods currently invoke // os.Exit instead of returning an error. - UpgradeDatabaseToVersion31(sqlStore) - UpgradeDatabaseToVersion32(sqlStore) - UpgradeDatabaseToVersion33(sqlStore) - UpgradeDatabaseToVersion34(sqlStore) - UpgradeDatabaseToVersion35(sqlStore) - UpgradeDatabaseToVersion36(sqlStore) - UpgradeDatabaseToVersion37(sqlStore) - UpgradeDatabaseToVersion38(sqlStore) - UpgradeDatabaseToVersion39(sqlStore) - UpgradeDatabaseToVersion310(sqlStore) - UpgradeDatabaseToVersion40(sqlStore) - UpgradeDatabaseToVersion41(sqlStore) - UpgradeDatabaseToVersion42(sqlStore) - UpgradeDatabaseToVersion43(sqlStore) - UpgradeDatabaseToVersion44(sqlStore) - UpgradeDatabaseToVersion45(sqlStore) - UpgradeDatabaseToVersion46(sqlStore) - UpgradeDatabaseToVersion47(sqlStore) - UpgradeDatabaseToVersion471(sqlStore) - UpgradeDatabaseToVersion472(sqlStore) - UpgradeDatabaseToVersion48(sqlStore) - UpgradeDatabaseToVersion481(sqlStore) - UpgradeDatabaseToVersion49(sqlStore) - UpgradeDatabaseToVersion410(sqlStore) - UpgradeDatabaseToVersion50(sqlStore) - UpgradeDatabaseToVersion51(sqlStore) - UpgradeDatabaseToVersion52(sqlStore) - UpgradeDatabaseToVersion53(sqlStore) - UpgradeDatabaseToVersion54(sqlStore) - UpgradeDatabaseToVersion55(sqlStore) - UpgradeDatabaseToVersion56(sqlStore) - UpgradeDatabaseToVersion57(sqlStore) - UpgradeDatabaseToVersion58(sqlStore) - UpgradeDatabaseToVersion59(sqlStore) - UpgradeDatabaseToVersion510(sqlStore) - UpgradeDatabaseToVersion511(sqlStore) - UpgradeDatabaseToVersion512(sqlStore) - UpgradeDatabaseToVersion513(sqlStore) - UpgradeDatabaseToVersion514(sqlStore) - UpgradeDatabaseToVersion515(sqlStore) - UpgradeDatabaseToVersion516(sqlStore) + upgradeDatabaseToVersion31(sqlStore) + upgradeDatabaseToVersion32(sqlStore) + upgradeDatabaseToVersion33(sqlStore) + upgradeDatabaseToVersion34(sqlStore) + upgradeDatabaseToVersion35(sqlStore) + upgradeDatabaseToVersion36(sqlStore) + upgradeDatabaseToVersion37(sqlStore) + upgradeDatabaseToVersion38(sqlStore) + upgradeDatabaseToVersion39(sqlStore) + upgradeDatabaseToVersion310(sqlStore) + upgradeDatabaseToVersion40(sqlStore) + upgradeDatabaseToVersion41(sqlStore) + upgradeDatabaseToVersion42(sqlStore) + upgradeDatabaseToVersion43(sqlStore) + upgradeDatabaseToVersion44(sqlStore) + upgradeDatabaseToVersion45(sqlStore) + upgradeDatabaseToVersion46(sqlStore) + upgradeDatabaseToVersion47(sqlStore) + upgradeDatabaseToVersion471(sqlStore) + upgradeDatabaseToVersion472(sqlStore) + upgradeDatabaseToVersion48(sqlStore) + upgradeDatabaseToVersion481(sqlStore) + upgradeDatabaseToVersion49(sqlStore) + upgradeDatabaseToVersion410(sqlStore) + upgradeDatabaseToVersion50(sqlStore) + upgradeDatabaseToVersion51(sqlStore) + upgradeDatabaseToVersion52(sqlStore) + upgradeDatabaseToVersion53(sqlStore) + upgradeDatabaseToVersion54(sqlStore) + upgradeDatabaseToVersion55(sqlStore) + upgradeDatabaseToVersion56(sqlStore) + upgradeDatabaseToVersion57(sqlStore) + upgradeDatabaseToVersion58(sqlStore) + upgradeDatabaseToVersion59(sqlStore) + upgradeDatabaseToVersion510(sqlStore) + upgradeDatabaseToVersion511(sqlStore) + upgradeDatabaseToVersion512(sqlStore) + upgradeDatabaseToVersion513(sqlStore) + upgradeDatabaseToVersion514(sqlStore) + upgradeDatabaseToVersion515(sqlStore) + upgradeDatabaseToVersion516(sqlStore) return nil } @@ -187,14 +187,14 @@ func shouldPerformUpgrade(sqlStore SqlStore, currentSchemaVersion string, expect return false } -func UpgradeDatabaseToVersion31(sqlStore SqlStore) { +func upgradeDatabaseToVersion31(sqlStore SqlStore) { if shouldPerformUpgrade(sqlStore, VERSION_3_0_0, VERSION_3_1_0) { sqlStore.CreateColumnIfNotExists("OutgoingWebhooks", "ContentType", "varchar(128)", "varchar(128)", "") saveSchemaVersion(sqlStore, VERSION_3_1_0) } } -func UpgradeDatabaseToVersion32(sqlStore SqlStore) { +func upgradeDatabaseToVersion32(sqlStore SqlStore) { if shouldPerformUpgrade(sqlStore, VERSION_3_1_0, VERSION_3_2_0) { sqlStore.CreateColumnIfNotExists("TeamMembers", "DeleteAt", "bigint(20)", "bigint", "0") @@ -208,7 +208,7 @@ func themeMigrationFailed(err error) { os.Exit(EXIT_THEME_MIGRATION) } -func UpgradeDatabaseToVersion33(sqlStore SqlStore) { +func upgradeDatabaseToVersion33(sqlStore SqlStore) { if shouldPerformUpgrade(sqlStore, VERSION_3_2_0, VERSION_3_3_0) { if sqlStore.DoesColumnExist("Users", "ThemeProps") { params := map[string]interface{}{ @@ -291,7 +291,7 @@ func UpgradeDatabaseToVersion33(sqlStore SqlStore) { } } -func UpgradeDatabaseToVersion34(sqlStore SqlStore) { +func upgradeDatabaseToVersion34(sqlStore SqlStore) { if shouldPerformUpgrade(sqlStore, VERSION_3_3_0, VERSION_3_4_0) { sqlStore.CreateColumnIfNotExists("Status", "Manual", "BOOLEAN", "BOOLEAN", "0") sqlStore.CreateColumnIfNotExists("Status", "ActiveChannel", "varchar(26)", "varchar(26)", "") @@ -300,7 +300,7 @@ func UpgradeDatabaseToVersion34(sqlStore SqlStore) { } } -func UpgradeDatabaseToVersion35(sqlStore SqlStore) { +func upgradeDatabaseToVersion35(sqlStore SqlStore) { if shouldPerformUpgrade(sqlStore, VERSION_3_4_0, VERSION_3_5_0) { sqlStore.GetMaster().Exec("UPDATE Users SET Roles = 'system_user' WHERE Roles = ''") sqlStore.GetMaster().Exec("UPDATE Users SET Roles = 'system_user system_admin' WHERE Roles = 'system_admin'") @@ -323,7 +323,7 @@ func UpgradeDatabaseToVersion35(sqlStore SqlStore) { } } -func UpgradeDatabaseToVersion36(sqlStore SqlStore) { +func upgradeDatabaseToVersion36(sqlStore SqlStore) { if shouldPerformUpgrade(sqlStore, VERSION_3_5_0, VERSION_3_6_0) { sqlStore.CreateColumnIfNotExists("Posts", "HasReactions", "tinyint", "boolean", "0") @@ -340,7 +340,7 @@ func UpgradeDatabaseToVersion36(sqlStore SqlStore) { } } -func UpgradeDatabaseToVersion37(sqlStore SqlStore) { +func upgradeDatabaseToVersion37(sqlStore SqlStore) { if shouldPerformUpgrade(sqlStore, VERSION_3_6_0, VERSION_3_7_0) { // Add EditAt column to Posts sqlStore.CreateColumnIfNotExists("Posts", "EditAt", " bigint", " bigint", "0") @@ -349,7 +349,7 @@ func UpgradeDatabaseToVersion37(sqlStore SqlStore) { } } -func UpgradeDatabaseToVersion38(sqlStore SqlStore) { +func upgradeDatabaseToVersion38(sqlStore SqlStore) { if shouldPerformUpgrade(sqlStore, VERSION_3_7_0, VERSION_3_8_0) { // Add the IsPinned column to posts. sqlStore.CreateColumnIfNotExists("Posts", "IsPinned", "boolean", "boolean", "0") @@ -358,7 +358,7 @@ func UpgradeDatabaseToVersion38(sqlStore SqlStore) { } } -func UpgradeDatabaseToVersion39(sqlStore SqlStore) { +func upgradeDatabaseToVersion39(sqlStore SqlStore) { if shouldPerformUpgrade(sqlStore, VERSION_3_8_0, VERSION_3_9_0) { sqlStore.CreateColumnIfNotExists("OAuthAccessData", "Scope", "varchar(128)", "varchar(128)", model.DEFAULT_SCOPE) sqlStore.RemoveTableIfExists("PasswordRecovery") @@ -367,19 +367,19 @@ func UpgradeDatabaseToVersion39(sqlStore SqlStore) { } } -func UpgradeDatabaseToVersion310(sqlStore SqlStore) { +func upgradeDatabaseToVersion310(sqlStore SqlStore) { if shouldPerformUpgrade(sqlStore, VERSION_3_9_0, VERSION_3_10_0) { saveSchemaVersion(sqlStore, VERSION_3_10_0) } } -func UpgradeDatabaseToVersion40(sqlStore SqlStore) { +func upgradeDatabaseToVersion40(sqlStore SqlStore) { if shouldPerformUpgrade(sqlStore, VERSION_3_10_0, VERSION_4_0_0) { saveSchemaVersion(sqlStore, VERSION_4_0_0) } } -func UpgradeDatabaseToVersion41(sqlStore SqlStore) { +func upgradeDatabaseToVersion41(sqlStore SqlStore) { if shouldPerformUpgrade(sqlStore, VERSION_4_0_0, VERSION_4_1_0) { // Increase maximum length of the Users table Roles column. if sqlStore.GetMaxLengthOfColumnIfExists("Users", "Roles") != "256" { @@ -392,19 +392,19 @@ func UpgradeDatabaseToVersion41(sqlStore SqlStore) { } } -func UpgradeDatabaseToVersion42(sqlStore SqlStore) { +func upgradeDatabaseToVersion42(sqlStore SqlStore) { if shouldPerformUpgrade(sqlStore, VERSION_4_1_0, VERSION_4_2_0) { saveSchemaVersion(sqlStore, VERSION_4_2_0) } } -func UpgradeDatabaseToVersion43(sqlStore SqlStore) { +func upgradeDatabaseToVersion43(sqlStore SqlStore) { if shouldPerformUpgrade(sqlStore, VERSION_4_2_0, VERSION_4_3_0) { saveSchemaVersion(sqlStore, VERSION_4_3_0) } } -func UpgradeDatabaseToVersion44(sqlStore SqlStore) { +func upgradeDatabaseToVersion44(sqlStore SqlStore) { if shouldPerformUpgrade(sqlStore, VERSION_4_3_0, VERSION_4_4_0) { // Add the IsActive column to UserAccessToken. sqlStore.CreateColumnIfNotExists("UserAccessTokens", "IsActive", "boolean", "boolean", "1") @@ -413,13 +413,13 @@ func UpgradeDatabaseToVersion44(sqlStore SqlStore) { } } -func UpgradeDatabaseToVersion45(sqlStore SqlStore) { +func upgradeDatabaseToVersion45(sqlStore SqlStore) { if shouldPerformUpgrade(sqlStore, VERSION_4_4_0, VERSION_4_5_0) { saveSchemaVersion(sqlStore, VERSION_4_5_0) } } -func UpgradeDatabaseToVersion46(sqlStore SqlStore) { +func upgradeDatabaseToVersion46(sqlStore SqlStore) { if shouldPerformUpgrade(sqlStore, VERSION_4_5_0, VERSION_4_6_0) { sqlStore.CreateColumnIfNotExists("IncomingWebhooks", "Username", "varchar(64)", "varchar(64)", "") sqlStore.CreateColumnIfNotExists("IncomingWebhooks", "IconURL", "varchar(1024)", "varchar(1024)", "") @@ -427,7 +427,7 @@ func UpgradeDatabaseToVersion46(sqlStore SqlStore) { } } -func UpgradeDatabaseToVersion47(sqlStore SqlStore) { +func upgradeDatabaseToVersion47(sqlStore SqlStore) { if shouldPerformUpgrade(sqlStore, VERSION_4_6_0, VERSION_4_7_0) { sqlStore.AlterColumnTypeIfExists("Users", "Position", "varchar(128)", "varchar(128)") sqlStore.AlterColumnTypeIfExists("OAuthAuthData", "State", "varchar(1024)", "varchar(1024)") @@ -437,37 +437,37 @@ func UpgradeDatabaseToVersion47(sqlStore SqlStore) { } } -// If any new instances started with 4.7, they would have the bad Email column on the -// ChannelMemberHistory table. So for those cases we need to do an upgrade between -// 4.7.0 and 4.7.1 -func UpgradeDatabaseToVersion471(sqlStore SqlStore) { +func upgradeDatabaseToVersion471(sqlStore SqlStore) { + // If any new instances started with 4.7, they would have the bad Email column on the + // ChannelMemberHistory table. So for those cases we need to do an upgrade between + // 4.7.0 and 4.7.1 if shouldPerformUpgrade(sqlStore, VERSION_4_7_0, VERSION_4_7_1) { sqlStore.RemoveColumnIfExists("ChannelMemberHistory", "Email") saveSchemaVersion(sqlStore, VERSION_4_7_1) } } -func UpgradeDatabaseToVersion472(sqlStore SqlStore) { +func upgradeDatabaseToVersion472(sqlStore SqlStore) { if shouldPerformUpgrade(sqlStore, VERSION_4_7_1, VERSION_4_7_2) { sqlStore.RemoveIndexIfExists("idx_channels_displayname", "Channels") saveSchemaVersion(sqlStore, VERSION_4_7_2) } } -func UpgradeDatabaseToVersion48(sqlStore SqlStore) { +func upgradeDatabaseToVersion48(sqlStore SqlStore) { if shouldPerformUpgrade(sqlStore, VERSION_4_7_2, VERSION_4_8_0) { saveSchemaVersion(sqlStore, VERSION_4_8_0) } } -func UpgradeDatabaseToVersion481(sqlStore SqlStore) { +func upgradeDatabaseToVersion481(sqlStore SqlStore) { if shouldPerformUpgrade(sqlStore, VERSION_4_8_0, VERSION_4_8_1) { sqlStore.RemoveIndexIfExists("idx_channels_displayname", "Channels") saveSchemaVersion(sqlStore, VERSION_4_8_1) } } -func UpgradeDatabaseToVersion49(sqlStore SqlStore) { +func upgradeDatabaseToVersion49(sqlStore SqlStore) { // This version of Mattermost includes an App-Layer migration which migrates from hard-coded roles configured by // a number of parameters in `config.json` to a `Roles` table in the database. The migration code can be seen // in the file `app/app.go` in the function `DoAdvancedPermissionsMigration()`. @@ -485,7 +485,7 @@ func UpgradeDatabaseToVersion49(sqlStore SqlStore) { } } -func UpgradeDatabaseToVersion410(sqlStore SqlStore) { +func upgradeDatabaseToVersion410(sqlStore SqlStore) { if shouldPerformUpgrade(sqlStore, VERSION_4_9_0, VERSION_4_10_0) { sqlStore.RemoveIndexIfExists("Name_2", "Channels") @@ -497,7 +497,7 @@ func UpgradeDatabaseToVersion410(sqlStore SqlStore) { } } -func UpgradeDatabaseToVersion50(sqlStore SqlStore) { +func upgradeDatabaseToVersion50(sqlStore SqlStore) { // This version of Mattermost includes an App-Layer migration which migrates from hard-coded emojis configured // in `config.json` to a `Permission` in the database. The migration code can be seen // in the file `app/app.go` in the function `DoEmojisPermissionsMigration()`. @@ -536,13 +536,13 @@ func UpgradeDatabaseToVersion50(sqlStore SqlStore) { } } -func UpgradeDatabaseToVersion51(sqlStore SqlStore) { +func upgradeDatabaseToVersion51(sqlStore SqlStore) { if shouldPerformUpgrade(sqlStore, VERSION_5_0_0, VERSION_5_1_0) { saveSchemaVersion(sqlStore, VERSION_5_1_0) } } -func UpgradeDatabaseToVersion52(sqlStore SqlStore) { +func upgradeDatabaseToVersion52(sqlStore SqlStore) { if shouldPerformUpgrade(sqlStore, VERSION_5_1_0, VERSION_5_2_0) { sqlStore.CreateColumnIfNotExists("OutgoingWebhooks", "Username", "varchar(64)", "varchar(64)", "") sqlStore.CreateColumnIfNotExists("OutgoingWebhooks", "IconURL", "varchar(1024)", "varchar(1024)", "") @@ -550,13 +550,13 @@ func UpgradeDatabaseToVersion52(sqlStore SqlStore) { } } -func UpgradeDatabaseToVersion53(sqlStore SqlStore) { +func upgradeDatabaseToVersion53(sqlStore SqlStore) { if shouldPerformUpgrade(sqlStore, VERSION_5_2_0, VERSION_5_3_0) { saveSchemaVersion(sqlStore, VERSION_5_3_0) } } -func UpgradeDatabaseToVersion54(sqlStore SqlStore) { +func upgradeDatabaseToVersion54(sqlStore SqlStore) { if shouldPerformUpgrade(sqlStore, VERSION_5_3_0, VERSION_5_4_0) { sqlStore.AlterColumnTypeIfExists("OutgoingWebhooks", "Description", "varchar(500)", "varchar(500)") sqlStore.AlterColumnTypeIfExists("IncomingWebhooks", "Description", "varchar(500)", "varchar(500)") @@ -569,13 +569,13 @@ func UpgradeDatabaseToVersion54(sqlStore SqlStore) { } } -func UpgradeDatabaseToVersion55(sqlStore SqlStore) { +func upgradeDatabaseToVersion55(sqlStore SqlStore) { if shouldPerformUpgrade(sqlStore, VERSION_5_4_0, VERSION_5_5_0) { saveSchemaVersion(sqlStore, VERSION_5_5_0) } } -func UpgradeDatabaseToVersion56(sqlStore SqlStore) { +func upgradeDatabaseToVersion56(sqlStore SqlStore) { if shouldPerformUpgrade(sqlStore, VERSION_5_5_0, VERSION_5_6_0) { sqlStore.CreateColumnIfNotExists("PluginKeyValueStore", "ExpireAt", "bigint(20)", "bigint", "0") @@ -595,15 +595,15 @@ func UpgradeDatabaseToVersion56(sqlStore SqlStore) { } -func UpgradeDatabaseToVersion57(sqlStore SqlStore) { +func upgradeDatabaseToVersion57(sqlStore SqlStore) { if shouldPerformUpgrade(sqlStore, VERSION_5_6_0, VERSION_5_7_0) { saveSchemaVersion(sqlStore, VERSION_5_7_0) } } -func UpgradeDatabaseToVersion58(sqlStore SqlStore) { +func upgradeDatabaseToVersion58(sqlStore SqlStore) { if shouldPerformUpgrade(sqlStore, VERSION_5_7_0, VERSION_5_8_0) { - // idx_channels_txt was removed in `UpgradeDatabaseToVersion50`, but merged as part of + // idx_channels_txt was removed in `upgradeDatabaseToVersion50`, but merged as part of // v5.1, so the migration wouldn't apply to anyone upgrading from v5.0. Remove it again to // bring the upgraded (from v5.0) and fresh install schemas back in sync. sqlStore.RemoveIndexIfExists("idx_channels_txt", "Channels") @@ -621,13 +621,13 @@ func UpgradeDatabaseToVersion58(sqlStore SqlStore) { } } -func UpgradeDatabaseToVersion59(sqlStore SqlStore) { +func upgradeDatabaseToVersion59(sqlStore SqlStore) { if shouldPerformUpgrade(sqlStore, VERSION_5_8_0, VERSION_5_9_0) { saveSchemaVersion(sqlStore, VERSION_5_9_0) } } -func UpgradeDatabaseToVersion510(sqlStore SqlStore) { +func upgradeDatabaseToVersion510(sqlStore SqlStore) { if shouldPerformUpgrade(sqlStore, VERSION_5_9_0, VERSION_5_10_0) { sqlStore.CreateColumnIfNotExistsNoDefault("Channels", "GroupConstrained", "tinyint(4)", "boolean") sqlStore.CreateColumnIfNotExistsNoDefault("Teams", "GroupConstrained", "tinyint(4)", "boolean") @@ -639,7 +639,7 @@ func UpgradeDatabaseToVersion510(sqlStore SqlStore) { } } -func UpgradeDatabaseToVersion511(sqlStore SqlStore) { +func upgradeDatabaseToVersion511(sqlStore SqlStore) { if shouldPerformUpgrade(sqlStore, VERSION_5_10_0, VERSION_5_11_0) { // Enforce all teams have an InviteID set var teams []*model.Team @@ -658,7 +658,7 @@ func UpgradeDatabaseToVersion511(sqlStore SqlStore) { } } -func UpgradeDatabaseToVersion512(sqlStore SqlStore) { +func upgradeDatabaseToVersion512(sqlStore SqlStore) { if shouldPerformUpgrade(sqlStore, VERSION_5_11_0, VERSION_5_12_0) { sqlStore.CreateColumnIfNotExistsNoDefault("TeamMembers", "SchemeGuest", "boolean", "boolean") sqlStore.CreateColumnIfNotExistsNoDefault("ChannelMembers", "SchemeGuest", "boolean", "boolean") @@ -674,7 +674,7 @@ func UpgradeDatabaseToVersion512(sqlStore SqlStore) { } } -func UpgradeDatabaseToVersion513(sqlStore SqlStore) { +func upgradeDatabaseToVersion513(sqlStore SqlStore) { if shouldPerformUpgrade(sqlStore, VERSION_5_12_0, VERSION_5_13_0) { // The previous jobs ran once per minute, cluttering the Jobs table with somewhat useless entries. Clean that up. sqlStore.GetMaster().Exec("DELETE FROM Jobs WHERE Type = 'plugins'") @@ -683,19 +683,19 @@ func UpgradeDatabaseToVersion513(sqlStore SqlStore) { } } -func UpgradeDatabaseToVersion514(sqlStore SqlStore) { +func upgradeDatabaseToVersion514(sqlStore SqlStore) { if shouldPerformUpgrade(sqlStore, VERSION_5_13_0, VERSION_5_14_0) { saveSchemaVersion(sqlStore, VERSION_5_14_0) } } -func UpgradeDatabaseToVersion515(sqlStore SqlStore) { +func upgradeDatabaseToVersion515(sqlStore SqlStore) { if shouldPerformUpgrade(sqlStore, VERSION_5_14_0, VERSION_5_15_0) { saveSchemaVersion(sqlStore, VERSION_5_15_0) } } -func UpgradeDatabaseToVersion516(sqlStore SqlStore) { +func upgradeDatabaseToVersion516(sqlStore SqlStore) { if shouldPerformUpgrade(sqlStore, VERSION_5_15_0, VERSION_5_16_0) { if sqlStore.DriverName() == model.DATABASE_DRIVER_POSTGRES { sqlStore.GetMaster().Exec("ALTER TABLE Tokens ALTER COLUMN Extra TYPE varchar(2048)") diff --git a/store/sqlstore/upgrade_test.go b/store/sqlstore/upgrade_test.go index 16b93f3a20..4bafc88140 100644 --- a/store/sqlstore/upgrade_test.go +++ b/store/sqlstore/upgrade_test.go @@ -15,41 +15,41 @@ func TestStoreUpgrade(t *testing.T) { sqlStore := ss.(SqlStore) t.Run("invalid currentModelVersion", func(t *testing.T) { - err := UpgradeDatabase(sqlStore, "notaversion") + err := upgradeDatabase(sqlStore, "notaversion") require.EqualError(t, err, "failed to parse current model version notaversion: No Major.Minor.Patch elements found") }) t.Run("upgrade from invalid version", func(t *testing.T) { saveSchemaVersion(sqlStore, "invalid") - err := UpgradeDatabase(sqlStore, "5.8.0") + err := upgradeDatabase(sqlStore, "5.8.0") require.EqualError(t, err, "failed to parse database schema version invalid: No Major.Minor.Patch elements found") require.Equal(t, "invalid", sqlStore.GetCurrentSchemaVersion()) }) t.Run("upgrade from unsupported version", func(t *testing.T) { saveSchemaVersion(sqlStore, "2.0.0") - err := UpgradeDatabase(sqlStore, "5.8.0") + err := upgradeDatabase(sqlStore, "5.8.0") require.EqualError(t, err, "Database schema version 2.0.0 is no longer supported. This Mattermost server supports automatic upgrades from schema version 3.0.0 through schema version 5.8.0. Please manually upgrade to at least version 3.0.0 before continuing.") require.Equal(t, "2.0.0", sqlStore.GetCurrentSchemaVersion()) }) t.Run("upgrade from earliest supported version", func(t *testing.T) { saveSchemaVersion(sqlStore, VERSION_3_0_0) - err := UpgradeDatabase(sqlStore, CURRENT_SCHEMA_VERSION) + err := upgradeDatabase(sqlStore, CURRENT_SCHEMA_VERSION) require.NoError(t, err) require.Equal(t, CURRENT_SCHEMA_VERSION, sqlStore.GetCurrentSchemaVersion()) }) t.Run("upgrade from no existing version", func(t *testing.T) { saveSchemaVersion(sqlStore, "") - err := UpgradeDatabase(sqlStore, CURRENT_SCHEMA_VERSION) + err := upgradeDatabase(sqlStore, CURRENT_SCHEMA_VERSION) require.NoError(t, err) require.Equal(t, CURRENT_SCHEMA_VERSION, sqlStore.GetCurrentSchemaVersion()) }) t.Run("upgrade schema running earlier minor version", func(t *testing.T) { saveSchemaVersion(sqlStore, "5.1.0") - err := UpgradeDatabase(sqlStore, "5.8.0") + err := upgradeDatabase(sqlStore, "5.8.0") require.NoError(t, err) // Assert CURRENT_SCHEMA_VERSION, not 5.8.0, since the migrations will move // past 5.8.0 regardless of the input parameter. @@ -58,21 +58,21 @@ func TestStoreUpgrade(t *testing.T) { t.Run("upgrade schema running later minor version", func(t *testing.T) { saveSchemaVersion(sqlStore, "5.29.0") - err := UpgradeDatabase(sqlStore, "5.8.0") + err := upgradeDatabase(sqlStore, "5.8.0") require.NoError(t, err) require.Equal(t, "5.29.0", sqlStore.GetCurrentSchemaVersion()) }) t.Run("upgrade schema running earlier major version", func(t *testing.T) { saveSchemaVersion(sqlStore, "4.1.0") - err := UpgradeDatabase(sqlStore, CURRENT_SCHEMA_VERSION) + err := upgradeDatabase(sqlStore, CURRENT_SCHEMA_VERSION) require.NoError(t, err) require.Equal(t, CURRENT_SCHEMA_VERSION, sqlStore.GetCurrentSchemaVersion()) }) t.Run("upgrade schema running later major version", func(t *testing.T) { saveSchemaVersion(sqlStore, "6.0.0") - err := UpgradeDatabase(sqlStore, "5.8.0") + err := upgradeDatabase(sqlStore, "5.8.0") require.EqualError(t, err, "Database schema version 6.0.0 is not supported. This Mattermost server supports only >=5.8.0, <6.0.0. Please upgrade to at least version 6.0.0 before continuing.") require.Equal(t, "6.0.0", sqlStore.GetCurrentSchemaVersion()) }) From 1930cc6a114f9169636577a4063a896400478d1e Mon Sep 17 00:00:00 2001 From: George Goldberg Date: Mon, 11 Nov 2019 14:33:30 +0000 Subject: [PATCH 09/18] MM-19790: Remove randomness in oauth test AuthData. (#12988) The ID field of Gitlab OAuth data was generated as a random number between 1-1000. Unfortunately, this gave a 1 in 1 million chance of two users generated this way colliding and meaning if you had two test users in a test case they silently ended up actually being the exact same user. We don't actually need to generate this randomly, so instead change to having the test cases explicitly set the ID number so the behaviour is as expected and completely deterministic. As you'd expect, with odds of exactly a million to 1, it just might (and in this case, it did indeed) happen. --- app/user_test.go | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/app/user_test.go b/app/user_test.go index 078f03ff56..f0316bfcb9 100644 --- a/app/user_test.go +++ b/app/user_test.go @@ -8,7 +8,6 @@ import ( "encoding/json" "image" "image/color" - "math/rand" "strings" "testing" "time" @@ -74,8 +73,7 @@ func TestCreateOAuthUser(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() - r := rand.New(rand.NewSource(time.Now().UnixNano())) - glUser := oauthgitlab.GitLabUser{Id: int64(r.Intn(1000)) + 1, Username: "o" + model.NewId(), Email: model.NewId() + "@simulator.amazonses.com", Name: "Joram Wilander"} + glUser := oauthgitlab.GitLabUser{Id: 42, Username: "o" + model.NewId(), Email: model.NewId() + "@simulator.amazonses.com", Name: "Joram Wilander"} json := glUser.ToJson() user, err := th.App.CreateOAuthUser(model.USER_AUTH_SERVICE_GITLAB, strings.NewReader(json), th.BasicTeam.Id) @@ -253,8 +251,8 @@ func TestUpdateOAuthUserAttrs(t *testing.T) { var user, user2 *model.User var gitlabUserObj oauthgitlab.GitLabUser - user, gitlabUserObj = createGitlabUser(t, th.App, username, email) - user2, _ = createGitlabUser(t, th.App, username2, email2) + user, gitlabUserObj = createGitlabUser(t, th.App, 1, username, email) + user2, _ = createGitlabUser(t, th.App, 2, username2, email2) t.Run("UpdateUsername", func(t *testing.T) { t.Run("NoExistingUserWithSameUsername", func(t *testing.T) { @@ -443,9 +441,8 @@ func getGitlabUserPayload(gitlabUser oauthgitlab.GitLabUser, t *testing.T) []byt return payload } -func createGitlabUser(t *testing.T, a *App, username string, email string) (*model.User, oauthgitlab.GitLabUser) { - r := rand.New(rand.NewSource(time.Now().UnixNano())) - gitlabUserObj := oauthgitlab.GitLabUser{Id: int64(r.Intn(1000)) + 1, Username: username, Login: "user1", Email: email, Name: "Test User"} +func createGitlabUser(t *testing.T, a *App, id int64, username string, email string) (*model.User, oauthgitlab.GitLabUser) { + gitlabUserObj := oauthgitlab.GitLabUser{Id: id, Username: username, Login: "user1", Email: email, Name: "Test User"} gitlabUser := getGitlabUserPayload(gitlabUserObj, t) var user *model.User From 36f3b144208b6d17fc0a2520f509a3a6b138fa87 Mon Sep 17 00:00:00 2001 From: Valentijn Nieman Date: Mon, 11 Nov 2019 17:15:19 -0500 Subject: [PATCH 10/18] MM-19337 Enable users to view archived channels (#12995) * First pass at searching archived channels * Create endpoint for archived channels * Update test for GetPublicChannelsForTeam * Fix unit test for archived channels * Rename includeDeleted to onlyDeleted * Remove new /archived endpoint in favour of existing /deleted endpoint * Fix broken test * Remove manage team permission from /deleted endpoint * Fix deletedChannels test * Test for searching archived channels * Only return private deleted channels user was a member of * SearchArchivedChannels also searches private channels (user is a member of) * Remove for loop to simplify append * Remove userId from Client4 searcArchivedChannels --- api4/channel.go | 44 ++++++++-- api4/channel_test.go | 122 +++++++++++++++++++++++++- app/channel.go | 10 ++- model/client4.go | 10 +++ store/sqlstore/channel_store.go | 71 ++++++++++++++- store/store.go | 3 +- store/storetest/channel_store.go | 12 +-- store/storetest/mocks/ChannelStore.go | 27 +++++- store/timer_layer.go | 4 +- 9 files changed, 280 insertions(+), 23 deletions(-) diff --git a/api4/channel.go b/api4/channel.go index 84275acc09..99c7d65134 100644 --- a/api4/channel.go +++ b/api4/channel.go @@ -27,6 +27,7 @@ func (api *API) InitChannel() { api.BaseRoutes.ChannelsForTeam.Handle("/deleted", api.ApiSessionRequired(getDeletedChannelsForTeam)).Methods("GET") api.BaseRoutes.ChannelsForTeam.Handle("/ids", api.ApiSessionRequired(getPublicChannelsByIdsForTeam)).Methods("POST") api.BaseRoutes.ChannelsForTeam.Handle("/search", api.ApiSessionRequired(searchChannelsForTeam)).Methods("POST") + api.BaseRoutes.ChannelsForTeam.Handle("/search_archived", api.ApiSessionRequired(searchArchivedChannelsForTeam)).Methods("POST") api.BaseRoutes.ChannelsForTeam.Handle("/autocomplete", api.ApiSessionRequired(autocompleteChannelsForTeam)).Methods("GET") api.BaseRoutes.ChannelsForTeam.Handle("/search_autocomplete", api.ApiSessionRequired(autocompleteChannelsForTeamForSearch)).Methods("GET") api.BaseRoutes.User.Handle("/teams/{team_id:[A-Za-z0-9]+}/channels", api.ApiSessionRequired(getChannelsForTeamForUser)).Methods("GET") @@ -681,12 +682,7 @@ func getDeletedChannelsForTeam(c *Context, w http.ResponseWriter, r *http.Reques return } - if !c.App.SessionHasPermissionToTeam(c.App.Session, c.Params.TeamId, model.PERMISSION_MANAGE_TEAM) { - c.SetPermissionError(model.PERMISSION_MANAGE_TEAM) - return - } - - channels, err := c.App.GetDeletedChannels(c.Params.TeamId, c.Params.Page*c.Params.PerPage, c.Params.PerPage) + channels, err := c.App.GetDeletedChannels(c.Params.TeamId, c.Params.Page*c.Params.PerPage, c.Params.PerPage, c.App.Session.UserId) if err != nil { c.Err = err return @@ -860,6 +856,42 @@ func searchChannelsForTeam(c *Context, w http.ResponseWriter, r *http.Request) { w.Write([]byte(channels.ToJson())) } +func searchArchivedChannelsForTeam(c *Context, w http.ResponseWriter, r *http.Request) { + c.RequireTeamId() + if c.Err != nil { + return + } + + props := model.ChannelSearchFromJson(r.Body) + if props == nil { + c.SetInvalidParam("channel_search") + return + } + + var channels *model.ChannelList + var err *model.AppError + if c.App.SessionHasPermissionToTeam(c.App.Session, c.Params.TeamId, model.PERMISSION_LIST_TEAM_CHANNELS) { + channels, err = c.App.SearchArchivedChannels(c.Params.TeamId, props.Term, c.App.Session.UserId) + } else { + // If the user is not a team member, return a 404 + if _, err = c.App.GetTeamMember(c.Params.TeamId, c.App.Session.UserId); err != nil { + c.Err = err + return + } + + channels, err = c.App.SearchArchivedChannels(c.Params.TeamId, props.Term, c.App.Session.UserId) + } + + if err != nil { + c.Err = err + return + } + + // Don't fill in channels props, since unused by client and potentially expensive. + + w.Write([]byte(channels.ToJson())) +} + func searchAllChannels(c *Context, w http.ResponseWriter, r *http.Request) { props := model.ChannelSearchFromJson(r.Body) if props == nil { diff --git a/api4/channel_test.go b/api4/channel_test.go index 3b634cd84a..4cefce513a 100644 --- a/api4/channel_test.go +++ b/api4/channel_test.go @@ -678,9 +678,6 @@ func TestGetDeletedChannelsForTeam(t *testing.T) { Client := th.Client team := th.BasicTeam - _, resp := Client.GetDeletedChannelsForTeam(team.Id, 0, 100, "") - CheckForbiddenStatus(t, resp) - th.LoginTeamAdmin() channels, resp := Client.GetDeletedChannelsForTeam(team.Id, 0, 100, "") @@ -706,6 +703,31 @@ func TestGetDeletedChannelsForTeam(t *testing.T) { t.Fatal("should be 2 deleted channels") } + th.LoginBasic() + + privateChannel1 := th.CreatePrivateChannel() + Client.DeleteChannel(privateChannel1.Id) + + channels, resp = Client.GetDeletedChannelsForTeam(team.Id, 0, 100, "") + CheckNoError(t, resp) + if len(channels) != numInitialChannelsForTeam+3 { + t.Fatal("should be 3 deleted channels") + } + + // Login as different user and create private channel + th.LoginBasic2() + privateChannel2 := th.CreatePrivateChannel() + Client.DeleteChannel(privateChannel2.Id) + + // Log back in as first user + th.LoginBasic() + + channels, resp = Client.GetDeletedChannelsForTeam(team.Id, 0, 100, "") + CheckNoError(t, resp) + if len(channels) != numInitialChannelsForTeam+3 { + t.Fatal("should still be 3 deleted channels", len(channels), numInitialChannelsForTeam+3) + } + channels, resp = Client.GetDeletedChannelsForTeam(team.Id, 0, 1, "") CheckNoError(t, resp) if len(channels) != 1 { @@ -1060,6 +1082,100 @@ func TestSearchChannels(t *testing.T) { }) } +func TestSearchArchivedChannels(t *testing.T) { + th := Setup().InitBasic() + defer th.TearDown() + Client := th.Client + + search := &model.ChannelSearch{Term: th.BasicChannel.Name} + + Client.DeleteChannel(th.BasicChannel.Id) + + channels, resp := Client.SearchArchivedChannels(th.BasicTeam.Id, search) + CheckNoError(t, resp) + + found := false + for _, c := range channels { + if c.Type != model.CHANNEL_OPEN { + t.Fatal("should only return public channels") + } + + if c.Id == th.BasicChannel.Id { + found = true + } + } + + if !found { + t.Fatal("didn't find channel") + } + + search.Term = th.BasicPrivateChannel.Name + Client.DeleteChannel(th.BasicPrivateChannel.Id) + + channels, resp = Client.SearchArchivedChannels(th.BasicTeam.Id, search) + CheckNoError(t, resp) + + found = false + for _, c := range channels { + if c.Id == th.BasicPrivateChannel.Id { + found = true + } + } + + if !found { + t.Fatal("couldn't find private channel") + } + + search.Term = "" + _, resp = Client.SearchArchivedChannels(th.BasicTeam.Id, search) + CheckNoError(t, resp) + + search.Term = th.BasicDeletedChannel.Name + _, resp = Client.SearchArchivedChannels(model.NewId(), search) + CheckNotFoundStatus(t, resp) + + _, resp = Client.SearchArchivedChannels("junk", search) + CheckBadRequestStatus(t, resp) + + _, resp = th.SystemAdminClient.SearchArchivedChannels(th.BasicTeam.Id, search) + CheckNoError(t, resp) + + // Check the appropriate permissions are enforced. + defaultRolePermissions := th.SaveDefaultRolePermissions() + defer func() { + th.RestoreDefaultRolePermissions(defaultRolePermissions) + }() + + // Remove list channels permission from the user + th.RemovePermissionFromRole(model.PERMISSION_LIST_TEAM_CHANNELS.Id, model.TEAM_USER_ROLE_ID) + + t.Run("Search for a BasicDeletedChannel, which the user is a member of", func(t *testing.T) { + search.Term = th.BasicDeletedChannel.Name + channelList, resp := Client.SearchArchivedChannels(th.BasicTeam.Id, search) + CheckNoError(t, resp) + + channelNames := []string{} + for _, c := range channelList { + channelNames = append(channelNames, c.Name) + } + require.Contains(t, channelNames, th.BasicDeletedChannel.Name) + }) + + t.Run("Remove the user from BasicDeletedChannel and search again, should still return", func(t *testing.T) { + th.App.RemoveUserFromChannel(th.BasicUser.Id, th.BasicUser.Id, th.BasicDeletedChannel) + + search.Term = th.BasicDeletedChannel.Name + channelList, resp := Client.SearchArchivedChannels(th.BasicTeam.Id, search) + CheckNoError(t, resp) + + channelNames := []string{} + for _, c := range channelList { + channelNames = append(channelNames, c.Name) + } + require.Contains(t, channelNames, th.BasicDeletedChannel.Name) + }) +} + func TestSearchAllChannels(t *testing.T) { th := Setup().InitBasic() defer th.TearDown() diff --git a/app/channel.go b/app/channel.go index aa0e54bbb8..cfde9ccd17 100644 --- a/app/channel.go +++ b/app/channel.go @@ -1260,8 +1260,8 @@ func (a *App) GetAllChannelsCount(opts model.ChannelSearchOpts) (int64, *model.A return a.Srv.Store.Channel().GetAllChannelsCount(storeOpts) } -func (a *App) GetDeletedChannels(teamId string, offset int, limit int) (*model.ChannelList, *model.AppError) { - return a.Srv.Store.Channel().GetDeleted(teamId, offset, limit) +func (a *App) GetDeletedChannels(teamId string, offset int, limit int, userId string) (*model.ChannelList, *model.AppError) { + return a.Srv.Store.Channel().GetDeleted(teamId, offset, limit, userId) } func (a *App) GetChannelsUserNotIn(teamId string, userId string, offset int, limit int) (*model.ChannelList, *model.AppError) { @@ -1860,6 +1860,12 @@ func (a *App) SearchChannels(teamId string, term string) (*model.ChannelList, *m return a.Srv.Store.Channel().SearchInTeam(teamId, term, includeDeleted) } +func (a *App) SearchArchivedChannels(teamId string, term string, userId string) (*model.ChannelList, *model.AppError) { + term = strings.TrimSpace(term) + + return a.Srv.Store.Channel().SearchArchivedInTeam(teamId, term, userId) +} + func (a *App) SearchChannelsForUser(userId, teamId, term string) (*model.ChannelList, *model.AppError) { includeDeleted := *a.Config().TeamSettings.ExperimentalViewArchivedChannels diff --git a/model/client4.go b/model/client4.go index 2dffa31291..7bc8730ddd 100644 --- a/model/client4.go +++ b/model/client4.go @@ -2235,6 +2235,16 @@ func (c *Client4) SearchChannels(teamId string, search *ChannelSearch) ([]*Chann return ChannelSliceFromJson(r.Body), BuildResponse(r) } +// SearchArchivedChannels returns the archived channels on a team matching the provided search term. +func (c *Client4) SearchArchivedChannels(teamId string, search *ChannelSearch) ([]*Channel, *Response) { + r, err := c.DoApiPost(c.GetChannelsForTeamRoute(teamId)+"/search_archived", search.ToJson()) + if err != nil { + return nil, BuildErrorResponse(r, err) + } + defer closeBody(r) + return ChannelSliceFromJson(r.Body), BuildResponse(r) +} + // SearchAllChannels search in all the channels. Must be a system administrator. func (c *Client4) SearchAllChannels(search *ChannelSearch) (*ChannelListWithTeamData, *Response) { r, err := c.DoApiPost(c.GetChannelsRoute()+"/search", search.ToJson()) diff --git a/store/sqlstore/channel_store.go b/store/sqlstore/channel_store.go index aba5d38d77..bf05278aaf 100644 --- a/store/sqlstore/channel_store.go +++ b/store/sqlstore/channel_store.go @@ -1036,7 +1036,7 @@ func (s SqlChannelStore) GetPublicChannelsForTeam(teamId string, offset int, lim PublicChannels pc ON (pc.Id = Channels.Id) WHERE pc.TeamId = :TeamId - AND pc.DeleteAt = 0 + AND pc.DeleteAt = 0 ORDER BY pc.DisplayName LIMIT :Limit OFFSET :Offset @@ -1242,10 +1242,24 @@ func (s SqlChannelStore) GetDeletedByName(teamId string, name string) (*model.Ch return &channel, nil } -func (s SqlChannelStore) GetDeleted(teamId string, offset int, limit int) (*model.ChannelList, *model.AppError) { +func (s SqlChannelStore) GetDeleted(teamId string, offset int, limit int, userId string) (*model.ChannelList, *model.AppError) { channels := &model.ChannelList{} - if _, err := s.GetReplica().Select(channels, "SELECT * FROM Channels WHERE (TeamId = :TeamId OR TeamId = '') AND DeleteAt != 0 ORDER BY DisplayName LIMIT :Limit OFFSET :Offset", map[string]interface{}{"TeamId": teamId, "Limit": limit, "Offset": offset}); err != nil { + query := ` + SELECT * FROM Channels + WHERE (TeamId = :TeamId OR TeamId = '') + AND DeleteAt != 0 + AND Type != 'P' + UNION + SELECT * FROM Channels + WHERE (TeamId = :TeamId OR TeamId = '') + AND DeleteAt != 0 + AND Type = 'P' + AND Id IN (SELECT ChannelId FROM ChannelMembers WHERE UserId = :UserId) + ORDER BY DisplayName LIMIT :Limit OFFSET :Offset + ` + + if _, err := s.GetReplica().Select(channels, query, map[string]interface{}{"TeamId": teamId, "Limit": limit, "Offset": offset, "UserId": userId}); err != nil { if err == sql.ErrNoRows { return nil, model.NewAppError("SqlChannelStore.GetDeleted", "store.sql_channel.get_deleted.missing.app_error", nil, "teamId="+teamId+", "+err.Error(), http.StatusNotFound) } @@ -2154,6 +2168,57 @@ func (s SqlChannelStore) SearchInTeam(teamId string, term string, includeDeleted }) } +func (s SqlChannelStore) SearchArchivedInTeam(teamId string, term string, userId string) (*model.ChannelList, *model.AppError) { + publicChannels, publicErr := s.performSearch(` + SELECT + Channels.* + FROM + Channels + JOIN + Channels c ON (c.Id = Channels.Id) + WHERE + c.TeamId = :TeamId + SEARCH_CLAUSE + AND c.DeleteAt != 0 + AND c.Type != 'P' + ORDER BY c.DisplayName + LIMIT 100 + `, term, map[string]interface{}{ + "TeamId": teamId, + "UserId": userId, + }) + + privateChannels, privateErr := s.performSearch(` + SELECT + Channels.* + FROM + Channels + JOIN + Channels c ON (c.Id = Channels.Id) + WHERE + c.TeamId = :TeamId + SEARCH_CLAUSE + AND c.DeleteAt != 0 + AND c.Type = 'P' + AND c.Id IN (SELECT ChannelId FROM ChannelMembers WHERE UserId = :UserId) + ORDER BY c.DisplayName + LIMIT 100 + `, term, map[string]interface{}{ + "TeamId": teamId, + "UserId": userId, + }) + + output := *publicChannels + output = append(output, *privateChannels...) + + outputErr := publicErr + if privateErr != nil { + outputErr = privateErr + } + + return &output, outputErr +} + func (s SqlChannelStore) SearchForUserInTeam(userId string, teamId string, term string, includeDeleted bool) (*model.ChannelList, *model.AppError) { deleteFilter := "AND c.DeleteAt = 0" if includeDeleted { diff --git a/store/store.go b/store/store.go index 59ef9f693e..330a95496e 100644 --- a/store/store.go +++ b/store/store.go @@ -125,7 +125,7 @@ type ChannelStore interface { GetByNames(team_id string, names []string, allowFromCache bool) ([]*model.Channel, *model.AppError) GetByNameIncludeDeleted(team_id string, name string, allowFromCache bool) (*model.Channel, *model.AppError) GetDeletedByName(team_id string, name string) (*model.Channel, *model.AppError) - GetDeleted(team_id string, offset int, limit int) (*model.ChannelList, *model.AppError) + GetDeleted(team_id string, offset int, limit int, userId string) (*model.ChannelList, *model.AppError) GetChannels(teamId string, userId string, includeDeleted bool) (*model.ChannelList, *model.AppError) GetAllChannels(page, perPage int, opts ChannelSearchOpts) (*model.ChannelListWithTeamData, *model.AppError) GetAllChannelsCount(opts ChannelSearchOpts) (int64, *model.AppError) @@ -170,6 +170,7 @@ type ChannelStore interface { AutocompleteInTeamForSearch(teamId string, userId string, term string, includeDeleted bool) (*model.ChannelList, *model.AppError) SearchAllChannels(term string, opts ChannelSearchOpts) (*model.ChannelListWithTeamData, *model.AppError) SearchInTeam(teamId string, term string, includeDeleted bool) (*model.ChannelList, *model.AppError) + SearchArchivedInTeam(teamId string, term string, userId string) (*model.ChannelList, *model.AppError) SearchForUserInTeam(userId string, teamId string, term string, includeDeleted bool) (*model.ChannelList, *model.AppError) SearchMore(userId string, teamId string, term string) (*model.ChannelList, *model.AppError) SearchGroupChannels(userId, term string) (*model.ChannelList, *model.AppError) diff --git a/store/storetest/channel_store.go b/store/storetest/channel_store.go index 29b88be307..0829335afc 100644 --- a/store/storetest/channel_store.go +++ b/store/storetest/channel_store.go @@ -687,13 +687,15 @@ func testChannelStoreGetDeleted(t *testing.T, ss store.Store) { o1.Name = "zz" + model.NewId() + "b" o1.Type = model.CHANNEL_OPEN + userId := model.NewId() + _, err := ss.Channel().Save(&o1, -1) require.Nil(t, err) err = ss.Channel().Delete(o1.Id, model.GetMillis()) require.Nil(t, err, "channel should have been deleted") - list, err := ss.Channel().GetDeleted(o1.TeamId, 0, 100) + list, err := ss.Channel().GetDeleted(o1.TeamId, 0, 100, userId) require.Nil(t, err, err) require.Len(t, *list, 1, "wrong list") require.Equal(t, o1.Name, (*list)[0].Name, "missing channel") @@ -706,7 +708,7 @@ func testChannelStoreGetDeleted(t *testing.T, ss store.Store) { _, err = ss.Channel().Save(&o2, -1) require.Nil(t, err) - list, err = ss.Channel().GetDeleted(o1.TeamId, 0, 100) + list, err = ss.Channel().GetDeleted(o1.TeamId, 0, 100, userId) require.Nil(t, err, err) require.Len(t, *list, 1, "wrong list") @@ -722,15 +724,15 @@ func testChannelStoreGetDeleted(t *testing.T, ss store.Store) { err = ss.Channel().Delete(o3.Id, model.GetMillis()) require.Nil(t, err, "channel should have been deleted") - list, err = ss.Channel().GetDeleted(o1.TeamId, 0, 100) + list, err = ss.Channel().GetDeleted(o1.TeamId, 0, 100, userId) require.Nil(t, err, err) require.Len(t, *list, 2, "wrong list length") - list, err = ss.Channel().GetDeleted(o1.TeamId, 0, 1) + list, err = ss.Channel().GetDeleted(o1.TeamId, 0, 1, userId) require.Nil(t, err, err) require.Len(t, *list, 1, "wrong list length") - list, err = ss.Channel().GetDeleted(o1.TeamId, 1, 1) + list, err = ss.Channel().GetDeleted(o1.TeamId, 1, 1, userId) require.Nil(t, err, err) require.Len(t, *list, 1, "wrong list length") diff --git a/store/storetest/mocks/ChannelStore.go b/store/storetest/mocks/ChannelStore.go index 8eb871ced0..5b3a58048e 100644 --- a/store/storetest/mocks/ChannelStore.go +++ b/store/storetest/mocks/ChannelStore.go @@ -647,7 +647,7 @@ func (_m *ChannelStore) GetChannelsByScheme(schemeId string, offset int, limit i } // GetDeleted provides a mock function with given fields: team_id, offset, limit -func (_m *ChannelStore) GetDeleted(team_id string, offset int, limit int) (*model.ChannelList, *model.AppError) { +func (_m *ChannelStore) GetDeleted(team_id string, offset int, limit int, userId string) (*model.ChannelList, *model.AppError) { ret := _m.Called(team_id, offset, limit) var r0 *model.ChannelList @@ -1539,6 +1539,31 @@ func (_m *ChannelStore) SearchInTeam(teamId string, term string, includeDeleted return r0, r1 } +// SearchArchivedInTeam provides a mock function with given fields: teamId, term, userId +func (_m *ChannelStore) SearchArchivedInTeam(teamId string, term string, userId string) (*model.ChannelList, *model.AppError) { + ret := _m.Called(teamId, term, userId) + + var r0 *model.ChannelList + if rf, ok := ret.Get(0).(func(string, string, string) *model.ChannelList); ok { + r0 = rf(teamId, term, userId) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*model.ChannelList) + } + } + + var r1 *model.AppError + if rf, ok := ret.Get(1).(func(string, string, string) *model.AppError); ok { + r1 = rf(teamId, term, userId) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 +} + // SearchMore provides a mock function with given fields: userId, teamId, term func (_m *ChannelStore) SearchMore(userId string, teamId string, term string) (*model.ChannelList, *model.AppError) { ret := _m.Called(userId, teamId, term) diff --git a/store/timer_layer.go b/store/timer_layer.go index 366150b79a..4ee55ba206 100644 --- a/store/timer_layer.go +++ b/store/timer_layer.go @@ -904,10 +904,10 @@ func (s *TimerLayerChannelStore) GetChannelsByScheme(schemeId string, offset int return resultVar0, resultVar1 } -func (s *TimerLayerChannelStore) GetDeleted(team_id string, offset int, limit int) (*model.ChannelList, *model.AppError) { +func (s *TimerLayerChannelStore) GetDeleted(team_id string, offset int, limit int, userId string) (*model.ChannelList, *model.AppError) { start := timemodule.Now() - resultVar0, resultVar1 := s.ChannelStore.GetDeleted(team_id, offset, limit) + resultVar0, resultVar1 := s.ChannelStore.GetDeleted(team_id, offset, limit, userId) elapsed := float64(timemodule.Since(start)) / float64(timemodule.Second) if s.Root.Metrics != nil { From 1155b084044e675a70860ceda197d6f60b27f1df Mon Sep 17 00:00:00 2001 From: Andres Date: Mon, 11 Nov 2019 21:29:21 -0800 Subject: [PATCH 11/18] MM - 19155 Change plugin helpers files to be golint complied (#12686) * MM-19155 Change plugin helpers files to be golint complied * add makefile target that checks if plugin/helpers files are golint compliant * MM-19155 - Change plugin helpers files to be golint complied * added comment documentation to exported plugin/helpers methods * changed variable names to be compliant with golint * Address feedback --- Makefile | 5 ++++- plugin/helpers.go | 4 ++++ plugin/helpers_bots.go | 24 +++++++++++++----------- 3 files changed, 21 insertions(+), 12 deletions(-) diff --git a/Makefile b/Makefile index ea99a27b6c..60ebc1dc33 100644 --- a/Makefile +++ b/Makefile @@ -230,7 +230,10 @@ check-prereqs: ## Checks prerequisite software status. ./scripts/prereq-check.sh # TODO: remove govet and gofmt checks once golangci-lint is being enforced. -check-style: govet gofmt check-licenses ## Runs govet and gofmt against all packages. +check-style: govet gofmt check-licenses check-plugin-golint ## Runs govet and gofmt against all packages and also ensures plugin package golint compliant + +check-plugin-golint: # Checks if golint returns any uncompliant code for any file that starts with plugin/helpers + @! golint ./plugin/ | grep plugin/helpers test-te-race: ## Checks for race conditions in the team edition. @echo Testing TE race conditions diff --git a/plugin/helpers.go b/plugin/helpers.go index 7b62fffa80..5a2e4e33ca 100644 --- a/plugin/helpers.go +++ b/plugin/helpers.go @@ -9,6 +9,9 @@ import ( "github.com/pkg/errors" ) +// Helpers provide a common patterns plugins use. +// +// Plugins obtain access to the Helpers by embedding MattermostPlugin. type Helpers interface { // EnsureBot either returns an existing bot user matching the given bot, or creates a bot user from the given bot. // Returns the id of the resulting bot. @@ -55,6 +58,7 @@ type Helpers interface { KVSetWithExpiryJSON(key string, value interface{}, expireInSeconds int64) error } +// HelpersImpl implements the helpers interface with an API that retrieves data on behalf of the plugin. type HelpersImpl struct { API API } diff --git a/plugin/helpers_bots.go b/plugin/helpers_bots.go index d7ab57ce64..44355a0397 100644 --- a/plugin/helpers_bots.go +++ b/plugin/helpers_bots.go @@ -4,12 +4,14 @@ package plugin import ( + "github.com/pkg/errors" + "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/utils" - "github.com/pkg/errors" ) -func (p *HelpersImpl) EnsureBot(bot *model.Bot) (retBotId string, retErr error) { +// EnsureBot implements Helpers.EnsureBot +func (p *HelpersImpl) EnsureBot(bot *model.Bot) (retBotID string, retErr error) { err := p.ensureServerVersion("5.10.0") if err != nil { return "", errors.Wrap(err, "failed to ensure bot") @@ -23,34 +25,34 @@ func (p *HelpersImpl) EnsureBot(bot *model.Bot) (retBotId string, retErr error) // If we fail for any reason, this could be a race between creation of bot and // retrieval from another EnsureBot. Just try the basic retrieve existing again. defer func() { - if retBotId == "" || retErr != nil { + if retBotID == "" || retErr != nil { var err error - var botIdBytes []byte + var botIDBytes []byte err = utils.ProgressiveRetry(func() error { - botIdBytes, err = p.API.KVGet(BOT_USER_KEY) + botIDBytes, err = p.API.KVGet(BOT_USER_KEY) if err != nil { return err } return nil }) - if err == nil && botIdBytes != nil { - retBotId = string(botIdBytes) + if err == nil && botIDBytes != nil { + retBotID = string(botIDBytes) retErr = nil } } }() - botIdBytes, kvGetErr := p.API.KVGet(BOT_USER_KEY) + botIDBytes, kvGetErr := p.API.KVGet(BOT_USER_KEY) if kvGetErr != nil { return "", errors.Wrap(kvGetErr, "failed to get bot") } // If the bot has already been created, there is nothing to do. - if botIdBytes != nil { - botId := string(botIdBytes) - return botId, nil + if botIDBytes != nil { + botID := string(botIDBytes) + return botID, nil } // Check for an existing bot user with that username. If one exists, then use that. From cdb61db9c4cf4e59e8a38ec979b331d24b5abd5f Mon Sep 17 00:00:00 2001 From: Carlos Tadeu Panato Junior Date: Tue, 12 Nov 2019 08:32:06 +0100 Subject: [PATCH 12/18] add stop-docker to stop make command (#13020) --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 60ebc1dc33..8f83b2b516 100644 --- a/Makefile +++ b/Makefile @@ -401,7 +401,7 @@ stop-client: ## Stops the webapp. cd $(BUILD_WEBAPP_DIR) && $(MAKE) stop -stop: stop-server stop-client ## Stops server and client. +stop: stop-server stop-client stop-docker ## Stops server, client and the docker compose. restart: restart-server restart-client ## Restarts the server and webapp. From c488b3c71828b40e8109fb15212d896209f87b73 Mon Sep 17 00:00:00 2001 From: "Sanele T. Mahlalela" Date: Tue, 12 Nov 2019 13:20:56 +0200 Subject: [PATCH 13/18] Fixed comments syntaxt in app config file (#13055) fixed syntax error in comment in config.go --- app/config.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/config.go b/app/config.go index b86dcf5abc..b2f33a8128 100644 --- a/app/config.go +++ b/app/config.go @@ -83,7 +83,7 @@ func (a *App) LimitedClientConfig() map[string]string { return a.Srv.limitedClientConfig } -// Registers a function with a given to be called when the config is reloaded and may have changed. The function +// Registers a function with a given listener to be called when the config is reloaded and may have changed. The function // will be called with two arguments: the old config and the new config. AddConfigListener returns a unique ID // for the listener that can later be used to remove it. func (s *Server) AddConfigListener(listener func(*model.Config, *model.Config)) string { @@ -104,7 +104,7 @@ func (a *App) RemoveConfigListener(id string) { } // ensurePostActionCookieSecret ensures that the key for encrypting PostActionCookie exists -// and future calls to PostAcrionCookieSecret will always return a valid key, same on all +// and future calls to PostActionCookieSecret will always return a valid key, same on all // servers in the cluster func (a *App) ensurePostActionCookieSecret() error { if a.Srv.postActionCookieSecret != nil { From e9ecb8fdaa81875e654fd15ec8a307ad42700bb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Espino?= Date: Tue, 12 Nov 2019 12:25:21 +0100 Subject: [PATCH 14/18] Simplifiying mail service API (#13036) --- app/email.go | 5 +- services/mailservice/mail.go | 62 ++++++++++++++++------- services/mailservice/mail_test.go | 84 ++++++++++++++++++++++++------- 3 files changed, 112 insertions(+), 39 deletions(-) diff --git a/app/email.go b/app/email.go index cf54a2af2d..d0166072d8 100644 --- a/app/email.go +++ b/app/email.go @@ -7,7 +7,6 @@ import ( "bytes" "fmt" "io" - "net/mail" "net/url" "path" "strings" @@ -525,8 +524,6 @@ func (a *App) SendMail(to, subject, htmlBody string) *model.AppError { func (a *App) SendMailWithEmbeddedFiles(to, subject, htmlBody string, embeddedFiles map[string]io.Reader) *model.AppError { license := a.License() config := a.Config() - fromMail := mail.Address{Name: *config.EmailSettings.FeedbackName, Address: *config.EmailSettings.FeedbackEmail} - replyTo := mail.Address{Name: *config.EmailSettings.FeedbackName, Address: *config.EmailSettings.ReplyToAddress} - return mailservice.SendMailUsingConfigAdvanced(to, to, fromMail, replyTo, subject, htmlBody, nil, embeddedFiles, nil, config, license != nil && *license.Features.Compliance) + return mailservice.SendMailWithEmbeddedFilesUsingConfig(to, subject, htmlBody, embeddedFiles, config, license != nil && *license.Features.Compliance) } diff --git a/services/mailservice/mail.go b/services/mailservice/mail.go index c02594f89f..bd18576d72 100644 --- a/services/mailservice/mail.go +++ b/services/mailservice/mail.go @@ -24,6 +24,18 @@ import ( "github.com/mattermost/mattermost-server/utils" ) +type mailData struct { + mimeTo string + smtpTo string + from mail.Address + replyTo mail.Address + subject string + htmlBody string + attachments []*model.FileInfo + embeddedFiles map[string]io.Reader + mimeHeaders map[string]string +} + // smtpClient is implemented by an smtp.Client. See https://golang.org/pkg/net/smtp/#Client. // type smtpClient interface { @@ -204,15 +216,29 @@ func TestConnection(config *model.Config) { defer c.Close() } -func SendMailUsingConfig(to, subject, htmlBody string, config *model.Config, enableComplianceFeatures bool) *model.AppError { +func SendMailWithEmbeddedFilesUsingConfig(to, subject, htmlBody string, embeddedFiles map[string]io.Reader, config *model.Config, enableComplianceFeatures bool) *model.AppError { fromMail := mail.Address{Name: *config.EmailSettings.FeedbackName, Address: *config.EmailSettings.FeedbackEmail} replyTo := mail.Address{Name: *config.EmailSettings.FeedbackName, Address: *config.EmailSettings.ReplyToAddress} - return SendMailUsingConfigAdvanced(to, to, fromMail, replyTo, subject, htmlBody, nil, nil, nil, config, enableComplianceFeatures) + mail := mailData{ + mimeTo: to, + smtpTo: to, + from: fromMail, + replyTo: replyTo, + subject: subject, + htmlBody: htmlBody, + embeddedFiles: embeddedFiles, + } + + return sendMailUsingConfigAdvanced(mail, config, enableComplianceFeatures) +} + +func SendMailUsingConfig(to, subject, htmlBody string, config *model.Config, enableComplianceFeatures bool) *model.AppError { + return SendMailWithEmbeddedFilesUsingConfig(to, subject, htmlBody, nil, config, enableComplianceFeatures) } // allows for sending an email with attachments and differing MIME/SMTP recipients -func SendMailUsingConfigAdvanced(mimeTo, smtpTo string, from, replyTo mail.Address, subject, htmlBody string, attachments []*model.FileInfo, embeddedFiles map[string]io.Reader, mimeHeaders map[string]string, config *model.Config, enableComplianceFeatures bool) *model.AppError { +func sendMailUsingConfigAdvanced(mail mailData, config *model.Config, enableComplianceFeatures bool) *model.AppError { if len(*config.EmailSettings.SMTPServer) == 0 { return nil } @@ -235,34 +261,34 @@ func SendMailUsingConfigAdvanced(mimeTo, smtpTo string, from, replyTo mail.Addre return err } - return SendMail(c, mimeTo, smtpTo, from, replyTo, subject, htmlBody, attachments, embeddedFiles, mimeHeaders, fileBackend, time.Now()) + return SendMail(c, mail, fileBackend, time.Now()) } -func SendMail(c smtpClient, mimeTo, smtpTo string, from, replyTo mail.Address, subject, htmlBody string, attachments []*model.FileInfo, embeddedFiles map[string]io.Reader, mimeHeaders map[string]string, fileBackend filesstore.FileBackend, date time.Time) *model.AppError { - mlog.Debug("sending mail", mlog.String("to", smtpTo), mlog.String("subject", subject)) +func SendMail(c smtpClient, mail mailData, fileBackend filesstore.FileBackend, date time.Time) *model.AppError { + mlog.Debug("sending mail", mlog.String("to", mail.smtpTo), mlog.String("subject", mail.subject)) - htmlMessage := "\r\n" + htmlBody + "" + htmlMessage := "\r\n" + mail.htmlBody + "" - txtBody, err := html2text.FromString(htmlBody) + txtBody, err := html2text.FromString(mail.htmlBody) if err != nil { mlog.Warn("Unable to convert html body to text", mlog.Err(err)) txtBody = "" } headers := map[string][]string{ - "From": {from.String()}, - "To": {mimeTo}, - "Subject": {encodeRFC2047Word(subject)}, + "From": {mail.from.String()}, + "To": {mail.mimeTo}, + "Subject": {encodeRFC2047Word(mail.subject)}, "Content-Transfer-Encoding": {"8bit"}, "Auto-Submitted": {"auto-generated"}, "Precedence": {"bulk"}, } - if len(replyTo.Address) > 0 { - headers["Reply-To"] = []string{replyTo.String()} + if len(mail.replyTo.Address) > 0 { + headers["Reply-To"] = []string{mail.replyTo.String()} } - for k, v := range mimeHeaders { + for k, v := range mail.mimeHeaders { headers[k] = []string{encodeRFC2047Word(v)} } @@ -272,11 +298,11 @@ func SendMail(c smtpClient, mimeTo, smtpTo string, from, replyTo mail.Address, s m.SetBody("text/plain", txtBody) m.AddAlternative("text/html", htmlMessage) - for name, reader := range embeddedFiles { + for name, reader := range mail.embeddedFiles { m.EmbedReader(name, reader) } - for _, fileInfo := range attachments { + for _, fileInfo := range mail.attachments { bytes, err := fileBackend.ReadFile(fileInfo.Path) if err != nil { return err @@ -290,11 +316,11 @@ func SendMail(c smtpClient, mimeTo, smtpTo string, from, replyTo mail.Address, s })) } - if err = c.Mail(from.Address); err != nil { + if err = c.Mail(mail.from.Address); err != nil { return model.NewAppError("SendMail", "utils.mail.send_mail.from_address.app_error", nil, err.Error(), http.StatusInternalServerError) } - if err = c.Rcpt(smtpTo); err != nil { + if err = c.Rcpt(mail.smtpTo); err != nil { return model.NewAppError("SendMail", "utils.mail.send_mail.to_address.app_error", nil, err.Error(), http.StatusInternalServerError) } diff --git a/services/mailservice/mail_test.go b/services/mailservice/mail_test.go index 2e2b87302f..9686f8d92e 100644 --- a/services/mailservice/mail_test.go +++ b/services/mailservice/mail_test.go @@ -128,6 +128,50 @@ func TestSendMailUsingConfig(t *testing.T) { } } +func TestSendMailWithEmbeddedFilesUsingConfig(t *testing.T) { + utils.T = utils.GetUserTranslations("en") + + fs, err := config.NewFileStore("config.json", false) + require.Nil(t, err) + + cfg := fs.Get() + + var emailTo = "test@example.com" + var emailSubject = "Testing this email" + var emailBody = "This is a test from autobot" + + //Delete all the messages before check the sample email + DeleteMailBox(emailTo) + + embeddedFiles := map[string]io.Reader{ + "test1.png": bytes.NewReader([]byte("xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx")), + "test2.png": bytes.NewReader([]byte("xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx")), + } + err2 := SendMailWithEmbeddedFilesUsingConfig(emailTo, emailSubject, emailBody, embeddedFiles, cfg, true) + require.Nil(t, err2, "Should connect to the SMTP Server") + + //Check if the email was send to the right email address + var resultsMailbox JSONMessageHeaderInbucket + err3 := RetryInbucket(5, func() error { + var err error + resultsMailbox, err = GetMailBox(emailTo) + return err + }) + if err3 != nil { + t.Log(err3) + t.Log("No email was received, maybe due load on the server. Skipping this verification") + } else { + if len(resultsMailbox) > 0 { + require.Contains(t, resultsMailbox[0].To[0], emailTo, "Wrong To: recipient") + resultsEmail, err := GetMessageFromMailbox(emailTo, resultsMailbox[0].ID) + require.Nil(t, err, "Could not get message from mailbox") + require.Contains(t, emailBody, resultsEmail.Body.Text, "Wrong received message %s", resultsEmail.Body.Text) + // Usign the message size because the inbucket API doesn't return embedded attachments through the API + require.Greater(t, resultsEmail.Size, 1500, "the file size should be more because the embedded attachemtns") + } + } +} + func TestSendMailUsingConfigAdvanced(t *testing.T) { utils.T = utils.GetUserTranslations("en") @@ -136,15 +180,8 @@ func TestSendMailUsingConfigAdvanced(t *testing.T) { cfg := fs.Get() - var mimeTo = "test@example.com" - var smtpTo = "test2@example.com" - var from = mail.Address{Name: "Nobody", Address: "nobody@mattermost.com"} - var replyTo = mail.Address{Name: "ReplyTo", Address: "reply_to@mattermost.com"} - var emailSubject = "Testing this email" - var emailBody = "This is a test from autobot" - //Delete all the messages before check the sample email - DeleteMailBox(smtpTo) + DeleteMailBox("test2@example.com") fileBackend, err := filesstore.NewFileBackend(&cfg.FileSettings, true) assert.Nil(t, err) @@ -178,31 +215,43 @@ func TestSendMailUsingConfigAdvanced(t *testing.T) { headers := make(map[string]string) headers["TestHeader"] = "TestValue" - err = SendMailUsingConfigAdvanced(mimeTo, smtpTo, from, replyTo, emailSubject, emailBody, attachments, embeddedFiles, headers, cfg, true) + mail := mailData{ + mimeTo: "test@example.com", + smtpTo: "test2@example.com", + from: mail.Address{Name: "Nobody", Address: "nobody@mattermost.com"}, + replyTo: mail.Address{Name: "ReplyTo", Address: "reply_to@mattermost.com"}, + subject: "Testing this email", + htmlBody: "This is a test from autobot", + attachments: attachments, + embeddedFiles: embeddedFiles, + mimeHeaders: headers, + } + + err = sendMailUsingConfigAdvanced(mail, cfg, true) require.Nil(t, err, "Should connect to the STMP Server: %v", err) //Check if the email was send to the right email address var resultsMailbox JSONMessageHeaderInbucket err = RetryInbucket(5, func() error { var mailErr error - resultsMailbox, mailErr = GetMailBox(smtpTo) + resultsMailbox, mailErr = GetMailBox(mail.smtpTo) return mailErr }) - require.Nil(t, err, "No emails found for address %s. error: %v", smtpTo, err) + require.Nil(t, err, "No emails found for address %s. error: %v", mail.smtpTo, err) require.NotEqual(t, len(resultsMailbox), 0) - require.Contains(t, resultsMailbox[0].To[0], mimeTo, "Wrong To recipient") + require.Contains(t, resultsMailbox[0].To[0], mail.mimeTo, "Wrong To recipient") - resultsEmail, err := GetMessageFromMailbox(smtpTo, resultsMailbox[0].ID) + resultsEmail, err := GetMessageFromMailbox(mail.smtpTo, resultsMailbox[0].ID) require.Nil(t, err) - require.Contains(t, emailBody, resultsEmail.Body.Text, "Wrong received message") + require.Contains(t, mail.htmlBody, resultsEmail.Body.Text, "Wrong received message") // verify that the To header of the email message is set to the MIME recipient, even though we got it out of the SMTP recipient's email inbox - assert.Equal(t, mimeTo, resultsEmail.Header["To"][0]) + assert.Equal(t, mail.mimeTo, resultsEmail.Header["To"][0]) // verify that the MIME from address is correct - unfortunately, we can't verify the SMTP from address - assert.Equal(t, from.String(), resultsEmail.Header["From"][0]) + assert.Equal(t, mail.from.String(), resultsEmail.Header["From"][0]) // check that the custom mime headers came through - header case seems to get mutated assert.Equal(t, "TestValue", resultsEmail.Header["Testheader"][0]) @@ -330,7 +379,8 @@ func TestSendMail(t *testing.T) { for testName, tc := range testCases { t.Run(testName, func(t *testing.T) { - appErr = SendMail(mocm, "", "", mail.Address{}, tc.replyTo, "", "", nil, nil, nil, mockBackend, time.Now()) + mail := mailData{"", "", mail.Address{}, tc.replyTo, "", "", nil, nil, nil} + appErr = SendMail(mocm, mail, mockBackend, time.Now()) require.Nil(t, appErr) if len(tc.contains) > 0 { require.Contains(t, string(mocm.data), tc.contains) From ae78449a20e04deb6f8b7afdcb68f3873d59e9e4 Mon Sep 17 00:00:00 2001 From: kanozec <30459655+kanozec@users.noreply.github.com> Date: Tue, 12 Nov 2019 19:33:05 +0800 Subject: [PATCH 15/18] Fix typo in server_app_adapters.go (#13042) --- app/server.go | 2 +- app/server_app_adapters.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/server.go b/app/server.go index fb7ec94f25..daad2ed0d8 100644 --- a/app/server.go +++ b/app/server.go @@ -186,7 +186,7 @@ func NewServer(options ...Option) (*Server, error) { return nil, errors.Wrapf(err, "unable to load Mattermost translation files") } - err := s.RunOldAppInitalization() + err := s.RunOldAppInitialization() if err != nil { return nil, err } diff --git a/app/server_app_adapters.go b/app/server_app_adapters.go index 272b488011..d4812b29ff 100644 --- a/app/server_app_adapters.go +++ b/app/server_app_adapters.go @@ -18,11 +18,11 @@ import ( "github.com/pkg/errors" ) -// This is a bridge between the old and new initalization for the context refactor. -// It calls app layer initalization code that then turns around and acts on the server. -// Don't add anything new here, new initilization should be done in the server and +// This is a bridge between the old and new initialization for the context refactor. +// It calls app layer initialization code that then turns around and acts on the server. +// Don't add anything new here, new initialization should be done in the server and // performed in the NewServer function. -func (s *Server) RunOldAppInitalization() error { +func (s *Server) RunOldAppInitialization() error { s.FakeApp().CreatePushNotificationsHub() s.FakeApp().StartPushNotificationsHubWorkers() From 111d076a86d8354afee4af7fc42d60bd8ccf4b20 Mon Sep 17 00:00:00 2001 From: Shobhit Gupta Date: Tue, 12 Nov 2019 03:43:10 -0800 Subject: [PATCH 16/18] Migrate tests from "api4/team_test.go" to use testify (#12902) --- api4/apitestlib.go | 8 +- api4/team_test.go | 626 +++++++++++++-------------------------------- 2 files changed, 181 insertions(+), 453 deletions(-) diff --git a/api4/apitestlib.go b/api4/apitestlib.go index 8be1af367f..6da7efe6f4 100644 --- a/api4/apitestlib.go +++ b/api4/apitestlib.go @@ -574,7 +574,9 @@ func CheckEtag(t *testing.T, data interface{}, resp *model.Response) { func CheckNoError(t *testing.T, resp *model.Response) { t.Helper() - require.Nil(t, resp.Error) + if resp.Error != nil { + require.FailNow(t, "Expected no error, got %q", resp.Error.Error()) + } } func checkHTTPStatus(t *testing.T, resp *model.Response, expectedStatus int, expectError bool) { @@ -637,8 +639,8 @@ func CheckInternalErrorStatus(t *testing.T, resp *model.Response) { func CheckErrorMessage(t *testing.T, resp *model.Response, errorId string) { t.Helper() - require.NotNil(t, resp.Error) - require.Equal(t, resp.Error.Id, errorId, "incorrect error message") + require.NotNilf(t, resp.Error, "should have errored with message: %s", errorId) + require.Equalf(t, resp.Error.Id, errorId, "incorrect error message, actual: %s, expected: %s", resp.Error.Id, errorId) } func CheckStartsWith(t *testing.T, value, prefix, message string) { diff --git a/api4/team_test.go b/api4/team_test.go index 20bf698e18..99e76d3a78 100644 --- a/api4/team_test.go +++ b/api4/team_test.go @@ -32,17 +32,11 @@ func TestCreateTeam(t *testing.T) { CheckNoError(t, resp) CheckCreatedStatus(t, resp) - if rteam.Name != team.Name { - t.Fatal("names did not match") - } + require.Equal(t, rteam.Name, team.Name, "names did not match") - if rteam.DisplayName != team.DisplayName { - t.Fatal("display names did not match") - } + require.Equal(t, rteam.DisplayName, team.DisplayName, "display names did not match") - if rteam.Type != team.Type { - t.Fatal("types did not match") - } + require.Equal(t, rteam.Type, team.Type, "types did not match") _, resp = Client.CreateTeam(rteam) CheckBadRequestStatus(t, resp) @@ -57,15 +51,10 @@ func TestCreateTeam(t *testing.T) { CheckErrorMessage(t, resp, "model.team.is_valid.characters.app_error") CheckBadRequestStatus(t, resp) - if r, err := Client.DoApiPost("/teams", "garbage"); err == nil { - t.Fatal("should have errored") - } else { - if r.StatusCode != http.StatusBadRequest { - t.Log("actual: " + strconv.Itoa(r.StatusCode)) - t.Log("expected: " + strconv.Itoa(http.StatusBadRequest)) - t.Fatal("wrong status code") - } - } + r, err := Client.DoApiPost("/teams", "garbage") + require.NotNil(t, err, "should have errored") + + require.Equalf(t, r.StatusCode, http.StatusBadRequest, "wrong status code, actual: %s, expected: %s", strconv.Itoa(r.StatusCode), strconv.Itoa(http.StatusBadRequest)) Client.Logout() @@ -80,9 +69,7 @@ func TestCreateTeam(t *testing.T) { CheckNoError(t, resp) CheckCreatedStatus(t, resp) - if *rteam.GroupConstrained != *groupConstrainedTeam.GroupConstrained { - t.Fatal("GroupConstrained flags do not match") - } + assert.Equal(t, *rteam.GroupConstrained, *groupConstrainedTeam.GroupConstrained, "GroupConstrained flags do not match") // Check the appropriate permissions are enforced. defaultRolePermissions := th.SaveDefaultRolePermissions() @@ -114,9 +101,7 @@ func TestCreateTeamSanitization(t *testing.T) { rteam, resp := th.Client.CreateTeam(team) CheckNoError(t, resp) - if rteam.Email == "" { - t.Fatal("should not have sanitized email") - } + require.NotEmpty(t, rteam.Email, "should not have sanitized email") }) t.Run("system admin", func(t *testing.T) { @@ -130,9 +115,7 @@ func TestCreateTeamSanitization(t *testing.T) { rteam, resp := th.SystemAdminClient.CreateTeam(team) CheckNoError(t, resp) - if rteam.Email == "" { - t.Fatal("should not have sanitized email") - } + require.NotEmpty(t, rteam.Email, "should not have sanitized email") }) } @@ -145,9 +128,7 @@ func TestGetTeam(t *testing.T) { rteam, resp := Client.GetTeam(team.Id, "") CheckNoError(t, resp) - if rteam.Id != team.Id { - t.Fatal("wrong team") - } + require.Equal(t, rteam.Id, team.Id, "wrong team") _, resp = Client.GetTeam("junk", "") CheckBadRequestStatus(t, resp) @@ -204,25 +185,20 @@ func TestGetTeamSanitization(t *testing.T) { rteam, resp := client.GetTeam(team.Id, "") CheckNoError(t, resp) - if rteam.Email != "" { - t.Fatal("should've sanitized email") - } + + require.Empty(t, rteam.Email, "should have sanitized email") }) t.Run("team admin", func(t *testing.T) { rteam, resp := th.Client.GetTeam(team.Id, "") CheckNoError(t, resp) - if rteam.Email == "" { - t.Fatal("should not have sanitized email") - } + require.NotEmpty(t, rteam.Email, "should not have sanitized email") }) t.Run("system admin", func(t *testing.T) { rteam, resp := th.SystemAdminClient.GetTeam(team.Id, "") CheckNoError(t, resp) - if rteam.Email == "" { - t.Fatal("should not have sanitized email") - } + require.NotEmpty(t, rteam.Email, "should not have sanitized email") }) } @@ -233,9 +209,7 @@ func TestGetTeamUnread(t *testing.T) { teamUnread, resp := Client.GetTeamUnread(th.BasicTeam.Id, th.BasicUser.Id) CheckNoError(t, resp) - if teamUnread.TeamId != th.BasicTeam.Id { - t.Fatal("wrong team id returned for regular user call") - } + require.Equal(t, teamUnread.TeamId, th.BasicTeam.Id, "wrong team id returned for regular user call") _, resp = Client.GetTeamUnread("junk", th.BasicUser.Id) CheckBadRequestStatus(t, resp) @@ -255,9 +229,7 @@ func TestGetTeamUnread(t *testing.T) { teamUnread, resp = th.SystemAdminClient.GetTeamUnread(th.BasicTeam.Id, th.BasicUser.Id) CheckNoError(t, resp) - if teamUnread.TeamId != th.BasicTeam.Id { - t.Fatal("wrong team id returned") - } + require.Equal(t, teamUnread.TeamId, th.BasicTeam.Id, "wrong team id returned") } func TestUpdateTeam(t *testing.T) { @@ -272,17 +244,13 @@ func TestUpdateTeam(t *testing.T) { uteam, resp := Client.UpdateTeam(team) CheckNoError(t, resp) - if uteam.Description != "updated description" { - t.Fatal("Update failed") - } + require.Equal(t, uteam.Description, "updated description", "Update failed") team.DisplayName = "Updated Name" uteam, resp = Client.UpdateTeam(team) CheckNoError(t, resp) - if uteam.DisplayName != "Updated Name" { - t.Fatal("Update failed") - } + require.Equal(t, uteam.DisplayName, "Updated Name", "Update failed") // Test GroupConstrained flag team.GroupConstrained = model.NewBool(true) @@ -290,58 +258,45 @@ func TestUpdateTeam(t *testing.T) { CheckNoError(t, resp) CheckOKStatus(t, resp) - if *rteam.GroupConstrained != *team.GroupConstrained { - t.Fatal("GroupConstrained flags do not match") - } + require.Equal(t, *rteam.GroupConstrained, *team.GroupConstrained, "GroupConstrained flags do not match") + team.GroupConstrained = nil team.AllowOpenInvite = true uteam, resp = Client.UpdateTeam(team) CheckNoError(t, resp) - if !uteam.AllowOpenInvite { - t.Fatal("Update failed") - } + require.True(t, uteam.AllowOpenInvite, "Update failed") team.InviteId = "inviteid1" uteam, resp = Client.UpdateTeam(team) CheckNoError(t, resp) - if uteam.InviteId == "inviteid1" { - t.Fatal("InviteID should not be updated") - } + require.NotEqual(t, uteam.InviteId, "inviteid1", "InviteID should not be updated") team.AllowedDomains = "domain" uteam, resp = Client.UpdateTeam(team) CheckNoError(t, resp) - if uteam.AllowedDomains != "domain" { - t.Fatal("Update failed") - } + require.Equal(t, uteam.AllowedDomains, "domain", "Update failed") team.Name = "Updated name" uteam, resp = Client.UpdateTeam(team) CheckNoError(t, resp) - if uteam.Name == "Updated name" { - t.Fatal("Should not update name") - } + require.NotEqual(t, uteam.Name, "Updated name", "Should not update name") team.Email = "test@domain.com" uteam, resp = Client.UpdateTeam(team) CheckNoError(t, resp) - if uteam.Email == "test@domain.com" { - t.Fatal("Should not update email") - } + require.NotEqual(t, uteam.Email, "test@domain.com", "Should not update email") team.Type = model.TEAM_INVITE uteam, resp = Client.UpdateTeam(team) CheckNoError(t, resp) - if uteam.Type == model.TEAM_INVITE { - t.Fatal("Should not update type") - } + require.NotEqual(t, uteam.Type, model.TEAM_INVITE, "Should not update type") originalTeamId := team.Id team.Id = model.NewId() @@ -349,9 +304,7 @@ func TestUpdateTeam(t *testing.T) { r, _ := Client.DoApiPut(Client.GetTeamRoute(originalTeamId), team.ToJson()) assert.Equal(t, http.StatusBadRequest, r.StatusCode) - if uteam.Id != originalTeamId { - t.Fatal("wrong team id") - } + require.Equal(t, uteam.Id, originalTeamId, "wrong team id") team.Id = "fake" _, resp = Client.UpdateTeam(team) @@ -384,17 +337,13 @@ func TestUpdateTeamSanitization(t *testing.T) { t.Run("team admin", func(t *testing.T) { rteam, resp := th.Client.UpdateTeam(team) CheckNoError(t, resp) - if rteam.Email == "" { - t.Fatal("should not have sanitized email for admin") - } + require.NotEmpty(t, rteam.Email, "should not have sanitized email for admin") }) t.Run("system admin", func(t *testing.T) { rteam, resp := th.SystemAdminClient.UpdateTeam(team) CheckNoError(t, resp) - if rteam.Email == "" { - t.Fatal("should not have sanitized email for admin") - } + require.NotEmpty(t, rteam.Email, "should not have sanitized email for admin") }) } @@ -416,21 +365,11 @@ func TestPatchTeam(t *testing.T) { rteam, resp := Client.PatchTeam(team.Id, patch) CheckNoError(t, resp) - if rteam.DisplayName != "Other name" { - t.Fatal("DisplayName did not update properly") - } - if rteam.Description != "Other description" { - t.Fatal("Description did not update properly") - } - if rteam.CompanyName != "Other company name" { - t.Fatal("CompanyName did not update properly") - } - if rteam.InviteId == "inviteid1" { - t.Fatal("InviteId should not update") - } - if !rteam.AllowOpenInvite { - t.Fatal("AllowOpenInvite did not update properly") - } + require.Equal(t, rteam.DisplayName, "Other name", "DisplayName did not update properly") + require.Equal(t, rteam.Description, "Other description", "Description did not update properly") + require.Equal(t, rteam.CompanyName, "Other company name", "CompanyName did not update properly") + require.NotEqual(t, rteam.InviteId, "inviteid1", "InviteId should not update") + require.True(t, rteam.AllowOpenInvite, "AllowOpenInvite did not update properly") // Test GroupConstrained flag patch.GroupConstrained = model.NewBool(true) @@ -438,9 +377,7 @@ func TestPatchTeam(t *testing.T) { CheckNoError(t, resp) CheckOKStatus(t, resp) - if *rteam.GroupConstrained != *patch.GroupConstrained { - t.Fatal("GroupConstrained flags do not match") - } + require.Equal(t, *rteam.GroupConstrained, *patch.GroupConstrained, "GroupConstrained flags do not match") patch.GroupConstrained = nil _, resp = Client.PatchTeam("junk", patch) @@ -449,15 +386,10 @@ func TestPatchTeam(t *testing.T) { _, resp = Client.PatchTeam(GenerateTestId(), patch) CheckForbiddenStatus(t, resp) - if r, err := Client.DoApiPut("/teams/"+team.Id+"/patch", "garbage"); err == nil { - t.Fatal("should have errored") - } else { - if r.StatusCode != http.StatusBadRequest { - t.Log("actual: " + strconv.Itoa(r.StatusCode)) - t.Log("expected: " + strconv.Itoa(http.StatusBadRequest)) - t.Fatal("wrong status code") - } - } + r, err := Client.DoApiPut("/teams/"+team.Id+"/patch", "garbage") + require.NotNil(t, err, "should have errored") + + require.Equalf(t, r.StatusCode, http.StatusBadRequest, "wrong status code, actual: %s, expected: %s", strconv.Itoa(r.StatusCode), strconv.Itoa(http.StatusBadRequest)) Client.Logout() _, resp = Client.PatchTeam(team.Id, patch) @@ -489,17 +421,13 @@ func TestPatchTeamSanitization(t *testing.T) { t.Run("team admin", func(t *testing.T) { rteam, resp := th.Client.PatchTeam(team.Id, &model.TeamPatch{}) CheckNoError(t, resp) - if rteam.Email == "" { - t.Fatal("should not have sanitized email for admin") - } + require.NotEmpty(t, rteam.Email, "should not have sanitized email for admin") }) t.Run("system admin", func(t *testing.T) { rteam, resp := th.SystemAdminClient.PatchTeam(team.Id, &model.TeamPatch{}) CheckNoError(t, resp) - if rteam.Email == "" { - t.Fatal("should not have sanitized email for admin") - } + require.NotEmpty(t, rteam.Email, "should not have sanitized email for admin") }) } @@ -532,24 +460,16 @@ func TestSoftDeleteTeam(t *testing.T) { ok, resp := Client.SoftDeleteTeam(team.Id) CheckNoError(t, resp) - if !ok { - t.Fatal("should have returned true") - } + require.True(t, ok, "should have returned true") rteam, err := th.App.GetTeam(team.Id) - if err != nil { - t.Fatal("should have returned archived team") - } - if rteam.DeleteAt == 0 { - t.Fatal("should have not set to zero") - } + require.Nil(t, err, "should have returned archived team") + require.NotEqual(t, rteam.DeleteAt, 0, "should have not set to zero") ok, resp = Client.SoftDeleteTeam("junk") CheckBadRequestStatus(t, resp) - if ok { - t.Fatal("should have returned false") - } + require.False(t, ok, "should have returned false") otherTeam := th.BasicTeam _, resp = Client.SoftDeleteTeam(otherTeam.Id) @@ -599,9 +519,7 @@ func TestPermanentDeleteTeam(t *testing.T) { ok, resp = Client.PermanentDeleteTeam("junk") CheckBadRequestStatus(t, resp) - if ok { - t.Fatal("should have returned false") - } + require.False(t, ok, "should have returned false") } func TestGetAllTeams(t *testing.T) { @@ -783,20 +701,15 @@ func TestGetAllTeamsSanitization(t *testing.T) { for _, rteam := range rteams { if rteam.Id == team.Id { teamFound = true - if rteam.Email == "" { - t.Fatal("should not have sanitized email for team admin") - } + require.NotEmpty(t, rteam.Email, "should not have sanitized email for team admin") } else if rteam.Id == team2.Id { team2Found = true - if rteam.Email != "" { - t.Fatal("should've sanitized email for non-admin") - } + require.Empty(t, rteam.Email, "should've sanitized email for non-admin") } } - if !teamFound || !team2Found { - t.Fatal("wasn't returned the expected teams so the test wasn't run correctly") - } + require.True(t, teamFound, "wasn't returned the expected teams so the test wasn't run correctly") + require.True(t, team2Found, "wasn't returned the expected teams so the test wasn't run correctly") }) t.Run("system admin", func(t *testing.T) { @@ -807,9 +720,7 @@ func TestGetAllTeamsSanitization(t *testing.T) { continue } - if rteam.Email == "" { - t.Fatal("should not have sanitized email") - } + require.NotEmpty(t, rteam.Email, "should not have sanitized email") } }) } @@ -823,9 +734,7 @@ func TestGetTeamByName(t *testing.T) { rteam, resp := Client.GetTeamByName(team.Name, "") CheckNoError(t, resp) - if rteam.Name != team.Name { - t.Fatal("wrong team") - } + require.Equal(t, rteam.Name, team.Name, "wrong team") _, resp = Client.GetTeamByName("junk", "") CheckNotFoundStatus(t, resp) @@ -882,25 +791,19 @@ func TestGetTeamByNameSanitization(t *testing.T) { rteam, resp := client.GetTeamByName(team.Name, "") CheckNoError(t, resp) - if rteam.Email != "" { - t.Fatal("should've sanitized email") - } + require.Empty(t, rteam.Email, "should've sanitized email") }) t.Run("team admin/non-admin", func(t *testing.T) { rteam, resp := th.Client.GetTeamByName(team.Name, "") CheckNoError(t, resp) - if rteam.Email == "" { - t.Fatal("should not have sanitized email") - } + require.NotEmpty(t, rteam.Email, "should not have sanitized email") }) t.Run("system admin", func(t *testing.T) { rteam, resp := th.SystemAdminClient.GetTeamByName(team.Name, "") CheckNoError(t, resp) - if rteam.Email == "" { - t.Fatal("should not have sanitized email") - } + require.NotEmpty(t, rteam.Email, "should not have sanitized email") }) } @@ -911,11 +814,9 @@ func TestSearchAllTeams(t *testing.T) { oTeam := th.BasicTeam oTeam.AllowOpenInvite = true - if updatedTeam, err := th.App.UpdateTeam(oTeam); err != nil { - t.Fatal(err) - } else { - oTeam.UpdateAt = updatedTeam.UpdateAt - } + updatedTeam, err := th.App.UpdateTeam(oTeam) + require.Nil(t, err, err) + oTeam.UpdateAt = updatedTeam.UpdateAt pTeam := &model.Team{DisplayName: "PName", Name: GenerateTestTeamName(), Email: th.GenerateTestEmail(), Type: model.TEAM_INVITE} Client.CreateTeam(pTeam) @@ -923,59 +824,41 @@ func TestSearchAllTeams(t *testing.T) { rteams, resp := Client.SearchTeams(&model.TeamSearch{Term: oTeam.Name}) CheckNoError(t, resp) - if len(rteams) != 1 { - t.Fatal("should have returned 1 team") - } + require.Len(t, rteams, 1, "should have returned 1 team") - if oTeam.Id != rteams[0].Id { - t.Fatal("invalid team") - } + require.Equal(t, oTeam.Id, rteams[0].Id, "invalid team") rteams, resp = Client.SearchTeams(&model.TeamSearch{Term: oTeam.DisplayName}) CheckNoError(t, resp) - if len(rteams) != 1 { - t.Fatal("should have returned 1 team") - } + require.Len(t, rteams, 1, "should have returned 1 team") - if rteams[0].Id != oTeam.Id { - t.Fatal("invalid team") - } + require.Equal(t, oTeam.Id, rteams[0].Id, "invalid team") rteams, resp = Client.SearchTeams(&model.TeamSearch{Term: pTeam.Name}) CheckNoError(t, resp) - if len(rteams) != 0 { - t.Fatal("should have not returned team") - } + require.Len(t, rteams, 0, "should have not returned team") rteams, resp = Client.SearchTeams(&model.TeamSearch{Term: pTeam.DisplayName}) CheckNoError(t, resp) - if len(rteams) != 0 { - t.Fatal("should have not returned team") - } + require.Len(t, rteams, 0, "should have not returned team") rteams, resp = th.SystemAdminClient.SearchTeams(&model.TeamSearch{Term: oTeam.Name}) CheckNoError(t, resp) - if len(rteams) != 1 { - t.Fatal("should have returned 1 team") - } + require.Len(t, rteams, 1, "should have returned 1 team") rteams, resp = th.SystemAdminClient.SearchTeams(&model.TeamSearch{Term: pTeam.DisplayName}) CheckNoError(t, resp) - if len(rteams) != 1 { - t.Fatal("should have returned 1 team") - } + require.Len(t, rteams, 1, "should have returned 1 team") rteams, resp = Client.SearchTeams(&model.TeamSearch{Term: "junk"}) CheckNoError(t, resp) - if len(rteams) != 0 { - t.Fatal("should have not returned team") - } + require.Len(t, rteams, 0, "should have not returned team") Client.Logout() @@ -1014,11 +897,8 @@ func TestSearchAllTeamsSanitization(t *testing.T) { rteams, resp := client.SearchTeams(&model.TeamSearch{Term: t.Name()}) CheckNoError(t, resp) for _, rteam := range rteams { - if rteam.Email != "" { - t.Fatal("should've sanitized email") - } else if rteam.AllowedDomains != "" { - t.Fatal("should've sanitized allowed domains") - } + require.Empty(t, rteam.Email, "should've sanitized email") + require.Empty(t, rteam.AllowedDomains, "should've sanitized allowed domains") } }) @@ -1031,11 +911,8 @@ func TestSearchAllTeamsSanitization(t *testing.T) { rteams, resp := client.SearchTeams(&model.TeamSearch{Term: t.Name()}) CheckNoError(t, resp) for _, rteam := range rteams { - if rteam.Email != "" { - t.Fatal("should've sanitized email") - } else if rteam.AllowedDomains != "" { - t.Fatal("should've sanitized allowed domains") - } + require.Empty(t, rteam.Email, "should've sanitized email") + require.Empty(t, rteam.AllowedDomains, "should've sanitized allowed domains") } }) @@ -1044,9 +921,7 @@ func TestSearchAllTeamsSanitization(t *testing.T) { CheckNoError(t, resp) for _, rteam := range rteams { if rteam.Id == team.Id || rteam.Id == team2.Id || rteam.Id == th.BasicTeam.Id { - if rteam.Email == "" { - t.Fatal("should not have sanitized email") - } + require.NotEmpty(t, rteam.Email, "should not have sanitized email") } } }) @@ -1055,9 +930,7 @@ func TestSearchAllTeamsSanitization(t *testing.T) { rteams, resp := th.SystemAdminClient.SearchTeams(&model.TeamSearch{Term: t.Name()}) CheckNoError(t, resp) for _, rteam := range rteams { - if rteam.Email == "" { - t.Fatal("should not have sanitized email") - } + require.NotEmpty(t, rteam.Email, "should not have sanitized email") } }) } @@ -1073,9 +946,7 @@ func TestGetTeamsForUser(t *testing.T) { teams, resp := Client.GetTeamsForUser(th.BasicUser.Id, "") CheckNoError(t, resp) - if len(teams) != 2 { - t.Fatal("wrong number of teams") - } + require.Len(t, teams, 2, "wrong number of teams") found1 := false found2 := false @@ -1087,9 +958,8 @@ func TestGetTeamsForUser(t *testing.T) { } } - if !found1 || !found2 { - t.Fatal("missing team") - } + require.True(t, found1, "missing team") + require.True(t, found2, "missing team") _, resp = Client.GetTeamsForUser("junk", "") CheckBadRequestStatus(t, resp) @@ -1139,9 +1009,7 @@ func TestGetTeamsForUserSanitization(t *testing.T) { continue } - if rteam.Email != "" { - t.Fatal("should've sanitized email") - } + require.Empty(t, rteam.Email, "should've sanitized email") } }) @@ -1153,9 +1021,7 @@ func TestGetTeamsForUserSanitization(t *testing.T) { continue } - if rteam.Email == "" { - t.Fatal("should not have sanitized email") - } + require.NotEmpty(t, rteam.Email, "should not have sanitized email") } }) @@ -1167,9 +1033,7 @@ func TestGetTeamsForUserSanitization(t *testing.T) { continue } - if rteam.Email == "" { - t.Fatal("should not have sanitized email") - } + require.NotEmpty(t, rteam.Email, "should not have sanitized email") } }) } @@ -1184,13 +1048,9 @@ func TestGetTeamMember(t *testing.T) { rmember, resp := Client.GetTeamMember(team.Id, user.Id, "") CheckNoError(t, resp) - if rmember.TeamId != team.Id { - t.Fatal("wrong team id") - } + require.Equal(t, rmember.TeamId, team.Id, "wrong team id") - if rmember.UserId != user.Id { - t.Fatal("wrong team id") - } + require.Equal(t, rmember.UserId, user.Id, "wrong user id") _, resp = Client.GetTeamMember("junk", user.Id, "") CheckBadRequestStatus(t, resp) @@ -1223,33 +1083,24 @@ func TestGetTeamMembers(t *testing.T) { t.Logf("rmembers count %v\n", len(rmembers)) - if len(rmembers) == 0 { - t.Fatal("should have results") - } + require.NotEqual(t, len(rmembers), 0, "should have results") for _, rmember := range rmembers { - if rmember.TeamId != team.Id || rmember.UserId == userNotMember.Id { - t.Fatal("user should be a member of team") - } + require.Equal(t, rmember.TeamId, team.Id, "user should be a member of team") + require.NotEqual(t, rmember.UserId, userNotMember.Id, "user should be a member of team") } rmembers, resp = Client.GetTeamMembers(team.Id, 0, 1, "") CheckNoError(t, resp) - if len(rmembers) != 1 { - t.Fatal("should be 1 per page") - } + require.Len(t, rmembers, 1, "should be 1 per page") rmembers, resp = Client.GetTeamMembers(team.Id, 1, 1, "") CheckNoError(t, resp) - if len(rmembers) != 1 { - t.Fatal("should be 1 per page") - } + require.Len(t, rmembers, 1, "should be 1 per page") rmembers, resp = Client.GetTeamMembers(team.Id, 10000, 100, "") CheckNoError(t, resp) - if len(rmembers) != 0 { - t.Fatal("should be no member") - } + require.Len(t, rmembers, 0, "should be no member") rmembers, resp = Client.GetTeamMembers(team.Id, 0, 2, "") CheckNoError(t, resp) @@ -1291,9 +1142,7 @@ func TestGetTeamMembersForUser(t *testing.T) { } } - if !found { - t.Fatal("missing team member") - } + require.True(t, found, "missing team member") _, resp = Client.GetTeamMembersForUser("junk", "") CheckBadRequestStatus(t, resp) @@ -1322,24 +1171,18 @@ func TestGetTeamMembersByIds(t *testing.T) { tm, resp := Client.GetTeamMembersByIds(th.BasicTeam.Id, []string{th.BasicUser.Id}) CheckNoError(t, resp) - if tm[0].UserId != th.BasicUser.Id { - t.Fatal("returned wrong user") - } + require.Equal(t, tm[0].UserId, th.BasicUser.Id, "returned wrong user") _, resp = Client.GetTeamMembersByIds(th.BasicTeam.Id, []string{}) CheckBadRequestStatus(t, resp) tm1, resp := Client.GetTeamMembersByIds(th.BasicTeam.Id, []string{"junk"}) CheckNoError(t, resp) - if len(tm1) > 0 { - t.Fatal("no users should be returned") - } + require.False(t, len(tm1) > 0, "no users should be returned") tm1, resp = Client.GetTeamMembersByIds(th.BasicTeam.Id, []string{"junk", th.BasicUser.Id}) CheckNoError(t, resp) - if len(tm1) != 1 { - t.Fatal("1 user should be returned") - } + require.Len(t, tm1, 1, "1 user should be returned") _, resp = Client.GetTeamMembersByIds("junk", []string{th.BasicUser.Id}) CheckBadRequestStatus(t, resp) @@ -1372,17 +1215,16 @@ func TestAddTeamMember(t *testing.T) { _, resp := th.SystemAdminClient.DemoteUserToGuest(guest.Id) CheckNoError(t, resp) - if err := th.App.RemoveUserFromTeam(th.BasicTeam.Id, th.BasicUser2.Id, ""); err != nil { - t.Fatalf(err.Error()) + err := th.App.RemoveUserFromTeam(th.BasicTeam.Id, th.BasicUser2.Id, "") + if err != nil { + require.FailNow(t, err.Error()) } // Regular user can't add a member to a team they don't belong to. th.LoginBasic2() _, resp = Client.AddTeamMember(team.Id, otherUser.Id) CheckForbiddenStatus(t, resp) - if resp.Error == nil { - t.Fatalf("Error is nil") - } + require.NotNil(t, resp.Error, "Error is nil") Client.Logout() // Regular user can add a member to a team they belong to. @@ -1392,25 +1234,17 @@ func TestAddTeamMember(t *testing.T) { CheckCreatedStatus(t, resp) // Check all the returned data. - if tm == nil { - t.Fatal("should have returned team member") - } + require.NotNil(t, tm, "should have returned team member") - if tm.UserId != otherUser.Id { - t.Fatal("user ids should have matched") - } + require.Equal(t, tm.UserId, otherUser.Id, "user ids should have matched") - if tm.TeamId != team.Id { - t.Fatal("team ids should have matched") - } + require.Equal(t, tm.TeamId, team.Id, "team ids should have matched") // Check with various invalid requests. tm, resp = Client.AddTeamMember(team.Id, "junk") CheckBadRequestStatus(t, resp) - if tm != nil { - t.Fatal("should have not returned team member") - } + require.Nil(t, tm, "should have not returned team member") _, resp = Client.AddTeamMember("junk", otherUser.Id) CheckBadRequestStatus(t, resp) @@ -1476,27 +1310,19 @@ func TestAddTeamMember(t *testing.T) { tm, resp = Client.AddTeamMemberFromInvite(token.Token, "") CheckNoError(t, resp) - if tm == nil { - t.Fatal("should have returned team member") - } + require.NotNil(t, tm, "should have returned team member") - if tm.UserId != otherUser.Id { - t.Fatal("user ids should have matched") - } + require.Equal(t, tm.UserId, otherUser.Id, "user ids should have matched") - if tm.TeamId != team.Id { - t.Fatal("team ids should have matched") - } + require.Equal(t, tm.TeamId, team.Id, "team ids should have matched") - _, err := th.App.Srv.Store.Token().GetByToken(token.Token) + _, err = th.App.Srv.Store.Token().GetByToken(token.Token) require.NotNil(t, err, "The token must be deleted after be used") tm, resp = Client.AddTeamMemberFromInvite("junk", "") CheckBadRequestStatus(t, resp) - if tm != nil { - t.Fatal("should have not returned team member") - } + require.Nil(t, tm, "should have not returned team member") // expired token of more than 50 hours token = model.NewToken(app.TOKEN_TYPE_TEAM_INVITATION, "") @@ -1534,24 +1360,16 @@ func TestAddTeamMember(t *testing.T) { tm, resp = Client.AddTeamMemberFromInvite("", team.InviteId) CheckNoError(t, resp) - if tm == nil { - t.Fatal("should have returned team member") - } + require.NotNil(t, tm, "should have returned team member") - if tm.UserId != otherUser.Id { - t.Fatal("user ids should have matched") - } + require.Equal(t, tm.UserId, otherUser.Id, "user ids should have matched") - if tm.TeamId != team.Id { - t.Fatal("team ids should have matched") - } + require.Equal(t, tm.TeamId, team.Id, "team ids should have matched") tm, resp = Client.AddTeamMemberFromInvite("", "junk") CheckNotFoundStatus(t, resp) - if tm != nil { - t.Fatal("should have not returned team member") - } + require.Nil(t, tm, "should have not returned team member") // Set a team to group-constrained team.GroupConstrained = model.NewBool(true) @@ -1691,9 +1509,8 @@ func TestAddTeamMembers(t *testing.T) { otherUser.Id, } - if err := th.App.RemoveUserFromTeam(th.BasicTeam.Id, th.BasicUser2.Id, ""); err != nil { - t.Fatalf(err.Error()) - } + err := th.App.RemoveUserFromTeam(th.BasicTeam.Id, th.BasicUser2.Id, "") + require.Nil(t, err) // Regular user can't add a member to a team they don't belong to. th.LoginBasic2() @@ -1708,17 +1525,11 @@ func TestAddTeamMembers(t *testing.T) { CheckCreatedStatus(t, resp) // Check all the returned data. - if tm[0] == nil { - t.Fatal("should have returned team member") - } + require.NotNil(t, tm[0], "should have returned team member") - if tm[0].UserId != otherUser.Id { - t.Fatal("user ids should have matched") - } + require.Equal(t, tm[0].UserId, otherUser.Id, "user ids should have matched") - if tm[0].TeamId != team.Id { - t.Fatal("team ids should have matched") - } + require.Equal(t, tm[0].TeamId, team.Id, "team ids should have matched") // Check with various invalid requests. _, resp = Client.AddTeamMembers("junk", userList) @@ -1783,7 +1594,7 @@ func TestAddTeamMembers(t *testing.T) { // Set a team to group-constrained team.GroupConstrained = model.NewBool(true) - _, err := th.App.UpdateTeam(team) + _, err = th.App.UpdateTeam(team) require.Nil(t, err) // User is not in associated groups so shouldn't be allowed @@ -1814,9 +1625,7 @@ func TestRemoveTeamMember(t *testing.T) { pass, resp := Client.RemoveTeamMember(th.BasicTeam.Id, th.BasicUser.Id) CheckNoError(t, resp) - if !pass { - t.Fatal("should have passed") - } + require.True(t, pass, "should have passed") _, resp = th.SystemAdminClient.AddTeamMember(th.BasicTeam.Id, th.BasicUser.Id) CheckNoError(t, resp) @@ -1860,17 +1669,11 @@ func TestGetTeamStats(t *testing.T) { rstats, resp := Client.GetTeamStats(team.Id, "") CheckNoError(t, resp) - if rstats.TeamId != team.Id { - t.Fatal("wrong team id") - } + require.Equal(t, rstats.TeamId, team.Id, "wrong team id") - if rstats.TotalMemberCount != 3 { - t.Fatal("wrong count") - } + require.Equal(t, rstats.TotalMemberCount, int64(3), "wrong count") - if rstats.ActiveMemberCount != 3 { - t.Fatal("wrong count") - } + require.Equal(t, rstats.ActiveMemberCount, int64(3), "wrong count") _, resp = Client.GetTeamStats("junk", "") CheckBadRequestStatus(t, resp) @@ -1887,13 +1690,9 @@ func TestGetTeamStats(t *testing.T) { rstats, resp = th.SystemAdminClient.GetTeamStats(team.Id, "") CheckNoError(t, resp) - if rstats.TotalMemberCount != 3 { - t.Fatal("wrong count") - } + require.Equal(t, rstats.TotalMemberCount, int64(3), "wrong count") - if rstats.ActiveMemberCount != 2 { - t.Fatal("wrong count") - } + require.Equal(t, rstats.ActiveMemberCount, int64(2), "wrong count") // login with different user and test if forbidden user := th.CreateUser() @@ -1918,9 +1717,7 @@ func TestUpdateTeamMemberRoles(t *testing.T) { // user 1 tries to promote user 2 ok, resp := Client.UpdateTeamMemberRoles(th.BasicTeam.Id, th.BasicUser2.Id, TEAM_ADMIN) CheckForbiddenStatus(t, resp) - if ok { - t.Fatal("should have returned false") - } + require.False(t, ok, "should have returned false") // user 1 tries to promote himself _, resp = Client.UpdateTeamMemberRoles(th.BasicTeam.Id, th.BasicUser.Id, TEAM_ADMIN) @@ -1933,9 +1730,7 @@ func TestUpdateTeamMemberRoles(t *testing.T) { // system admin promotes user 1 ok, resp = SystemAdminClient.UpdateTeamMemberRoles(th.BasicTeam.Id, th.BasicUser.Id, TEAM_ADMIN) CheckNoError(t, resp) - if !ok { - t.Fatal("should have returned true") - } + require.True(t, ok, "should have returned true") // user 1 (team admin) promotes user 2 _, resp = Client.UpdateTeamMemberRoles(th.BasicTeam.Id, th.BasicUser2.Id, TEAM_ADMIN) @@ -2099,15 +1894,11 @@ func TestGetMyTeamsUnread(t *testing.T) { teams, resp := Client.GetTeamsUnreadForUser(user.Id, "") CheckNoError(t, resp) - if len(teams) == 0 { - t.Fatal("should have results") - } + require.NotEqual(t, len(teams), 0, "should have results") teams, resp = Client.GetTeamsUnreadForUser(user.Id, th.BasicTeam.Id) CheckNoError(t, resp) - if len(teams) != 0 { - t.Fatal("should not have results") - } + require.Len(t, teams, 0, "should not have results") _, resp = Client.GetTeamsUnreadForUser("fail", "") CheckBadRequestStatus(t, resp) @@ -2222,56 +2013,40 @@ func TestImportTeam(t *testing.T) { var data []byte var err error data, err = testutils.ReadTestFile("Fake_Team_Import.zip") - if err != nil && len(data) == 0 { - t.Fatal("Error while reading the test file.") - } + + require.False(t, err != nil && len(data) == 0, "Error while reading the test file.") // Import the channels/users/posts fileResp, resp := th.SystemAdminClient.ImportTeam(data, binary.Size(data), "slack", "Fake_Team_Import.zip", th.BasicTeam.Id) CheckNoError(t, resp) fileData, err := base64.StdEncoding.DecodeString(fileResp["results"]) - if err != nil { - t.Fatal("failed to decode base64 results data") - } + require.Nil(t, err, "failed to decode base64 results data") fileReturned := fmt.Sprintf("%s", fileData) - if !strings.Contains(fileReturned, "darth.vader@stardeath.com") { - t.Log(fileReturned) - t.Fatal("failed to report the user was imported") - } + require.Truef(t, strings.Contains(fileReturned, "darth.vader@stardeath.com"), "failed to report the user was imported, fileReturned: %s", fileReturned) // Checking the imported users importedUser, resp := th.SystemAdminClient.GetUserByUsername("bot_test", "") CheckNoError(t, resp) - if importedUser.Username != "bot_test" { - t.Fatal("username should match with the imported user") - } + require.Equal(t, importedUser.Username, "bot_test", "username should match with the imported user") importedUser, resp = th.SystemAdminClient.GetUserByUsername("lordvader", "") CheckNoError(t, resp) - if importedUser.Username != "lordvader" { - t.Fatal("username should match with the imported user") - } + require.Equal(t, importedUser.Username, "lordvader", "username should match with the imported user") // Checking the imported Channels importedChannel, resp := th.SystemAdminClient.GetChannelByName("testchannel", th.BasicTeam.Id, "") CheckNoError(t, resp) - if importedChannel.Name != "testchannel" { - t.Fatal("names did not match expected: testchannel") - } + require.Equal(t, importedChannel.Name, "testchannel", "names did not match expected: testchannel") importedChannel, resp = th.SystemAdminClient.GetChannelByName("general", th.BasicTeam.Id, "") CheckNoError(t, resp) - if importedChannel.Name != "general" { - t.Fatal("names did not match expected: general") - } + require.Equal(t, importedChannel.Name, "general", "names did not match expected: general") posts, resp := th.SystemAdminClient.GetPostsForChannel(importedChannel.Id, 0, 60, "") CheckNoError(t, resp) - if posts.Posts[posts.Order[3]].Message != "This is a test post to test the import process" { - t.Fatal("missing posts in the import process") - } + require.Equal(t, posts.Posts[posts.Order[3]].Message, "This is a test post to test the import process", "missing posts in the import process") }) t.Run("MissingFile", func(t *testing.T) { @@ -2283,9 +2058,7 @@ func TestImportTeam(t *testing.T) { var data []byte var err error data, err = testutils.ReadTestFile("Fake_Team_Import.zip") - if err != nil && len(data) == 0 { - t.Fatal("Error while reading the test file.") - } + require.False(t, err != nil && len(data) == 0, "Error while reading the test file.") // Import the channels/users/posts _, resp := th.Client.ImportTeam(data, binary.Size(data), "slack", "Fake_Team_Import.zip", th.BasicTeam.Id) @@ -2315,16 +2088,12 @@ func TestInviteUsersToTeam(t *testing.T) { th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableEmailInvitations = false }) _, resp := th.SystemAdminClient.InviteUsersToTeam(th.BasicTeam.Id, emailList) - if resp.Error == nil { - t.Fatal("Should be disabled") - } + require.NotNil(t, resp.Error, "Should be disabled") th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableEmailInvitations = true }) okMsg, resp := th.SystemAdminClient.InviteUsersToTeam(th.BasicTeam.Id, emailList) CheckNoError(t, resp) - if !okMsg { - t.Fatal("should return true") - } + require.True(t, okMsg, "should return true") nameFormat := *th.App.Config().TeamSettings.TeammateNameDisplay expectedSubject := utils.T("api.templates.invite_subject", @@ -2345,16 +2114,10 @@ func TestInviteUsersToTeam(t *testing.T) { t.Log("No email was received, maybe due load on the server. Disabling this verification") } if err == nil && len(resultsMailbox) > 0 { - if !strings.ContainsAny(resultsMailbox[len(resultsMailbox)-1].To[0], email) { - t.Fatal("Wrong To recipient") - } else { - if resultsEmail, err := mailservice.GetMessageFromMailbox(email, resultsMailbox[len(resultsMailbox)-1].ID); err == nil { - if resultsEmail.Subject != expectedSubject { - t.Log(resultsEmail.Subject) - t.Log(expectedSubject) - t.Fatal("Wrong Subject") - } - } + require.True(t, strings.ContainsAny(resultsMailbox[len(resultsMailbox)-1].To[0], email), "Wrong To recipient") + resultsEmail, err := mailservice.GetMessageFromMailbox(email, resultsMailbox[len(resultsMailbox)-1].ID) + if err == nil { + require.Equalf(t, resultsEmail.Subject, expectedSubject, "Wrong Subject, actual: %s, expected: %s", resultsEmail.Subject, expectedSubject) } } } @@ -2364,42 +2127,30 @@ func TestInviteUsersToTeam(t *testing.T) { t.Run("restricted domains", func(t *testing.T) { err := th.App.InviteNewUsersToTeam(emailList, th.BasicTeam.Id, th.BasicUser.Id) - if err == nil { - t.Fatal("Adding users with non-restricted domains was allowed") - } - if err.Where != "InviteNewUsersToTeam" || err.Id != "api.team.invite_members.invalid_email.app_error" { - t.Log(err) - t.Fatal("Got wrong error message!") - } + require.NotNil(t, err, "Adding users with non-restricted domains was allowed") + + require.Equalf(t, err.Where, "InviteNewUsersToTeam", "%v, Got wrong error message!", err) + require.Equalf(t, err.Id, "api.team.invite_members.invalid_email.app_error", "%v, Got wrong error message!", err) }) t.Run("override restricted domains", func(t *testing.T) { th.BasicTeam.AllowedDomains = "invalid.com,common.com" - if _, err := th.App.UpdateTeam(th.BasicTeam); err == nil { - t.Fatal("Should not update the team") - } + _, err := th.App.UpdateTeam(th.BasicTeam) + require.NotNil(t, err, "Should not update the team") th.BasicTeam.AllowedDomains = "common.com" - if _, err := th.App.UpdateTeam(th.BasicTeam); err != nil { - t.Log(err) - t.Fatal("Should update the team") - } + _, err = th.App.UpdateTeam(th.BasicTeam) + require.Nilf(t, err, "%v, Should update the team", err) - if err := th.App.InviteNewUsersToTeam([]string{"test@global.com"}, th.BasicTeam.Id, th.BasicUser.Id); err == nil || err.Where != "InviteNewUsersToTeam" { - t.Log(err) - t.Fatal("Per team restriction should take precedence over the global restriction") - } + err = th.App.InviteNewUsersToTeam([]string{"test@global.com"}, th.BasicTeam.Id, th.BasicUser.Id) + require.NotNilf(t, err, "%v, Per team restriction should take precedence over the global restriction", err) + require.Equalf(t, err.Where, "InviteNewUsersToTeam", "%v, Per team restriction should take precedence over the global restriction", err) - if err := th.App.InviteNewUsersToTeam([]string{"test@common.com"}, th.BasicTeam.Id, th.BasicUser.Id); err != nil { - t.Log(err) - t.Fatal("Failed to invite user which was common between team and global domain restriction") - } - - if err := th.App.InviteNewUsersToTeam([]string{"test@invalid.com"}, th.BasicTeam.Id, th.BasicUser.Id); err == nil { - t.Log(err) - t.Fatal("Should not invite user") - } + err = th.App.InviteNewUsersToTeam([]string{"test@common.com"}, th.BasicTeam.Id, th.BasicUser.Id) + require.Nilf(t, err, "%v, Failed to invite user which was common between team and global domain restriction", err) + err = th.App.InviteNewUsersToTeam([]string{"test@invalid.com"}, th.BasicTeam.Id, th.BasicUser.Id) + require.NotNilf(t, err, "%v, Should not invite user", err) }) } @@ -2438,27 +2189,21 @@ func TestInviteGuestsToTeam(t *testing.T) { th.App.UpdateConfig(func(cfg *model.Config) { *cfg.GuestAccountsSettings.Enable = true }) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableEmailInvitations = false }) _, resp = th.SystemAdminClient.InviteGuestsToTeam(th.BasicTeam.Id, emailList, []string{th.BasicChannel.Id}, "test-message") - if resp.Error == nil { - t.Fatal("Should be disabled") - } + require.NotNil(t, resp.Error, "Should be disabled") th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableEmailInvitations = true }) th.App.SetLicense(nil) _, resp = th.SystemAdminClient.InviteGuestsToTeam(th.BasicTeam.Id, emailList, []string{th.BasicChannel.Id}, "test-message") - if resp.Error == nil { - t.Fatal("Should be disabled") - } + require.NotNil(t, resp.Error, "Should be disabled") th.App.SetLicense(model.NewTestLicense("")) defer th.App.SetLicense(nil) okMsg, resp := th.SystemAdminClient.InviteGuestsToTeam(th.BasicTeam.Id, emailList, []string{th.BasicChannel.Id}, "test-message") CheckNoError(t, resp) - if !okMsg { - t.Fatal("should return true") - } + require.True(t, okMsg, "should return true") nameFormat := *th.App.Config().TeamSettings.TeammateNameDisplay expectedSubject := utils.T("api.templates.invite_guest_subject", @@ -2479,16 +2224,10 @@ func TestInviteGuestsToTeam(t *testing.T) { t.Log("No email was received, maybe due load on the server. Disabling this verification") } if err == nil && len(resultsMailbox) > 0 { - if !strings.ContainsAny(resultsMailbox[len(resultsMailbox)-1].To[0], email) { - t.Fatal("Wrong To recipient") - } else { - if resultsEmail, err := mailservice.GetMessageFromMailbox(email, resultsMailbox[len(resultsMailbox)-1].ID); err == nil { - if resultsEmail.Subject != expectedSubject { - t.Log(resultsEmail.Subject) - t.Log(expectedSubject) - t.Fatal("Wrong Subject") - } - } + require.True(t, strings.ContainsAny(resultsMailbox[len(resultsMailbox)-1].To[0], email), "Wrong To recipient") + resultsEmail, err := mailservice.GetMessageFromMailbox(email, resultsMailbox[len(resultsMailbox)-1].ID) + if err == nil { + require.Equalf(t, resultsEmail.Subject, expectedSubject, "Wrong Subject, actual: %s, expected: %s", resultsEmail.Subject, expectedSubject) } } } @@ -2527,13 +2266,9 @@ func TestGetTeamInviteInfo(t *testing.T) { team, resp := Client.GetTeamInviteInfo(team.InviteId) CheckNoError(t, resp) - if team.DisplayName == "" { - t.Fatal("should not be empty") - } + require.NotEmpty(t, team.DisplayName, "should not be empty") - if team.Email != "" { - t.Fatal("should be empty") - } + require.Empty(t, team.Email, "should be empty") team.InviteId = "12345678901234567890123456789012" team, resp = th.SystemAdminClient.UpdateTeam(team) @@ -2553,22 +2288,18 @@ func TestSetTeamIcon(t *testing.T) { team := th.BasicTeam data, err := testutils.ReadTestFile("test.png") - if err != nil { - t.Fatal(err) - } + require.Nil(t, err, err) th.LoginTeamAdmin() ok, resp := Client.SetTeamIcon(team.Id, data) - if !ok { - t.Fatal(resp.Error) - } + require.True(t, ok, resp.Error) + CheckNoError(t, resp) ok, resp = Client.SetTeamIcon(model.NewId(), data) - if ok { - t.Fatal("Should return false, set team icon not allowed") - } + require.False(t, ok, "Should return false, set team icon not allowed") + CheckForbiddenStatus(t, resp) th.LoginBasic() @@ -2579,7 +2310,7 @@ func TestSetTeamIcon(t *testing.T) { } else if resp.StatusCode == http.StatusUnauthorized { CheckUnauthorizedStatus(t, resp) } else { - t.Fatal("Should have failed either forbidden or unauthorized") + require.Fail(t, "Should have failed either forbidden or unauthorized") } Client.Logout() @@ -2590,7 +2321,7 @@ func TestSetTeamIcon(t *testing.T) { } else if resp.StatusCode == http.StatusUnauthorized { CheckUnauthorizedStatus(t, resp) } else { - t.Fatal("Should have failed either forbidden or unauthorized") + require.Fail(t, "Should have failed either forbidden or unauthorized") } teamBefore, err := th.App.GetTeam(team.Id) @@ -2604,9 +2335,8 @@ func TestSetTeamIcon(t *testing.T) { 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) - } + err = th.cleanupTestFile(info) + require.Nil(t, err, err) } func TestGetTeamIcon(t *testing.T) { @@ -2638,18 +2368,14 @@ func TestRemoveTeamIcon(t *testing.T) { _, 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") - } + require.Equal(t, teamAfter.LastTeamIconUpdate, int64(0), "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") - } + require.Equal(t, teamAfter.LastTeamIconUpdate, int64(0), "should update LastTeamIconUpdate to 0") Client.SetTeamIcon(team.Id, data) Client.Logout() From bf693b6a0c99f813a64afe13c9be766bb25545d4 Mon Sep 17 00:00:00 2001 From: ishanray Date: Tue, 12 Nov 2019 09:14:57 -0500 Subject: [PATCH 17/18] Fix typo by removing extra p in function name (#12996) --- web/static.go | 2 +- web/unsupported_browser.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/web/static.go b/web/static.go index 57c1126a4f..9f20f2958a 100644 --- a/web/static.go +++ b/web/static.go @@ -60,7 +60,7 @@ func (w *Web) InitStatic() { func root(c *Context, w http.ResponseWriter, r *http.Request) { if !CheckClientCompatability(r.UserAgent()) { - renderUnsuppportedBrowser(c.App, w, r) + renderUnsupportedBrowser(c.App, w, r) return } diff --git a/web/unsupported_browser.go b/web/unsupported_browser.go index db02ab8a39..39de6ddb2c 100644 --- a/web/unsupported_browser.go +++ b/web/unsupported_browser.go @@ -44,7 +44,7 @@ type SystemBrowser struct { MakeDefaultString string } -func renderUnsuppportedBrowser(app *app.App, w http.ResponseWriter, r *http.Request) { +func renderUnsupportedBrowser(app *app.App, w http.ResponseWriter, r *http.Request) { w.Header().Set("Cache-Control", "no-store") page := utils.NewHTMLTemplate(app.HTMLTemplates(), "unsupported_browser") From 0c8b580458c362446cdcb3fbaec873ff3c3065e3 Mon Sep 17 00:00:00 2001 From: Chanwit Piromplad Date: Tue, 12 Nov 2019 21:15:32 +0700 Subject: [PATCH 18/18] migrate test to use testify (#12894) * migrate test to use testify * use require.NotEmpty --- utils/license_test.go | 18 ++++++------------ utils/urlencode_test.go | 17 +++++------------ 2 files changed, 11 insertions(+), 24 deletions(-) diff --git a/utils/license_test.go b/utils/license_test.go index 1c909ff847..513b351671 100644 --- a/utils/license_test.go +++ b/utils/license_test.go @@ -14,26 +14,20 @@ import ( func TestValidateLicense(t *testing.T) { b1 := []byte("junk") - if ok, _ := ValidateLicense(b1); ok { - t.Fatal("should have failed - bad license") - } + ok, _ := ValidateLicense(b1) + require.False(t, ok, "should have failed - bad license") b2 := []byte("junkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunkjunk") - if ok, _ := ValidateLicense(b2); ok { - t.Fatal("should have failed - bad license") - } + ok, _ = ValidateLicense(b2) + require.False(t, ok, "should have failed - bad license") } func TestGetLicenseFileLocation(t *testing.T) { fileName := GetLicenseFileLocation("") - if len(fileName) == 0 { - t.Fatal("invalid default file name") - } + require.NotEmpty(t, fileName, "invalid default file name") fileName = GetLicenseFileLocation("mattermost.mattermost-license") - if fileName != "mattermost.mattermost-license" { - t.Fatal("invalid file name") - } + require.Equal(t, fileName, "mattermost.mattermost-license", "invalid file name") } func TestGetLicenseFileFromDisk(t *testing.T) { diff --git a/utils/urlencode_test.go b/utils/urlencode_test.go index 2ba453c73a..54bd3b7329 100644 --- a/utils/urlencode_test.go +++ b/utils/urlencode_test.go @@ -5,6 +5,8 @@ package utils import ( "testing" + + "github.com/stretchr/testify/require" ) func TestUrlEncode(t *testing.T) { @@ -12,24 +14,15 @@ func TestUrlEncode(t *testing.T) { toEncode := "testing 1 2 3" encoded := UrlEncode(toEncode) - if encoded != "testing%201%202%203" { - t.Log(encoded) - t.Fatal("should be equal") - } + require.Equal(t, encoded, "testing%201%202%203") toEncode = "testing123" encoded = UrlEncode(toEncode) - if encoded != "testing123" { - t.Log(encoded) - t.Fatal("should be equal") - } + require.Equal(t, encoded, "testing123") toEncode = "testing$#~123" encoded = UrlEncode(toEncode) - if encoded != "testing%24%23~123" { - t.Log(encoded) - t.Fatal("should be equal") - } + require.Equal(t, encoded, "testing%24%23~123") }