MM-56881 Validate and ensure valid CustomStatus is stored (#26287)

* don't allow invalid CustomStatus

* allow empty emoji in custom status

* lint fix

* add english translation

* update for review comments.

* fix bad fix

---------

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

server/i18n/en.json

@@ -9770,6 +9770,10 @@
     "id": "model.user.is_valid.id.app_error",
     "translation": "Invalid user id."
   },
+  {
+    "id": "model.user.is_valid.invalidProperty.app_error",
+    "translation": "Invalid props (custom status)"
+  },
   {
     "id": "model.user.is_valid.last_name.app_error",
     "translation": "Invalid last name."

server/public/model/custom_status.go

@@ -35,10 +35,6 @@ type CustomStatus struct {
 }
 
 func (cs *CustomStatus) PreSave() {
-	if cs.Emoji == "" {
-		cs.Emoji = DefaultCustomStatusEmoji
-	}
-
 	if cs.Duration == "" && !cs.ExpiresAt.Before(time.Now()) {
 		cs.Duration = "date_and_time"
 	}

server/public/model/user.go

@@ -399,6 +399,13 @@ func (u *User) IsValid() *AppError {
 			map[string]any{"Limit": UserRolesMaxLength}, "user_id="+u.Id+" roles_limit="+u.Roles, http.StatusBadRequest)
 	}
 
+	if u.Props != nil {
+		if !u.ValidateCustomStatus() {
+			return NewAppError("User.IsValid", "model.user.is_valid.invalidProperty.app_error",
+				map[string]any{"Props": u.Props}, "user_id="+u.Id, http.StatusBadRequest)
+		}
+	}
+
 	return nil
 }
 
@@ -472,6 +479,12 @@ func (u *User) PreSave() {
 	if u.Password != "" {
 		u.Password = HashPassword(u.Password)
 	}
+
+	cs := u.GetCustomStatus()
+	if cs != nil {
+		cs.PreSave()
+		u.SetCustomStatus(cs)
+	}
 }
 
 // PreUpdate should be run before updating the user in the db.
@@ -508,6 +521,14 @@ func (u *User) PreUpdate() {
 		}
 		u.NotifyProps[MentionKeysNotifyProp] = strings.Join(goodKeys, ",")
 	}
+
+	if u.Props != nil {
+		cs := u.GetCustomStatus()
+		if cs != nil {
+			cs.PreSave()
+			u.SetCustomStatus(cs)
+		}
+	}
 }
 
 func (u *User) SetDefaultNotifications() {
@@ -711,6 +732,17 @@ func (u *User) ClearCustomStatus() {
 	u.Props[UserPropsKeyCustomStatus] = ""
 }
 
+func (u *User) ValidateCustomStatus() bool {
+	status, exists := u.Props[UserPropsKeyCustomStatus]
+	if exists && status != "" {
+		cs := u.GetCustomStatus()
+		if cs == nil {
+			return false
+		}
+	}
+	return true
+}
+
 func (u *User) GetFullName() string {
 	if u.FirstName != "" && u.LastName != "" {
 		return u.FirstName + " " + u.LastName

server/public/model/user_test.go

@@ -355,3 +355,24 @@ func TestUserSlice(t *testing.T) {
 		assert.Len(t, nonBotUsers, 1)
 	})
 }
+
+func TestValidateCustomStatus(t *testing.T) {
+	t.Run("ValidateCustomStatus", func(t *testing.T) {
+		user0 := &User{Id: "user0", DeleteAt: 0, IsBot: true}
+
+		user0.Props = map[string]string{UserPropsKeyCustomStatus: ""}
+		assert.True(t, user0.ValidateCustomStatus())
+
+		user0.Props[UserPropsKeyCustomStatus] = "hello"
+		assert.False(t, user0.ValidateCustomStatus())
+
+		user0.Props[UserPropsKeyCustomStatus] = "{\"emoji\":{\"foo\":\"bar\"}}"
+		assert.True(t, user0.ValidateCustomStatus())
+
+		user0.Props[UserPropsKeyCustomStatus] = "{\"text\": \"hello\"}"
+		assert.True(t, user0.ValidateCustomStatus())
+
+		user0.Props[UserPropsKeyCustomStatus] = "{\"wrong\": \"hello\"}"
+		assert.True(t, user0.ValidateCustomStatus())
+	})
+}

webapp/channels/src/components/status_dropdown/status_dropdown.tsx

@@ -342,7 +342,7 @@ export class StatusDropdown extends React.PureComponent<Props, State> {
                     time={customStatus.expires_at}
                     timezone={this.props.timezone}
                     className={classNames('custom_status__expiry', {
-                        padded: customStatus?.text.length > 0,
+                        padded: customStatus?.text?.length > 0,
                     })}
                     withinBrackets={true}
                 />
@@ -355,7 +355,7 @@ export class StatusDropdown extends React.PureComponent<Props, State> {
                     modalId={ModalIdentifiers.CUSTOM_STATUS}
                     dialogType={CustomStatusModal}
                     className={classNames('MenuItem__primary-text custom_status__row', {
-                        flex: customStatus?.text.length === 0,
+                        flex: customStatus?.text?.length === 0,
                     })}
                     id={'status-menu-custom-status'}
                 >
@@ -385,7 +385,7 @@ export class StatusDropdown extends React.PureComponent<Props, State> {
         const {intl} = this.props;
         const needsConfirm = this.isUserOutOfOffice() && this.props.autoResetPref === '';
         const {status, customStatus, isCustomStatusExpired, currentUser} = this.props;
-        const isStatusSet = customStatus && (customStatus.text.length > 0 || customStatus.emoji.length > 0) && !isCustomStatusExpired;
+        const isStatusSet = customStatus && !isCustomStatusExpired && (customStatus.text?.length > 0 || customStatus.emoji?.length > 0);
 
         const setOnline = needsConfirm ? () => this.showStatusChangeConfirmation('online') : this.setOnline;
         const setDnd = needsConfirm ? () => this.showStatusChangeConfirmation('dnd') : this.setDnd;

webapp/channels/src/selectors/views/custom_status.test.ts

@@ -40,6 +40,15 @@ describe('getCustomStatus', () => {
         expect(getCustomStatus(store.getState(), user.id)).toBeUndefined();
     });
 
+    it('should return undefined when user with invalid json for custom status set', async () => {
+        const store = await configureStore();
+        const newUser = {...user};
+        newUser.props.customStatus = 'not a JSON string';
+
+        (UserSelectors.getUser as jest.Mock).mockReturnValue(user);
+        expect(getCustomStatus(store.getState(), user.id)).toBeUndefined();
+    });
+
     it('should return customStatus object when there is custom status set', async () => {
         const store = await configureStore();
         const newUser = {...user};

webapp/channels/src/selectors/views/custom_status.ts

@@ -26,7 +26,15 @@ export function makeGetCustomStatus(): (state: GlobalState, userID?: string) =>
         (state: GlobalState, userID?: string) => (userID ? getUser(state, userID) : getCurrentUser(state)),
         (user) => {
             const userProps = user?.props || {};
-            return userProps.customStatus ? JSON.parse(userProps.customStatus) : undefined;
+            let customStatus;
+            if (userProps.customStatus) {
+                try {
+                    customStatus = JSON.parse(userProps.customStatus);
+                } catch (error) {
+                    // do nothing if invalid, return undefined custom status.
+                }
+            }
+            return customStatus;
         },
     );
 }