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

@@ -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()
 }

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)
+	}
+}

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()