diff --git a/server/channels/api4/post.go b/server/channels/api4/post.go index cf2f0e36d5..cb7ad741fc 100644 --- a/server/channels/api4/post.go +++ b/server/channels/api4/post.go @@ -5,6 +5,7 @@ package api4 import ( "encoding/json" + "fmt" "net/http" "strconv" "time" @@ -75,6 +76,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 @@ -827,6 +834,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 @@ -888,6 +902,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 diff --git a/server/channels/api4/post_test.go b/server/channels/api4/post_test.go index d3aaea401e..11e1ff9b89 100644 --- a/server/channels/api4/post_test.go +++ b/server/channels/api4/post_test.go @@ -173,6 +173,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(context.Background(), &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(context.Background(), post) @@ -418,7 +438,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) @@ -428,7 +448,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(context.Background(), &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) { @@ -1085,6 +1126,26 @@ func TestUpdatePost(t *testing.T) { CheckBadRequestStatus(t, resp) }) + 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(context.Background(), 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(context.Background()) _, resp, err := client.UpdatePost(context.Background(), rpost.Id, rpost) @@ -1295,6 +1356,33 @@ func TestPatchPost(t *testing.T) { CheckBadRequestStatus(t, resp) require.Equal(t, "api.post.update_post.permissions_time_limit.app_error", err.(*model.AppError).Id, "should be time limit error") }) + + 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(context.Background(), post) + require.NoError(t, createErr) + + patch := &model.PostPatch{} + patch.Props = &model.StringInterface{model.PostPropsFromWebhook: "true"} + _, patchResp, patchErr := client.PatchPost(context.Background(), post.Id, patch) + + require.Error(t, patchErr) + CheckBadRequestStatus(t, patchResp) + }) } func TestPinPost(t *testing.T) { diff --git a/server/channels/app/post.go b/server/channels/app/post.go index 546702dab8..ab4c6ec298 100644 --- a/server/channels/app/post.go +++ b/server/channels/app/post.go @@ -98,8 +98,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] isCRTEnabled := a.IsCRTEnabledForUser(c, post.UserId) isCRTReply := post.RootId != "" && isCRTEnabled if !fromWebhook && !fromBot && !isCRTReply { @@ -236,11 +236,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 diff --git a/server/channels/web/context.go b/server/channels/web/context.go index 8e28329011..215afb6a0c 100644 --- a/server/channels/web/context.go +++ b/server/channels/web/context.go @@ -222,6 +222,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) } @@ -270,6 +274,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 diff --git a/server/public/model/post.go b/server/public/model/post.go index 169f810112..fe0d4d6fb1 100644 --- a/server/public/model/post.go +++ b/server/public/model/post.go @@ -63,15 +63,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" @@ -470,6 +473,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() diff --git a/server/public/model/post_test.go b/server/public/model/post_test.go index 747ceb19fd..1e47c52bd5 100644 --- a/server/public/model/post_test.go +++ b/server/public/model/post_test.go @@ -142,6 +142,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{} diff --git a/server/public/model/session.go b/server/public/model/session.go index d219909e53..51a747e2e0 100644 --- a/server/public/model/session.go +++ b/server/public/model/session.go @@ -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() } diff --git a/server/public/model/session_test.go b/server/public/model/session_test.go index e1814e80e6..2d2355b7fd 100644 --- a/server/public/model/session_test.go +++ b/server/public/model/session_test.go @@ -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()) + }) + } +}