Dismiss basic auth dialogue implicitly

RESOLVED FIXED in Firefox 55

Status

defect
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: julius.schwartzenberg, Assigned: julius.schwartzenberg)

Tracking

(Blocks 1 bug)

unspecified
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(2 attachments)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Firefox/45.0
Build ID: 20170203041559

Steps to reproduce:

Run a Selenium+WebDriver test that opens a URL in this form:
http://longusername@somehostname:8888/somepath


Actual results:

The test hung with a prompt asking whether I was sure I wanted to login with 'longusername' to 'somehostname'.


Expected results:

There shouldn't have been any prompt, the test should have finished properly.
Summary: Tests are interrupted by username+password physhing prompts → Tests are interrupted by username+password phishing prompts
Do you have any kind of further information about the pref you are using here?
There is some documentation on this preference here: http://kb.mozillazine.org/Network.http.phishy-userpass-length

The number is the maximum allowed number of characters HTTP basic auth username+password. The maximum is 255. By setting this preference, the dialog is suppressed similarly to how other security warnings are suppressed.
So none of our test harnesses is modifying this preference, and I would say that Marionette shouldn't do this neither. Usually test writers know their websites and as such can handle that dialog if in some cases the username and password are longer.

But I would leave this decision up to David or Andreas.
Flags: needinfo?(dburns)
Flags: needinfo?(ato)
Thanks for your patch.  It cannot be directly applied because there are some strange filenames in the patch file, but I will apply it and submit it through mozreview.

(In reply to Henrik Skupin (:whimboo) from comment #3)
> So none of our test harnesses is modifying this preference, and I would say
> that Marionette shouldn't do this neither. Usually test writers know their
> websites and as such can handle that dialog if in some cases the username
> and password are longer.

But this patch is not modifying Marionette: it is modifying geckodriver,
and WebDriver users expect this behaviour.  This is a known bug with our
WebDriver implementation.

If setting this preference causes the global modal to go away, then I’m all
for it.
Flags: needinfo?(dburns)
Flags: needinfo?(ato)
OS: Unspecified → All
Hardware: Unspecified → All
Blocks: webdriver
Summary: Tests are interrupted by username+password phishing prompts → Dismiss basic auth dialogue implicitly
Yes, that's the official Mozilla preference to suppress that dialog.

I included the path names with the Mercurial changeset ID in case it would be useful to know on which changeset I based the patch, I think it should apply with the right -p switch in any case.

In some of our tests we switch between different users within a single run, so being able to specify the username+password through the URL is very useful. It would be great if you could apply it!
It would be helpful if you generated an actual .patch file instead of a diff.  This would be much easier for me to apply, but I’m fine to do the manual legwork too.
You mean a Mercurial patch? I didn't manage to figure out how to fetch just the geckodriver with Mercurial, which is why I used diff with just the geckodriver part with a tarball from the Hg web interface. I thought it would give a similar result, except that maybe the strip path option would need to be used. If there's a better trick I'd be glad to learn about it. If you can apply it manually that's great of course. Thanks!
Assignee: nobody → julius.schwartzenberg
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Comment on attachment 8875031 [details]
Bug 1370597 - Dismiss basic auth dialogue implicitly;

https://reviewboard.mozilla.org/r/146388/#review150396
Attachment #8875031 - Flags: review?(ato) → review+
Thanks for your patch!  I’ve rebased, reviewed it, and sent it off for integration.
Pushed by atolfsen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/215afe84f16e
Dismiss basic auth dialogue implicitly; r=ato
(In reply to Andreas Tolfsen ‹:ato› from comment #4)
> But this patch is not modifying Marionette: it is modifying geckodriver,
> and WebDriver users expect this behaviour.  This is a known bug with our
> WebDriver implementation.

I see. In that case, lets get it in and we can ship it with the upcoming 0.16.2 release of geckodriver, which I'm currently preparing.
Blocks: 1369709
https://hg.mozilla.org/mozilla-central/rev/215afe84f16e
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.