1395886 - Add support for proxy authentication (username / password)

Status: NEW

(Remote Protocol :: Marionette, enhancement, P3)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

To allow the usage of proxies which require authentication, support for setting the username / password has to be added to Marionette.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Here is the webdriver-rust part for this bug: https://github.com/mozilla/webdriver-rust/pull/123

Implementing this feature actually means that Marionette has to use the password manager to store the credentials. There are some gaps to solve:

1) Correctly save the username and password in the password manager
2) Ensure it will be used when accessing the proxy server without requesting it from the user
3) Ensure 2) also works after a restart

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Henrik Skupin (:whimboo) from comment #1)
> Here is the webdriver-rust part for this bug:
> https://github.com/mozilla/webdriver-rust/pull/123

Given that webdriver is part of the tree now, I will re-work this PR so it applies to m-c.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

The demand for this feature might be low at the moment. So we will defer the implementation for a while.

Comment 4

[Øyvind Strømmen (:insulaventus)]

Any idea when this will be implemented?

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2]]

It depends on how many different requests we get. So far no-one asked for it. As such we do not have an ETA for it yet.

Comment 6

[Øyvind Strømmen (:insulaventus)]

I see. Is there an official channel for asking for new features? 

Also, in addition to the webdriver-rust part, which other components would need to be updated. I'm asking because I might be interested in contributing to having the feature implemented. If such a contribution would be welcome of course.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Sure, and we would totally support you if you want to contribute to get this feature implemented. Basically all what is needed fro webdriver-rust I have already written and is part of https://github.com/mozilla/webdriver-rust/pull/123. Given that the repository has been moved into mozilla-central, the patch needs to be adapted.

Otherwise all the important work which needs to be done has to be in Marionette and is JS code. It mostly involves work around capability parsing, and interacting with the Firefox password manager to set the username and password for the specific proxy host.

https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Using_nsIPasswordManager

Overall it might not be that complicated.

If you have questions as best join us on IRC and ask me directly. Details in how to get started can be found here: https://wiki.mozilla.org/User:Mjzffr/New_Contributors.

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+2]]

CC'ing Ian who wants to work on this bug.

Comment 9

[Ian MacLeod (:skeletor)]

I would indeed like to work on this bug! I have a couple of questions:

Am I right that usernames and passwords should be stored in a nsIPasswordManager that would be a property of the session.Proxy class in session.js?

It seems that Proxy can have host and port info for each possible protocol. Should it then hold a separate username and password for each protocol in a single PasswordManager? If so, should it store a username for each protocol outside of the PasswordManager to handle cases of two different protocols with the same host but different authentication values?

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Ian MacLeod (:skeletor) from comment #9)
> Am I right that usernames and passwords should be stored in a
> nsIPasswordManager that would be a property of the session.Proxy class in
> session.js?

We don't need a property for that. When initializing the proxy data you would just create an instance of this interface. It has to be used to store the data for the given host. Later it would be not necessary given that Firefox itself uses the password manager to retrieve the information.

> It seems that Proxy can have host and port info for each possible protocol.
> Should it then hold a separate username and password for each protocol in a
> single PasswordManager? If so, should it store a username for each protocol
> outside of the PasswordManager to handle cases of two different protocols
> with the same host but different authentication values?

Hm, it's a good question in how we have to handle that case. I believe that there should still be a separation between different protocols when using the same host. But it could still be that for each proxy type the same host/port combination is used, and as such we should just overwrite the data. Personally I would call this more a user error. But maybe the webdriver spec should be clearer about such a situation. Would you mind filing an issue at https://github.com/w3c/webdriver/issues/ so it can be made more clear, or should I do it? Just let me know.

Comment 11

[Ian MacLeod (:skeletor)]

Here's a quick rundown on the problems I am having with loginManager.

When I try to use Services.logins in https://dxr.mozilla.org/mozilla-central/source/testing/marionette/session.js and run test_session.js, an "Initialization failed" error is thrown in obj-x86_64-pc-mingw32/dist/bin/components/storage-json.js. If I dump() the value of OS.Constants.Path.profileDir in storage-json.js its value is undefined.

When I run the test at https://dxr.mozilla.org/mozilla-central/source/browser/components/extensions/test/xpcshell/test_ext_browsingData_passwords.js there is no problem during the initialization of the loginManager, and dumping OS.Constants.Path.profileDir in storage-json.js yields a defined value.

I've tried putting a delay before the tests in test_session.js run, and I've tried initializing the loginManager in the same way in both files, but the results haven't changed.

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Ian, do you see the same problem when you have this change in session.js, and normally run our Marionette unit tests? Or is that failure only present for xpcshell tests? If it's only visible for xpcshell, would you mind to pastebin your changes/additions? I would like to test it out myself locally. Btw. which platform are you on?

Comment 13

[Ian MacLeod (:skeletor)]

It works when I run the Marionette unit tests. test_proxy.js still has an expected failure, but the initialization of the loginManager succeeds. I'm on 64 bit Windows 10.

Here's a pastebin of the diff showing my only change to session.js: https://pastebin.com/5t4dTvXb

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Ian MacLeod (:skeletor) from comment #11)
> Here's a quick rundown on the problems I am having with loginManager.
> 
> When I try to use Services.logins in
> https://dxr.mozilla.org/mozilla-central/source/testing/marionette/session.js
> and run test_session.js, an "Initialization failed" error is thrown in
> obj-x86_64-pc-mingw32/dist/bin/components/storage-json.js. If I dump() the
> value of OS.Constants.Path.profileDir in storage-json.js its value is
> undefined.

I tried myself and with a debug build I see this helpful output:

0:03.79 PROCESS_OUTPUT: Thread-3 (pid:24845) "[24845, Main Thread] WARNING: NSS will be initialized without a profile directory. Some things may not work as expected.: file /builds/worker/workspace/build/src/security/manager/ssl/nsNSSComponent.cpp, line 1790"

Which means that there is no profile directory set, which is also described here:
https://wiki.mozilla.org/Security/CryptoEngineering/Platform_Use_of_NSS#The_Desired_Setup

Tim, do you know if it is possible to run certain xpcshell tests with a profile directory? We would need that for the Marionette tests. If not we would have to catch this failure in Marionette (session.js), and could not set any proxy authentication for the new session command. Mainly this would only affect xpcshell tests only, so for this case we could write Marionette unit tests.

Comment 15

[Tim Taubert [:ttaubert] (inactive)]

(In reply to Henrik Skupin (:whimboo) from comment #14)
> Tim, do you know if it is possible to run certain xpcshell tests with a
> profile directory? We would need that for the Marionette tests.

I don't know much about the implementation of the xpcshell test framework, sorry.

Comment 16

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Tim Taubert [:ttaubert] from comment #15)
> > Tim, do you know if it is possible to run certain xpcshell tests with a
> > profile directory? We would need that for the Marionette tests.
> 
> I don't know much about the implementation of the xpcshell test framework,
> sorry.

Ok, thanks anyway. We might have to find someone else then, but it's not a blocker right now given that for normal unit tests it should work.

Ian, the last patch which you submitted via pastebin does still not apply for me. Can you please check that again?

Comment 17

[Henrik Skupin [:whimboo][⌚️UTC+2]]

As we noticed this bug is kinda complex to solve. As such Ian will work on something else, and we might have to fix this bug ourselves when there is time for. Ian, thank you anyway for trying it, and pointing us to the known issues!

Comment 18

[Henrik Skupin [:whimboo][⌚️UTC+2]]

The problem as has been seen above was that the call to the login manager happened in proxy.fromJSON(), while it should have happened in proxy.init(). Why this is happening you can read here on bug 1411207 comment 3.

Basically what we should do is to add properties on the proxy class like:

this.ftpProxyAuth = {username: "…", password: "…"}

Then init() can take this dictionary and setup the logins.

Ian, maybe that solves your blocker and you might be interested to continue to work on it?

Comment 19

[Ian MacLeod (:skeletor)]

Thanks! Yeah, I think I'd be interested in continuing to work on this now, I'll let you know if I get stuck!

Comment 20

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Ian MacLeod (:skeletor) from comment #19)
> Thanks! Yeah, I think I'd be interested in continuing to work on this now,
> I'll let you know if I get stuck!

Hi Ian, I wanted to check back with you. I hope all is good on your side, and maybe we can find some time to continue the work on this bug? 

The latest state I have is that storing the credentials works, but for the very first time when accessing the proxy the authentication dialog pops-up, even with the preference set to auto-login.

Comment 21

[Ian MacLeod (:skeletor)]

Hi, yes, that is still where I'm at. I will try to be around the next few days to work on it.

Comment 22

[Henrik Skupin [:whimboo][⌚️UTC+2]]

That sounds good. I have seen that you tried to reach me over the weekend. But I wasn't around at this time.

So for now I would suggest that you just continue on the patch, and when testing the changes fill out the dialog yourself. Once a working patch is available, I will apply the patch myself locally and have a look. Does that work for you?

Comment 23

[Ian MacLeod (:skeletor)]

Here is an update on where I'm at with this bug:
1. When I start a Marionette session using a proxy server with authentication, I store the login in the LoginManager. When the server requests authentication, I get a popup with the username and password fields filled in. I want to use the login from the LoginManager without a popup appearing, but I haven't managed to get that to work. I have tried setting "signon.autologin.proxy" to true in server.js to no avail. I get the same behavior when I am using Firefox without Marionette.
2. When adding logins to LoginManager an httpRealm is required. The proxy server includes an HTTP Realm when it requests authentication and if the login's stored value doesn't match, then the login isn't used. I'm not sure what the best way is to acquire this httpRealm for the proxy server.
3. An exception is thrown when adding a login if one already exists for that host/port/httpRealm. I was wondering what the right behavior is when this happens, and it seems to me that if the existing login matches the new one we can just continue, but if they are different we should throw a new exception, does that seem right?

Comment 24

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Ian MacLeod (:skeletor) from comment #23)
> 1. When I start a Marionette session using a proxy server with
> authentication, I store the login in the LoginManager. When the server
> requests authentication, I get a popup with the username and password fields
> filled in. I want to use the login from the LoginManager without a popup
> appearing, but I haven't managed to get that to work. I have tried setting
> "signon.autologin.proxy" to true in server.js to no avail. I get the same
> behavior when I am using Firefox without Marionette.

This might be bug 973064. It would be good if you could verify that, and if that is the case we should reopen that other bug.

For now we could add a workaround to the test which switches to the dialog, and accepts it. Check the modal dialog tests under Marionette unit tests in how to do it.

> 2. When adding logins to LoginManager an httpRealm is required. The proxy
> server includes an HTTP Realm when it requests authentication and if the
> login's stored value doesn't match, then the login isn't used. I'm not sure
> what the best way is to acquire this httpRealm for the proxy server.

If we really have to use the same HTTPRealm for storing the login credentials we would have to initiate a GET request to retrieve it. Maybe a HEAD request would also be sufficient. Could you check with eg curl how the proxy responds on both of such requests?

> 3. An exception is thrown when adding a login if one already exists for that
> host/port/httpRealm. I was wondering what the right behavior is when this
> happens, and it seems to me that if the existing login matches the new one
> we can just continue, but if they are different we should throw a new
> exception, does that seem right?

The WebDriver spec doesn't tell anything about it, but I would say we take the very first credential, and simply ignore the exception. We could write an info or warning message to the log mentioning that.

Ian, it would be great if you could upload the current code as WIP, so I could have a look at it, and test myself. Thanks.

Comment 25

[Ian MacLeod (:skeletor)]

(In reply to Henrik Skupin (:whimboo) from comment #24)
> This might be bug 973064. It would be good if you could verify that, and if
> that is the case we should reopen that other bug.

The behavior that I'm seeing is very similar to what Manish was reporting. One difference is that I have to close the alert 5 or 6 times before it gives up. How would I go about verifying that I'm experiencing the same issue?

> For now we could add a workaround to the test which switches to the dialog,
> and accepts it. Check the modal dialog tests under Marionette unit tests in
> how to do it.

Yeah, that works for me!

> If we really have to use the same HTTPRealm for storing the login
> credentials we would have to initiate a GET request to retrieve it. Maybe a
> HEAD request would also be sufficient. Could you check with eg curl how the
> proxy responds on both of such requests?

So I didn't have success sending requests to the proxy server directly, but if I send an unauthenticated request using the proxy server as a proxy it responds with a Proxy-Authenticate header which contains the httpRealm. For example:
curl -I -x <proxy url> <random url>

> Ian, it would be great if you could upload the current code as WIP, so I
> could have a look at it, and test myself. Thanks.

Here is a export of my current patch: https://pastebin.com/kh2Nv1SN

Let me know if it would be better if I pushed it for review.

Comment 26

[Ian MacLeod (:skeletor)]

The current version of my patch is here: https://pastebin.com/1bP0kLjw

Comment 27

[Ian MacLeod (:skeletor)]

Attachment: Bug 1395886 Add support for proxy authentication

Review commit: https://reviewboard.mozilla.org/r/216616/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/216616/

Comment 28

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Ian MacLeod (:skeletor) from comment #25)
> (In reply to Henrik Skupin (:whimboo) from comment #24)
> > This might be bug 973064. It would be good if you could verify that, and if
> > that is the case we should reopen that other bug.
> 
> The behavior that I'm seeing is very similar to what Manish was reporting.
> One difference is that I have to close the alert 5 or 6 times before it
> gives up. How would I go about verifying that I'm experiencing the same
> issue?

Personally I would not care how many times it pops-up before it automatically signs in. So if you can replicate it via the following steps I would suggest we reopen the bug.

1. Create a new Firefox profile
2. Setup a proxy which needs authentication
3. Open a URL and enter the proxy authentication details
4. Restart Firefox
5. Open a URL
6. Check if auto-signin works
7. Repeat steps 5 and 6 until it works

Also I would suggest while doing this test you enable HTTP logging via the environment variable. See here for details:
https://developer.mozilla.org/en-US/docs/Mozilla/Debugging/HTTP_logging

> > For now we could add a workaround to the test which switches to the dialog,
> > and accepts it. Check the modal dialog tests under Marionette unit tests in
> > how to do it.
> 
> Yeah, that works for me!

Whereby we would have problems if you really have to do it 5 or 6 times. Maybe lets investigate the above problem first in depth.

> > If we really have to use the same HTTPRealm for storing the login
> > credentials we would have to initiate a GET request to retrieve it. Maybe a
> > HEAD request would also be sufficient. Could you check with eg curl how the
> > proxy responds on both of such requests?
> 
> So I didn't have success sending requests to the proxy server directly, but
> if I send an unauthenticated request using the proxy server as a proxy it
> responds with a Proxy-Authenticate header which contains the httpRealm. For
> example:
> curl -I -x <proxy url> <random url>

Sadly I do not have a proxy with authentication setup locally. Do you know of a public service which offers that? That would make it easier for me to setup something to check myself. For now I found the following which might help us:

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Proxy-Authenticate

Otherwise I don't know if there are some helper APIs available in Firefox which we could use, to get our proxy authentication data added to the Login manager without having to retrieve the realm ourselves. Gijs, do you know something?

Comment 29

[:Gijs (he/him)]

(In reply to Henrik Skupin (:whimboo) from comment #28)
> Otherwise I don't know if there are some helper APIs available in Firefox
> which we could use, to get our proxy authentication data added to the Login
> manager without having to retrieve the realm ourselves. Gijs, do you know
> something?

I expect the best thing to do would be to make a request using 'fetch' or XMLHttpRequest, and read headers. But maybe Matt has a better idea given he knows about login manager stuff.

Comment 30

[Ian MacLeod (:skeletor)]

(In reply to Henrik Skupin (:whimboo) from comment #28)
> Personally I would not care how many times it pops-up before it
> automatically signs in. So if you can replicate it via the following steps I
> would suggest we reopen the bug.
> 
> 1. Create a new Firefox profile
> 2. Setup a proxy which needs authentication
> 3. Open a URL and enter the proxy authentication details
> 4. Restart Firefox
> 5. Open a URL
> 6. Check if auto-signin works
> 7. Repeat steps 5 and 6 until it works

So it seems that with the latest version of Firefox I no longer experience this behavior. I tried downloading an older version (56.0b9) and I do get the same behavior as before. Would it still be useful to collect the HTTP logs for the older version?

I was actually still getting a popup when running the Marionette tests on the latest version, but I just now realized that I should set the "signon.autologin.proxy" setting in geckoinstance.py - not just server.js. Now everything seems to be working as intended except for the httpRealm issue. I will see if I can update the patch to use a head request to get the realm.

> Sadly I do not have a proxy with authentication setup locally. Do you know
> of a public service which offers that? That would make it easier for me to
> setup something to check myself. For now I found the following which might
> help us:
> 
> https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Proxy-Authenticate

There are a lot of proxy services out there, but I didn't see any for public use that include authentication. There are paid/private ones like https://www.myprivateproxy.net, but I have never tried any so I don't really have any basis to recommend one. I created a proxy server using a Digital Ocean droplet for testing that I will leave up if you want to use it. The information to connect to it is in the patch I uploaded.

Comment 31

[Matthew N. [:MattN]]

(In reply to Tim Taubert [:ttaubert] from comment #15)
> (In reply to Henrik Skupin (:whimboo) from comment #14)
> > Tim, do you know if it is possible to run certain xpcshell tests with a
> > profile directory? We would need that for the Marionette tests.
> 
> I don't know much about the implementation of the xpcshell test framework,
> sorry.

do_get_profile() is what you need to use in xpcshell to get it to setup a profile. See https://developer.mozilla.org/en-US/docs/Mozilla/QA/Writing_xpcshell-based_unit_tests#Environment

(In reply to :Gijs from comment #29)
> (In reply to Henrik Skupin (:whimboo) from comment #28)
> > Otherwise I don't know if there are some helper APIs available in Firefox
> > which we could use, to get our proxy authentication data added to the Login
> > manager without having to retrieve the realm ourselves. Gijs, do you know
> > something?
> 
> I expect the best thing to do would be to make a request using 'fetch' or
> XMLHttpRequest, and read headers. But maybe Matt has a better idea given he
> knows about login manager stuff.

Yeah, that's what you'd have to do. There isn't a way to add an HTTP Auth login that matches all realms. If there is a helper to retrieve the realm then it's probably in the networking layer and probably not accessible via JS and wouldn't help with much I think.

Comment 32

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Ian MacLeod (:skeletor) from comment #30)
> So it seems that with the latest version of Firefox I no longer experience
> this behavior. I tried downloading an older version (56.0b9) and I do get
> the same behavior as before. Would it still be useful to collect the HTTP
> logs for the older version?

No, but it would be good to know which bug fixed that. So what do you mean with latest version? Nightly, 59 beta, or 58 release? 
 
> I was actually still getting a popup when running the Marionette tests on
> the latest version, but I just now realized that I should set the
> "signon.autologin.proxy" setting in geckoinstance.py - not just server.js.

We evaluate the value of the pref at runtime when it is needed:

https://dxr.mozilla.org/mozilla-central/rev/ef1fefe4c6d1f95e2bdf872094e02e85c962aa86/toolkit/components/passwordmgr/nsLoginManagerPrompter.js#572

So it's not only once when starting Firefox. I wonder if server.js is actually setting the preferences after your new proxy code . If that is the case we have to get this changed in a different bug, which should block this one.

> Now everything seems to be working as intended except for the httpRealm
> issue. I will see if I can update the patch to use a head request to get the
> realm.

Great. Let me know if you still have problems to get this done. Gijs already gave some great info in how to get this done via `fetch`.

> have any basis to recommend one. I created a proxy server using a Digital
> Ocean droplet for testing that I will leave up if you want to use it. The
> information to connect to it is in the patch I uploaded.

Thanks. For now this is fine, but for landing I have to find a better solution. Maybe just a local mock proxy.

(In reply to Matthew N. [:MattN] (PM if requests are blocking you) from comment #31)
> do_get_profile() is what you need to use in xpcshell to get it to setup a
> profile. See
> https://developer.mozilla.org/en-US/docs/Mozilla/QA/Writing_xpcshell-
> based_unit_tests#Environment

Oh, wonderful! Thanks!

Comment 33

[Ian MacLeod (:skeletor)]

(In reply to Henrik Skupin (:whimboo) from comment #32)
> No, but it would be good to know which bug fixed that. So what do you mean
> with latest version? Nightly, 59 beta, or 58 release? 

The 58 release. I installed a couple previous releases and found that the behavior changed between the 57.0.4 and 58.0 releases. Unfortunately, I've just moved and did not bring this computer with me, so it will be hard to provide more details moving forward.

> We evaluate the value of the pref at runtime when it is needed:
> 
> https://dxr.mozilla.org/mozilla-central/rev/
> ef1fefe4c6d1f95e2bdf872094e02e85c962aa86/toolkit/components/passwordmgr/
> nsLoginManagerPrompter.js#572
> 
> So it's not only once when starting Firefox. I wonder if server.js is
> actually setting the preferences after your new proxy code . If that is the
> case we have to get this changed in a different bug, which should block this
> one.

I'm not too sure of my understanding of this, but it seems that when Firefox is launched by the tests, marionette.prefs.recommended is false, which means that it ignores the settings found here:
https://dxr.mozilla.org/mozilla-central/source/testing/marionette/server.js#60

I think it was just my mistake to not realize that adding the preference in this place alone wouldn't affect the tests, is that not right?

> Great. Let me know if you still have problems to get this done. Gijs already
> gave some great info in how to get this done via `fetch`.

I am still having some issues with this. The response from the proxy server only seems to include the Proxy-Authenticate header when it's a 407. As when I was testing with curl, I can't get the server to respond with a 407 by sending a GET request to the server; it seems that I have to send a CONNECT request instead. But, from what I can tell, it isn't possible to use fetch or XMLHttpRequest to send a CONNECT request (https://fetch.spec.whatwg.org/#forbidden-method).

Is there some reliable way to get a 407 response from a proxy server using a GET or HEAD request?

Comment 34

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Ian MacLeod (:skeletor) from comment #33)
Hi Ian, sorry that I missed to give an update here. In case of questions it would be still good if you could pop into the IRC channel when you find the time.

> (In reply to Henrik Skupin (:whimboo) from comment #32)
> > No, but it would be good to know which bug fixed that. So what do you mean
> > with latest version? Nightly, 59 beta, or 58 release? 
> 
> The 58 release. I installed a couple previous releases and found that the
> behavior changed between the 57.0.4 and 58.0 releases. Unfortunately, I've
> just moved and did not bring this computer with me, so it will be hard to
> provide more details moving forward.

Ok, in that case no problem. If you find time it would be still interesting to know if that was fixed by one of the beta releases of 58, or before its first public beta.

> > We evaluate the value of the pref at runtime when it is needed:
> > 
> > https://dxr.mozilla.org/mozilla-central/rev/
> > ef1fefe4c6d1f95e2bdf872094e02e85c962aa86/toolkit/components/passwordmgr/
> > nsLoginManagerPrompter.js#572
> > 
> > So it's not only once when starting Firefox. I wonder if server.js is
> > actually setting the preferences after your new proxy code . If that is the
> > case we have to get this changed in a different bug, which should block this
> > one.
> 
> I'm not too sure of my understanding of this, but it seems that when Firefox
> is launched by the tests, marionette.prefs.recommended is false, which means
> that it ignores the settings found here:
> https://dxr.mozilla.org/mozilla-central/source/testing/marionette/server.
> js#60

Yes, when you add a preference for Marionette it also has to be done in geckoinstance.py. The recommended prefs are used by geckodriver, which also maybe need an update later.

> I am still having some issues with this. The response from the proxy server
> only seems to include the Proxy-Authenticate header when it's a 407. As when
> I was testing with curl, I can't get the server to respond with a 407 by
> sending a GET request to the server; it seems that I have to send a CONNECT
> request instead. But, from what I can tell, it isn't possible to use fetch
> or XMLHttpRequest to send a CONNECT request
> (https://fetch.spec.whatwg.org/#forbidden-method).
> 
> Is there some reliable way to get a 407 response from a proxy server using a
> GET or HEAD request?

If you can get the pref setting fixed I could have a look at your patch and see if I can figure out how to do it by using the referenced proxy server.

Comment 35

[Ian MacLeod (:skeletor)]

Attachment: Bug 1395886 Add support for proxy authentication

Review commit: https://reviewboard.mozilla.org/r/228858/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/228858/

Comment 36

[Code Review Bot [:reviewbot]]

Comment on attachment 8960076 [details]
Bug 1395886 Add support for proxy authentication

https://reviewboard.mozilla.org/r/228858/#review234526


Code analysis found 11 defects in this patch:
 - 11 defects found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: testing/marionette/session.js:266
(Diff revision 1)
>          }
>        }
>  
> -      if (url.username != "" ||
> -          url.password != "" ||
> -          url.pathname != "/" ||
> +      let login = null;
> +      if (url.username !== "") {
> +        login = Components.classes["@mozilla.org/login-manager/loginInfo;1"]

Error: Use cc rather than components.classes [eslint: mozilla/use-cc-etc]

::: testing/marionette/session.js:269
(Diff revision 1)
> -      if (url.username != "" ||
> -          url.password != "" ||
> -          url.pathname != "/" ||
> +      let login = null;
> +      if (url.username !== "") {
> +        login = Components.classes["@mozilla.org/login-manager/loginInfo;1"]
> +          .createInstance(Ci.nsILoginInfo);
> +        login.init(
> +          `moz-proxy://${url.hostname}:${port}`,

Error: Expected indentation of 12 spaces but found 10. [eslint: indent-legacy]

::: testing/marionette/session.js:270
(Diff revision 1)
> -          url.password != "" ||
> -          url.pathname != "/" ||
> +      if (url.username !== "") {
> +        login = Components.classes["@mozilla.org/login-manager/loginInfo;1"]
> +          .createInstance(Ci.nsILoginInfo);
> +        login.init(
> +          `moz-proxy://${url.hostname}:${port}`,
> +          null,

Error: Expected indentation of 12 spaces but found 10. [eslint: indent-legacy]

::: testing/marionette/session.js:271
(Diff revision 1)
> -          url.pathname != "/" ||
> +        login = Components.classes["@mozilla.org/login-manager/loginInfo;1"]
> +          .createInstance(Ci.nsILoginInfo);
> +        login.init(
> +          `moz-proxy://${url.hostname}:${port}`,
> +          null,
> +          "Squid proxy-caching web server",

Error: Expected indentation of 12 spaces but found 10. [eslint: indent-legacy]

::: testing/marionette/session.js:272
(Diff revision 1)
> +          .createInstance(Ci.nsILoginInfo);
> +        login.init(
> +          `moz-proxy://${url.hostname}:${port}`,
> +          null,
> +          "Squid proxy-caching web server",
> +          url.username,

Error: Expected indentation of 12 spaces but found 10. [eslint: indent-legacy]

::: testing/marionette/session.js:273
(Diff revision 1)
> +        login.init(
> +          `moz-proxy://${url.hostname}:${port}`,
> +          null,
> +          "Squid proxy-caching web server",
> +          url.username,
> +          url.password,

Error: Expected indentation of 12 spaces but found 10. [eslint: indent-legacy]

::: testing/marionette/session.js:274
(Diff revision 1)
> +          `moz-proxy://${url.hostname}:${port}`,
> +          null,
> +          "Squid proxy-caching web server",
> +          url.username,
> +          url.password,
> +          "",

Error: Expected indentation of 12 spaces but found 10. [eslint: indent-legacy]

::: testing/marionette/session.js:275
(Diff revision 1)
> +          null,
> +          "Squid proxy-caching web server",
> +          url.username,
> +          url.password,
> +          "",
> +          "");

Error: Expected indentation of 12 spaces but found 10. [eslint: indent-legacy]

::: testing/marionette/session.js:368
(Diff revision 1)
> -        return `${hostname}:${port}`;
> +        hostname = `${hostname}:${port}`;
> +      }
> +
> +      let logins = Services.logins.findLogins(
> +        {},
> +        `moz-proxy://${hostname}`,

Error: Expected indentation of 10 spaces but found 8. [eslint: indent-legacy]

::: testing/marionette/session.js:369
(Diff revision 1)
> +      }
> +
> +      let logins = Services.logins.findLogins(
> +        {},
> +        `moz-proxy://${hostname}`,
> +        null,

Error: Expected indentation of 10 spaces but found 8. [eslint: indent-legacy]

::: testing/marionette/session.js:370
(Diff revision 1)
> +
> +      let logins = Services.logins.findLogins(
> +        {},
> +        `moz-proxy://${hostname}`,
> +        null,
> +        "Squid proxy-caching web server",

Error: Expected indentation of 10 spaces but found 8. [eslint: indent-legacy]

Comment 37

[Ian MacLeod (:skeletor)]

Comment on attachment 8960076 [details]
Bug 1395886 Add support for proxy authentication

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/228858/diff/1-2/

Comment 38

[David Burns :automatedtester]

:skeletor

Are you still working on this bug?

Comment 39

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 8960076 [details]
Bug 1395886 Add support for proxy authentication

I'm sorry for not having replied on this patch yet. But at the point when it was uploaded we still had problems with how to best check for the authentication. By now we have a new mozbase module called mozproxy (https://firefox-source-docs.mozilla.org/mozbase/mozproxy.html) which allows to handle connections to a proxy server in this case mitmproxy (https://mitmproxy.org/).

mitmproxy can also handle authentication which seems to be what we want here for testing purposes.

Removing review request given that the patch is outdated.

If Ian has no time or interest to continue we will have to come back to this patch at some point in the future.

Comment 40

[Steven Penny]

I need this, and its even worse because with Windows Firefox https_proxy is not recognized, so I have essentially no way to automate setting the proxy - which means I would have to manually set the proxy every time, or at a minimum the proxy credentials

Comment 41

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Steven, what do you mean with https_proxy? There is no such capability for WebDriver. As the example of the initial comment contains it is sslProxy.

Comment 42

[Steven Penny]

yes, I am well aware

however given that sslProxy currently does not support authentication, I was trying to find any solution, only to discover that Firefox ALSO does not recognize https_proxy on Windows, so I have no way to automate setting proxy with authentication

Comment 43

[Steven Penny]

https://github.com/mozilla/gecko-dev/blob/338230dfeb14c047b55228a21b3e49657f00ae83/toolkit/system/unixproxy/nsUnixSystemProxySettings.cpp#L158-L206

Comment 44

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Steven, so proxyType="system" in the session capabilities doesn't work? Alternatively you could also use the auto-proxy settings by specifying a proxyAutoconfigUrl. If both don't work please file another bug given that this one is for authentication only. Thanks.

Comment 45

[Steven Penny]

so proxyType="system" in the session capabilities doesn't work?

no. if you use this https_proxy is not recognized, and Firefox doesnt recognize http_proxy on Windows even ignoring Marionette

Alternatively you could also use the auto-proxy settings by specifying a proxyAutoconfigUrl

no, its well known that PAC files don't support authentication either

If both don't work please file another bug given that this one is for authentication only

https://bugzilla.mozilla.org/show_bug.cgi?id=1944213