As a security precaution, we have turned on the setting "Require API key authentication for API requests" for everyone. If this has broken something, please contact bugzilla-admin@mozilla.org
Last Comment Bug 1280448 - Support implicit navigation with basic auth credentials
: Support implicit navigation with basic auth credentials
Status: ASSIGNED
:
Product: Testing
Classification: Components
Component: Marionette (show other bugs)
: Version 3
: All All
: -- normal (vote)
: ---
Assigned To: Andreas Tolfsen ‹:ato›
:
:
Mentors:
Depends on:
Blocks: webdriver
  Show dependency treegraph
 
Reported: 2016-06-16 06:44 PDT by Andreas Tolfsen ‹:ato›
Modified: 2016-06-18 04:44 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments

Description User image Andreas Tolfsen ‹:ato› 2016-06-16 06:44:32 PDT
Navigating to a URL with basic access authentication, for example "https://foo:bar@example.org/", looses the credentials when navigating. The server will respond with a 401 status code and an authentication dialogue.
Comment 1 User image Andreas Tolfsen ‹:ato› 2016-06-16 06:48:53 PDT
I think we should be calling the browser’s loadURI function (https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/Method/loadURI) but currently we are setting the current WindowProxy object’s location property.
Comment 2 User image David Burns :automatedtester 2016-06-16 06:55:45 PDT
we need to implement the setting of basic auth via modals. There has been some basic work done in FirefoxDriver.

Endpoint example https://github.com/SeleniumHQ/selenium/blob/master/py/selenium/webdriver/remote/remote_connection.py#L284
Comment 3 User image Andreas Tolfsen ‹:ato› 2016-06-17 08:31:03 PDT
(In reply to David Burns :automatedtester from comment #2)
> we need to implement the setting of basic auth via modals. There has been
> some basic work done in FirefoxDriver.
> 
> Endpoint example
> https://github.com/SeleniumHQ/selenium/blob/master/py/selenium/webdriver/
> remote/remote_connection.py#L284

Setting credentials for auth prompts is not in the specification at all.  Did we decide to add that as a separate endpoint to the current level or postpone it because it is related to permissions?  I can’t quite remember.

Anyway, this seems unrelated to this bug which is about _navigating_ to a URL that _contains_ basic auth information, which Marionette does not currently support.  This is in violation of the spec.
Comment 4 User image Andreas Tolfsen ‹:ato› 2016-06-18 04:29:08 PDT
So I dug some more and I my above conclusion is wrong.  On my system we’re not actually _leaving out_ the basic auth components of the URL when navigating.  Experimenting with delegating content loading to chrome by calling browser.loadURI makes no difference.

What happens is that I get an nsIPrompt dialogue that isn’t handled, causing the page load timeout to be hit.  The dialogue comes with the following message:

    You are about to log in to the site “127.0.0.1” with the username “user”.

The dialogue originates in nsHttpChannelAuthProvider::ConfirmAuth, where nsIPrompt’s ConfirmEx is called.  As far as I can tell there’s no way to bypass it by setting any preferences.  However, the dialogue does trigger the common-dialog-loaded observer we set up in testing/marionette/driver.js for handling user prompts.

Now the problem is that I have not yet found a way to _differentiate_ the various types of dialogues that cause common- and tabmodal dialogues to be created.  A common-dialog-loaded or tabmodal-dialog-loaded observation event tells us that a dialogue is open, but not which type.  The available types are alert, alertCheck, confirm, confirmCheck, confirmEx, prompt, promptUsernameAndPassword, promptPassword, and select.

The current user prompt handling in Marionette basically does not look at what type of prompt it is dealing with.  This means if faced with a phishing prompt such as this (caused by nsIPromptService.confirm or nsIPromptService.confirmEx) in theory can be dismissed using a call to the GeckoDriver#dismissDialog command.  I think it might be too fragile to inspect the contents/structure of the prompt to determine where it originates from.

I will also note that it’s only Firefox that makes use of a dialogue to warn users they might be submitting authentication information to an unwanted site in a phishing attempt.  Chrome does not do this.  A liberal interpretation of the WebDriver spec, that says the implementation “must navigate” would be that nothing (i.e. a dialogue caused by navigation) should prevent us from reaching our destination.  In this context I think this interpretation makes the most sense since different browser disagree on what to bug the user with.  The case with alert/confirm prompts is different because they’re explicit.
Comment 5 User image Andreas Tolfsen ‹:ato› 2016-06-18 04:44:16 PDT
Selenium’s FirefoxDriver works around this problem by _replacing_ the functions on Ci.nsIPrompt, Ci.nsIAuthPrompt, Ci.nsIAuthPrompt2, and Ci.nsIWritablePropertyBag2 with a delegator pattern class.  When a user in Firefox calls nsIPrompt.confirmEx, this hits their delegator first which then calls the original implementation in nsIPrompt.confirmEx and then signals their own modal dialogue handling which takes the appropriate steps to handle it.

The delegator is called ObservingAlert, which is registered as a new JS module called DrivenPromptService:

    https://github.com/SeleniumHQ/selenium/blob/master/javascript/firefox-driver/js/promptService.js

And the handling occurs in fxdriver.modals:

    https://github.com/SeleniumHQ/selenium/blob/master/javascript/firefox-driver/js/modals.js

This approach strikes me as more invasive than the solution we’ve opted for with Marionette, where we merely observe that a dialogue of whatever type appears, but does give a lot more control and granularity to handle different cases without guessing.

A known problem with Marionette’s current approach is also that we don’t associate prompts with individual <browser>’s, which is a rather serious bug.  This is described in further detail in bug 1263661.

Note You need to log in before you can comment on or make changes to this bug.