From da03c3b529576a8fcde6f2c9a171fa6cca012830 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 28 Mar 2023 15:20:34 +0100 Subject: [PATCH] Fix an issue where participants could potentially view video without being displayed themselves Fix from @robintown, manual merge due to Github having issues. --- spec/test-utils/webrtc.ts | 11 +++- spec/unit/webrtc/groupCall.spec.ts | 102 +++++++++++++++++++++++++---- src/webrtc/groupCall.ts | 51 ++++++++++++++- 3 files changed, 146 insertions(+), 18 deletions(-) diff --git a/spec/test-utils/webrtc.ts b/spec/test-utils/webrtc.ts index fce633536..c1a2c2436 100644 --- a/spec/test-utils/webrtc.ts +++ b/spec/test-utils/webrtc.ts @@ -499,12 +499,13 @@ export class MockMatrixCall extends TypedEventEmitter(), stream: new MockMediaStream("stream"), - }; + } as unknown as CallFeed; public remoteUsermediaFeed?: CallFeed; public remoteScreensharingFeed?: CallFeed; @@ -522,6 +523,14 @@ export class MockMatrixCall extends TypedEventEmitter { - if (type === EventType.GroupCallMemberPrefix) { - return userId === undefined - ? (FAKE_STATE_EVENTS as MatrixEvent[]) - : (FAKE_STATE_EVENTS.find((e) => e.getStateKey() === userId) as MatrixEvent); - } else { - const fakeEvent = { getContent: () => ({}), getTs: () => 0 } as MatrixEvent; - return userId === undefined ? [fakeEvent] : fakeEvent; - } -}; +const mockGetStateEvents = + (events: MatrixEvent[] = FAKE_STATE_EVENTS as MatrixEvent[]) => + (type: EventType, userId?: string): MatrixEvent[] | MatrixEvent | null => { + if (type === EventType.GroupCallMemberPrefix) { + return userId === undefined ? events : events.find((e) => e.getStateKey() === userId) ?? null; + } else { + const fakeEvent = { getContent: () => ({}), getTs: () => 0 } as MatrixEvent; + return userId === undefined ? [fakeEvent] : fakeEvent; + } + }; const ONE_HOUR = 1000 * 60 * 60; @@ -567,7 +569,7 @@ describe("Group Call", function () { // the call starts muted, so unmute to get in the right state to test await groupCall.setMicrophoneMuted(false); - mockCall.localUsermediaFeed.setAudioVideoMuted.mockReset(); + mocked(mockCall.localUsermediaFeed.setAudioVideoMuted).mockReset(); let metadataUpdateResolve: () => void; const metadataUpdatePromise = new Promise((resolve) => { @@ -811,7 +813,7 @@ describe("Group Call", function () { mockClient = typedMockClient as unknown as MatrixClient; room = new Room(FAKE_ROOM_ID, mockClient, FAKE_USER_ID_1); - room.currentState.getStateEvents = jest.fn().mockImplementation(mockGetStateEvents); + room.currentState.getStateEvents = jest.fn().mockImplementation(mockGetStateEvents()); room.currentState.members[FAKE_USER_ID_1] = { userId: FAKE_USER_ID_1, membership: "join", @@ -987,7 +989,14 @@ describe("Group Call", function () { mockClient = typedMockClient as unknown as MatrixClient; room = new Room(FAKE_ROOM_ID, mockClient, FAKE_USER_ID_2); - room.getMember = jest.fn().mockImplementation((userId) => ({ userId })); + room.currentState.members[FAKE_USER_ID_1] = { + userId: FAKE_USER_ID_1, + membership: "join", + } as unknown as RoomMember; + room.currentState.members[FAKE_USER_ID_2] = { + userId: FAKE_USER_ID_2, + membership: "join", + } as unknown as RoomMember; groupCall = await createAndEnterGroupCall(mockClient, room); }); @@ -1060,6 +1069,71 @@ describe("Group Call", function () { expect(call.answerWithCallFeeds).toHaveBeenCalled(); }); + const aliceEnters = () => { + room.currentState.getStateEvents = jest.fn().mockImplementation( + mockGetStateEvents([ + { + getContent: () => ({ + "m.calls": [ + { + "m.call_id": groupCall.groupCallId, + "m.devices": [ + { + device_id: FAKE_DEVICE_ID_1, + session_id: FAKE_SESSION_ID_1, + expires_ts: Date.now() + ONE_HOUR, + feeds: [], + }, + ], + }, + ], + }), + getStateKey: () => FAKE_USER_ID_1, + getRoomId: () => FAKE_ROOM_ID, + getTs: () => 0, + }, + ] as unknown as MatrixEvent[]), + ); + room.currentState.emit(RoomStateEvent.Update, room.currentState); + }; + + const aliceLeaves = () => { + room.currentState.getStateEvents = jest + .fn() + .mockImplementation(mockGetStateEvents([] as unknown as MatrixEvent[])); + room.currentState.emit(RoomStateEvent.Update, room.currentState); + }; + + it("enables tracks on expected calls, then disables them when the participant leaves", async () => { + aliceEnters(); + + const mockCall = new MockMatrixCall(room.roomId, groupCall.groupCallId); + mockCall.answerWithCallFeeds.mockImplementation(([feed]) => (mockCall.localUsermediaFeed = feed)); + mockClient.emit(CallEventHandlerEvent.Incoming, mockCall as unknown as MatrixCall); + + // Tracks should be enabled + expect(mockCall.localUsermediaFeed.stream.getTracks().every((t) => t.enabled)).toBe(true); + + aliceLeaves(); + + // Tracks should be disabled + expect(mockCall.localUsermediaFeed.stream.getTracks().every((t) => !t.enabled)).toBe(true); + }); + + it("disables tracks on unexpected calls, then enables them when the participant joins", async () => { + const mockCall = new MockMatrixCall(room.roomId, groupCall.groupCallId); + mockCall.answerWithCallFeeds.mockImplementation(([feed]) => (mockCall.localUsermediaFeed = feed)); + mockClient.emit(CallEventHandlerEvent.Incoming, mockCall as unknown as MatrixCall); + + // Tracks should be disabled + expect(mockCall.localUsermediaFeed.stream.getTracks().every((t) => !t.enabled)).toBe(true); + + aliceEnters(); + + // Tracks should be enabled + expect(mockCall.localUsermediaFeed.stream.getTracks().every((t) => t.enabled)).toBe(true); + }); + describe("handles call being replaced", () => { let callChangedListener: jest.Mock; let oldMockCall: MockMatrixCall; @@ -1157,7 +1231,7 @@ describe("Group Call", function () { userId: FAKE_USER_ID_2, membership: "join", } as unknown as RoomMember; - room.currentState.getStateEvents = jest.fn().mockImplementation(mockGetStateEvents); + room.currentState.getStateEvents = jest.fn().mockImplementation(mockGetStateEvents()); groupCall = await createAndEnterGroupCall(mockClient, room); }); diff --git a/src/webrtc/groupCall.ts b/src/webrtc/groupCall.ts index 0504066aa..e378ad2c2 100644 --- a/src/webrtc/groupCall.ts +++ b/src/webrtc/groupCall.ts @@ -351,6 +351,17 @@ export class GroupCall extends TypedEventEmitter< ); } + /** + * Determines whether the given call is one that we were expecting to exist + * given our knowledge of who is participating in the group call. + */ + private callExpected(call: MatrixCall): boolean { + const userId = getCallUserId(call); + const member = userId === null ? null : this.room.getMember(userId); + const deviceId = call.getOpponentDeviceId(); + return member !== null && deviceId !== undefined && this.participants.get(member)?.get(deviceId) !== undefined; + } + public async initLocalCallFeed(): Promise { if (this.state !== GroupCallState.LocalCallFeedUninitialized) { throw new Error(`Cannot initialize local call feed in the "${this.state}" state.`); @@ -630,7 +641,9 @@ export class GroupCall extends TypedEventEmitter< this.initWithAudioMuted = muted; } - this.forEachCall((call) => setTracksEnabled(call.localUsermediaFeed!.stream.getAudioTracks(), !muted)); + this.forEachCall((call) => + setTracksEnabled(call.localUsermediaFeed!.stream.getAudioTracks(), !muted && this.callExpected(call)), + ); this.emit(GroupCallEvent.LocalMuteStateChanged, muted, this.isLocalVideoMuted()); if (!sendUpdatesBefore) await sendUpdates(); @@ -679,6 +692,12 @@ export class GroupCall extends TypedEventEmitter< this.forEachCall((call) => updates.push(call.setLocalVideoMuted(muted))); await Promise.all(updates); + // We setTracksEnabled again, independently from the call doing it + // internally, since we might not be expecting the call + this.forEachCall((call) => + setTracksEnabled(call.localUsermediaFeed!.stream.getVideoTracks(), !muted && this.callExpected(call)), + ); + this.emit(GroupCallEvent.LocalMuteStateChanged, this.isMicrophoneMuted(), muted); return true; @@ -812,9 +831,18 @@ export class GroupCall extends TypedEventEmitter< ); if (prevCall) this.disposeCall(prevCall, CallErrorCode.Replaced); - this.initCall(newCall); - newCall.answerWithCallFeeds(this.getLocalFeeds().map((feed) => feed.clone())); + + const feeds = this.getLocalFeeds().map((feed) => feed.clone()); + if (!this.callExpected(newCall)) { + // Disable our tracks for users not explicitly participating in the + // call but trying to receive the feeds + for (const feed of feeds) { + setTracksEnabled(feed.stream.getAudioTracks(), false); + setTracksEnabled(feed.stream.getVideoTracks(), false); + } + } + newCall.answerWithCallFeeds(feeds); deviceMap.set(newCall.getOpponentDeviceId()!, newCall); this.calls.set(opponentUserId, deviceMap); @@ -1476,6 +1504,23 @@ export class GroupCall extends TypedEventEmitter< private onRoomState = (): void => this.updateParticipants(); private onParticipantsChanged = (): void => { + // Re-run setTracksEnabled on all calls, so that participants that just + // left get denied access to our media, and participants that just + // joined get granted access + this.forEachCall((call) => { + const expected = this.callExpected(call); + for (const feed of call.getLocalFeeds()) { + setTracksEnabled( + feed.stream.getAudioTracks(), + !feed.isAudioMuted() && expected, + ); + setTracksEnabled( + feed.stream.getVideoTracks(), + !feed.isVideoMuted() && expected, + ); + } + }); + if (this.state === GroupCallState.Entered) this.placeOutgoingCalls(); };