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 --- api/post_test.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) (limited to 'api') diff --git a/api/post_test.go b/api/post_test.go index f57c2e05c..c01a5fa93 100644 --- a/api/post_test.go +++ b/api/post_test.go @@ -61,6 +61,11 @@ func TestCreatePost(t *testing.T) { t.Fatal("Newly craeted post shouldn't have EditAt set") } + _, err = Client.CreatePost(&model.Post{ChannelId: channel1.Id, Message: "#hashtag a" + model.NewId() + "a", Type: model.POST_SYSTEM_GENERIC}) + if err == nil { + t.Fatal("should have failed - bad post type") + } + post2 := &model.Post{ChannelId: channel1.Id, Message: "zz" + model.NewId() + "a", RootId: rpost1.Data.(*model.Post).Id} rpost2, err := Client.CreatePost(post2) if err != nil { @@ -454,13 +459,12 @@ func TestUpdatePost(t *testing.T) { } } - post3 := &model.Post{ChannelId: channel1.Id, Message: "zz" + model.NewId() + "a", Type: model.POST_JOIN_LEAVE} - rpost3, err := Client.CreatePost(post3) + rpost3, err := th.App.CreatePost(&model.Post{ChannelId: channel1.Id, Message: "zz" + model.NewId() + "a", Type: model.POST_JOIN_LEAVE, UserId: th.BasicUser.Id}, channel1, false) if err != nil { t.Fatal(err) } - up3 := &model.Post{Id: rpost3.Data.(*model.Post).Id, ChannelId: channel1.Id, Message: "zz" + model.NewId() + " update post 3"} + up3 := &model.Post{Id: rpost3.Id, ChannelId: channel1.Id, Message: "zz" + model.NewId() + " update post 3"} if _, err := Client.UpdatePost(up3); err == nil { t.Fatal("shouldn't have been able to update system message") } -- 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 --- api/apitestlib.go | 13 +- api/team.go | 24 +++- api/team_test.go | 357 ++++++++++++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 380 insertions(+), 14 deletions(-) (limited to 'api') diff --git a/api/apitestlib.go b/api/apitestlib.go index c0fd79ae9..3c64d2430 100644 --- a/api/apitestlib.go +++ b/api/apitestlib.go @@ -4,6 +4,7 @@ package api import ( + "strings" "time" "github.com/mattermost/mattermost-server/api4" @@ -130,8 +131,8 @@ func (me *TestHelper) CreateTeam(client *model.Client) *model.Team { id := model.NewId() team := &model.Team{ DisplayName: "dn_" + id, - Name: "name" + id, - Email: "success+" + id + "@simulator.amazonses.com", + Name: GenerateTestTeamName(), + Email: GenerateTestEmail(), Type: model.TEAM_OPEN, } @@ -308,6 +309,14 @@ func (me *TestHelper) LoginSystemAdmin() { utils.EnableDebugLogForTest() } +func GenerateTestEmail() string { + return strings.ToLower("success+" + model.NewId() + "@simulator.amazonses.com") +} + +func GenerateTestTeamName() string { + return "faketeam" + model.NewRandomString(6) +} + func (me *TestHelper) TearDown() { me.App.Shutdown() } diff --git a/api/team.go b/api/team.go index 8a8d3c935..49b20686d 100644 --- a/api/team.go +++ b/api/team.go @@ -67,6 +67,8 @@ func createTeam(c *Context, w http.ResponseWriter, r *http.Request) { return } + // Don't sanitize the team here since the user will be a team admin and their session won't reflect that yet + w.Write([]byte(rteam.ToJson())) } @@ -82,11 +84,10 @@ func GetAllTeamListings(c *Context, w http.ResponseWriter, r *http.Request) { m := make(map[string]*model.Team) for _, v := range teams { m[v.Id] = v - if !app.SessionHasPermissionTo(c.Session, model.PERMISSION_MANAGE_SYSTEM) { - m[v.Id].Sanitize() - } } + sanitizeTeamMap(c.Session, m) + w.Write([]byte(model.TeamMapToJson(m))) } @@ -112,6 +113,8 @@ func getAll(c *Context, w http.ResponseWriter, r *http.Request) { m[v.Id] = v } + sanitizeTeamMap(c.Session, m) + w.Write([]byte(model.TeamMapToJson(m))) } @@ -207,7 +210,7 @@ func addUserToTeamFromInvite(c *Context, w http.ResponseWriter, r *http.Request) return } - team.Sanitize() + app.SanitizeTeam(c.Session, team) w.Write([]byte(team.ToJson())) } @@ -241,6 +244,8 @@ func getTeamByName(c *Context, w http.ResponseWriter, r *http.Request) { } } + app.SanitizeTeam(c.Session, team) + w.Write([]byte(team.ToJson())) return } @@ -294,6 +299,8 @@ func updateTeam(c *Context, w http.ResponseWriter, r *http.Request) { return } + app.SanitizeTeam(c.Session, updatedTeam) + w.Write([]byte(updatedTeam.ToJson())) } @@ -342,6 +349,9 @@ func getMyTeam(c *Context, w http.ResponseWriter, r *http.Request) { return } else { w.Header().Set(model.HEADER_ETAG_SERVER, team.Etag()) + + app.SanitizeTeam(c.Session, team) + w.Write([]byte(team.ToJson())) return } @@ -529,3 +539,9 @@ func getTeamMembersByIds(c *Context, w http.ResponseWriter, r *http.Request) { return } } + +func sanitizeTeamMap(session model.Session, teams map[string]*model.Team) { + for _, team := range teams { + app.SanitizeTeam(session, team) + } +} diff --git a/api/team_test.go b/api/team_test.go index ea29b9d6f..1e4b36433 100644 --- a/api/team_test.go +++ b/api/team_test.go @@ -56,6 +56,49 @@ func TestCreateTeam(t *testing.T) { } } +func TestCreateTeamSanitization(t *testing.T) { + th := Setup().InitBasic().InitSystemAdmin() + defer th.TearDown() + + // Non-admin users can create a team, but they become a team admin by doing so + + t.Run("team admin", func(t *testing.T) { + team := &model.Team{ + DisplayName: t.Name() + "_1", + Name: GenerateTestTeamName(), + Email: GenerateTestEmail(), + Type: model.TEAM_OPEN, + AllowedDomains: "simulator.amazonses.com", + } + + if res, err := th.BasicClient.CreateTeam(team); err != nil { + t.Fatal(err) + } else if rteam := res.Data.(*model.Team); rteam.Email == "" { + t.Fatal("should not have sanitized email") + } else if rteam.AllowedDomains == "" { + t.Fatal("should not have sanitized allowed domains") + } + }) + + t.Run("system admin", func(t *testing.T) { + team := &model.Team{ + DisplayName: t.Name() + "_2", + Name: GenerateTestTeamName(), + Email: GenerateTestEmail(), + Type: model.TEAM_OPEN, + AllowedDomains: "simulator.amazonses.com", + } + + if res, err := th.SystemAdminClient.CreateTeam(team); err != nil { + t.Fatal(err) + } else if rteam := res.Data.(*model.Team); rteam.Email == "" { + t.Fatal("should not have sanitized email") + } else if rteam.AllowedDomains == "" { + t.Fatal("should not have sanitized allowed domains") + } + }) +} + func TestAddUserToTeam(t *testing.T) { th := Setup().InitSystemAdmin().InitBasic() defer th.TearDown() @@ -253,6 +296,77 @@ func TestGetAllTeams(t *testing.T) { } } +func TestGetAllTeamsSanitization(t *testing.T) { + th := Setup().InitBasic().InitSystemAdmin() + defer th.TearDown() + + var team *model.Team + if res, err := th.BasicClient.CreateTeam(&model.Team{ + DisplayName: t.Name() + "_1", + Name: GenerateTestTeamName(), + Email: GenerateTestEmail(), + Type: model.TEAM_OPEN, + AllowedDomains: "simulator.amazonses.com", + }); err != nil { + t.Fatal(err) + } else { + team = res.Data.(*model.Team) + } + + var team2 *model.Team + if res, err := th.SystemAdminClient.CreateTeam(&model.Team{ + DisplayName: t.Name() + "_2", + Name: GenerateTestTeamName(), + Email: GenerateTestEmail(), + Type: model.TEAM_OPEN, + AllowedDomains: "simulator.amazonses.com", + }); err != nil { + t.Fatal(err) + } else { + team2 = res.Data.(*model.Team) + } + + t.Run("team admin/team user", func(t *testing.T) { + if res, err := th.BasicClient.GetAllTeams(); err != nil { + t.Fatal(err) + } else { + for _, rteam := range res.Data.(map[string]*model.Team) { + if rteam.Id == team.Id { + if rteam.Email == "" { + t.Fatal("should not have sanitized email for team admin") + } else if rteam.AllowedDomains == "" { + t.Fatal("should not have sanitized allowed domains for team admin") + } + } else if rteam.Id == team2.Id { + if rteam.Email != "" { + t.Fatal("should've sanitized email for non-admin") + } else if rteam.AllowedDomains != "" { + t.Fatal("should've sanitized allowed domains for non-admin") + } + } + } + } + }) + + t.Run("system admin", func(t *testing.T) { + if res, err := th.SystemAdminClient.GetAllTeams(); err != nil { + t.Fatal(err) + } else { + for _, rteam := range res.Data.(map[string]*model.Team) { + if rteam.Id != team.Id && rteam.Id != team2.Id { + continue + } + + if rteam.Email == "" { + t.Fatal("should not have sanitized email") + } else if rteam.AllowedDomains == "" { + t.Fatal("should not have sanitized allowed domains") + } + } + } + }) +} + func TestGetAllTeamListings(t *testing.T) { th := Setup().InitBasic() defer th.TearDown() @@ -277,10 +391,7 @@ func TestGetAllTeamListings(t *testing.T) { } else { teams := r1.Data.(map[string]*model.Team) if teams[team.Id].Name != team.Name { - t.Fatal() - } - if teams[team.Id].Email != "" { - t.Fatal("Non admin users shoudn't get full listings") + t.Fatal("team name doesn't match") } } @@ -294,14 +405,84 @@ func TestGetAllTeamListings(t *testing.T) { } else { teams := r1.Data.(map[string]*model.Team) if teams[team.Id].Name != team.Name { - t.Fatal() - } - if teams[team.Id].Email != team.Email { - t.Fatal() + t.Fatal("team name doesn't match") } } } +func TestGetAllTeamListingsSanitization(t *testing.T) { + th := Setup().InitBasic().InitSystemAdmin() + defer th.TearDown() + + var team *model.Team + if res, err := th.BasicClient.CreateTeam(&model.Team{ + DisplayName: t.Name() + "_1", + Name: GenerateTestTeamName(), + Email: GenerateTestEmail(), + Type: model.TEAM_OPEN, + AllowedDomains: "simulator.amazonses.com", + AllowOpenInvite: true, + }); err != nil { + t.Fatal(err) + } else { + team = res.Data.(*model.Team) + } + + var team2 *model.Team + if res, err := th.SystemAdminClient.CreateTeam(&model.Team{ + DisplayName: t.Name() + "_2", + Name: GenerateTestTeamName(), + Email: GenerateTestEmail(), + Type: model.TEAM_OPEN, + AllowedDomains: "simulator.amazonses.com", + AllowOpenInvite: true, + }); err != nil { + t.Fatal(err) + } else { + team2 = res.Data.(*model.Team) + } + + t.Run("team admin/non-admin", func(t *testing.T) { + if res, err := th.BasicClient.GetAllTeamListings(); err != nil { + t.Fatal(err) + } else { + for _, rteam := range res.Data.(map[string]*model.Team) { + if rteam.Id == team.Id { + if rteam.Email == "" { + t.Fatal("should not have sanitized email for team admin") + } else if rteam.AllowedDomains == "" { + t.Fatal("should not have sanitized allowed domains for team admin") + } + } else if rteam.Id == team2.Id { + if rteam.Email != "" { + t.Fatal("should've sanitized email for non-admin") + } else if rteam.AllowedDomains != "" { + t.Fatal("should've sanitized allowed domains for non-admin") + } + } + } + } + }) + + t.Run("system admin", func(t *testing.T) { + if res, err := th.SystemAdminClient.GetAllTeamListings(); err != nil { + t.Fatal(err) + } else { + for _, rteam := range res.Data.(map[string]*model.Team) { + if rteam.Id != team.Id && rteam.Id != team2.Id { + continue + } + + if rteam.Email == "" { + t.Fatal("should not have sanitized email") + } else if rteam.AllowedDomains == "" { + t.Fatal("should not have sanitized allowed domains") + } + } + } + }) +} + func TestTeamPermDelete(t *testing.T) { th := Setup().InitBasic() defer th.TearDown() @@ -476,6 +657,52 @@ func TestUpdateTeamDisplayName(t *testing.T) { } } +func TestUpdateTeamSanitization(t *testing.T) { + th := Setup().InitBasic().InitSystemAdmin() + defer th.TearDown() + + var team *model.Team + if res, err := th.BasicClient.CreateTeam(&model.Team{ + DisplayName: t.Name() + "_1", + Name: GenerateTestTeamName(), + Email: GenerateTestEmail(), + Type: model.TEAM_OPEN, + AllowedDomains: "simulator.amazonses.com", + }); err != nil { + t.Fatal(err) + } else { + team = res.Data.(*model.Team) + } + + // Non-admin users cannot update the team + + t.Run("team admin", func(t *testing.T) { + // API v3 always assumes you're updating the current team + th.BasicClient.SetTeamId(team.Id) + + if res, err := th.BasicClient.UpdateTeam(team); err != nil { + t.Fatal(err) + } else if rteam := res.Data.(*model.Team); rteam.Email == "" { + t.Fatal("should not have sanitized email for admin") + } else if rteam.AllowedDomains == "" { + t.Fatal("should not have sanitized allowed domains") + } + }) + + t.Run("system admin", func(t *testing.T) { + // API v3 always assumes you're updating the current team + th.SystemAdminClient.SetTeamId(team.Id) + + if res, err := th.SystemAdminClient.UpdateTeam(team); err != nil { + t.Fatal(err) + } else if rteam := res.Data.(*model.Team); rteam.Email == "" { + t.Fatal("should not have sanitized email for admin") + } else if rteam.AllowedDomains == "" { + t.Fatal("should not have sanitized allowed domains") + } + }) +} + func TestFuzzyTeamCreate(t *testing.T) { th := Setup().InitBasic() defer th.TearDown() @@ -537,6 +764,65 @@ func TestGetMyTeam(t *testing.T) { } } +func TestGetMyTeamSanitization(t *testing.T) { + th := Setup().InitBasic().InitSystemAdmin() + defer th.TearDown() + + var team *model.Team + if res, err := th.BasicClient.CreateTeam(&model.Team{ + DisplayName: t.Name() + "_1", + Name: GenerateTestTeamName(), + Email: GenerateTestEmail(), + Type: model.TEAM_OPEN, + AllowedDomains: "simulator.amazonses.com", + }); err != nil { + t.Fatal(err) + } else { + team = res.Data.(*model.Team) + } + + t.Run("team user", func(t *testing.T) { + th.LinkUserToTeam(th.BasicUser2, team) + + client := th.CreateClient() + client.Must(client.Login(th.BasicUser2.Email, th.BasicUser2.Password)) + + client.SetTeamId(team.Id) + + if res, err := client.GetMyTeam(""); err != nil { + t.Fatal(err) + } else if rteam := res.Data.(*model.Team); rteam.Email != "" { + t.Fatal("should've sanitized email") + } else if rteam.AllowedDomains != "" { + t.Fatal("should've sanitized allowed domains") + } + }) + + t.Run("team admin", func(t *testing.T) { + th.BasicClient.SetTeamId(team.Id) + + if res, err := th.BasicClient.GetMyTeam(""); err != nil { + t.Fatal(err) + } else if rteam := res.Data.(*model.Team); rteam.Email == "" { + t.Fatal("should not have sanitized email") + } else if rteam.AllowedDomains == "" { + t.Fatal("should not have sanitized allowed domains") + } + }) + + t.Run("system admin", func(t *testing.T) { + th.SystemAdminClient.SetTeamId(team.Id) + + if res, err := th.SystemAdminClient.GetMyTeam(""); err != nil { + t.Fatal(err) + } else if rteam := res.Data.(*model.Team); rteam.Email == "" { + t.Fatal("should not have sanitized email") + } else if rteam.AllowedDomains == "" { + t.Fatal("should not have sanitized allowed domains") + } + }) +} + func TestGetTeamMembers(t *testing.T) { th := Setup().InitBasic() defer th.TearDown() @@ -898,6 +1184,61 @@ func TestGetTeamByName(t *testing.T) { } } +func TestGetTeamByNameSanitization(t *testing.T) { + th := Setup().InitBasic().InitSystemAdmin() + defer th.TearDown() + + var team *model.Team + if res, err := th.BasicClient.CreateTeam(&model.Team{ + DisplayName: t.Name() + "_1", + Name: GenerateTestTeamName(), + Email: GenerateTestEmail(), + Type: model.TEAM_OPEN, + AllowedDomains: "simulator.amazonses.com", + }); err != nil { + t.Fatal(err) + } else { + team = res.Data.(*model.Team) + } + + t.Run("team user", func(t *testing.T) { + th.LinkUserToTeam(th.BasicUser2, team) + + client := th.CreateClient() + client.Must(client.Login(th.BasicUser2.Email, th.BasicUser2.Password)) + + if res, err := client.GetTeamByName(team.Name); err != nil { + t.Fatal(err) + } else if rteam := res.Data.(*model.Team); rteam.Email != "" { + t.Fatal("should've sanitized email") + } else if rteam.AllowedDomains != "" { + t.Fatal("should've sanitized allowed domains") + } + }) + + t.Run("team admin", func(t *testing.T) { + if res, err := th.BasicClient.GetTeamByName(team.Name); err != nil { + t.Fatal(err) + } else if rteam := res.Data.(*model.Team); rteam.Email == "" { + t.Fatal("should not have sanitized email") + } else if rteam.AllowedDomains == "" { + t.Fatal("should not have sanitized allowed domains") + } + }) + + t.Run("system admin", func(t *testing.T) { + th.SystemAdminClient.SetTeamId(team.Id) + + if res, err := th.SystemAdminClient.GetTeamByName(team.Name); err != nil { + t.Fatal(err) + } else if rteam := res.Data.(*model.Team); rteam.Email == "" { + t.Fatal("should not have sanitized email") + } else if rteam.AllowedDomains == "" { + t.Fatal("should not have sanitized allowed domains") + } + }) +} + func TestFindTeamByName(t *testing.T) { th := Setup().InitBasic() defer th.TearDown() -- cgit v1.2.3-1-g7c22 From aa2b82727f0f1b3edb79f6d31c04b8fd0d718455 Mon Sep 17 00:00:00 2001 From: Chris Date: Wed, 11 Oct 2017 12:16:04 -0700 Subject: fix race condition in tests (#7609) --- api/apitestlib.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) (limited to 'api') diff --git a/api/apitestlib.go b/api/apitestlib.go index 3c64d2430..a38c20813 100644 --- a/api/apitestlib.go +++ b/api/apitestlib.go @@ -4,6 +4,7 @@ package api import ( + "net" "strings" "time" @@ -86,7 +87,20 @@ func ReloadConfigForSetup() { *utils.Cfg.TeamSettings.EnableOpenServer = true } +func (me *TestHelper) waitForConnectivity() { + for i := 0; i < 1000; i++ { + _, err := net.Dial("tcp", "localhost"+*utils.Cfg.ServiceSettings.ListenAddress) + if err == nil { + return + } + time.Sleep(time.Millisecond * 20) + } + panic("unable to connect") +} + func (me *TestHelper) InitBasic() *TestHelper { + me.waitForConnectivity() + me.BasicClient = me.CreateClient() me.BasicUser = me.CreateUser(me.BasicClient) me.LoginBasic() @@ -106,6 +120,8 @@ func (me *TestHelper) InitBasic() *TestHelper { } func (me *TestHelper) InitSystemAdmin() *TestHelper { + me.waitForConnectivity() + me.SystemAdminClient = me.CreateClient() me.SystemAdminUser = me.CreateUser(me.SystemAdminClient) me.SystemAdminUser.Password = "Password1" -- cgit v1.2.3-1-g7c22