Merge branch 'main' into test-public-user

.github/workflows/check-pull-request-title.yml

@@ -0,0 +1,35 @@
+name: check-pull-request-title
+
+on:
+  pull_request:
+    types: [edited, opened]
+
+permissions:
+  contents: read
+
+jobs:
+  run:
+    runs-on: ubuntu-22.04
+    steps:
+      - name: Validate PR title
+        run: |
+          set -e
+
+          pr_name="${{ github.event.pull_request.title }}"
+          echo "PR Title: '${pr_name}'"
+
+          # Regex to check following:
+          # Starts with "VIDCS-" followed by digits
+          # A colon (:) followed by exactly one space
+          # followed by the actual PR title text
+          regex="^VIDCS-[0-9]+: [^ ].*$"
+
+          if [[ "$pr_name" =~ $regex ]]; then
+            echo "Acceptable title"
+            exit 0
+          else
+            echo "Rejected title"
+            echo "Reason: The PR title must start with 'VIDCS-' followed by digits, a colon (:), and exactly one space, with the rest of the title immediately after the space."
+            exit 1
+          fi
+        shell: bash

.github/workflows/lint.yml

@@ -10,7 +10,7 @@ concurrency:
 
 jobs:
   run:
-    runs-on: [self-hosted]
+    runs-on: ubuntu-22.04
 
     steps:
       - name: Checkout
@@ -41,29 +41,6 @@ jobs:
         run: |
           yarn lint:filenames
 
-      - name: Validate PR title
-        run: |
-          set -e
-
-          pr_name="${{ github.event.pull_request.title }}"
-          echo "PR Title: '$pr_name'"
-
-          # Regex to check following:
-          # Starts with "VIDCS-" followed by digits
-          # A colon (:) followed by exactly one space
-          # followed by the actual PR title text
-          regex="^VIDCS-[0-9]+: [^ ].*$"
-
-          if [[ "$pr_name" =~ $regex ]]; then
-            echo "Acceptable title"
-            exit 0
-          else
-            echo "Rejected title"
-            echo "Reason: The PR title must start with 'VIDCS-' followed by digits, a colon (:), and exactly one space, with the rest of the title immediately after the space."
-            exit 1
-          fi
-        shell: bash
-
       - name: Run license check
         run: |
           npx license-checker --json > scripts/allLicenseResults.json

.github/workflows/run-tests.yml

@@ -15,7 +15,7 @@ env:
 
 jobs:
   run:
-    runs-on: ubuntu-latest
+    runs-on: ubuntu-22.04
 
     steps:
       - name: Check for "skip-ci" label

.github/workflows/update-screenshots.yml

@@ -15,7 +15,7 @@ env:
 
 jobs:
   run:
-    runs-on: ubuntu-latest
+    runs-on: ubuntu-22.04
     permissions:
       contents: write
     steps:

backend/package.json

@@ -22,7 +22,7 @@
     "body-parser": "^1.20.3",
     "cors": "^2.8.5",
     "dotenv": "^16.0.3",
-    "express": "^4.21.0",
+    "express": "^4.21.2",
     "form-data": "^4.0.1",
     "opentok": "^2.16.0"
   },

frontend/package.json

@@ -59,6 +59,6 @@
     "resize-observer-polyfill": "^1.5.1",
     "tailwindcss": "^3.4.14",
     "typedoc": "^0.26.10",
-    "vite": "^5.4"
+    "vite": "^5.4.6"
   }
 }

frontend/src/components/RoomNameInput/RoomNameInput.spec.tsx

@@ -12,21 +12,60 @@ describe('RoomNameInputComponent', () => {
     setHasError = vi.fn();
   });
 
-  it('should set the room name if the input is valid', () => {
-    render(
-      <RoomNameInput
-        setRoomName={setRoomName}
-        setHasError={setHasError}
-        roomName="test-room"
-        hasError={false}
-      />
-    );
+  describe('should set the room name', () => {
+    const testCases = [
+      { input: 'test_room' },
+      { input: 'test+room' },
+      { input: 'another-test_room' },
+      { input: '123testroom' },
+    ];
 
-    const input = screen.getByRole('textbox');
-    fireEvent.change(input, { target: { value: 'new-test-room' } });
+    testCases.forEach(({ input }) => {
+      it('if the input is valid', () => {
+        render(
+          <RoomNameInput
+            setRoomName={setRoomName}
+            setHasError={setHasError}
+            roomName="test-room"
+            hasError={false}
+          />
+        );
 
-    expect(setHasError).toHaveBeenCalledWith(false);
-    expect(setRoomName).toHaveBeenCalledWith('new-test-room');
+        const inputBox = screen.getByRole('textbox');
+        fireEvent.change(inputBox, { target: { value: input } });
+
+        expect(setHasError).toHaveBeenCalledWith(false);
+        expect(setRoomName).toHaveBeenCalledWith(input);
+      });
+    });
+  });
+
+  describe('should reject the name', () => {
+    const testCases = [
+      { input: 'test@room' },
+      { input: 'invalid#name' },
+      { input: 'invalid/room/name' },
+      { input: 'invalid%room%name' },
+    ];
+
+    testCases.forEach(({ input }) => {
+      it('if the input is invalid', () => {
+        render(
+          <RoomNameInput
+            setRoomName={setRoomName}
+            setHasError={setHasError}
+            roomName="test-room"
+            hasError={false}
+          />
+        );
+
+        const inputBox = screen.getByRole('textbox');
+        fireEvent.change(inputBox, { target: { value: input } });
+
+        expect(setHasError).toHaveBeenCalledWith(true);
+        expect(setRoomName).not.toHaveBeenCalledWith(input);
+      });
+    });
   });
 
   it('should set the room name as lowercase even if the parameter is a mix of both lower and upper case', () => {

frontend/src/components/RoomNameInput/RoomNameInput.tsx

@@ -1,6 +1,7 @@
 import { SetStateAction, Dispatch, ReactElement, ChangeEvent } from 'react';
 import { InputAdornment, TextField } from '@mui/material';
 import { Keyboard } from '@mui/icons-material';
+import isValidRoomName from '../../utils/isValidRoomName';
 
 export type RoomNameInputProps = {
   setRoomName: Dispatch<SetStateAction<string>>;
@@ -26,12 +27,6 @@ const RoomNameInput = ({
   hasError,
   setHasError,
 }: RoomNameInputProps): ReactElement => {
-  const validateRoomName = (name: string) => {
-    // Regular expression to allow letters, numbers, underscores, and hyphens only
-    const regex = /^[a-z0-9_-]+$/;
-    return regex.test(name);
-  };
-
   const handleChange = (textChangeEvent: ChangeEvent<HTMLInputElement>) => {
     const newValue = textChangeEvent.target.value.toLowerCase();
 
@@ -42,7 +37,7 @@ const RoomNameInput = ({
       return;
     }
 
-    if (validateRoomName(newValue)) {
+    if (isValidRoomName(newValue)) {
       setHasError(false);
       setRoomName(newValue);
     } else {

frontend/src/components/SoundTest/SoundTest.tsx

@@ -16,7 +16,7 @@ export type SoundTestProps = {
  */
 const SoundTest = ({ children }: SoundTestProps): ReactElement => {
   const [audioIsPlaying, setAudioIsPlaying] = useState(false);
-  const audio = useMemo(() => new Audio('http://localhost:3345/sound.mp3'), []);
+  const audio = useMemo(() => new Audio('/sound.mp3'), []);
   const { audioOutput } = useAudioOutputContext();
 
   useEffect(() => {

frontend/src/components/WaitingRoom/UserNameInput/UserNameInput.tsx

@@ -6,6 +6,7 @@ import { useNavigate } from 'react-router-dom';
 import useUserContext from '../../../hooks/useUserContext';
 import { UserType } from '../../../Context/user';
 import useRoomName from '../../../hooks/useRoomName';
+import isValidRoomName from '../../../utils/isValidRoomName';
 
 export type UserNameInputProps = {
   username: string;
@@ -67,7 +68,9 @@ const UsernameInput = ({ username, setUsername }: UserNameInputProps): ReactElem
   const handleJoinClick = (event: MouseEvent) => {
     event.preventDefault();
     if (validateForm() && roomName) {
-      const sanitizedRoomName = encodeURIComponent(roomName);
+      if (!isValidRoomName(roomName)) {
+        return;
+      }
       setUser((prevUser: UserType) => ({
         ...prevUser,
         defaultSettings: {
@@ -79,7 +82,7 @@ const UsernameInput = ({ username, setUsername }: UserNameInputProps): ReactElem
       // This takes the user to the meeting room and allows them to enter it
       // Otherwise if they entered the room directly, they are going to be redirected back to the waiting room
       // Setting hasAccess is required so that we are not redirected back to the waiting room
-      navigate(`/room/${sanitizedRoomName}`, {
+      navigate(`/room/${roomName}`, {
         state: {
           hasAccess: true,
         },

frontend/src/pages/MeetingRoom/MeetingRoom.tsx

@@ -11,6 +11,7 @@ import RightPanel from '../../components/MeetingRoom/RightPanel';
 import useRoomName from '../../hooks/useRoomName';
 import useHeight from '../../hooks/useHeight';
 import { PUBLISHING_BLOCKED_CAPTION } from '../../utils/constants';
+import isValidRoomName from '../../utils/isValidRoomName';
 
 /**
  * MeetingRoom Component
@@ -49,9 +50,8 @@ const MeetingRoom = (): ReactElement => {
   const navigate = useNavigate();
 
   useEffect(() => {
-    if (joinRoom && roomName) {
-      const sanitizedRoomName = encodeURIComponent(roomName);
-      joinRoom(sanitizedRoomName);
+    if (joinRoom && isValidRoomName(roomName)) {
+      joinRoom(roomName);
     }
     return () => {
       // Ensure to disconnect session when unmounting meeting room in order

frontend/src/utils/isValidRoomName/index.ts

@@ -0,0 +1,3 @@
+import isValidRoomName from './isValidRoomName';
+
+export default isValidRoomName;

frontend/src/utils/isValidRoomName/isValidRoomName.spec.ts

@@ -0,0 +1,21 @@
+import { describe, it, expect } from 'vitest';
+import isValidRoomName from './isValidRoomName';
+
+describe('isValidRoomName', () => {
+  const testCases = [
+    { input: 'room_name', expected: true },
+    { input: 'room+name', expected: true },
+    { input: 'another-room_name', expected: true },
+    { input: '123roomname', expected: true },
+    { input: 'room@name', expected: false },
+    { input: 'room#name', expected: false },
+    { input: 'room$name', expected: false },
+  ];
+
+  testCases.forEach(({ input, expected }) => {
+    it(`should return ${expected} for "${input}"`, () => {
+      const result = isValidRoomName(input);
+      expect(result).toBe(expected);
+    });
+  });
+});

frontend/src/utils/isValidRoomName/isValidRoomName.ts

@@ -0,0 +1,7 @@
+const isValidRoomName = (name: string) => {
+  // Regular expression to allow letters, numbers, underscores, hyphens, and plus sign only
+  const regex = /^[a-z0-9_+-]+$/;
+  return regex.test(name);
+};
+
+export default isValidRoomName;

integration-tests/playwright.config.ts

@@ -23,6 +23,7 @@ const executablePath = isMac ? '/Applications/Opera.app/Contents/MacOS/Opera' :
 
 const fakeDeviceChromiumFlags = [
   ...chromiumFlags,
+  '--headless=new',
   '--use-fake-device-for-media-stream=device-count=5',
 ];
 

integration-tests/tests/multiparty.spec.ts

@@ -62,6 +62,10 @@ test('should display username on publisher and subscribers', async ({
 
   await pageOne.waitForSelector('.publisher', { state: 'visible' });
 
+  await pageOne
+    .getByTestId('publisher-container')
+    .getByText('User One')
+    .waitFor({ state: 'visible' });
   expect(await pageOne.getByTestId('publisher-container').getByText('User One')).toBeVisible();
 
   const pageTwo = await context.newPage();
@@ -91,6 +95,8 @@ test('should display initials on publisher and subscribers', async ({
   await waitAndClickFirefox(pageOne, browserName);
 
   await pageOne.waitForSelector('.publisher', { state: 'visible' });
+
+  await pageOne.getByText(/SO/).waitFor({ state: 'visible' });
   await expect(await pageOne.getByText(/SO/)).toBeVisible();
 
   const pageTwo = await context.newPage();
@@ -133,6 +139,7 @@ test.describe('display name for screenshare', () => {
     });
 
     await pageOne.waitForSelector('.publisher', { state: 'visible' });
+    await pageTwo.waitForSelector('.subscriber', { state: 'visible' });
     const screenshareButton = await pageOne.getByTestId('ScreenShareIcon');
     await screenshareButton.click();
 

package.json

@@ -71,9 +71,13 @@
     "typescript": "^5.4.5"
   },
   "resolutions": {
-    "wrap-ansi": "7.0.0"
-  },
-  "dependencies": {
-    "rollup": "^4.22.4"
+    "wrap-ansi": "7.0.0",
+    "path-to-regexp": "^0.1.12",
+    "express": "^4.21.2",
+    "axios": "^1.7.4",
+    "cross-spawn": "^7.0.5",
+    "rollup": "^4.22.4",
+    "vite": "^5.4.12",
+    "nanoid": "^3.3.8"
   }
 }