Merge pull request #7626 from Tainan404/issue-7517

Fix DOM injection in number of rooms select

bigbluebutton-html5/imports/api/breakouts/server/methods/createBreakout.js

@@ -1,7 +1,7 @@
 import { Meteor } from 'meteor/meteor';
 import { check } from 'meteor/check';
 import RedisPubSub from '/imports/startup/server/redis';
-import _ from 'lodash';
+import Logger from '/imports/startup/server/logger';
 
 export default function createBreakoutRoom(credentials, rooms, durationInMinutes, record = false) {
   const REDIS_CONFIG = Meteor.settings.private.redis;
@@ -18,7 +18,7 @@ export default function createBreakoutRoom(credentials, rooms, durationInMinutes
   check(requesterToken, String);
 
   const eventName = 'CreateBreakoutRoomsCmdMsg';
-
+  if (rooms.length > 8) return Logger.info(`Attempt to create breakout rooms with invalid number of rooms in meeting id=${meetingId}`);
   const payload = {
     record,
     durationInMinutes,

bigbluebutton-html5/imports/ui/components/actions-bar/create-breakout-room/component.jsx

@@ -101,6 +101,10 @@ const intlMessages = defineMessages({
     id: 'app.createBreakoutRoom.roomTime',
     description: 'used to provide current room time for aria label',
   },
+  numberOfRoomsIsValid: {
+    id: 'app.createBreakoutRoom.numberOfRoomsError',
+    description: 'Label an error message',
+  },
 });
 
 const MIN_BREAKOUT_ROOMS = 2;
@@ -148,6 +152,7 @@ class BreakoutRoom extends Component {
     this.setRecord = this.setRecord.bind(this);
     this.blurDurationTime = this.blurDurationTime.bind(this);
     this.removeRoomUsers = this.removeRoomUsers.bind(this);
+    this.renderErrorMessages = this.renderErrorMessages.bind(this);
 
     this.state = {
       numberOfRooms: MIN_BREAKOUT_ROOMS,
@@ -160,6 +165,7 @@ class BreakoutRoom extends Component {
       preventClosing: true,
       valid: true,
       record: false,
+      numberOfRoomsIsValid: true,
     };
 
     this.breakoutFormId = _.uniqueId('breakout-form-');
@@ -203,12 +209,18 @@ class BreakoutRoom extends Component {
       users,
       freeJoin,
       record,
+      numberOfRoomsIsValid,
     } = this.state;
 
     if (users.length === this.getUserByRoom(0).length && !freeJoin) {
       this.setState({ valid: false });
       return;
     }
+
+    if (!numberOfRoomsIsValid) {
+      return;
+    }
+
     this.setState({ preventClosing: false });
     const { numberOfRooms, durationTime } = this.state;
     const rooms = _.range(1, numberOfRooms + 1).map(value => ({
@@ -340,7 +352,10 @@ class BreakoutRoom extends Component {
 
     const idxUser = users.findIndex(user => user.userId === userId);
     users[idxUser].room = room;
-    this.setState({ users });
+    this.setState({
+      users,
+      valid: this.getUserByRoom(0).length !== users.length,
+    });
   }
 
   increaseDurationTime() {
@@ -364,7 +379,12 @@ class BreakoutRoom extends Component {
   }
 
   changeNumberOfRooms(event) {
-    this.setState({ numberOfRooms: Number.parseInt(event.target.value, 10) });
+    const numberOfRooms = Number.parseInt(event.target.value, 10);
+    this.setState({
+      numberOfRooms,
+      numberOfRoomsIsValid: numberOfRooms <= MAX_BREAKOUT_ROOMS
+      && numberOfRooms >= MIN_BREAKOUT_ROOMS,
+    });
   }
 
   renderRoomsGrid() {
@@ -373,6 +393,9 @@ class BreakoutRoom extends Component {
       valid,
       numberOfRooms,
     } = this.state;
+    const rooms = (numberOfRooms > MAX_BREAKOUT_ROOMS
+      || numberOfRooms < MIN_BREAKOUT_ROOMS)
+      ? 0 : numberOfRooms;
     const allowDrop = (ev) => {
       ev.preventDefault();
     };
@@ -398,7 +421,7 @@ class BreakoutRoom extends Component {
           </span>
         </div>
         {
-          _.range(1, numberOfRooms + 1).map(value => (
+          _.range(1, rooms + 1).map(value => (
             <div key={`room-${value}`}>
               <p className={styles.freeJoinLabel}>
                 {intl.formatMessage(intlMessages.roomLabel, { 0: (value) })}
@@ -421,88 +444,102 @@ class BreakoutRoom extends Component {
     const {
       numberOfRooms,
       durationTime,
+      numberOfRoomsIsValid,
     } = this.state;
     if (isInvitation) return null;
 
     return (
-      <div className={styles.breakoutSettings} key={this.breakoutFormId}>
-        <div>
-          <p className={styles.labelText} aria-hidden>
-            {intl.formatMessage(intlMessages.numberOfRooms)}
-          </p>
-          <select
-            id="numberOfRooms"
-            name="numberOfRooms"
-            className={styles.inputRooms}
-            value={numberOfRooms}
-            onChange={this.changeNumberOfRooms}
-            aria-label={intl.formatMessage(intlMessages.numberOfRooms)}
-          >
-            {
-              _.range(MIN_BREAKOUT_ROOMS, MAX_BREAKOUT_ROOMS + 1).map(item => (<option key={_.uniqueId('value-')}>{item}</option>))
-            }
-          </select>
-        </div>
-        <label htmlFor="breakoutRoomTime">
-          <p className={styles.labelText} aria-hidden>
-            {intl.formatMessage(intlMessages.duration)}
-          </p>
-          <div className={styles.durationArea}>
-            <input
-              type="number"
-              className={styles.duration}
-              min="1"
-              value={durationTime}
-              onChange={this.changeDurationTime}
-              onBlur={this.blurDurationTime}
-              aria-label={intl.formatMessage(intlMessages.duration)}
-            />
-            <HoldButton
-              key="decrease-breakout-time"
-              exec={this.decreaseDurationTime}
-              minBound={MIN_BREAKOUT_ROOMS}
-              value={durationTime}
-              className={styles.btnStyle}
+      <React.Fragment>
+        <div className={styles.breakoutSettings} key={this.breakoutFormId}>
+          <div>
+            <p
+              className={cx(styles.labelText, !numberOfRoomsIsValid
+                && styles.withError)}
+              aria-hidden
             >
-              <Button
-                label={intl.formatMessage(intlMessages.minusRoomTime)}
-                aria-label={
-                  `${intl.formatMessage(intlMessages.minusRoomTime)} ${intl.formatMessage(intlMessages.roomTime, { 0: durationTime - 1 })}`
-                }
-                icon="substract"
-                onClick={() => {}}
-                hideLabel
-                circle
-                size="sm"
-              />
-            </HoldButton>
-            <HoldButton
-              key="increase-breakout-time"
-              exec={this.increaseDurationTime}
-              className={styles.btnStyle}
+              {intl.formatMessage(intlMessages.numberOfRooms)}
+            </p>
+            <select
+              id="numberOfRooms"
+              name="numberOfRooms"
+              className={cx(styles.inputRooms, !numberOfRoomsIsValid
+                && styles.errorBorder)}
+              value={numberOfRooms}
+              onChange={this.changeNumberOfRooms}
+              aria-label={intl.formatMessage(intlMessages.numberOfRooms)}
             >
-              <Button
-                label={intl.formatMessage(intlMessages.addRoomTime)}
-                aria-label={
-                  `${intl.formatMessage(intlMessages.addRoomTime)} ${intl.formatMessage(intlMessages.roomTime, { 0: durationTime + 1 })}`
-                }
-                icon="add"
-                onClick={() => {}}
-                hideLabel
-                circle
-                size="sm"
-              />
-            </HoldButton>
+              {
+                _.range(MIN_BREAKOUT_ROOMS, MAX_BREAKOUT_ROOMS + 1).map(item => (<option key={_.uniqueId('value-')}>{item}</option>))
+              }
+            </select>
           </div>
-        </label>
-        <Button
-          label={intl.formatMessage(intlMessages.randomlyAssign)}
-          className={styles.randomlyAssignBtn}
-          onClick={this.onAssignRandomly}
-          size="sm"
-          color="default"
-        />
-      </div>
+          <label htmlFor="breakoutRoomTime">
+            <p className={styles.labelText} aria-hidden>
+              {intl.formatMessage(intlMessages.duration)}
+            </p>
+            <div className={styles.durationArea}>
+              <input
+                type="number"
+                className={styles.duration}
+                min="1"
+                value={durationTime}
+                onChange={this.changeDurationTime}
+                onBlur={this.blurDurationTime}
+                aria-label={intl.formatMessage(intlMessages.duration)}
+              />
+              <HoldButton
+                key="decrease-breakout-time"
+                exec={this.decreaseDurationTime}
+                minBound={MIN_BREAKOUT_ROOMS}
+                value={durationTime}
+                className={styles.btnStyle}
+              >
+                <Button
+                  label={intl.formatMessage(intlMessages.minusRoomTime)}
+                  aria-label={
+                    `${intl.formatMessage(intlMessages.minusRoomTime)} ${intl.formatMessage(intlMessages.roomTime, { 0: durationTime - 1 })}`
+                  }
+                  icon="substract"
+                  onClick={() => {}}
+                  hideLabel
+                  circle
+                  size="sm"
+                />
+              </HoldButton>
+              <HoldButton
+                key="increase-breakout-time"
+                exec={this.increaseDurationTime}
+                className={styles.btnStyle}
+              >
+                <Button
+                  label={intl.formatMessage(intlMessages.addRoomTime)}
+                  aria-label={
+                    `${intl.formatMessage(intlMessages.addRoomTime)} ${intl.formatMessage(intlMessages.roomTime, { 0: durationTime + 1 })}`
+                  }
+                  icon="add"
+                  onClick={() => {}}
+                  hideLabel
+                  circle
+                  size="sm"
+                />
+              </HoldButton>
+            </div>
+          </label>
+          <Button
+            label={intl.formatMessage(intlMessages.randomlyAssign)}
+            className={styles.randomlyAssignBtn}
+            onClick={this.onAssignRandomly}
+            size="sm"
+            color="default"
+            disabled={!numberOfRoomsIsValid}
+          />
+        </div>
+        <span className={!numberOfRoomsIsValid
+          ? styles.withError : styles.dontShow}
+        >
+          {intl.formatMessage(intlMessages.numberOfRoomsIsValid)}
+        </span>
+      </React.Fragment>
     );
   }
 
@@ -630,6 +667,30 @@ class BreakoutRoom extends Component {
     );
   }
 
+  renderErrorMessages() {
+    const {
+      intl,
+    } = this.props;
+    const {
+      valid,
+      numberOfRoomsIsValid,
+    } = this.state;
+    return (
+      <React.Fragment>
+        {!valid
+          && (
+          <span className={styles.withError}>
+            {intl.formatMessage(intlMessages.leastOneWarnBreakout)}
+          </span>)}
+        {!numberOfRoomsIsValid
+          && (
+          <span className={styles.withError}>
+            {intl.formatMessage(intlMessages.numberOfRoomsIsValid)}
+          </span>)}
+      </React.Fragment>
+    );
+  }
+
   renderDesktop() {
     return [
       this.renderBreakoutForm(),
@@ -642,14 +703,21 @@ class BreakoutRoom extends Component {
     const { intl } = this.props;
     const { formFillLevel } = this.state;
     if (formFillLevel === 2) {
-      return this.renderRoomSortList();
+      return [
+        this.renderErrorMessages(),
+        this.renderRoomSortList(),
+      ];
     }
 
     if (formFillLevel === 3) {
-      return this.renderSelectUserScreen();
+      return [
+        this.renderErrorMessages(),
+        this.renderSelectUserScreen(),
+      ];
     }
 
     return [
+      this.renderErrorMessages(),
       this.renderBreakoutForm(),
       this.renderCheckboxes(),
       this.renderButtonSetLevel(2, intl.formatMessage(intlMessages.nextLabel)),
@@ -679,7 +747,11 @@ class BreakoutRoom extends Component {
 
   render() {
     const { intl, isInvitation } = this.props;
-    const { preventClosing } = this.state;
+    const {
+      preventClosing,
+      valid,
+      numberOfRoomsIsValid,
+    } = this.state;
 
     const BROWSER_RESULTS = browser();
     const isMobileBrowser = BROWSER_RESULTS.mobile || BROWSER_RESULTS.os.includes('Android');
@@ -697,6 +769,7 @@ class BreakoutRoom extends Component {
               ? intl.formatMessage(intlMessages.invitationConfirm)
               : intl.formatMessage(intlMessages.confirmButton),
             callback: isInvitation ? this.onInviteBreakout : this.onCreateBreakouts,
+            disabled: !valid || !numberOfRoomsIsValid,
           }
         }
         dismiss={{

bigbluebutton-html5/imports/ui/components/actions-bar/create-breakout-room/styles.scss

@@ -27,7 +27,7 @@ input[type="number"]::-webkit-outer-spin-button, input[type="number"]::-webkit-i
   grid-template-columns: 2fr 2fr 1fr; 
   grid-template-rows: 1fr;
   grid-gap: 1rem;
-  margin-bottom: 1rem;
+
   @include mq($small-only) {
     grid-template-columns: 1fr ;
     grid-template-rows: 1fr 1fr 1fr; 
@@ -334,4 +334,13 @@ input[type="number"]::-webkit-outer-spin-button, input[type="number"]::-webkit-i
 .checkBoxesContainer {
   display: flex;
   flex-direction: row;
+  margin-top: 1rem;
+}
+
+.withError {
+  color: var(--color-danger);
+}
+
+.errorBorder {
+  border-color: var(--color-danger) !important;
 }

bigbluebutton-html5/private/locales/en.json

@@ -639,6 +639,7 @@
     "app.createBreakoutRoom.leastOneWarnBreakout": "You must place at least one user in a breakout room.",
     "app.createBreakoutRoom.modalDesc": "Complete the steps below to create rooms in your session, To add participants to a room.",
     "app.createBreakoutRoom.roomTime": "{0} minutes",
+    "app.createBreakoutRoom.numberOfRoomsError": "The number of rooms is invalid.",
     "app.externalVideo.start": "Share a new video",
     "app.externalVideo.title": "Share a YouTube video",
     "app.externalVideo.input": "YouTube video URL",