diff --git a/.circleci/config.yml b/.circleci/config.yml index fc344c5fce..04b56c4bdd 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -63,6 +63,8 @@ jobs: at: /go/src/github.com/mattermost/ - run: command: | + echo "Installing golangci-lint" + curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh| sh -s -- -b /usr/local/bin v1.21.0 cd mattermost-server make config-reset make check-style BUILD_NUMBER='${CIRCLE_BRANCH}-${CIRCLE_BUILD_NUM}' diff --git a/Makefile b/Makefile index 8f83b2b516..3244fedb94 100644 --- a/Makefile +++ b/Makefile @@ -46,7 +46,6 @@ else endif # Golang Flags -export GO111MODULE=on GOPATH ?= $(shell go env GOPATH) GOFLAGS ?= $(GOFLAGS:) -mod=vendor GO=go @@ -149,31 +148,10 @@ else endif -govet: ## Runs govet against all packages. - @echo Running GOVET - env GO111MODULE=off $(GO) get golang.org/x/tools/go/analysis/passes/shadow/cmd/shadow - $(GO) vet $(GOFLAGS) $(ALL_PACKAGES) || exit 1 - $(GO) vet -vettool=$(GOPATH)/bin/shadow $(GOFLAGS) $(ALL_PACKAGES) || exit 1 +plugin-checker: $(GO) run $(GOFLAGS) ./plugin/checker -gofmt: ## Runs gofmt against all packages. - @echo Running GOFMT - - @for package in $(TE_PACKAGES) $(EE_PACKAGES); do \ - echo "Checking "$$package; \ - files=$$($(GO) list $(GOFLAGS) -f '{{range .GoFiles}}{{$$.Dir}}/{{.}} {{end}}' $$package); \ - if [ "$$files" ]; then \ - gofmt_output=$$(gofmt -d -s $$files 2>&1); \ - if [ "$$gofmt_output" ]; then \ - echo "$$gofmt_output"; \ - echo "gofmt failure"; \ - exit 1; \ - fi; \ - fi; \ - done - @echo "gofmt success"; \ - -golangci-lint: ## Run golangci-lint on codebasis +golangci-lint: ## Run golangci-lint on codebase # https://stackoverflow.com/a/677212/1027058 (check if a command exists or not) @if ! [ -x "$$(command -v golangci-lint)" ]; then \ echo "golangci-lint is not installed. Please see https://github.com/golangci/golangci-lint#install for installation instructions."; \ @@ -229,8 +207,7 @@ check-licenses: ## Checks license status. 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 check-plugin-golint ## Runs govet and gofmt against all packages and also ensures plugin package golint compliant +check-style: golangci-lint plugin-checker check-licenses check-plugin-golint ## Runs golangci 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 diff --git a/api4/commands_test.go b/api4/commands_test.go index 5ba0e5f918..7dce542648 100644 --- a/api4/commands_test.go +++ b/api4/commands_test.go @@ -9,6 +9,7 @@ import ( "time" "github.com/mattermost/mattermost-server/model" + "github.com/stretchr/testify/require" ) func TestEchoCommand(t *testing.T) { @@ -20,20 +21,16 @@ func TestEchoCommand(t *testing.T) { echoTestString := "/echo test" - if r1 := Client.Must(Client.ExecuteCommand(channel1.Id, echoTestString)).(*model.CommandResponse); r1 == nil { - t.Fatal("Echo command failed to execute") - } + r1 := Client.Must(Client.ExecuteCommand(channel1.Id, echoTestString)).(*model.CommandResponse) + require.NotNil(t, r1, "Echo command failed to execute") - if r1 := Client.Must(Client.ExecuteCommand(channel1.Id, "/echo ")).(*model.CommandResponse); r1 == nil { - t.Fatal("Echo command failed to execute") - } + r1 = Client.Must(Client.ExecuteCommand(channel1.Id, "/echo ")).(*model.CommandResponse) + require.NotNil(t, r1, "Echo command failed to execute") time.Sleep(100 * time.Millisecond) p1 := Client.Must(Client.GetPostsForChannel(channel1.Id, 0, 2, "")).(*model.PostList) - if len(p1.Order) != 2 { - t.Fatal("Echo command failed to send") - } + require.Len(t, p1.Order, 2, "Echo command failed to send") } func TestGroupmsgCommands(t *testing.T) { @@ -57,25 +54,18 @@ func TestGroupmsgCommands(t *testing.T) { rs1 := Client.Must(Client.ExecuteCommand(th.BasicChannel.Id, "/groupmsg "+user2.Username+","+user3.Username)).(*model.CommandResponse) group1 := model.GetGroupNameFromUserIds([]string{user1.Id, user2.Id, user3.Id}) - - if !strings.HasSuffix(rs1.GotoLocation, "/"+team.Name+"/channels/"+group1) { - t.Fatal("failed to create group channel") - } + require.True(t, strings.HasSuffix(rs1.GotoLocation, "/"+team.Name+"/channels/"+group1), "failed to create group channel") rs2 := Client.Must(Client.ExecuteCommand(th.BasicChannel.Id, "/groupmsg "+user3.Username+","+user4.Username+" foobar")).(*model.CommandResponse) group2 := model.GetGroupNameFromUserIds([]string{user1.Id, user3.Id, user4.Id}) - if !strings.HasSuffix(rs2.GotoLocation, "/"+team.Name+"/channels/"+group2) { - t.Fatal("failed to create second direct channel") - } - if result := Client.Must(Client.SearchPosts(team.Id, "foobar", false)).(*model.PostList); len(result.Order) == 0 { - t.Fatal("post did not get sent to direct message") - } + require.True(t, strings.HasSuffix(rs2.GotoLocation, "/"+team.Name+"/channels/"+group2), "failed to create second direct channel") + + result := Client.Must(Client.SearchPosts(team.Id, "foobar", false)).(*model.PostList) + require.NotEqual(t, 0, len(result.Order), "post did not get sent to direct message") rs3 := Client.Must(Client.ExecuteCommand(th.BasicChannel.Id, "/groupmsg "+user2.Username+","+user3.Username)).(*model.CommandResponse) - if !strings.HasSuffix(rs3.GotoLocation, "/"+team.Name+"/channels/"+group1) { - t.Fatal("failed to go back to existing group channel") - } + require.True(t, strings.HasSuffix(rs3.GotoLocation, "/"+team.Name+"/channels/"+group1), "failed to go back to existing group channel") Client.Must(Client.ExecuteCommand(th.BasicChannel.Id, "/groupmsg "+user2.Username+" foobar")) Client.Must(Client.ExecuteCommand(th.BasicChannel.Id, "/groupmsg "+user2.Username+","+user3.Username+","+user4.Username+","+user5.Username+","+user6.Username+","+user7.Username+","+user8.Username+","+user9.Username+" foobar")) @@ -91,19 +81,13 @@ func TestInvitePeopleCommand(t *testing.T) { channel := th.BasicChannel r1 := Client.Must(Client.ExecuteCommand(channel.Id, "/invite_people test@example.com")).(*model.CommandResponse) - if r1 == nil { - t.Fatal("Command failed to execute") - } + require.NotNil(t, r1, "Command failed to execute") r2 := Client.Must(Client.ExecuteCommand(channel.Id, "/invite_people test1@example.com test2@example.com")).(*model.CommandResponse) - if r2 == nil { - t.Fatal("Command failed to execute") - } + require.NotNil(t, r2, "Command failed to execute") r3 := Client.Must(Client.ExecuteCommand(channel.Id, "/invite_people")).(*model.CommandResponse) - if r3 == nil { - t.Fatal("Command failed to execute") - } + require.NotNil(t, r3, "Command failed to execute") } // also used to test /open (see command_open_test.go) @@ -129,14 +113,10 @@ func testJoinCommands(t *testing.T, alias string) { channel3 := Client.Must(Client.CreateDirectChannel(th.BasicUser.Id, user2.Id)).(*model.Channel) rs5 := Client.Must(Client.ExecuteCommand(channel0.Id, "/"+alias+" "+channel2.Name)).(*model.CommandResponse) - if !strings.HasSuffix(rs5.GotoLocation, "/"+team.Name+"/channels/"+channel2.Name) { - t.Fatal("failed to join channel") - } + require.True(t, strings.HasSuffix(rs5.GotoLocation, "/"+team.Name+"/channels/"+channel2.Name), "failed to join channel") rs6 := Client.Must(Client.ExecuteCommand(channel0.Id, "/"+alias+" "+channel3.Name)).(*model.CommandResponse) - if strings.HasSuffix(rs6.GotoLocation, "/"+team.Name+"/channels/"+channel3.Name) { - t.Fatal("should not have joined direct message channel") - } + require.False(t, strings.HasSuffix(rs6.GotoLocation, "/"+team.Name+"/channels/"+channel3.Name), "should not have joined direct message channel") c1 := Client.Must(Client.GetChannelsForTeamForUser(th.BasicTeam.Id, th.BasicUser.Id, "")).([]*model.Channel) @@ -146,10 +126,7 @@ func testJoinCommands(t *testing.T, alias string) { found = true } } - - if !found { - t.Fatal("did not join channel") - } + require.True(t, found, "did not join channel") } func TestJoinCommands(t *testing.T) { @@ -171,9 +148,7 @@ func TestLoadTestHelpCommands(t *testing.T) { th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableTesting = true }) rs := Client.Must(Client.ExecuteCommand(channel.Id, "/test help")).(*model.CommandResponse) - if !strings.Contains(rs.Text, "Mattermost testing commands to help") { - t.Fatal(rs.Text) - } + require.True(t, strings.Contains(rs.Text, "Mattermost testing commands to help"), rs.Text) time.Sleep(2 * time.Second) } @@ -193,9 +168,7 @@ func TestLoadTestSetupCommands(t *testing.T) { th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableTesting = true }) rs := Client.Must(Client.ExecuteCommand(channel.Id, "/test setup fuzz 1 1 1")).(*model.CommandResponse) - if rs.Text != "Created environment" { - t.Fatal(rs.Text) - } + require.Equal(t, "Created environment", rs.Text, rs.Text) time.Sleep(2 * time.Second) } @@ -215,9 +188,7 @@ func TestLoadTestUsersCommands(t *testing.T) { th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableTesting = true }) rs := Client.Must(Client.ExecuteCommand(channel.Id, "/test users fuzz 1 2")).(*model.CommandResponse) - if rs.Text != "Added users" { - t.Fatal(rs.Text) - } + require.Equal(t, "Added users", rs.Text, rs.Text) time.Sleep(2 * time.Second) } @@ -237,9 +208,7 @@ func TestLoadTestChannelsCommands(t *testing.T) { th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableTesting = true }) rs := Client.Must(Client.ExecuteCommand(channel.Id, "/test channels fuzz 1 2")).(*model.CommandResponse) - if rs.Text != "Added channels" { - t.Fatal(rs.Text) - } + require.Equal(t, "Added channels", rs.Text, rs.Text) time.Sleep(2 * time.Second) } @@ -259,9 +228,7 @@ func TestLoadTestPostsCommands(t *testing.T) { th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableTesting = true }) rs := Client.Must(Client.ExecuteCommand(channel.Id, "/test posts fuzz 2 3 2")).(*model.CommandResponse) - if rs.Text != "Added posts" { - t.Fatal(rs.Text) - } + require.Equal(t, "Added posts", rs.Text, rs.Text) time.Sleep(2 * time.Second) } @@ -286,19 +253,13 @@ func TestLeaveCommands(t *testing.T) { channel3 := Client.Must(Client.CreateDirectChannel(th.BasicUser.Id, user2.Id)).(*model.Channel) rs1 := Client.Must(Client.ExecuteCommand(channel1.Id, "/leave")).(*model.CommandResponse) - if !strings.HasSuffix(rs1.GotoLocation, "/"+team.Name+"/channels/"+model.DEFAULT_CHANNEL) { - t.Fatal("failed to leave open channel 1") - } + require.True(t, strings.HasSuffix(rs1.GotoLocation, "/"+team.Name+"/channels/"+model.DEFAULT_CHANNEL), "failed to leave open channel 1") rs2 := Client.Must(Client.ExecuteCommand(channel2.Id, "/leave")).(*model.CommandResponse) - if !strings.HasSuffix(rs2.GotoLocation, "/"+team.Name+"/channels/"+model.DEFAULT_CHANNEL) { - t.Fatal("failed to leave private channel 1") - } + require.True(t, strings.HasSuffix(rs2.GotoLocation, "/"+team.Name+"/channels/"+model.DEFAULT_CHANNEL), "failed to leave private channel 1") _, err := Client.ExecuteCommand(channel3.Id, "/leave") - if err == nil { - t.Fatal("should fail leaving direct channel") - } + require.NotNil(t, err, "should fail leaving direct channel") cdata := Client.Must(Client.GetChannelsForTeamForUser(th.BasicTeam.Id, th.BasicUser.Id, "")).([]*model.Channel) @@ -308,16 +269,12 @@ func TestLeaveCommands(t *testing.T) { found = true } } - - if found { - t.Fatal("did not leave right channels") - } + require.False(t, found, "did not leave right channels") for _, c := range cdata { if c.Name == model.DEFAULT_CHANNEL { - if _, err := Client.RemoveUserFromChannel(c.Id, th.BasicUser.Id); err == nil { - t.Fatal("should have errored on leaving default channel") - } + _, err := Client.RemoveUserFromChannel(c.Id, th.BasicUser.Id) + require.NotNil(t, err, "should have errored on leaving default channel") break } } @@ -340,28 +297,19 @@ func TestMeCommand(t *testing.T) { testString := "/me hello" r1 := Client.Must(Client.ExecuteCommand(channel.Id, testString)).(*model.CommandResponse) - if r1 == nil { - t.Fatal("Command failed to execute") - } + require.NotNil(t, r1, "Command failed to execute") time.Sleep(100 * time.Millisecond) p1 := Client.Must(Client.GetPostsForChannel(channel.Id, 0, 2, "")).(*model.PostList) - if len(p1.Order) != 2 { - t.Fatal("Command failed to send") - } else { - pt := p1.Posts[p1.Order[0]].Type - if pt != model.POST_ME { - t.Log(pt) - t.Fatalf("invalid post type, got '%s', wanted '%s'", pt, model.POST_ME) - } - msg := p1.Posts[p1.Order[0]].Message - want := "*hello*" - if msg != want { - t.Log(msg) - t.Fatalf("invalid me response message, got '%s', wanted '%s'", msg, want) - } - } + require.Len(t, p1.Order, 2, "Command failed to send") + + pt := p1.Posts[p1.Order[0]].Type + require.Equal(t, model.POST_ME, pt, "invalid post type") + + msg := p1.Posts[p1.Order[0]].Message + want := "*hello*" + require.Equal(t, want, msg, "invalid me response") } func TestMsgCommands(t *testing.T) { @@ -379,22 +327,25 @@ func TestMsgCommands(t *testing.T) { Client.Must(Client.CreateDirectChannel(th.BasicUser.Id, user3.Id)) rs1 := Client.Must(Client.ExecuteCommand(th.BasicChannel.Id, "/msg "+user2.Username)).(*model.CommandResponse) - if !strings.HasSuffix(rs1.GotoLocation, "/"+team.Name+"/channels/"+user1.Id+"__"+user2.Id) && !strings.HasSuffix(rs1.GotoLocation, "/"+team.Name+"/channels/"+user2.Id+"__"+user1.Id) { - t.Fatal("failed to create direct channel") - } + require.Condition(t, func() bool { + return strings.HasSuffix(rs1.GotoLocation, "/"+team.Name+"/channels/"+user1.Id+"__"+user2.Id) || + strings.HasSuffix(rs1.GotoLocation, "/"+team.Name+"/channels/"+user2.Id+"__"+user1.Id) + }, "failed to create direct channel") rs2 := Client.Must(Client.ExecuteCommand(th.BasicChannel.Id, "/msg "+user3.Username+" foobar")).(*model.CommandResponse) - if !strings.HasSuffix(rs2.GotoLocation, "/"+team.Name+"/channels/"+user1.Id+"__"+user3.Id) && !strings.HasSuffix(rs2.GotoLocation, "/"+team.Name+"/channels/"+user3.Id+"__"+user1.Id) { - t.Fatal("failed to create second direct channel") - } - if result := Client.Must(Client.SearchPosts(th.BasicTeam.Id, "foobar", false)).(*model.PostList); len(result.Order) == 0 { - t.Fatalf("post did not get sent to direct message") - } + require.Condition(t, func() bool { + return strings.HasSuffix(rs2.GotoLocation, "/"+team.Name+"/channels/"+user1.Id+"__"+user3.Id) || + strings.HasSuffix(rs2.GotoLocation, "/"+team.Name+"/channels/"+user3.Id+"__"+user1.Id) + }, "failed to create second direct channel") + + result := Client.Must(Client.SearchPosts(th.BasicTeam.Id, "foobar", false)).(*model.PostList) + require.NotEqual(t, 0, len(result.Order), "post did not get sent to direct message") rs3 := Client.Must(Client.ExecuteCommand(th.BasicChannel.Id, "/msg "+user2.Username)).(*model.CommandResponse) - if !strings.HasSuffix(rs3.GotoLocation, "/"+team.Name+"/channels/"+user1.Id+"__"+user2.Id) && !strings.HasSuffix(rs3.GotoLocation, "/"+team.Name+"/channels/"+user2.Id+"__"+user1.Id) { - t.Fatal("failed to go back to existing direct channel") - } + require.Condition(t, func() bool { + return strings.HasSuffix(rs3.GotoLocation, "/"+team.Name+"/channels/"+user1.Id+"__"+user2.Id) || + strings.HasSuffix(rs3.GotoLocation, "/"+team.Name+"/channels/"+user2.Id+"__"+user1.Id) + }, "failed to go back to existing direct channel") Client.Must(Client.ExecuteCommand(th.BasicChannel.Id, "/msg "+th.BasicUser.Username+" foobar")) Client.Must(Client.ExecuteCommand(th.BasicChannel.Id, "/msg junk foobar")) @@ -435,21 +386,13 @@ func TestShrugCommand(t *testing.T) { testString := "/shrug" r1 := Client.Must(Client.ExecuteCommand(channel.Id, testString)).(*model.CommandResponse) - if r1 == nil { - t.Fatal("Command failed to execute") - } + require.NotNil(t, r1, "Command failed to execute") time.Sleep(100 * time.Millisecond) p1 := Client.Must(Client.GetPostsForChannel(channel.Id, 0, 2, "")).(*model.PostList) - if len(p1.Order) != 2 { - t.Fatal("Command failed to send") - } else { - if p1.Posts[p1.Order[0]].Message != `¯\\\_(ツ)\_/¯` { - t.Log(p1.Posts[p1.Order[0]].Message) - t.Fatal("invalid shrug response") - } - } + require.Len(t, p1.Order, 2, "Command failed to send") + require.Equal(t, `¯\\\_(ツ)\_/¯`, p1.Posts[p1.Order[0]].Message, "invalid shrug response") } func TestStatusCommands(t *testing.T) { @@ -467,15 +410,10 @@ func commandAndTest(t *testing.T, th *TestHelper, status string) { user := th.BasicUser r1 := Client.Must(Client.ExecuteCommand(channel.Id, "/"+status)).(*model.CommandResponse) - if r1 == nil { - t.Fatal("Command failed to execute") - } + require.NotEqual(t, "Command failed to execute", r1) time.Sleep(1000 * time.Millisecond) rstatus := Client.Must(Client.GetUserStatus(user.Id, "")).(*model.Status) - - if rstatus.Status != status { - t.Fatal("Error setting status " + status) - } + require.Equal(t, status, rstatus.Status, "Error setting status") } diff --git a/api4/group.go b/api4/group.go index 4f799c3b58..a0434e0ed8 100644 --- a/api4/group.go +++ b/api4/group.go @@ -575,23 +575,40 @@ func getGroups(c *Context, w http.ResponseWriter, r *http.Request) { c.Err = model.NewAppError("Api4.getGroups", "api.ldap_groups.license_error", nil, "", http.StatusNotImplemented) return } + var teamID, channelID string + + if id := c.Params.NotAssociatedToTeam; model.IsValidId(id) { + teamID = id + } + + if id := c.Params.NotAssociatedToChannel; model.IsValidId(id) { + channelID = id + } + + if teamID == "" && channelID == "" { + c.Err = model.NewAppError("Api4.getGroups", "api.getGroups.invalid_or_missing_channel_or_team_id", nil, "", http.StatusBadRequest) + return + } opts := model.GroupSearchOpts{ Q: c.Params.Q, IncludeMemberCount: c.Params.IncludeMemberCount, } - teamID := c.Params.NotAssociatedToTeam - if len(teamID) == 26 { - if !c.App.SessionHasPermissionToTeam(c.App.Session, teamID, model.PERMISSION_VIEW_TEAM) { - c.SetPermissionError(model.PERMISSION_VIEW_TEAM) + if teamID != "" { + _, err := c.App.GetTeam(teamID) + if err != nil { + c.Err = err + return + } + if !c.App.SessionHasPermissionToTeam(c.App.Session, teamID, model.PERMISSION_MANAGE_TEAM) { + c.SetPermissionError(model.PERMISSION_MANAGE_TEAM) return } opts.NotAssociatedToTeam = teamID } - channelID := c.Params.NotAssociatedToChannel - if len(channelID) == 26 { + if channelID != "" { channel, err := c.App.GetChannel(channelID) if err != nil { c.Err = err diff --git a/api4/group_test.go b/api4/group_test.go index a94066efdc..b6cb0361ae 100644 --- a/api4/group_test.go +++ b/api4/group_test.go @@ -8,11 +8,9 @@ import ( "net/http" "testing" - "github.com/stretchr/testify/require" - - "github.com/stretchr/testify/assert" - "github.com/mattermost/mattermost-server/model" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestGetGroup(t *testing.T) { @@ -765,6 +763,20 @@ func TestGetGroups(t *testing.T) { th.App.SetLicense(model.NewTestLicense("ldap")) + _, response = th.SystemAdminClient.GetGroups(opts) + CheckBadRequestStatus(t, response) + + _, response = th.SystemAdminClient.UpdateChannelRoles(th.BasicChannel.Id, th.BasicUser.Id, "") + require.Nil(t, response.Error) + + opts.NotAssociatedToChannel = th.BasicChannel.Id + + _, response = th.Client.GetGroups(opts) + CheckForbiddenStatus(t, response) + + _, response = th.SystemAdminClient.UpdateChannelRoles(th.BasicChannel.Id, th.BasicUser.Id, "channel_user channel_admin") + require.Nil(t, response.Error) + groups, response := th.SystemAdminClient.GetGroups(opts) assert.Nil(t, response.Error) assert.ElementsMatch(t, []*model.Group{group, th.Group}, groups) @@ -787,7 +799,7 @@ func TestGetGroups(t *testing.T) { _, response = th.Client.GetGroups(opts) CheckForbiddenStatus(t, response) - _, response = th.SystemAdminClient.UpdateTeamMemberRoles(th.BasicTeam.Id, th.BasicUser.Id, "team_user") + _, response = th.SystemAdminClient.UpdateTeamMemberRoles(th.BasicTeam.Id, th.BasicUser.Id, "team_user team_admin") require.Nil(t, response.Error) _, response = th.Client.GetGroups(opts) diff --git a/api4/plugin.go b/api4/plugin.go index 18ce219975..67d29b4c11 100644 --- a/api4/plugin.go +++ b/api4/plugin.go @@ -8,6 +8,7 @@ package api4 import ( "bytes" "encoding/json" + "io" "io/ioutil" "net/http" "net/url" @@ -31,6 +32,7 @@ func (api *API) InitPlugin() { api.BaseRoutes.Plugins.Handle("", api.ApiSessionRequired(getPlugins)).Methods("GET") api.BaseRoutes.Plugin.Handle("", api.ApiSessionRequired(removePlugin)).Methods("DELETE") api.BaseRoutes.Plugins.Handle("/install_from_url", api.ApiSessionRequired(installPluginFromUrl)).Methods("POST") + api.BaseRoutes.Plugins.Handle("/marketplace", api.ApiSessionRequired(installMarketplacePlugin)).Methods("POST") api.BaseRoutes.Plugins.Handle("/statuses", api.ApiSessionRequired(getPluginStatuses)).Methods("GET") api.BaseRoutes.Plugin.Handle("/enable", api.ApiSessionRequired(enablePlugin)).Methods("POST") @@ -42,7 +44,8 @@ func (api *API) InitPlugin() { } func uploadPlugin(c *Context, w http.ResponseWriter, r *http.Request) { - if !*c.App.Config().PluginSettings.Enable || !*c.App.Config().PluginSettings.EnableUploads { + config := c.App.Config() + if !*config.PluginSettings.Enable || !*config.PluginSettings.EnableUploads || *config.PluginSettings.RequirePluginSignature { c.Err = model.NewAppError("uploadPlugin", "app.plugin.upload_disabled.app_error", nil, "", http.StatusNotImplemented) return } @@ -81,19 +84,12 @@ func uploadPlugin(c *Context, w http.ResponseWriter, r *http.Request) { if len(m.Value["force"]) > 0 && m.Value["force"][0] == "true" { force = true } - manifest, unpackErr := c.App.InstallPlugin(file, force) - if unpackErr != nil { - c.Err = unpackErr - return - } - - w.WriteHeader(http.StatusCreated) - w.Write([]byte(manifest.ToJson())) + installPlugin(c, w, file, force) } func installPluginFromUrl(c *Context, w http.ResponseWriter, r *http.Request) { - if !*c.App.Config().PluginSettings.Enable { + if !*c.App.Config().PluginSettings.Enable || *c.App.Config().PluginSettings.RequirePluginSignature { c.Err = model.NewAppError("installPluginFromUrl", "app.plugin.disabled.app_error", nil, "", http.StatusNotImplemented) return } @@ -103,46 +99,57 @@ func installPluginFromUrl(c *Context, w http.ResponseWriter, r *http.Request) { return } + force := r.URL.Query().Get("force") == "true" downloadUrl := r.URL.Query().Get("plugin_download_url") - if !model.IsValidHttpUrl(downloadUrl) { - c.Err = model.NewAppError("installPluginFromUrl", "api.plugin.install.invalid_url.app_error", nil, "", http.StatusBadRequest) - return - } - - u, err := url.ParseRequestURI(downloadUrl) + pluginFile, err := downloadFromUrl(c, downloadUrl) if err != nil { - c.Err = model.NewAppError("installPluginFromUrl", "api.plugin.install.invalid_url.app_error", nil, "", http.StatusBadRequest) + c.Err = err return } - if !*c.App.Config().PluginSettings.AllowInsecureDownloadUrl && u.Scheme != "https" { - c.Err = model.NewAppError("installPluginFromUrl", "api.plugin.install.insecure_url.app_error", nil, "", http.StatusBadRequest) + installPlugin(c, w, pluginFile, force) +} + +func installMarketplacePlugin(c *Context, w http.ResponseWriter, r *http.Request) { + if !*c.App.Config().PluginSettings.Enable { + c.Err = model.NewAppError("installMarketplacePlugin", "app.plugin.disabled.app_error", nil, "", http.StatusNotImplemented) return } - client := c.App.HTTPService.MakeClient(true) - client.Timeout = INSTALL_PLUGIN_FROM_URL_HTTP_REQUEST_TIMEOUT + if !*c.App.Config().PluginSettings.EnableMarketplace { + c.Err = model.NewAppError("installMarketplacePlugin", "app.plugin.marketplace_disabled.app_error", nil, "", http.StatusNotImplemented) + return + } - resp, err := client.Get(downloadUrl) + if !c.App.SessionHasPermissionTo(c.App.Session, model.PERMISSION_MANAGE_SYSTEM) { + c.SetPermissionError(model.PERMISSION_MANAGE_SYSTEM) + return + } + + pluginRequest, err := model.PluginRequestFromReader(r.Body) if err != nil { - c.Err = model.NewAppError("installPluginFromUrl", "api.plugin.install.download_failed.app_error", nil, err.Error(), http.StatusBadRequest) + c.Err = model.NewAppError("installMarketplacePlugin", "app.plugin.marketplace_plugin_request.app_error", nil, err.Error(), http.StatusNotImplemented) return } - defer resp.Body.Close() - - force := false - if r.URL.Query().Get("force") == "true" { - force = true + plugin, appErr := c.App.GetMarketplacePlugin(pluginRequest) + if appErr != nil { + c.Err = appErr + return } - fileBytes, err := ioutil.ReadAll(resp.Body) + pluginFile, appErr := downloadFromUrl(c, plugin.DownloadURL) + if appErr != nil { + c.Err = appErr + return + } + signature, err := plugin.DecodeSignature() if err != nil { - c.Err = model.NewAppError("installPluginFromUrl", "api.plugin.install.reading_stream_failed.app_error", nil, err.Error(), http.StatusBadRequest) + c.Err = model.NewAppError("installMarketplacePlugin", "app.plugin.signature_decode.app_error", nil, err.Error(), http.StatusNotImplemented) return } - manifest, appErr := c.App.InstallPlugin(bytes.NewReader(fileBytes), force) + manifest, appErr := c.App.InstallPluginWithSignature(pluginFile, signature) if appErr != nil { c.Err = appErr return @@ -348,3 +355,43 @@ func parseMarketplacePluginFilter(u *url.URL) (*model.MarketplacePluginFilter, e ServerVersion: serverVersion, }, nil } + +func downloadFromUrl(c *Context, downloadUrl string) (io.ReadSeeker, *model.AppError) { + if !model.IsValidHttpUrl(downloadUrl) { + return nil, model.NewAppError("downloadFromUrl", "api.plugin.install.invalid_url.app_error", nil, "", http.StatusBadRequest) + } + + u, err := url.ParseRequestURI(downloadUrl) + if err != nil { + return nil, model.NewAppError("downloadFromUrl", "api.plugin.install.invalid_url.app_error", nil, "", http.StatusBadRequest) + } + if !*c.App.Config().PluginSettings.AllowInsecureDownloadUrl && u.Scheme != "https" { + return nil, model.NewAppError("downloadFromUrl", "api.plugin.install.insecure_url.app_error", nil, "", http.StatusBadRequest) + } + + client := c.App.HTTPService.MakeClient(true) + client.Timeout = INSTALL_PLUGIN_FROM_URL_HTTP_REQUEST_TIMEOUT + + resp, err := client.Get(downloadUrl) + if err != nil { + return nil, model.NewAppError("downloadFromUrl", "api.plugin.install.download_failed.app_error", nil, err.Error(), http.StatusBadRequest) + } + defer resp.Body.Close() + + fileBytes, err := ioutil.ReadAll(resp.Body) + if err != nil { + return nil, model.NewAppError("downloadFromUrl", "api.plugin.install.reading_stream_failed.app_error", nil, err.Error(), http.StatusBadRequest) + } + + return bytes.NewReader(fileBytes), nil +} + +func installPlugin(c *Context, w http.ResponseWriter, plugin io.ReadSeeker, force bool) { + manifest, appErr := c.App.InstallPlugin(plugin, force) + if appErr != nil { + c.Err = appErr + return + } + w.WriteHeader(http.StatusCreated) + w.Write([]byte(manifest.ToJson())) +} diff --git a/api4/plugin_test.go b/api4/plugin_test.go index a1732b4db1..8934ac4c87 100644 --- a/api4/plugin_test.go +++ b/api4/plugin_test.go @@ -5,6 +5,7 @@ package api4 import ( "bytes" + "encoding/base64" "encoding/json" "io/ioutil" "net/http" @@ -206,7 +207,7 @@ func TestPlugin(t *testing.T) { // Deactivate error case ok, resp = th.SystemAdminClient.DisablePlugin("junk") - CheckBadRequestStatus(t, resp) + CheckNotFoundStatus(t, resp) assert.False(t, ok) // Get error cases @@ -241,7 +242,7 @@ func TestPlugin(t *testing.T) { // Remove error cases ok, resp = th.SystemAdminClient.RemovePlugin(manifest.Id) - CheckBadRequestStatus(t, resp) + CheckNotFoundStatus(t, resp) assert.False(t, ok) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.PluginSettings.Enable = false }) @@ -253,7 +254,7 @@ func TestPlugin(t *testing.T) { CheckForbiddenStatus(t, resp) _, resp = th.SystemAdminClient.RemovePlugin("bad.id") - CheckBadRequestStatus(t, resp) + CheckNotFoundStatus(t, resp) } func TestNotifyClusterPluginEvent(t *testing.T) { @@ -543,9 +544,9 @@ func TestGetInstalledMarketplacePlugins(t *testing.T) { samplePlugins := []*model.MarketplacePlugin{ { BaseMarketplacePlugin: &model.BaseMarketplacePlugin{ - HomepageURL: "https://github.com/mattermost/mattermost-plugin-nps", - IconData: "http://example.com/icon.svg", - DownloadURL: "https://github.com/mattermost/mattermost-plugin-nps/releases/download/v1.0.3/com.mattermost.nps-1.0.3.tar.gz", + HomepageURL: "https://example.com/mattermost/mattermost-plugin-nps", + IconData: "https://example.com/icon.svg", + DownloadURL: "https://example.com/mattermost/mattermost-plugin-nps/releases/download/v1.0.3/com.mattermost.nps-1.0.3.tar.gz", Manifest: &model.Manifest{ Id: "com.mattermost.nps", Name: "User Satisfaction Surveys", @@ -671,9 +672,9 @@ func TestSearchGetMarketplacePlugins(t *testing.T) { samplePlugins := []*model.MarketplacePlugin{ { BaseMarketplacePlugin: &model.BaseMarketplacePlugin{ - HomepageURL: "https://github.com/mattermost/mattermost-plugin-nps", + HomepageURL: "example.com/mattermost/mattermost-plugin-nps", IconData: "Cjxzdmcgdmlld0JveD0nMCAwIDEwNSA5MycgeG1sbnM9J2h0dHA6Ly93d3cudzMub3JnLzIwMDAvc3ZnJz4KPHBhdGggZD0nTTY2LDBoMzl2OTN6TTM4LDBoLTM4djkzek01MiwzNWwyNSw1OGgtMTZsLTgtMThoLTE4eicgZmlsbD0nI0VEMUMyNCcvPgo8L3N2Zz4K", - DownloadURL: "https://github.com/mattermost/mattermost-plugin-nps/releases/download/v1.0.3/com.mattermost.nps-1.0.3.tar.gz", + DownloadURL: "example.com/mattermost/mattermost-plugin-nps/releases/download/v1.0.3/com.mattermost.nps-1.0.3.tar.gz", Manifest: &model.Manifest{ Id: "com.mattermost.nps", Name: "User Satisfaction Surveys", @@ -769,6 +770,184 @@ func TestSearchGetMarketplacePlugins(t *testing.T) { }) } +func TestInstallMarketplacePlugin(t *testing.T) { + th := Setup().InitBasic() + defer th.TearDown() + + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.PluginSettings.Enable = true + *cfg.PluginSettings.EnableUploads = true + *cfg.PluginSettings.EnableMarketplace = false + }) + path, _ := fileutils.FindDir("tests") + signatureFilename := "testpluginv2.tar.gz.sig" + signatureFileReader, err := os.Open(filepath.Join(path, signatureFilename)) + require.Nil(t, err) + sigFile, err := ioutil.ReadAll(signatureFileReader) + require.Nil(t, err) + pluginSignature := base64.StdEncoding.EncodeToString(sigFile) + + tarData, err := ioutil.ReadFile(filepath.Join(path, "testpluginv2.tar.gz")) + require.NoError(t, err) + pluginServer := httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) { + res.WriteHeader(http.StatusOK) + res.Write(tarData) + })) + defer pluginServer.Close() + + samplePlugins := []*model.MarketplacePlugin{ + { + BaseMarketplacePlugin: &model.BaseMarketplacePlugin{ + HomepageURL: "https://example.com/mattermost/mattermost-plugin-nps", + IconData: "https://example.com/icon.svg", + DownloadURL: pluginServer.URL, + Manifest: &model.Manifest{ + Id: "testplugin_v2", + Name: "testplugin_v2", + Description: "dsgsdg_v2", + Version: "1.2.2", + MinServerVersion: "", + }, + }, + InstalledVersion: "", + }, + { + BaseMarketplacePlugin: &model.BaseMarketplacePlugin{ + HomepageURL: "https://example.com/mattermost/mattermost-plugin-nps", + IconData: "https://example.com/icon.svg", + DownloadURL: pluginServer.URL, + Manifest: &model.Manifest{ + Id: "testplugin_v2", + Name: "testplugin_v2", + Description: "dsgsdg_v2", + Version: "1.2.3", + MinServerVersion: "", + }, + Signature: pluginSignature, + }, + InstalledVersion: "", + }, + } + request := &model.InstallMarketplacePluginRequest{Id: "", Version: ""} + t.Run("marketplace disabled", func(t *testing.T) { + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.PluginSettings.EnableMarketplace = false + *cfg.PluginSettings.MarketplaceUrl = "invalid.com" + }) + plugin, resp := th.SystemAdminClient.InstallMarketplacePlugin(request) + CheckNotImplementedStatus(t, resp) + require.Nil(t, plugin) + }) + t.Run("RequirePluginSignature enabled", func(t *testing.T) { + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.PluginSettings.Enable = true + *cfg.PluginSettings.RequirePluginSignature = true + }) + manifest, resp := th.SystemAdminClient.UploadPlugin(bytes.NewReader(tarData)) + CheckNotImplementedStatus(t, resp) + require.Nil(t, manifest) + + manifest, resp = th.SystemAdminClient.InstallPluginFromUrl("some_url", true) + CheckNotImplementedStatus(t, resp) + require.Nil(t, manifest) + }) + + t.Run("no server", func(t *testing.T) { + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.PluginSettings.EnableMarketplace = true + *cfg.PluginSettings.MarketplaceUrl = "invalid.com" + }) + + plugin, resp := th.SystemAdminClient.InstallMarketplacePlugin(request) + CheckInternalErrorStatus(t, resp) + require.Nil(t, plugin) + }) + + t.Run("no permission", func(t *testing.T) { + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.PluginSettings.EnableMarketplace = true + *cfg.PluginSettings.MarketplaceUrl = "invalid.com" + }) + + plugin, resp := th.Client.InstallMarketplacePlugin(request) + CheckForbiddenStatus(t, resp) + require.Nil(t, plugin) + }) + + t.Run("plugin not found on the server", func(t *testing.T) { + testServer := httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) { + res.WriteHeader(http.StatusOK) + json, err := json.Marshal([]*model.MarketplacePlugin{}) + require.NoError(t, err) + res.Write(json) + })) + defer testServer.Close() + + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.PluginSettings.EnableMarketplace = true + *cfg.PluginSettings.MarketplaceUrl = testServer.URL + }) + pRequest := &model.InstallMarketplacePluginRequest{Id: "some_plugin_id", Version: "0.0.1"} + plugin, resp := th.SystemAdminClient.InstallMarketplacePlugin(pRequest) + CheckInternalErrorStatus(t, resp) + require.Nil(t, plugin) + }) + + t.Run("plugin not verified", func(t *testing.T) { + testServer := httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) { + res.WriteHeader(http.StatusOK) + json, err := json.Marshal([]*model.MarketplacePlugin{samplePlugins[0]}) + require.NoError(t, err) + res.Write(json) + })) + defer testServer.Close() + + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.PluginSettings.EnableMarketplace = true + *cfg.PluginSettings.MarketplaceUrl = testServer.URL + *cfg.PluginSettings.AllowInsecureDownloadUrl = true + }) + pRequest := &model.InstallMarketplacePluginRequest{Id: "testplugin_v2", Version: "1.2.2"} + plugin, resp := th.SystemAdminClient.InstallMarketplacePlugin(pRequest) + CheckInternalErrorStatus(t, resp) + require.Nil(t, plugin) + }) + + t.Run("verify, install and remove plugin", func(t *testing.T) { + testServer := httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) { + res.WriteHeader(http.StatusOK) + json, err := json.Marshal([]*model.MarketplacePlugin{samplePlugins[1]}) + require.NoError(t, err) + res.Write(json) + })) + defer testServer.Close() + + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.PluginSettings.EnableMarketplace = true + *cfg.PluginSettings.MarketplaceUrl = testServer.URL + }) + + pRequest := &model.InstallMarketplacePluginRequest{Id: "testplugin_v2", Version: "1.2.3"} + manifest, resp := th.SystemAdminClient.InstallMarketplacePlugin(pRequest) + CheckNoError(t, resp) + require.NotNil(t, manifest) + require.Equal(t, "testplugin_v2", manifest.Id) + require.Equal(t, "1.2.3", manifest.Version) + + filePath := filepath.Join(*th.App.Config().PluginSettings.Directory, "testplugin_v2.sig") + savedSigFile, err := th.App.ReadFile(filePath) + require.Nil(t, err) + require.EqualValues(t, sigFile, savedSigFile) + + ok, resp := th.SystemAdminClient.RemovePlugin(manifest.Id) + CheckNoError(t, resp) + assert.True(t, ok) + exists, err := th.App.FileExists(filePath) + require.Nil(t, err) + require.False(t, exists) + }) +} + func findClusterMessages(event string, msgs []*model.ClusterMessage) []*model.ClusterMessage { var result []*model.ClusterMessage for _, msg := range msgs { diff --git a/api4/reaction_test.go b/api4/reaction_test.go index 57cb04dc39..b3cc8a28ee 100644 --- a/api4/reaction_test.go +++ b/api4/reaction_test.go @@ -7,9 +7,9 @@ import ( "strings" "testing" - "github.com/stretchr/testify/assert" - "github.com/mattermost/mattermost-server/model" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestSaveReaction(t *testing.T) { @@ -34,35 +34,22 @@ func TestSaveReaction(t *testing.T) { t.Run("successful-reaction", func(t *testing.T) { rr, resp := Client.SaveReaction(reaction) CheckNoError(t, resp) + require.Equal(t, reaction.UserId, rr.UserId, "UserId did not match") + require.Equal(t, reaction.PostId, rr.PostId, "PostId did not match") + require.Equal(t, reaction.EmojiName, rr.EmojiName, "EmojiName did not match") + require.NotEqual(t, 0, rr.CreateAt, "CreateAt should exist") - if rr.UserId != reaction.UserId { - t.Fatal("UserId did not match") - } - - if rr.PostId != reaction.PostId { - t.Fatal("PostId did not match") - } - - if rr.EmojiName != reaction.EmojiName { - t.Fatal("EmojiName did not match") - } - - if rr.CreateAt == 0 { - t.Fatal("CreateAt should exist") - } - - if reactions, err := th.App.GetReactionsForPost(postId); err != nil && len(reactions) != 1 { - t.Fatal("didn't save reaction correctly") - } + reactions, err := th.App.GetReactionsForPost(postId) + require.Nil(t, err) + require.Equal(t, 1, len(reactions), "didn't save reaction correctly") }) t.Run("duplicated-reaction", func(t *testing.T) { _, resp := Client.SaveReaction(reaction) CheckNoError(t, resp) - - if reactions, err := th.App.GetReactionsForPost(postId); err != nil && len(reactions) != 1 { - t.Fatal("should have not save duplicated reaction") - } + reactions, err := th.App.GetReactionsForPost(postId) + require.Nil(t, err) + require.Equal(t, 1, len(reactions), "should have not save duplicated reaction") }) t.Run("save-second-reaction", func(t *testing.T) { @@ -70,14 +57,11 @@ func TestSaveReaction(t *testing.T) { rr, resp := Client.SaveReaction(reaction) CheckNoError(t, resp) + require.Equal(t, rr.EmojiName, reaction.EmojiName, "EmojiName did not match") - if rr.EmojiName != reaction.EmojiName { - t.Fatal("EmojiName did not match") - } - - if reactions, err := th.App.GetReactionsForPost(postId); err != nil && len(reactions) != 2 { - t.Fatal("should have save multiple reactions") - } + reactions, err := th.App.GetReactionsForPost(postId) + require.Nil(t, err, "error saving multiple reactions") + require.Equal(t, len(reactions), 2, "should have save multiple reactions") }) t.Run("saving-special-case", func(t *testing.T) { @@ -85,14 +69,11 @@ func TestSaveReaction(t *testing.T) { rr, resp := Client.SaveReaction(reaction) CheckNoError(t, resp) + require.Equal(t, reaction.EmojiName, rr.EmojiName, "EmojiName did not match") - if rr.EmojiName != reaction.EmojiName { - t.Fatal("EmojiName did not match") - } - - if reactions, err := th.App.GetReactionsForPost(postId); err != nil && len(reactions) != 3 { - t.Fatal("should have save multiple reactions") - } + reactions, err := th.App.GetReactionsForPost(postId) + require.Nil(t, err) + require.Equal(t, 3, len(reactions), "should have save multiple reactions") }) t.Run("react-to-not-existing-post-id", func(t *testing.T) { @@ -167,9 +148,9 @@ func TestSaveReaction(t *testing.T) { _, resp := Client.SaveReaction(reaction) CheckForbiddenStatus(t, resp) - if reactions, err := th.App.GetReactionsForPost(postId); err != nil || len(reactions) != 3 { - t.Fatal("should have not created a reactions") - } + reactions, err := th.App.GetReactionsForPost(postId) + require.Nil(t, err) + require.Equal(t, 3, len(reactions), "should have not created a reactions") th.AddPermissionToRole(model.PERMISSION_ADD_REACTION.Id, model.CHANNEL_USER_ROLE_ID) }) @@ -192,9 +173,9 @@ func TestSaveReaction(t *testing.T) { _, resp := Client.SaveReaction(reaction) CheckForbiddenStatus(t, resp) - if reactions, err := th.App.GetReactionsForPost(post.Id); err != nil || len(reactions) != 0 { - t.Fatal("should have not created a reaction") - } + reactions, err := th.App.GetReactionsForPost(post.Id) + require.Nil(t, err) + require.Equal(t, 0, len(reactions), "should have not created a reaction") th.App.RemoveLicense() th.App.UpdateConfig(func(cfg *model.Config) { *cfg.TeamSettings.ExperimentalTownSquareIsReadOnly = false }) @@ -218,9 +199,9 @@ func TestSaveReaction(t *testing.T) { _, resp := Client.SaveReaction(reaction) CheckForbiddenStatus(t, resp) - if reactions, err := th.App.GetReactionsForPost(post.Id); err != nil || len(reactions) != 0 { - t.Fatal("should have not created a reaction") - } + reactions, err := th.App.GetReactionsForPost(post.Id) + require.Nil(t, err) + require.Equal(t, 0, len(reactions), "should have not created a reaction") }) } @@ -263,11 +244,9 @@ func TestGetReactions(t *testing.T) { var reactions []*model.Reaction for _, userReaction := range userReactions { - if reaction, err := th.App.Srv.Store.Reaction().Save(userReaction); err != nil { - t.Fatal(err) - } else { - reactions = append(reactions, reaction) - } + reaction, err := th.App.Srv.Store.Reaction().Save(userReaction) + require.Nil(t, err) + reactions = append(reactions, reaction) } t.Run("get-reactions", func(t *testing.T) { @@ -345,70 +324,68 @@ func TestDeleteReaction(t *testing.T) { t.Run("delete-reaction", func(t *testing.T) { th.App.SaveReactionForPost(r1) - if reactions, err := th.App.GetReactionsForPost(postId); err != nil || len(reactions) != 1 { - t.Fatal("didn't save reaction correctly") - } + reactions, err := th.App.GetReactionsForPost(postId) + require.Nil(t, err) + require.Equal(t, 1, len(reactions), "didn't save reaction correctly") ok, resp := Client.DeleteReaction(r1) CheckNoError(t, resp) - if !ok { - t.Fatal("should have returned true") - } + require.True(t, ok, "should have returned true") - if reactions, err := th.App.GetReactionsForPost(postId); err != nil || len(reactions) != 0 { - t.Fatal("should have deleted reaction") - } + reactions, err = th.App.GetReactionsForPost(postId) + require.Nil(t, err) + require.Equal(t, 0, len(reactions), "should have deleted reaction") }) t.Run("delete-reaction-when-post-has-multiple-reactions", func(t *testing.T) { th.App.SaveReactionForPost(r1) th.App.SaveReactionForPost(r2) - if reactions, err := th.App.GetReactionsForPost(postId); err != nil || len(reactions) != 2 { - t.Fatal("didn't save reactions correctly") - } + reactions, err := th.App.GetReactionsForPost(postId) + require.Nil(t, err) + require.Equal(t, len(reactions), 2, "didn't save reactions correctly") _, resp := Client.DeleteReaction(r2) CheckNoError(t, resp) - if reactions, err := th.App.GetReactionsForPost(postId); err != nil || len(reactions) != 1 || *reactions[0] != *r1 { - t.Fatal("should have deleted 1 reaction only") - } + reactions, err = th.App.GetReactionsForPost(postId) + require.Nil(t, err) + require.Equal(t, 1, len(reactions), "should have deleted only 1 reaction") + require.Equal(t, *r1, *reactions[0], "should have deleted 1 reaction only") }) t.Run("delete-reaction-when-plus-one-reaction-name", func(t *testing.T) { th.App.SaveReactionForPost(r3) - if reactions, err := th.App.GetReactionsForPost(postId); err != nil || len(reactions) != 2 { - t.Fatal("didn't save reactions correctly") - } + reactions, err := th.App.GetReactionsForPost(postId) + require.Nil(t, err) + require.Equal(t, 2, len(reactions), "didn't save reactions correctly") _, resp := Client.DeleteReaction(r3) CheckNoError(t, resp) - if reactions, err := th.App.GetReactionsForPost(postId); err != nil || len(reactions) != 1 || *reactions[0] != *r1 { - t.Fatal("should have deleted 1 reaction only") - } + reactions, err = th.App.GetReactionsForPost(postId) + require.Nil(t, err) + require.Equal(t, 1, len(reactions), "should have deleted 1 reaction only") + require.Equal(t, *r1, *reactions[0], "should have deleted 1 reaction only") }) t.Run("delete-reaction-made-by-another-user", func(t *testing.T) { th.LoginBasic2() th.App.SaveReactionForPost(r4) - if reactions, err := th.App.GetReactionsForPost(postId); err != nil || len(reactions) != 2 { - t.Fatal("didn't save reaction correctly") - } + reactions, err := th.App.GetReactionsForPost(postId) + require.Nil(t, err) + require.Equal(t, 2, len(reactions), "didn't save reaction correctly") th.LoginBasic() ok, resp := Client.DeleteReaction(r4) CheckForbiddenStatus(t, resp) - if ok { - t.Fatal("should have returned false") - } + require.False(t, ok, "should have returned false") - if reactions, err := th.App.GetReactionsForPost(postId); err != nil || len(reactions) != 2 { - t.Fatal("should have not deleted a reaction") - } + reactions, err = th.App.GetReactionsForPost(postId) + require.Nil(t, err) + require.Equal(t, 2, len(reactions), "should have not deleted a reaction") }) t.Run("delete-reaction-from-not-existing-post-id", func(t *testing.T) { @@ -469,9 +446,9 @@ func TestDeleteReaction(t *testing.T) { _, resp = th.SystemAdminClient.DeleteReaction(r4) CheckNoError(t, resp) - if reactions, err := th.App.GetReactionsForPost(postId); err != nil || len(reactions) != 0 { - t.Fatal("should have deleted both reactions") - } + reactions, err := th.App.GetReactionsForPost(postId) + require.Nil(t, err) + require.Equal(t, 0, len(reactions), "should have deleted both reactions") }) t.Run("unable-to-delete-reaction-without-permissions", func(t *testing.T) { @@ -483,9 +460,9 @@ func TestDeleteReaction(t *testing.T) { _, resp := Client.DeleteReaction(r1) CheckForbiddenStatus(t, resp) - if reactions, err := th.App.GetReactionsForPost(postId); err != nil || len(reactions) != 1 { - t.Fatal("should have not deleted a reactions") - } + reactions, err := th.App.GetReactionsForPost(postId) + require.Nil(t, err) + require.Equal(t, 1, len(reactions), "should have not deleted a reactions") th.AddPermissionToRole(model.PERMISSION_REMOVE_REACTION.Id, model.CHANNEL_USER_ROLE_ID) }) @@ -496,9 +473,9 @@ func TestDeleteReaction(t *testing.T) { _, resp := th.SystemAdminClient.DeleteReaction(r1) CheckForbiddenStatus(t, resp) - if reactions, err := th.App.GetReactionsForPost(postId); err != nil || len(reactions) != 1 { - t.Fatal("should have not deleted a reactions") - } + reactions, err := th.App.GetReactionsForPost(postId) + require.Nil(t, err) + require.Equal(t, 1, len(reactions), "should have not deleted a reactions") th.AddPermissionToRole(model.PERMISSION_REMOVE_OTHERS_REACTIONS.Id, model.SYSTEM_ADMIN_ROLE_ID) }) @@ -520,18 +497,18 @@ func TestDeleteReaction(t *testing.T) { r1, resp := Client.SaveReaction(reaction) CheckNoError(t, resp) - if reactions, err := th.App.GetReactionsForPost(postId); err != nil || len(reactions) != 1 { - t.Fatal("should have created a reaction") - } + reactions, err := th.App.GetReactionsForPost(postId) + require.Nil(t, err) + require.Equal(t, 1, len(reactions), "should have created a reaction") th.App.UpdateConfig(func(cfg *model.Config) { *cfg.TeamSettings.ExperimentalTownSquareIsReadOnly = true }) _, resp = th.SystemAdminClient.DeleteReaction(r1) CheckForbiddenStatus(t, resp) - if reactions, err := th.App.GetReactionsForPost(postId); err != nil || len(reactions) != 1 { - t.Fatal("should have not deleted a reaction") - } + reactions, err = th.App.GetReactionsForPost(postId) + require.Nil(t, err) + require.Equal(t, 1, len(reactions), "should have not deleted a reaction") th.App.RemoveLicense() th.App.UpdateConfig(func(cfg *model.Config) { *cfg.TeamSettings.ExperimentalTownSquareIsReadOnly = false }) @@ -552,19 +529,19 @@ func TestDeleteReaction(t *testing.T) { r1, resp := Client.SaveReaction(reaction) CheckNoError(t, resp) - if reactions, err := th.App.GetReactionsForPost(postId); err != nil || len(reactions) != 1 { - t.Fatal("should have created a reaction") - } + reactions, err := th.App.GetReactionsForPost(postId) + require.Nil(t, err) + require.Equal(t, 1, len(reactions), "should have created a reaction") - err := th.App.DeleteChannel(channel, userId) + err = th.App.DeleteChannel(channel, userId) assert.Nil(t, err) _, resp = Client.SaveReaction(r1) CheckForbiddenStatus(t, resp) - if reactions, err := th.App.GetReactionsForPost(post.Id); err != nil || len(reactions) != 1 { - t.Fatal("should have not deleted a reaction") - } + reactions, err = th.App.GetReactionsForPost(post.Id) + require.Nil(t, err) + require.Equal(t, 1, len(reactions), "should have not deleted a reaction") }) } @@ -618,12 +595,9 @@ func TestGetBulkReactions(t *testing.T) { for _, userReaction := range userReactions { reactions := expectedPostIdsReactionsMap[userReaction.PostId] - if reaction, err := th.App.Srv.Store.Reaction().Save(userReaction); err != nil { - t.Fatal(err) - } else { - reactions = append(reactions, reaction) - - } + reaction, err := th.App.Srv.Store.Reaction().Save(userReaction) + require.Nil(t, err) + reactions = append(reactions, reaction) expectedPostIdsReactionsMap[userReaction.PostId] = reactions } diff --git a/api4/system.go b/api4/system.go index 8e192bb382..385ab28c63 100644 --- a/api4/system.go +++ b/api4/system.go @@ -410,7 +410,27 @@ func pushNotificationAck(c *Context, w http.ResponseWriter, r *http.Request) { } err := c.App.SendAckToPushProxy(ack) - if err != nil { + if ack.NotificationType == model.PUSH_TYPE_ID_LOADED { + if err != nil { + // Log the error only, then continue to fetch notification message + c.App.NotificationsLog.Error("Notification ack not sent to push proxy", + mlog.String("ackId", ack.Id), + mlog.String("type", ack.NotificationType), + mlog.String("postId", ack.PostId), + mlog.String("status", err.Error()), + ) + } + + msg, appErr := c.App.BuildFetchedPushNotificationMessage(ack.PostId, c.App.Session.UserId) + if appErr != nil { + c.Err = model.NewAppError("pushNotificationAck", "api.push_notification.id_loaded.fetch.app_error", nil, appErr.Error(), http.StatusInternalServerError) + return + } + + w.Write([]byte(msg.ToJson())) + + return + } else if err != nil { c.Err = model.NewAppError("pushNotificationAck", "api.push_notifications_ack.forward.app_error", nil, err.Error(), http.StatusInternalServerError) return } diff --git a/api4/team_test.go b/api4/team_test.go index 99e76d3a78..53ff834b74 100644 --- a/api4/team_test.go +++ b/api4/team_test.go @@ -102,6 +102,7 @@ func TestCreateTeamSanitization(t *testing.T) { rteam, resp := th.Client.CreateTeam(team) CheckNoError(t, resp) require.NotEmpty(t, rteam.Email, "should not have sanitized email") + require.NotEmpty(t, rteam.InviteId, "should not have sanitized inviteid") }) t.Run("system admin", func(t *testing.T) { @@ -116,6 +117,7 @@ func TestCreateTeamSanitization(t *testing.T) { rteam, resp := th.SystemAdminClient.CreateTeam(team) CheckNoError(t, resp) require.NotEmpty(t, rteam.Email, "should not have sanitized email") + require.NotEmpty(t, rteam.InviteId, "should not have sanitized inviteid") }) } @@ -187,18 +189,37 @@ func TestGetTeamSanitization(t *testing.T) { CheckNoError(t, resp) require.Empty(t, rteam.Email, "should have sanitized email") + require.NotEmpty(t, rteam.InviteId, "should not have sanitized inviteid") + }) + + t.Run("team user without invite permissions", func(t *testing.T) { + th.RemovePermissionFromRole(model.PERMISSION_INVITE_USER.Id, model.TEAM_USER_ROLE_ID) + th.LinkUserToTeam(th.BasicUser2, team) + + client := th.CreateClient() + th.LoginBasic2WithClient(client) + + rteam, resp := client.GetTeam(team.Id, "") + CheckNoError(t, resp) + + require.Empty(t, rteam.Email, "should have sanitized email") + require.Empty(t, rteam.InviteId, "should have sanitized inviteid") }) t.Run("team admin", func(t *testing.T) { rteam, resp := th.Client.GetTeam(team.Id, "") CheckNoError(t, resp) + require.NotEmpty(t, rteam.Email, "should not have sanitized email") + require.NotEmpty(t, rteam.InviteId, "should not have sanitized inviteid") }) t.Run("system admin", func(t *testing.T) { rteam, resp := th.SystemAdminClient.GetTeam(team.Id, "") CheckNoError(t, resp) + require.NotEmpty(t, rteam.Email, "should not have sanitized email") + require.NotEmpty(t, rteam.InviteId, "should not have sanitized inviteid") }) } @@ -337,13 +358,17 @@ func TestUpdateTeamSanitization(t *testing.T) { t.Run("team admin", func(t *testing.T) { rteam, resp := th.Client.UpdateTeam(team) CheckNoError(t, resp) + require.NotEmpty(t, rteam.Email, "should not have sanitized email for admin") + require.NotEmpty(t, rteam.InviteId, "should not have sanitized inviteid") }) t.Run("system admin", func(t *testing.T) { rteam, resp := th.SystemAdminClient.UpdateTeam(team) CheckNoError(t, resp) + require.NotEmpty(t, rteam.Email, "should not have sanitized email for admin") + require.NotEmpty(t, rteam.InviteId, "should not have sanitized inviteid") }) } @@ -421,13 +446,17 @@ 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) + require.NotEmpty(t, rteam.Email, "should not have sanitized email for admin") + require.NotEmpty(t, rteam.InviteId, "should not have sanitized inviteid") }) t.Run("system admin", func(t *testing.T) { rteam, resp := th.SystemAdminClient.PatchTeam(team.Id, &model.TeamPatch{}) CheckNoError(t, resp) + require.NotEmpty(t, rteam.Email, "should not have sanitized email for admin") + require.NotEmpty(t, rteam.InviteId, "should not have sanitized inviteid") }) } @@ -702,9 +731,11 @@ func TestGetAllTeamsSanitization(t *testing.T) { if rteam.Id == team.Id { teamFound = true require.NotEmpty(t, rteam.Email, "should not have sanitized email for team admin") + require.NotEmpty(t, rteam.InviteId, "should not have sanitized inviteid") } else if rteam.Id == team2.Id { team2Found = true - require.Empty(t, rteam.Email, "should've sanitized email for non-admin") + require.Empty(t, rteam.Email, "should have sanitized email for team admin") + require.Empty(t, rteam.InviteId, "should have sanitized inviteid") } } @@ -721,6 +752,7 @@ func TestGetAllTeamsSanitization(t *testing.T) { } require.NotEmpty(t, rteam.Email, "should not have sanitized email") + require.NotEmpty(t, rteam.InviteId, "should not have sanitized inviteid") } }) } @@ -791,19 +823,40 @@ func TestGetTeamByNameSanitization(t *testing.T) { rteam, resp := client.GetTeamByName(team.Name, "") CheckNoError(t, resp) + require.Empty(t, rteam.Email, "should've sanitized email") + require.NotEmpty(t, rteam.InviteId, "should have not sanitized inviteid") + }) + + t.Run("team user without invite permissions", func(t *testing.T) { + th.RemovePermissionFromRole(model.PERMISSION_INVITE_USER.Id, model.TEAM_USER_ROLE_ID) + th.LinkUserToTeam(th.BasicUser2, team) + + client := th.CreateClient() + + th.LoginBasic2WithClient(client) + + rteam, resp := client.GetTeam(team.Id, "") + CheckNoError(t, resp) + + require.Empty(t, rteam.Email, "should have sanitized email") + require.Empty(t, rteam.InviteId, "should have sanitized inviteid") }) t.Run("team admin/non-admin", func(t *testing.T) { rteam, resp := th.Client.GetTeamByName(team.Name, "") CheckNoError(t, resp) + require.NotEmpty(t, rteam.Email, "should not have sanitized email") + require.NotEmpty(t, rteam.InviteId, "should have not sanitized inviteid") }) t.Run("system admin", func(t *testing.T) { rteam, resp := th.SystemAdminClient.GetTeamByName(team.Name, "") CheckNoError(t, resp) + require.NotEmpty(t, rteam.Email, "should not have sanitized email") + require.NotEmpty(t, rteam.InviteId, "should have not sanitized inviteid") }) } @@ -899,6 +952,7 @@ func TestSearchAllTeamsSanitization(t *testing.T) { for _, rteam := range rteams { require.Empty(t, rteam.Email, "should've sanitized email") require.Empty(t, rteam.AllowedDomains, "should've sanitized allowed domains") + require.Empty(t, rteam.InviteId, "should have sanitized inviteid") } }) @@ -913,6 +967,7 @@ func TestSearchAllTeamsSanitization(t *testing.T) { for _, rteam := range rteams { require.Empty(t, rteam.Email, "should've sanitized email") require.Empty(t, rteam.AllowedDomains, "should've sanitized allowed domains") + require.NotEmpty(t, rteam.InviteId, "should have not sanitized inviteid") } }) @@ -922,6 +977,7 @@ func TestSearchAllTeamsSanitization(t *testing.T) { for _, rteam := range rteams { if rteam.Id == team.Id || rteam.Id == team2.Id || rteam.Id == th.BasicTeam.Id { require.NotEmpty(t, rteam.Email, "should not have sanitized email") + require.NotEmpty(t, rteam.InviteId, "should have not sanitized inviteid") } } }) @@ -931,6 +987,7 @@ func TestSearchAllTeamsSanitization(t *testing.T) { CheckNoError(t, resp) for _, rteam := range rteams { require.NotEmpty(t, rteam.Email, "should not have sanitized email") + require.NotEmpty(t, rteam.InviteId, "should have not sanitized inviteid") } }) } @@ -1010,6 +1067,27 @@ func TestGetTeamsForUserSanitization(t *testing.T) { } require.Empty(t, rteam.Email, "should've sanitized email") + require.NotEmpty(t, rteam.InviteId, "should have not sanitized inviteid") + } + }) + + t.Run("team user without invite permissions", func(t *testing.T) { + th.LinkUserToTeam(th.BasicUser2, team) + th.LinkUserToTeam(th.BasicUser2, team2) + + client := th.CreateClient() + th.RemovePermissionFromRole(model.PERMISSION_INVITE_USER.Id, model.TEAM_USER_ROLE_ID) + th.LoginBasic2WithClient(client) + + rteams, resp := client.GetTeamsForUser(th.BasicUser2.Id, "") + CheckNoError(t, resp) + for _, rteam := range rteams { + if rteam.Id != team.Id && rteam.Id != team2.Id { + continue + } + + require.Empty(t, rteam.Email, "should have sanitized email") + require.Empty(t, rteam.InviteId, "should have sanitized inviteid") } }) @@ -1022,6 +1100,7 @@ func TestGetTeamsForUserSanitization(t *testing.T) { } require.NotEmpty(t, rteam.Email, "should not have sanitized email") + require.NotEmpty(t, rteam.InviteId, "should have not sanitized inviteid") } }) @@ -1034,6 +1113,7 @@ func TestGetTeamsForUserSanitization(t *testing.T) { } require.NotEmpty(t, rteam.Email, "should not have sanitized email") + require.NotEmpty(t, rteam.InviteId, "should have not sanitized inviteid") } }) } diff --git a/api4/user.go b/api4/user.go index 0892624ed0..d6a8d67b33 100644 --- a/api4/user.go +++ b/api4/user.go @@ -1051,6 +1051,11 @@ func updateUserActive(c *Context, w http.ResponseWriter, r *http.Request) { return } + if active && user.IsGuest() && !*c.App.Config().GuestAccountsSettings.Enable { + c.Err = model.NewAppError("updateUserActive", "api.user.update_active.cannot_enable_guest_when_guest_feature_is_disabled.app_error", nil, "userId="+c.Params.UserId, http.StatusUnauthorized) + return + } + if _, err = c.App.UpdateActive(user, active); err != nil { c.Err = err } diff --git a/api4/user_test.go b/api4/user_test.go index 003573fd09..5f31f43992 100644 --- a/api4/user_test.go +++ b/api4/user_test.go @@ -7,13 +7,11 @@ import ( "fmt" "net/http" "regexp" - "strconv" "strings" "testing" "time" "github.com/dgryski/dgoogauth" - "github.com/mattermost/mattermost-server/app" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/services/mailservice" @@ -34,14 +32,8 @@ func TestCreateUser(t *testing.T) { _, _ = th.Client.Login(user.Email, user.Password) - if ruser.Nickname != user.Nickname { - t.Fatal("nickname didn't match") - } - - if ruser.Roles != model.SYSTEM_USER_ROLE_ID { - t.Log(ruser.Roles) - t.Fatal("did not clear roles") - } + require.Equal(t, user.Nickname, ruser.Nickname, "nickname didn't match") + require.Equal(t, model.SYSTEM_USER_ROLE_ID, ruser.Roles, "did not clear roles") CheckUserSanitization(t, ruser) @@ -169,22 +161,16 @@ func TestCreateUserWithToken(t *testing.T) { CheckCreatedStatus(t, resp) th.Client.Login(user.Email, user.Password) - if ruser.Nickname != user.Nickname { - t.Fatal("nickname didn't match") - } - if ruser.Roles != model.SYSTEM_USER_ROLE_ID { - t.Log(ruser.Roles) - t.Fatal("did not clear roles") - } + require.Equal(t, user.Nickname, ruser.Nickname) + require.Equal(t, model.SYSTEM_USER_ROLE_ID, ruser.Roles, "should clear roles") CheckUserSanitization(t, ruser) _, err := th.App.Srv.Store.Token().GetByToken(token.Token) require.NotNil(t, err, "The token must be deleted after being used") - if teams, err := th.App.GetTeamsForUser(ruser.Id); err != nil || len(teams) == 0 { - t.Fatal("The user must have teams") - } else if teams[0].Id != th.BasicTeam.Id { - t.Fatal("The user joined team must be the team provided.") - } + teams, err := th.App.GetTeamsForUser(ruser.Id) + require.Nil(t, err) + require.NotEmpty(t, teams, "The user must have teams") + require.Equal(t, th.BasicTeam.Id, teams[0].Id, "The user joined team must be the team provided.") }) t.Run("NoToken", func(t *testing.T) { @@ -271,13 +257,8 @@ func TestCreateUserWithToken(t *testing.T) { CheckCreatedStatus(t, resp) th.Client.Login(user.Email, user.Password) - if ruser.Nickname != user.Nickname { - t.Fatal("nickname didn't match") - } - if ruser.Roles != model.SYSTEM_USER_ROLE_ID { - t.Log(ruser.Roles) - t.Fatal("did not clear roles") - } + require.Equal(t, user.Nickname, ruser.Nickname) + require.Equal(t, model.SYSTEM_USER_ROLE_ID, ruser.Roles, "should clear roles") CheckUserSanitization(t, ruser) _, err := th.App.Srv.Store.Token().GetByToken(token.Token) require.NotNil(t, err, "The token must be deleted after be used") @@ -298,13 +279,8 @@ func TestCreateUserWithInviteId(t *testing.T) { CheckCreatedStatus(t, resp) th.Client.Login(user.Email, user.Password) - if ruser.Nickname != user.Nickname { - t.Fatal("nickname didn't match") - } - if ruser.Roles != model.SYSTEM_USER_ROLE_ID { - t.Log(ruser.Roles) - t.Fatal("did not clear roles") - } + require.Equal(t, user.Nickname, ruser.Nickname) + require.Equal(t, model.SYSTEM_USER_ROLE_ID, ruser.Roles, "should clear roles") CheckUserSanitization(t, ruser) }) @@ -394,13 +370,8 @@ func TestCreateUserWithInviteId(t *testing.T) { CheckCreatedStatus(t, resp) th.Client.Login(user.Email, user.Password) - if ruser.Nickname != user.Nickname { - t.Fatal("nickname didn't match") - } - if ruser.Roles != model.SYSTEM_USER_ROLE_ID { - t.Log(ruser.Roles) - t.Fatal("did not clear roles") - } + require.Equal(t, user.Nickname, ruser.Nickname) + require.Equal(t, model.SYSTEM_USER_ROLE_ID, ruser.Roles, "should clear roles") CheckUserSanitization(t, ruser) }) } @@ -412,9 +383,7 @@ func TestGetMe(t *testing.T) { ruser, resp := th.Client.GetMe("") CheckNoError(t, resp) - if ruser.Id != th.BasicUser.Id { - t.Fatal("wrong user") - } + require.Equal(t, th.BasicUser.Id, ruser.Id) th.Client.Logout() _, resp = th.Client.GetMe("") @@ -434,9 +403,7 @@ func TestGetUser(t *testing.T) { CheckNoError(t, resp) CheckUserSanitization(t, ruser) - if ruser.Email != user.Email { - t.Fatal("emails did not match") - } + require.Equal(t, user.Email, ruser.Email) assert.NotNil(t, ruser.Props) assert.Equal(t, ruser.Props["testpropkey"], "testpropvalue") @@ -458,15 +425,9 @@ func TestGetUser(t *testing.T) { ruser, resp = th.Client.GetUser(user.Id, "") CheckNoError(t, resp) - if ruser.Email != "" { - t.Fatal("email should be blank") - } - if ruser.FirstName != "" { - t.Fatal("first name should be blank") - } - if ruser.LastName != "" { - t.Fatal("last name should be blank") - } + require.Empty(t, ruser.Email, "email should be blank") + require.Empty(t, ruser.FirstName, "first name should be blank") + require.Empty(t, ruser.LastName, "last name should be blank") th.Client.Logout() _, resp = th.Client.GetUser(user.Id, "") @@ -474,15 +435,9 @@ func TestGetUser(t *testing.T) { // System admins should ignore privacy settings ruser, _ = th.SystemAdminClient.GetUser(user.Id, resp.Etag) - if ruser.Email == "" { - t.Fatal("email should not be blank") - } - if ruser.FirstName == "" { - t.Fatal("first name should not be blank") - } - if ruser.LastName == "" { - t.Fatal("last name should not be blank") - } + require.NotEmpty(t, ruser.Email, "email should not be blank") + require.NotEmpty(t, ruser.FirstName, "first name should not be blank") + require.NotEmpty(t, ruser.LastName, "last name should not be blank") } func TestGetUserWithAcceptedTermsOfServiceForOtherUser(t *testing.T) { @@ -499,9 +454,7 @@ func TestGetUserWithAcceptedTermsOfServiceForOtherUser(t *testing.T) { CheckNoError(t, resp) CheckUserSanitization(t, ruser) - if ruser.Email != user.Email { - t.Fatal("emails did not match") - } + require.Equal(t, user.Email, ruser.Email) assert.Empty(t, ruser.TermsOfServiceId) @@ -511,9 +464,7 @@ func TestGetUserWithAcceptedTermsOfServiceForOtherUser(t *testing.T) { CheckNoError(t, resp) CheckUserSanitization(t, ruser) - if ruser.Email != user.Email { - t.Fatal("emails did not match") - } + require.Equal(t, user.Email, ruser.Email) // user TOS data cannot be fetched for other users by non-admin users assert.Empty(t, ruser.TermsOfServiceId) @@ -531,9 +482,7 @@ func TestGetUserWithAcceptedTermsOfService(t *testing.T) { CheckNoError(t, resp) CheckUserSanitization(t, ruser) - if ruser.Email != user.Email { - t.Fatal("emails did not match") - } + require.Equal(t, user.Email, ruser.Email) assert.Empty(t, ruser.TermsOfServiceId) @@ -543,9 +492,7 @@ func TestGetUserWithAcceptedTermsOfService(t *testing.T) { CheckNoError(t, resp) CheckUserSanitization(t, ruser) - if ruser.Email != user.Email { - t.Fatal("emails did not match") - } + require.Equal(t, user.Email, ruser.Email) // a user can view their own TOS details assert.Equal(t, tos.Id, ruser.TermsOfServiceId) @@ -564,9 +511,7 @@ func TestGetUserWithAcceptedTermsOfServiceWithAdminUser(t *testing.T) { CheckNoError(t, resp) CheckUserSanitization(t, ruser) - if ruser.Email != user.Email { - t.Fatal("emails did not match") - } + require.Equal(t, user.Email, ruser.Email) assert.Empty(t, ruser.TermsOfServiceId) @@ -576,9 +521,7 @@ func TestGetUserWithAcceptedTermsOfServiceWithAdminUser(t *testing.T) { CheckNoError(t, resp) CheckUserSanitization(t, ruser) - if ruser.Email != user.Email { - t.Fatal("emails did not match") - } + require.Equal(t, user.Email, ruser.Email) // admin can view anyone's TOS details assert.Equal(t, tos.Id, ruser.TermsOfServiceId) @@ -623,9 +566,7 @@ func TestGetUserByUsername(t *testing.T) { CheckNoError(t, resp) CheckUserSanitization(t, ruser) - if ruser.Email != user.Email { - t.Fatal("emails did not match") - } + require.Equal(t, user.Email, ruser.Email) ruser, resp = th.Client.GetUserByUsername(user.Username, resp.Etag) CheckEtag(t, ruser, resp) @@ -640,21 +581,13 @@ func TestGetUserByUsername(t *testing.T) { ruser, resp = th.Client.GetUserByUsername(th.BasicUser2.Username, "") CheckNoError(t, resp) - if ruser.Email != "" { - t.Fatal("email should be blank") - } - if ruser.FirstName != "" { - t.Fatal("first name should be blank") - } - if ruser.LastName != "" { - t.Fatal("last name should be blank") - } + require.Empty(t, ruser.Email, "email should be blank") + require.Empty(t, ruser.FirstName, "first name should be blank") + require.Empty(t, ruser.LastName, "last name should be blank") ruser, resp = th.Client.GetUserByUsername(th.BasicUser.Username, "") CheckNoError(t, resp) - if len(ruser.NotifyProps) == 0 { - t.Fatal("notify props should be sent") - } + require.NotEmpty(t, ruser.NotifyProps, "notify props should be sent") th.Client.Logout() _, resp = th.Client.GetUserByUsername(user.Username, "") @@ -662,15 +595,9 @@ func TestGetUserByUsername(t *testing.T) { // System admins should ignore privacy settings ruser, _ = th.SystemAdminClient.GetUserByUsername(user.Username, resp.Etag) - if ruser.Email == "" { - t.Fatal("email should not be blank") - } - if ruser.FirstName == "" { - t.Fatal("first name should not be blank") - } - if ruser.LastName == "" { - t.Fatal("last name should not be blank") - } + require.NotEmpty(t, ruser.Email, "email should not be blank") + require.NotEmpty(t, ruser.FirstName, "first name should not be blank") + require.NotEmpty(t, ruser.LastName, "last name should not be blank") } func TestGetUserByUsernameWithAcceptedTermsOfService(t *testing.T) { @@ -683,9 +610,7 @@ func TestGetUserByUsernameWithAcceptedTermsOfService(t *testing.T) { CheckNoError(t, resp) CheckUserSanitization(t, ruser) - if ruser.Email != user.Email { - t.Fatal("emails did not match") - } + require.Equal(t, user.Email, ruser.Email) tos, _ := th.App.CreateTermsOfService("Dummy TOS", user.Id) th.App.SaveUserTermsOfService(ruser.Id, tos.Id, true) @@ -694,13 +619,9 @@ func TestGetUserByUsernameWithAcceptedTermsOfService(t *testing.T) { CheckNoError(t, resp) CheckUserSanitization(t, ruser) - if ruser.Email != user.Email { - t.Fatal("emails did not match") - } + require.Equal(t, user.Email, ruser.Email) - if ruser.TermsOfServiceId != tos.Id { - t.Fatal("Terms of service ID didn't match") - } + require.Equal(t, tos.Id, ruser.TermsOfServiceId, "Terms of service ID should match") } func TestGetUserByEmail(t *testing.T) { @@ -719,9 +640,7 @@ func TestGetUserByEmail(t *testing.T) { CheckNoError(t, resp) CheckUserSanitization(t, ruser) - if ruser.Email != user.Email { - t.Fatal("emails did not match") - } + require.Equal(t, user.Email, ruser.Email) }) t.Run("should return not modified when provided with a matching etag", func(t *testing.T) { @@ -829,14 +748,10 @@ func TestSearchUsers(t *testing.T) { users, resp := th.Client.SearchUsers(search) CheckNoError(t, resp) - if !findUserInList(th.BasicUser.Id, users) { - t.Fatal("should have found user") - } + require.True(t, findUserInList(th.BasicUser.Id, users), "should have found user") _, err := th.App.UpdateActive(th.BasicUser2, false) - if err != nil { - t.Fatal(err) - } + require.Nil(t, err) search.Term = th.BasicUser2.Username search.AllowInactive = false @@ -844,18 +759,14 @@ func TestSearchUsers(t *testing.T) { users, resp = th.Client.SearchUsers(search) CheckNoError(t, resp) - if findUserInList(th.BasicUser2.Id, users) { - t.Fatal("should not have found user") - } + require.False(t, findUserInList(th.BasicUser2.Id, users), "should not have found user") search.AllowInactive = true users, resp = th.Client.SearchUsers(search) CheckNoError(t, resp) - if !findUserInList(th.BasicUser2.Id, users) { - t.Fatal("should have found user") - } + require.True(t, findUserInList(th.BasicUser2.Id, users), "should have found user") search.Term = th.BasicUser.Username search.AllowInactive = false @@ -864,18 +775,14 @@ func TestSearchUsers(t *testing.T) { users, resp = th.Client.SearchUsers(search) CheckNoError(t, resp) - if !findUserInList(th.BasicUser.Id, users) { - t.Fatal("should have found user") - } + require.True(t, findUserInList(th.BasicUser.Id, users), "should have found user") search.NotInChannelId = th.BasicChannel.Id users, resp = th.Client.SearchUsers(search) CheckNoError(t, resp) - if findUserInList(th.BasicUser.Id, users) { - t.Fatal("should not have found user") - } + require.False(t, findUserInList(th.BasicUser.Id, users), "should not have found user") search.TeamId = "" search.NotInChannelId = "" @@ -884,9 +791,7 @@ func TestSearchUsers(t *testing.T) { users, resp = th.Client.SearchUsers(search) CheckNoError(t, resp) - if !findUserInList(th.BasicUser.Id, users) { - t.Fatal("should have found user") - } + require.True(t, findUserInList(th.BasicUser.Id, users), "should have found user") search.InChannelId = "" search.NotInChannelId = th.BasicChannel.Id @@ -917,9 +822,7 @@ func TestSearchUsers(t *testing.T) { users, resp = th.Client.SearchUsers(search) CheckNoError(t, resp) - if findUserInList(th.BasicUser.Id, users) { - t.Fatal("should not have found user") - } + require.False(t, findUserInList(th.BasicUser.Id, users), "should not have found user") oddUser := th.CreateUser() search.Term = oddUser.Username @@ -927,9 +830,7 @@ func TestSearchUsers(t *testing.T) { users, resp = th.Client.SearchUsers(search) CheckNoError(t, resp) - if !findUserInList(oddUser.Id, users) { - t.Fatal("should have found user") - } + require.True(t, findUserInList(oddUser.Id, users), "should have found user") _, resp = th.SystemAdminClient.AddTeamMember(th.BasicTeam.Id, oddUser.Id) CheckNoError(t, resp) @@ -937,9 +838,7 @@ func TestSearchUsers(t *testing.T) { users, resp = th.Client.SearchUsers(search) CheckNoError(t, resp) - if findUserInList(oddUser.Id, users) { - t.Fatal("should not have found user") - } + require.False(t, findUserInList(oddUser.Id, users), "should not have found user") search.NotInTeamId = model.NewId() _, resp = th.Client.SearchUsers(search) @@ -951,9 +850,7 @@ func TestSearchUsers(t *testing.T) { th.App.UpdateConfig(func(cfg *model.Config) { *cfg.PrivacySettings.ShowFullName = false }) _, err = th.App.UpdateActive(th.BasicUser2, true) - if err != nil { - t.Fatal(err) - } + require.Nil(t, err) search.InChannelId = "" search.NotInTeamId = "" @@ -961,25 +858,19 @@ func TestSearchUsers(t *testing.T) { users, resp = th.Client.SearchUsers(search) CheckNoError(t, resp) - if findUserInList(th.BasicUser2.Id, users) { - t.Fatal("should not have found user") - } + require.False(t, findUserInList(th.BasicUser2.Id, users), "should not have found user") search.Term = th.BasicUser2.FirstName users, resp = th.Client.SearchUsers(search) CheckNoError(t, resp) - if findUserInList(th.BasicUser2.Id, users) { - t.Fatal("should not have found user") - } + require.False(t, findUserInList(th.BasicUser2.Id, users), "should not have found user") search.Term = th.BasicUser2.LastName users, resp = th.Client.SearchUsers(search) CheckNoError(t, resp) - if findUserInList(th.BasicUser2.Id, users) { - t.Fatal("should not have found user") - } + require.False(t, findUserInList(th.BasicUser2.Id, users), "should not have found user") search.Term = th.BasicUser.FirstName search.InChannelId = th.BasicChannel.Id @@ -988,9 +879,7 @@ func TestSearchUsers(t *testing.T) { users, resp = th.SystemAdminClient.SearchUsers(search) CheckNoError(t, resp) - if !findUserInList(th.BasicUser.Id, users) { - t.Fatal("should have found user") - } + require.True(t, findUserInList(th.BasicUser.Id, users), "should have found user") } func findUserInList(id string, users []*model.User) bool { @@ -1265,14 +1154,10 @@ func TestGetProfileImage(t *testing.T) { data, resp := th.Client.GetProfileImage(user.Id, "") CheckNoError(t, resp) - if len(data) == 0 { - t.Fatal("Should not be empty") - } + require.NotEmpty(t, data, "should not be empty") _, resp = th.Client.GetProfileImage(user.Id, resp.Etag) - if resp.StatusCode == http.StatusNotModified { - t.Fatal("Shouldn't have hit etag") - } + require.NotEqual(t, http.StatusNotModified, resp.StatusCode, "should not hit etag") _, resp = th.Client.GetProfileImage("junk", "") CheckBadRequestStatus(t, resp) @@ -1288,9 +1173,8 @@ func TestGetProfileImage(t *testing.T) { CheckNoError(t, resp) info := &model.FileInfo{Path: "/users/" + user.Id + "/profile.png"} - if err := th.cleanupTestFile(info); err != nil { - t.Fatal(err) - } + err := th.cleanupTestFile(info) + require.NoError(t, err) } func TestGetUsersByIds(t *testing.T) { @@ -1316,18 +1200,15 @@ func TestGetUsersByIds(t *testing.T) { users, resp := th.Client.GetUsersByIds([]string{"junk"}) CheckNoError(t, resp) - if len(users) > 0 { - t.Fatal("no users should be returned") - } + require.Empty(t, users, "no users should be returned") }) t.Run("should still return users for valid IDs when invalid IDs are specified", func(t *testing.T) { users, resp := th.Client.GetUsersByIds([]string{"junk", th.BasicUser.Id}) CheckNoError(t, resp) - if len(users) != 1 { - t.Fatal("1 user should be returned") - } + + require.Len(t, users, 1, "1 user should be returned") }) t.Run("should return error when not logged in", func(t *testing.T) { @@ -1402,9 +1283,7 @@ func TestGetUsersByUsernames(t *testing.T) { users, resp := th.Client.GetUsersByUsernames([]string{th.BasicUser.Username}) CheckNoError(t, resp) - if users[0].Id != th.BasicUser.Id { - t.Fatal("returned wrong user") - } + require.Equal(t, th.BasicUser.Id, users[0].Id) CheckUserSanitization(t, users[0]) _, resp = th.Client.GetUsersByIds([]string{}) @@ -1412,15 +1291,11 @@ func TestGetUsersByUsernames(t *testing.T) { users, resp = th.Client.GetUsersByUsernames([]string{"junk"}) CheckNoError(t, resp) - if len(users) > 0 { - t.Fatal("no users should be returned") - } + require.Empty(t, users, "no users should be returned") users, resp = th.Client.GetUsersByUsernames([]string{"junk", th.BasicUser.Username}) CheckNoError(t, resp) - if len(users) != 1 { - t.Fatal("1 user should be returned") - } + require.Len(t, users, 1, "1 user should be returned") th.Client.Logout() _, resp = th.Client.GetUsersByUsernames([]string{th.BasicUser.Username}) @@ -1439,9 +1314,7 @@ func TestGetTotalUsersStat(t *testing.T) { rstats, resp := th.Client.GetTotalUsersStats("") CheckNoError(t, resp) - if rstats.TotalUsersCount != total { - t.Fatal("wrong count") - } + require.Equal(t, total, rstats.TotalUsersCount) } func TestUpdateUser(t *testing.T) { @@ -1459,15 +1332,9 @@ func TestUpdateUser(t *testing.T) { CheckNoError(t, resp) CheckUserSanitization(t, ruser) - if ruser.Nickname != "Joram Wilander" { - t.Fatal("Nickname did not update properly") - } - if ruser.Roles != model.SYSTEM_USER_ROLE_ID { - t.Fatal("Roles should not have updated") - } - if ruser.LastPasswordUpdate == 123 { - t.Fatal("LastPasswordUpdate should not have updated") - } + require.Equal(t, "Joram Wilander", ruser.Nickname, "Nickname should update properly") + require.Equal(t, model.SYSTEM_USER_ROLE_ID, ruser.Roles, "Roles should not update") + require.NotEqual(t, 123, ruser.LastPasswordUpdate, "LastPasswordUpdate should not update") ruser.Email = th.GenerateTestEmail() _, resp = th.Client.UpdateUser(ruser) @@ -1486,15 +1353,9 @@ func TestUpdateUser(t *testing.T) { _, resp = th.Client.UpdateUser(ruser) CheckForbiddenStatus(t, resp) - if r, err := th.Client.DoApiPut("/users/"+ruser.Id, "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 := th.Client.DoApiPut("/users/"+ruser.Id, "garbage") + require.Error(t, err) + require.Equal(t, http.StatusBadRequest, r.StatusCode) session, _ := th.App.GetSession(th.Client.AuthToken) session.IsOAuth = true @@ -1541,50 +1402,26 @@ func TestPatchUser(t *testing.T) { CheckNoError(t, resp) CheckUserSanitization(t, ruser) - if ruser.Nickname != "Joram Wilander" { - t.Fatal("Nickname did not update properly") - } - if ruser.FirstName != "Joram" { - t.Fatal("FirstName did not update properly") - } - if ruser.LastName != "Wilander" { - t.Fatal("LastName did not update properly") - } - if ruser.Position != "" { - t.Fatal("Position did not update properly") - } - if ruser.Username != user.Username { - t.Fatal("Username should not have updated") - } - if ruser.Password != "" { - t.Fatal("Password should not be returned") - } - if ruser.NotifyProps["comment"] != "somethingrandom" { - t.Fatal("NotifyProps did not update properly") - } - if ruser.Timezone["useAutomaticTimezone"] != "true" { - t.Fatal("useAutomaticTimezone did not update properly") - } - if ruser.Timezone["automaticTimezone"] != "America/New_York" { - t.Fatal("automaticTimezone did not update properly") - } - if ruser.Timezone["manualTimezone"] != "" { - t.Fatal("manualTimezone did not update properly") - } + require.Equal(t, "Joram Wilander", ruser.Nickname, "Nickname should update properly") + require.Equal(t, "Joram", ruser.FirstName, "FirstName should update properly") + require.Equal(t, "Wilander", ruser.LastName, "LastName should update properly") + require.Empty(t, ruser.Position, "Position should update properly") + require.Equal(t, user.Username, ruser.Username, "Username should not update") + require.Empty(t, ruser.Password, "Password should not be returned") + require.Equal(t, "somethingrandom", ruser.NotifyProps["comment"], "NotifyProps should update properly") + require.Equal(t, "true", ruser.Timezone["useAutomaticTimezone"], "useAutomaticTimezone should update properly") + require.Equal(t, "America/New_York", ruser.Timezone["automaticTimezone"], "automaticTimezone should update properly") + require.Empty(t, ruser.Timezone["manualTimezone"], "manualTimezone should update properly") err := th.App.CheckPasswordAndAllCriteria(ruser, *patch.Password, "") assert.Error(t, err, "Password should not match") currentPassword := user.Password user, err = th.App.GetUser(ruser.Id) - if err != nil { - t.Fatal("User Get shouldn't error") - } + require.Nil(t, err) err = th.App.CheckPasswordAndAllCriteria(user, currentPassword, "") - if err != nil { - t.Fatal("Password should still match") - } + require.Nil(t, err, "Password should still match") patch = &model.UserPatch{} patch.Email = model.NewString(th.GenerateTestEmail()) @@ -1596,9 +1433,7 @@ func TestPatchUser(t *testing.T) { ruser, resp = th.Client.PatchUser(user.Id, patch) CheckNoError(t, resp) - if ruser.Email != *patch.Email { - t.Fatal("Email did not update properly") - } + require.Equal(t, *patch.Email, ruser.Email, "Email should update properly") patch.Username = model.NewString(th.BasicUser2.Username) _, resp = th.Client.PatchUser(user.Id, patch) @@ -1613,15 +1448,9 @@ func TestPatchUser(t *testing.T) { _, resp = th.Client.PatchUser(model.NewId(), patch) CheckForbiddenStatus(t, resp) - if r, err := th.Client.DoApiPut("/users/"+user.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 := th.Client.DoApiPut("/users/"+user.Id+"/patch", "garbage") + require.Error(t, err) + require.Equal(t, http.StatusBadRequest, r.StatusCode) session, _ := th.App.GetSession(th.Client.AuthToken) session.IsOAuth = true @@ -1661,9 +1490,8 @@ func TestUpdateUserAuth(t *testing.T) { userAuth.Password = user.Password // Regular user can not use endpoint - if _, err := th.SystemAdminClient.UpdateUserAuth(user.Id, userAuth); err == nil { - t.Fatal("Shouldn't have permissions. Only Admins") - } + _, respErr := th.SystemAdminClient.UpdateUserAuth(user.Id, userAuth) + require.NotNil(t, respErr, "Shouldn't have permissions. Only Admins") userAuth.AuthData = model.NewString("test@test.com") userAuth.AuthService = model.USER_AUTH_SERVICE_SAML @@ -1672,23 +1500,16 @@ func TestUpdateUserAuth(t *testing.T) { CheckNoError(t, resp) // AuthData and AuthService are set, password is set to empty - if *ruser.AuthData != *userAuth.AuthData { - t.Fatal("Should have set the correct AuthData") - } - if ruser.AuthService != model.USER_AUTH_SERVICE_SAML { - t.Fatal("Should have set the correct AuthService") - } - if ruser.Password != "" { - t.Fatal("Password should be empty") - } + require.Equal(t, *userAuth.AuthData, *ruser.AuthData) + require.Equal(t, model.USER_AUTH_SERVICE_SAML, ruser.AuthService) + require.Empty(t, ruser.Password) // When AuthData or AuthService are empty, password must be valid userAuth.AuthData = user.AuthData userAuth.AuthService = "" userAuth.Password = "1" - if _, err := th.SystemAdminClient.UpdateUserAuth(user.Id, userAuth); err == nil { - t.Fatal("Should have errored - user password not valid") - } + _, respErr = th.SystemAdminClient.UpdateUserAuth(user.Id, userAuth) + require.NotNil(t, respErr) // Regular user can not use endpoint user2 := th.CreateUser() @@ -1701,9 +1522,8 @@ func TestUpdateUserAuth(t *testing.T) { userAuth.AuthData = user.AuthData userAuth.AuthService = user.AuthService userAuth.Password = user.Password - if _, err := th.SystemAdminClient.UpdateUserAuth(user.Id, userAuth); err == nil { - t.Fatal("Should have errored") - } + _, respErr = th.SystemAdminClient.UpdateUserAuth(user.Id, userAuth) + require.NotNil(t, respErr, "Should have errored") } func TestDeleteUser(t *testing.T) { @@ -1778,9 +1598,8 @@ func assertExpectedWebsocketEvent(t *testing.T, client *model.WebSocketClient, e for { select { case resp, ok := <-client.EventChannel: - if !ok { - t.Fatalf("channel closed before receiving expected event %s", model.WEBSOCKET_EVENT_USER_UPDATED) - } else if resp.Event == model.WEBSOCKET_EVENT_USER_UPDATED { + require.Truef(t, ok, "channel closed before receiving expected event %s", model.WEBSOCKET_EVENT_USER_UPDATED) + if resp.Event == model.WEBSOCKET_EVENT_USER_UPDATED { test(resp) return } @@ -1792,13 +1611,11 @@ func assertExpectedWebsocketEvent(t *testing.T, client *model.WebSocketClient, e func assertWebsocketEventUserUpdatedWithEmail(t *testing.T, client *model.WebSocketClient, email string) { assertExpectedWebsocketEvent(t, client, model.WEBSOCKET_EVENT_USER_UPDATED, func(event *model.WebSocketEvent) { - if eventUser, ok := event.Data["user"].(map[string]interface{}); !ok { - t.Fatalf("expected user") - } else if userEmail, ok := eventUser["email"].(string); !ok { - t.Fatalf("expected email %s, but got nil", email) - } else { - assert.Equal(t, email, userEmail) - } + eventUser, ok := event.Data["user"].(map[string]interface{}) + require.True(t, ok, "expected user") + userEmail, ok := eventUser["email"].(string) + require.Truef(t, ok, "expected email %s, but got nil", email) + assert.Equal(t, email, userEmail) }) } @@ -1813,25 +1630,19 @@ func TestUpdateUserActive(t *testing.T) { pass, resp := th.Client.UpdateUserActive(user.Id, false) CheckNoError(t, resp) - if !pass { - t.Fatal("should have returned true") - } + require.True(t, pass) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.TeamSettings.EnableUserDeactivation = false }) pass, resp = th.Client.UpdateUserActive(user.Id, false) CheckUnauthorizedStatus(t, resp) - if pass { - t.Fatal("should have returned false") - } + require.False(t, pass) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.TeamSettings.EnableUserDeactivation = true }) pass, resp = th.Client.UpdateUserActive(user.Id, false) CheckUnauthorizedStatus(t, resp) - if pass { - t.Fatal("should have returned false") - } + require.False(t, pass) th.LoginBasic2() @@ -1878,9 +1689,8 @@ func TestUpdateUserActive(t *testing.T) { webSocketClient.Listen() time.Sleep(300 * time.Millisecond) - if resp := <-webSocketClient.ResponseChannel; resp.Status != model.STATUS_OK { - t.Fatal("should have responded OK to authentication challenge") - } + resp := <-webSocketClient.ResponseChannel + require.Equal(t, model.STATUS_OK, resp.Status) adminWebSocketClient, err := th.CreateWebSocketSystemAdminClient() assert.Nil(t, err) @@ -1889,26 +1699,68 @@ func TestUpdateUserActive(t *testing.T) { adminWebSocketClient.Listen() time.Sleep(300 * time.Millisecond) - if resp := <-adminWebSocketClient.ResponseChannel; resp.Status != model.STATUS_OK { - t.Fatal("should have responded OK to authentication challenge") - } + resp = <-adminWebSocketClient.ResponseChannel + require.Equal(t, model.STATUS_OK, resp.Status) // Verify that both admins and regular users see the email when privacy settings allow same. th.App.UpdateConfig(func(cfg *model.Config) { *cfg.PrivacySettings.ShowEmailAddress = true }) - _, resp := th.SystemAdminClient.UpdateUserActive(user.Id, false) - CheckNoError(t, resp) + _, respErr := th.SystemAdminClient.UpdateUserActive(user.Id, false) + CheckNoError(t, respErr) assertWebsocketEventUserUpdatedWithEmail(t, webSocketClient, user.Email) assertWebsocketEventUserUpdatedWithEmail(t, adminWebSocketClient, user.Email) // Verify that only admins see the email when privacy settings hide emails. th.App.UpdateConfig(func(cfg *model.Config) { *cfg.PrivacySettings.ShowEmailAddress = false }) - _, resp = th.SystemAdminClient.UpdateUserActive(user.Id, true) - CheckNoError(t, resp) + _, respErr = th.SystemAdminClient.UpdateUserActive(user.Id, true) + CheckNoError(t, respErr) assertWebsocketEventUserUpdatedWithEmail(t, webSocketClient, "") assertWebsocketEventUserUpdatedWithEmail(t, adminWebSocketClient, user.Email) }) + + t.Run("activate guest should fail when guests feature is disable", func(t *testing.T) { + th := Setup().InitBasic() + defer th.TearDown() + + id := model.NewId() + guest := &model.User{ + Email: "success+" + id + "@simulator.amazonses.com", + Username: "un_" + id, + Nickname: "nn_" + id, + Password: "Password1", + EmailVerified: true, + } + user, err := th.App.CreateGuest(guest) + require.Nil(t, err) + th.App.UpdateActive(user, false) + + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.GuestAccountsSettings.Enable = false }) + defer th.App.UpdateConfig(func(cfg *model.Config) { *cfg.GuestAccountsSettings.Enable = true }) + _, resp := th.SystemAdminClient.UpdateUserActive(user.Id, true) + CheckUnauthorizedStatus(t, resp) + }) + + t.Run("activate guest should work when guests feature is enabled", func(t *testing.T) { + th := Setup().InitBasic() + defer th.TearDown() + + id := model.NewId() + guest := &model.User{ + Email: "success+" + id + "@simulator.amazonses.com", + Username: "un_" + id, + Nickname: "nn_" + id, + Password: "Password1", + EmailVerified: true, + } + user, err := th.App.CreateGuest(guest) + require.Nil(t, err) + th.App.UpdateActive(user, false) + + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.GuestAccountsSettings.Enable = true }) + _, resp := th.SystemAdminClient.UpdateUserActive(user.Id, true) + CheckNoError(t, resp) + }) } func TestGetUsers(t *testing.T) { @@ -1926,26 +1778,19 @@ func TestGetUsers(t *testing.T) { rusers, resp = th.Client.GetUsers(0, 1, "") CheckNoError(t, resp) - if len(rusers) != 1 { - t.Fatal("should be 1 per page") - } + require.Len(t, rusers, 1, "should be 1 per page") rusers, resp = th.Client.GetUsers(1, 1, "") CheckNoError(t, resp) - if len(rusers) != 1 { - t.Fatal("should be 1 per page") - } + require.Len(t, rusers, 1, "should be 1 per page") rusers, resp = th.Client.GetUsers(10000, 100, "") CheckNoError(t, resp) - if len(rusers) != 0 { - t.Fatal("should be no users") - } + require.Empty(t, rusers, "should be no users") // Check default params for page and per_page - if _, err := th.Client.DoApiGet("/users", ""); err != nil { - t.Fatal("should not have errored") - } + _, err := th.Client.DoApiGet("/users", "") + require.Nil(t, err) th.Client.Logout() _, resp = th.Client.GetUsers(0, 60, "") @@ -1962,18 +1807,14 @@ func TestGetNewUsersInTeam(t *testing.T) { lastCreateAt := model.GetMillis() for _, u := range rusers { - if u.CreateAt > lastCreateAt { - t.Fatal("bad sorting") - } + require.LessOrEqual(t, u.CreateAt, lastCreateAt, "right sorting") lastCreateAt = u.CreateAt CheckUserSanitization(t, u) } rusers, resp = th.Client.GetNewUsersInTeam(teamId, 1, 1, "") CheckNoError(t, resp) - if len(rusers) != 1 { - t.Fatal("should be 1 per page") - } + require.Len(t, rusers, 1, "should be 1 per page") th.Client.Logout() _, resp = th.Client.GetNewUsersInTeam(teamId, 1, 1, "") @@ -1991,17 +1832,13 @@ func TestGetRecentlyActiveUsersInTeam(t *testing.T) { CheckNoError(t, resp) for _, u := range rusers { - if u.LastActivityAt == 0 { - t.Fatal("did not return last activity at") - } + require.NotZero(t, u.LastActivityAt, "should return last activity at") CheckUserSanitization(t, u) } rusers, resp = th.Client.GetRecentlyActiveUsersInTeam(teamId, 0, 1, "") CheckNoError(t, resp) - if len(rusers) != 1 { - t.Fatal("should be 1 per page") - } + require.Len(t, rusers, 1, "should be 1 per page") th.Client.Logout() _, resp = th.Client.GetRecentlyActiveUsersInTeam(teamId, 0, 1, "") @@ -2012,9 +1849,8 @@ func TestGetUsersWithoutTeam(t *testing.T) { th := Setup().InitBasic() defer th.TearDown() - if _, resp := th.Client.GetUsersWithoutTeam(0, 100, ""); resp.Error == nil { - t.Fatal("should prevent non-admin user from getting users without a team") - } + _, resp := th.Client.GetUsersWithoutTeam(0, 100, "") + require.Error(t, resp.Error, "should prevent non-admin user from getting users without a team") // These usernames need to appear in the first 100 users for this to work @@ -2049,11 +1885,8 @@ func TestGetUsersWithoutTeam(t *testing.T) { } } - if found1 { - t.Fatal("shouldn't have returned user that has a team") - } else if !found2 { - t.Fatal("should've returned user that has no teams") - } + require.False(t, found1, "should not return user that as a team") + require.True(t, found2, "should return user that has no teams") } func TestGetUsersInTeam(t *testing.T) { @@ -2072,21 +1905,15 @@ func TestGetUsersInTeam(t *testing.T) { rusers, resp = th.Client.GetUsersInTeam(teamId, 0, 1, "") CheckNoError(t, resp) - if len(rusers) != 1 { - t.Fatal("should be 1 per page") - } + require.Len(t, rusers, 1, "should be 1 per page") rusers, resp = th.Client.GetUsersInTeam(teamId, 1, 1, "") CheckNoError(t, resp) - if len(rusers) != 1 { - t.Fatal("should be 1 per page") - } + require.Len(t, rusers, 1, "should be 1 per page") rusers, resp = th.Client.GetUsersInTeam(teamId, 10000, 100, "") CheckNoError(t, resp) - if len(rusers) != 0 { - t.Fatal("should be no users") - } + require.Empty(t, rusers, "should be no users") th.Client.Logout() _, resp = th.Client.GetUsersInTeam(teamId, 0, 60, "") @@ -2154,21 +1981,15 @@ func TestGetUsersInChannel(t *testing.T) { rusers, resp = th.Client.GetUsersInChannel(channelId, 0, 1, "") CheckNoError(t, resp) - if len(rusers) != 1 { - t.Fatal("should be 1 per page") - } + require.Len(t, rusers, 1, "should be 1 per page") rusers, resp = th.Client.GetUsersInChannel(channelId, 1, 1, "") CheckNoError(t, resp) - if len(rusers) != 1 { - t.Fatal("should be 1 per page") - } + require.Len(t, rusers, 1, "should be 1 per page") rusers, resp = th.Client.GetUsersInChannel(channelId, 10000, 100, "") CheckNoError(t, resp) - if len(rusers) != 0 { - t.Fatal("should be no users") - } + require.Len(t, rusers, 0, "should be no users") th.Client.Logout() _, resp = th.Client.GetUsersInChannel(channelId, 0, 60, "") @@ -2200,16 +2021,11 @@ func TestGetUsersNotInChannel(t *testing.T) { rusers, resp = th.Client.GetUsersNotInChannel(teamId, channelId, 0, 1, "") CheckNoError(t, resp) - if len(rusers) != 1 { - t.Log(len(rusers)) - t.Fatal("should be 1 per page") - } + require.Len(t, rusers, 1, "should be 1 per page") rusers, resp = th.Client.GetUsersNotInChannel(teamId, channelId, 10000, 100, "") CheckNoError(t, resp) - if len(rusers) != 0 { - t.Fatal("should be no users") - } + require.Len(t, rusers, 0, "should be no users") th.Client.Logout() _, resp = th.Client.GetUsersNotInChannel(teamId, channelId, 0, 60, "") @@ -2250,9 +2066,7 @@ func TestCheckUserMfa(t *testing.T) { required, resp := th.Client.CheckUserMfa(th.BasicUser.Email) CheckNoError(t, resp) - if required { - t.Fatal("should be false - mfa not active") - } + require.False(t, required, "mfa not active") _, resp = th.Client.CheckUserMfa("") CheckBadRequestStatus(t, resp) @@ -2262,9 +2076,7 @@ func TestCheckUserMfa(t *testing.T) { required, resp = th.Client.CheckUserMfa(th.BasicUser.Email) CheckNoError(t, resp) - if required { - t.Fatal("should be false - mfa not active") - } + require.False(t, required, "mfa not active") th.App.SetLicense(model.NewTestLicense("mfa")) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableMultifactorAuthentication = true }) @@ -2274,18 +2086,14 @@ func TestCheckUserMfa(t *testing.T) { required, resp = th.Client.CheckUserMfa(th.BasicUser.Email) CheckNoError(t, resp) - if required { - t.Fatal("should be false - mfa not active") - } + require.False(t, required, "mfa not active") th.Client.Logout() required, resp = th.Client.CheckUserMfa(th.BasicUser.Email) CheckNoError(t, resp) - if required { - t.Fatal("should be false - mfa not active") - } + require.False(t, required, "mfa not active") th.App.UpdateConfig(func(c *model.Config) { *c.ServiceSettings.DisableLegacyMFA = true @@ -2314,13 +2122,14 @@ func TestUserLoginMFAFlow(t *testing.T) { assert.Nil(t, err) // Fake user has MFA enabled - if err = th.Server.Store.User().UpdateMfaActive(th.BasicUser.Id, true); err != nil { - t.Fatal(err) - } + err = th.Server.Store.User().UpdateMfaActive(th.BasicUser.Id, true) + require.Nil(t, err) - if err = th.Server.Store.User().UpdateMfaSecret(th.BasicUser.Id, secret.Secret); err != nil { - t.Fatal(err) - } + err = th.Server.Store.User().UpdateMfaActive(th.BasicUser.Id, true) + require.Nil(t, err) + + err = th.Server.Store.User().UpdateMfaSecret(th.BasicUser.Id, secret.Secret) + require.Nil(t, err) user, resp := th.Client.Login(th.BasicUser.Email, th.BasicUser.Password) CheckErrorMessage(t, resp, "mfa.validate_token.authenticate.app_error") @@ -2346,13 +2155,11 @@ func TestUserLoginMFAFlow(t *testing.T) { assert.Nil(t, err) // Fake user has MFA enabled - if err = th.Server.Store.User().UpdateMfaActive(th.BasicUser.Id, true); err != nil { - t.Fatal(err) - } + err = th.Server.Store.User().UpdateMfaActive(th.BasicUser.Id, true) + require.Nil(t, err) - if err = th.Server.Store.User().UpdateMfaSecret(th.BasicUser.Id, secret.Secret); err != nil { - t.Fatal(err) - } + err = th.Server.Store.User().UpdateMfaSecret(th.BasicUser.Id, secret.Secret) + require.Nil(t, err) code := dgoogauth.ComputeCode(secret.Secret, time.Now().UTC().Unix()/30) @@ -2404,9 +2211,7 @@ func TestUpdateUserPassword(t *testing.T) { pass, resp := th.Client.UpdateUserPassword(th.BasicUser.Id, th.BasicUser.Password, password) CheckNoError(t, resp) - if !pass { - t.Fatal("should have returned true") - } + require.True(t, pass) _, resp = th.Client.UpdateUserPassword(th.BasicUser.Id, password, "") CheckBadRequestStatus(t, resp) @@ -2455,9 +2260,7 @@ func TestUpdateUserPassword(t *testing.T) { pass, resp = th.SystemAdminClient.UpdateUserPassword(th.BasicUser.Id, "", adminSetPassword) CheckNoError(t, resp) - if !pass { - t.Fatal("should have returned true") - } + require.True(t, pass) _, resp = th.Client.Login(th.BasicUser.Email, adminSetPassword) CheckNoError(t, resp) @@ -2474,17 +2277,13 @@ func TestResetPassword(t *testing.T) { mailservice.DeleteMailBox(user.Email) success, resp := th.Client.SendPasswordResetEmail(user.Email) CheckNoError(t, resp) - if !success { - t.Fatal("should have succeeded") - } + require.True(t, success, "should succeed") _, resp = th.Client.SendPasswordResetEmail("") CheckBadRequestStatus(t, resp) // Should not leak whether the email is attached to an account or not success, resp = th.Client.SendPasswordResetEmail("notreal@example.com") CheckNoError(t, resp) - if !success { - t.Fatal("should have succeeded") - } + require.True(t, success, "should succeed") // Check if the email was send to the right email address and the recovery key match var resultsMailbox mailservice.JSONMessageHeaderInbucket err := mailservice.RetryInbucket(5, func() error { @@ -2498,20 +2297,13 @@ func TestResetPassword(t *testing.T) { } var recoveryTokenString string if err == nil && len(resultsMailbox) > 0 { - if !strings.ContainsAny(resultsMailbox[0].To[0], user.Email) { - t.Fatal("Wrong To recipient") - } else { - var resultsEmail mailservice.JSONMessageInbucket - if resultsEmail, err = mailservice.GetMessageFromMailbox(user.Email, resultsMailbox[0].ID); err == nil { - loc := strings.Index(resultsEmail.Body.Text, "token=") - if loc == -1 { - t.Log(resultsEmail.Body.Text) - t.Fatal("Code not found in email") - } - loc += 6 - recoveryTokenString = resultsEmail.Body.Text[loc : loc+model.TOKEN_SIZE] - } - } + require.Contains(t, resultsMailbox[0].To[0], user.Email, "Correct To recipient") + resultsEmail, mailErr := mailservice.GetMessageFromMailbox(user.Email, resultsMailbox[0].ID) + require.NoError(t, mailErr) + loc := strings.Index(resultsEmail.Body.Text, "token=") + require.NotEqual(t, -1, loc, "Code should be found in email") + loc += 6 + recoveryTokenString = resultsEmail.Body.Text[loc : loc+model.TOKEN_SIZE] } recoveryToken, err := th.App.Srv.Store.Token().GetByToken(recoveryTokenString) require.Nil(t, err, "Recovery token not found (%s)", recoveryTokenString) @@ -2532,17 +2324,14 @@ func TestResetPassword(t *testing.T) { CheckBadRequestStatus(t, resp) success, resp = th.Client.ResetPassword(recoveryToken.Token, "newpwd") CheckNoError(t, resp) - if !success { - t.Fatal("should have succeeded") - } + require.True(t, success) th.Client.Login(user.Email, "newpwd") th.Client.Logout() _, resp = th.Client.ResetPassword(recoveryToken.Token, "newpwd") CheckBadRequestStatus(t, resp) authData := model.NewId() - if _, err := th.App.Srv.Store.User().UpdateAuthData(user.Id, "random", &authData, "", true); err != nil { - t.Fatal(err) - } + _, err = th.App.Srv.Store.User().UpdateAuthData(user.Id, "random", &authData, "", true) + require.Nil(t, err) _, resp = th.Client.SendPasswordResetEmail(user.Email) CheckBadRequestStatus(t, resp) } @@ -2557,9 +2346,7 @@ func TestGetSessions(t *testing.T) { sessions, resp := th.Client.GetSessions(user.Id, "") for _, session := range sessions { - if session.UserId != user.Id { - t.Fatal("user id does not match session user id") - } + require.Equal(t, user.Id, session.UserId, "user id should match session user id") } CheckNoError(t, resp) @@ -2593,13 +2380,9 @@ func TestRevokeSessions(t *testing.T) { user := th.BasicUser th.Client.Login(user.Email, user.Password) sessions, _ := th.Client.GetSessions(user.Id, "") - if len(sessions) == 0 { - t.Fatal("sessions should exist") - } + require.NotZero(t, len(sessions), "sessions should exist") for _, session := range sessions { - if session.UserId != user.Id { - t.Fatal("user id does not match session user id") - } + require.Equal(t, user.Id, session.UserId, "user id does not match session user id") } session := sessions[0] @@ -2613,9 +2396,7 @@ func TestRevokeSessions(t *testing.T) { CheckBadRequestStatus(t, resp) status, resp := th.Client.RevokeSession(user.Id, session.Id) - if !status { - t.Fatal("user session revoke unsuccessful") - } + require.True(t, status, "user session revoke successfuly") CheckNoError(t, resp) th.LoginBasic() @@ -2634,13 +2415,9 @@ func TestRevokeSessions(t *testing.T) { CheckBadRequestStatus(t, resp) sessions, _ = th.SystemAdminClient.GetSessions(th.SystemAdminUser.Id, "") - if len(sessions) == 0 { - t.Fatal("sessions should exist") - } + require.NotEmpty(t, sessions, "sessions should exist") for _, session := range sessions { - if session.UserId != th.SystemAdminUser.Id { - t.Fatal("user id does not match session user id") - } + require.Equal(t, th.SystemAdminUser.Id, session.UserId, "user id should match session user id") } session = sessions[0] @@ -2662,9 +2439,7 @@ func TestRevokeAllSessions(t *testing.T) { CheckBadRequestStatus(t, resp) status, resp := th.Client.RevokeAllSessions(user.Id) - if !status { - t.Fatal("user all sessions revoke unsuccessful") - } + require.True(t, status, "user all sessions revoke unsuccessful") CheckNoError(t, resp) th.Client.Logout() @@ -2674,17 +2449,13 @@ func TestRevokeAllSessions(t *testing.T) { th.Client.Login(user.Email, user.Password) sessions, _ := th.Client.GetSessions(user.Id, "") - if len(sessions) < 1 { - t.Fatal("session should exist") - } + require.NotEmpty(t, sessions, "session should exist") _, resp = th.Client.RevokeAllSessions(user.Id) CheckNoError(t, resp) sessions, _ = th.SystemAdminClient.GetSessions(user.Id, "") - if len(sessions) != 0 { - t.Fatal("no sessions should exist for user") - } + require.Empty(t, sessions, "no sessions should exist for user") _, resp = th.Client.RevokeAllSessions(user.Id) CheckUnauthorizedStatus(t, resp) @@ -2787,9 +2558,7 @@ func TestGetUserAudits(t *testing.T) { audits, resp := th.Client.GetUserAudits(user.Id, 0, 100, "") for _, audit := range audits { - if audit.UserId != user.Id { - t.Fatal("user id does not match audit user id") - } + require.Equal(t, user.Id, audit.UserId, "user id should match audit user id") } CheckNoError(t, resp) @@ -2814,9 +2583,7 @@ func TestVerifyUserEmail(t *testing.T) { ruser, _ := th.Client.CreateUser(&user) token, err := th.App.CreateVerifyEmailToken(ruser.Id, email) - if err != nil { - t.Fatal("Unable to create email verify token") - } + require.Nil(t, err, "Unable to create email verify token") _, resp := th.Client.VerifyUserEmail(token.Token) CheckNoError(t, resp) @@ -2835,9 +2602,7 @@ func TestSendVerificationEmail(t *testing.T) { pass, resp := th.Client.SendVerificationEmail(th.BasicUser.Email) CheckNoError(t, resp) - if !pass { - t.Fatal("should have passed") - } + require.True(t, pass, "should have passed") _, resp = th.Client.SendVerificationEmail("") CheckBadRequestStatus(t, resp) @@ -2857,20 +2622,14 @@ func TestSetProfileImage(t *testing.T) { user := th.BasicUser data, err := testutils.ReadTestFile("test.png") - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) ok, resp := th.Client.SetProfileImage(user.Id, data) - if !ok { - t.Fatal(resp.Error) - } + require.Truef(t, ok, "%v", resp.Error) CheckNoError(t, resp) ok, resp = th.Client.SetProfileImage(model.NewId(), data) - if ok { - t.Fatal("Should return false, set profile image not allowed") - } + require.False(t, ok, "Should return false, set profile image not allowed") CheckForbiddenStatus(t, resp) // status code returns either forbidden or unauthorized @@ -2896,9 +2655,8 @@ func TestSetProfileImage(t *testing.T) { assert.True(t, buser.LastPictureUpdate < ruser.LastPictureUpdate, "Picture should have updated for user") info := &model.FileInfo{Path: "users/" + user.Id + "/profile.png"} - if err := th.cleanupTestFile(info); err != nil { - t.Fatal(err) - } + err = th.cleanupTestFile(info) + require.Nil(t, err) } func TestSetDefaultProfileImage(t *testing.T) { @@ -2907,15 +2665,11 @@ func TestSetDefaultProfileImage(t *testing.T) { user := th.BasicUser ok, resp := th.Client.SetDefaultProfileImage(user.Id) - if !ok { - t.Fatal(resp.Error) - } + require.True(t, ok) CheckNoError(t, resp) ok, resp = th.Client.SetDefaultProfileImage(model.NewId()) - if ok { - t.Fatal("Should return false, set profile image not allowed") - } + require.False(t, ok, "Should return false, set profile image not allowed") CheckForbiddenStatus(t, resp) // status code returns either forbidden or unauthorized @@ -2938,9 +2692,8 @@ func TestSetDefaultProfileImage(t *testing.T) { assert.Equal(t, int64(0), ruser.LastPictureUpdate, "Picture should have resetted to default") info := &model.FileInfo{Path: "users/" + user.Id + "/profile.png"} - if err := th.cleanupTestFile(info); err != nil { - t.Fatal(err) - } + cleanupErr := th.cleanupTestFile(info) + require.Nil(t, cleanupErr) } func TestLogin(t *testing.T) { @@ -2987,9 +2740,7 @@ func TestLogin(t *testing.T) { t.Run("login with terms_of_service set", func(t *testing.T) { termsOfService, err := th.App.CreateTermsOfService("terms of service", th.BasicUser.Id) - if err != nil { - t.Fatal(err) - } + require.Nil(t, err) success, resp := th.Client.RegisterTermsOfServiceAction(th.BasicUser.Id, termsOfService.Id, true) CheckNoError(t, resp) @@ -3205,9 +2956,7 @@ func TestSwitchAccount(t *testing.T) { link, resp := th.Client.SwitchAccountType(sr) CheckNoError(t, resp) - if link == "" { - t.Fatal("bad link") - } + require.NotEmpty(t, link, "bad link") th.App.SetLicense(model.NewTestLicense()) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.ExperimentalEnableAuthenticationTransfer = false }) @@ -3253,9 +3002,8 @@ func TestSwitchAccount(t *testing.T) { th.LoginBasic() fakeAuthData := model.NewId() - if _, err := th.App.Srv.Store.User().UpdateAuthData(th.BasicUser.Id, model.USER_AUTH_SERVICE_GITLAB, &fakeAuthData, th.BasicUser.Email, true); err != nil { - t.Fatal(err) - } + _, err := th.App.Srv.Store.User().UpdateAuthData(th.BasicUser.Id, model.USER_AUTH_SERVICE_GITLAB, &fakeAuthData, th.BasicUser.Email, true) + require.Nil(t, err) sr = &model.SwitchRequest{ CurrentService: model.USER_AUTH_SERVICE_GITLAB, @@ -3267,10 +3015,7 @@ func TestSwitchAccount(t *testing.T) { link, resp = th.Client.SwitchAccountType(sr) CheckNoError(t, resp) - if link != "/login?extra=signin_change" { - t.Log(link) - t.Fatal("bad link") - } + require.Equal(t, "/login?extra=signin_change", link) th.Client.Logout() _, resp = th.Client.Login(th.BasicUser.Email, th.BasicUser.Password) @@ -3812,30 +3557,22 @@ func TestSearchUserAccessToken(t *testing.T) { rtokens, resp := th.SystemAdminClient.SearchUserAccessTokens(&model.UserAccessTokenSearch{Term: th.BasicUser.Id}) CheckNoError(t, resp) - if len(rtokens) != 1 { - t.Fatal("should have 1 tokens") - } + require.Len(t, rtokens, 1, "should have 1 token") rtokens, resp = th.SystemAdminClient.SearchUserAccessTokens(&model.UserAccessTokenSearch{Term: token.Id}) CheckNoError(t, resp) - if len(rtokens) != 1 { - t.Fatal("should have 1 tokens") - } + require.Len(t, rtokens, 1, "should have 1 token") rtokens, resp = th.SystemAdminClient.SearchUserAccessTokens(&model.UserAccessTokenSearch{Term: th.BasicUser.Username}) CheckNoError(t, resp) - if len(rtokens) != 1 { - t.Fatal("should have 1 tokens") - } + require.Len(t, rtokens, 1, "should have 1 token") rtokens, resp = th.SystemAdminClient.SearchUserAccessTokens(&model.UserAccessTokenSearch{Term: "not found"}) CheckNoError(t, resp) - if len(rtokens) != 0 { - t.Fatal("should have 0 tokens") - } + require.Len(t, rtokens, 0, "should have 1 tokens") } func TestRevokeUserAccessToken(t *testing.T) { @@ -4293,9 +4030,8 @@ func TestGetUsersByStatus(t *testing.T) { Email: th.GenerateTestEmail(), Type: model.TEAM_OPEN, }) - if err != nil { - t.Fatalf("failed to create team: %v", err) - } + + require.Nil(t, err, "failed to create team") channel, err := th.App.CreateChannel(&model.Channel{ DisplayName: "dn_" + model.NewId(), @@ -4304,9 +4040,7 @@ func TestGetUsersByStatus(t *testing.T) { TeamId: team.Id, CreatorId: model.NewId(), }, false) - if err != nil { - t.Fatalf("failed to create channel: %v", err) - } + require.Nil(t, err, "failed to create channel") createUserWithStatus := func(username string, status string) *model.User { id := model.NewId() @@ -4317,9 +4051,7 @@ func TestGetUsersByStatus(t *testing.T) { Nickname: "nn_" + id, Password: "Password1", }) - if err != nil { - t.Fatalf("failed to create user: %v", err) - } + require.Nil(t, err, "failed to create user") th.LinkUserToTeam(user, team) th.AddUserToChannel(user, channel) @@ -4344,15 +4076,12 @@ func TestGetUsersByStatus(t *testing.T) { dndUser2 := createUserWithStatus("dnd2", model.STATUS_DND) client := th.CreateClient() - if _, resp := client.Login(onlineUser2.Username, "Password1"); resp.Error != nil { - t.Fatal(resp.Error) - } + _, resp := client.Login(onlineUser2.Username, "Password1") + require.Nil(t, resp.Error) t.Run("sorting by status then alphabetical", func(t *testing.T) { usersByStatus, resp := client.GetUsersInChannelByStatus(channel.Id, 0, 8, "") - if resp.Error != nil { - t.Fatal(resp.Error) - } + require.Nil(t, resp.Error) expectedUsersByStatus := []*model.User{ onlineUser1, @@ -4364,65 +4093,37 @@ func TestGetUsersByStatus(t *testing.T) { offlineUser1, offlineUser2, } - - if len(usersByStatus) != len(expectedUsersByStatus) { - t.Fatalf("received only %v users, expected %v", len(usersByStatus), len(expectedUsersByStatus)) - } + require.Equal(t, len(expectedUsersByStatus), len(usersByStatus)) for i := range usersByStatus { - if usersByStatus[i].Id != expectedUsersByStatus[i].Id { - t.Fatalf("received user %v at index %v, expected %v", usersByStatus[i].Username, i, expectedUsersByStatus[i].Username) - } + require.Equal(t, expectedUsersByStatus[i].Id, usersByStatus[i].Id) } }) t.Run("paging", func(t *testing.T) { usersByStatus, resp := client.GetUsersInChannelByStatus(channel.Id, 0, 3, "") - if resp.Error != nil { - t.Fatal(resp.Error) - } - - if len(usersByStatus) != 3 { - t.Fatal("received too many users") - } - - if usersByStatus[0].Id != onlineUser1.Id && usersByStatus[1].Id != onlineUser2.Id { - t.Fatal("expected to receive online users first") - } - - if usersByStatus[2].Id != awayUser1.Id { - t.Fatal("expected to receive away users second") - } + require.Nil(t, resp.Error) + require.Len(t, usersByStatus, 3) + require.Equal(t, onlineUser1.Id, usersByStatus[0].Id, "online users first") + require.Equal(t, onlineUser2.Id, usersByStatus[1].Id, "online users first") + require.Equal(t, awayUser1.Id, usersByStatus[2].Id, "expected to receive away users second") usersByStatus, resp = client.GetUsersInChannelByStatus(channel.Id, 1, 3, "") - if resp.Error != nil { - t.Fatal(resp.Error) - } + require.Nil(t, resp.Error) - if usersByStatus[0].Id != awayUser2.Id { - t.Fatal("expected to receive away users second") - } - - if usersByStatus[1].Id != dndUser1.Id && usersByStatus[2].Id != dndUser2.Id { - t.Fatal("expected to receive dnd users third") - } + require.Equal(t, awayUser2.Id, usersByStatus[0].Id, "expected to receive away users second") + require.Equal(t, dndUser1.Id, usersByStatus[1].Id, "expected to receive dnd users third") + require.Equal(t, dndUser2.Id, usersByStatus[2].Id, "expected to receive dnd users third") usersByStatus, resp = client.GetUsersInChannelByStatus(channel.Id, 1, 4, "") - if resp.Error != nil { - t.Fatal(resp.Error) - } + require.Nil(t, resp.Error) - if len(usersByStatus) != 4 { - t.Fatal("received too many users") - } + require.Len(t, usersByStatus, 4) + require.Equal(t, dndUser1.Id, usersByStatus[0].Id, "expected to receive dnd users third") + require.Equal(t, dndUser2.Id, usersByStatus[1].Id, "expected to receive dnd users third") - if usersByStatus[0].Id != dndUser1.Id && usersByStatus[1].Id != dndUser2.Id { - t.Fatal("expected to receive dnd users third") - } - - if usersByStatus[2].Id != offlineUser1.Id && usersByStatus[3].Id != offlineUser2.Id { - t.Fatal("expected to receive offline users last") - } + require.Equal(t, offlineUser1.Id, usersByStatus[2].Id, "expected to receive offline users last") + require.Equal(t, offlineUser2.Id, usersByStatus[3].Id, "expected to receive offline users last") }) } @@ -4435,18 +4136,14 @@ func TestRegisterTermsOfServiceAction(t *testing.T) { assert.Nil(t, success) termsOfService, err := th.App.CreateTermsOfService("terms of service", th.BasicUser.Id) - if err != nil { - t.Fatal(err) - } + require.Nil(t, err) success, resp = th.Client.RegisterTermsOfServiceAction(th.BasicUser.Id, termsOfService.Id, true) CheckNoError(t, resp) assert.True(t, *success) _, err = th.App.GetUser(th.BasicUser.Id) - if err != nil { - t.Fatal(err) - } + require.Nil(t, err) } func TestGetUserTermsOfService(t *testing.T) { @@ -4457,9 +4154,7 @@ func TestGetUserTermsOfService(t *testing.T) { CheckErrorMessage(t, resp, "store.sql_user_terms_of_service.get_by_user.no_rows.app_error") termsOfService, err := th.App.CreateTermsOfService("terms of service", th.BasicUser.Id) - if err != nil { - t.Fatal(err) - } + require.Nil(t, err) success, resp := th.Client.RegisterTermsOfServiceAction(th.BasicUser.Id, termsOfService.Id, true) CheckNoError(t, resp) @@ -4553,9 +4248,8 @@ func TestLoginLockout(t *testing.T) { CheckErrorMessage(t, resp, "api.user.check_user_login_attempts.too_many.app_error") // Fake user has MFA enabled - if err := th.Server.Store.User().UpdateMfaActive(th.BasicUser2.Id, true); err != nil { - t.Fatal(err) - } + err := th.Server.Store.User().UpdateMfaActive(th.BasicUser2.Id, true) + require.Nil(t, err) _, resp = th.Client.LoginWithMFA(th.BasicUser2.Email, th.BasicUser2.Password, "000000") CheckErrorMessage(t, resp, "api.user.check_user_mfa.bad_code.app_error") _, resp = th.Client.LoginWithMFA(th.BasicUser2.Email, th.BasicUser2.Password, "000000") @@ -4568,9 +4262,8 @@ func TestLoginLockout(t *testing.T) { CheckErrorMessage(t, resp, "api.user.check_user_login_attempts.too_many.app_error") // Fake user has MFA disabled - if err := th.Server.Store.User().UpdateMfaActive(th.BasicUser2.Id, false); err != nil { - t.Fatal(err) - } + err = th.Server.Store.User().UpdateMfaActive(th.BasicUser2.Id, false) + require.Nil(t, err) //Check if lock is active _, resp = th.Client.Login(th.BasicUser2.Email, th.BasicUser2.Password) diff --git a/app/notification.go b/app/notification.go index 1b4aeae959..5bd906bebb 100644 --- a/app/notification.go +++ b/app/notification.go @@ -139,7 +139,7 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod updateMentionChans = append(updateMentionChans, umc) } - notification := &postNotification{ + notification := &PostNotification{ post: post, channel: channel, profileMap: profileMap, @@ -691,7 +691,7 @@ func addMentionKeywordsForUser(keywords map[string][]string, profile *model.User } // Represents either an email or push notification and contains the fields required to send it to any user. -type postNotification struct { +type PostNotification struct { channel *model.Channel post *model.Post profileMap map[string]*model.User @@ -701,7 +701,7 @@ type postNotification struct { // Returns the name of the channel for this notification. For direct messages, this is the sender's name // preceeded by an at sign. For group messages, this is a comma-separated list of the members of the // channel, with an option to exclude the recipient of the message from that list. -func (n *postNotification) GetChannelName(userNameFormat string, excludeId string) string { +func (n *PostNotification) GetChannelName(userNameFormat, excludeId string) string { switch n.channel.Type { case model.CHANNEL_DIRECT: return n.sender.GetDisplayNameWithPrefix(userNameFormat, "@") @@ -723,7 +723,7 @@ func (n *postNotification) GetChannelName(userNameFormat string, excludeId strin // Returns the name of the sender of this notification, accounting for things like system messages // and whether or not the username has been overridden by an integration. -func (n *postNotification) GetSenderName(userNameFormat string, overridesAllowed bool) string { +func (n *PostNotification) GetSenderName(userNameFormat string, overridesAllowed bool) string { if n.post.IsSystemMessage() { return utils.T("system.message.name") } diff --git a/app/notification_email.go b/app/notification_email.go index 20af2a7fe3..65c5ff0f8d 100644 --- a/app/notification_email.go +++ b/app/notification_email.go @@ -18,7 +18,7 @@ import ( "github.com/mattermost/mattermost-server/utils" ) -func (a *App) sendNotificationEmail(notification *postNotification, user *model.User, team *model.Team) *model.AppError { +func (a *App) sendNotificationEmail(notification *PostNotification, user *model.User, team *model.Team) *model.AppError { channel := notification.channel post := notification.post diff --git a/app/notification_push.go b/app/notification_push.go index f20f3cd669..e14d195acb 100644 --- a/app/notification_push.go +++ b/app/notification_push.go @@ -110,7 +110,7 @@ func (a *App) sendPushNotificationToAllSessions(msg *model.PushNotification, use return nil } -func (a *App) sendPushNotification(notification *postNotification, user *model.User, explicitMention, channelWideMention bool, replyToThreadType string) { +func (a *App) sendPushNotification(notification *PostNotification, user *model.User, explicitMention, channelWideMention bool, replyToThreadType string) { cfg := a.Config() channel := notification.channel post := notification.post @@ -134,6 +134,22 @@ func (a *App) sendPushNotification(notification *postNotification, user *model.U } } +func (a *App) getFetchedPushNotificationMessage(postMessage, senderName, channelType string, hasFiles bool, userLocale i18n.TranslateFunc) string { + // If the post only has images then push an appropriate message + if len(postMessage) == 0 && hasFiles { + if channelType == model.CHANNEL_DIRECT { + return strings.Trim(userLocale("api.post.send_notifications_and_forget.push_image_only"), " ") + } + return senderName + userLocale("api.post.send_notifications_and_forget.push_image_only") + } + + if channelType == model.CHANNEL_DIRECT { + return model.ClearMentionTags(postMessage) + } + + return senderName + ": " + model.ClearMentionTags(postMessage) +} + func (a *App) getPushNotificationMessage(postMessage string, explicitMention, channelWideMention, hasFiles bool, senderName, channelName, channelType, replyToThreadType string, userLocale i18n.TranslateFunc) string { @@ -428,9 +444,113 @@ func DoesStatusAllowPushNotification(userNotifyProps model.StringMap, status *mo return false } +func (a *App) BuildFetchedPushNotificationMessage(postId string, userId string) (model.PushNotification, *model.AppError) { + msg := model.PushNotification{ + Type: model.PUSH_TYPE_ID_LOADED, + Category: model.CATEGORY_CAN_REPLY, + Version: model.PUSH_MESSAGE_V2, + } + + post, err := a.GetSinglePost(postId) + if err != nil { + return msg, err + } + + channel, err := a.GetChannel(post.ChannelId) + if err != nil { + return msg, err + } + + user, err := a.GetUser(userId) + if err != nil { + return msg, err + } + + sender, err := a.GetUser(post.UserId) + if err != nil { + return msg, err + } + + msg.PostId = post.Id + msg.RootId = post.RootId + msg.SenderId = post.UserId + msg.ChannelId = channel.Id + msg.TeamId = channel.TeamId + + notification := &PostNotification{ + post: post, + channel: channel, + sender: sender, + } + + cfg := a.Config() + nameFormat := a.GetNotificationNameFormat(user) + channelName := notification.GetChannelName(nameFormat, user.Id) + senderName := notification.GetSenderName(nameFormat, *cfg.ServiceSettings.EnablePostUsernameOverride) + + msg.ChannelName = channelName + + msg.SenderName = senderName + if ou, ok := post.Props["override_username"].(string); ok && *cfg.ServiceSettings.EnablePostUsernameOverride { + msg.OverrideUsername = ou + msg.SenderName = ou + } + + if oi, ok := post.Props["override_icon_url"].(string); ok && *cfg.ServiceSettings.EnablePostIconOverride { + msg.OverrideIconUrl = oi + } + + if fw, ok := post.Props["from_webhook"].(string); ok { + msg.FromWebhook = fw + } + + userLocale := utils.GetUserTranslations(user.Locale) + hasFiles := post.FileIds != nil && len(post.FileIds) > 0 + + msg.Message = a.getFetchedPushNotificationMessage(post.Message, msg.SenderName, channel.Type, hasFiles, userLocale) + + return msg, nil +} + func (a *App) BuildPushNotificationMessage(post *model.Post, user *model.User, channel *model.Channel, channelName string, senderName string, explicitMention bool, channelWideMention bool, replyToThreadType string) (*model.PushNotification, *model.AppError) { + var msg *model.PushNotification + + cfg := a.Config() + contentsConfig := *cfg.EmailSettings.PushNotificationContents + if contentsConfig == model.ID_LOADED_NOTIFICATION { + msg = a.buildIdLoadedPushNotificationMessage(post, user) + } else { + msg = a.buildFullPushNotificationMessage(post, user, channel, channelName, senderName, explicitMention, channelWideMention, replyToThreadType) + } + + badge, err := a.getPushNotificationBadge(user, channel) + if err != nil { + return nil, err + } + msg.Badge = badge + + return msg, nil +} + +func (a *App) buildIdLoadedPushNotificationMessage(post *model.Post, user *model.User) *model.PushNotification { + userLocale := utils.GetUserTranslations(user.Locale) + msg := &model.PushNotification{ + PostId: post.Id, + ChannelId: post.ChannelId, + Category: model.CATEGORY_CAN_REPLY, + Version: model.PUSH_MESSAGE_V2, + Type: model.PUSH_TYPE_ID_LOADED, + Message: userLocale("api.push_notification.id_loaded.default_message"), + } + + return msg +} + +func (a *App) buildFullPushNotificationMessage(post *model.Post, user *model.User, channel *model.Channel, channelName string, senderName string, + explicitMention bool, channelWideMention bool, replyToThreadType string) *model.PushNotification { + msg := &model.PushNotification{ Category: model.CATEGORY_CAN_REPLY, Version: model.PUSH_MESSAGE_V2, @@ -442,22 +562,6 @@ func (a *App) BuildPushNotificationMessage(post *model.Post, user *model.User, c SenderId: post.UserId, } - if user.NotifyProps["push"] == "all" { - unreadCount, err := a.Srv.Store.User().GetAnyUnreadPostCountForChannel(user.Id, channel.Id) - if err != nil { - return nil, err - } - - msg.Badge = int(unreadCount) - } else { - unreadCount, err := a.Srv.Store.User().GetUnreadCount(user.Id) - if err != nil { - return nil, err - } - - msg.Badge = int(unreadCount) - } - cfg := a.Config() contentsConfig := *cfg.EmailSettings.PushNotificationContents if contentsConfig != model.GENERIC_NO_CHANNEL_NOTIFICATION || channel.Type == model.CHANNEL_DIRECT { @@ -483,5 +587,18 @@ func (a *App) BuildPushNotificationMessage(post *model.Post, user *model.User, c msg.Message = a.getPushNotificationMessage(post.Message, explicitMention, channelWideMention, hasFiles, msg.SenderName, channelName, channel.Type, replyToThreadType, userLocale) - return msg, nil + return msg +} + +func (a *App) getPushNotificationBadge(user *model.User, channel *model.Channel) (int, *model.AppError) { + var unreadCount int64 + var err *model.AppError + + if user.NotifyProps["push"] == "all" { + unreadCount, err = a.Srv.Store.User().GetAnyUnreadPostCountForChannel(user.Id, channel.Id) + } else { + unreadCount, err = a.Srv.Store.User().GetUnreadCount(user.Id) + } + + return int(unreadCount), err } diff --git a/app/notification_push_test.go b/app/notification_push_test.go index f34ff0c150..bb1bf1f72f 100644 --- a/app/notification_push_test.go +++ b/app/notification_push_test.go @@ -899,7 +899,7 @@ func TestGetPushNotificationMessage(t *testing.T) { } } -func TestBuildPushNotificationMessage(t *testing.T) { +func TestBuildPushNotificationMessageMentions(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() @@ -949,3 +949,67 @@ func TestBuildPushNotificationMessage(t *testing.T) { }) } } + +func TestBuildPushNotificationMessageContents(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + team := th.CreateTeam() + sender := th.CreateUser() + receiver := th.CreateUser() + th.LinkUserToTeam(sender, team) + th.LinkUserToTeam(receiver, team) + channel := th.CreateChannel(team) + th.AddUserToChannel(sender, channel) + th.AddUserToChannel(receiver, channel) + + post := th.CreatePost(channel) + + explicitMention := false + channelWideMention := false + replyToThreadType := "" + + receiverLocale := utils.GetUserTranslations(receiver.Locale) + + for name, tc := range map[string]struct { + contentsConfig string + expectedMsg *model.PushNotification + }{ + "only post ID, channel ID, and message included in push notification": { + contentsConfig: model.ID_LOADED_NOTIFICATION, + expectedMsg: &model.PushNotification{ + PostId: post.Id, + ChannelId: post.ChannelId, + Category: model.CATEGORY_CAN_REPLY, + Version: model.PUSH_MESSAGE_V2, + Type: model.PUSH_TYPE_ID_LOADED, + Message: receiverLocale("api.push_notification.id_loaded.default_message"), + }, + }, + "full contents included in push notification": { + contentsConfig: model.GENERIC_NOTIFICATION, + expectedMsg: &model.PushNotification{ + Category: model.CATEGORY_CAN_REPLY, + Version: model.PUSH_MESSAGE_V2, + Type: model.PUSH_TYPE_MESSAGE, + PostId: post.Id, + TeamId: channel.TeamId, + ChannelId: channel.Id, + ChannelName: channel.Name, + RootId: post.RootId, + SenderId: post.UserId, + SenderName: sender.Username, + Message: fmt.Sprintf("%s posted a message.", sender.Username), + }, + }, + } { + t.Run(name, func(t *testing.T) { + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.EmailSettings.PushNotificationContents = tc.contentsConfig }) + + msg, err := th.App.BuildPushNotificationMessage(post, receiver, channel, channel.Name, sender.Username, explicitMention, channelWideMention, replyToThreadType) + + require.Nil(t, err) + assert.Equal(t, tc.expectedMsg, msg) + }) + } +} diff --git a/app/notification_test.go b/app/notification_test.go index 29fe9bc89d..a9d453ee23 100644 --- a/app/notification_test.go +++ b/app/notification_test.go @@ -1540,7 +1540,7 @@ func TestPostNotificationGetChannelName(t *testing.T) { }, } { t.Run(name, func(t *testing.T) { - notification := &postNotification{ + notification := &PostNotification{ channel: testCase.channel, sender: sender, profileMap: profileMap, @@ -1625,7 +1625,7 @@ func TestPostNotificationGetSenderName(t *testing.T) { post = testCase.post } - notification := &postNotification{ + notification := &PostNotification{ channel: channel, post: post, sender: sender, diff --git a/app/plugin.go b/app/plugin.go index e543a7de51..4440e9a3fa 100644 --- a/app/plugin.go +++ b/app/plugin.go @@ -19,6 +19,12 @@ import ( "github.com/pkg/errors" ) +type pluginSignaturePath struct { + pluginId string + path string + signaturePath string +} + // GetPluginsEnvironment returns the plugin environment for use if plugins are enabled and // initialized. // @@ -164,10 +170,18 @@ func (a *App) InitPlugins(pluginDir, webappPluginDir string) { return nil } - if fileReader, err := os.Open(walkPath); err != nil { + fileReader, err := os.Open(walkPath) + if err != nil { mlog.Error("Failed to open prepackaged plugin", mlog.Err(err), mlog.String("path", walkPath)) - } else if _, err := a.installPluginLocally(fileReader, true); err != nil { - mlog.Error("Failed to unpack prepackaged plugin", mlog.Err(err), mlog.String("path", walkPath)) + return nil + } + defer fileReader.Close() + + mlog.Debug("Installing prepackaged plugin", mlog.String("path", walkPath)) + + _, appErr := a.installPluginLocally(fileReader, nil, installPluginLocallyOnlyIfNewOrUpgrade) + if appErr != nil { + mlog.Error("Failed to unpack prepackaged plugin", mlog.Err(appErr), mlog.String("path", walkPath)) } return nil @@ -226,35 +240,33 @@ func (a *App) SyncPlugins() *model.AppError { } // Install plugins from the file store. - fileStorePaths, appErr := a.ListDirectory(fileStorePluginFolder) + pluginSignaturePathMap, appErr := a.getPluginsFromFolder() if appErr != nil { - return model.NewAppError("SyncPlugins", "app.plugin.sync.list_filestore.app_error", nil, appErr.Error(), http.StatusInternalServerError) + return appErr } - if len(fileStorePaths) == 0 { - mlog.Info("Found no files in plugins file store") - return nil - } - - for _, path := range fileStorePaths { - if !strings.HasSuffix(path, ".tar.gz") { - mlog.Warn("Ignoring non-plugin in file store", mlog.String("bundle", path)) - continue - } - - var reader filesstore.ReadCloseSeeker - reader, appErr = a.FileReader(path) + for _, plugin := range pluginSignaturePathMap { + reader, appErr := a.FileReader(plugin.path) if appErr != nil { - mlog.Error("Failed to open plugin bundle from file store.", mlog.String("bundle", path), mlog.Err(appErr)) + mlog.Error("Failed to open plugin bundle from file store.", mlog.String("bundle", plugin.path), mlog.Err(appErr)) continue } defer reader.Close() - mlog.Info("Syncing plugin from file store", mlog.String("bundle", path)) - if _, err := a.installPluginLocally(reader, true); err != nil { - mlog.Error("Failed to sync plugin from file store", mlog.String("bundle", path), mlog.Err(err)) + var signature filesstore.ReadCloseSeeker + if *a.Config().PluginSettings.RequirePluginSignature { + signature, appErr = a.FileReader(plugin.signaturePath) + if appErr != nil { + mlog.Error("Failed to open plugin signature from file store.", mlog.Err(appErr)) + continue + } + defer signature.Close() + } + + mlog.Info("Syncing plugin from file store", mlog.String("bundle", plugin.path)) + if _, err := a.installPluginLocally(reader, signature, installPluginLocallyAlways); err != nil { + mlog.Error("Failed to sync plugin from file store", mlog.String("bundle", plugin.path), mlog.Err(err)) } } - return nil } @@ -364,7 +376,7 @@ func (a *App) DisablePlugin(id string) *model.AppError { } if manifest == nil { - return model.NewAppError("DisablePlugin", "app.plugin.not_installed.app_error", nil, "", http.StatusBadRequest) + return model.NewAppError("DisablePlugin", "app.plugin.not_installed.app_error", nil, "", http.StatusNotFound) } a.UpdateConfig(func(cfg *model.Config) { @@ -410,6 +422,24 @@ func (a *App) GetPlugins() (*model.PluginsResponse, *model.AppError) { return resp, nil } +// GetMarketplacePlugin returns plugin from marketplace-server +func (a *App) GetMarketplacePlugin(request *model.InstallMarketplacePluginRequest) (*model.BaseMarketplacePlugin, *model.AppError) { + marketplaceClient, err := marketplace.NewClient( + *a.Config().PluginSettings.MarketplaceUrl, + a.HTTPService, + ) + if err != nil { + return nil, model.NewAppError("GetMarketplacePlugin", "app.plugin.marketplace_client.app_error", nil, err.Error(), http.StatusInternalServerError) + } + + filter := &model.MarketplacePluginFilter{Filter: request.Id} + plugin, err := marketplaceClient.GetPlugin(filter, request.Version) + if err != nil { + return nil, model.NewAppError("GetMarketplacePlugin", "app.plugin.marketplace_plugins.not_found.app_error", nil, err.Error(), http.StatusInternalServerError) + } + return plugin, nil +} + // GetMarketplacePlugins returns a list of plugins from the marketplace-server, // and plugins that are installed locally. func (a *App) GetMarketplacePlugins(filter *model.MarketplacePluginFilter) ([]*model.MarketplacePlugin, *model.AppError) { @@ -557,3 +587,33 @@ func (a *App) notifyPluginEnabled(manifest *model.Manifest) error { return nil } + +func (a *App) getPluginsFromFolder() (map[string]*pluginSignaturePath, *model.AppError) { + fileStorePaths, appErr := a.ListDirectory(fileStorePluginFolder) + if appErr != nil { + return nil, model.NewAppError("getPluginsFromDir", "app.plugin.sync.list_filestore.app_error", nil, appErr.Error(), http.StatusInternalServerError) + } + pluginSignaturePathMap := make(map[string]*pluginSignaturePath) + for _, path := range fileStorePaths { + if strings.HasSuffix(path, ".tar.gz") { + id := strings.TrimSuffix(filepath.Base(path), ".tar.gz") + helper := &pluginSignaturePath{ + pluginId: id, + path: path, + signaturePath: "", + } + pluginSignaturePathMap[id] = helper + } + } + for _, path := range fileStorePaths { + if strings.HasSuffix(path, ".sig") { + id := strings.TrimSuffix(filepath.Base(path), ".sig") + if val, ok := pluginSignaturePathMap[id]; !ok { + mlog.Error("Unknown signature", mlog.String("path", path)) + } else { + val.signaturePath = path + } + } + } + return pluginSignaturePathMap, nil +} diff --git a/app/plugin_install.go b/app/plugin_install.go index 26b768418e..c8303bee12 100644 --- a/app/plugin_install.go +++ b/app/plugin_install.go @@ -31,8 +31,8 @@ // Finally, in addition to managed plugins, note that there are unmanaged and prepackaged plugins. // Unmanaged plugins are plugins installed manually to the configured local directory (PluginSettings.Directory). // Prepackaged plugins are included with the server. They otherwise follow the above flow, except do not get uploaded -// to the filestore. Prepackaged plugins override all other plugins with the same plugin id. Managed plugins -// override unmanaged plugins with the same plugin id. +// to the filestore. Prepackaged plugins override all other plugins with the same plugin id, but only when the prepackaged +// plugin is newer. Managed plugins unconditionally override unmanaged plugins with the same plugin id. // package app @@ -44,9 +44,11 @@ import ( "os" "path/filepath" + "github.com/blang/semver" "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/plugin" + "github.com/mattermost/mattermost-server/services/filesstore" "github.com/mattermost/mattermost-server/utils" ) @@ -60,16 +62,38 @@ const fileStorePluginFolder = "plugins" func (a *App) InstallPluginFromData(data model.PluginEventData) { mlog.Debug("Installing plugin as per cluster message", mlog.String("plugin_id", data.Id)) - fileStorePath := a.getBundleStorePath(data.Id) - reader, appErr := a.FileReader(fileStorePath) + pluginSignaturePathMap, appErr := a.getPluginsFromFolder() if appErr != nil { - mlog.Error("Failed to open plugin bundle from filestore.", mlog.String("path", fileStorePath), mlog.Err(appErr)) + mlog.Error("Failed to get plugin signatures from filestore. Can't install plugin from data.", mlog.Err(appErr)) + return + } + plugin, ok := pluginSignaturePathMap[data.Id] + if !ok { + mlog.Error("Failed to get plugin signature from filestore. Can't install plugin from data.", mlog.String("plugin id", data.Id)) + return + } + + reader, appErr := a.FileReader(plugin.path) + if appErr != nil { + mlog.Error("Failed to open plugin bundle from file store.", mlog.String("bundle", plugin.path), mlog.Err(appErr)) + return } defer reader.Close() - manifest, appErr := a.installPluginLocally(reader, true) + var signature filesstore.ReadCloseSeeker + if *a.Config().PluginSettings.RequirePluginSignature { + signature, appErr = a.FileReader(plugin.signaturePath) + if appErr != nil { + mlog.Error("Failed to open plugin signature from file store.", mlog.Err(appErr)) + return + } + defer signature.Close() + } + + manifest, appErr := a.installPluginLocally(reader, signature, installPluginLocallyAlways) if appErr != nil { - mlog.Error("Failed to unpack plugin from filestore", mlog.Err(appErr), mlog.String("path", fileStorePath)) + mlog.Error("Failed to sync plugin from file store", mlog.String("bundle", plugin.path), mlog.Err(appErr)) + return } if err := a.notifyPluginEnabled(manifest); err != nil { @@ -93,20 +117,36 @@ func (a *App) RemovePluginFromData(data model.PluginEventData) { } } -// InstallPlugin unpacks and installs a plugin but does not enable or activate it. -func (a *App) InstallPlugin(pluginFile io.ReadSeeker, replace bool) (*model.Manifest, *model.AppError) { - return a.installPlugin(pluginFile, replace) +// InstallPluginWithSignature verifies and installs plugin. +func (a *App) InstallPluginWithSignature(pluginFile, signature io.ReadSeeker) (*model.Manifest, *model.AppError) { + return a.installPlugin(pluginFile, signature, installPluginLocallyAlways) } -func (a *App) installPlugin(pluginFile io.ReadSeeker, replace bool) (*model.Manifest, *model.AppError) { - manifest, appErr := a.installPluginLocally(pluginFile, replace) +// InstallPlugin unpacks and installs a plugin but does not enable or activate it. +func (a *App) InstallPlugin(pluginFile io.ReadSeeker, replace bool) (*model.Manifest, *model.AppError) { + installationStrategy := installPluginLocallyOnlyIfNew + if replace { + installationStrategy = installPluginLocallyAlways + } + + return a.installPlugin(pluginFile, nil, installationStrategy) +} + +func (a *App) installPlugin(pluginFile, signature io.ReadSeeker, installationStrategy pluginInstallationStrategy) (*model.Manifest, *model.AppError) { + manifest, appErr := a.installPluginLocally(pluginFile, signature, installationStrategy) if appErr != nil { return nil, appErr } + if signature != nil { + signature.Seek(0, 0) + if _, appErr = a.WriteFile(signature, a.getSignatureStorePath(manifest.Id)); appErr != nil { + return nil, model.NewAppError("saveSignature", "app.plugin.store_signature.app_error", nil, appErr.Error(), http.StatusInternalServerError) + } + } + // Store bundle in the file store to allow access from other servers. pluginFile.Seek(0, 0) - if _, appErr := a.WriteFile(pluginFile, a.getBundleStorePath(manifest.Id)); appErr != nil { return nil, model.NewAppError("uploadPlugin", "app.plugin.store_bundle.app_error", nil, appErr.Error(), http.StatusInternalServerError) } @@ -129,11 +169,28 @@ func (a *App) installPlugin(pluginFile io.ReadSeeker, replace bool) (*model.Mani return manifest, nil } -func (a *App) installPluginLocally(pluginFile io.ReadSeeker, replace bool) (*model.Manifest, *model.AppError) { +type pluginInstallationStrategy int + +const ( + // installPluginLocallyOnlyIfNew installs the given plugin locally only if no plugin with the same id has been unpacked. + installPluginLocallyOnlyIfNew pluginInstallationStrategy = iota + // installPluginLocallyOnlyIfNewOrUpgrade installs the given plugin locally only if no plugin with the same id has been unpacked, or if such a plugin is older. + installPluginLocallyOnlyIfNewOrUpgrade + // installPluginLocallyAlways unconditionally installs the given plugin locally only, clobbering any existing plugin with the same id. + installPluginLocallyAlways +) + +func (a *App) installPluginLocally(pluginFile, signature io.ReadSeeker, installationStrategy pluginInstallationStrategy) (*model.Manifest, *model.AppError) { pluginsEnvironment := a.GetPluginsEnvironment() if pluginsEnvironment == nil { return nil, model.NewAppError("installPluginLocally", "app.plugin.disabled.app_error", nil, "", http.StatusNotImplemented) } + // verify signature + if signature != nil { + if err := a.VerifyPlugin(pluginFile, signature); err != nil { + return nil, err + } + } tmpDir, err := ioutil.TempDir("", "plugintmp") if err != nil { @@ -141,6 +198,7 @@ func (a *App) installPluginLocally(pluginFile io.ReadSeeker, replace bool) (*mod } defer os.RemoveAll(tmpDir) + pluginFile.Seek(0, 0) if err = utils.ExtractTarGz(pluginFile, tmpDir); err != nil { return nil, model.NewAppError("installPluginLocally", "app.plugin.extract.app_error", nil, err.Error(), http.StatusBadRequest) } @@ -169,16 +227,45 @@ func (a *App) installPluginLocally(pluginFile io.ReadSeeker, replace bool) (*mod return nil, model.NewAppError("installPluginLocally", "app.plugin.install.app_error", nil, err.Error(), http.StatusInternalServerError) } - // Check that there is no plugin with the same ID + // Check for plugins installed with the same ID. + var existingManifest *model.Manifest for _, bundle := range bundles { if bundle.Manifest != nil && bundle.Manifest.Id == manifest.Id { - if !replace { - return nil, model.NewAppError("installPluginLocally", "app.plugin.install_id.app_error", nil, "", http.StatusBadRequest) + existingManifest = bundle.Manifest + break + } + } + + if existingManifest != nil { + // Return an error if already installed and strategy disallows installation. + if installationStrategy == installPluginLocallyOnlyIfNew { + return nil, model.NewAppError("installPluginLocally", "app.plugin.install_id.app_error", nil, "", http.StatusBadRequest) + } + + // Skip installation if already installed and newer. + if installationStrategy == installPluginLocallyOnlyIfNewOrUpgrade { + var version, existingVersion semver.Version + + version, err = semver.Parse(manifest.Version) + if err != nil { + return nil, model.NewAppError("installPluginLocally", "app.plugin.invalid_version.app_error", nil, "", http.StatusBadRequest) } - if err := a.removePluginLocally(manifest.Id); err != nil { - return nil, model.NewAppError("installPluginLocally", "app.plugin.install_id_failed_remove.app_error", nil, "", http.StatusBadRequest) + existingVersion, err = semver.Parse(existingManifest.Version) + if err != nil { + return nil, model.NewAppError("installPluginLocally", "app.plugin.invalid_version.app_error", nil, "", http.StatusBadRequest) } + + if version.LTE(existingVersion) { + mlog.Debug("Skipping local installation of plugin since existing version is newer", mlog.String("plugin_id", manifest.Id)) + return nil, nil + } + } + + // Otherwise remove the existing installation prior to install below. + mlog.Debug("Removing existing installation of plugin before local install", mlog.String("plugin_id", existingManifest.Id), mlog.String("version", existingManifest.Version)) + if err := a.removePluginLocally(existingManifest.Id); err != nil { + return nil, model.NewAppError("installPluginLocally", "app.plugin.install_id_failed_remove.app_error", nil, "", http.StatusBadRequest) } } @@ -240,9 +327,12 @@ func (a *App) removePlugin(id string) *model.AppError { if !bundleExist { return nil } - if err := a.RemoveFile(storePluginFileName); err != nil { + if err = a.RemoveFile(storePluginFileName); err != nil { return model.NewAppError("removePlugin", "app.plugin.remove_bundle.app_error", nil, err.Error(), http.StatusInternalServerError) } + if err = a.removeSignature(id); err != nil { + mlog.Error("Can't remove signature", mlog.Err(err)) + } a.notifyClusterPluginEvent( model.CLUSTER_EVENT_REMOVE_PLUGIN, @@ -280,7 +370,7 @@ func (a *App) removePluginLocally(id string) *model.AppError { } if manifest == nil { - return model.NewAppError("removePlugin", "app.plugin.not_installed.app_error", nil, "", http.StatusBadRequest) + return model.NewAppError("removePlugin", "app.plugin.not_installed.app_error", nil, "", http.StatusNotFound) } pluginsEnvironment.Deactivate(id) @@ -294,6 +384,26 @@ func (a *App) removePluginLocally(id string) *model.AppError { return nil } +func (a *App) removeSignature(pluginId string) *model.AppError { + filePath := a.getSignatureStorePath(pluginId) + exists, err := a.FileExists(filePath) + if err != nil { + return model.NewAppError("removeSignature", "app.plugin.remove_bundle.app_error", nil, err.Error(), http.StatusInternalServerError) + } + if !exists { + mlog.Debug("no plugin signature to remove", mlog.String("plugin_id", pluginId)) + return nil + } + if err = a.RemoveFile(filePath); err != nil { + return model.NewAppError("removeSignature", "app.plugin.remove_bundle.app_error", nil, err.Error(), http.StatusInternalServerError) + } + return nil +} + func (a *App) getBundleStorePath(id string) string { return filepath.Join(fileStorePluginFolder, fmt.Sprintf("%s.tar.gz", id)) } + +func (a *App) getSignatureStorePath(id string) string { + return filepath.Join(fileStorePluginFolder, fmt.Sprintf("%s.sig", id)) +} diff --git a/app/plugin_install_test.go b/app/plugin_install_test.go new file mode 100644 index 0000000000..1a3e41aae0 --- /dev/null +++ b/app/plugin_install_test.go @@ -0,0 +1,258 @@ +package app + +import ( + "archive/tar" + "bytes" + "compress/gzip" + "io" + "sort" + "testing" + + "github.com/mattermost/mattermost-server/model" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type nilReadSeeker struct { +} + +func (r *nilReadSeeker) Read(p []byte) (int, error) { + return 0, io.EOF +} + +func (r *nilReadSeeker) Seek(offset int64, whence int) (int64, error) { + return 0, nil +} + +type testFile struct { + Name, Body string +} + +func makeInMemoryGzipTarFile(t *testing.T, files []testFile) *bytes.Reader { + var buf bytes.Buffer + gzWriter := gzip.NewWriter(&buf) + + tgz := tar.NewWriter(gzWriter) + + for _, file := range files { + hdr := &tar.Header{ + Name: file.Name, + Mode: 0600, + Size: int64(len(file.Body)), + } + err := tgz.WriteHeader(hdr) + require.NoError(t, err, "failed to write %s to in-memory tar file", file.Name) + _, err = tgz.Write([]byte(file.Body)) + require.NoError(t, err, "failed to write body of %s to in-memory tar file", file.Name) + } + err := tgz.Close() + require.NoError(t, err, "failed to close in-memory tar file") + + err = gzWriter.Close() + require.NoError(t, err, "failed to close in-memory tar.gz file") + + return bytes.NewReader(buf.Bytes()) +} + +type byBundleInfoId []*model.BundleInfo + +func (b byBundleInfoId) Len() int { return len(b) } +func (b byBundleInfoId) Swap(i, j int) { b[i], b[j] = b[j], b[i] } +func (b byBundleInfoId) Less(i, j int) bool { return b[i].Manifest.Id < b[j].Manifest.Id } + +func TestInstallPluginLocally(t *testing.T) { + t.Run("invalid tar", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + actualManifest, appErr := th.App.installPluginLocally(&nilReadSeeker{}, nil, installPluginLocallyOnlyIfNew) + require.NotNil(t, appErr) + assert.Equal(t, "app.plugin.extract.app_error", appErr.Id, appErr.Error()) + require.Nil(t, actualManifest) + }) + + t.Run("missing manifest", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + reader := makeInMemoryGzipTarFile(t, []testFile{ + {"test", "test file"}, + }) + + actualManifest, appErr := th.App.installPluginLocally(reader, nil, installPluginLocallyOnlyIfNew) + require.NotNil(t, appErr) + assert.Equal(t, "app.plugin.manifest.app_error", appErr.Id, appErr.Error()) + require.Nil(t, actualManifest) + }) + + installPlugin := func(t *testing.T, th *TestHelper, id, version string, installationStrategy pluginInstallationStrategy) (*model.Manifest, *model.AppError) { + t.Helper() + + manifest := &model.Manifest{ + Id: id, + Version: version, + } + reader := makeInMemoryGzipTarFile(t, []testFile{ + {"plugin.json", manifest.ToJson()}, + }) + + actualManifest, appError := th.App.installPluginLocally(reader, nil, installationStrategy) + if actualManifest != nil { + require.Equal(t, manifest, actualManifest) + } + + return actualManifest, appError + } + + t.Run("invalid plugin id", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + actualManifest, appErr := installPlugin(t, th, "invalid#plugin#id", "version", installPluginLocallyOnlyIfNew) + require.NotNil(t, appErr) + assert.Equal(t, "app.plugin.invalid_id.app_error", appErr.Id, appErr.Error()) + require.Nil(t, actualManifest) + }) + + // The following tests fail mysteriously on CI due to an unexpected bundle being present. + // This exists to clean up manually until we figure out what test isn't cleaning up after + // itself. + cleanExistingBundles := func(t *testing.T, th *TestHelper) { + pluginsEnvironment := th.App.GetPluginsEnvironment() + require.NotNil(t, pluginsEnvironment) + bundleInfos, err := pluginsEnvironment.Available() + require.Nil(t, err) + + for _, bundleInfo := range bundleInfos { + err := th.App.removePluginLocally(bundleInfo.Manifest.Id) + require.Nilf(t, err, "failed to remove existing plugin %s", bundleInfo.Manifest.Id) + } + } + + assertBundleInfoManifests := func(t *testing.T, th *TestHelper, manifests []*model.Manifest) { + pluginsEnvironment := th.App.GetPluginsEnvironment() + require.NotNil(t, pluginsEnvironment) + bundleInfos, err := pluginsEnvironment.Available() + require.Nil(t, err) + + sort.Sort(byBundleInfoId(bundleInfos)) + + actualManifests := make([]*model.Manifest, 0, len(bundleInfos)) + for _, bundleInfo := range bundleInfos { + actualManifests = append(actualManifests, bundleInfo.Manifest) + } + + require.Equal(t, manifests, actualManifests) + } + + t.Run("no plugins already installed", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + cleanExistingBundles(t, th) + + manifest, appErr := installPlugin(t, th, "valid", "0.0.1", installPluginLocallyOnlyIfNew) + require.Nil(t, appErr) + require.NotNil(t, manifest) + + assertBundleInfoManifests(t, th, []*model.Manifest{manifest}) + }) + + t.Run("different plugin already installed", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + cleanExistingBundles(t, th) + + otherManifest, appErr := installPlugin(t, th, "other", "0.0.1", installPluginLocallyOnlyIfNew) + require.Nil(t, appErr) + require.NotNil(t, otherManifest) + + manifest, appErr := installPlugin(t, th, "valid", "0.0.1", installPluginLocallyOnlyIfNew) + require.Nil(t, appErr) + require.NotNil(t, manifest) + + assertBundleInfoManifests(t, th, []*model.Manifest{otherManifest, manifest}) + }) + + t.Run("same plugin already installed", func(t *testing.T) { + t.Run("install only if new", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + cleanExistingBundles(t, th) + + existingManifest, appErr := installPlugin(t, th, "valid", "0.0.1", installPluginLocallyOnlyIfNew) + require.Nil(t, appErr) + require.NotNil(t, existingManifest) + + manifest, appErr := installPlugin(t, th, "valid", "0.0.1", installPluginLocallyOnlyIfNew) + require.NotNil(t, appErr) + require.Equal(t, "app.plugin.install_id.app_error", appErr.Id, appErr.Error()) + require.Nil(t, manifest) + + assertBundleInfoManifests(t, th, []*model.Manifest{existingManifest}) + }) + + t.Run("install if upgrade, but older", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + cleanExistingBundles(t, th) + + existingManifest, appErr := installPlugin(t, th, "valid", "0.0.2", installPluginLocallyOnlyIfNewOrUpgrade) + require.Nil(t, appErr) + require.NotNil(t, existingManifest) + + manifest, appErr := installPlugin(t, th, "valid", "0.0.1", installPluginLocallyOnlyIfNewOrUpgrade) + require.Nil(t, appErr) + require.Nil(t, manifest) + + assertBundleInfoManifests(t, th, []*model.Manifest{existingManifest}) + }) + + t.Run("install if upgrade, but same version", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + cleanExistingBundles(t, th) + + existingManifest, appErr := installPlugin(t, th, "valid", "0.0.2", installPluginLocallyOnlyIfNewOrUpgrade) + require.Nil(t, appErr) + require.NotNil(t, existingManifest) + + manifest, appErr := installPlugin(t, th, "valid", "0.0.2", installPluginLocallyOnlyIfNewOrUpgrade) + require.Nil(t, appErr) + require.Nil(t, manifest) + + assertBundleInfoManifests(t, th, []*model.Manifest{existingManifest}) + }) + + t.Run("install if upgrade, newer version", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + cleanExistingBundles(t, th) + + existingManifest, appErr := installPlugin(t, th, "valid", "0.0.2", installPluginLocallyOnlyIfNewOrUpgrade) + require.Nil(t, appErr) + require.NotNil(t, existingManifest) + + manifest, appErr := installPlugin(t, th, "valid", "0.0.3", installPluginLocallyOnlyIfNewOrUpgrade) + require.Nil(t, appErr) + require.NotNil(t, manifest) + + assertBundleInfoManifests(t, th, []*model.Manifest{manifest}) + }) + + t.Run("install always, old version", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + cleanExistingBundles(t, th) + + existingManifest, appErr := installPlugin(t, th, "valid", "0.0.2", installPluginLocallyAlways) + require.Nil(t, appErr) + require.NotNil(t, existingManifest) + + manifest, appErr := installPlugin(t, th, "valid", "0.0.1", installPluginLocallyAlways) + require.Nil(t, appErr) + require.NotNil(t, manifest) + + assertBundleInfoManifests(t, th, []*model.Manifest{manifest}) + }) + }) +} diff --git a/app/plugin_public_keys.go b/app/plugin_public_keys.go new file mode 100644 index 0000000000..cec41201ea --- /dev/null +++ b/app/plugin_public_keys.go @@ -0,0 +1,46 @@ +// Copyright (c) 2016-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package app + +var mattermostPluginPublicKey []byte = []byte(`-----BEGIN PGP PUBLIC KEY BLOCK----- + +mQGNBF2gen8BDADKQObdPa6PagvYYMHNGIswCU9mVjOxr5g6niGQ/AxMW7AaHpkk +16/oAzJ+DSyJRRgJMlFbN0iKBrZ6pi1pO5eS4l1CWW3eATr+32gW40SuS/sgzVrS +OYPocqtsC9XWHK2j/UFbaI7aivnUYKIuBzhAWdcUYggjd1qgmM18zWYkuV1Jnywu +Xue3Vsc/pLGqybG/EkBmZHRktr4fNn2xEjmKnUKp28vMF4Pz5e8/2qklSsc9UVl5 +avkex+glOeJSWF3L7S5CmHAWVgQNwKoJrvq7pKOUsZqrHScjyujeKp1Y6cUZdcBF +8bsF1I+J2RxQFqcC6O08x29948P8UkOv4/FpUGYhx6tcqnQ0PdT4fjPslRapvdZo +RiGdlvJLKUvhfRF0cgPxflde7M42cV5saOXKyaF2hPJi/SsFkTVSnyCyixdr8z+M +QMIcgtrQ26ig9s5J0h2j3y9sgvvTh1nxE/XWOlrXjCVohNSRWZBjX1PEd2dAk4ZK +OEB5YcST61kbhd0AEQEAAbRBTWF0dGVybW9zdCBJbmMuIChQbHVnaW4gc2lnbmlu +ZyBSU0EgZGV2LWtleSkgPGFsaUBtYXR0ZXJtb3N0LmNvbT6JAc4EEwEIADgWIQTz ++s5F4N5kLIvWqOZMfGViwZLMHwUCXaB6fwIbAwULCQgHAgYVCgkICwIEFgIDAQIe +AQIXgAAKCRBMfGViwZLMH/NoC/0UAvpTvT1sBD6qFpUOPZUmUSLLndtLzuYoMqID +0vvTdxb1PdbQpVX2sMuS19upyAmkVRh50uxGcsOLU/lUaF8C1C22zeGvtdkbw+79 +Gv1AYlyCCEanhQSdH4z/t8W8nBcSw8kA+423guSzlIrrRSPCIyHSTP/MlwituN1+ +wEUlXMXnjY4nNpyik+e9LoKK05zCy1mYswAnx1I5IH44iOfjqjz2FGv3iFhuc5rt +cEC26RyYCNVH7mIcCwd25/Np+IQbftfUVEugr1OGsSvdbAA3qWRtC9Q7VcFXy3A8 +1svxkGPiZw60oxkG9V5v1l/ETCWztzvZvXXXZcWNaDb81rpn2LFeFulJKBxLLonC +gR/8l1hJAt8uS8ymOQRpVK9QVztlyxtZWZ7FxsfsC4AXthU3VFZjLUd1Tf4k5eW2 +ov9JPjcSHHBQp6ScjtSgLTb4s2B5mD7VFBhuFOTWs1mbpVaRVIpguvYKIdxtDfek +0bjPQSI62K9G8mKGE4SqibfXhhO5AY0EXaB6fwEMALrPejAgOh7IWxmWJPO++8Fv +8eJD5nU7I3I4cWgJolXDSP4gEpkwlfHzAn7BJwTKTvZ5oDqpQCQV3mwqumQlRBKS +DHXU3b1Z4MOq3SbQlFfNduTCzKa7a79/DFf96TXilpVW/XT3HdN69810oCfo87Ub +/fx2G6h9JLaxdwJ57b/8Ej4eNbclGgE4GYHP9Xf0FX7F2xIqaIm/RCTGf7uGlaU0 +RmeEFmy69T7jUAGI7g1gN1eldQ0F1q2HPuhP4iP39ZAz9K4Oyzl+B2IcHXyH2MjP +WXfgjVi87O5rEUvA/cpYU5WFc8hflP7cil16rb/PiALzEx+GCpdARxvtMT/IbK/3 +luC2l/uw2ZYwtaL+8e9vyDOkVaWTD408Q51qrIANWwwLUSn71TuImGxCDzeuN79V +/T5PSjR5o/s6lR0CGzNL/B3MziuD2Vr5Wl1LYkJfTlgmGrnm6aJ/zKbrOMnkeAu5 +Q0VgVOyibKhTu31WdXJ/jbhPQ5yd4UkduSAODStsRQARAQABiQG2BBgBCAAgFiEE +8/rOReDeZCyL1qjmTHxlYsGSzB8FAl2gen8CGwwACgkQTHxlYsGSzB8v7Av+IC9I +t7U3W51hCXH2wNcaSi8hxSYpFMl7GMX9zSKE8nKDmKBXUV7RJtU3cpGiGvgl+LLw +qBtjahRP+PU8AQSLL/4W97ldQrrdnOET6mtEiJylliA187SkimSixyy31YnUKDn6 +PIeapJaoJ+JI22VhqbGd5tJCDbjTRFyiJP0L6vCEUAoLhpaqsqUiUw86//USl3uh +P+G9m2z3QPmxVFP/xZFEbihprpe/AccDLFjTwEWAMag6vV0NoI0E+JGeICKtzkxB +Pgi71N/jHKULPVMPXkaD30GT4k72lmuwqfvLz9uEhgSeAHakma8wUlp0aSw+kk4a +dZmqpBXcl6VFSDpCJXANUS4IUqjVqnK4nAGONR4JFaoejtAnmlz61EtjWuzPYjQS +0dL1Jv69WXLal7tzTJOZLekHas8DxzMgkID4IXCaSjwDb34mVgdaWyD1E302U3eX +IfS6J8Zp6Bs1baubHXFifXU6SV805b6i46/1m99OPsVH85zCUHvu4asaiLcR +=qIjw +-----END PGP PUBLIC KEY BLOCK-----`) diff --git a/app/plugin_signature.go b/app/plugin_signature.go new file mode 100644 index 0000000000..e05e93b341 --- /dev/null +++ b/app/plugin_signature.go @@ -0,0 +1,132 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package app + +import ( + "bytes" + "io" + "io/ioutil" + "net/http" + "path/filepath" + + "github.com/mattermost/mattermost-server/mlog" + "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/utils" + "github.com/pkg/errors" + "golang.org/x/crypto/openpgp" + "golang.org/x/crypto/openpgp/armor" +) + +// GetPluginPublicKeyFiles returns all public keys listed in the config. +func (a *App) GetPluginPublicKeyFiles() ([]string, *model.AppError) { + return a.Config().PluginSettings.SignaturePublicKeyFiles, nil +} + +// GetPublicKey will return the actual public key saved in the `name` file. +func (a *App) GetPublicKey(name string) ([]byte, *model.AppError) { + data, err := a.Srv.configStore.GetFile(name) + if err != nil { + return nil, model.NewAppError("GetPublicKey", "app.plugin.get_public_key.get_file.app_error", nil, err.Error(), http.StatusInternalServerError) + } + return data, nil +} + +// AddPublicKey will add plugin public key to the config. Overwrites the previous file +func (a *App) AddPublicKey(name string, key io.Reader) *model.AppError { + if model.IsSamlFile(&a.Config().SamlSettings, name) { + return model.NewAppError("AddPublicKey", "app.plugin.modify_saml.app_error", nil, "", http.StatusInternalServerError) + } + data, err := ioutil.ReadAll(key) + if err != nil { + return model.NewAppError("AddPublicKey", "app.plugin.write_file.read.app_error", nil, err.Error(), http.StatusInternalServerError) + } + err = a.Srv.configStore.SetFile(name, data) + if err != nil { + return model.NewAppError("AddPublicKey", "app.plugin.write_file.saving.app_error", nil, err.Error(), http.StatusInternalServerError) + } + + a.UpdateConfig(func(cfg *model.Config) { + if !utils.StringInSlice(name, cfg.PluginSettings.SignaturePublicKeyFiles) { + cfg.PluginSettings.SignaturePublicKeyFiles = append(cfg.PluginSettings.SignaturePublicKeyFiles, name) + } + }) + + return nil +} + +// DeletePublicKey will delete plugin public key from the config. +func (a *App) DeletePublicKey(name string) *model.AppError { + if model.IsSamlFile(&a.Config().SamlSettings, name) { + return model.NewAppError("AddPublicKey", "app.plugin.modify_saml.app_error", nil, "", http.StatusInternalServerError) + } + filename := filepath.Base(name) + if err := a.Srv.configStore.RemoveFile(filename); err != nil { + return model.NewAppError("DeletePublicKey", "app.plugin.delete_public_key.delete.app_error", nil, err.Error(), http.StatusInternalServerError) + } + + a.UpdateConfig(func(cfg *model.Config) { + cfg.PluginSettings.SignaturePublicKeyFiles = utils.RemoveStringFromSlice(filename, cfg.PluginSettings.SignaturePublicKeyFiles) + }) + + return nil +} + +// VerifyPlugin checks that the given signature corresponds to the given plugin and matches a trusted certificate. +func (a *App) VerifyPlugin(plugin, signature io.ReadSeeker) *model.AppError { + if err := verifySignature(bytes.NewReader(mattermostPluginPublicKey), plugin, signature); err == nil { + return nil + } + publicKeys, appErr := a.GetPluginPublicKeyFiles() + if appErr != nil { + return appErr + } + for _, pk := range publicKeys { + pkBytes, appErr := a.GetPublicKey(pk) + if appErr != nil { + mlog.Error("Unable to get public key for ", mlog.String("filename", pk)) + continue + } + publicKey := bytes.NewReader(pkBytes) + plugin.Seek(0, 0) + signature.Seek(0, 0) + if err := verifySignature(publicKey, plugin, signature); err == nil { + return nil + } + } + return model.NewAppError("VerifyPlugin", "api.plugin.verify_plugin.app_error", nil, "", http.StatusInternalServerError) +} + +func verifySignature(publicKey, message, signatrue io.Reader) error { + pk, err := decodeIfArmored(publicKey) + if err != nil { + return errors.Wrap(err, "can't decode public key") + } + s, err := decodeIfArmored(signatrue) + if err != nil { + return errors.Wrap(err, "can't decode signature") + } + return verifyBinarySignature(pk, message, s) +} + +func verifyBinarySignature(publicKey, signedFile, signature io.Reader) error { + keyring, err := openpgp.ReadKeyRing(publicKey) + if err != nil { + return errors.Wrap(err, "can't read public key") + } + if _, err = openpgp.CheckDetachedSignature(keyring, signedFile, signature); err != nil { + return errors.Wrap(err, "error while checking the signature") + } + return nil +} +func decodeIfArmored(reader io.Reader) (io.Reader, error) { + readBytes, err := ioutil.ReadAll(reader) + if err != nil { + return nil, errors.Wrap(err, "can't read the file") + } + block, err := armor.Decode(bytes.NewReader(readBytes)) + if err != nil { + return bytes.NewReader(readBytes), nil + } + return block.Body, nil +} diff --git a/app/plugin_signature_test.go b/app/plugin_signature_test.go new file mode 100644 index 0000000000..c1ae5a8ca9 --- /dev/null +++ b/app/plugin_signature_test.go @@ -0,0 +1,102 @@ +// Copyright (c) 2017-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package app + +import ( + "io/ioutil" + "os" + "path/filepath" + "testing" + + "github.com/mattermost/mattermost-server/utils/fileutils" + "github.com/stretchr/testify/require" +) + +func TestPluginPublicKeys(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + path, _ := fileutils.FindDir("tests") + publicKeyFilename := "test-public-key.plugin.gpg" + publicKey, err := ioutil.ReadFile(filepath.Join(path, publicKeyFilename)) + require.Nil(t, err) + fileReader, err := os.Open(filepath.Join(path, publicKeyFilename)) + require.Nil(t, err) + defer fileReader.Close() + th.App.AddPublicKey(publicKeyFilename, fileReader) + file, err := th.App.GetPublicKey(publicKeyFilename) + require.Nil(t, err) + require.Equal(t, publicKey, file) + _, err = th.App.GetPublicKey("wrong file name") + require.NotNil(t, err) + _, err = th.App.GetPublicKey("wrong-file-name.plugin.gpg") + require.NotNil(t, err) + + err = th.App.DeletePublicKey("wrong file name") + require.Nil(t, err) + err = th.App.DeletePublicKey("wrong-file-name.plugin.gpg") + require.Nil(t, err) + + err = th.App.DeletePublicKey(publicKeyFilename) + require.Nil(t, err) + _, err = th.App.GetPublicKey(publicKeyFilename) + require.NotNil(t, err) +} + +func TestVerifySignature(t *testing.T) { + path, _ := fileutils.FindDir("tests") + pluginFilename := "testplugin.tar.gz" + signatureFilename := "testplugin.tar.gz.sig" + armoredSignatureFilename := "testplugin.tar.gz.asc" + publicKeyFilename := "development-public-key.gpg" + armoredPublicKeyFilename := "development-public-key.asc" + t.Run("verify armored signature and armored public key", func(t *testing.T) { + publicKeyFileReader, err := os.Open(filepath.Join(path, armoredPublicKeyFilename)) + require.Nil(t, err) + defer publicKeyFileReader.Close() + pluginFileReader, err := os.Open(filepath.Join(path, pluginFilename)) + require.Nil(t, err) + defer pluginFileReader.Close() + signatureFileReader, err := os.Open(filepath.Join(path, armoredSignatureFilename)) + require.Nil(t, err) + defer signatureFileReader.Close() + require.Nil(t, verifySignature(publicKeyFileReader, pluginFileReader, signatureFileReader)) + }) + t.Run("verify non armored signature and armored public key", func(t *testing.T) { + publicKeyFileReader, err := os.Open(filepath.Join(path, armoredPublicKeyFilename)) + require.Nil(t, err) + defer publicKeyFileReader.Close() + pluginFileReader, err := os.Open(filepath.Join(path, pluginFilename)) + require.Nil(t, err) + defer pluginFileReader.Close() + signatureFileReader, err := os.Open(filepath.Join(path, signatureFilename)) + require.Nil(t, err) + defer signatureFileReader.Close() + require.Nil(t, verifySignature(publicKeyFileReader, pluginFileReader, signatureFileReader)) + }) + t.Run("verify armored signature and non armored public key", func(t *testing.T) { + publicKeyFileReader, err := os.Open(filepath.Join(path, publicKeyFilename)) + require.Nil(t, err) + defer publicKeyFileReader.Close() + pluginFileReader, err := os.Open(filepath.Join(path, pluginFilename)) + require.Nil(t, err) + defer pluginFileReader.Close() + armoredSignatureFileReader, err := os.Open(filepath.Join(path, armoredSignatureFilename)) + require.Nil(t, err) + defer armoredSignatureFileReader.Close() + require.Nil(t, verifySignature(publicKeyFileReader, pluginFileReader, armoredSignatureFileReader)) + }) + t.Run("verify non armored signature and non armored public key", func(t *testing.T) { + publicKeyFileReader, err := os.Open(filepath.Join(path, publicKeyFilename)) + require.Nil(t, err) + defer publicKeyFileReader.Close() + pluginFileReader, err := os.Open(filepath.Join(path, pluginFilename)) + require.Nil(t, err) + defer pluginFileReader.Close() + signatureFileReader, err := os.Open(filepath.Join(path, signatureFilename)) + require.Nil(t, err) + defer signatureFileReader.Close() + require.Nil(t, verifySignature(publicKeyFileReader, pluginFileReader, signatureFileReader)) + }) +} diff --git a/app/plugin_test.go b/app/plugin_test.go index aa2f5654ac..53df7bfd5c 100644 --- a/app/plugin_test.go +++ b/app/plugin_test.go @@ -483,7 +483,7 @@ func TestPluginSync(t *testing.T) { s3Port := os.Getenv("CI_MINIO_PORT") if s3Port == "" { - s3Port = "9001" + s3Port = "9000" } s3Endpoint := fmt.Sprintf("%s:%s", s3Host, s3Port) @@ -508,6 +508,7 @@ func TestPluginSync(t *testing.T) { *cfg.PluginSettings.Enable = true *cfg.PluginSettings.Directory = "./test-plugins" *cfg.PluginSettings.ClientDirectory = "./test-client-plugins" + *cfg.PluginSettings.RequirePluginSignature = false }) th.App.UpdateConfig(testCase.ConfigFunc) @@ -530,7 +531,7 @@ func TestPluginSync(t *testing.T) { // Check if installed pluginStatus, err := env.Statuses() require.Nil(t, err) - require.True(t, len(pluginStatus) == 1) + require.Len(t, pluginStatus, 1) require.Equal(t, pluginStatus[0].PluginId, "testplugin") // Bundle removed from the file store case @@ -543,7 +544,54 @@ func TestPluginSync(t *testing.T) { // Check if removed pluginStatus, err = env.Statuses() require.Nil(t, err) - require.True(t, len(pluginStatus) == 0) + require.Len(t, pluginStatus, 0) + + // RequirePluginSignature = true case + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.PluginSettings.RequirePluginSignature = true + }) + pluginFileReader, err := os.Open(filepath.Join(path, "testplugin.tar.gz")) + require.NoError(t, err) + defer pluginFileReader.Close() + _, appErr = th.App.WriteFile(pluginFileReader, th.App.getBundleStorePath("testplugin.tar.gz")) + checkNoError(t, appErr) + // no signature + appErr = th.App.SyncPlugins() + checkNoError(t, appErr) + pluginStatus, err = env.Statuses() + require.Nil(t, err) + require.Len(t, pluginStatus, 0) + + // Wrong signature + signatureFileReader, err := os.Open(filepath.Join(path, "testpluginv2.tar.gz.sig")) + require.NoError(t, err) + defer signatureFileReader.Close() + filePath := fmt.Sprintf("%s.sig", th.App.getBundleStorePath("testplugin")) + _, appErr = th.App.WriteFile(signatureFileReader, filePath) + checkNoError(t, appErr) + + appErr = th.App.SyncPlugins() + checkNoError(t, appErr) + + pluginStatus, err = env.Statuses() + require.Nil(t, err) + require.Len(t, pluginStatus, 0) + + // Correct signature + signatureFileReader, err = os.Open(filepath.Join(path, "testplugin.tar.gz.sig")) + require.NoError(t, err) + defer signatureFileReader.Close() + filePath = fmt.Sprintf("%s.sig", th.App.getBundleStorePath("testplugin")) + _, appErr = th.App.WriteFile(signatureFileReader, filePath) + checkNoError(t, appErr) + + appErr = th.App.SyncPlugins() + checkNoError(t, appErr) + + pluginStatus, err = env.Statuses() + require.Nil(t, err) + require.Len(t, pluginStatus, 1) + require.Equal(t, pluginStatus[0].PluginId, "testplugin") }) } } diff --git a/app/server.go b/app/server.go index daad2ed0d8..319e8cecdc 100644 --- a/app/server.go +++ b/app/server.go @@ -261,6 +261,21 @@ func NewServer(options ...Option) (*Server, error) { s.StartElasticsearch() } + s.AddConfigListener(func(oldConfig *model.Config, newConfig *model.Config) { + if *oldConfig.GuestAccountsSettings.Enable && !*newConfig.GuestAccountsSettings.Enable { + if appErr := s.FakeApp().DeactivateGuests(); appErr != nil { + mlog.Error("Unable to deactivate guest accounts", mlog.Err(appErr)) + } + } + }) + + // Disable active guest accounts on first run if guest accounts are disabled + if !*s.Config().GuestAccountsSettings.Enable { + if appErr := s.FakeApp().DeactivateGuests(); appErr != nil { + mlog.Error("Unable to deactivate guest accounts", mlog.Err(appErr)) + } + } + s.initJobs() if s.runjobs { diff --git a/app/team.go b/app/team.go index c8e2546a39..6357c08568 100644 --- a/app/team.go +++ b/app/team.go @@ -1333,10 +1333,19 @@ func (a *App) GetTeamIdFromQuery(query url.Values) (string, *model.AppError) { } func (a *App) SanitizeTeam(session model.Session, team *model.Team) *model.Team { - if !a.SessionHasPermissionToTeam(session, team.Id, model.PERMISSION_MANAGE_TEAM) { - team.Sanitize() + if a.SessionHasPermissionToTeam(session, team.Id, model.PERMISSION_MANAGE_TEAM) { + return team } + if a.SessionHasPermissionToTeam(session, team.Id, model.PERMISSION_INVITE_USER) { + inviteId := team.InviteId + team.Sanitize() + team.InviteId = inviteId + return team + } + + team.Sanitize() + return team } diff --git a/app/team_test.go b/app/team_test.go index e9b7cd5462..6755c2bb32 100644 --- a/app/team_test.go +++ b/app/team_test.go @@ -419,6 +419,7 @@ func TestSanitizeTeam(t *testing.T) { team := &model.Team{ Id: model.NewId(), Email: th.MakeEmail(), + InviteId: model.NewId(), AllowedDomains: "example.com", } @@ -443,6 +444,7 @@ func TestSanitizeTeam(t *testing.T) { sanitized := th.App.SanitizeTeam(session, copyTeam()) require.Empty(t, sanitized.Email, "should've sanitized team") + require.Empty(t, sanitized.InviteId, "should've sanitized inviteid") }) t.Run("user of the team", func(t *testing.T) { @@ -460,6 +462,7 @@ func TestSanitizeTeam(t *testing.T) { sanitized := th.App.SanitizeTeam(session, copyTeam()) require.Empty(t, sanitized.Email, "should've sanitized team") + require.NotEmpty(t, sanitized.InviteId, "should have not sanitized inviteid") }) t.Run("team admin", func(t *testing.T) { @@ -477,6 +480,7 @@ func TestSanitizeTeam(t *testing.T) { sanitized := th.App.SanitizeTeam(session, copyTeam()) require.NotEmpty(t, sanitized.Email, "shouldn't have sanitized team") + require.NotEmpty(t, sanitized.InviteId, "shouldn't have sanitized inviteid") }) t.Run("team admin of another team", func(t *testing.T) { @@ -494,6 +498,7 @@ func TestSanitizeTeam(t *testing.T) { sanitized := th.App.SanitizeTeam(session, copyTeam()) require.Empty(t, sanitized.Email, "should've sanitized team") + require.Empty(t, sanitized.InviteId, "should've sanitized inviteid") }) t.Run("system admin, not a user of team", func(t *testing.T) { @@ -511,6 +516,7 @@ func TestSanitizeTeam(t *testing.T) { sanitized := th.App.SanitizeTeam(session, copyTeam()) require.NotEmpty(t, sanitized.Email, "shouldn't have sanitized team") + require.NotEmpty(t, sanitized.InviteId, "shouldn't have sanitized inviteid") }) t.Run("system admin, user of team", func(t *testing.T) { @@ -528,6 +534,7 @@ func TestSanitizeTeam(t *testing.T) { sanitized := th.App.SanitizeTeam(session, copyTeam()) require.NotEmpty(t, sanitized.Email, "shouldn't have sanitized team") + require.NotEmpty(t, sanitized.InviteId, "shouldn't have sanitized inviteid") }) } diff --git a/app/user.go b/app/user.go index 878ce66a31..f491a0d841 100644 --- a/app/user.go +++ b/app/user.go @@ -943,28 +943,28 @@ func (a *App) UpdatePasswordAsUser(userId, currentPassword, newPassword string) return a.UpdatePasswordSendEmail(user, newPassword, T("api.user.update_password.menu")) } -func (a *App) userDeactivated(user *model.User) *model.AppError { - if err := a.RevokeAllSessions(user.Id); err != nil { +func (a *App) userDeactivated(userId string) *model.AppError { + if err := a.RevokeAllSessions(userId); err != nil { return err } - a.SetStatusOffline(user.Id, false) + a.SetStatusOffline(userId, false) if *a.Config().ServiceSettings.DisableBotsWhenOwnerIsDeactivated { - a.disableUserBots(user.Id) + a.disableUserBots(userId) } return nil } -func (a *App) invalidateUserChannelMembersCaches(user *model.User) *model.AppError { - teamsForUser, err := a.GetTeamsForUser(user.Id) +func (a *App) invalidateUserChannelMembersCaches(userId string) *model.AppError { + teamsForUser, err := a.GetTeamsForUser(userId) if err != nil { return err } for _, team := range teamsForUser { - channelsForUser, err := a.GetChannelsForUser(team.Id, user.Id, false) + channelsForUser, err := a.GetChannelsForUser(team.Id, userId, false) if err != nil { return err } @@ -992,12 +992,12 @@ func (a *App) UpdateActive(user *model.User, active bool) (*model.User, *model.A ruser := userUpdate.New if !active { - if err := a.userDeactivated(ruser); err != nil { + if err := a.userDeactivated(ruser.Id); err != nil { return nil, err } } - a.invalidateUserChannelMembersCaches(user) + a.invalidateUserChannelMembersCaches(user.Id) a.InvalidateCacheForUser(user.Id) a.sendUpdatedUserEvent(*ruser) @@ -1005,6 +1005,27 @@ func (a *App) UpdateActive(user *model.User, active bool) (*model.User, *model.A return ruser, nil } +func (a *App) DeactivateGuests() *model.AppError { + userIds, err := a.Srv.Store.User().DeactivateGuests() + if err != nil { + return err + } + + for _, userId := range userIds { + if err := a.userDeactivated(userId); err != nil { + return err + } + } + + a.Srv.Store.Channel().ClearCaches() + a.Srv.Store.User().ClearCaches() + + message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_GUESTS_DEACTIVATED, "", "", "", nil) + a.Publish(message) + + return nil +} + func (a *App) GetSanitizeOptions(asAdmin bool) map[string]bool { options := a.Config().GetSanitizeOptions() if asAdmin { diff --git a/app/user_test.go b/app/user_test.go index f0316bfcb9..d8cf99bd80 100644 --- a/app/user_test.go +++ b/app/user_test.go @@ -1164,3 +1164,27 @@ func TestDemoteUserToGuest(t *testing.T) { assert.Len(t, *channelMembers, 3) }) } + +func TestDeactivateGuests(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + guest1 := th.CreateGuest() + guest2 := th.CreateGuest() + user := th.CreateUser() + + err := th.App.DeactivateGuests() + require.Nil(t, err) + + guest1, err = th.App.GetUser(guest1.Id) + assert.Nil(t, err) + assert.NotEqual(t, int64(0), guest1.DeleteAt) + + guest2, err = th.App.GetUser(guest2.Id) + assert.Nil(t, err) + assert.NotEqual(t, int64(0), guest2.DeleteAt) + + user, err = th.App.GetUser(user.Id) + assert.Nil(t, err) + assert.Equal(t, int64(0), user.DeleteAt) +} diff --git a/build/Jenkinsfile.pr b/build/Jenkinsfile.pr index 8a0fa14567..b18a91cba9 100644 --- a/build/Jenkinsfile.pr +++ b/build/Jenkinsfile.pr @@ -100,6 +100,8 @@ pipeline { ansiColor('xterm') { sh """ cd /go/src/github.com/mattermost/mattermost-server + echo "Installing golangci-lint" + curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh| sh -s -- -b /usr/local/bin v1.21.0 make config-reset make check-style BUILD_NUMBER='${BRANCH_NAME}-${BUILD_NUMBER}' make build BUILD_NUMBER='${BRANCH_NAME}-${BUILD_NUMBER}' diff --git a/cmd/mattermost/commands/plugin.go b/cmd/mattermost/commands/plugin.go index 810e972ad9..9d53deda0a 100644 --- a/cmd/mattermost/commands/plugin.go +++ b/cmd/mattermost/commands/plugin.go @@ -4,9 +4,12 @@ package commands import ( - "errors" + "net/http" "os" + "path/filepath" + "github.com/mattermost/mattermost-server/model" + "github.com/pkg/errors" "github.com/spf13/cobra" ) @@ -55,14 +58,46 @@ var PluginListCmd = &cobra.Command{ RunE: pluginListCmdF, } +var PluginPublicKeysCmd = &cobra.Command{ + Use: "keys", + Short: "List public keys", + Long: "List names of all public keys installed on your Mattermost server.", + Example: ` plugin keys + plugin keys --verbose`, + RunE: pluginPublicKeysCmdF, +} + +var PluginAddPublicKeyCmd = &cobra.Command{ + Use: "add [keys]", + Short: "Adds public key(s)", + Long: "Adds public key(s) for plugins on your Mattermost server.", + Example: ` plugin keys add my-pk-file1 my-pk-file2`, + RunE: pluginAddPublicKeyCmdF, +} + +var PluginDeletePublicKeyCmd = &cobra.Command{ + Use: "delete [keys]", + Short: "Deletes public key(s)", + Long: "Deletes public key(s) for plugins on your Mattermost server.", + Example: ` plugin keys delete my-pk-file1 my-pk-file2`, + RunE: pluginDeletePublicKeyCmdF, +} + func init() { + PluginPublicKeysCmd.Flags().Bool("verbose", false, "List names and details of all public keys installed on your Mattermost server.") + PluginPublicKeysCmd.AddCommand( + PluginAddPublicKeyCmd, + PluginDeletePublicKeyCmd, + ) PluginCmd.AddCommand( PluginAddCmd, PluginDeleteCmd, PluginEnableCmd, PluginDisableCmd, PluginListCmd, + PluginPublicKeysCmd, ) + RootCmd.AddCommand(PluginCmd) } @@ -169,7 +204,7 @@ func pluginListCmdF(command *cobra.Command, args []string) error { pluginsResp, appErr := a.GetPlugins() if appErr != nil { - return errors.New("Unable to list plugins. Error: " + appErr.Error()) + return errors.Wrap(appErr, "Unable to list plugins.") } CommandPrettyPrintln("Listing active plugins") @@ -184,3 +219,88 @@ func pluginListCmdF(command *cobra.Command, args []string) error { return nil } + +func pluginPublicKeysCmdF(command *cobra.Command, args []string) error { + a, err := InitDBCommandContextCobra(command) + if err != nil { + return err + } + defer a.Shutdown() + + verbose, err := command.Flags().GetBool("verbose") + if err != nil { + return errors.Wrap(err, "Failed reading verbose flag.") + } + + pluginPublicKeysResp, appErr := a.GetPluginPublicKeyFiles() + if appErr != nil { + return errors.Wrap(appErr, "Unable to list public keys.") + } + + if verbose { + for _, publicKey := range pluginPublicKeysResp { + key, err := a.GetPublicKey(publicKey) + if err != nil { + CommandPrintErrorln("Unable to get plugin public key: " + publicKey + ". Error: " + err.Error()) + } + CommandPrettyPrintln("Plugin name: " + publicKey + ". \nPublic key: \n" + string(key) + "\n") + } + } else { + for _, publicKey := range pluginPublicKeysResp { + CommandPrettyPrintln(publicKey) + } + } + + return nil +} + +func pluginAddPublicKeyCmdF(command *cobra.Command, args []string) error { + a, err := InitDBCommandContextCobra(command) + if err != nil { + return err + } + defer a.Shutdown() + + if len(args) < 1 { + return errors.New("Expected at least one argument. See help text for details.") + } + + for _, pkFile := range args { + filename := filepath.Base(pkFile) + fileReader, err := os.Open(pkFile) + if err != nil { + return model.NewAppError("AddPublicKey", "api.plugin.add_public_key.open.app_error", nil, err.Error(), http.StatusInternalServerError) + } + defer fileReader.Close() + + if err := a.AddPublicKey(filename, fileReader); err != nil { + CommandPrintErrorln("Unable to add public key: " + pkFile + ". Error: " + err.Error()) + } else { + CommandPrettyPrintln("Added public key: " + pkFile) + } + } + + return nil +} + +func pluginDeletePublicKeyCmdF(command *cobra.Command, args []string) error { + a, err := InitDBCommandContextCobra(command) + if err != nil { + return err + } + defer a.Shutdown() + + if len(args) < 1 { + return errors.New("Expected at least one argument. See help text for details.") + } + + for _, pkFile := range args { + if err := a.DeletePublicKey(pkFile); err != nil { + CommandPrintErrorln("Unable to delete public key: " + pkFile + ". Error: " + err.Error()) + } else { + CommandPrettyPrintln("Deleted public key: " + pkFile) + } + } + + return nil +} diff --git a/cmd/mattermost/commands/plugin_test.go b/cmd/mattermost/commands/plugin_test.go index 4f79c9154b..4f0dd7f13b 100644 --- a/cmd/mattermost/commands/plugin_test.go +++ b/cmd/mattermost/commands/plugin_test.go @@ -1,3 +1,5 @@ +// Copyright (c) 2017-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. package commands import ( @@ -44,3 +46,75 @@ func TestPlugin(t *testing.T) { th.CheckCommand(t, "plugin", "delete", "testplugin") } + +func TestPluginPublicKeys(t *testing.T) { + th := Setup().InitBasic() + defer th.TearDown() + + cfg := th.Config() + cfg.PluginSettings.SignaturePublicKeyFiles = []string{"public-key"} + th.SetConfig(cfg) + + output := th.CheckCommand(t, "plugin", "keys") + assert.Contains(t, output, "public-key") + assert.NotContains(t, output, "Plugin name:") +} + +func TestPluginPublicKeyDetails(t *testing.T) { + th := Setup().InitBasic() + defer th.TearDown() + + cfg := th.Config() + cfg.PluginSettings.SignaturePublicKeyFiles = []string{"public-key"} + + th.SetConfig(cfg) + + output := th.CheckCommand(t, "plugin", "keys", "--verbose", "true") + assert.Contains(t, output, "Plugin name: public-key") + output = th.CheckCommand(t, "plugin", "keys", "--verbose") + assert.Contains(t, output, "Plugin name: public-key") +} + +func TestAddPluginPublicKeys(t *testing.T) { + th := Setup().InitBasic() + defer th.TearDown() + + cfg := th.Config() + cfg.PluginSettings.SignaturePublicKeyFiles = []string{"public-key"} + th.SetConfig(cfg) + + err := th.RunCommand(t, "plugin", "keys", "add", "pk1") + assert.NotNil(t, err) +} + +func TestDeletePluginPublicKeys(t *testing.T) { + th := Setup().InitBasic() + defer th.TearDown() + + cfg := th.Config() + cfg.PluginSettings.SignaturePublicKeyFiles = []string{"pk1"} + th.SetConfig(cfg) + + output := th.CheckCommand(t, "plugin", "keys", "delete", "pk1") + assert.Contains(t, output, "Deleted public key: pk1") +} + +func TestPluginPublicKeysFlow(t *testing.T) { + th := Setup().InitBasic() + defer th.TearDown() + + path, _ := fileutils.FindDir("tests") + name := "test-public-key.plugin.gpg" + output := th.CheckCommand(t, "plugin", "keys", "add", filepath.Join(path, name)) + assert.Contains(t, output, "Added public key: "+filepath.Join(path, name)) + + output = th.CheckCommand(t, "plugin", "keys") + assert.Contains(t, output, name) + assert.NotContains(t, output, "Plugin name:") + + output = th.CheckCommand(t, "plugin", "keys", "--verbose") + assert.Contains(t, output, "Plugin name: "+name) + + output = th.CheckCommand(t, "plugin", "keys", "delete", name) + assert.Contains(t, output, "Deleted public key: "+name) +} diff --git a/config/client.go b/config/client.go index a0f47f86a4..a6fed84bd0 100644 --- a/config/client.go +++ b/config/client.go @@ -20,6 +20,7 @@ func GenerateClientConfig(c *model.Config, diagnosticId string, license *model.L props["RestrictDirectMessage"] = *c.TeamSettings.RestrictDirectMessage props["EnableXToLeaveChannelsFromLHS"] = strconv.FormatBool(*c.TeamSettings.EnableXToLeaveChannelsFromLHS) props["TeammateNameDisplay"] = *c.TeamSettings.TeammateNameDisplay + props["LockTeammateNameDisplay"] = strconv.FormatBool(*c.TeamSettings.LockTeammateNameDisplay) props["ExperimentalPrimaryTeam"] = *c.TeamSettings.ExperimentalPrimaryTeam props["ExperimentalViewArchivedChannels"] = strconv.FormatBool(*c.TeamSettings.ExperimentalViewArchivedChannels) diff --git a/config/migrate.go b/config/migrate.go index a39cd10d59..720a6ebba5 100644 --- a/config/migrate.go +++ b/config/migrate.go @@ -24,6 +24,8 @@ func Migrate(from, to string) error { files := []string{*sourceConfig.SamlSettings.IdpCertificateFile, *sourceConfig.SamlSettings.PublicCertificateFile, *sourceConfig.SamlSettings.PrivateKeyFile} + files = append(files, sourceConfig.PluginSettings.SignaturePublicKeyFiles...) + for _, file := range files { err = migrateFile(file, source, destination) diff --git a/i18n/en.json b/i18n/en.json index c0a8161d9f..1089b4ce3b 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -1340,6 +1340,10 @@ "id": "api.file.write_file_locally.writing.app_error", "translation": "Encountered an error writing to local server storage" }, + { + "id": "api.getGroups.invalid_or_missing_channel_or_team_id", + "translation": "Invalid/Missing channel ID or Team ID." + }, { "id": "api.incoming_webhook.disabled.app_error", "translation": "Incoming webhooks have been disabled by the system admin." @@ -1508,6 +1512,10 @@ "id": "api.outgoing_webhook.disabled.app_error", "translation": "Outgoing webhooks have been disabled by the system admin." }, + { + "id": "api.plugin.add_public_key.open.app_error", + "translation": "An error occurred while opening the public key file." + }, { "id": "api.plugin.install.download_failed.app_error", "translation": "An error occurred while downloading the plugin." @@ -1536,6 +1544,10 @@ "id": "api.plugin.upload.no_file.app_error", "translation": "Missing file in multipart/form request" }, + { + "id": "api.plugin.verify_plugin.app_error", + "translation": "Unable to verify plugin signature." + }, { "id": "api.post.check_for_out_of_channel_groups_mentions.message.multiple", "translation": "@{{.Usernames}} and @{{.LastUsername}} did not get notified by this mention because they are not in the channel. They cannot be added to the channel because they are not a member of the linked groups. To add them to this channel, they must be added to the linked groups." @@ -1702,6 +1714,14 @@ "id": "api.push_notification.disabled.app_error", "translation": "Push Notifications are disabled on this server." }, + { + "id": "api.push_notification.id_loaded.default_message", + "translation": "You've received a new message." + }, + { + "id": "api.push_notification.id_loaded.fetch.app_error", + "translation": "An error occurred fetching the ID-loaded push notification" + }, { "id": "api.push_notifications_ack.forward.app_error", "translation": "An error occurred sending the receipt delivery to the push notification service" @@ -2626,6 +2646,10 @@ "id": "api.user.send_welcome_email_and_forget.failed.error", "translation": "Failed to send welcome email successfully" }, + { + "id": "api.user.update_active.cannot_enable_guest_when_guest_feature_is_disabled.app_error", + "translation": "You cannot activate a guest account because Guest Access feature is not enabled." + }, { "id": "api.user.update_active.not_enable.app_error", "translation": "You cannot deactivate yourself because this feature is not enabled. Please contact your System Administrator." @@ -3462,6 +3486,10 @@ "id": "app.plugin.deactivate.app_error", "translation": "Unable to deactivate plugin" }, + { + "id": "app.plugin.delete_public_key.delete.app_error", + "translation": "An error occurred while deleting the public key." + }, { "id": "app.plugin.disabled.app_error", "translation": "Plugins have been disabled. Please check your logs for details." @@ -3486,6 +3514,10 @@ "id": "app.plugin.get_plugins.app_error", "translation": "Unable to get active plugins" }, + { + "id": "app.plugin.get_public_key.get_file.app_error", + "translation": "An error occurred while getting the public key from the store." + }, { "id": "app.plugin.get_statuses.app_error", "translation": "Unable to get plugin statuses" @@ -3506,6 +3538,10 @@ "id": "app.plugin.invalid_id.app_error", "translation": "Plugin Id must be at least {{.Min}} characters, at most {{.Max}} characters and match {{.Regex}}." }, + { + "id": "app.plugin.invalid_version.app_error", + "translation": "Plugin version could not be parsed" + }, { "id": "app.plugin.manifest.app_error", "translation": "Unable to find manifest for extracted plugin" @@ -3518,14 +3554,26 @@ "id": "app.plugin.marketplace_disabled.app_error", "translation": "Marketplace has been disabled. Please check your logs for details." }, + { + "id": "app.plugin.marketplace_plugin_request.app_error", + "translation": "Failed to decode the marketplace plugin request." + }, { "id": "app.plugin.marketplace_plugins.app_error", "translation": "Failed to get plugins from the marketplace server." }, + { + "id": "app.plugin.marketplace_plugins.not_found.app_error", + "translation": "Plugin not found." + }, { "id": "app.plugin.marshal.app_error", "translation": "Failed to marshal marketplace plugins." }, + { + "id": "app.plugin.modify_saml.app_error", + "translation": "Can't modify saml files." + }, { "id": "app.plugin.mvdir.app_error", "translation": "Unable to move plugin from temporary directory to final destination. Another plugin may be using the same directory name." @@ -3546,10 +3594,18 @@ "id": "app.plugin.restart.app_error", "translation": "Unable to restart plugin on upgrade." }, + { + "id": "app.plugin.signature_decode.app_error", + "translation": "Unable to decode base64 signature." + }, { "id": "app.plugin.store_bundle.app_error", "translation": "Unable to store the plugin to the configured file store." }, + { + "id": "app.plugin.store_signature.app_error", + "translation": "Unable to store the plugin signature to the configured file store." + }, { "id": "app.plugin.sync.list_filestore.app_error", "translation": "Error reading files from the plugins folder in the file store." @@ -3566,6 +3622,14 @@ "id": "app.plugin.webapp_bundle.app_error", "translation": "Unable to generate plugin webapp bundle." }, + { + "id": "app.plugin.write_file.read.app_error", + "translation": "An error occurred while reading the file." + }, + { + "id": "app.plugin.write_file.saving.app_error", + "translation": "An error occurred while saving the file." + }, { "id": "app.role.check_roles_exist.role_not_found", "translation": "The provided role does not exist" @@ -7162,6 +7226,14 @@ "id": "store.sql_user.update.username_taken.app_error", "translation": "This username is already taken. Please choose another." }, + { + "id": "store.sql_user.update_active_for_multiple_users.getting_changed_users.app_error", + "translation": "Unable to get the list of deactivate guests ids" + }, + { + "id": "store.sql_user.update_active_for_multiple_users.updating.app_error", + "translation": "Unable to deactivate guests" + }, { "id": "store.sql_user.update_auth_data.app_error", "translation": "Unable to update the auth data" diff --git a/model/bot.go b/model/bot.go index 18d64fec53..362c439ec7 100644 --- a/model/bot.go +++ b/model/bot.go @@ -8,6 +8,7 @@ import ( "fmt" "io" "net/http" + "strings" "unicode/utf8" ) @@ -217,3 +218,15 @@ func (l *BotList) Etag() string { func MakeBotNotFoundError(userId string) *AppError { return NewAppError("SqlBotStore.Get", "store.sql_bot.get.missing.app_error", map[string]interface{}{"user_id": userId}, "", http.StatusNotFound) } + +func IsBotDMChannel(channel *Channel, botUserID string) bool { + if channel.Type != CHANNEL_DIRECT { + return false + } + + if !strings.HasPrefix(channel.Name, botUserID+"__") && !strings.HasSuffix(channel.Name, "__"+botUserID) { + return false + } + + return true +} diff --git a/model/bot_test.go b/model/bot_test.go index 9b1e8a3cb2..3a7a0db9fc 100644 --- a/model/bot_test.go +++ b/model/bot_test.go @@ -683,3 +683,45 @@ func TestBotListEtag(t *testing.T) { }) } } + +func TestIsBotChannel(t *testing.T) { + for _, test := range []struct { + Name string + Channel *Channel + Expected bool + }{ + { + Name: "not a direct channel", + Channel: &Channel{Type: CHANNEL_OPEN}, + Expected: false, + }, + { + Name: "a direct channel with another user", + Channel: &Channel{ + Name: "user1__user2", + Type: CHANNEL_DIRECT, + }, + Expected: false, + }, + { + Name: "a direct channel with the name containing the bot's ID first", + Channel: &Channel{ + Name: "botUserID__user2", + Type: CHANNEL_DIRECT, + }, + Expected: true, + }, + { + Name: "a direct channel with the name containing the bot's ID second", + Channel: &Channel{ + Name: "user1__botUserID", + Type: CHANNEL_DIRECT, + }, + Expected: true, + }, + } { + t.Run(test.Name, func(t *testing.T) { + assert.Equal(t, test.Expected, IsBotDMChannel(test.Channel, "botUserID")) + }) + } +} diff --git a/model/client4.go b/model/client4.go index 09b753f481..54457b6d3c 100644 --- a/model/client4.go +++ b/model/client4.go @@ -4472,6 +4472,21 @@ func (c *Client4) InstallPluginFromUrl(downloadUrl string, force bool) (*Manifes return ManifestFromJson(r.Body), BuildResponse(r) } +// InstallMarketplacePlugin will install marketplace plugin. +// WARNING: PLUGINS ARE STILL EXPERIMENTAL. THIS FUNCTION IS SUBJECT TO CHANGE. +func (c *Client4) InstallMarketplacePlugin(request *InstallMarketplacePluginRequest) (*Manifest, *Response) { + json, err := request.ToJson() + if err != nil { + return nil, &Response{Error: NewAppError("InstallMarketplacePlugin", "model.client.plugin_request_to_json.app_error", nil, err.Error(), http.StatusBadRequest)} + } + r, appErr := c.DoApiPost(c.GetPluginsRoute()+"/marketplace", json) + if appErr != nil { + return nil, BuildErrorResponse(r, appErr) + } + defer closeBody(r) + return ManifestFromJson(r.Body), BuildResponse(r) +} + // GetPlugins will return a list of plugin manifests for currently active plugins. // WARNING: PLUGINS ARE STILL EXPERIMENTAL. THIS FUNCTION IS SUBJECT TO CHANGE. func (c *Client4) GetPlugins() (*PluginsResponse, *Response) { diff --git a/model/cluster_message.go b/model/cluster_message.go index 04d8673ceb..4ce99843ef 100644 --- a/model/cluster_message.go +++ b/model/cluster_message.go @@ -24,6 +24,9 @@ const ( CLUSTER_EVENT_CLEAR_SESSION_CACHE_FOR_USER = "clear_session_user" CLUSTER_EVENT_INVALIDATE_CACHE_FOR_ROLES = "inv_roles" CLUSTER_EVENT_INVALIDATE_CACHE_FOR_SCHEMES = "inv_schemes" + CLUSTER_EVENT_INVALIDATE_CACHE_FOR_EMOJIS_BY_ID = "inv_emojis_by_id" + CLUSTER_EVENT_INVALIDATE_CACHE_FOR_EMOJIS_ID_BY_NAME = "inv_emojis_id_by_name" + CLUSTER_EVENT_INVALIDATE_CACHE_FOR_CHANNEL_MEMBER_COUNTS = "inv_channel_member_counts" CLUSTER_EVENT_CLEAR_SESSION_CACHE_FOR_ALL_USERS = "inv_all_user_sessions" CLUSTER_EVENT_INSTALL_PLUGIN = "install_plugin" CLUSTER_EVENT_REMOVE_PLUGIN = "remove_plugin" diff --git a/model/config.go b/model/config.go index 482bbdf75d..079a01c593 100644 --- a/model/config.go +++ b/model/config.go @@ -48,6 +48,7 @@ const ( GENERIC_NOTIFICATION = "generic" GENERIC_NOTIFICATION_SERVER = "https://push-test.mattermost.com" FULL_NOTIFICATION = "full" + ID_LOADED_NOTIFICATION = "id_loaded" DIRECT_MESSAGE_ANY = "any" DIRECT_MESSAGE_TEAM = "team" @@ -1521,6 +1522,7 @@ type TeamSettings struct { ExperimentalEnableAutomaticReplies *bool ExperimentalHideTownSquareinLHS *bool ExperimentalTownSquareIsReadOnly *bool + LockTeammateNameDisplay *bool ExperimentalPrimaryTeam *string ExperimentalDefaultChannels []string } @@ -1667,6 +1669,10 @@ func (s *TeamSettings) SetDefaults() { if s.ExperimentalViewArchivedChannels == nil { s.ExperimentalViewArchivedChannels = NewBool(false) } + + if s.LockTeammateNameDisplay == nil { + s.LockTeammateNameDisplay = NewBool(false) + } } type ClientRequirements struct { @@ -2239,7 +2245,9 @@ type PluginSettings struct { Plugins map[string]map[string]interface{} PluginStates map[string]*PluginState EnableMarketplace *bool + RequirePluginSignature *bool MarketplaceUrl *string + SignaturePublicKeyFiles []string } func (s *PluginSettings) SetDefaults(ls LogSettings) { @@ -2287,6 +2295,14 @@ func (s *PluginSettings) SetDefaults(ls LogSettings) { if s.MarketplaceUrl == nil || *s.MarketplaceUrl == "" || *s.MarketplaceUrl == PLUGIN_SETTINGS_OLD_MARKETPLACE_URL { s.MarketplaceUrl = NewString(PLUGIN_SETTINGS_DEFAULT_MARKETPLACE_URL) } + + if s.RequirePluginSignature == nil { + s.RequirePluginSignature = NewBool(false) + } + + if s.SignaturePublicKeyFiles == nil { + s.SignaturePublicKeyFiles = []string{} + } } type GlobalRelayMessageExportSettings struct { diff --git a/model/config_test.go b/model/config_test.go index a63b17a8f2..79624f3591 100644 --- a/model/config_test.go +++ b/model/config_test.go @@ -170,7 +170,6 @@ func TestConfigIsValidFakeAlgorithm(t *testing.T) { require.Equal(t, "model.config.is_valid.saml_canonical_algorithm.app_error", err.Message) *c1.SamlSettings.CanonicalAlgorithm = temp - temp = *c1.SamlSettings.SignatureAlgorithm *c1.SamlSettings.SignatureAlgorithm = "Fake Algorithm" err = c1.SamlSettings.isValid() if err == nil { diff --git a/model/integration_action.go b/model/integration_action.go index 64898e242b..177c416445 100644 --- a/model/integration_action.go +++ b/model/integration_action.go @@ -44,6 +44,9 @@ type PostAction struct { // The text on the button, or in the select placeholder. Name string `json:"name,omitempty"` + // If the action is disabled. + Disabled bool `json:"disabled,omitempty"` + // DataSource indicates the data source for the select action. If left // empty, the select is populated from Options. Other supported values // are "users" and "channels". diff --git a/model/marketplace_plugin.go b/model/marketplace_plugin.go index 154bdde00d..b925f91184 100644 --- a/model/marketplace_plugin.go +++ b/model/marketplace_plugin.go @@ -4,18 +4,25 @@ package model import ( + "bytes" + "encoding/base64" "encoding/json" "io" "net/url" "strconv" + + "github.com/pkg/errors" ) // BaseMarketplacePlugin is a Mattermost plugin received from the marketplace server. type BaseMarketplacePlugin struct { - HomepageURL string `json:"homepage_url"` - DownloadURL string `json:"download_url"` - IconData string `json:"icon_data"` - Manifest *Manifest `json:"manifest"` + HomepageURL string `json:"homepage_url"` + IconData string `json:"icon_data"` + DownloadURL string `json:"download_url"` + ReleaseNotesURL string `json:"release_notes_url"` + // Signature represents a signature of a plugin saved in base64 encoding. + Signature string `json:"signature"` + Manifest *Manifest `json:"manifest"` } // MarketplacePlugin is a state aware marketplace plugin. @@ -48,6 +55,15 @@ func MarketplacePluginsFromReader(reader io.Reader) ([]*MarketplacePlugin, error return plugins, nil } +// DecodeSignature Decodes signature and returns ReadSeeker. +func (plugin *BaseMarketplacePlugin) DecodeSignature() (io.ReadSeeker, error) { + signatureBytes, err := base64.StdEncoding.DecodeString(plugin.Signature) + if err != nil { + return nil, errors.Wrap(err, "Unable to decode base64 signature.") + } + return bytes.NewReader(signatureBytes), nil +} + // MarketplacePluginFilter describes the parameters to request a list of plugins. type MarketplacePluginFilter struct { Page int @@ -67,3 +83,28 @@ func (filter *MarketplacePluginFilter) ApplyToURL(u *url.URL) { q.Add("server_version", filter.ServerVersion) u.RawQuery = q.Encode() } + +// InstallMarketplacePluginRequest struct describes parameters of the requested plugin. +type InstallMarketplacePluginRequest struct { + Id string `json:"id"` + Version string `json:"version"` +} + +// PluginRequestFromReader decodes a json-encoded plugin request from the given io.Reader. +func PluginRequestFromReader(reader io.Reader) (*InstallMarketplacePluginRequest, error) { + var r *InstallMarketplacePluginRequest + err := json.NewDecoder(reader).Decode(&r) + if err != nil { + return nil, err + } + return r, nil +} + +// ToJson method will return json from plugin request. +func (r *InstallMarketplacePluginRequest) ToJson() (string, error) { + b, err := json.Marshal(r) + if err != nil { + return "", err + } + return string(b), nil +} diff --git a/model/push_notification.go b/model/push_notification.go index e846f565d7..00eb3955f8 100644 --- a/model/push_notification.go +++ b/model/push_notification.go @@ -15,6 +15,7 @@ const ( PUSH_NOTIFY_APPLE_REACT_NATIVE = "apple_rn" PUSH_NOTIFY_ANDROID_REACT_NATIVE = "android_rn" + PUSH_TYPE_ID_LOADED = "id_loaded" PUSH_TYPE_MESSAGE = "message" PUSH_TYPE_CLEAR = "clear" PUSH_TYPE_UPDATE_BADGE = "update_badge" @@ -39,6 +40,7 @@ type PushNotificationAck struct { ClientReceivedAt int64 `json:"received_at"` ClientPlatform string `json:"platform"` NotificationType string `json:"type"` + PostId string `json:"post_id,omitempty"` } type PushNotification struct { @@ -46,23 +48,23 @@ type PushNotification struct { Platform string `json:"platform"` ServerId string `json:"server_id"` DeviceId string `json:"device_id"` - Category string `json:"category"` - Sound string `json:"sound"` - Message string `json:"message"` - Badge int `json:"badge"` - ContentAvailable int `json:"cont_ava"` - TeamId string `json:"team_id"` - ChannelId string `json:"channel_id"` PostId string `json:"post_id"` - RootId string `json:"root_id"` - ChannelName string `json:"channel_name"` - Type string `json:"type"` - SenderId string `json:"sender_id"` - SenderName string `json:"sender_name"` - OverrideUsername string `json:"override_username"` - OverrideIconUrl string `json:"override_icon_url"` - FromWebhook string `json:"from_webhook"` - Version string `json:"version"` + Category string `json:"category,omitempty"` + Sound string `json:"sound,omitempty"` + Message string `json:"message,omitempty"` + Badge int `json:"badge,omitempty"` + ContentAvailable int `json:"cont_ava,omitempty"` + TeamId string `json:"team_id,omitempty"` + ChannelId string `json:"channel_id,omitempty"` + RootId string `json:"root_id,omitempty"` + ChannelName string `json:"channel_name,omitempty"` + Type string `json:"type,omitempty"` + SenderId string `json:"sender_id,omitempty"` + SenderName string `json:"sender_name,omitempty"` + OverrideUsername string `json:"override_username,omitempty"` + OverrideIconUrl string `json:"override_icon_url,omitempty"` + FromWebhook string `json:"from_webhook,omitempty"` + Version string `json:"version,omitempty"` } func (me *PushNotification) ToJson() string { diff --git a/model/team.go b/model/team.go index b727db95d1..44abd88cda 100644 --- a/model/team.go +++ b/model/team.go @@ -268,6 +268,7 @@ func CleanTeamName(s string) string { func (o *Team) Sanitize() { o.Email = "" + o.InviteId = "" } func (t *Team) Patch(patch *TeamPatch) { diff --git a/model/utils.go b/model/utils.go index 039e756c82..ea7c7f0781 100644 --- a/model/utils.go +++ b/model/utils.go @@ -624,3 +624,8 @@ func GetPreferredTimezone(timezone StringMap) string { return timezone["manualTimezone"] } + +// IsSamlFile checks if filename is a SAML file. +func IsSamlFile(saml *SamlSettings, filename string) bool { + return filename == *saml.PublicCertificateFile || filename == *saml.PrivateKeyFile || filename == *saml.IdpCertificateFile +} diff --git a/model/websocket_message.go b/model/websocket_message.go index c259d8b49e..38273d2c48 100644 --- a/model/websocket_message.go +++ b/model/websocket_message.go @@ -52,6 +52,7 @@ const ( WEBSOCKET_EVENT_LICENSE_CHANGED = "license_changed" WEBSOCKET_EVENT_CONFIG_CHANGED = "config_changed" WEBSOCKET_EVENT_OPEN_DIALOG = "open_dialog" + WEBSOCKET_EVENT_GUESTS_DEACTIVATED = "guests_deactivated" ) type WebSocketMessage interface { diff --git a/plugin/api.go b/plugin/api.go index 69f5ca0b8a..4205a162f2 100644 --- a/plugin/api.go +++ b/plugin/api.go @@ -664,7 +664,7 @@ type API interface { // Minimum server version: 5.6 GetFileLink(fileId string) (string, *model.AppError) - // ReadFileAtPath reads the file from the backend for a specific path + // ReadFile reads the file from the backend for a specific path // // @tag File // Minimum server version: 5.3 diff --git a/plugin/client_rpc.go b/plugin/client_rpc.go index a902252df8..1eb8ce08da 100644 --- a/plugin/client_rpc.go +++ b/plugin/client_rpc.go @@ -525,7 +525,7 @@ func (s *hooksRPCServer) FileWillBeUploaded(args *Z_FileWillBeUploadedArgs, retu return nil } -// MessageWillBePosted is in this file because of the difficulty of identifiying which fields need special behaviour. +// MessageWillBePosted is in this file because of the difficulty of identifying which fields need special behaviour. // The special behaviour needed is decoding the returned post into the original one to avoid the unintentional removal // of fields by older plugins. func init() { @@ -565,8 +565,8 @@ func (s *hooksRPCServer) MessageWillBePosted(args *Z_MessageWillBePostedArgs, re return nil } -// MessageWillBeUpdated is in this file because of the difficulty of identifiying which fields need special behaviour. -// The special behavour needed is decoding the returned post into the original one to avoid the unintentional removal +// MessageWillBeUpdated is in this file because of the difficulty of identifying which fields need special behaviour. +// The special behaviour needed is decoding the returned post into the original one to avoid the unintentional removal // of fields by older plugins. func init() { hookNameToId["MessageWillBeUpdated"] = MessageWillBeUpdatedId diff --git a/plugin/helpers.go b/plugin/helpers.go index 5a2e4e33ca..31cc728d1c 100644 --- a/plugin/helpers.go +++ b/plugin/helpers.go @@ -14,10 +14,11 @@ import ( // 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. + // A profile image or icon image may be optionally passed in to be set for the existing or newly created bot. // Returns the id of the resulting bot. // // Minimum server version: 5.10 - EnsureBot(bot *model.Bot) (string, error) + EnsureBot(bot *model.Bot, options ...EnsureBotOption) (string, error) // KVSetJSON stores a key-value pair, unique per plugin, marshalling the given value as a JSON string. // @@ -56,6 +57,18 @@ type Helpers interface { // // Minimum server version: 5.6 KVSetWithExpiryJSON(key string, value interface{}, expireInSeconds int64) error + + // ShouldProcessMessage returns if the message should be processed by a message hook. + // + // Use this method to avoid processing unnecessary messages in a MessageHasBeenPosted + // or MessageWillBePosted hook, and indeed in some cases avoid an infinite loop between + // two automated bots or plugins. + // + // The behaviour is customizable using the given options, since plugin needs may vary. + // By default, system messages and messages from bots will be skipped. + // + // Minimum server version: 5.2 + ShouldProcessMessage(post *model.Post, options ...ShouldProcessMessageOption) (bool, error) } // HelpersImpl implements the helpers interface with an API that retrieves data on behalf of the plugin. diff --git a/plugin/helpers_bots.go b/plugin/helpers_bots.go index 44355a0397..142ccc49b3 100644 --- a/plugin/helpers_bots.go +++ b/plugin/helpers_bots.go @@ -4,19 +4,187 @@ package plugin import ( + "io/ioutil" + "path/filepath" + "github.com/pkg/errors" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/utils" ) +type ensureBotOptions struct { + ProfileImagePath string + IconImagePath string +} + +type EnsureBotOption func(*ensureBotOptions) + +func ProfileImagePath(path string) EnsureBotOption { + return func(args *ensureBotOptions) { + args.ProfileImagePath = path + } +} + +func IconImagePath(path string) EnsureBotOption { + return func(args *ensureBotOptions) { + args.IconImagePath = path + } +} + // EnsureBot implements Helpers.EnsureBot -func (p *HelpersImpl) EnsureBot(bot *model.Bot) (retBotID string, retErr error) { +func (p *HelpersImpl) EnsureBot(bot *model.Bot, options ...EnsureBotOption) (retBotID string, retErr error) { err := p.ensureServerVersion("5.10.0") if err != nil { return "", errors.Wrap(err, "failed to ensure bot") } + // Default options + o := &ensureBotOptions{ + ProfileImagePath: "", + IconImagePath: "", + } + + for _, setter := range options { + setter(o) + } + + botID, err := p.ensureBot(bot) + if err != nil { + return "", err + } + + err = p.setBotImages(botID, o.ProfileImagePath, o.IconImagePath) + if err != nil { + return "", err + } + return botID, nil +} + +type ShouldProcessMessageOption func(*shouldProcessMessageOptions) + +type shouldProcessMessageOptions struct { + AllowSystemMessages bool + AllowBots bool + FilterChannelIDs []string + FilterUserIDs []string + OnlyBotDMs bool +} + +// AllowSystemMessages configures a call to ShouldProcessMessage to return true for system messages. +// +// As it is typically desirable only to consume messages from users of the system, ShouldProcessMessage ignores system messages by default. +func AllowSystemMessages() ShouldProcessMessageOption { + return func(options *shouldProcessMessageOptions) { + options.AllowSystemMessages = true + } +} + +// AllowBots configures a call to ShouldProcessMessage to return true for bot posts. +// +// As it is typically desirable only to consume messages from human users of the system, ShouldProcessMessage ignores bot messages by default. When allowing bots, take care to avoid a loop where two plugins respond to each others posts repeatedly. +func AllowBots() ShouldProcessMessageOption { + return func(options *shouldProcessMessageOptions) { + options.AllowBots = true + } +} + +// FilterChannelIDs configures a call to ShouldProcessMessage to return true only for the given channels. +// +// By default, posts from all channels are allowed to be processed. +func FilterChannelIDs(filterChannelIDs []string) ShouldProcessMessageOption { + return func(options *shouldProcessMessageOptions) { + options.FilterChannelIDs = filterChannelIDs + } +} + +// FilterUserIDs configures a call to ShouldProcessMessage to return true only for the given users. +// +// By default, posts from all non-bot users are allowed. +func FilterUserIDs(filterUserIDs []string) ShouldProcessMessageOption { + return func(options *shouldProcessMessageOptions) { + options.FilterUserIDs = filterUserIDs + } +} + +// OnlyBotDMs configures a call to ShouldProcessMessage to return true only for direct messages sent to the bot created by EnsureBot. +// +// By default, posts from all channels are allowed. +func OnlyBotDMs() ShouldProcessMessageOption { + return func(options *shouldProcessMessageOptions) { + options.OnlyBotDMs = true + } +} + +// ShouldProcessMessage implements Helpers.ShouldProcessMessage +func (p *HelpersImpl) ShouldProcessMessage(post *model.Post, options ...ShouldProcessMessageOption) (bool, error) { + messageProcessOptions := &shouldProcessMessageOptions{} + for _, option := range options { + option(messageProcessOptions) + } + + botIdBytes, kvGetErr := p.API.KVGet(BOT_USER_KEY) + if kvGetErr != nil { + return false, errors.Wrap(kvGetErr, "failed to get bot") + } + + if botIdBytes != nil { + if post.UserId == string(botIdBytes) { + return false, nil + } + } + + if post.IsSystemMessage() && !messageProcessOptions.AllowSystemMessages { + return false, nil + } + + if !messageProcessOptions.AllowBots { + user, appErr := p.API.GetUser(post.UserId) + if appErr != nil { + return false, errors.Wrap(appErr, "unable to get user") + } + + if user.IsBot { + return false, nil + } + } + + if len(messageProcessOptions.FilterChannelIDs) != 0 && !utils.StringInSlice(post.ChannelId, messageProcessOptions.FilterChannelIDs) { + return false, nil + } + + if len(messageProcessOptions.FilterUserIDs) != 0 && !utils.StringInSlice(post.UserId, messageProcessOptions.FilterUserIDs) { + return false, nil + } + + if botIdBytes != nil && messageProcessOptions.OnlyBotDMs { + channel, appErr := p.API.GetChannel(post.ChannelId) + if appErr != nil { + return false, errors.Wrap(appErr, "unable to get channel") + } + + if !model.IsBotDMChannel(channel, string(botIdBytes)) { + return false, nil + } + } + + return true, nil +} + +func (p *HelpersImpl) readFile(path string) ([]byte, error) { + bundlePath, err := p.API.GetBundlePath() + if err != nil { + return nil, errors.Wrap(err, "failed to get bundle path") + } + + imageBytes, err := ioutil.ReadFile(filepath.Join(bundlePath, path)) + if err != nil { + return nil, errors.Wrap(err, "failed to read image") + } + return imageBytes, nil +} + +func (p *HelpersImpl) ensureBot(bot *model.Bot) (retBotID string, retErr error) { // Must provide a bot with a username if bot == nil || len(bot.Username) < 1 { return "", errors.New("passed a bad bot, nil or no username") @@ -79,3 +247,27 @@ func (p *HelpersImpl) EnsureBot(bot *model.Bot) (retBotID string, retErr error) return createdBot.UserId, nil } + +func (p *HelpersImpl) setBotImages(botID, profileImagePath, iconImagePath string) error { + if profileImagePath != "" { + imageBytes, err := p.readFile(profileImagePath) + if err != nil { + return errors.Wrap(err, "failed to read profile image") + } + appErr := p.API.SetProfileImage(botID, imageBytes) + if appErr != nil { + return errors.Wrap(appErr, "failed to set profile image") + } + } + if iconImagePath != "" { + imageBytes, err := p.readFile(iconImagePath) + if err != nil { + return errors.Wrap(err, "failed to read icon image") + } + appErr := p.API.SetBotIconImage(botID, imageBytes) + if appErr != nil { + return errors.Wrap(appErr, "failed to set icon image") + } + } + return nil +} diff --git a/plugin/helpers_bots_test.go b/plugin/helpers_bots_test.go index f3b985951a..8960e61efe 100644 --- a/plugin/helpers_bots_test.go +++ b/plugin/helpers_bots_test.go @@ -4,12 +4,15 @@ package plugin_test import ( + "io/ioutil" + "path/filepath" "testing" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/plugin" "github.com/mattermost/mattermost-server/plugin/plugintest" "github.com/mattermost/mattermost-server/plugin/plugintest/mock" + "github.com/mattermost/mattermost-server/utils/fileutils" "github.com/stretchr/testify/assert" ) @@ -95,6 +98,77 @@ func TestEnsureBot(t *testing.T) { assert.Equal(t, "", botId) assert.NotNil(t, err) }) + + t.Run("should set the bot profile image when specified", func(t *testing.T) { + expectedBotId := model.NewId() + api := setupAPI() + + testsDir, _ := fileutils.FindDir("tests") + testImage := filepath.Join(testsDir, "test.png") + imageBytes, err := ioutil.ReadFile(testImage) + + api.On("KVGet", plugin.BOT_USER_KEY).Return([]byte(expectedBotId), nil) + api.On("GetBundlePath").Return("", nil) + api.On("SetProfileImage", expectedBotId, imageBytes).Return(nil) + api.On("GetServerVersion").Return("5.10.0") + defer api.AssertExpectations(t) + + p := &plugin.HelpersImpl{} + p.API = api + + assert.Nil(t, err) + + botId, err := p.EnsureBot(testbot, plugin.ProfileImagePath(testImage)) + assert.Equal(t, expectedBotId, botId) + assert.Nil(t, err) + }) + + t.Run("should set the bot icon image when specified", func(t *testing.T) { + expectedBotId := model.NewId() + api := setupAPI() + + testsDir, _ := fileutils.FindDir("tests") + testImage := filepath.Join(testsDir, "test.png") + imageBytes, err := ioutil.ReadFile(testImage) + assert.Nil(t, err) + + api.On("KVGet", plugin.BOT_USER_KEY).Return([]byte(expectedBotId), nil) + api.On("GetBundlePath").Return("", nil) + api.On("SetBotIconImage", expectedBotId, imageBytes).Return(nil) + api.On("GetServerVersion").Return("5.10.0") + defer api.AssertExpectations(t) + + p := &plugin.HelpersImpl{} + p.API = api + + botId, err := p.EnsureBot(testbot, plugin.IconImagePath(testImage)) + assert.Equal(t, expectedBotId, botId) + assert.Nil(t, err) + }) + + t.Run("should set both the profile image and bot icon image when specified", func(t *testing.T) { + expectedBotId := model.NewId() + api := setupAPI() + + testsDir, _ := fileutils.FindDir("tests") + testImage := filepath.Join(testsDir, "test.png") + imageBytes, err := ioutil.ReadFile(testImage) + assert.Nil(t, err) + + api.On("KVGet", plugin.BOT_USER_KEY).Return([]byte(expectedBotId), nil) + api.On("GetBundlePath").Return("", nil) + api.On("SetProfileImage", expectedBotId, imageBytes).Return(nil) + api.On("SetBotIconImage", expectedBotId, imageBytes).Return(nil) + api.On("GetServerVersion").Return("5.10.0") + defer api.AssertExpectations(t) + + p := &plugin.HelpersImpl{} + p.API = api + + botId, err := p.EnsureBot(testbot, plugin.ProfileImagePath(testImage), plugin.IconImagePath(testImage)) + assert.Equal(t, expectedBotId, botId) + assert.Nil(t, err) + }) }) t.Run("if bot doesn't exist", func(t *testing.T) { @@ -179,5 +253,198 @@ func TestEnsureBot(t *testing.T) { assert.Equal(t, "", botId) assert.NotNil(t, err) }) + + t.Run("should create bot and set the bot profile image when specified", func(t *testing.T) { + expectedBotId := model.NewId() + api := setupAPI() + + testsDir, _ := fileutils.FindDir("tests") + testImage := filepath.Join(testsDir, "test.png") + imageBytes, err := ioutil.ReadFile(testImage) + assert.Nil(t, err) + + api.On("KVGet", plugin.BOT_USER_KEY).Return(nil, nil) + api.On("GetUserByUsername", testbot.Username).Return(nil, nil) + api.On("CreateBot", testbot).Return(&model.Bot{ + UserId: expectedBotId, + }, nil) + api.On("KVSet", plugin.BOT_USER_KEY, []byte(expectedBotId)).Return(nil) + api.On("GetBundlePath").Return("", nil) + api.On("SetProfileImage", expectedBotId, imageBytes).Return(nil) + api.On("GetServerVersion").Return("5.10.0") + defer api.AssertExpectations(t) + + p := &plugin.HelpersImpl{} + p.API = api + + botId, err := p.EnsureBot(testbot, plugin.ProfileImagePath(testImage)) + assert.Equal(t, expectedBotId, botId) + assert.Nil(t, err) + }) + + t.Run("should create bot and set the bot icon image when specified", func(t *testing.T) { + expectedBotId := model.NewId() + api := setupAPI() + + testsDir, _ := fileutils.FindDir("tests") + testImage := filepath.Join(testsDir, "test.png") + imageBytes, err := ioutil.ReadFile(testImage) + assert.Nil(t, err) + + api.On("KVGet", plugin.BOT_USER_KEY).Return(nil, nil) + api.On("GetUserByUsername", testbot.Username).Return(nil, nil) + api.On("CreateBot", testbot).Return(&model.Bot{ + UserId: expectedBotId, + }, nil) + api.On("KVSet", plugin.BOT_USER_KEY, []byte(expectedBotId)).Return(nil) + api.On("GetBundlePath").Return("", nil) + api.On("SetBotIconImage", expectedBotId, imageBytes).Return(nil) + api.On("GetServerVersion").Return("5.10.0") + defer api.AssertExpectations(t) + + p := &plugin.HelpersImpl{} + p.API = api + + botId, err := p.EnsureBot(testbot, plugin.IconImagePath(testImage)) + assert.Equal(t, expectedBotId, botId) + assert.Nil(t, err) + }) + + t.Run("should create bot and set both the profile image and bot icon image when specified", func(t *testing.T) { + expectedBotId := model.NewId() + api := setupAPI() + + testsDir, _ := fileutils.FindDir("tests") + testImage := filepath.Join(testsDir, "test.png") + imageBytes, err := ioutil.ReadFile(testImage) + assert.Nil(t, err) + + api.On("KVGet", plugin.BOT_USER_KEY).Return(nil, nil) + api.On("GetUserByUsername", testbot.Username).Return(nil, nil) + api.On("CreateBot", testbot).Return(&model.Bot{ + UserId: expectedBotId, + }, nil) + api.On("KVSet", plugin.BOT_USER_KEY, []byte(expectedBotId)).Return(nil) + api.On("GetBundlePath").Return("", nil) + api.On("SetProfileImage", expectedBotId, imageBytes).Return(nil) + api.On("SetBotIconImage", expectedBotId, imageBytes).Return(nil) + api.On("GetServerVersion").Return("5.10.0") + defer api.AssertExpectations(t) + + p := &plugin.HelpersImpl{} + p.API = api + + botId, err := p.EnsureBot(testbot, plugin.ProfileImagePath(testImage), plugin.IconImagePath(testImage)) + assert.Equal(t, expectedBotId, botId) + assert.Nil(t, err) + }) + }) +} + +func TestShouldProcessMessage(t *testing.T) { + p := &plugin.HelpersImpl{} + expectedBotId := model.NewId() + + setupAPI := func() *plugintest.API { + return &plugintest.API{} + } + + t.Run("should not respond to itself", func(t *testing.T) { + api := setupAPI() + api.On("KVGet", plugin.BOT_USER_KEY).Return([]byte(expectedBotId), nil) + p.API = api + shouldProcessMessage, _ := p.ShouldProcessMessage(&model.Post{Type: model.POST_HEADER_CHANGE, UserId: expectedBotId}, plugin.AllowSystemMessages(), plugin.AllowBots()) + + assert.False(t, shouldProcessMessage) + }) + + t.Run("should not process as the post is generated by system", func(t *testing.T) { + shouldProcessMessage, _ := p.ShouldProcessMessage(&model.Post{Type: model.POST_HEADER_CHANGE}) + + assert.False(t, shouldProcessMessage) + }) + + t.Run("should not process as the post is sent to another channel", func(t *testing.T) { + channelID := "channel-id" + api := setupAPI() + api.On("GetChannel", channelID).Return(&model.Channel{Id: channelID, Type: model.CHANNEL_GROUP}, nil) + p.API = api + api.On("KVGet", plugin.BOT_USER_KEY).Return([]byte(expectedBotId), nil) + + shouldProcessMessage, _ := p.ShouldProcessMessage(&model.Post{ChannelId: channelID}, plugin.AllowSystemMessages(), plugin.AllowBots(), plugin.FilterChannelIDs([]string{"another-channel-id"})) + + assert.False(t, shouldProcessMessage) + }) + + t.Run("should not process as the post is created by bot", func(t *testing.T) { + userID := "user-id" + channelID := "1" + api := setupAPI() + p.API = api + api.On("GetUser", userID).Return(&model.User{IsBot: true}, nil) + api.On("KVGet", plugin.BOT_USER_KEY).Return([]byte(expectedBotId), nil) + + shouldProcessMessage, _ := p.ShouldProcessMessage(&model.Post{UserId: userID, ChannelId: channelID}, + plugin.AllowSystemMessages(), plugin.FilterUserIDs([]string{"another-user-id"})) + + assert.False(t, shouldProcessMessage) + }) + + t.Run("should not process the message as the post is not in bot dm channel", func(t *testing.T) { + userID := "user-id" + channelID := "1" + channel := model.Channel{ + Name: "user1__" + expectedBotId, + Type: model.CHANNEL_OPEN, + } + api := setupAPI() + api.On("GetChannel", channelID).Return(&channel, nil) + api.On("KVGet", plugin.BOT_USER_KEY).Return([]byte(expectedBotId), nil) + p.API = api + + shouldProcessMessage, _ := p.ShouldProcessMessage(&model.Post{UserId: userID, ChannelId: channelID}, plugin.AllowSystemMessages(), plugin.AllowBots(), plugin.OnlyBotDMs()) + + assert.False(t, shouldProcessMessage) + }) + + t.Run("should process the message", func(t *testing.T) { + channelID := "1" + api := setupAPI() + api.On("KVGet", plugin.BOT_USER_KEY).Return([]byte(expectedBotId), nil) + p.API = api + + shouldProcessMessage, _ := p.ShouldProcessMessage(&model.Post{UserId: "1", Type: model.POST_HEADER_CHANGE, ChannelId: channelID}, + plugin.AllowSystemMessages(), plugin.FilterChannelIDs([]string{channelID}), plugin.AllowBots(), plugin.FilterUserIDs([]string{"1"})) + + assert.True(t, shouldProcessMessage) + }) + + t.Run("should process the message for plugin without a bot", func(t *testing.T) { + channelID := "1" + api := setupAPI() + api.On("KVGet", plugin.BOT_USER_KEY).Return(nil, nil) + p.API = api + + shouldProcessMessage, _ := p.ShouldProcessMessage(&model.Post{UserId: "1", Type: model.POST_HEADER_CHANGE, ChannelId: channelID}, + plugin.AllowSystemMessages(), plugin.FilterChannelIDs([]string{channelID}), plugin.AllowBots(), plugin.FilterUserIDs([]string{"1"})) + + assert.True(t, shouldProcessMessage) + }) + + t.Run("should process the message when filter channel and filter users list is empty", func(t *testing.T) { + channelID := "1" + api := setupAPI() + channel := model.Channel{ + Name: "user1__" + expectedBotId, + Type: model.CHANNEL_DIRECT, + } + api.On("GetChannel", channelID).Return(&channel, nil) + api.On("KVGet", plugin.BOT_USER_KEY).Return([]byte(expectedBotId), nil) + p.API = api + + shouldProcessMessage, _ := p.ShouldProcessMessage(&model.Post{UserId: "1", Type: model.POST_HEADER_CHANGE, ChannelId: channelID}, + plugin.AllowSystemMessages(), plugin.AllowBots()) + + assert.True(t, shouldProcessMessage) }) } diff --git a/plugin/plugintest/helpers.go b/plugin/plugintest/helpers.go index 066c0b2b8c..e8839b8185 100644 --- a/plugin/plugintest/helpers.go +++ b/plugin/plugintest/helpers.go @@ -6,6 +6,7 @@ package plugintest import ( model "github.com/mattermost/mattermost-server/model" + plugin "github.com/mattermost/mattermost-server/plugin" mock "github.com/stretchr/testify/mock" ) @@ -14,20 +15,27 @@ type Helpers struct { mock.Mock } -// EnsureBot provides a mock function with given fields: bot -func (_m *Helpers) EnsureBot(bot *model.Bot) (string, error) { - ret := _m.Called(bot) +// EnsureBot provides a mock function with given fields: bot, options +func (_m *Helpers) EnsureBot(bot *model.Bot, options ...plugin.EnsureBotOption) (string, error) { + _va := make([]interface{}, len(options)) + for _i := range options { + _va[_i] = options[_i] + } + var _ca []interface{} + _ca = append(_ca, bot) + _ca = append(_ca, _va...) + ret := _m.Called(_ca...) var r0 string - if rf, ok := ret.Get(0).(func(*model.Bot) string); ok { - r0 = rf(bot) + if rf, ok := ret.Get(0).(func(*model.Bot, ...plugin.EnsureBotOption) string); ok { + r0 = rf(bot, options...) } else { r0 = ret.Get(0).(string) } var r1 error - if rf, ok := ret.Get(1).(func(*model.Bot) error); ok { - r1 = rf(bot) + if rf, ok := ret.Get(1).(func(*model.Bot, ...plugin.EnsureBotOption) error); ok { + r1 = rf(bot, options...) } else { r1 = ret.Error(1) } @@ -125,3 +133,31 @@ func (_m *Helpers) KVSetWithExpiryJSON(key string, value interface{}, expireInSe return r0 } + +// ShouldProcessMessage provides a mock function with given fields: post, options +func (_m *Helpers) ShouldProcessMessage(post *model.Post, options ...plugin.ShouldProcessMessageOption) (bool, error) { + _va := make([]interface{}, len(options)) + for _i := range options { + _va[_i] = options[_i] + } + var _ca []interface{} + _ca = append(_ca, post) + _ca = append(_ca, _va...) + ret := _m.Called(_ca...) + + var r0 bool + if rf, ok := ret.Get(0).(func(*model.Post, ...plugin.ShouldProcessMessageOption) bool); ok { + r0 = rf(post, options...) + } else { + r0 = ret.Get(0).(bool) + } + + var r1 error + if rf, ok := ret.Get(1).(func(*model.Post, ...plugin.ShouldProcessMessageOption) error); ok { + r1 = rf(post, options...) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} diff --git a/services/marketplace/client.go b/services/marketplace/client.go index 9b03dd8c50..b6860125cb 100644 --- a/services/marketplace/client.go +++ b/services/marketplace/client.go @@ -62,6 +62,19 @@ func (c *Client) GetPlugins(request *model.MarketplacePluginFilter) ([]*model.Ba } } +func (c *Client) GetPlugin(filter *model.MarketplacePluginFilter, pluginVersion string) (*model.BaseMarketplacePlugin, error) { + plugins, err := c.GetPlugins(filter) + if err != nil { + return nil, err + } + for _, plugin := range plugins { + if plugin.Manifest.Version == pluginVersion { + return plugin, nil + } + } + return nil, errors.New("plugin not found") +} + // closeBody ensures the Body of an http.Response is properly closed. func closeBody(r *http.Response) { if r.Body != nil { diff --git a/store/localcachelayer/channel_layer.go b/store/localcachelayer/channel_layer.go new file mode 100644 index 0000000000..4ced8f8c42 --- /dev/null +++ b/store/localcachelayer/channel_layer.go @@ -0,0 +1,65 @@ +// Copyright (c) 2017-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package localcachelayer + +import ( + "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/store" +) + +type LocalCacheChannelStore struct { + store.ChannelStore + rootStore *LocalCacheStore +} + +func (s *LocalCacheChannelStore) handleClusterInvalidateChannelMemberCounts(msg *model.ClusterMessage) { + if msg.Data == CLEAR_CACHE_MESSAGE_DATA { + s.rootStore.channelMemberCountsCache.Purge() + } else { + s.rootStore.channelMemberCountsCache.Remove(msg.Data) + } +} + +func (s LocalCacheChannelStore) ClearCaches() { + s.rootStore.doClearCacheCluster(s.rootStore.channelMemberCountsCache) + s.ChannelStore.ClearCaches() + if s.rootStore.metrics != nil { + s.rootStore.metrics.IncrementMemCacheInvalidationCounter("Channel Member Counts - Purge") + } +} + +func (s LocalCacheChannelStore) InvalidateMemberCount(channelId string) { + s.rootStore.doInvalidateCacheCluster(s.rootStore.channelMemberCountsCache, channelId) + if s.rootStore.metrics != nil { + s.rootStore.metrics.IncrementMemCacheInvalidationCounter("Channel Member Counts - Remove by ChannelId") + } +} + +func (s LocalCacheChannelStore) GetMemberCount(channelId string, allowFromCache bool) (int64, *model.AppError) { + if allowFromCache { + if count := s.rootStore.doStandardReadCache(s.rootStore.channelMemberCountsCache, channelId); count != nil { + return count.(int64), nil + } + } + count, err := s.ChannelStore.GetMemberCount(channelId, allowFromCache) + + if allowFromCache && err == nil { + s.rootStore.doStandardAddToCache(s.rootStore.channelMemberCountsCache, channelId, count) + } + + return count, err +} + +func (s LocalCacheChannelStore) GetMemberCountFromCache(channelId string) int64 { + if count := s.rootStore.doStandardReadCache(s.rootStore.channelMemberCountsCache, channelId); count != nil { + return count.(int64) + } + + count, err := s.GetMemberCount(channelId, true) + if err != nil { + return 0 + } + + return count +} diff --git a/store/localcachelayer/channel_layer_test.go b/store/localcachelayer/channel_layer_test.go new file mode 100644 index 0000000000..91396ffec3 --- /dev/null +++ b/store/localcachelayer/channel_layer_test.go @@ -0,0 +1,88 @@ +package localcachelayer + +import ( + "testing" + + "github.com/mattermost/mattermost-server/store/storetest" + "github.com/mattermost/mattermost-server/store/storetest/mocks" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestChannelStore(t *testing.T) { + StoreTest(t, storetest.TestReactionStore) +} + +func TestChannelStoreChannelMemberCountsCache(t *testing.T) { + countResult := int64(10) + + t.Run("first call not cached, second cached and returning same data", func(t *testing.T) { + mockStore := getMockStore() + cachedStore := NewLocalCacheLayer(mockStore, nil, nil) + + count, err := cachedStore.Channel().GetMemberCount("id", true) + require.Nil(t, err) + assert.Equal(t, count, countResult) + mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetMemberCount", 1) + count, err = cachedStore.Channel().GetMemberCount("id", true) + require.Nil(t, err) + assert.Equal(t, count, countResult) + mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetMemberCount", 1) + }) + + t.Run("first call not cached, second force no cached", func(t *testing.T) { + mockStore := getMockStore() + cachedStore := NewLocalCacheLayer(mockStore, nil, nil) + + cachedStore.Channel().GetMemberCount("id", true) + mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetMemberCount", 1) + cachedStore.Channel().GetMemberCount("id", false) + mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetMemberCount", 2) + }) + + t.Run("first call force no cached, second not cached, third cached", func(t *testing.T) { + mockStore := getMockStore() + cachedStore := NewLocalCacheLayer(mockStore, nil, nil) + + cachedStore.Channel().GetMemberCount("id", false) + mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetMemberCount", 1) + cachedStore.Channel().GetMemberCount("id", true) + mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetMemberCount", 2) + cachedStore.Channel().GetMemberCount("id", true) + mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetMemberCount", 2) + }) + + t.Run("first call with GetMemberCountFromCache not cached, second cached and returning same data", func(t *testing.T) { + mockStore := getMockStore() + cachedStore := NewLocalCacheLayer(mockStore, nil, nil) + + count := cachedStore.Channel().GetMemberCountFromCache("id") + assert.Equal(t, count, countResult) + mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetMemberCount", 1) + count = cachedStore.Channel().GetMemberCountFromCache("id") + assert.Equal(t, count, countResult) + mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetMemberCount", 1) + }) + + t.Run("first call not cached, clear cache, second call not cached", func(t *testing.T) { + mockStore := getMockStore() + cachedStore := NewLocalCacheLayer(mockStore, nil, nil) + + cachedStore.Channel().GetMemberCount("id", true) + mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetMemberCount", 1) + cachedStore.Channel().ClearCaches() + cachedStore.Channel().GetMemberCount("id", true) + mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetMemberCount", 2) + }) + + t.Run("first call not cached, invalidate cache, second call not cached", func(t *testing.T) { + mockStore := getMockStore() + cachedStore := NewLocalCacheLayer(mockStore, nil, nil) + + cachedStore.Channel().GetMemberCount("id", true) + mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetMemberCount", 1) + cachedStore.Channel().InvalidateMemberCount("id") + cachedStore.Channel().GetMemberCount("id", true) + mockStore.Channel().(*mocks.ChannelStore).AssertNumberOfCalls(t, "GetMemberCount", 2) + }) +} diff --git a/store/localcachelayer/emoji_layer.go b/store/localcachelayer/emoji_layer.go new file mode 100644 index 0000000000..be6afb65c7 --- /dev/null +++ b/store/localcachelayer/emoji_layer.go @@ -0,0 +1,100 @@ +// Copyright (c) 2017-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package localcachelayer + +import ( + "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/store" +) + +type LocalCacheEmojiStore struct { + store.EmojiStore + rootStore *LocalCacheStore +} + +func (es *LocalCacheEmojiStore) handleClusterInvalidateEmojiById(msg *model.ClusterMessage) { + if msg.Data == CLEAR_CACHE_MESSAGE_DATA { + es.rootStore.emojiCacheById.Purge() + } else { + es.rootStore.emojiCacheById.Remove(msg.Data) + } +} + +func (es *LocalCacheEmojiStore) handleClusterInvalidateEmojiIdByName(msg *model.ClusterMessage) { + if msg.Data == CLEAR_CACHE_MESSAGE_DATA { + es.rootStore.emojiIdCacheByName.Purge() + } else { + es.rootStore.emojiIdCacheByName.Remove(msg.Data) + } +} + +func (es LocalCacheEmojiStore) Get(id string, allowFromCache bool) (*model.Emoji, *model.AppError) { + if allowFromCache { + if emoji, ok := es.getFromCacheById(id); ok { + return emoji, nil + } + } + + emoji, err := es.EmojiStore.Get(id, allowFromCache) + + if allowFromCache && err == nil { + es.addToCache(emoji) + } + + return emoji, err +} + +func (es LocalCacheEmojiStore) GetByName(name string, allowFromCache bool) (*model.Emoji, *model.AppError) { + if id, ok := model.GetSystemEmojiId(name); ok { + return es.Get(id, allowFromCache) + } + + if allowFromCache { + if emoji, ok := es.getFromCacheByName(name); ok { + return emoji, nil + } + } + + emoji, err := es.EmojiStore.GetByName(name, allowFromCache) + + if allowFromCache && err == nil { + es.addToCache(emoji) + } + + return emoji, err +} + +func (es LocalCacheEmojiStore) Delete(emoji *model.Emoji, time int64) *model.AppError { + err := es.EmojiStore.Delete(emoji, time) + + if err == nil { + es.removeFromCache(emoji) + } + + return err +} + +func (es LocalCacheEmojiStore) addToCache(emoji *model.Emoji) { + es.rootStore.doStandardAddToCache(es.rootStore.emojiCacheById, emoji.Id, emoji) + es.rootStore.doStandardAddToCache(es.rootStore.emojiIdCacheByName, emoji.Name, emoji.Id) +} + +func (es LocalCacheEmojiStore) getFromCacheById(id string) (*model.Emoji, bool) { + if emoji := es.rootStore.doStandardReadCache(es.rootStore.emojiCacheById, id); emoji != nil { + return emoji.(*model.Emoji), true + } + return nil, false +} + +func (es LocalCacheEmojiStore) getFromCacheByName(name string) (*model.Emoji, bool) { + if emojiId := es.rootStore.doStandardReadCache(es.rootStore.emojiIdCacheByName, name); emojiId != nil { + return es.getFromCacheById(emojiId.(string)) + } + return nil, false +} + +func (es LocalCacheEmojiStore) removeFromCache(emoji *model.Emoji) { + es.rootStore.doInvalidateCacheCluster(es.rootStore.emojiCacheById, emoji.Id) + es.rootStore.doInvalidateCacheCluster(es.rootStore.emojiIdCacheByName, emoji.Name) +} diff --git a/store/localcachelayer/emoji_layer_test.go b/store/localcachelayer/emoji_layer_test.go new file mode 100644 index 0000000000..3aec28ae29 --- /dev/null +++ b/store/localcachelayer/emoji_layer_test.go @@ -0,0 +1,136 @@ +// Copyright (c) 2017-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package localcachelayer + +import ( + "testing" + + "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/store/storetest" + "github.com/mattermost/mattermost-server/store/storetest/mocks" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestEmojiStore(t *testing.T) { + StoreTest(t, storetest.TestEmojiStore) +} + +func TestEmojiStoreCache(t *testing.T) { + fakeEmoji := model.Emoji{Id: "123", Name: "name123"} + + t.Run("first call by id not cached, second cached and returning same data", func(t *testing.T) { + mockStore := getMockStore() + cachedStore := NewLocalCacheLayer(mockStore, nil, nil) + + emoji, err := cachedStore.Emoji().Get("123", true) + require.Nil(t, err) + assert.Equal(t, emoji, &fakeEmoji) + mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "Get", 1) + emoji, err = cachedStore.Emoji().Get("123", true) + require.Nil(t, err) + assert.Equal(t, emoji, &fakeEmoji) + mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "Get", 1) + }) + + t.Run("first call by name not cached, second cached and returning same data", func(t *testing.T) { + mockStore := getMockStore() + cachedStore := NewLocalCacheLayer(mockStore, nil, nil) + + emoji, err := cachedStore.Emoji().GetByName("name123", true) + require.Nil(t, err) + assert.Equal(t, emoji, &fakeEmoji) + mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "GetByName", 1) + emoji, err = cachedStore.Emoji().GetByName("name123", true) + require.Nil(t, err) + assert.Equal(t, emoji, &fakeEmoji) + mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "GetByName", 1) + }) + + t.Run("first call by id not cached, second force no cached", func(t *testing.T) { + mockStore := getMockStore() + cachedStore := NewLocalCacheLayer(mockStore, nil, nil) + + cachedStore.Emoji().Get("123", true) + mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "Get", 1) + cachedStore.Emoji().Get("123", false) + mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "Get", 2) + }) + + t.Run("first call by name not cached, second force no cached", func(t *testing.T) { + mockStore := getMockStore() + cachedStore := NewLocalCacheLayer(mockStore, nil, nil) + + cachedStore.Emoji().GetByName("name123", true) + mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "GetByName", 1) + cachedStore.Emoji().GetByName("name123", false) + mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "GetByName", 2) + }) + + t.Run("first call by id force no cached, second not cached, third cached", func(t *testing.T) { + mockStore := getMockStore() + cachedStore := NewLocalCacheLayer(mockStore, nil, nil) + + cachedStore.Emoji().Get("123", false) + mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "Get", 1) + cachedStore.Emoji().Get("123", true) + mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "Get", 2) + cachedStore.Emoji().Get("123", true) + mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "Get", 2) + }) + + t.Run("first call by id force no cached, second not cached, third cached", func(t *testing.T) { + mockStore := getMockStore() + cachedStore := NewLocalCacheLayer(mockStore, nil, nil) + + cachedStore.Emoji().GetByName("name123", false) + mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "GetByName", 1) + cachedStore.Emoji().GetByName("name123", true) + mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "GetByName", 2) + cachedStore.Emoji().GetByName("name123", true) + mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "GetByName", 2) + }) + + t.Run("first call by id, second call by name cached", func(t *testing.T) { + mockStore := getMockStore() + cachedStore := NewLocalCacheLayer(mockStore, nil, nil) + + cachedStore.Emoji().Get("123", true) + mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "Get", 1) + cachedStore.Emoji().GetByName("name123", true) + mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "GetByName", 0) + }) + + t.Run("first call by name, second call by id cached", func(t *testing.T) { + mockStore := getMockStore() + cachedStore := NewLocalCacheLayer(mockStore, nil, nil) + + cachedStore.Emoji().GetByName("name123", true) + mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "GetByName", 1) + cachedStore.Emoji().Get("123", true) + mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "Get", 0) + }) + + t.Run("first call by id not cached, invalidate, and then not cached again", func(t *testing.T) { + mockStore := getMockStore() + cachedStore := NewLocalCacheLayer(mockStore, nil, nil) + + cachedStore.Emoji().Get("123", true) + mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "Get", 1) + cachedStore.Emoji().Delete(&fakeEmoji, 0) + cachedStore.Emoji().Get("123", true) + mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "Get", 2) + }) + + t.Run("first call by name not cached, invalidate, and then not cached again", func(t *testing.T) { + mockStore := getMockStore() + cachedStore := NewLocalCacheLayer(mockStore, nil, nil) + + cachedStore.Emoji().GetByName("name123", true) + mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "GetByName", 1) + cachedStore.Emoji().Delete(&fakeEmoji, 0) + cachedStore.Emoji().GetByName("name123", true) + mockStore.Emoji().(*mocks.EmojiStore).AssertNumberOfCalls(t, "GetByName", 2) + }) +} diff --git a/store/localcachelayer/layer.go b/store/localcachelayer/layer.go index b799d5d163..83973aedb6 100644 --- a/store/localcachelayer/layer.go +++ b/store/localcachelayer/layer.go @@ -20,19 +20,30 @@ const ( SCHEME_CACHE_SIZE = 20000 SCHEME_CACHE_SEC = 30 * 60 + EMOJI_CACHE_SIZE = 5000 + EMOJI_CACHE_SEC = 30 * 60 + + CHANNEL_MEMBERS_COUNTS_CACHE_SIZE = model.CHANNEL_CACHE_SIZE + CHANNEL_MEMBERS_COUNTS_CACHE_SEC = 30 * 60 + CLEAR_CACHE_MESSAGE_DATA = "" ) type LocalCacheStore struct { store.Store - metrics einterfaces.MetricsInterface - cluster einterfaces.ClusterInterface - reaction LocalCacheReactionStore - reactionCache *utils.Cache - role LocalCacheRoleStore - roleCache *utils.Cache - scheme LocalCacheSchemeStore - schemeCache *utils.Cache + metrics einterfaces.MetricsInterface + cluster einterfaces.ClusterInterface + reaction LocalCacheReactionStore + reactionCache *utils.Cache + role LocalCacheRoleStore + roleCache *utils.Cache + scheme LocalCacheSchemeStore + schemeCache *utils.Cache + emoji LocalCacheEmojiStore + emojiCacheById *utils.Cache + emojiIdCacheByName *utils.Cache + channel LocalCacheChannelStore + channelMemberCountsCache *utils.Cache } func NewLocalCacheLayer(baseStore store.Store, metrics einterfaces.MetricsInterface, cluster einterfaces.ClusterInterface) LocalCacheStore { @@ -47,11 +58,19 @@ func NewLocalCacheLayer(baseStore store.Store, metrics einterfaces.MetricsInterf localCacheStore.role = LocalCacheRoleStore{RoleStore: baseStore.Role(), rootStore: &localCacheStore} localCacheStore.schemeCache = utils.NewLruWithParams(SCHEME_CACHE_SIZE, "Scheme", SCHEME_CACHE_SEC, model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_SCHEMES) localCacheStore.scheme = LocalCacheSchemeStore{SchemeStore: baseStore.Scheme(), rootStore: &localCacheStore} + localCacheStore.emojiCacheById = utils.NewLruWithParams(EMOJI_CACHE_SIZE, "EmojiById", EMOJI_CACHE_SEC, model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_EMOJIS_BY_ID) + localCacheStore.emojiIdCacheByName = utils.NewLruWithParams(EMOJI_CACHE_SIZE, "EmojiByName", EMOJI_CACHE_SEC, model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_EMOJIS_ID_BY_NAME) + localCacheStore.emoji = LocalCacheEmojiStore{EmojiStore: baseStore.Emoji(), rootStore: &localCacheStore} + localCacheStore.channelMemberCountsCache = utils.NewLruWithParams(CHANNEL_MEMBERS_COUNTS_CACHE_SIZE, "ChannelMemberCounts", CHANNEL_MEMBERS_COUNTS_CACHE_SEC, model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_CHANNEL_MEMBER_COUNTS) + localCacheStore.channel = LocalCacheChannelStore{ChannelStore: baseStore.Channel(), rootStore: &localCacheStore} if cluster != nil { cluster.RegisterClusterMessageHandler(model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_REACTIONS, localCacheStore.reaction.handleClusterInvalidateReaction) cluster.RegisterClusterMessageHandler(model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_ROLES, localCacheStore.role.handleClusterInvalidateRole) cluster.RegisterClusterMessageHandler(model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_SCHEMES, localCacheStore.scheme.handleClusterInvalidateScheme) + cluster.RegisterClusterMessageHandler(model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_EMOJIS_BY_ID, localCacheStore.emoji.handleClusterInvalidateEmojiById) + cluster.RegisterClusterMessageHandler(model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_EMOJIS_ID_BY_NAME, localCacheStore.emoji.handleClusterInvalidateEmojiIdByName) + cluster.RegisterClusterMessageHandler(model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_CHANNEL_MEMBER_COUNTS, localCacheStore.channel.handleClusterInvalidateChannelMemberCounts) } return localCacheStore } @@ -68,6 +87,14 @@ func (s LocalCacheStore) Scheme() store.SchemeStore { return s.scheme } +func (s LocalCacheStore) Emoji() store.EmojiStore { + return s.emoji +} + +func (s LocalCacheStore) Channel() store.ChannelStore { + return s.channel +} + func (s LocalCacheStore) DropAllTables() { s.Invalidate() s.Store.DropAllTables() @@ -118,4 +145,7 @@ func (s *LocalCacheStore) doClearCacheCluster(cache *utils.Cache) { func (s *LocalCacheStore) Invalidate() { s.doClearCacheCluster(s.reactionCache) + s.doClearCacheCluster(s.emojiCacheById) + s.doClearCacheCluster(s.emojiIdCacheByName) + s.doClearCacheCluster(s.channelMemberCountsCache) } diff --git a/store/localcachelayer/main_test.go b/store/localcachelayer/main_test.go index 6022826e81..b9ad5b20cf 100644 --- a/store/localcachelayer/main_test.go +++ b/store/localcachelayer/main_test.go @@ -41,6 +41,22 @@ func getMockStore() *mocks.Store { mockSchemesStore.On("PermanentDeleteAll").Return(nil) mockStore.On("Scheme").Return(&mockSchemesStore) + fakeEmoji := model.Emoji{Id: "123", Name: "name123"} + mockEmojiStore := mocks.EmojiStore{} + mockEmojiStore.On("Get", "123", true).Return(&fakeEmoji, nil) + mockEmojiStore.On("Get", "123", false).Return(&fakeEmoji, nil) + mockEmojiStore.On("GetByName", "name123", true).Return(&fakeEmoji, nil) + mockEmojiStore.On("GetByName", "name123", false).Return(&fakeEmoji, nil) + mockEmojiStore.On("Delete", &fakeEmoji, int64(0)).Return(nil) + mockStore.On("Emoji").Return(&mockEmojiStore) + + mockCount := int64(10) + mockChannelStore := mocks.ChannelStore{} + mockChannelStore.On("ClearCaches").Return() + mockChannelStore.On("GetMemberCount", "id", true).Return(mockCount, nil) + mockChannelStore.On("GetMemberCount", "id", false).Return(mockCount, nil) + mockStore.On("Channel").Return(&mockChannelStore) + return &mockStore } diff --git a/store/sqlstore/channel_store.go b/store/sqlstore/channel_store.go index bcfee6da38..4235b69e37 100644 --- a/store/sqlstore/channel_store.go +++ b/store/sqlstore/channel_store.go @@ -29,9 +29,6 @@ const ( ALL_CHANNEL_MEMBERS_NOTIFY_PROPS_FOR_CHANNEL_CACHE_SIZE = model.SESSION_CACHE_SIZE ALL_CHANNEL_MEMBERS_NOTIFY_PROPS_FOR_CHANNEL_CACHE_SEC = 1800 // 30 mins - CHANNEL_MEMBERS_COUNTS_CACHE_SIZE = model.CHANNEL_CACHE_SIZE - CHANNEL_MEMBERS_COUNTS_CACHE_SEC = 1800 // 30 mins - CHANNEL_GUESTS_COUNTS_CACHE_SIZE = model.CHANNEL_CACHE_SIZE CHANNEL_GUESTS_COUNTS_CACHE_SEC = 1800 // 30 mins @@ -283,7 +280,6 @@ type publicChannel struct { Purpose string `json:"purpose"` } -var channelMemberCountsCache = utils.NewLru(CHANNEL_MEMBERS_COUNTS_CACHE_SIZE) var channelPinnedPostCountsCache = utils.NewLru(CHANNEL_PINNEDPOSTS_COUNTS_CACHE_SIZE) var channelGuestCountsCache = utils.NewLru(CHANNEL_GUESTS_COUNTS_CACHE_SIZE) var allChannelMembersForUserCache = utils.NewLru(ALL_CHANNEL_MEMBERS_FOR_USER_CACHE_SIZE) @@ -292,7 +288,6 @@ var channelCache = utils.NewLru(model.CHANNEL_CACHE_SIZE) var channelByNameCache = utils.NewLru(model.CHANNEL_CACHE_SIZE) func (s SqlChannelStore) ClearCaches() { - channelMemberCountsCache.Purge() channelPinnedPostCountsCache.Purge() channelGuestCountsCache.Purge() allChannelMembersForUserCache.Purge() @@ -301,7 +296,6 @@ func (s SqlChannelStore) ClearCaches() { channelByNameCache.Purge() if s.metrics != nil { - s.metrics.IncrementMemCacheInvalidationCounter("Channel Member Counts - Purge") s.metrics.IncrementMemCacheInvalidationCounter("Channel Pinned Post Counts - Purge") s.metrics.IncrementMemCacheInvalidationCounter("All Channel Members for User - Purge") s.metrics.IncrementMemCacheInvalidationCounter("All Channel Members Notify Props for Channel - Purge") @@ -1585,46 +1579,14 @@ func (s SqlChannelStore) GetAllChannelMembersNotifyPropsForChannel(channelId str } func (s SqlChannelStore) InvalidateMemberCount(channelId string) { - channelMemberCountsCache.Remove(channelId) - if s.metrics != nil { - s.metrics.IncrementMemCacheInvalidationCounter("Channel Member Counts - Remove by ChannelId") - } } func (s SqlChannelStore) GetMemberCountFromCache(channelId string) int64 { - if cacheItem, ok := channelMemberCountsCache.Get(channelId); ok { - if s.metrics != nil { - s.metrics.IncrementMemCacheHitCounter("Channel Member Counts") - } - return cacheItem.(int64) - } - - if s.metrics != nil { - s.metrics.IncrementMemCacheMissCounter("Channel Member Counts") - } - - count, err := s.GetMemberCount(channelId, true) - if err != nil { - return 0 - } - + count, _ := s.GetMemberCount(channelId, true) return count } func (s SqlChannelStore) GetMemberCount(channelId string, allowFromCache bool) (int64, *model.AppError) { - if allowFromCache { - if cacheItem, ok := channelMemberCountsCache.Get(channelId); ok { - if s.metrics != nil { - s.metrics.IncrementMemCacheHitCounter("Channel Member Counts") - } - return cacheItem.(int64), nil - } - } - - if s.metrics != nil { - s.metrics.IncrementMemCacheMissCounter("Channel Member Counts") - } - count, err := s.GetReplica().SelectInt(` SELECT count(*) @@ -1639,10 +1601,6 @@ func (s SqlChannelStore) GetMemberCount(channelId string, allowFromCache bool) ( return 0, model.NewAppError("SqlChannelStore.GetMemberCount", "store.sql_channel.get_member_count.app_error", nil, "channel_id="+channelId+", "+err.Error(), http.StatusInternalServerError) } - if allowFromCache { - channelMemberCountsCache.AddWithExpiresInSecs(channelId, count, CHANNEL_MEMBERS_COUNTS_CACHE_SEC) - } - return count, nil } diff --git a/store/sqlstore/compliance_store.go b/store/sqlstore/compliance_store.go index b41884d3a1..7f04e458b0 100644 --- a/store/sqlstore/compliance_store.go +++ b/store/sqlstore/compliance_store.go @@ -216,6 +216,7 @@ func (s SqlComplianceStore) MessageExport(after int64, limit int) ([]*model.Mess Posts.DeleteAt AS PostDeleteAt, Posts.Message AS PostMessage, Posts.Type AS PostType, + Posts.Props AS PostProps, Posts.OriginalId AS PostOriginalId, Posts.RootId AS PostRootId, Posts.Props AS PostProps, @@ -243,7 +244,7 @@ func (s SqlComplianceStore) MessageExport(after int64, limit int) ([]*model.Mess LEFT JOIN Bots ON Bots.UserId = Posts.UserId WHERE (Posts.CreateAt > :StartTime OR Posts.EditAt > :StartTime OR Posts.DeleteAt > :StartTime) AND - Posts.Type = '' + Posts.Type NOT LIKE 'system_%' ORDER BY PostUpdateAt LIMIT :Limit` diff --git a/store/sqlstore/emoji_store.go b/store/sqlstore/emoji_store.go index 60868b326c..981388761b 100644 --- a/store/sqlstore/emoji_store.go +++ b/store/sqlstore/emoji_store.go @@ -11,17 +11,8 @@ import ( "github.com/mattermost/mattermost-server/einterfaces" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/store" - "github.com/mattermost/mattermost-server/utils" ) -const ( - EMOJI_CACHE_SIZE = 5000 - EMOJI_CACHE_SEC = 1800 // 30 mins -) - -var emojiCacheById = utils.NewLru(EMOJI_CACHE_SIZE) -var emojiIdCacheByName = utils.NewLru(EMOJI_CACHE_SIZE) - type SqlEmojiStore struct { SqlStore metrics einterfaces.MetricsInterface @@ -66,26 +57,10 @@ func (es SqlEmojiStore) Save(emoji *model.Emoji) (*model.Emoji, *model.AppError) } func (es SqlEmojiStore) Get(id string, allowFromCache bool) (*model.Emoji, *model.AppError) { - if allowFromCache { - if emoji, ok := es.getFromCacheById(id); ok { - return emoji, nil - } - } - return es.getBy("Id", id, allowFromCache) } func (es SqlEmojiStore) GetByName(name string, allowFromCache bool) (*model.Emoji, *model.AppError) { - if id, ok := model.GetSystemEmojiId(name); ok { - return es.Get(id, allowFromCache) - } - - if allowFromCache { - if emoji, ok := es.getFromCacheByName(name); ok { - return emoji, nil - } - } - return es.getBy("Name", name, allowFromCache) } @@ -139,8 +114,6 @@ func (es SqlEmojiStore) Delete(emoji *model.Emoji, time int64) *model.AppError { return model.NewAppError("SqlEmojiStore.Delete", "store.sql_emoji.delete.no_results", nil, "id="+emoji.Id, http.StatusBadRequest) } - es.removeFromCache(emoji) - return nil } @@ -193,51 +166,5 @@ func (es SqlEmojiStore) getBy(what string, key interface{}, addToCache bool) (*m return nil, model.NewAppError("SqlEmojiStore.GetByName", "store.sql_emoji.get.app_error", nil, "key="+fmt.Sprintf("%v", key)+", "+err.Error(), status) } - if addToCache { - es.addToCache(emoji) - } - return emoji, nil } - -func (es SqlEmojiStore) addToCache(emoji *model.Emoji) { - emojiCacheById.AddWithExpiresInSecs(emoji.Id, emoji, EMOJI_CACHE_SEC) - emojiIdCacheByName.AddWithExpiresInSecs(emoji.Name, emoji.Id, EMOJI_CACHE_SEC) -} - -func (es SqlEmojiStore) getFromCacheById(id string) (*model.Emoji, bool) { - if cacheItem, ok := emojiCacheById.Get(id); ok { - es.incrementMemCacheHitCounter("Emoji") - return cacheItem.(*model.Emoji), true - } - es.incrementMemCacheMissCounter("Emoji") - return nil, false -} - -func (es SqlEmojiStore) getFromCacheByName(name string) (*model.Emoji, bool) { - if id, ok := emojiIdCacheByName.Get(name); ok { - return es.getFromCacheById(id.(string)) - } - - es.incrementMemCacheMissCounter("Emoji") - return nil, false -} - -func (es SqlEmojiStore) incrementMemCacheHitCounter(cache string) { - if es.metrics == nil { - return - } - es.metrics.IncrementMemCacheHitCounter(cache) -} - -func (es SqlEmojiStore) incrementMemCacheMissCounter(cache string) { - if es.metrics == nil { - return - } - es.metrics.IncrementMemCacheMissCounter(cache) -} - -func (es SqlEmojiStore) removeFromCache(emoji *model.Emoji) { - emojiCacheById.Remove(emoji.Id) - emojiIdCacheByName.Remove(emoji.Name) -} diff --git a/store/sqlstore/upgrade.go b/store/sqlstore/upgrade.go index 2f923ad424..84b901abde 100644 --- a/store/sqlstore/upgrade.go +++ b/store/sqlstore/upgrade.go @@ -18,7 +18,8 @@ import ( ) const ( - CURRENT_SCHEMA_VERSION = VERSION_5_16_0 + CURRENT_SCHEMA_VERSION = VERSION_5_17_0 + VERSION_5_17_0 = "5.17.0" VERSION_5_16_0 = "5.16.0" VERSION_5_15_0 = "5.15.0" VERSION_5_14_0 = "5.14.0" @@ -163,6 +164,7 @@ func upgradeDatabase(sqlStore SqlStore, currentModelVersionString string) error upgradeDatabaseToVersion514(sqlStore) upgradeDatabaseToVersion515(sqlStore) upgradeDatabaseToVersion516(sqlStore) + upgradeDatabaseToVersion517(sqlStore) return nil } @@ -721,3 +723,9 @@ func upgradeDatabaseToVersion516(sqlStore SqlStore) { sqlStore.CreateIndexIfNotExists("idx_groupchannels_channelid", "GroupChannels", "ChannelId") } } + +func upgradeDatabaseToVersion517(sqlStore SqlStore) { + if shouldPerformUpgrade(sqlStore, VERSION_5_16_0, VERSION_5_17_0) { + saveSchemaVersion(sqlStore, VERSION_5_17_0) + } +} diff --git a/store/sqlstore/user_store.go b/store/sqlstore/user_store.go index 04ae9a5430..4c4625e3dd 100644 --- a/store/sqlstore/user_store.go +++ b/store/sqlstore/user_store.go @@ -141,6 +141,40 @@ func (us SqlUserStore) Save(user *model.User) (*model.User, *model.AppError) { return user, nil } +func (us SqlUserStore) DeactivateGuests() ([]string, *model.AppError) { + curTime := model.GetMillis() + updateQuery := us.getQueryBuilder().Update("Users"). + Set("UpdateAt", curTime). + Set("DeleteAt", curTime). + Where(sq.Eq{"Roles": "system_guest"}). + Where(sq.Eq{"DeleteAt": 0}) + + queryString, args, err := updateQuery.ToSql() + if err != nil { + return nil, model.NewAppError("SqlUserStore.UpdateActiveForMultipleUsers", "store.sql_user.app_error", nil, err.Error(), http.StatusInternalServerError) + } + + _, err = us.GetMaster().Exec(queryString, args...) + if err != nil { + return nil, model.NewAppError("SqlUserStore.UpdateActiveForMultipleUsers", "store.sql_user.update_active_for_multiple_users.updating.app_error", nil, err.Error(), http.StatusInternalServerError) + } + + selectQuery := us.getQueryBuilder().Select("Id").From("Users").Where(sq.Eq{"DeleteAt": curTime}) + + queryString, args, err = selectQuery.ToSql() + if err != nil { + return nil, model.NewAppError("SqlUserStore.UpdateActiveForMultipleUsers", "store.sql_user.app_error", nil, err.Error(), http.StatusInternalServerError) + } + + userIds := []string{} + _, err = us.GetMaster().Select(&userIds, queryString, args...) + if err != nil { + return nil, model.NewAppError("SqlUserStore.UpdateActiveForMultipleUsers", "store.sql_user.update_active_for_multiple_users.getting_changed_users.app_error", nil, err.Error(), http.StatusInternalServerError) + } + + return userIds, nil +} + func (us SqlUserStore) Update(user *model.User, trustedUpdateData bool) (*model.UserUpdate, *model.AppError) { user.PreUpdate() diff --git a/store/store.go b/store/store.go index 11e5c6dd67..deae1682f9 100644 --- a/store/store.go +++ b/store/store.go @@ -301,6 +301,7 @@ type UserStore interface { GetChannelGroupUsers(channelID string) ([]*model.User, *model.AppError) PromoteGuestToUser(userID string) *model.AppError DemoteUserToGuest(userID string) *model.AppError + DeactivateGuests() ([]string, *model.AppError) } type BotStore interface { diff --git a/store/storetest/emoji_store.go b/store/storetest/emoji_store.go index 4479aa18dd..dc3e2a39c4 100644 --- a/store/storetest/emoji_store.go +++ b/store/storetest/emoji_store.go @@ -21,7 +21,6 @@ func TestEmojiStore(t *testing.T, ss store.Store) { t.Run("EmojiGetMultipleByName", func(t *testing.T) { testEmojiGetMultipleByName(t, ss) }) t.Run("EmojiGetList", func(t *testing.T) { testEmojiGetList(t, ss) }) t.Run("EmojiSearch", func(t *testing.T) { testEmojiSearch(t, ss) }) - t.Run("EmojiCaching", func(t *testing.T) { testEmojiCaching(t, ss) }) } func testEmojiSaveDelete(t *testing.T, ss store.Store) { @@ -91,61 +90,6 @@ func testEmojiGet(t *testing.T, ss store.Store) { } } -func testEmojiCaching(t *testing.T, ss store.Store) { - emojis := make([]*model.Emoji, 3) - for i := range emojis { - emojis[i] = &model.Emoji{ - CreatorId: model.NewId(), - Name: model.NewId(), - } - } - - for _, emoji := range emojis { - _, err := ss.Emoji().Save(emoji) - require.Nil(t, err) - } - defer func() { - for _, emoji := range emojis { - err := ss.Emoji().Delete(emoji, time.Now().Unix()) - require.Nil(t, err) - } - }() - - var retrievedEmoji *model.Emoji - var cachedEmoji *model.Emoji - var err *model.AppError - - for _, emoji := range emojis { - cachedEmoji, err = ss.Emoji().Get(emoji.Id, true) - assert.Nilf(t, err, "should be able to retrieve emoji with id %v", emoji.Id) - - retrievedEmoji, err = ss.Emoji().Get(emoji.Id, false) - if assert.Nilf(t, err, "should be able to retrieve emoji with id %v", emoji.Id) { - assert.Falsef(t, retrievedEmoji == cachedEmoji, "should not be the same as cached with id %v", emoji.Id) - } - - retrievedEmoji, err = ss.Emoji().Get(emoji.Id, true) - if assert.Nilf(t, err, "should be able to retrieve emoji with id %v", emoji.Id) { - assert.Truef(t, retrievedEmoji == cachedEmoji, "should be the cached emoji with id %v", emoji.Id) - } - - retrievedEmoji, err = ss.Emoji().GetByName(emoji.Name, false) - if assert.Nilf(t, err, "should be able to retrieve emoji with name %v", emoji.Name) { - assert.Falsef(t, retrievedEmoji == cachedEmoji, "should not be the same as cached with name %v", emoji.Name) - } - - retrievedEmoji, _ = ss.Emoji().GetByName(emoji.Name, true) - if assert.Nilf(t, err, "should be able to retrieve emoji with name %v", emoji.Name) { - assert.Truef(t, retrievedEmoji == cachedEmoji, "should be the cached emoji with name %v", emoji.Name) - } - } - - _, err = ss.Emoji().Get(model.NewId(), false) - assert.NotNilf(t, err, "should not retrieve emoji with unsaved ID") - _, err = ss.Emoji().GetByName(model.NewId(), false) - assert.NotNilf(t, err, "should not retrieve emoji with unsaved name") -} - func testEmojiGetByName(t *testing.T, ss store.Store) { emojis := []model.Emoji{ { diff --git a/store/storetest/mocks/UserStore.go b/store/storetest/mocks/UserStore.go index c0bb32b9a1..61be3b4374 100644 --- a/store/storetest/mocks/UserStore.go +++ b/store/storetest/mocks/UserStore.go @@ -128,6 +128,31 @@ func (_m *UserStore) Count(options model.UserCountOptions) (int64, *model.AppErr return r0, r1 } +// DeactivateGuests provides a mock function with given fields: +func (_m *UserStore) DeactivateGuests() ([]string, *model.AppError) { + ret := _m.Called() + + var r0 []string + if rf, ok := ret.Get(0).(func() []string); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]string) + } + } + + var r1 *model.AppError + if rf, ok := ret.Get(1).(func() *model.AppError); ok { + r1 = rf() + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.AppError) + } + } + + return r0, r1 +} + // DemoteUserToGuest provides a mock function with given fields: userID func (_m *UserStore) DemoteUserToGuest(userID string) *model.AppError { ret := _m.Called(userID) diff --git a/store/storetest/user_store.go b/store/storetest/user_store.go index bd67187bf2..d02646f7bc 100644 --- a/store/storetest/user_store.go +++ b/store/storetest/user_store.go @@ -80,6 +80,7 @@ func TestUserStore(t *testing.T, ss store.Store, s SqlSupplier) { t.Run("GetChannelGroupUsers", func(t *testing.T) { testUserStoreGetChannelGroupUsers(t, ss) }) t.Run("PromoteGuestToUser", func(t *testing.T) { testUserStorePromoteGuestToUser(t, ss) }) t.Run("DemoteUserToGuest", func(t *testing.T) { testUserStoreDemoteUserToGuest(t, ss) }) + t.Run("DeactivateGuests", func(t *testing.T) { testDeactivateGuests(t, ss) }) t.Run("ResetLastPictureUpdate", func(t *testing.T) { testUserStoreResetLastPictureUpdate(t, ss) }) } @@ -4206,6 +4207,7 @@ func testUserStorePromoteGuestToUser(t *testing.T, ss store.Store) { Roles: "system_user", }) require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(user.Id)) }() teamId := model.NewId() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: user.Id, SchemeGuest: true, SchemeUser: false}, 999) @@ -4251,6 +4253,7 @@ func testUserStorePromoteGuestToUser(t *testing.T, ss store.Store) { Roles: "system_user system_admin", }) require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(user.Id)) }() teamId := model.NewId() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: user.Id, SchemeGuest: true, SchemeUser: false}, 999) @@ -4295,6 +4298,7 @@ func testUserStorePromoteGuestToUser(t *testing.T, ss store.Store) { Roles: "system_guest", }) require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(user.Id)) }() err = ss.User().PromoteGuestToUser(user.Id) assert.Nil(t, err) @@ -4315,6 +4319,7 @@ func testUserStorePromoteGuestToUser(t *testing.T, ss store.Store) { Roles: "system_guest", }) require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(user.Id)) }() teamId := model.NewId() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: user.Id, SchemeGuest: true, SchemeUser: false}, 999) @@ -4344,6 +4349,7 @@ func testUserStorePromoteGuestToUser(t *testing.T, ss store.Store) { Roles: "system_guest", }) require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(user.Id)) }() teamId := model.NewId() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: user.Id, SchemeGuest: true, SchemeUser: false}, 999) @@ -4388,6 +4394,7 @@ func testUserStorePromoteGuestToUser(t *testing.T, ss store.Store) { Roles: "system_guest custom_role", }) require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(user.Id)) }() teamId := model.NewId() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: user.Id, SchemeGuest: true, SchemeUser: false}, 999) @@ -4432,6 +4439,7 @@ func testUserStorePromoteGuestToUser(t *testing.T, ss store.Store) { Roles: "system_guest", }) require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(user1.Id)) }() teamId1 := model.NewId() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId1, UserId: user1.Id, SchemeGuest: true, SchemeUser: false}, 999) @@ -4459,6 +4467,7 @@ func testUserStorePromoteGuestToUser(t *testing.T, ss store.Store) { Roles: "system_guest", }) require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(user2.Id)) }() teamId2 := model.NewId() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId2, UserId: user2.Id, SchemeGuest: true, SchemeUser: false}, 999) @@ -4513,6 +4522,7 @@ func testUserStoreDemoteUserToGuest(t *testing.T, ss store.Store) { Roles: "system_guest", }) require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(user.Id)) }() teamId := model.NewId() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: user.Id, SchemeGuest: false, SchemeUser: true}, 999) @@ -4558,6 +4568,7 @@ func testUserStoreDemoteUserToGuest(t *testing.T, ss store.Store) { Roles: "system_user system_admin", }) require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(user.Id)) }() teamId := model.NewId() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: user.Id, SchemeGuest: true, SchemeUser: false}, 999) @@ -4602,6 +4613,7 @@ func testUserStoreDemoteUserToGuest(t *testing.T, ss store.Store) { Roles: "system_user", }) require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(user.Id)) }() err = ss.User().DemoteUserToGuest(user.Id) assert.Nil(t, err) @@ -4622,6 +4634,7 @@ func testUserStoreDemoteUserToGuest(t *testing.T, ss store.Store) { Roles: "system_user", }) require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(user.Id)) }() teamId := model.NewId() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: user.Id, SchemeGuest: false, SchemeUser: true}, 999) @@ -4651,6 +4664,7 @@ func testUserStoreDemoteUserToGuest(t *testing.T, ss store.Store) { Roles: "system_user", }) require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(user.Id)) }() teamId := model.NewId() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: user.Id, SchemeGuest: false, SchemeUser: true}, 999) @@ -4695,6 +4709,7 @@ func testUserStoreDemoteUserToGuest(t *testing.T, ss store.Store) { Roles: "system_user custom_role", }) require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(user.Id)) }() teamId := model.NewId() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId, UserId: user.Id, SchemeGuest: false, SchemeUser: true}, 999) @@ -4739,6 +4754,7 @@ func testUserStoreDemoteUserToGuest(t *testing.T, ss store.Store) { Roles: "system_user", }) require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(user1.Id)) }() teamId1 := model.NewId() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId1, UserId: user1.Id, SchemeGuest: false, SchemeUser: true}, 999) @@ -4766,6 +4782,7 @@ func testUserStoreDemoteUserToGuest(t *testing.T, ss store.Store) { Roles: "system_user", }) require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(user2.Id)) }() teamId2 := model.NewId() _, err = ss.Team().SaveMember(&model.TeamMember{TeamId: teamId2, UserId: user2.Id, SchemeGuest: false, SchemeUser: true}, 999) @@ -4806,6 +4823,84 @@ func testUserStoreDemoteUserToGuest(t *testing.T, ss store.Store) { }) } +func testDeactivateGuests(t *testing.T, ss store.Store) { + // create users + t.Run("Must disable all guests and no regular user or already deactivated users", func(t *testing.T) { + guest1Random := model.NewId() + guest1, err := ss.User().Save(&model.User{ + Email: guest1Random + "@test.com", + Username: "un_" + guest1Random, + Nickname: "nn_" + guest1Random, + FirstName: "f_" + guest1Random, + LastName: "l_" + guest1Random, + Password: "Password1", + Roles: "system_guest", + }) + require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(guest1.Id)) }() + + guest2Random := model.NewId() + guest2, err := ss.User().Save(&model.User{ + Email: guest2Random + "@test.com", + Username: "un_" + guest2Random, + Nickname: "nn_" + guest2Random, + FirstName: "f_" + guest2Random, + LastName: "l_" + guest2Random, + Password: "Password1", + Roles: "system_guest", + }) + require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(guest2.Id)) }() + + guest3Random := model.NewId() + guest3, err := ss.User().Save(&model.User{ + Email: guest3Random + "@test.com", + Username: "un_" + guest3Random, + Nickname: "nn_" + guest3Random, + FirstName: "f_" + guest3Random, + LastName: "l_" + guest3Random, + Password: "Password1", + Roles: "system_guest", + DeleteAt: 10, + }) + require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(guest3.Id)) }() + + regularUserRandom := model.NewId() + regularUser, err := ss.User().Save(&model.User{ + Email: regularUserRandom + "@test.com", + Username: "un_" + regularUserRandom, + Nickname: "nn_" + regularUserRandom, + FirstName: "f_" + regularUserRandom, + LastName: "l_" + regularUserRandom, + Password: "Password1", + Roles: "system_user", + }) + require.Nil(t, err) + defer func() { require.Nil(t, ss.User().PermanentDelete(regularUser.Id)) }() + + ids, err := ss.User().DeactivateGuests() + require.Nil(t, err) + assert.ElementsMatch(t, []string{guest1.Id, guest2.Id}, ids) + + u, err := ss.User().Get(guest1.Id) + require.Nil(t, err) + assert.NotEqual(t, u.DeleteAt, int64(0)) + + u, err = ss.User().Get(guest2.Id) + require.Nil(t, err) + assert.NotEqual(t, u.DeleteAt, int64(0)) + + u, err = ss.User().Get(guest3.Id) + require.Nil(t, err) + assert.Equal(t, u.DeleteAt, int64(10)) + + u, err = ss.User().Get(regularUser.Id) + require.Nil(t, err) + assert.Equal(t, u.DeleteAt, int64(0)) + }) +} + func testUserStoreResetLastPictureUpdate(t *testing.T, ss store.Store) { u1 := &model.User{} u1.Email = MakeEmail() diff --git a/templates/globalrelay_compliance_export_message.html b/templates/globalrelay_compliance_export_message.html index ff3bcd9fdf..27de9cf80d 100644 --- a/templates/globalrelay_compliance_export_message.html +++ b/templates/globalrelay_compliance_export_message.html @@ -2,6 +2,7 @@