Merge branch 'release-4.3'

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

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

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
@@ -599,12 +604,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)
+				}
 			}
 		}
 

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

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

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

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