Skip to content

Commit

Permalink
MM-52792: Backport to 7.8 (#24195) (#24935)
Browse files Browse the repository at this point in the history
  • Loading branch information
calebroseland committed Oct 18, 2023
1 parent bd98594 commit 745ff30
Show file tree
Hide file tree
Showing 8 changed files with 249 additions and 13 deletions.
21 changes: 21 additions & 0 deletions api4/post.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package api4

import (
"encoding/json"
"fmt"
"net/http"
"strconv"
"time"
Expand Down Expand Up @@ -74,6 +75,12 @@ func createPost(c *Context, w http.ResponseWriter, r *http.Request) {
c.SetPermissionError(model.PermissionCreatePost)
return
}
if *c.App.Config().ServiceSettings.ExperimentalEnableHardenedMode {
if reservedProps := post.ContainsIntegrationsReservedProps(); len(reservedProps) > 0 && !c.AppContext.Session().IsIntegration() {
c.SetInvalidParamWithDetails("props", fmt.Sprintf("Cannot use props reserved for integrations. props: %v", reservedProps))
return
}
}

if post.CreateAt != 0 && !c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionManageSystem) {
post.CreateAt = 0
Expand Down Expand Up @@ -750,6 +757,13 @@ func updatePost(c *Context, w http.ResponseWriter, r *http.Request) {
return
}

if *c.App.Config().ServiceSettings.ExperimentalEnableHardenedMode {
if reservedProps := post.ContainsIntegrationsReservedProps(); len(reservedProps) > 0 && !c.AppContext.Session().IsIntegration() {
c.SetInvalidParamWithDetails("props", fmt.Sprintf("Cannot use props reserved for integrations. props: %v", reservedProps))
return
}
}

if !c.App.SessionHasPermissionToChannelByPost(*c.AppContext.Session(), c.Params.PostId, model.PermissionEditPost) {
c.SetPermissionError(model.PermissionEditPost)
return
Expand Down Expand Up @@ -806,6 +820,13 @@ func patchPost(c *Context, w http.ResponseWriter, r *http.Request) {
audit.AddEventParameterAuditable(auditRec, "patch", &post)
defer c.LogAuditRecWithLevel(auditRec, app.LevelContent)

if *c.App.Config().ServiceSettings.ExperimentalEnableHardenedMode {
if reservedProps := post.ContainsIntegrationsReservedProps(); len(reservedProps) > 0 && !c.AppContext.Session().IsIntegration() {
c.SetInvalidParamWithDetails("props", fmt.Sprintf("Cannot use props reserved for integrations. props: %v", reservedProps))
return
}
}

// Updating the file_ids of a post is not a supported operation and will be ignored
post.FileIds = nil

Expand Down
92 changes: 90 additions & 2 deletions api4/post_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,26 @@ func TestCreatePost(t *testing.T) {
}
})

t.Run("err with integrations-reserved props", func(t *testing.T) {
originalHardenedModeSetting := *th.App.Config().ServiceSettings.ExperimentalEnableHardenedMode
th.App.UpdateConfig(func(cfg *model.Config) {
*cfg.ServiceSettings.ExperimentalEnableHardenedMode = true
})

defer th.App.UpdateConfig(func(cfg *model.Config) {
*cfg.ServiceSettings.ExperimentalEnableHardenedMode = originalHardenedModeSetting
})

_, postResp, postErr := client.CreatePost(&model.Post{
ChannelId: th.BasicChannel.Id,
Message: "with props",
Props: model.StringInterface{model.PostPropsFromWebhook: "true"},
})

require.Error(t, postErr)
CheckBadRequestStatus(t, postResp)
})

post.RootId = ""
post.Type = model.PostTypeSystemGeneric
_, resp, err := client.CreatePost(post)
Expand Down Expand Up @@ -245,7 +265,7 @@ func TestCreatePostWithOAuthClient(t *testing.T) {
Message: "test message",
})
require.NoError(t, err)
assert.NotContains(t, post.GetProps(), "from_oauth_app", "contains from_oauth_app prop when not using OAuth client")
assert.NotContains(t, post.GetProps(), model.PostPropsFromOAuthApp, fmt.Sprintf("contains %s prop when not using OAuth client", model.PostPropsOverrideUsername))

client := th.CreateClient()
client.SetOAuthToken(session.Token)
Expand All @@ -255,7 +275,28 @@ func TestCreatePostWithOAuthClient(t *testing.T) {
})

require.NoError(t, err)
assert.Contains(t, post.GetProps(), "from_oauth_app", "missing from_oauth_app prop when using OAuth client")
assert.Contains(t, post.GetProps(), model.PostPropsFromOAuthApp, fmt.Sprintf("missing %s prop when using OAuth client", model.PostPropsOverrideUsername))

t.Run("allow username and icon overrides", func(t *testing.T) {
originalHardenedModeSetting := *th.App.Config().ServiceSettings.ExperimentalEnableHardenedMode
th.App.UpdateConfig(func(cfg *model.Config) {
*cfg.ServiceSettings.ExperimentalEnableHardenedMode = true
})

defer th.App.UpdateConfig(func(cfg *model.Config) {
*cfg.ServiceSettings.ExperimentalEnableHardenedMode = originalHardenedModeSetting
})

post, _, err = client.CreatePost(&model.Post{
ChannelId: th.BasicChannel.Id,
Message: "test message",
Props: model.StringInterface{model.PostPropsOverrideUsername: "newUsernameValue", model.PostPropsOverrideIconURL: "iconUrlOverrideValue"},
})

require.NoError(t, err)
assert.Contains(t, post.GetProps(), model.PostPropsOverrideUsername, fmt.Sprintf("missing %s prop when using OAuth client", model.PostPropsOverrideUsername))
assert.Contains(t, post.GetProps(), model.PostPropsOverrideIconURL, fmt.Sprintf("missing %s prop when using OAuth client", model.PostPropsOverrideIconURL))
})
}

func TestCreatePostEphemeral(t *testing.T) {
Expand Down Expand Up @@ -850,6 +891,26 @@ func TestUpdatePost(t *testing.T) {
assert.NotEqual(t, rpost3.Attachments(), rrupost3.Attachments())
})

t.Run("err with integrations-reserved props", func(t *testing.T) {
originalHardenedModeSetting := *th.App.Config().ServiceSettings.ExperimentalEnableHardenedMode
th.App.UpdateConfig(func(cfg *model.Config) {
*cfg.ServiceSettings.ExperimentalEnableHardenedMode = true
})

defer th.App.UpdateConfig(func(cfg *model.Config) {
*cfg.ServiceSettings.ExperimentalEnableHardenedMode = originalHardenedModeSetting
})

_, resp, err := client.UpdatePost(rpost.Id, &model.Post{
ChannelId: th.BasicChannel.Id,
Message: "with props",
Props: model.StringInterface{model.PostPropsFromWebhook: "true"},
})

require.Error(t, err)
CheckBadRequestStatus(t, resp)
})

t.Run("logged out", func(t *testing.T) {
client.Logout()
_, resp, err := client.UpdatePost(rpost.Id, rpost)
Expand Down Expand Up @@ -1035,6 +1096,33 @@ func TestPatchPost(t *testing.T) {
_, _, err = client.PatchPost(post.Id, patch)
require.NoError(t, err)
})

t.Run("err with integrations-reserved props", func(t *testing.T) {

originalHardenedModeSetting := *th.App.Config().ServiceSettings.ExperimentalEnableHardenedMode
th.App.UpdateConfig(func(cfg *model.Config) {
*cfg.ServiceSettings.ExperimentalEnableHardenedMode = true
})

defer th.App.UpdateConfig(func(cfg *model.Config) {
*cfg.ServiceSettings.ExperimentalEnableHardenedMode = originalHardenedModeSetting
})

post := &model.Post{
ChannelId: channel.Id,
Message: "#hashtag a message",
CreateAt: model.GetMillis() - 2000,
}
post, _, createErr := th.SystemAdminClient.CreatePost(post)
require.NoError(t, createErr)

patch := &model.PostPatch{}
patch.Props = &model.StringInterface{model.PostPropsFromWebhook: "true"}
_, patchResp, patchErr := client.PatchPost(post.Id, patch)

require.Error(t, patchErr)
CheckBadRequestStatus(t, patchResp)
})
}

func TestPinPost(t *testing.T) {
Expand Down
8 changes: 4 additions & 4 deletions app/post.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ func (a *App) CreatePostAsUser(c request.CTX, post *model.Post, currentSessionId
// the post does NOT have from_webhook prop set (e.g. Zapier app), and
// the post does NOT have from_bot set (e.g. from discovering the user is a bot within CreatePost), and
// the post is NOT a reply post with CRT enabled
_, fromWebhook := post.GetProps()["from_webhook"]
_, fromBot := post.GetProps()["from_bot"]
_, fromWebhook := post.GetProps()[model.PostPropsFromWebhook]
_, fromBot := post.GetProps()[model.PostPropsFromBot]
isCRTReply := post.RootId != "" && a.IsCRTEnabledForUser(c, post.UserId)
if !fromWebhook && !fromBot && !isCRTReply {
if _, err := a.MarkChannelsAsViewed(c, []string{post.ChannelId}, post.UserId, currentSessionId, true); err != nil {
Expand Down Expand Up @@ -200,11 +200,11 @@ func (a *App) CreatePost(c request.CTX, post *model.Post, channel *model.Channel
}

if user.IsBot {
post.AddProp("from_bot", "true")
post.AddProp(model.PostPropsFromBot, "true")
}

if c.Session().IsOAuth {
post.AddProp("from_oauth_app", "true")
post.AddProp(model.PostPropsFromOAuthApp, "true")
}

var ephemeralPost *model.Post
Expand Down
50 changes: 43 additions & 7 deletions model/post.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,15 +62,18 @@ const (

PropsAddChannelMember = "add_channel_member"

PostPropsAddedUserId = "addedUserId"
PostPropsDeleteBy = "deleteBy"
PostPropsOverrideIconURL = "override_icon_url"
PostPropsOverrideIconEmoji = "override_icon_emoji"

PostPropsAddedUserId = "addedUserId"
PostPropsDeleteBy = "deleteBy"
PostPropsOverrideIconURL = "override_icon_url"
PostPropsOverrideIconEmoji = "override_icon_emoji"
PostPropsOverrideUsername = "override_username"
PostPropsFromWebhook = "from_webhook"
PostPropsFromBot = "from_bot"
PostPropsFromOAuthApp = "from_oauth_app"
PostPropsWebhookDisplayName = "webhook_display_name"
PostPropsMentionHighlightDisabled = "mentionHighlightDisabled"
PostPropsGroupHighlightDisabled = "disable_group_highlight"

PostPropsPreviewedPost = "previewed_post"
PostPropsPreviewedPost = "previewed_post"

PostPriorityUrgent = "urgent"
PostPropsRequestedAck = "requested_ack"
Expand Down Expand Up @@ -460,6 +463,39 @@ func (o *Post) SanitizeProps() {
}
}

func (o *Post) ContainsIntegrationsReservedProps() []string {
return containsIntegrationsReservedProps(o.GetProps())
}

func (o *PostPatch) ContainsIntegrationsReservedProps() []string {
if o == nil || o.Props == nil {
return nil
}
return containsIntegrationsReservedProps(*o.Props)
}

func containsIntegrationsReservedProps(props StringInterface) []string {
foundProps := []string{}

if props != nil {
reservedProps := []string{
PostPropsFromWebhook,
PostPropsOverrideUsername,
PostPropsWebhookDisplayName,
PostPropsOverrideIconURL,
PostPropsOverrideIconEmoji,
}

for _, key := range reservedProps {
if _, ok := props[key]; ok {
foundProps = append(foundProps, key)
}
}
}

return foundProps
}

func (o *Post) PreSave() {
if o.Id == "" {
o.Id = NewId()
Expand Down
35 changes: 35 additions & 0 deletions model/post_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,41 @@ func TestPostSanitizeProps(t *testing.T) {
require.NotNil(t, post3.GetProp("attachments"))
}

func TestPost_ContainsIntegrationsReservedProps(t *testing.T) {
post1 := &Post{
Message: "test",
}
keys1 := post1.ContainsIntegrationsReservedProps()
require.Len(t, keys1, 0)

post2 := &Post{
Message: "test",
Props: StringInterface{
"from_webhook": "true",
"webhook_display_name": "overridden_display_name",
"override_username": "overridden_username",
"override_icon_url": "a-custom-url",
"override_icon_emoji": ":custom_emoji_name:",
},
}
keys2 := post2.ContainsIntegrationsReservedProps()
require.Len(t, keys2, 5)
}

func TestPostPatch_ContainsIntegrationsReservedProps(t *testing.T) {
postPatch1 := &PostPatch{
Props: &StringInterface{
"from_webhook": "true",
},
}
keys1 := postPatch1.ContainsIntegrationsReservedProps()
require.Len(t, keys1, 1)

postPatch2 := &PostPatch{}
keys2 := postPatch2.ContainsIntegrationsReservedProps()
require.Len(t, keys2, 0)
}

func TestPost_AttachmentsEqual(t *testing.T) {
post1 := &Post{}
post2 := &Post{}
Expand Down
28 changes: 28 additions & 0 deletions model/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,34 @@ func (s *Session) IsOAuthUser() bool {
return isOAuthUser
}

func (s *Session) IsBotUser() bool {
val, ok := s.Props[SessionPropIsBot]
if !ok {
return false
}
if val == SessionPropIsBotValue {
return true
}
return false
}

func (s *Session) IsUserAccessToken() bool {
val, ok := s.Props[SessionPropType]
if !ok {
return false
}
if val == SessionTypeUserAccessToken {
return true
}
return false
}

// Returns true when session is authenticated as a bot, by personal access token, or is an OAuth app.
// Does not indicate other forms of integrations e.g. webhooks, slash commands, etc.
func (s *Session) IsIntegration() bool {
return s.IsBotUser() || s.IsUserAccessToken() || s.IsOAuth
}

func (s *Session) IsSSOLogin() bool {
return s.IsOAuthUser() || s.IsSaml()
}
Expand Down
20 changes: 20 additions & 0 deletions model/session_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,3 +133,23 @@ func TestSessionIsOAuthUser(t *testing.T) {
})
}
}

func TestIsIntegration(t *testing.T) {
testCases := []struct {
Description string
Session Session
IsIntegration bool
}{
{"False on empty props", Session{}, false},
{"True when is OAuth App", Session{IsOAuth: true}, true},
{"True when session is bot", Session{Props: StringMap{SessionPropIsBot: SessionPropIsBotValue}}, true},
{"True when session is user access token", Session{Props: StringMap{SessionPropType: SessionTypeUserAccessToken}}, true},
{"Not affected by Props[UserAuthServiceIsOAuth]", Session{Props: StringMap{UserAuthServiceIsOAuth: strconv.FormatBool(true)}}, false},
}

for _, tc := range testCases {
t.Run(tc.Description, func(t *testing.T) {
require.Equal(t, tc.IsIntegration, tc.Session.IsIntegration())
})
}
}
8 changes: 8 additions & 0 deletions web/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,10 @@ func (c *Context) SetInvalidParam(parameter string) {
c.Err = NewInvalidParamError(parameter)
}

func (c *Context) SetInvalidParamWithDetails(parameter string, details string) {
c.Err = NewInvalidParamDetailedError(parameter, details)
}

func (c *Context) SetInvalidParamWithErr(parameter string, err error) {
c.Err = NewInvalidParamError(parameter).Wrap(err)
}
Expand Down Expand Up @@ -269,6 +273,10 @@ func (c *Context) HandleEtag(etag string, routeName string, w http.ResponseWrite
return false
}

func NewInvalidParamDetailedError(parameter string, details string) *model.AppError {
err := model.NewAppError("Context", "api.context.invalid_body_param.app_error", map[string]any{"Name": parameter}, details, http.StatusBadRequest)
return err
}
func NewInvalidParamError(parameter string) *model.AppError {
err := model.NewAppError("Context", "api.context.invalid_body_param.app_error", map[string]any{"Name": parameter}, "", http.StatusBadRequest)
return err
Expand Down

0 comments on commit 745ff30

Please sign in to comment.