Check permission on mute mic, only if no audio track exists. (#3359)

* check permission only if no audio track

* fix linter issues

* add missing tests for perfect negotiation pattern

* add null case in unit tests for audio muting

* fix issue with type MediaStream

* force right type of mock methode

* format code

spec/unit/webrtc/call.spec.ts

@@ -25,6 +25,7 @@ import {
     CallType,
     CallState,
     CallParty,
+    CallDirection,
 } from "../../../src/webrtc/call";
 import {
     MCallAnswer,
@@ -1712,4 +1713,110 @@ describe("Call", function () {
             expect(onReplace).toHaveBeenCalled();
         });
     });
+    describe("should handle glare in negotiation process", () => {
+        beforeEach(async () => {
+            // cut methods not want to test
+            call.hangup = () => null;
+            call.isLocalOnHold = () => true;
+            // @ts-ignore
+            call.updateRemoteSDPStreamMetadata = jest.fn();
+            // @ts-ignore
+            call.getRidOfRTXCodecs = jest.fn();
+            // @ts-ignore
+            call.createAnswer = jest.fn().mockResolvedValue({});
+            // @ts-ignore
+            call.sendVoipEvent = jest.fn();
+        });
+
+        it("and reject remote offer if not polite and have pending local offer", async () => {
+            // not polite user == CallDirection.Outbound
+            call.direction = CallDirection.Outbound;
+            // have already a local offer
+            // @ts-ignore
+            call.makingOffer = true;
+            const offerEvent = makeMockEvent("@test:foo", {
+                description: {
+                    type: "offer",
+                    sdp: DUMMY_SDP,
+                },
+            });
+            // @ts-ignore
+            call.peerConn = {
+                signalingState: "have-local-offer",
+                setRemoteDescription: jest.fn(),
+            };
+            await call.onNegotiateReceived(offerEvent);
+            expect(call.peerConn?.setRemoteDescription).not.toHaveBeenCalled();
+        });
+
+        it("and not reject remote offer if not polite and do have pending answer", async () => {
+            // not polite user == CallDirection.Outbound
+            call.direction = CallDirection.Outbound;
+            // have not a local offer
+            // @ts-ignore
+            call.makingOffer = false;
+
+            // If we have a setRemoteDescription() answer operation pending, then
+            // we will be "stable" by the time the next setRemoteDescription() is
+            // executed, so we count this being readyForOffer when deciding whether to
+            // ignore the offer.
+            // @ts-ignore
+            call.isSettingRemoteAnswerPending = true;
+            const offerEvent = makeMockEvent("@test:foo", {
+                description: {
+                    type: "offer",
+                    sdp: DUMMY_SDP,
+                },
+            });
+            // @ts-ignore
+            call.peerConn = {
+                signalingState: "have-local-offer",
+                setRemoteDescription: jest.fn(),
+            };
+            await call.onNegotiateReceived(offerEvent);
+            expect(call.peerConn?.setRemoteDescription).toHaveBeenCalled();
+        });
+
+        it("and not reject remote offer if not polite and do not have pending local offer", async () => {
+            // not polite user == CallDirection.Outbound
+            call.direction = CallDirection.Outbound;
+            // have no local offer
+            // @ts-ignore
+            call.makingOffer = false;
+            const offerEvent = makeMockEvent("@test:foo", {
+                description: {
+                    type: "offer",
+                    sdp: DUMMY_SDP,
+                },
+            });
+            // @ts-ignore
+            call.peerConn = {
+                signalingState: "stable",
+                setRemoteDescription: jest.fn(),
+            };
+            await call.onNegotiateReceived(offerEvent);
+            expect(call.peerConn?.setRemoteDescription).toHaveBeenCalled();
+        });
+
+        it("and if polite do rollback pending local offer", async () => {
+            // polite user == CallDirection.Inbound
+            call.direction = CallDirection.Inbound;
+            // have already a local offer
+            // @ts-ignore
+            call.makingOffer = true;
+            const offerEvent = makeMockEvent("@test:foo", {
+                description: {
+                    type: "offer",
+                    sdp: DUMMY_SDP,
+                },
+            });
+            // @ts-ignore
+            call.peerConn = {
+                signalingState: "have-local-offer",
+                setRemoteDescription: jest.fn(),
+            };
+            await call.onNegotiateReceived(offerEvent);
+            expect(call.peerConn?.setRemoteDescription).toHaveBeenCalled();
+        });
+    });
 });

spec/unit/webrtc/groupCall.spec.ts

@@ -517,8 +517,7 @@ describe("Group Call", function () {
                 await groupCall.setMicrophoneMuted(false);
                 expect(groupCall.isMicrophoneMuted()).toEqual(false);
 
-                jest.advanceTimersByTime(groupCall.pttMaxTransmitTime + 100);
-
+                await jest.advanceTimersByTimeAsync(groupCall.pttMaxTransmitTime + 100);
                 expect(groupCall.isMicrophoneMuted()).toEqual(true);
             });
 
@@ -585,7 +584,15 @@ describe("Group Call", function () {
                 });
                 mockCall.sendMetadataUpdate = jest.fn().mockReturnValue(metadataUpdatePromise);
 
+                const getUserMediaStreamFlush = Promise.resolve("stream");
+                // @ts-ignore
+                mockCall.cleint = {
+                    getMediaHandler: {
+                        getUserMediaStream: jest.fn().mockReturnValue(getUserMediaStreamFlush),
+                    },
+                };
                 const mutePromise = groupCall.setMicrophoneMuted(true);
+                await getUserMediaStreamFlush;
                 // we should be muted at this point, before the metadata update has been sent
                 expect(groupCall.isMicrophoneMuted()).toEqual(true);
                 expect(mockCall.localUsermediaFeed.setAudioVideoMuted).toHaveBeenCalled();
@@ -892,14 +899,34 @@ describe("Group Call", function () {
                 expect(await groupCall.setMicrophoneMuted(false)).toBe(false);
             });
 
-            it("returns false when no permission for audio stream", async () => {
+            it("returns false when no permission for audio stream and localCallFeed do not have an audio track", async () => {
                 const groupCall = await createAndEnterGroupCall(mockClient, room);
+                // @ts-ignore
+                jest.spyOn(groupCall.localCallFeed, "hasAudioTrack", "get").mockReturnValue(false);
                 jest.spyOn(mockClient.getMediaHandler(), "getUserMediaStream").mockRejectedValueOnce(
                     new Error("No Permission"),
                 );
                 expect(await groupCall.setMicrophoneMuted(false)).toBe(false);
             });
 
+            it("returns false when user media stream null", async () => {
+                const groupCall = await createAndEnterGroupCall(mockClient, room);
+                // @ts-ignore
+                jest.spyOn(groupCall.localCallFeed, "hasAudioTrack", "get").mockReturnValue(false);
+                // @ts-ignore
+                jest.spyOn(mockClient.getMediaHandler(), "getUserMediaStream").mockResolvedValue({} as MediaStream);
+                expect(await groupCall.setMicrophoneMuted(false)).toBe(false);
+            });
+
+            it("returns true when no permission for audio stream but localCallFeed has a audio track already", async () => {
+                const groupCall = await createAndEnterGroupCall(mockClient, room);
+                // @ts-ignore
+                jest.spyOn(groupCall.localCallFeed, "hasAudioTrack", "get").mockReturnValue(true);
+                jest.spyOn(mockClient.getMediaHandler(), "getUserMediaStream");
+                expect(mockClient.getMediaHandler().getUserMediaStream).not.toHaveBeenCalled();
+                expect(await groupCall.setMicrophoneMuted(false)).toBe(true);
+            });
+
             it("returns false when unmuting video with no video device", async () => {
                 const groupCall = await createAndEnterGroupCall(mockClient, room);
                 jest.spyOn(mockClient.getMediaHandler(), "hasVideoDevice").mockResolvedValue(false);

src/webrtc/callFeed.ts

@@ -128,7 +128,7 @@ export class CallFeed extends TypedEventEmitter<CallFeedEvent, EventHandlerMap>
         this.emit(CallFeedEvent.ConnectedChanged, this.connected);
     }
 
-    private get hasAudioTrack(): boolean {
+    public get hasAudioTrack(): boolean {
         return this.stream.getAudioTracks().length > 0;
     }
 

src/webrtc/groupCall.ts

@@ -655,27 +655,9 @@ export class GroupCall extends TypedEventEmitter<
                 `GroupCall ${this.groupCallId} setMicrophoneMuted() (streamId=${this.localCallFeed.stream.id}, muted=${muted})`,
             );
 
-            // We needed this here to avoid an error in case user join a call without a device.
-            // I can not use .then .catch functions because linter :-(
-            try {
-                if (!muted) {
-                    const stream = await this.client
-                        .getMediaHandler()
-                        .getUserMediaStream(true, !this.localCallFeed.isVideoMuted());
-                    if (stream === null) {
-                        // if case permission denied to get a stream stop this here
-                        /* istanbul ignore next */
-                        logger.log(
-                            `GroupCall ${this.groupCallId} setMicrophoneMuted() no device to receive local stream, muted=${muted}`,
-                        );
-                        return false;
-                    }
-                }
-            } catch (e) {
-                /* istanbul ignore next */
-                logger.log(
-                    `GroupCall ${this.groupCallId} setMicrophoneMuted() no device or permission to receive local stream, muted=${muted}`,
-                );
+            const hasPermission = await this.checkAudioPermissionIfNecessary(muted);
+
+            if (!hasPermission) {
                 return false;
             }
 
@@ -700,6 +682,42 @@ export class GroupCall extends TypedEventEmitter<
         return true;
     }
 
+    /**
+     * If we allow entering a call without a camera and without video, it can happen that the access rights to the
+     * devices have not yet been queried. If a stream does not yet have an audio track, we assume that the rights have
+     * not yet been checked.
+     *
+     * `this.client.getMediaHandler().getUserMediaStream` clones the current stream, so it only wanted to be called when
+     * not Audio Track exists.
+     * As such, this is a compromise, because, the access rights should always be queried before the call.
+     */
+    private async checkAudioPermissionIfNecessary(muted: boolean): Promise<boolean> {
+        // We needed this here to avoid an error in case user join a call without a device.
+        try {
+            if (!muted && this.localCallFeed && !this.localCallFeed.hasAudioTrack) {
+                const stream = await this.client
+                    .getMediaHandler()
+                    .getUserMediaStream(true, !this.localCallFeed.isVideoMuted());
+                if (stream?.getTracks().length === 0) {
+                    // if case permission denied to get a stream stop this here
+                    /* istanbul ignore next */
+                    logger.log(
+                        `GroupCall ${this.groupCallId} setMicrophoneMuted() no device to receive local stream, muted=${muted}`,
+                    );
+                    return false;
+                }
+            }
+        } catch (e) {
+            /* istanbul ignore next */
+            logger.log(
+                `GroupCall ${this.groupCallId} setMicrophoneMuted() no device or permission to receive local stream, muted=${muted}`,
+            );
+            return false;
+        }
+
+        return true;
+    }
+
     /**
      * Sets the mute state of the local participants's video.
      * @param muted - Whether to mute the video