1493982 - SetSinkId: Implement authorization mechanism

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, enhancement, P3)

Description

[Alex Chronopoulos [:achronop]]

We need a way to authorize an application to play audio through the requested output device. 

One idea is to create something similar to the gUM  authorization model for the output device (prompt the user to allow or deny the access to a device).

Chrome's authorization model is to allow any device that has been previously allowed a gUM call. Note that Chrome prompt only once, the first time, after that it remembers the answer for that site until permission is being revoked explicitly .

Comment 1

[CoolCmd]

(In reply to Alex Chronopoulos [:achronop] from comment #0)
> One idea is to create something similar to the gUM  authorization model for
> the output device (prompt the user to allow or deny the access to a device).
why not use the gUM itself? for example, the current standard allows following MediaTrackConstraints:

{audio: {deviceId: [d1.deviceId, d2.deviceId]}}

where

d1.kind == d2.kind == 'audiooutput'

> Chrome's authorization model is to allow any device that has been previously
> allowed a gUM call.
this is ugly authorization model:
1) not all devices have audio input (microphone is not connected, blocked for privacy, etc).
2) audio input is much more dangerous than audio output.

Comment 2

[Alex Chronopoulos [:achronop]]

We could use something similar to gUM. The difference here is that the device is selected in advance so we want a doorhanger to allow or not the selected device.

Comment 3

[Alex Chronopoulos [:achronop]]

Chrome's authorization model is to allow any device that has been previously allowed a gUM call.

According to crbug.com/865799 this is a bug and they are planning to create an authorization prompt for output devices.

Comment 4

[Sagar]

Tried this just now on meet.google.com with the pref media.setsinkid.enabled set to true on MacOS 10.15.3 (19D76) with Firefox Nightly 76.0a1 (2020-03-29) (64-bit)

It does not work for WebRTC during the call but does work for the 'Test' audio bit (likely a local file)

What's the status?

Comment 5

[Alex Chronopoulos [:achronop]]

See https://bugzilla.mozilla.org/show_bug.cgi?id=1498512#c11

Comment 6

[Alex Chronopoulos [:achronop]]

Attachment: Bug 1493982 - Implement an authorization model for SetSinkId. r?jib

An application will be authorized to change the sink device (execute setsinkid) if a getUserMedia with the microphone permission has been accepted. This is not the best thing to do but Chrome has been doing the same for some time now thus if we were more restrictive fewer users would use it. In addition to that, the authorization model of setsinkid is discussed actively in the spec so we will update it when a decision has been taken.

Comment 7

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

The current plan is to implement the new selectAudioOutput API for obtaining a deviceId to use with setSinkId which solves authorization.

Comment 8

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Hi Karl, this patch adds the "speaker-selection" permissions policy which we're still missing AFAICT. Wanna commandeer it and repurpose it for selectAudioOutput, or do you have a patch in the works for this elsewhere?

Comment 9

[Karl Tomlinson (:karlt)]

Thanks. I'd forgotten that this patch included feature policy checks for bug 1577199, which is still missing. I'll sort something out for that, but it is likely to be different enough that it may be preferable to start a new revision rather than to commandeer this patch for bug 1577199.

Comment 10

[Karl Tomlinson (:karlt)]

Attachment: Bug 1493982 remove declaration of missing EnumerateDevices() method r?jib

Depends on D117985

Comment 11

[Karl Tomlinson (:karlt)]

Attachment: Bug 1493982 don't catch exceptions thrown by testharness asserts r?jib

Depends on D118436

Comment 12

[Karl Tomlinson (:karlt)]

Attachment: Bug 1493982 test a different device in each iteration r=jib

Depends on D118437

Comment 13

[Karl Tomlinson (:karlt)]

Attachment: Bug 1493982 use getUserMedia() to expose some output devices r?jib

Without this change, enumerateDevices() always returns no audiooutput devices
since https://github.com/w3c/mediacapture-output/pull/109

Mozilla will usually run the test without media.navigator.streams.fake because
fake input devices are in different groups to the output devices. The
exception is Mozilla's Mac test machines because they have no real audio input
device. With fake input devices, there are no exposed output devices to test.
Another test will be added separately to expose an output device via
selectAudioOutput().

Depends on D118438

Comment 14

[Karl Tomlinson (:karlt)]

Attachment: Bug 1493982 Test setSinkId() before and after selectAudioOutput() r?jib

Depends on D118439

Comment 15

[Karl Tomlinson (:karlt)]

Attachment: Bug 1493982 track explicitly granted audio output devices r?jib

Depends on D118440

Comment 16

[Karl Tomlinson (:karlt)]

Attachment: Bug 1493982 add mCanExposeMicrophoneInfo and set from getUserMedia() r?jib

This will be used to limit exposure of audio output devices by group id.
Exposure of microphone devices is not yet limited but covered by bug 1528042.

Depends on D118441

Comment 17

[Karl Tomlinson (:karlt)]

Attachment: Bug 1493982 limit speaker exposure in enumerateDevices() to those granted by selectAudioOutput() and getUserMedia() r?jib

selectAudioOutput() grants are per-device, but getUserMedia() grants expose all
speakers associated with any microphone.

When exposed, speaker devices have labels even when there is no active capture device.

Depends on D118442

Comment 18

[Karl Tomlinson (:karlt)]

Attachment: Bug 1493982 move GetSinkDevice() to MediaDevices for access to device exposure history r?jib

Depends on D118443

Comment 19

[Karl Tomlinson (:karlt)]

Attachment: Bug 1493982 accept only sinkIds exposed via enumerateDevices() r?jib

and reject with NotFoundError otherwise.

Depends on D118444

Comment 20

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Thanks Karl, do you need me to approve Alex's patch here, or do you plan on gating this on the "speaker-selection" permissions policy in a different bug?

It might be good to land that at the same time, to not expose this API in all iframes.

Comment 21

[Karl Tomlinson (:karlt)]

(In reply to Jan-Ivar Bruaroey [:jib] (needinfo? me) from comment #20)

Thanks Karl, do you need me to approve Alex's patch here, or do you plan on gating this on the "speaker-selection" permissions policy in a different bug?

I have abandoned Alex's patch, thanks, and plan to cover that in bug 1577199.

It might be good to land that at the same time, to not expose this API in all iframes.

Changes here are strictly reducing the exposure of devices currently being exposed, but "speaker-selection" shouldn't take long to add if there's a good reason to wait for that.

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/171787d11d24
remove declaration of missing EnumerateDevices() method r=jib
https://hg.mozilla.org/integration/autoland/rev/e2902ed93301
don't catch exceptions thrown by testharness asserts r=jib
https://hg.mozilla.org/integration/autoland/rev/b720ac41da2c
test a different device in each iteration r=jib
https://hg.mozilla.org/integration/autoland/rev/b10f8d05a934
use getUserMedia() to expose some output devices r=jib
https://hg.mozilla.org/integration/autoland/rev/799062439b47
Test setSinkId() before and after selectAudioOutput() r=jib

Comment 23

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/29498 for changes under testing/web-platform/tests

Comment 24

[Karl Tomlinson (:karlt)]

Attachment: Bug 1493982 remove trailing whitespace

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/65fdb9177835
Fix trailing whitespace lint failure in setSinkId-with-selectAudioOutput.https.html

Comment 26

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 27

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/171787d11d24
https://hg.mozilla.org/mozilla-central/rev/e2902ed93301
https://hg.mozilla.org/mozilla-central/rev/b720ac41da2c
https://hg.mozilla.org/mozilla-central/rev/b10f8d05a934
https://hg.mozilla.org/mozilla-central/rev/799062439b47
https://hg.mozilla.org/mozilla-central/rev/65fdb9177835

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d386961b7c3c
track explicitly granted audio output devices r=jib
https://hg.mozilla.org/integration/autoland/rev/a4f134902b8e
add mCanExposeMicrophoneInfo and set from getUserMedia() r=jib

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/35d3a828f598
limit speaker exposure in enumerateDevices() to those granted by selectAudioOutput() and getUserMedia() r=jib
https://hg.mozilla.org/integration/autoland/rev/08db56e52b6d
move GetSinkDevice() to MediaDevices for access to device exposure history r=jib
https://hg.mozilla.org/integration/autoland/rev/83c72e4bcbfe
accept only sinkIds exposed via enumerateDevices() r=jib

Comment 30

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/d386961b7c3c
https://hg.mozilla.org/mozilla-central/rev/a4f134902b8e
https://hg.mozilla.org/mozilla-central/rev/35d3a828f598
https://hg.mozilla.org/mozilla-central/rev/08db56e52b6d
https://hg.mozilla.org/mozilla-central/rev/83c72e4bcbfe