MM-37417: Push notification authz fix. (#18009)

* MM-37417: Push notification authz fix.

* MM-37417: Tests new app method.

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 = c.App.GetPostIfAuthorized(post.Id, c.AppContext.Session()); err != nil {
-	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
-		}
-	}
-
 	if c.HandleEtag(list.Etag(), "Get Post Thread", w, r) {
 		return
 	}

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

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

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)

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

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

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