From 37b1e6d048fc8302c727c3bc7ce73ac32c2ba93c Mon Sep 17 00:00:00 2001 From: Martin Kraft Date: Wed, 28 Jul 2021 13:47:45 -0400 Subject: [PATCH] MM-37417: Push notification authz fix. (#18009) * MM-37417: Push notification authz fix. * MM-37417: Tests new app method. --- api4/post.go | 35 ++-------------------------- api4/system.go | 5 ++++ api4/system_test.go | 20 +++++++++++++++- app/app_iface.go | 1 + app/opentracing/opentracing_layer.go | 22 +++++++++++++++++ app/post.go | 24 +++++++++++++++++++ app/post_test.go | 26 +++++++++++++++++++++ 7 files changed, 99 insertions(+), 34 deletions(-) diff --git a/api4/post.go b/api4/post.go index cf70752a7d..7e2dfa3c2f 100644 --- a/api4/post.go +++ b/api4/post.go @@ -348,30 +348,12 @@ func getPost(c *Context, w http.ResponseWriter, r *http.Request) { return } - post, err := c.App.GetSinglePost(c.Params.PostId) + post, err := c.App.GetPostIfAuthorized(c.Params.PostId, c.AppContext.Session()) if err != nil { c.Err = err return } - channel, err := c.App.GetChannel(post.ChannelId) - if err != nil { - c.Err = err - return - } - - if !c.App.SessionHasPermissionToChannel(*c.AppContext.Session(), channel.Id, model.PermissionReadChannel) { - if channel.Type == model.ChannelTypeOpen { - if !c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), channel.TeamId, model.PermissionReadPublicChannel) { - c.SetPermissionError(model.PermissionReadPublicChannel) - return - } - } else { - c.SetPermissionError(model.PermissionReadChannel) - return - } - } - post = c.App.PreparePostForClient(post, false, false) if c.HandleEtag(post.Etag(), "Get Post", w, r) { @@ -442,24 +424,11 @@ func getPostThread(c *Context, w http.ResponseWriter, r *http.Request) { return } - channel, err := c.App.GetChannel(post.ChannelId) - if err != nil { + if _, err = c.App.GetPostIfAuthorized(post.Id, c.AppContext.Session()); err != nil { c.Err = err return } - if !c.App.SessionHasPermissionToChannel(*c.AppContext.Session(), channel.Id, model.PermissionReadChannel) { - if channel.Type == model.ChannelTypeOpen { - if !c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), channel.TeamId, model.PermissionReadPublicChannel) { - c.SetPermissionError(model.PermissionReadPublicChannel) - return - } - } else { - c.SetPermissionError(model.PermissionReadChannel) - return - } - } - if c.HandleEtag(list.Etag(), "Get Post Thread", w, r) { return } diff --git a/api4/system.go b/api4/system.go index 4ae5765595..d3374433d7 100644 --- a/api4/system.go +++ b/api4/system.go @@ -513,6 +513,11 @@ func pushNotificationAck(c *Context, w http.ResponseWriter, r *http.Request) { return } + if _, appErr := c.App.GetPostIfAuthorized(ack.PostId, c.AppContext.Session()); appErr != nil { + c.Err = appErr + return + } + if !*c.App.Config().EmailSettings.SendPushNotifications { c.Err = model.NewAppError("pushNotificationAck", "api.push_notification.disabled.app_error", nil, "", http.StatusNotImplemented) return diff --git a/api4/system_test.go b/api4/system_test.go index 6463a65131..a8c9b38808 100644 --- a/api4/system_test.go +++ b/api4/system_test.go @@ -4,6 +4,7 @@ package api4 import ( + "bytes" "fmt" "io/ioutil" "net/http" @@ -733,10 +734,11 @@ func TestServerBusy503(t *testing.T) { } func TestPushNotificationAck(t *testing.T) { - th := Setup(t) + th := Setup(t).InitBasic() api := Init(th.App, th.Server.Router) session, _ := th.App.GetSession(th.Client.AuthToken) defer th.TearDown() + t.Run("should return error when the ack body is not passed", func(t *testing.T) { handler := api.ApiHandler(pushNotificationAck) resp := httptest.NewRecorder() @@ -747,4 +749,20 @@ func TestPushNotificationAck(t *testing.T) { assert.Equal(t, http.StatusBadRequest, resp.Code) assert.NotNil(t, resp.Body) }) + + t.Run("should return error when the ack post is not authorized for the user", func(t *testing.T) { + privateChannel := th.CreateChannelWithClient(th.SystemAdminClient, model.ChannelTypePrivate) + privatePost := th.CreatePostWithClient(th.SystemAdminClient, privateChannel) + + handler := api.ApiHandler(pushNotificationAck) + resp := httptest.NewRecorder() + req := httptest.NewRequest("POST", "/api/v4/notifications/ack", nil) + req.Header.Set(model.HeaderAuth, "Bearer "+session.Token) + req.Body = ioutil.NopCloser(bytes.NewBufferString(fmt.Sprintf(`{"id":"123", "is_id_loaded":true, "post_id":"%s"}`, privatePost.Id))) + + handler.ServeHTTP(resp, req) + assert.Equal(t, http.StatusForbidden, resp.Code) + fmt.Printf("DEBUG/resp.Body: %+v\n", resp.Body) + assert.NotNil(t, resp.Body) + }) } diff --git a/app/app_iface.go b/app/app_iface.go index a2ff74d465..da7df6e5c4 100644 --- a/app/app_iface.go +++ b/app/app_iface.go @@ -686,6 +686,7 @@ type AppIface interface { GetPostAfterTime(channelID string, time int64, collapsedThreads bool) (*model.Post, *model.AppError) GetPostIdAfterTime(channelID string, time int64, collapsedThreads bool) (string, *model.AppError) GetPostIdBeforeTime(channelID string, time int64, collapsedThreads bool) (string, *model.AppError) + GetPostIfAuthorized(postID string, session *model.Session) (*model.Post, *model.AppError) GetPostThread(postID string, skipFetchThreads, collapsedThreads, collapsedThreadsExtended bool, userID string) (*model.PostList, *model.AppError) GetPosts(channelID string, offset int, limit int) (*model.PostList, *model.AppError) GetPostsAfterPost(options model.GetPostsOptions) (*model.PostList, *model.AppError) diff --git a/app/opentracing/opentracing_layer.go b/app/opentracing/opentracing_layer.go index 0ddc2ba788..6baa336a56 100644 --- a/app/opentracing/opentracing_layer.go +++ b/app/opentracing/opentracing_layer.go @@ -7447,6 +7447,28 @@ func (a *OpenTracingAppLayer) GetPostIdBeforeTime(channelID string, time int64, return resultVar0, resultVar1 } +func (a *OpenTracingAppLayer) GetPostIfAuthorized(postID string, session *model.Session) (*model.Post, *model.AppError) { + origCtx := a.ctx + span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetPostIfAuthorized") + + a.ctx = newCtx + a.app.Srv().Store.SetContext(newCtx) + defer func() { + a.app.Srv().Store.SetContext(origCtx) + a.ctx = origCtx + }() + + defer span.Finish() + resultVar0, resultVar1 := a.app.GetPostIfAuthorized(postID, session) + + if resultVar1 != nil { + span.LogFields(spanlog.Error(resultVar1)) + ext.Error.Set(span, true) + } + + return resultVar0, resultVar1 +} + func (a *OpenTracingAppLayer) GetPostThread(postID string, skipFetchThreads bool, collapsedThreads bool, collapsedThreadsExtended bool, userID string) (*model.PostList, *model.AppError) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetPostThread") diff --git a/app/post.go b/app/post.go index 57dfbd7b1d..b8a80eabb3 100644 --- a/app/post.go +++ b/app/post.go @@ -1568,3 +1568,27 @@ func isPostMention(user *model.User, post *model.Post, keywords map[string][]str func (a *App) GetThreadMembershipsForUser(userID, teamID string) ([]*model.ThreadMembership, error) { return a.Srv().Store.Thread().GetMembershipsForUser(userID, teamID) } + +func (a *App) GetPostIfAuthorized(postID string, session *model.Session) (*model.Post, *model.AppError) { + post, err := a.GetSinglePost(postID) + if err != nil { + return nil, err + } + + channel, err := a.GetChannel(post.ChannelId) + if err != nil { + return nil, err + } + + if !a.SessionHasPermissionToChannel(*session, channel.Id, model.PermissionReadChannel) { + if channel.Type == model.ChannelTypeOpen { + if !a.SessionHasPermissionToTeam(*session, channel.TeamId, model.PermissionReadPublicChannel) { + return nil, a.MakePermissionError(session, []*model.Permission{model.PermissionReadPublicChannel}) + } + } else { + return nil, a.MakePermissionError(session, []*model.Permission{model.PermissionReadChannel}) + } + } + + return post, nil +} diff --git a/app/post_test.go b/app/post_test.go index b2350e2e53..6f359d098d 100644 --- a/app/post_test.go +++ b/app/post_test.go @@ -2301,3 +2301,29 @@ func TestAutofollowOnPostingAfterUnfollow(t *testing.T) { require.Nil(t, err) require.True(t, m.Following) } + +func TestGetPostIfAuthorized(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + privateChannel := th.CreatePrivateChannel(th.BasicTeam) + post, err := th.App.CreatePost(th.Context, &model.Post{UserId: th.BasicUser.Id, ChannelId: privateChannel.Id, Message: "Hello"}, privateChannel, false, false) + require.Nil(t, err) + require.NotNil(t, post) + + session1, err := th.App.CreateSession(&model.Session{UserId: th.BasicUser.Id, Props: model.StringMap{}}) + require.Nil(t, err) + require.NotNil(t, session1) + + session2, err := th.App.CreateSession(&model.Session{UserId: th.BasicUser2.Id, Props: model.StringMap{}}) + require.Nil(t, err) + require.NotNil(t, session2) + + // User is not authorized to get post + _, err = th.App.GetPostIfAuthorized(post.Id, session2) + require.NotNil(t, err) + + // User is authorized to get post + _, err = th.App.GetPostIfAuthorized(post.Id, session1) + require.Nil(t, err) +}