1429432 - Consider making Notifications require SecureContext

Status: RESOLVED FIXED

(Core :: DOM: Notifications, enhancement, P1)

Description

[Jonathan Kingston [:jkt] he/him]

Notifications contribute to annoyances in Firefox (Insecure ISP, Cafe wifi could inject these annoyances etc) and also increase HTTPS adoption.

Chrome has already done this in 62. [1]

[1] https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/n37ij1E_1aY

Comment 1

[Johann Hofmann [:johannh]]

This should probably live in the DOM component. There's likely a dupe of it already. FWIW, I'm still planning to add telemetry for this soon.

Comment 2

[Daniel Veditz [:dveditz]]

If chrome has already done this months ago can't we Just Do It?

Comment 3

[Johann Hofmann [:johannh]]

(In reply to Daniel Veditz [:dveditz] from comment #2)
> If chrome has already done this months ago can't we Just Do It?

Well, the telemetry mentioned in comment 1 has been added (https://mzl.la/2neZb8z) and it shows that ~7.4% of permission prompts shown for notifications still come from HTTP websites (quite a lot considering none of them work in Chrome). OTOH the fact that none of them work in Chrome makes me think that most of these sites are pretty spammy and turning this off for them might actually be a service to the user.

I'm not 100% sure who maintains notifications on DOM side nowadays, it might be worth having them weigh in here. Anne, Andrew, do you know?

Comment 4

[Anne (:annevk)]

As far as I know they don't really have an active owner.

Comment 5

[Andrew Overholt [:overholt]]

Anne's correct. I agree with comment 3.

Comment 6

[Johann Hofmann [:johannh]]

I can look into this once the thing that I'm currently doing is done...

Comment 7

[Johann Hofmann [:johannh]]

It would probably be good to get this done in 67 to reduce the noise in our upcoming permission prompt annoyance experiments...

Comment 8

[Johann Hofmann [:johannh]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ca7cc04dc7f17de04c10128a8f4258728a0a4e45

Comment 9

[Johann Hofmann [:johannh]]

Attachment: Bug 1429432 - Require Secure Context for Notifications. r=Ehsan

Comment 10

[Johann Hofmann [:johannh]]

Intent: https://groups.google.com/forum/#!topic/mozilla.dev.platform/FMPrIMGBNtg

Comment 11

[Johann Hofmann [:johannh]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0416434c218b96f4093a4486cfb930a3d2812862

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/279a75b5a6d4
Require Secure Context for Notifications. r=Ehsan

Comment 13

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b3e646df6c5e
Backed out changeset 279a75b5a6d4 for failing at test_notification_insecure_context.html on a CLOSED TREE.

Comment 14

[Raul Gurzau (:RaulG)]

Backed out changeset 279a75b5a6d4 (bug 1429432) for failing at test_notification_insecure_context.html on a CLOSED TREE.

Backout link: https://hg.mozilla.org/integration/autoland/rev/b3e646df6c5e74f625bae22528ab6a6ff739033f

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=279a75b5a6d42a41176750f113594139060d8924&selectedJob=230666485

Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=230666485&repo=autoland&lineNumber=2114

Log snippet:

[task 2019-02-27T00:54:57.212Z] 00:54:57 INFO - TEST-START | dom/notification/test/mochitest/test_notification_insecure_context.html
[task 2019-02-27T00:54:57.304Z] 00:54:57 INFO - GECKO(1430) | ++DOMWINDOW == 37 (0xdd818800) [pid = 1430] [serial = 37] [outer = 0xe1bb7420]
[task 2019-02-27T00:54:57.425Z] 00:54:57 INFO - TEST-INFO | started process screentopng
[task 2019-02-27T00:54:57.926Z] 00:54:57 INFO - TEST-INFO | screentopng: exit 0
[task 2019-02-27T00:54:57.928Z] 00:54:57 INFO - TEST-UNEXPECTED-FAIL | dom/notification/test/mochitest/test_notification_insecure_context.html | Denied permission in insecure context - got "granted", expected "denied"
[task 2019-02-27T00:54:57.930Z] 00:54:57 INFO - SimpleTest.is@SimpleTest/SimpleTest.js:320:16
[task 2019-02-27T00:54:57.931Z] 00:54:57 INFO - runTest@dom/notification/test/mochitest/test_notification_insecure_context.html:32:5
[task 2019-02-27T00:54:57.933Z] 00:54:57 INFO - async*@dom/notification/test/mochitest/test_notification_insecure_context.html:43:5
[task 2019-02-27T00:54:57.934Z] 00:54:57 INFO - TEST-PASS | dom/notification/test/mochitest/test_notification_insecure_context.html | Granted permission in insecure context with pref set
[task 2019-02-27T00:54:57.936Z] 00:54:57 INFO - GECKO(1430) | MEMORY STAT | vsize 593MB | residentFast 278MB | heapAllocated 94MB
[task 2019-02-27T00:54:57.938Z] 00:54:57 INFO - TEST-OK | dom/notification/test/mochitest/test_notification_insecure_context.html | took 297ms

Comment 15

[Johann Hofmann [:johannh]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=dac939d32f6d494bbe68f92fc2cbdf14c87964fa

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ebc59bbdd7ac
Require Secure Context for Notifications. r=Ehsan

Comment 17

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/ebc59bbdd7ac

Comment 18

[Chris Mills [:cmills]]

Note to MDN writers:

I've added a note about this to the Fx67 rel notes:
https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/67#Security

In terms of docs this needs BCD, secure context labels, notifications api adding to secure context page, and anything else you think is needed (check what the notifications guides say too).

Comment 19

[Chris Mills [:cmills]]

OK, documentation done.

I have submitted a PR to update the compat data for this: https://github.com/mdn/browser-compat-data/pull/4091

I have also updated all the notificatios API pages to icnlude secure context banners:
https://developer.mozilla.org/en-US/docs/Web/API/Notifications_API/Using_the_Notifications_API
https://developer.mozilla.org/en-US/docs/Web/API/Notifications_API
https://developer.mozilla.org/en-US/docs/Web/API/Notification

Finally, I updated the secure contexts page to include this updated info:
https://developer.mozilla.org/en-US/docs/Web/Security/Secure_Contexts/features_restricted_to_secure_contexts

Comment 20

[Kohei Yoshino [:kohei]]

Posted site compatibility note: https://www.fxsitecompat.com/en-CA/docs/2019/notifications-api-can-no-longer-be-used-on-insecure-sites/

Comment 21

[Cam]

This change seems to have clobbered WebRTC permission request popups on localhost for FF dev-edition.

Comment 22

[Cam]

Sorry nevermind it was actually this that did it: https://bugzilla.mozilla.org/show_bug.cgi?id=1335740

Comment 23

[Robert Rothenberg]

It would be nice if there was a way to manually remove the https restriction for specific sites, if nothing else, for web development.

Having to set up an htttp daemon as a reverse proxy + SSL certificate just to be able to develop and test this feature makes it a non-starter.

Comment 24

[Robert Rothenberg]

Actually, there is a dom.webnotifications.allowinsecure option. But that's not ideal.

It would be nice to add a site-specific exception.

Comment 25

[Anne (:annevk)]

Well, you could use GitHub or Glitch or some such, but there's bug 1409841 to see if we should add some kind of setting.

Comment 26

[Robert Rothenberg]

It seems you can add a site-specific workaround:

1. In about:config set dom.webnotifications.allowinsecure to true
2. Visit the http site in question, and allow notifications
3. In about:config set dom.webnotifications.allowinsecure back to fall