From 0da0cf1a21d7dea61f4459ee71bf01b6ca362aee Mon Sep 17 00:00:00 2001 From: George Goldberg Date: Mon, 9 Oct 2017 18:14:27 +0100 Subject: PLT-7826: Don't fetch posts from store if ES returns none. (#7596) --- app/post.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) (limited to 'app') diff --git a/app/post.go b/app/post.go index 497cab5a6..4a465450b 100644 --- a/app/post.go +++ b/app/post.go @@ -602,12 +602,14 @@ func (a *App) SearchPostsInTeam(terms string, userId string, teamId string, isOr // Get the posts postList := model.NewPostList() - if presult := <-a.Srv.Store.Post().GetPostsByIds(postIds); presult.Err != nil { - return nil, presult.Err - } else { - for _, p := range presult.Data.([]*model.Post) { - postList.AddPost(p) - postList.AddOrder(p.Id) + if len(postIds) > 0 { + if presult := <-a.Srv.Store.Post().GetPostsByIds(postIds); presult.Err != nil { + return nil, presult.Err + } else { + for _, p := range presult.Data.([]*model.Post) { + postList.AddPost(p) + postList.AddOrder(p.Id) + } } } -- cgit v1.2.3-1-g7c22 From 9adaf53e110e0e806b21903111aacb93129668cb Mon Sep 17 00:00:00 2001 From: Joram Wilander Date: Mon, 9 Oct 2017 13:30:48 -0400 Subject: PLT-7818 Updates to post type (#7579) * Updates to post type * Update tests --- app/command.go | 5 +++++ app/command_test.go | 20 ++++++++++++++++++++ app/post.go | 5 +++++ app/webhook.go | 5 +++++ app/webhook_test.go | 5 +++++ 5 files changed, 40 insertions(+) (limited to 'app') diff --git a/app/command.go b/app/command.go index 6e439537e..811294b6d 100644 --- a/app/command.go +++ b/app/command.go @@ -41,6 +41,11 @@ func (a *App) CreateCommandPost(post *model.Post, teamId string, response *model post.Message = parseSlackLinksToMarkdown(response.Text) post.CreateAt = model.GetMillis() + if strings.HasPrefix(post.Type, model.POST_SYSTEM_MESSAGE_PREFIX) { + err := model.NewAppError("CreateCommandPost", "api.context.invalid_param.app_error", map[string]interface{}{"Name": "post.type"}, "", http.StatusBadRequest) + return nil, err + } + if response.Attachments != nil { parseSlackAttachment(post, response.Attachments) } diff --git a/app/command_test.go b/app/command_test.go index b37e78ea9..de4822436 100644 --- a/app/command_test.go +++ b/app/command_test.go @@ -45,3 +45,23 @@ func TestMoveCommand(t *testing.T) { assert.Nil(t, err) assert.EqualValues(t, targetTeam.Id, retrievedCommand.TeamId) } + +func TestCreateCommandPost(t *testing.T) { + th := Setup().InitBasic() + defer th.TearDown() + + post := &model.Post{ + ChannelId: th.BasicChannel.Id, + UserId: th.BasicUser.Id, + Type: model.POST_SYSTEM_GENERIC, + } + + resp := &model.CommandResponse{ + Text: "some message", + } + + _, err := th.App.CreateCommandPost(post, th.BasicTeam.Id, resp) + if err == nil && err.Id != "api.context.invalid_param.app_error" { + t.Fatal("should have failed - bad post type") + } +} diff --git a/app/post.go b/app/post.go index 4a465450b..fa929b844 100644 --- a/app/post.go +++ b/app/post.go @@ -29,6 +29,11 @@ func (a *App) CreatePostAsUser(post *model.Post) (*model.Post, *model.AppError) channel = result.Data.(*model.Channel) } + if strings.HasPrefix(post.Type, model.POST_SYSTEM_MESSAGE_PREFIX) { + err := model.NewAppError("CreatePostAsUser", "api.context.invalid_param.app_error", map[string]interface{}{"Name": "post.type"}, "", http.StatusBadRequest) + return nil, err + } + if channel.DeleteAt != 0 { err := model.NewAppError("createPost", "api.post.create_post.can_not_post_to_deleted.error", nil, "", http.StatusBadRequest) return nil, err diff --git a/app/webhook.go b/app/webhook.go index 9d9b24b10..1530ba94a 100644 --- a/app/webhook.go +++ b/app/webhook.go @@ -131,6 +131,11 @@ func (a *App) CreateWebhookPost(userId string, channel *model.Channel, text, ove post := &model.Post{UserId: userId, ChannelId: channel.Id, Message: text, Type: postType} post.AddProp("from_webhook", "true") + if strings.HasPrefix(post.Type, model.POST_SYSTEM_MESSAGE_PREFIX) { + err := model.NewAppError("CreateWebhookPost", "api.context.invalid_param.app_error", map[string]interface{}{"Name": "post.type"}, "", http.StatusBadRequest) + return nil, err + } + if metrics := a.Metrics; metrics != nil { metrics.IncrementWebhookPost() } diff --git a/app/webhook_test.go b/app/webhook_test.go index 5699addbf..b9ba35f43 100644 --- a/app/webhook_test.go +++ b/app/webhook_test.go @@ -44,4 +44,9 @@ func TestCreateWebhookPost(t *testing.T) { t.Fatal(k) } } + + _, err = th.App.CreateWebhookPost(hook.UserId, th.BasicChannel, "foo", "user", "http://iconurl", nil, model.POST_SYSTEM_GENERIC) + if err == nil { + t.Fatal("should have failed - bad post type") + } } -- cgit v1.2.3-1-g7c22 From e522a1c2e49f5d21e45dd66f83d06e10fc3cdb67 Mon Sep 17 00:00:00 2001 From: Harrison Healey Date: Mon, 9 Oct 2017 13:30:59 -0400 Subject: PLT-7811 Standardized team sanitization flow (#7586) * post-4.3 commit (#7581) * reduce store boiler plate (#7585) * fix GetPostsByIds error (#7591) * PLT-7811 Standardized team sanitization flow * Fixed TestGetAllTeamListings * Stopped sanitizing teams for team admins * Removed debug logging * Added TearDown to sanitization tests that needed it --- app/team.go | 26 +++++-- app/team_test.go | 214 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 235 insertions(+), 5 deletions(-) (limited to 'app') diff --git a/app/team.go b/app/team.go index 4bfb617a8..1bc77c9f0 100644 --- a/app/team.go +++ b/app/team.go @@ -104,8 +104,6 @@ func (a *App) UpdateTeam(team *model.Team) (*model.Team, *model.AppError) { return nil, result.Err } - oldTeam.Sanitize() - a.sendUpdatedTeamEvent(oldTeam) return oldTeam, nil @@ -124,16 +122,18 @@ func (a *App) PatchTeam(teamId string, patch *model.TeamPatch) (*model.Team, *mo return nil, err } - updatedTeam.Sanitize() - a.sendUpdatedTeamEvent(updatedTeam) return updatedTeam, nil } func (a *App) sendUpdatedTeamEvent(team *model.Team) { + sanitizedTeam := &model.Team{} + *sanitizedTeam = *team + sanitizedTeam.Sanitize() + message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_UPDATE_TEAM, "", "", "", nil) - message.Add("team", team.ToJson()) + message.Add("team", sanitizedTeam.ToJson()) a.Go(func() { a.Publish(message) }) @@ -833,3 +833,19 @@ func (a *App) GetTeamIdFromQuery(query url.Values) (string, *model.AppError) { return "", nil } + +func SanitizeTeam(session model.Session, team *model.Team) *model.Team { + if !SessionHasPermissionToTeam(session, team.Id, model.PERMISSION_MANAGE_TEAM) { + team.Sanitize() + } + + return team +} + +func SanitizeTeams(session model.Session, teams []*model.Team) []*model.Team { + for _, team := range teams { + SanitizeTeam(session, team) + } + + return teams +} diff --git a/app/team_test.go b/app/team_test.go index 7992dd0c3..61ae03f74 100644 --- a/app/team_test.go +++ b/app/team_test.go @@ -179,3 +179,217 @@ func TestPermanentDeleteTeam(t *testing.T) { t.Fatal(err) } } + +func TestSanitizeTeam(t *testing.T) { + th := Setup() + defer th.TearDown() + + team := &model.Team{ + Id: model.NewId(), + Email: th.MakeEmail(), + AllowedDomains: "example.com", + } + copyTeam := func() *model.Team { + copy := &model.Team{} + *copy = *team + return copy + } + + t.Run("not a user of the team", func(t *testing.T) { + userId := model.NewId() + session := model.Session{ + Roles: model.ROLE_SYSTEM_USER.Id, + TeamMembers: []*model.TeamMember{ + { + UserId: userId, + TeamId: model.NewId(), + Roles: model.ROLE_TEAM_USER.Id, + }, + }, + } + + sanitized := SanitizeTeam(session, copyTeam()) + if sanitized.Email != "" && sanitized.AllowedDomains != "" { + t.Fatal("should've sanitized team") + } + }) + + t.Run("user of the team", func(t *testing.T) { + userId := model.NewId() + session := model.Session{ + Roles: model.ROLE_SYSTEM_USER.Id, + TeamMembers: []*model.TeamMember{ + { + UserId: userId, + TeamId: team.Id, + Roles: model.ROLE_TEAM_USER.Id, + }, + }, + } + + sanitized := SanitizeTeam(session, copyTeam()) + if sanitized.Email != "" && sanitized.AllowedDomains != "" { + t.Fatal("should've sanitized team") + } + }) + + t.Run("team admin", func(t *testing.T) { + userId := model.NewId() + session := model.Session{ + Roles: model.ROLE_SYSTEM_USER.Id, + TeamMembers: []*model.TeamMember{ + { + UserId: userId, + TeamId: team.Id, + Roles: model.ROLE_TEAM_USER.Id + " " + model.ROLE_TEAM_ADMIN.Id, + }, + }, + } + + sanitized := SanitizeTeam(session, copyTeam()) + if sanitized.Email == "" && sanitized.AllowedDomains == "" { + t.Fatal("shouldn't have sanitized team") + } + }) + + t.Run("team admin of another team", func(t *testing.T) { + userId := model.NewId() + session := model.Session{ + Roles: model.ROLE_SYSTEM_USER.Id, + TeamMembers: []*model.TeamMember{ + { + UserId: userId, + TeamId: model.NewId(), + Roles: model.ROLE_TEAM_USER.Id + " " + model.ROLE_TEAM_ADMIN.Id, + }, + }, + } + + sanitized := SanitizeTeam(session, copyTeam()) + if sanitized.Email != "" && sanitized.AllowedDomains != "" { + t.Fatal("should've sanitized team") + } + }) + + t.Run("system admin, not a user of team", func(t *testing.T) { + userId := model.NewId() + session := model.Session{ + Roles: model.ROLE_SYSTEM_USER.Id + " " + model.ROLE_SYSTEM_ADMIN.Id, + TeamMembers: []*model.TeamMember{ + { + UserId: userId, + TeamId: model.NewId(), + Roles: model.ROLE_TEAM_USER.Id, + }, + }, + } + + sanitized := SanitizeTeam(session, copyTeam()) + if sanitized.Email == "" && sanitized.AllowedDomains == "" { + t.Fatal("shouldn't have sanitized team") + } + }) + + t.Run("system admin, user of team", func(t *testing.T) { + userId := model.NewId() + session := model.Session{ + Roles: model.ROLE_SYSTEM_USER.Id + " " + model.ROLE_SYSTEM_ADMIN.Id, + TeamMembers: []*model.TeamMember{ + { + UserId: userId, + TeamId: team.Id, + Roles: model.ROLE_TEAM_USER.Id, + }, + }, + } + + sanitized := SanitizeTeam(session, copyTeam()) + if sanitized.Email == "" && sanitized.AllowedDomains == "" { + t.Fatal("shouldn't have sanitized team") + } + }) +} + +func TestSanitizeTeams(t *testing.T) { + th := Setup() + defer th.TearDown() + + t.Run("not a system admin", func(t *testing.T) { + teams := []*model.Team{ + { + Id: model.NewId(), + Email: th.MakeEmail(), + AllowedDomains: "example.com", + }, + { + Id: model.NewId(), + Email: th.MakeEmail(), + AllowedDomains: "example.com", + }, + } + + userId := model.NewId() + session := model.Session{ + Roles: model.ROLE_SYSTEM_USER.Id, + TeamMembers: []*model.TeamMember{ + { + UserId: userId, + TeamId: teams[0].Id, + Roles: model.ROLE_TEAM_USER.Id, + }, + { + UserId: userId, + TeamId: teams[1].Id, + Roles: model.ROLE_TEAM_USER.Id + " " + model.ROLE_TEAM_ADMIN.Id, + }, + }, + } + + sanitized := SanitizeTeams(session, teams) + + if sanitized[0].Email != "" && sanitized[0].AllowedDomains != "" { + t.Fatal("should've sanitized first team") + } + + if sanitized[1].Email == "" && sanitized[1].AllowedDomains == "" { + t.Fatal("shouldn't have sanitized second team") + } + }) + + t.Run("system admin", func(t *testing.T) { + teams := []*model.Team{ + { + Id: model.NewId(), + Email: th.MakeEmail(), + AllowedDomains: "example.com", + }, + { + Id: model.NewId(), + Email: th.MakeEmail(), + AllowedDomains: "example.com", + }, + } + + userId := model.NewId() + session := model.Session{ + Roles: model.ROLE_SYSTEM_USER.Id + " " + model.ROLE_SYSTEM_ADMIN.Id, + TeamMembers: []*model.TeamMember{ + { + UserId: userId, + TeamId: teams[0].Id, + Roles: model.ROLE_TEAM_USER.Id, + }, + }, + } + + sanitized := SanitizeTeams(session, teams) + + if sanitized[0].Email == "" && sanitized[0].AllowedDomains == "" { + t.Fatal("shouldn't have sanitized first team") + } + + if sanitized[1].Email == "" && sanitized[1].AllowedDomains == "" { + t.Fatal("shouldn't have sanitized second team") + } + }) +} -- cgit v1.2.3-1-g7c22