Bug 1796191 - Separate midi and midi-sysex permissions. r=gsvelto.

Existing tests are updated so they pass with this change, and a specific
test case is added to make sure that adding midi-sysex permission does
not automatically add permission to midi.

Differential Revision: https://phabricator.services.mozilla.com/D160370
This commit is contained in:
nchevobbe 2022-10-27 07:22:02 +00:00
parent b43d1756e5
commit 51aa9fcd1f
3 changed files with 118 additions and 51 deletions

View file

@ -50,9 +50,13 @@ NS_IMETHODIMP
MIDIPermissionRequest::GetTypes(nsIArray** aTypes) {
NS_ENSURE_ARG_POINTER(aTypes);
nsTArray<nsString> options;
// NB: We always request midi-sysex, and the base |midi| permission is unused.
// This could be cleaned up at some point.
// The previous implementation made no differences between midi and
// midi-sysex. The check on the SitePermsAddonProvider pref should be removed
// at the same time as the old implementation.
if (mNeedsSysex || !StaticPrefs::dom_sitepermsaddon_provider_enabled()) {
options.AppendElement(u"sysex"_ns);
}
return nsContentPermissionUtils::CreatePermissionArray(mType, options,
aTypes);
}
@ -96,23 +100,24 @@ MIDIPermissionRequest::Run() {
return NS_OK;
}
// Both the spec and our original implementation of WebMIDI have two
// conceptual permission levels: with and without sysex functionality.
// However, our current implementation just has one level, and requires the
// more-powerful |midi-sysex| permission irrespective of the mode requested in
// requestMIDIAccess.
constexpr auto kPermName = "midi-sysex"_ns;
nsCString permName = "midi"_ns;
// The previous implementation made no differences between midi and
// midi-sysex. The check on the SitePermsAddonProvider pref should be removed
// at the same time as the old implementation.
if (mNeedsSysex || !StaticPrefs::dom_sitepermsaddon_provider_enabled()) {
permName.Append("-sysex");
}
// First, check for an explicit allow/deny. Note that we want to support
// granting a permission on the base domain and then using it on a subdomain,
// which is why we use the non-"Exact" variants of these APIs. See bug
// 1757218.
if (nsContentUtils::IsSitePermAllow(mPrincipal, kPermName)) {
if (nsContentUtils::IsSitePermAllow(mPrincipal, permName)) {
Allow(JS::UndefinedHandleValue);
return NS_OK;
}
if (nsContentUtils::IsSitePermDeny(mPrincipal, kPermName)) {
if (nsContentUtils::IsSitePermDeny(mPrincipal, permName)) {
Cancel();
return NS_OK;
}
@ -121,7 +126,7 @@ MIDIPermissionRequest::Run() {
// auto-deny (except for localhost).
if (StaticPrefs::dom_webmidi_gated() &&
!StaticPrefs::dom_sitepermsaddon_provider_enabled() &&
!nsContentUtils::HasSitePerm(mPrincipal, kPermName) &&
!nsContentUtils::HasSitePerm(mPrincipal, permName) &&
!mPrincipal->GetIsLoopbackHost()) {
Cancel();
return NS_OK;

View file

@ -50,16 +50,14 @@ add_task(async function testRequestMIDIAccess() {
"midi-sysex value should have UNKNOWN permission"
);
info("Request midi access");
info("Request midi-sysex access");
let onAddonInstallBlockedNotification = waitForNotification(
"addon-install-blocked"
);
await SpecialPowers.spawn(gBrowser.selectedBrowser, [], () => {
content.midiAccessRequestPromise = content.wrappedJSObject.navigator.requestMIDIAccess(
{
content.midiAccessRequestPromise = content.navigator.requestMIDIAccess({
sysex: true,
}
);
});
});
info("Deny site permission addon install");
@ -88,28 +86,26 @@ add_task(async function testRequestMIDIAccess() {
"SecurityError: WebMIDI requires a site permission add-on to activate"
);
info("Request midi access again");
info("Request midi-sysex access again");
onAddonInstallBlockedNotification = waitForNotification(
"addon-install-blocked"
);
await SpecialPowers.spawn(gBrowser.selectedBrowser, [], () => {
content.midiAccessRequestPromise = content.wrappedJSObject.navigator.requestMIDIAccess(
{
content.midiAccessRequestPromise = content.navigator.requestMIDIAccess({
sysex: true,
}
);
});
});
info("Accept site permission addon install");
addonInstallPanel = await onAddonInstallBlockedNotification;
notification = addonInstallPanel.childNodes[0];
const dialogPromise = waitForInstallDialog();
let dialogPromise = waitForInstallDialog();
notification.button.click();
let installDialog = await dialogPromise;
installDialog.button.click();
info("Wait for the midi access request promise to resolve");
const accessGranted = await SpecialPowers.spawn(
info("Wait for the midi-sysex access request promise to resolve");
let accessGranted = await SpecialPowers.spawn(
gBrowser.selectedBrowser,
[],
async () => {
@ -133,14 +129,22 @@ add_task(async function testRequestMIDIAccess() {
),
"midi-sysex value should have ALLOW permission"
);
ok(
await SpecialPowers.testPermission(
"midi",
SpecialPowers.Services.perms.UNKNOWN_ACTION,
{ url: EXAMPLE_COM_URL }
),
"but midi should have UNKNOWN permission"
);
info("Check that we don't prompt user again once they installed the addon");
const accessPromiseState = await SpecialPowers.spawn(
gBrowser.selectedBrowser,
[],
async () => {
return content.wrappedJSObject.navigator
.requestMIDIAccess()
return content.navigator
.requestMIDIAccess({ sysex: true })
.then(() => "resolved");
}
);
@ -150,6 +154,56 @@ add_task(async function testRequestMIDIAccess() {
"requestMIDIAccess resolved without user prompt"
);
info("Request midi access without sysex");
onAddonInstallBlockedNotification = waitForNotification(
"addon-install-blocked"
);
await SpecialPowers.spawn(gBrowser.selectedBrowser, [], () => {
content.midiNoSysexAccessRequestPromise = content.navigator.requestMIDIAccess();
});
info("Accept site permission addon install");
addonInstallPanel = await onAddonInstallBlockedNotification;
notification = addonInstallPanel.childNodes[0];
dialogPromise = waitForInstallDialog();
notification.button.click();
installDialog = await dialogPromise;
installDialog.button.click();
info("Wait for the midi access request promise to resolve");
accessGranted = await SpecialPowers.spawn(
gBrowser.selectedBrowser,
[],
async () => {
try {
await content.midiNoSysexAccessRequestPromise;
return true;
} catch (e) {}
delete content.midiNoSysexAccessRequestPromise;
return false;
}
);
ok(accessGranted, "requestMIDIAccess resolved");
info("Check that both midi-sysex and midi are now set");
ok(
await SpecialPowers.testPermission(
"midi-sysex",
SpecialPowers.Services.perms.ALLOW_ACTION,
{ url: EXAMPLE_COM_URL }
),
"midi-sysex value should have ALLOW permission"
);
ok(
await SpecialPowers.testPermission(
"midi",
SpecialPowers.Services.perms.ALLOW_ACTION,
{ url: EXAMPLE_COM_URL }
),
"and midi value should also have ALLOW permission"
);
info("Check that we don't prompt user again when they perm denied");
// remove permission to have a clean state
await SpecialPowers.removePermission("midi-sysex", {
@ -160,11 +214,9 @@ add_task(async function testRequestMIDIAccess() {
"addon-install-blocked"
);
await SpecialPowers.spawn(gBrowser.selectedBrowser, [], () => {
content.midiAccessRequestPromise = content.wrappedJSObject.navigator.requestMIDIAccess(
{
content.midiAccessRequestPromise = content.navigator.requestMIDIAccess({
sysex: true,
}
);
});
});
info("Perm-deny site permission addon install");
@ -189,14 +241,14 @@ add_task(async function testRequestMIDIAccess() {
);
is(rejectionMessage, "SecurityError", "requestMIDIAccess was rejected");
info("Request midi access again");
info("Request midi-sysex access again");
rejectionMessage = await SpecialPowers.spawn(
gBrowser.selectedBrowser,
[],
async () => {
let errorMessage;
try {
await content.wrappedJSObject.navigator.requestMIDIAccess({
await content.navigator.requestMIDIAccess({
sysex: true,
});
} catch (e) {
@ -229,7 +281,7 @@ add_task(async function testIframeRequestMIDIAccess() {
"midi-sysex value should have UNKNOWN permission"
);
info("Request midi access from the same-origin iframe");
info("Request midi-sysex access from the same-origin iframe");
const sameOriginIframeBrowsingContext = await SpecialPowers.spawn(
gBrowser.selectedBrowser,
[],
@ -242,11 +294,9 @@ add_task(async function testIframeRequestMIDIAccess() {
"addon-install-blocked"
);
await SpecialPowers.spawn(sameOriginIframeBrowsingContext, [], () => {
content.midiAccessRequestPromise = content.wrappedJSObject.navigator.requestMIDIAccess(
{
content.midiAccessRequestPromise = content.navigator.requestMIDIAccess({
sysex: true,
}
);
});
});
info("Accept site permission addon install");
@ -257,7 +307,7 @@ add_task(async function testIframeRequestMIDIAccess() {
let installDialog = await dialogPromise;
installDialog.button.click();
info("Wait for the midi access request promise to resolve");
info("Wait for the midi-sysex access request promise to resolve");
const accessGranted = await SpecialPowers.spawn(
sameOriginIframeBrowsingContext,
[],
@ -290,8 +340,8 @@ add_task(async function testIframeRequestMIDIAccess() {
gBrowser.selectedBrowser,
[],
async () => {
return content.wrappedJSObject.navigator
.requestMIDIAccess()
return content.navigator
.requestMIDIAccess({ sysex: true })
.then(() => "resolved");
}
);
@ -328,7 +378,7 @@ add_task(async function testIframeRequestMIDIAccess() {
async () => {
let errorName;
try {
await content.wrappedJSObject.navigator.requestMIDIAccess({
await content.navigator.requestMIDIAccess({
sysex: true,
});
} catch (e) {
@ -383,14 +433,16 @@ add_task(async function testRequestMIDIAccessLocalhost() {
);
info(
"Request midi access should not prompt for addon install on locahost, but for permission"
"Request midi-sysex access should not prompt for addon install on locahost, but for permission"
);
let popupShown = BrowserTestUtils.waitForEvent(
PopupNotifications.panel,
"popupshown"
);
await SpecialPowers.spawn(gBrowser.selectedBrowser, [], () => {
content.midiAccessRequestPromise = content.navigator.requestMIDIAccess();
content.midiAccessRequestPromise = content.navigator.requestMIDIAccess({
sysex: true,
});
});
await popupShown;
is(
@ -404,7 +456,7 @@ add_task(async function testRequestMIDIAccessLocalhost() {
.querySelector(".popup-notification-primary-button")
.click();
info("Wait for the midi access request promise to resolve");
info("Wait for the midi-sysex access request promise to resolve");
const accessGranted = await SpecialPowers.spawn(
gBrowser.selectedBrowser,
[],
@ -426,7 +478,7 @@ add_task(async function testRequestMIDIAccessLocalhost() {
"popupshown"
);
await SpecialPowers.spawn(gBrowser.selectedBrowser, [], async () => {
content.navigator.requestMIDIAccess();
content.navigator.requestMIDIAccess({ sysex: true });
});
await popupShown;
is(

View file

@ -58,6 +58,11 @@
SpecialPowers.Services.perms.DENY_ACTION,
document
);
await SpecialPowers.addPermission(
"midi",
SpecialPowers.Services.perms.DENY_ACTION,
document
);
for (let sysex of [false, true]) {
try {
await navigator.requestMIDIAccess({ sysex });
@ -87,7 +92,11 @@
SpecialPowers.Services.perms.ALLOW_ACTION,
document
);
await SpecialPowers.addPermission(
"midi",
SpecialPowers.Services.perms.ALLOW_ACTION,
document
);
// Gated permission should allow access after addon inserted permission.
for (let sysex of [false, true]) {
try {
@ -107,6 +116,7 @@
// Remove the permission.
await SpecialPowers.removePermission("midi-sysex", document);
await SpecialPowers.removePermission("midi", document);
SimpleTest.finish();
}