1694418 - Update http auth modal prompt for proton

Status: VERIFIED FIXED

(Toolkit Graveyard :: Notifications and Alerts, enhancement, P3)

Description

[:Gijs (he/him)]

The prompt title should be the domain (x-ref bug 1693008) - no protocol.

The prompt text should be:

This site is asking you to sign in.

(We deliberately want to stop displaying the "realm" information)

The input fields should be "Username" and "Password".
They should appear above the input fields.

The button texts should be:

Sign in

Cancel

See complete spec on figma.

Comment 1

[:Gijs (he/him)]

Update priority to reflect proton priorities.

Comment 2

[Erica Wright [:ewright]]

What about for cross-origin cases? Currently: "1$S is requesting your username and password. WARNING: Your password will not be sent to the website you are currently visiting"

Comment 3

[Meridel [:meridel]]

Would this be classified as a JavaScript Modal Dialog: OnBeforeUnload () modal? And, do you have a screenshot of the current modal + copy that you can share with me?

Comment 4

[:Gijs (he/him)]

(In reply to Meridel from comment #3)

Would this be classified as a JavaScript Modal Dialog: OnBeforeUnload () modal?

No, it's the same as the normal http auth dialog for which the spec has complete text and a design, but with an extra warning text at the end of the descriptive text.

And, do you have a screenshot of the current modal + copy that you can share with me?

You can see the dialog yourself by copy-pasting this URL:

data:text/html,<iframe src="https://jigsaw.w3.org/HTTP/Basic/">

into the URL bar in a new tab.

Comment 5

[Meridel [:meridel]]

Thanks, Gijs. I have a draft and have flagged Emanuela to review in the deck. This is a bit unusual because a warning icon may be warranted but the placement isn't quite right. Will circle back.

Comment 6

[Meridel [:meridel]]

Emanuela, please see my proposal (which is not working), and let me know your thoughts. Happy to meet and discuss!

https://docs.google.com/presentation/d/1YtPJEvUigRybLkuoALYvl63MVg_a6-Db4orDzNdEkIk/edit#slide=id.gc50cde94a0_3_0

Comment 7

[Meridel [:meridel]]

Copy has been updated and is reflected in slide 47 of the content deck, as well as in the Figma page. Signed off by security team and legal.

Copy deck: https://docs.google.com/presentation/d/1YtPJEvUigRybLkuoALYvl63MVg_a6-Db4orDzNdEkIk/edit#slide=id.gc6499ad969_0_0
Figma: https://www.figma.com/file/FjUe6ORvXZgJvI3rPuTV33/Desktop-UI-(Mozilla-Confidential-)?node-id=5%3A2

Comment 8

[:Gijs (he/him)]

The patch in bug 1699183 moves the icon we want to use here.

Comment 9

[:Gijs (he/him)]

Attachment: Bug 1694418 - show origins in the title of the auth prompt, r?pbz!,mtigley!

Depends on D108713

Comment 10

[:Gijs (he/him)]

Attachment: Bug 1694418 - update username/password strings in the http auth dialog and pre-land other strings, r?mtigley

Depends on D108857

Comment 11

[Romain Testard [:RT]]

Marking as P1. Per experience review we agreed to mark as P1 bug the ones that will block MR1.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8bb3057a5996
update username/password strings in the http auth dialog and pre-land other strings, r=mtigley,fluent-reviewers,flod

Comment 13

[Natalia Csoregi [:nataliaCs]]

Backed out for failures on browser_ext_optionsPage_modals.js

backout: https://hg.mozilla.org/integration/autoland/rev/9328d26aec28ef3ac4f0d8d82a241ec944d652e2

push: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&revision=8bb3057a5996e04407204b45a68713577a59974a&selectedTaskRun=apiPgKHZTo6lfEEU-yAxUw.0

failure log: https://treeherder.mozilla.org/logviewer?job_id=336705716&repo=autoland&lineNumber=13759

[task 2021-04-15T21:03:39.484Z] 21:03:39 INFO - TEST-INFO | screenshot: exit 0
[task 2021-04-15T21:03:39.485Z] 21:03:39 INFO - Buffered messages logged at 21:03:38
[task 2021-04-15T21:03:39.485Z] 21:03:39 INFO - Entering test bound test_tab_options_modals
[task 2021-04-15T21:03:39.485Z] 21:03:39 INFO - Extension loaded
[task 2021-04-15T21:03:39.486Z] 21:03:39 INFO - Buffered messages logged at 21:03:39
[task 2021-04-15T21:03:39.486Z] 21:03:39 INFO - Console message: Warning: attempting to write 20155 bytes to preference extensions.webextensions.uuids. This is bad for general performance and memory usage. Such an amount of data should rather be written to an external file. This preference will not be sent to any content processes.
[task 2021-04-15T21:03:39.487Z] 21:03:39 INFO - Wait the options_ui modal to be opened
[task 2021-04-15T21:03:39.487Z] 21:03:39 INFO - Buffered messages finished
[task 2021-04-15T21:03:39.487Z] 21:03:39 INFO - TEST-UNEXPECTED-FAIL | browser/components/extensions/test/browser/browser_ext_optionsPage_modals.js | Expect a tab modal opened for the about addons tab - 0 == 1 - JS frame :: chrome://mochitests/content/browser/browser/components/extensions/test/browser/browser_ext_optionsPage_modals.js :: test_tab_options_modals :: line 71
[task 2021-04-15T21:03:39.487Z] 21:03:39 INFO - Stack trace:
[task 2021-04-15T21:03:39.488Z] 21:03:39 INFO - chrome://mochitests/content/browser/browser/components/extensions/test/browser/browser_ext_optionsPage_modals.js:test_tab_options_modals:71
[task 2021-04-15T21:03:39.488Z] 21:03:39 INFO - Not taking screenshot here: see the one that was previously logged

also failing: https://treeherder.mozilla.org/logviewer?job_id=336702545&repo=autoland&lineNumber=12870

[task 2021-04-15T20:48:29.455Z] 20:48:29 ERROR - TEST-UNEXPECTED-ERROR | testing/marionette/harness/marionette_harness/tests/unit/test_execute_async_script.py TestExecuteAsyncContent.test_return_value_on_alert | marionette_driver.errors.ScriptTimeoutException: Timed out after 1000 ms
[task 2021-04-15T20:48:29.455Z] 20:48:29 INFO - stacktrace:
[task 2021-04-15T20:48:29.456Z] 20:48:29 INFO - WebDriverError@chrome://marionette/content/error.js:181:5
[task 2021-04-15T20:48:29.456Z] 20:48:29 INFO - ScriptTimeoutError@chrome://marionette/content/error.js:423:5
[task 2021-04-15T20:48:29.456Z] 20:48:29 INFO - evaluate.sandbox/timeoutPromise</scriptTimeoutID<@chrome://marionette/content/evaluate.js:107:16
[task 2021-04-15T20:48:29.457Z] 20:48:29 INFO - notify@resource://gre/modules/Timer.jsm:62:17
[task 2021-04-15T20:48:29.457Z] 20:48:29 INFO - Traceback (most recent call last):
[task 2021-04-15T20:48:29.457Z] 20:48:29 INFO - File "C:\Users\task_1618518821\build\venv\lib\site-packages\marionette_harness\marionette_test\testcases.py", line 214, in run
[task 2021-04-15T20:48:29.457Z] 20:48:29 INFO - testMethod()
[task 2021-04-15T20:48:29.458Z] 20:48:29 INFO - File "C:\Users\task_1618518821\build\tests\marionette\tests\testing\marionette\harness\marionette_harness\tests\unit\test_execute_async_script.py", line 207, in test_return_value_on_alert
[task 2021-04-15T20:48:29.458Z] 20:48:29 INFO - res = self.marionette.execute_async_script("alert()")
[task 2021-04-15T20:48:29.459Z] 20:48:29 INFO - File "C:\Users\task_1618518821\build\venv\lib\site-packages\marionette_driver\marionette.py", line 1729, in execute_async_script
[task 2021-04-15T20:48:29.459Z] 20:48:29 INFO - rv = self._send_message("WebDriver:ExecuteAsyncScript", body, key="value")
[task 2021-04-15T20:48:29.460Z] 20:48:29 INFO - File "C:\Users\task_1618518821\build\venv\lib\site-packages\marionette_driver\decorators.py", line 27, in _
[task 2021-04-15T20:48:29.460Z] 20:48:29 INFO - return func(*args, **kwargs)
[task 2021-04-15T20:48:29.460Z] 20:48:29 INFO - File "C:\Users\task_1618518821\build\venv\lib\site-packages\marionette_driver\marionette.py", line 629, in _send_message
[task 2021-04-15T20:48:29.461Z] 20:48:29 INFO - self._handle_error(err)
[task 2021-04-15T20:48:29.461Z] 20:48:29 INFO - File "C:\Users\task_1618518821\build\venv\lib\site-packages\marionette_driver\marionette.py", line 651, in _handle_error
[task 2021-04-15T20:48:29.461Z] 20:48:29 INFO - raise errors.lookup(error)(message, stacktrace=stacktrace)
[task 2021-04-15T20:48:29.462Z] 20:48:29 INFO - TEST-INFO took 1075ms

Comment 14

[:Gijs (he/him)]

Bah, we apparently use tabmodalprompts for http auth still in some edgecases? Relanding without the DTD removal for now, that'll fix the tests...

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/81a9057ae48b
update username/password strings in the http auth dialog and pre-land other strings, r=mtigley,fluent-reviewers,flod

Comment 16

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/81a9057ae48b

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/669b31a70306
show origins in the title of the auth prompt, r=pbz,mtigley

Comment 18

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/669b31a70306

Comment 19

[:Gijs (he/him)]

Oops, forgot to clear leave-open.

Comment 20

[Rares Doghi, Desktop QA]

This issue is verified as fixed in our latest beta 89.0b5 bulds on Mac, Windows and Ubuntu.

Comment 21

[Julian Reschke]

Out of curiosity:

(We deliberately want to stop displaying the "realm" information)

Why did you decide to do that?

Comment 22

[:Gijs (he/him)]

(In reply to Julian Reschke from comment #21)

Out of curiosity:

(We deliberately want to stop displaying the "realm" information)

Why did you decide to do that?

In no particular order:

• it's a spoofing vector
• in adversarial situations it's basically arbitrary attacker-controlled text displayed in browser UI, which is tricky to display safely at the best of times, even besides spoofing
• other browsers do not display it
• cases where the realm is "necessary" because there is more than 1 realm on the same host are (a) setups that we should not encourage, and (b) vanishingly rare, (c) in most of those cases, we expect users know how they end up somewhere ("I clicked a link to app X instead of app Y")

Comment 23

[phep]

(In reply to :Gijs (he/him) from comment #22)

May I respectfully disagree on the the relevance of the 2 last items by pointing you to Chrome issue #544244 "HTTP basic auth credentials prompt should make the origin stand out more" were the subject has been debated at length and showed how relative was the relevance of your first 2 items.

Please reconsider the removal of the AuthName strings since :

• it's contrary to RFC7235 section 2.2,
• it breaks much more site authentication system than you say : the Chrome issue comments clearly show how wrong is your 4th item,
• it does not bring much security back in the man-in-the-middle case (once again see the above cited Chrome issue).

Please consider instead improving the display of the server provided string by adding some warning.

Thanks in advance,

phep