Added DM GM restriction for flagging a post (#37841)

* Added DM GM restriction for flagging a post

* Added DM/GM check in other content reviewer paths as well

* Fixed an order of operation
This commit is contained in:
Harshil Sharma
2026-08-13 08:05:24 +05:30
committed by GitHub
parent 27a5abe2d4
commit 65b1437d08
11 changed files with 450 additions and 2 deletions
+5
View File
@@ -844,6 +844,11 @@ func getChannel(c *Context, w http.ResponseWriter, r *http.Request) {
return
}
checkChannelFlaggable(c, channel)
if c.Err != nil {
return
}
requireTeamContentReviewer(c, c.AppContext.Session().UserId, channel.TeamId)
if c.Err != nil {
return
+22
View File
@@ -2330,6 +2330,28 @@ func TestGetChannel(t *testing.T) {
require.Error(t, err)
CheckForbiddenStatus(t, resp)
})
t.Run("Content reviewer should not be able to get a DM or GM channel", func(t *testing.T) {
th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuEnterpriseAdvanced))
appErr := setBasicCommonReviewerConfig(th)
require.Nil(t, appErr)
contentReviewClient := th.CreateClient()
_, _, err := contentReviewClient.Login(context.Background(), th.BasicUser.Email, th.BasicUser.Password)
require.NoError(t, err)
dmPost := createDmPost(t, th, contentReviewClient)
_, resp, err := contentReviewClient.GetChannelAsContentReviewer(context.Background(), dmPost.ChannelId, "", dmPost.Id)
require.Error(t, err)
CheckBadRequestStatus(t, resp)
CheckErrorID(t, err, "api.data_spillage.error.invalid_channel_type")
gmPost := createGmPost(t, th, contentReviewClient)
_, resp, err = contentReviewClient.GetChannelAsContentReviewer(context.Background(), gmPost.ChannelId, "", gmPost.Id)
require.Error(t, err)
CheckBadRequestStatus(t, resp)
CheckErrorID(t, err, "api.data_spillage.error.invalid_channel_type")
})
}
func TestGetDeletedChannelsForTeam(t *testing.T) {
+31
View File
@@ -196,6 +196,11 @@ func flagPost(c *Context, w http.ResponseWriter, r *http.Request) {
return
}
checkChannelFlaggable(c, channel)
if c.Err != nil {
return
}
enabled, appErr := c.App.ContentFlaggingEnabledForTeam(channel.TeamId)
if appErr != nil {
c.Err = appErr
@@ -283,6 +288,11 @@ func getPostPropertyValues(c *Context, w http.ResponseWriter, r *http.Request) {
return
}
checkChannelFlaggable(c, channel)
if c.Err != nil {
return
}
userId := c.AppContext.Session().UserId
requireTeamContentReviewer(c, userId, channel.TeamId)
if c.Err != nil {
@@ -336,6 +346,11 @@ func getFlaggedPost(c *Context, w http.ResponseWriter, r *http.Request) {
return
}
checkChannelFlaggable(c, channel)
if c.Err != nil {
return
}
requireTeamContentReviewer(c, userId, channel.TeamId)
if c.Err != nil {
return
@@ -443,6 +458,11 @@ func keepRemoveFlaggedPostChecks(c *Context, r *http.Request) (*model.FlagConten
return nil, "", nil
}
checkChannelFlaggable(c, channel)
if c.Err != nil {
return nil, "", nil
}
requireTeamContentReviewer(c, userId, channel.TeamId)
if c.Err != nil {
return nil, "", nil
@@ -589,6 +609,11 @@ func assignFlaggedPostReviewer(c *Context, w http.ResponseWriter, r *http.Reques
return
}
checkChannelFlaggable(c, channel)
if c.Err != nil {
return
}
assignedBy := c.AppContext.Session().UserId
requireTeamContentReviewer(c, assignedBy, channel.TeamId)
if c.Err != nil {
@@ -621,3 +646,9 @@ func checkPostTypeFlaggable(c *Context, post *model.Post) {
c.Err = model.NewAppError("checkPostTypeFlaggable", "api.data_spillage.error.invalid_post_type", map[string]any{"PostType": post.Type}, "", http.StatusBadRequest)
}
}
func checkChannelFlaggable(c *Context, channel *model.Channel) {
if channel.IsGroupOrDirect() {
c.Err = model.NewAppError("checkChannelFlaggable", "api.data_spillage.error.invalid_channel_type", nil, "", http.StatusBadRequest)
}
}
@@ -55,6 +55,11 @@ func generateFlaggedPostReport(c *Context, w http.ResponseWriter, r *http.Reques
return
}
checkChannelFlaggable(c, channel)
if c.Err != nil {
return
}
requireTeamContentReviewer(c, userId, channel.TeamId)
if c.Err != nil {
return
@@ -203,4 +203,30 @@ func TestGenerateFlaggedPostReport(t *testing.T) {
}
require.True(t, foundEdit, "edit history entry should be present in the report archive")
})
t.Run("Should not allow generating a report for a post in a DM channel", func(t *testing.T) {
appErr := setBasicCommonReviewerConfig(th)
require.Nil(t, appErr)
post := createDmPost(t, th, client)
report, resp, err := client.GenerateFlaggedPostReport(context.Background(), post.Id, &model.FlagContentActionRequest{})
require.Error(t, err)
CheckBadRequestStatus(t, resp)
CheckErrorID(t, err, "api.data_spillage.error.invalid_channel_type")
require.Empty(t, report)
})
t.Run("Should not allow generating a report for a post in a GM channel", func(t *testing.T) {
appErr := setBasicCommonReviewerConfig(th)
require.Nil(t, appErr)
post := createGmPost(t, th, client)
report, resp, err := client.GenerateFlaggedPostReport(context.Background(), post.Id, &model.FlagContentActionRequest{})
require.Error(t, err)
CheckBadRequestStatus(t, resp)
CheckErrorID(t, err, "api.data_spillage.error.invalid_channel_type")
require.Empty(t, report)
})
}
@@ -113,6 +113,19 @@ func flagPostViaAPI(t *testing.T, client *model.Client4, postId string) {
require.Equal(t, http.StatusOK, resp.StatusCode)
}
func createDmPost(t *testing.T, th *TestHelper, client *model.Client4) *model.Post {
t.Helper()
dmChannel := th.CreateDmChannel(t, th.BasicUser2)
return th.CreatePostWithClient(t, client, dmChannel)
}
func createGmPost(t *testing.T, th *TestHelper, client *model.Client4) *model.Post {
t.Helper()
gmChannel, appErr := th.App.CreateGroupChannel(th.Context, []string{th.BasicUser.Id, th.BasicUser2.Id, th.SystemAdminUser.Id}, th.BasicUser.Id)
require.Nil(t, appErr)
return th.CreatePostWithClient(t, client, gmChannel)
}
func uploadFileAndCreatePost(t *testing.T, th *TestHelper, client *model.Client4) (*model.Post, *model.FileInfo) {
t.Helper()
data, err := testutils.ReadTestFile("test.png")
@@ -440,6 +453,36 @@ func TestGetPostPropertyValues(t *testing.T) {
require.NotNil(t, propertyValues)
require.Len(t, propertyValues, 6)
})
t.Run("Should not allow getting property values of a post in a DM channel", func(t *testing.T) {
th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuEnterpriseAdvanced))
appErr := setBasicCommonReviewerConfig(th)
require.Nil(t, appErr)
post := createDmPost(t, th, client)
propertyValues, resp, err := client.GetPostPropertyValues(context.Background(), post.Id)
require.Error(t, err)
CheckBadRequestStatus(t, resp)
CheckErrorID(t, err, "api.data_spillage.error.invalid_channel_type")
require.Nil(t, propertyValues)
})
t.Run("Should not allow getting property values of a post in a GM channel", func(t *testing.T) {
th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuEnterpriseAdvanced))
appErr := setBasicCommonReviewerConfig(th)
require.Nil(t, appErr)
post := createGmPost(t, th, client)
propertyValues, resp, err := client.GetPostPropertyValues(context.Background(), post.Id)
require.Error(t, err)
CheckBadRequestStatus(t, resp)
CheckErrorID(t, err, "api.data_spillage.error.invalid_channel_type")
require.Nil(t, propertyValues)
})
}
func TestGetFlaggedPost(t *testing.T) {
@@ -519,6 +562,36 @@ func TestGetFlaggedPost(t *testing.T) {
require.Equal(t, 1, len(flaggedPost.Metadata.Files))
require.Equal(t, fileInfo.Id, flaggedPost.Metadata.Files[0].Id)
})
t.Run("Should not allow getting a post in a DM channel", func(t *testing.T) {
th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuEnterpriseAdvanced))
appErr := setBasicCommonReviewerConfig(th)
require.Nil(t, appErr)
post := createDmPost(t, th, client)
flaggedPost, resp, err := client.GetContentFlaggedPost(context.Background(), post.Id)
require.Error(t, err)
CheckBadRequestStatus(t, resp)
CheckErrorID(t, err, "api.data_spillage.error.invalid_channel_type")
require.Nil(t, flaggedPost)
})
t.Run("Should not allow getting a post in a GM channel", func(t *testing.T) {
th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuEnterpriseAdvanced))
appErr := setBasicCommonReviewerConfig(th)
require.Nil(t, appErr)
post := createGmPost(t, th, client)
flaggedPost, resp, err := client.GetContentFlaggedPost(context.Background(), post.Id)
require.Error(t, err)
CheckBadRequestStatus(t, resp)
CheckErrorID(t, err, "api.data_spillage.error.invalid_channel_type")
require.Nil(t, flaggedPost)
})
}
func TestFlagPost(t *testing.T) {
@@ -654,6 +727,73 @@ func TestFlagPost(t *testing.T) {
require.Error(t, err)
CheckBadRequestStatus(t, response)
})
t.Run("Should not allow flagging a post in a DM channel", func(t *testing.T) {
th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuEnterpriseAdvanced))
defer th.RemoveLicense(t)
appErr := setBasicCommonReviewerConfig(th)
require.Nil(t, appErr)
dmChannel := th.CreateDmChannel(t, th.BasicUser2)
post := th.CreatePostWithClient(t, client, dmChannel)
flagRequest := &model.FlagContentRequest{
Reason: "Classification mismatch",
Comment: "This is sensitive data",
}
resp, err := client.FlagPostForContentReview(context.Background(), post.Id, flagRequest)
require.Error(t, err)
CheckBadRequestStatus(t, resp)
CheckErrorID(t, err, "api.data_spillage.error.invalid_channel_type")
})
t.Run("Should not allow flagging a post in a GM channel", func(t *testing.T) {
th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuEnterpriseAdvanced))
defer th.RemoveLicense(t)
appErr := setBasicCommonReviewerConfig(th)
require.Nil(t, appErr)
gmChannel, appErr := th.App.CreateGroupChannel(th.Context, []string{th.BasicUser.Id, th.BasicUser2.Id, th.SystemAdminUser.Id}, th.BasicUser.Id)
require.Nil(t, appErr)
post := th.CreatePostWithClient(t, client, gmChannel)
flagRequest := &model.FlagContentRequest{
Reason: "Classification mismatch",
Comment: "This is sensitive data",
}
resp, err := client.FlagPostForContentReview(context.Background(), post.Id, flagRequest)
require.Error(t, err)
CheckBadRequestStatus(t, resp)
CheckErrorID(t, err, "api.data_spillage.error.invalid_channel_type")
})
t.Run("Should reject DM posts by channel type before the team enabled check", func(t *testing.T) {
th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuEnterpriseAdvanced))
defer th.RemoveLicense(t)
// With per team reviewers, ContentFlaggingEnabledForTeam("") returns false for a DM, so
// without the channel type check running first this would surface the misleading
// "not_available_on_team" error instead.
appErr := setNonReviewerConfig(th)
require.Nil(t, appErr)
dmChannel := th.CreateDmChannel(t, th.BasicUser2)
post := th.CreatePostWithClient(t, client, dmChannel)
flagRequest := &model.FlagContentRequest{
Reason: "Classification mismatch",
Comment: "This is sensitive data",
}
resp, err := client.FlagPostForContentReview(context.Background(), post.Id, flagRequest)
require.Error(t, err)
CheckBadRequestStatus(t, resp)
CheckErrorID(t, err, "api.data_spillage.error.invalid_channel_type")
})
}
func TestGetTeamPostReportingFeatureStatus(t *testing.T) {
@@ -914,6 +1054,36 @@ func TestAssignContentFlaggingReviewer(t *testing.T) {
require.NoError(t, err)
require.Equal(t, http.StatusOK, resp.StatusCode)
})
t.Run("Should not allow assigning a reviewer to a post in a DM channel", func(t *testing.T) {
th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuEnterpriseAdvanced))
defer th.RemoveLicense(t)
appErr := setBasicCommonReviewerConfig(th)
require.Nil(t, appErr)
post := createDmPost(t, th, client)
resp, err := client.AssignContentFlaggingReviewer(context.Background(), post.Id, th.BasicUser.Id)
require.Error(t, err)
CheckBadRequestStatus(t, resp)
CheckErrorID(t, err, "api.data_spillage.error.invalid_channel_type")
})
t.Run("Should not allow assigning a reviewer to a post in a GM channel", func(t *testing.T) {
th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuEnterpriseAdvanced))
defer th.RemoveLicense(t)
appErr := setBasicCommonReviewerConfig(th)
require.Nil(t, appErr)
post := createGmPost(t, th, client)
resp, err := client.AssignContentFlaggingReviewer(context.Background(), post.Id, th.BasicUser.Id)
require.Error(t, err)
CheckBadRequestStatus(t, resp)
CheckErrorID(t, err, "api.data_spillage.error.invalid_channel_type")
})
}
func TestRemoveFlaggedPost(t *testing.T) {
@@ -1076,6 +1246,42 @@ func TestRemoveFlaggedPost(t *testing.T) {
_, err2 = th.App.Srv().Store().Post().GetSingle(th.Context, editHistoryPostId, true)
require.Error(t, err2, "Edit history post should be permanently deleted")
})
t.Run("Should not allow removing a post in a DM channel", func(t *testing.T) {
th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuEnterpriseAdvanced))
defer th.RemoveLicense(t)
appErr := setBasicCommonReviewerConfig(th)
require.Nil(t, appErr)
post := createDmPost(t, th, client)
actionRequest := &model.FlagContentActionRequest{
Comment: "Removing this post",
}
resp, err := client.RemoveFlaggedPost(context.Background(), post.Id, actionRequest)
require.Error(t, err)
CheckBadRequestStatus(t, resp)
CheckErrorID(t, err, "api.data_spillage.error.invalid_channel_type")
})
t.Run("Should not allow removing a post in a GM channel", func(t *testing.T) {
th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuEnterpriseAdvanced))
defer th.RemoveLicense(t)
appErr := setBasicCommonReviewerConfig(th)
require.Nil(t, appErr)
post := createGmPost(t, th, client)
actionRequest := &model.FlagContentActionRequest{
Comment: "Removing this post",
}
resp, err := client.RemoveFlaggedPost(context.Background(), post.Id, actionRequest)
require.Error(t, err)
CheckBadRequestStatus(t, resp)
CheckErrorID(t, err, "api.data_spillage.error.invalid_channel_type")
})
}
func TestKeepFlaggedPost(t *testing.T) {
@@ -1309,4 +1515,40 @@ func TestKeepFlaggedPost(t *testing.T) {
}
}
})
t.Run("Should not allow keeping a post in a DM channel", func(t *testing.T) {
th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuEnterpriseAdvanced))
defer th.RemoveLicense(t)
appErr := setBasicCommonReviewerConfig(th)
require.Nil(t, appErr)
post := createDmPost(t, th, client)
actionRequest := &model.FlagContentActionRequest{
Comment: "Keeping this post",
}
resp, err := client.KeepFlaggedPost(context.Background(), post.Id, actionRequest)
require.Error(t, err)
CheckBadRequestStatus(t, resp)
CheckErrorID(t, err, "api.data_spillage.error.invalid_channel_type")
})
t.Run("Should not allow keeping a post in a GM channel", func(t *testing.T) {
th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuEnterpriseAdvanced))
defer th.RemoveLicense(t)
appErr := setBasicCommonReviewerConfig(th)
require.Nil(t, appErr)
post := createGmPost(t, th, client)
actionRequest := &model.FlagContentActionRequest{
Comment: "Keeping this post",
}
resp, err := client.KeepFlaggedPost(context.Background(), post.Id, actionRequest)
require.Error(t, err)
CheckBadRequestStatus(t, resp)
CheckErrorID(t, err, "api.data_spillage.error.invalid_channel_type")
})
}
+5
View File
@@ -555,6 +555,11 @@ func getFile(c *Context, w http.ResponseWriter, r *http.Request) {
return
}
checkChannelFlaggable(c, channel)
if c.Err != nil {
return
}
requireTeamContentReviewer(c, c.AppContext.Session().UserId, channel.TeamId)
if c.Err != nil {
return
+23
View File
@@ -1042,6 +1042,29 @@ func TestGetFileAsContentReviewer(t *testing.T) {
})
}
})
t.Run("reviewer cannot fetch a file from a DM or GM channel", func(t *testing.T) {
sent, err := testutils.ReadTestFile("test.png")
require.NoError(t, err)
dmChannel := th.CreateDmChannel(t, th.BasicUser2)
gmChannel, appErr := th.App.CreateGroupChannel(th.Context, []string{th.BasicUser.Id, th.BasicUser2.Id, th.SystemAdminUser.Id}, th.BasicUser.Id)
require.Nil(t, appErr)
for _, channel := range []*model.Channel{dmChannel, gmChannel} {
fileResponse, _, err := th.Client.UploadFile(context.Background(), sent, channel.Id, "test.png")
require.NoError(t, err)
require.Len(t, fileResponse.FileInfos, 1)
post := th.CreatePostInChannelWithFiles(t, channel, fileResponse.FileInfos[0])
data, response, err := th.Client.GetFileAsContentReviewer(context.Background(), fileResponse.FileInfos[0].Id, post.Id)
require.Error(t, err)
CheckBadRequestStatus(t, response)
CheckErrorID(t, err, "api.data_spillage.error.invalid_channel_type")
require.Empty(t, data, "no file content should be returned")
}
})
}
func TestGetFileAsSystemAdmin(t *testing.T) {
+12 -2
View File
@@ -326,8 +326,8 @@ func getTeam(c *Context, w http.ResponseWriter, r *http.Request) {
}
flaggedPostId := r.URL.Query().Get("flagged_post_id")
requireFlaggedPost(c, flaggedPostId)
if c.Err != nil {
if flaggedPostId == "" {
c.SetInvalidParam("flagged_post_id")
return
}
@@ -343,11 +343,21 @@ func getTeam(c *Context, w http.ResponseWriter, r *http.Request) {
return
}
checkChannelFlaggable(c, channel)
if c.Err != nil {
return
}
if channel.TeamId != team.Id {
c.Err = model.NewAppError("getTeam", "api.team.get_team.flagged_post_mismatch.app_error", nil, "", http.StatusBadRequest)
return
}
requireFlaggedPost(c, flaggedPostId)
if c.Err != nil {
return
}
isContentReviewer = true
}
+75
View File
@@ -591,6 +591,81 @@ func TestGetTeam(t *testing.T) {
require.Error(t, err)
CheckForbiddenStatus(t, resp)
})
t.Run("Content reviewer should not be able to get a team via a DM or GM post", func(t *testing.T) {
th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuEnterpriseAdvanced))
appErr := setBasicCommonReviewerConfig(th)
require.Nil(t, appErr)
contentReviewClient := th.CreateClient()
_, _, err := contentReviewClient.Login(context.Background(), th.BasicUser.Email, th.BasicUser.Password)
require.NoError(t, err)
flagRequest := model.FlagContentRequest{
Reason: "Classification mismatch",
Comment: "This is sensitive content",
}
testCases := []struct {
name string
post *model.Post
flagged bool
}{
{"flagged DM post", createDmPost(t, th, contentReviewClient), true},
{"flagged GM post", createGmPost(t, th, contentReviewClient), true},
{"unflagged DM post", createDmPost(t, th, contentReviewClient), false},
{"unflagged GM post", createGmPost(t, th, contentReviewClient), false},
}
for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
if testCase.flagged {
flagErr := th.App.FlagPost(th.Context, testCase.post, "", th.BasicUser.Id, flagRequest)
require.Nil(t, flagErr)
}
_, resp, err := contentReviewClient.GetTeamAsContentReviewer(context.Background(), th.BasicTeam.Id, "", testCase.post.Id)
require.Error(t, err)
CheckBadRequestStatus(t, resp)
CheckErrorID(t, err, "api.data_spillage.error.invalid_channel_type")
})
}
})
t.Run("Content reviewer should not be able to get a team via a post from another team", func(t *testing.T) {
th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuEnterpriseAdvanced))
appErr := setBasicCommonReviewerConfig(th)
require.Nil(t, appErr)
contentReviewClient := th.CreateClient()
_, _, err := contentReviewClient.Login(context.Background(), th.BasicUser.Email, th.BasicUser.Password)
require.NoError(t, err)
otherTeam := th.CreateTeam(t)
otherChannel := th.CreateChannelWithClientAndTeam(t, contentReviewClient, model.ChannelTypeOpen, otherTeam.Id)
// As above, the flagged and the unflagged post have to fail identically so the
// error doesn't disclose the flag status of a post outside the requested team.
for _, testCase := range []struct {
name string
flagged bool
}{
{"flagged post", true},
{"unflagged post", false},
} {
t.Run(testCase.name, func(t *testing.T) {
post := th.CreatePostWithClient(t, contentReviewClient, otherChannel)
if testCase.flagged {
flagPostViaAPI(t, contentReviewClient, post.Id)
}
_, resp, err := contentReviewClient.GetTeamAsContentReviewer(context.Background(), th.BasicTeam.Id, "", post.Id)
require.Error(t, err)
CheckBadRequestStatus(t, resp)
CheckErrorID(t, err, "api.team.get_team.flagged_post_mismatch.app_error")
})
}
})
}
func TestGetTeamSanitization(t *testing.T) {
+4
View File
@@ -2165,6 +2165,10 @@
"id": "api.data_spillage.error.disabled",
"translation": "Data Spillage Handling feature is disabled."
},
{
"id": "api.data_spillage.error.invalid_channel_type",
"translation": "Data Spillage Handling is not available for direct and group message channels."
},
{
"id": "api.data_spillage.error.invalid_post_type",
"translation": "Quarantining a post of type '{{.PostType}}' is not allowed."