1308817 - HTTP basic auth goes directly to 401 error page when foxyproxy is enabled

Status: RESOLVED FIXED

(Firefox :: Extension Compatibility, defect)

Description

[dx]

This is a regression introduced on 2016-09-01 between af5df7bd and d3bf9bbe, with the fix for this bug:

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

Steps to reproduce:

1. Install foxyproxy standard https://addons.mozilla.org/en-US/firefox/addon/foxyproxy-standard/
2. Ensure it's enabled (toolbar icon not crossed out, set to "Use proxies based on their pre-defined patterns and priorities")
3. Go to a page requiring http basic auth, such as http://test.webdav.org/auth-basic/

Expected results:

- Get asked for user/pass

Actual results:

- Get 401 error page from the server

Other notes:

- Doesn't happen in non-e10s windows.
- One workaround I've used (before finding out that it was foxyproxy) is to pass the credentials in the url bar, in the format http://user:[e-mail]

mozregression logs:

44:38.56 INFO: Last good revision: af5df7bd1d265bb95c76b69fa4fbcc60a84094f9
44:38.56 INFO: First bad revision: d3bf9bbe73ba35c4257dcd10339f6ccecbb1fd5e
44:38.56 INFO: Pushlog:
https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=af5df7bd1d265bb95c76b69fa4fbcc60a84094f9&tochange=d3bf9bbe73ba35c4257dcd10339f6ccecbb1fd5e

44:40.57 INFO: Looks like the following bug has the changes which introduced the regression:
https://bugzilla.mozilla.org/show_bug.cgi?id=1266836

Comment 1

[Noah (NoahSUMO/SolidSnake) [:Noah]]

CC'ing Eric to this bug as he might have some insight to offer. :)

Comment 2

[Matthew N. [:MattN]]

Hi dx, can you confirm that the problem still happens on the latest Nightly (with and without e10s)? There was a followup bug (bug 1301109) which may have fixed this.

Comment 3

[dx]

Yes, still happens with nightly buildid 20161010030204

Comment 4

[Gerry Chang [:gchang]]

Track 51+ as regression.

Comment 5

[Marcia Knous [:marcia]]

Makes sense to track this for 52+ as well. Will need an owner for the bug.

Comment 6

[PwrSurge]

Under browser console, the following error is displayed when trying to load an HTTP basic auth site:

TypeError: prompt.setE10sData is not a function[Learn More]  authPromptProvider.jsm:267:11

authPromptProvider.jsm is from FoxyProxy which calls setE10sData but this function no longer exists as per the changes made https://bugzilla.mozilla.org/show_bug.cgi?id=1266836

- void setE10sData(in nsIDOMElement aBrowser, in nsIDOMWindow aOpener);
+ void setOpener(in nsIDOMWindow aOpener);

Migrating all setE10sData calls to setOpener should resolve this issue.  However, i'm unsure why FoxyProxy even needs to do this in the first place.

Comment 7

[Liz Henry (:lizzard) (relman/hg->git project)]

MattN, can you take this on or help us find an owner? Thanks.

Comment 8

[Gerry Chang [:gchang]]

Hi dolske,
could you help find an owner for this if MattN is busy working on other bugs?

Comment 9

[Justin Dolske [:Dolske]]

Password manager is Panos' thing now, so he should make any resourcing decision here.

That said, comment 6 makes this sounds like a straight-up addon-compat issue... (Thanks for the informative comment!) We changed/renamed some internal APIs, and so now FoxyProxy should be fixed to use the new interfaces. This is, unfortunately, the kind of problem that has been an issue with Firefox since the beginning, and is exactly what WebExtensions is meant to address (by giving addons stable APIs, instead of the raw gory details of Firefox internals).

In some cases we've implemented backwards-compatibility shims, but usually only when API usage is widespread and it's not practical to get addons updated in a timely manner. I think it's technically possible to do so here, but it would be more expedient for FoxyProxy to update.

Comment 10

[Gerry Chang [:gchang]]

Mark 51 fix-optional as the internal API is changed. FoxyProxy should fix this. If we have other approach to fix the backwards-compatibility, I'm still happy to take this in 51 early beta.

Comment 11

[Panos Astithas (he/him) [:past] (please ni?)]

I agree with Justin that this is something better handled in the add-on code. From a quick glance at their website it appears to be actively maintained, so I would expect something as simple as the change in comment 6 (thanks!) to be fairly straightforward for them. Has this issue been raised in their support tracker? I see that their code is open source, so even the change described in comment 6 could be submitted to them as a patch for consideration.

Comment 12

[Andy McKay]

One of the authors, Eric Jung is active on Bugzilla and has already been need info'd on this bug.

Comment 13

[Eric Jung [:ericjung]]

Thanks for the work!

Comment 14

[Eric Jung [:ericjung]]

Can anyone tell me when this code will be shipped in a final release (not nightly or beta)?

Comment 15

[Jorge Villalobos [:jorgev] (he/him)]

(In reply to Eric H. Jung from comment #14)
> Can anyone tell me when this code will be shipped in a final release (not
> nightly or beta)?

Bug 1266836 will ship in Firefox 51, to be released on January 24th (https://wiki.mozilla.org/RapidRelease/Calendar).

Comment 16

[Eric Jung [:ericjung]]

(In reply to PwrSurge from comment #6)
> Under browser console, the following error is displayed when trying to load
> an HTTP basic auth site:
> 
> TypeError: prompt.setE10sData is not a function[Learn More] 
> authPromptProvider.jsm:267:11
> 
> authPromptProvider.jsm is from FoxyProxy which calls setE10sData but this
> function no longer exists as per the changes made
> https://bugzilla.mozilla.org/show_bug.cgi?id=1266836
> 
> - void setE10sData(in nsIDOMElement aBrowser, in nsIDOMWindow aOpener);
> + void setOpener(in nsIDOMWindow aOpener);
> 
> Migrating all setE10sData calls to setOpener should resolve this issue. 
> However, i'm unsure why FoxyProxy even needs to do this in the first place.

This does not fix the problem:

  var prompt = factory.getPrompt(win, Components.interfaces.nsIAuthPrompt2);
  if(prompt instanceof Components.interfaces.nsILoginManagerPrompter) {
    // https://bugzilla.mozilla.org/show_bug.cgi?id=1308817
    prompt.setOpener(win);
  }

yields the error:

  JavaScript error: resource://foxyproxy/authPromptProvider.jsm, line 268: TypeError: prompt.setOpener is not a function

when visiting:

https://httpbin.org/basic-auth/someuser/somepassword

Comment 17

[Eric Jung [:ericjung]]

I have no idea if this needs to be done. The code seems to work without calling either of these:

if(prompt instanceof Components.interfaces.nsILoginManagerPrompter) {
  // https://bugzilla.mozilla.org/show_bug.cgi?id=1308817
  // https://dxr.mozilla.org/mozilla-central/source/toolkit/components/passwordmgr/nsLoginManagerPrompter.js
  prompt.init(win);
  prompt.opener = win;
}

Is there anyone who is familiar enough with nsILoginManagerPrompter to advise?

Comment 18

[Matthew N. [:MattN]]

(In reply to Eric H. Jung from comment #16)
>   var prompt = factory.getPrompt(win, Components.interfaces.nsIAuthPrompt2);
>   if(prompt instanceof Components.interfaces.nsILoginManagerPrompter) {
>     // https://bugzilla.mozilla.org/show_bug.cgi?id=1308817
>     prompt.setOpener(win);
>   }
> 
> yields the error:
> 
>   JavaScript error: resource://foxyproxy/authPromptProvider.jsm, line 268:
> TypeError: prompt.setOpener is not a function

Right, it's an attribute, not a method as you figured out in comment 17.

(In reply to Eric H. Jung from comment #17)
> I have no idea if this needs to be done. The code seems to work without
> calling either of these:
> 
> if(prompt instanceof Components.interfaces.nsILoginManagerPrompter) {
>   // https://bugzilla.mozilla.org/show_bug.cgi?id=1308817
>   //
> https://dxr.mozilla.org/mozilla-central/source/toolkit/components/
> passwordmgr/nsLoginManagerPrompter.js
>   prompt.init(win);
>   prompt.opener = win;
> }
> 
> Is there anyone who is familiar enough with nsILoginManagerPrompter to
> advise?

Did you read the comment at https://dxr.mozilla.org/mozilla-central/rev/8ff550409e1d1f8b54f6f7f115545dbef857be0b/toolkit/components/passwordmgr/nsILoginManagerPrompter.idl#39-45 ?

Comment 19

[Matthew N. [:MattN]]

You were previously doing 
> prompt.setE10sData(win.gBrowser.selectedBrowser, null);
so to retain previous behaviour
> prompt.init(win);
is sufficient (leave opener null). I'd have to look more to understand if it's actually correct for all cases though.

Comment 20

[Matthew N. [:MattN]]

But you also need to be doing:
> prompt.browser = win.gBrowser.selectedBrowser;

(assuming that's actually the correct browser which I kinda doubt but matches your old behaviour)

Comment 21

[Matthew N. [:MattN]]

Comment 20 is assuming you're in the parent process. Otherwise it's not necessary.

Comment 22

[Mossroy]

I'm still having this issue with Firefox 50.1.0 (on Ubuntu 16.04) and FoxyProxy Standard 4.6.3
Disabling FoxyProxy is a workaround.
Downgrading FoxyProxy Standard to 4.5.7 or 4.5.6 is also a workaround

Comment 23

[Mossroy]

After upgrading to Firefox 51, the problem seems to be gone with FoxyProxy Standard 4.6.3.
Does that mean that the latest versions of FoxyProxy are only compatible with Firefox>=51?
If it's the case, they should be marked as such in install.rdf, so that users of older Firefox versions are not upgraded automatically, and keep a compatible version.
Keep in mind that not everybody want/can upgrade to the latest version of Firefox. For example the users of Firefox ESR version (which is still a 45.x for now)

Comment 24

[Eric Jung [:ericjung]]

(In reply to Mossroy from comment #22)
> I'm still having this issue with Firefox 50.1.0 (on Ubuntu 16.04) and
> FoxyProxy Standard 4.6.3
> Disabling FoxyProxy is a workaround.
> Downgrading FoxyProxy Standard to 4.5.7 or 4.5.6 is also a workaround

Please use FoxyProxy Standard 4.6.5.