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

…26494)

Automatic Merge

server/i18n/en.json

@@ -9259,6 +9259,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

@@ -37,10 +37,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

@@ -392,6 +392,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
 }
 
@@ -465,6 +472,12 @@ func (u *User) PreSave() {
 	if u.Password != "" {
 		u.Password = HashPassword(u.Password)
 	}
+
+	cs := u.GetCustomStatus()
+	if cs != nil {
+		cs.PreSave()
+		u.SetCustomStatus(cs)
+	}
 }
 
 // The following are some GraphQL methods necessary to return the
@@ -542,6 +555,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() {
@@ -744,6 +765,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

@@ -353,3 +353,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

@@ -300,7 +300,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}
                 />
@@ -313,7 +313,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'}
                 >
@@ -344,7 +344,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

@@ -39,6 +39,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

@@ -24,7 +24,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;
         },
     );
 }