Ignore ack and notification counts if notifications are blocked by the device (#27570)

* Ignore performance counts if notifications are blocked by the device

* Change the endpoint to allow more information

* Add tests and API description

* Remove wrong test

* Address feedback

* Only update the cache when there is no error

* Follow same casing as other props

* use one single endpoint

* Fix tests

* Fix i18n

---------

Co-authored-by: Mattermost Build <build@mattermost.com>

api/v4/source/users.yaml

@@ -1752,27 +1752,37 @@
     put:
       tags:
         - users
-      summary: Attach mobile device
+      summary: Attach mobile device and extra props to the session object
       description: >
-        Attach a mobile device id to the currently logged in session. This will
+        Attach extra props to the session object of the currently logged in session.
-        enable push notifications for a user, if configured by the server.
+
+        Adding a mobile device id will enable push notifications for a user,
+        if configured by the server.
+
+        Other props are also available, like whether the device has notifications
+        disabled and the mobile version.
 
         ##### Permissions
 
         Must be authenticated.
-      operationId: AttachDeviceId
+      operationId: AttachDeviceExtraProps
       requestBody:
         content:
           application/json:
             schema:
               type: object
-              required:
-                - device_id
               properties:
                 device_id:
                   description: Mobile device id. For Android prefix the id with `android:`
                     and Apple with `apple:`
                   type: string
+                deviceNotificationDisabled:
+                  description: Whether the mobile device has notifications disabled.
+                    Accepted values are "true" or "false".
+                  type: string
+                mobileVersion:
+                  description: Mobile app version. The version must be parseable as a semver.
+                  type: string
         required: true
       responses:
         "200":

server/channels/api4/system.go

@@ -674,7 +674,19 @@ func pushNotificationAck(c *Context, w http.ResponseWriter, r *http.Request) {
 	}
 
 	if ack.NotificationType == model.PushTypeMessage {
-		c.App.CountNotificationAck(model.NotificationTypePush, ack.ClientPlatform)
+		session := c.AppContext.Session()
+		ignoreNotificationACK := session.Props[model.SessionPropDeviceNotificationDisabled] == "true"
+		if ignoreNotificationACK && ack.ClientPlatform == "ios" {
+			// iOS doesn't send ack when the notificications are disabled
+			// so we restore the value the moment we receive an ack
+			c.App.SetExtraSessionProps(session, map[string]string{
+				model.SessionPropDeviceNotificationDisabled: "false",
+			})
+			c.App.ClearSessionCacheForUser(session.UserId)
+		}
+		if !ignoreNotificationACK {
+			c.App.CountNotificationAck(model.NotificationTypePush, ack.ClientPlatform)
+		}
 	}
 
 	err := c.App.SendAckToPushProxy(&ack)

server/channels/api4/system_test.go

@@ -885,6 +885,64 @@ func TestPushNotificationAck(t *testing.T) {
 		assert.Equal(t, http.StatusForbidden, resp.Code)
 		assert.NotNil(t, resp.Body)
 	})
+
+	ttcc := []struct {
+		name          string
+		propValue     string
+		platform      string
+		expectedValue string
+	}{
+		{
+			name:          "should set session prop device notification disabled to false if an ack is sent from iOS",
+			propValue:     "true",
+			platform:      "ios",
+			expectedValue: "false",
+		},
+		{
+			name:          "no change if empty",
+			propValue:     "",
+			platform:      "ios",
+			expectedValue: "",
+		},
+		{
+			name:          "no change if false",
+			propValue:     "false",
+			platform:      "ios",
+			expectedValue: "false",
+		},
+		{
+			name:          "no change on Android",
+			propValue:     "true",
+			platform:      "android",
+			expectedValue: "true",
+		},
+	}
+	for _, tc := range ttcc {
+		t.Run(tc.name, func(t *testing.T) {
+			defer func() {
+				session.AddProp(model.SessionPropDeviceNotificationDisabled, "")
+				th.Server.Store().Session().UpdateProps(session)
+				th.App.ClearSessionCacheForUser(session.UserId)
+			}()
+
+			session.AddProp(model.SessionPropDeviceNotificationDisabled, tc.propValue)
+			err := th.Server.Store().Session().UpdateProps(session)
+			th.App.ClearSessionCacheForUser(session.UserId)
+			assert.NoError(t, err)
+
+			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 = io.NopCloser(bytes.NewBufferString(fmt.Sprintf(`{"id":"123", "is_id_loaded":true, "platform": "%s", "post_id":"%s", "type": "%s"}`, tc.platform, th.BasicPost.Id, model.PushTypeMessage)))
+
+			handler.ServeHTTP(resp, req)
+			updatedSession, _ := th.App.GetSession(th.Client.AuthToken)
+			assert.Equal(t, tc.expectedValue, updatedSession.Props[model.SessionPropDeviceNotificationDisabled])
+			storeSession, _ := th.Server.Store().Session().Get(th.Context, session.Id)
+			assert.Equal(t, tc.expectedValue, storeSession.Props[model.SessionPropDeviceNotificationDisabled])
+		})
+	}
 }
 
 func TestCompleteOnboarding(t *testing.T) {

server/channels/api4/user.go

@@ -13,6 +13,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/blang/semver/v4"
 	"github.com/mattermost/mattermost/server/public/model"
 	"github.com/mattermost/mattermost/server/public/shared/mlog"
 
@@ -74,7 +75,7 @@ func (api *API) InitUser() {
 	api.BaseRoutes.User.Handle("/sessions/revoke", api.APISessionRequired(revokeSession)).Methods(http.MethodPost)
 	api.BaseRoutes.User.Handle("/sessions/revoke/all", api.APISessionRequired(revokeAllSessionsForUser)).Methods(http.MethodPost)
 	api.BaseRoutes.Users.Handle("/sessions/revoke/all", api.APISessionRequired(revokeAllSessionsAllUsers)).Methods(http.MethodPost)
-	api.BaseRoutes.Users.Handle("/sessions/device", api.APISessionRequired(attachDeviceId)).Methods(http.MethodPut)
+	api.BaseRoutes.Users.Handle("/sessions/device", api.APISessionRequired(handleDeviceProps)).Methods(http.MethodPut)
 	api.BaseRoutes.User.Handle("/audits", api.APISessionRequired(getUserAudits)).Methods(http.MethodGet)
 
 	api.BaseRoutes.User.Handle("/tokens", api.APISessionRequired(createUserAccessToken)).Methods(http.MethodPost)
@@ -2210,15 +2211,49 @@ func revokeAllSessionsAllUsers(c *Context, w http.ResponseWriter, r *http.Reques
 	ReturnStatusOK(w)
 }
 
-func attachDeviceId(c *Context, w http.ResponseWriter, r *http.Request) {
+func handleDeviceProps(c *Context, w http.ResponseWriter, r *http.Request) {
-	props := model.MapFromJSON(r.Body)
+	receivedProps := model.MapFromJSON(r.Body)
+	deviceId := receivedProps["device_id"]
 
-	deviceId := props["device_id"]
+	newProps := map[string]string{}
-	if deviceId == "" {
+
-		c.SetInvalidParam("device_id")
+	deviceNotificationsDisabled := receivedProps[model.SessionPropDeviceNotificationDisabled]
+	if deviceNotificationsDisabled != "" {
+		if deviceNotificationsDisabled != "false" && deviceNotificationsDisabled != "true" {
+			c.SetInvalidParam(model.SessionPropDeviceNotificationDisabled)
+			return
+		}
+
+		newProps[model.SessionPropDeviceNotificationDisabled] = deviceNotificationsDisabled
+	}
+
+	mobileVersion := receivedProps[model.SessionPropMobileVersion]
+	if mobileVersion != "" {
+		if _, err := semver.Parse(mobileVersion); err != nil {
+			c.SetInvalidParam(model.SessionPropMobileVersion)
+			return
+		}
+		newProps[model.SessionPropMobileVersion] = mobileVersion
+	}
+
+	if deviceId != "" {
+		attachDeviceId(c, w, r, deviceId)
+	}
+
+	if c.Err != nil {
 		return
 	}
 
+	if err := c.App.SetExtraSessionProps(c.AppContext.Session(), newProps); err != nil {
+		c.Err = err
+		return
+	}
+
+	c.App.ClearSessionCacheForUser(c.AppContext.Session().UserId)
+	ReturnStatusOK(w)
+}
+
+func attachDeviceId(c *Context, w http.ResponseWriter, r *http.Request, deviceId string) {
 	auditRec := c.MakeAuditRecord("attachDeviceId", audit.Fail)
 	defer c.LogAuditRec(auditRec)
 	audit.AddEventParameter(auditRec, "device_id", deviceId)
@@ -2266,8 +2301,6 @@ func attachDeviceId(c *Context, w http.ResponseWriter, r *http.Request) {
 
 	auditRec.Success()
 	c.LogAudit("")
-
-	ReturnStatusOK(w)
 }
 
 func getUserAudits(c *Context, w http.ResponseWriter, r *http.Request) {

server/channels/api4/user_test.go

@@ -3691,7 +3691,7 @@ func TestAttachDeviceId(t *testing.T) {
 					*cfg.ServiceSettings.SiteURL = tc.SiteURL
 				})
 
-				resp, err := th.Client.AttachDeviceId(context.Background(), deviceId)
+				resp, err := th.Client.AttachDeviceProps(context.Background(), map[string]string{"device_id": deviceId})
 				require.NoError(t, err)
 
 				cookies := resp.Header.Get("Set-Cookie")
@@ -3704,19 +3704,88 @@ func TestAttachDeviceId(t *testing.T) {
 		}
 	})
 
-	t.Run("invalid device id", func(t *testing.T) {
-		resp, err := th.Client.AttachDeviceId(context.Background(), "")
-		require.Error(t, err)
-		CheckBadRequestStatus(t, resp)
-	})
-
 	t.Run("not logged in", func(t *testing.T) {
 		th.Client.Logout(context.Background())
 
-		resp, err := th.Client.AttachDeviceId(context.Background(), "")
+		resp, err := th.Client.AttachDeviceProps(context.Background(), map[string]string{})
 		require.Error(t, err)
 		CheckUnauthorizedStatus(t, resp)
 	})
+
+	// Props related tests
+
+	client := th.CreateClient()
+	th.LoginBasicWithClient(client)
+
+	resetSession := func(session *model.Session) {
+		session.AddProp(model.SessionPropDeviceNotificationDisabled, "")
+		session.AddProp(model.SessionPropMobileVersion, "")
+		th.Server.Store().Session().UpdateProps(session)
+		th.App.ClearSessionCacheForUser(session.UserId)
+	}
+
+	t.Run("No props will return ok and no changes in the session", func(t *testing.T) {
+		session, _ := th.App.GetSession(client.AuthToken)
+		defer resetSession(session)
+		res, err := client.AttachDeviceProps(context.Background(), map[string]string{})
+		assert.NoError(t, err)
+
+		updatedSession, _ := th.App.GetSession(client.AuthToken)
+		storeSession, _ := th.Server.Store().Session().Get(th.Context, session.Id)
+		assert.Equal(t, http.StatusOK, res.StatusCode)
+		assert.Equal(t, session.Props, updatedSession.Props)
+		assert.Equal(t, session.Props, storeSession.Props)
+	})
+	t.Run("Unknown props will be ignored, returning ok and no changes in the session", func(t *testing.T) {
+		session, _ := th.App.GetSession(client.AuthToken)
+		defer resetSession(session)
+		res, err := client.AttachDeviceProps(context.Background(), map[string]string{"unknownProp": "foo"})
+		assert.NoError(t, err)
+
+		updatedSession, _ := th.App.GetSession(client.AuthToken)
+		storeSession, _ := th.Server.Store().Session().Get(th.Context, session.Id)
+		assert.Equal(t, http.StatusOK, res.StatusCode)
+		assert.Equal(t, session.Props, updatedSession.Props)
+		assert.Equal(t, session.Props, storeSession.Props)
+	})
+	t.Run("Invalid disabled notification prop will return an error and no changes in the session", func(t *testing.T) {
+		session, _ := th.App.GetSession(client.AuthToken)
+		defer resetSession(session)
+		res, err := client.AttachDeviceProps(context.Background(), map[string]string{model.SessionPropDeviceNotificationDisabled: "foo"})
+		assert.Error(t, err)
+
+		updatedSession, _ := th.App.GetSession(client.AuthToken)
+		storeSession, _ := th.Server.Store().Session().Get(th.Context, session.Id)
+		assert.Equal(t, http.StatusBadRequest, res.StatusCode)
+		assert.Equal(t, session.Props, updatedSession.Props)
+		assert.Equal(t, session.Props, storeSession.Props)
+	})
+	t.Run("Invalid version will return an error and no changes in the session", func(t *testing.T) {
+		session, _ := th.App.GetSession(client.AuthToken)
+		defer resetSession(session)
+		res, err := client.AttachDeviceProps(context.Background(), map[string]string{model.SessionPropMobileVersion: "foo"})
+		assert.Error(t, err)
+
+		updatedSession, _ := th.App.GetSession(client.AuthToken)
+		storeSession, _ := th.Server.Store().Session().Get(th.Context, session.Id)
+		assert.Equal(t, http.StatusBadRequest, res.StatusCode)
+		assert.Equal(t, session.Props, updatedSession.Props)
+		assert.Equal(t, session.Props, storeSession.Props)
+	})
+	t.Run("Will update props", func(t *testing.T) {
+		session, _ := th.App.GetSession(client.AuthToken)
+		defer resetSession(session)
+		res, err := client.AttachDeviceProps(context.Background(), map[string]string{model.SessionPropDeviceNotificationDisabled: "true", model.SessionPropMobileVersion: "2.19.0"})
+		assert.NoError(t, err)
+
+		updatedSession, _ := th.App.GetSession(client.AuthToken)
+		storeSession, _ := th.Server.Store().Session().Get(th.Context, session.Id)
+		assert.Equal(t, http.StatusOK, res.StatusCode)
+		assert.Equal(t, "true", updatedSession.Props[model.SessionPropDeviceNotificationDisabled])
+		assert.Equal(t, "true", storeSession.Props[model.SessionPropDeviceNotificationDisabled])
+		assert.Equal(t, "2.19.0", updatedSession.Props[model.SessionPropMobileVersion])
+		assert.Equal(t, "2.19.0", storeSession.Props[model.SessionPropMobileVersion])
+	})
 }
 
 func TestGetUserAudits(t *testing.T) {

server/channels/app/app_iface.go

@@ -1117,6 +1117,7 @@ type AppIface interface {
 	SetChannels(ch *Channels)
 	SetCustomStatus(c request.CTX, userID string, cs *model.CustomStatus) *model.AppError
 	SetDefaultProfileImage(c request.CTX, user *model.User) *model.AppError
+	SetExtraSessionProps(session *model.Session, newProps map[string]string) *model.AppError
 	SetFileSearchableContent(rctx request.CTX, fileID string, data string) *model.AppError
 	SetPhase2PermissionsMigrationStatus(isComplete bool) error
 	SetPluginKey(pluginID string, key string, value []byte) *model.AppError

server/channels/app/notification_push.go

@@ -218,7 +218,10 @@ func (a *App) sendPushNotificationToAllSessions(rctx request.CTX, msg *model.Pus
 		}
 
 		if msg.Type == model.PushTypeMessage {
-			a.CountNotification(model.NotificationTypePush, tmpMessage.Platform)
+			// If we are ignoring the ack, we don't count the send
+			if session.Props[model.SessionPropDeviceNotificationDisabled] != "true" {
+				a.CountNotification(model.NotificationTypePush, tmpMessage.Platform)
+			}
 		}
 	}
 

server/channels/app/opentracing/opentracing_layer.go

@@ -16673,6 +16673,28 @@ func (a *OpenTracingAppLayer) SetDefaultProfileImage(c request.CTX, user *model.
 	return resultVar0
 }
 
+func (a *OpenTracingAppLayer) SetExtraSessionProps(session *model.Session, newProps map[string]string) *model.AppError {
+	origCtx := a.ctx
+	span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.SetExtraSessionProps")
+
+	a.ctx = newCtx
+	a.app.Srv().Store().SetContext(newCtx)
+	defer func() {
+		a.app.Srv().Store().SetContext(origCtx)
+		a.ctx = origCtx
+	}()
+
+	defer span.Finish()
+	resultVar0 := a.app.SetExtraSessionProps(session, newProps)
+
+	if resultVar0 != nil {
+		span.LogFields(spanlog.Error(resultVar0))
+		ext.Error.Set(span, true)
+	}
+
+	return resultVar0
+}
+
 func (a *OpenTracingAppLayer) SetFileSearchableContent(rctx request.CTX, fileID string, data string) *model.AppError {
 	origCtx := a.ctx
 	span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.SetFileSearchableContent")

server/channels/app/session.go

@@ -291,6 +291,29 @@ func (a *App) AttachDeviceId(sessionID string, deviceID string, expiresAt int64)
 	return nil
 }
 
+func (a *App) SetExtraSessionProps(session *model.Session, newProps map[string]string) *model.AppError {
+	changed := false
+	for k, v := range newProps {
+		if session.Props[k] == v {
+			continue
+		}
+
+		session.AddProp(k, v)
+		changed = true
+	}
+
+	if !changed {
+		return nil
+	}
+
+	err := a.Srv().Store().Session().UpdateProps(session)
+	if err != nil {
+		return model.NewAppError("SetExtraSessionProps", "app.session.set_extra_session_prop.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
+	}
+
+	return nil
+}
+
 // ExtendSessionExpiryIfNeeded extends Session.ExpiresAt based on session lengths in config.
 // A new ExpiresAt is only written if enough time has elapsed since last update.
 // Returns true only if the session was extended.

server/channels/app/session_test.go

@@ -445,3 +445,48 @@ func TestSessionsLimit(t *testing.T) {
 		require.Equal(t, sessions[i].Id, sess.Id)
 	}
 }
+
+func TestSetExtraSessionProps(t *testing.T) {
+	th := Setup(t).InitBasic()
+	defer th.TearDown()
+
+	r := &http.Request{}
+	w := httptest.NewRecorder()
+	session, _ := th.App.DoLogin(th.Context, w, r, th.BasicUser, "", false, false, false)
+
+	resetSession := func(session *model.Session) {
+		session.AddProp("testProp", "")
+		th.Server.Store().Session().UpdateProps(session)
+		th.App.ClearSessionCacheForUser(session.UserId)
+	}
+	t.Run("do not update the session if there are no props", func(t *testing.T) {
+		defer resetSession(session)
+		th.App.SetExtraSessionProps(session, map[string]string{})
+		updatedSession, _ := th.App.GetSession(session.Token)
+		storeSession, _ := th.Server.Store().Session().Get(th.Context, session.Id)
+		assert.Equal(t, session, updatedSession)
+		assert.Equal(t, session, storeSession)
+	})
+	t.Run("update the session with the selected prop", func(t *testing.T) {
+		defer resetSession(session)
+		th.App.SetExtraSessionProps(session, map[string]string{"testProp": "true"})
+		updatedSession, _ := th.App.GetSession(session.Token)
+		storeSession, _ := th.Server.Store().Session().Get(th.Context, session.Id)
+		assert.Equal(t, "true", updatedSession.Props["testProp"])
+		assert.Equal(t, "true", storeSession.Props["testProp"])
+	})
+	t.Run("do not update the session if the prop is the same", func(t *testing.T) {
+		defer resetSession(session)
+		session.AddProp("testProp", "true")
+		th.Server.Store().Session().UpdateProps(session)
+		th.App.ClearSessionCacheForUser(session.UserId)
+
+		th.App.SetExtraSessionProps(session, map[string]string{"testProp": "true"})
+		updatedSession, _ := th.App.GetSession(session.Token)
+		storeSession, _ := th.Server.Store().Session().Get(th.Context, session.Id)
+		assert.Equal(t, session, updatedSession)
+		assert.Equal(t, session, storeSession)
+		assert.Equal(t, "true", updatedSession.Props["testProp"])
+		assert.Equal(t, "true", storeSession.Props["testProp"])
+	})
+}

server/i18n/en.json

@@ -6490,6 +6490,10 @@
     "id": "app.session.save.existing.app_error",
     "translation": "Unable to update existing session."
   },
+  {
+    "id": "app.session.set_extra_session_prop.app_error",
+    "translation": "Unable to update the extra session properties."
+  },
   {
     "id": "app.session.update_device_id.app_error",
     "translation": "Unable to update the device id."

server/public/model/client4.go

@@ -1727,10 +1727,9 @@ func (c *Client4) RevokeSessionsFromAllUsers(ctx context.Context) (*Response, er
 	return BuildResponse(r), nil
 }
 
-// AttachDeviceId attaches a mobile device ID to the current session.
+// AttachDeviceProps attaches a mobile device ID to the current session and other props.
-func (c *Client4) AttachDeviceId(ctx context.Context, deviceId string) (*Response, error) {
+func (c *Client4) AttachDeviceProps(ctx context.Context, newProps map[string]string) (*Response, error) {
-	requestBody := map[string]string{"device_id": deviceId}
+	r, err := c.DoAPIPut(ctx, c.usersRoute()+"/sessions/device", MapToJSON(newProps))
-	r, err := c.DoAPIPut(ctx, c.usersRoute()+"/sessions/device", MapToJSON(requestBody))
 	if err != nil {
 		return BuildResponse(r), err
 	}

server/public/model/session.go

@@ -12,26 +12,28 @@ import (
 )
 
 const (
-	SessionCookieToken                = "MMAUTHTOKEN"
+	SessionCookieToken                    = "MMAUTHTOKEN"
-	SessionCookieUser                 = "MMUSERID"
+	SessionCookieUser                     = "MMUSERID"
-	SessionCookieCsrf                 = "MMCSRF"
+	SessionCookieCsrf                     = "MMCSRF"
-	SessionCookieCloudUrl             = "MMCLOUDURL"
+	SessionCookieCloudUrl                 = "MMCLOUDURL"
-	SessionCacheSize                  = 35000
+	SessionCacheSize                      = 35000
-	SessionPropPlatform               = "platform"
+	SessionPropPlatform                   = "platform"
-	SessionPropOs                     = "os"
+	SessionPropOs                         = "os"
-	SessionPropBrowser                = "browser"
+	SessionPropBrowser                    = "browser"
-	SessionPropType                   = "type"
+	SessionPropType                       = "type"
-	SessionPropUserAccessTokenId      = "user_access_token_id"
+	SessionPropUserAccessTokenId          = "user_access_token_id"
-	SessionPropIsBot                  = "is_bot"
+	SessionPropIsBot                      = "is_bot"
-	SessionPropIsBotValue             = "true"
+	SessionPropIsBotValue                 = "true"
-	SessionPropOAuthAppID             = "oauth_app_id"
+	SessionPropOAuthAppID                 = "oauth_app_id"
-	SessionPropMattermostAppID        = "mattermost_app_id"
+	SessionPropMattermostAppID            = "mattermost_app_id"
-	SessionTypeUserAccessToken        = "UserAccessToken"
+	SessionPropDeviceNotificationDisabled = "device_notification_disabled"
-	SessionTypeCloudKey               = "CloudKey"
+	SessionPropMobileVersion              = "mobile_version"
-	SessionTypeRemoteclusterToken     = "RemoteClusterToken"
+	SessionTypeUserAccessToken            = "UserAccessToken"
-	SessionPropIsGuest                = "is_guest"
+	SessionTypeCloudKey                   = "CloudKey"
-	SessionActivityTimeout            = 1000 * 60 * 5  // 5 minutes
+	SessionTypeRemoteclusterToken         = "RemoteClusterToken"
-	SessionUserAccessTokenExpiryHours = 100 * 365 * 24 // 100 years
+	SessionPropIsGuest                    = "is_guest"
+	SessionActivityTimeout                = 1000 * 60 * 5  // 5 minutes
+	SessionUserAccessTokenExpiryHours     = 100 * 365 * 24 // 100 years
 )
 
 //msgp:tuple StringMap