425078 - not showing authentication dialog box when request is made throught XMLHttpRequest in chrome window

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P2)

Description

[Adam Rambousek]

User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.8.1.13pre) Gecko/20080210 SeaMonkey/1.1.8pre
Build Identifier: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.8.1.13pre) Gecko/20080210 SeaMonkey/1.1.8pre

continuation of #422135
if the testcase http://nebosuker.net/ff3-ajax-auth/ is opened in chrome window, authentication dialog is not shown and status 401 is returned immediatelly

I'm attaching test extensions - it adds two entries to Tools menu: "Auth test - chrome" opens testcase in window with "chrome" parameter, "Auth test - normal" opens testcase in window without "chrome" parameter

In chrome window, clicking on buttons nor link, won't display authentication dialog. In normal window, authentication dialog is displayed after clicking on buttons or link.

Reproducible: Always

Steps to Reproduce:
open chrome window, try to get a password-protected (basic/digest) file through XmlHttpRequest
Actual Results:  
request fails automatically if you have not succeeded in authentication in the
session before

Expected Results:  
authentication dialog box (to enter username and password manually) is showed 
if you have not succeeded in authentication in the session before

Comment 1

[Adam Rambousek]

Attachment: test extension

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Do you have a regression range? Was this caused by the changes in bug 422135/bug 422808, or some earlier change?

Comment 3

[Adam Rambousek]

it was caused by some earlier change, I suppose. It didn't work before bug 422135, but worked in FF2. bug 422135 only fixed this for non-chrome windows.

Comment 4

[:Gavin Sharp [email: [e-mail]]]

Can you find a 1-day regression range, using -trunk builds from ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/ ?

Comment 5

[Adam Rambousek]

ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2008/03/2008-03-02-04-trunk/firefox-3.0b4pre.en-US.linux-i686.tar.bz2 is OK

ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2008/03/2008-03-03-04-trunk/firefox-3.0b4pre.en-US.linux-i686.tar.bz2 is broken

Comment 6

[:Gavin Sharp [email: [e-mail]]]

Hmm, that points to bug 382437. I can see how that would have caused that back then, but it should have since been fixed by bug 422808.

Can you confirm that it doesn't work in the latest nightly? Your comment 0 doesn't say which build you tested this in.

ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-trunk/

Comment 7

[Adam Rambousek]

it still does not work in latest NB
Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9b5pre) Gecko/2008032504 Minefield/3.0b5pre

Comment 8

[:Gavin Sharp [email: [e-mail]]]

A simplified testcase is:
var x=new XMLHttpRequest
x.open('GET', "http://gavinsharp.com/secret/", true)
x.send(null);

run from a chrome context (where http://gavinsharp.com/secret/ is any site that requires authentication).

This was originally broken by bug 382437, which made NS_NewAuthPrompter return a (broken, at the time) nsIAuthPrompt implemented by login manager. That nsIAuthPrompt implementation was fixed in bug 422135, which would have caused this to work again - but then we also fixed bug 422808 at about the same time, which caused this bug.

This bug exists because chrome XMLHttpRequest prompting previously relied on finding the nsIAuthPrompt implementation on the XMLHttpRequest object, which we removed in bug 422808. The fallback to the docshell's prompts doesn't work for XHR from chrome docshells because nsDocShell::GetAuthPrompt doesn't return an auth prompt for chrome docshells (i.e. when mAllowAuth is false).

It looks like the "don't return an auth prompt for chrome docshells" behavior was originally meant to prevent auth prompts from being displayed for email messages in mailnews (bug 51631). I guess XMLHttpRequest behavior was not relevant to that bug, because mail can't run script by default.

I'm not sure how we should fix this. I suppose we could just back out bug 422808, but that would regress some of the bugs those changes fixed. Another option would be to introduce some way to indicate that the docshell should return an nsIAuthPrompt despite the fact that it's of type "chrome", and have XHR use that. I suppose we could also just not fix this, and require chrome users of XHR to provide their own nsIAuthPrompt (e.g. by forwarding calls to the login manager impl), but that doesn't sound optimal.

sicking, thoughts?

Comment 9

[Justin Dolske [:Dolske]]

Ugh. None of the choices here seem nice... Chrome callers shouldn't have to jump through hoops to get authentication working, unless they actually should (bug 51631). Damned if you do, damned if you don't. :/

It would be preferable if XHR could set some "no, really, give me a prompt" flag/arg to override the "no prompts for chrome" docshell behavior... But there's enough indirection here that such action-at-a-distance might not be easy.

Comment 10

[Damon Sicore (:damons)]

Fixing Priority.  P2.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Couldn't we change the patch for bug 422808 to try the docshell, and if that fails return a prompt ourselves?

That said, what bugs did the change in bug 422808 fix?  I don't see any marked as deps there...

Comment 13

[Boris Zbarsky [:bzbarsky]]

Ah, ok.  That still seems like it should work if we try to get the prompt from the docshell first and only fall back to our own prompt if that fails.

Comment 14

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #11)
> That said, what bugs did the change in bug 422808 fix?  I don't see any marked
> as deps there...

See bug 422808 comment 2 and bug 422808 comment 24.

Comment 15

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Honestly, it seems to me like the problem here is that chrome docshells won't return an auth prompt. It makes sense that mailnews won't do that, but I'm not sure why firefox wouldn't.

Comment 16

[Boris Zbarsky [:bzbarsky]]

I'd be fine with returning auth prompts from all chrome docshells.  For mailnews, the relevant docshell is the content one anyway, isn't it?  So I'm not actually sure how the chrome docshell behavior affects mailnews...

I do think that XMLHttpRequest should behave identically in Firefox chrome and Thunderbird chrome.

Comment 17

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: Patch to fix

So this fixes it. I felt a bit nervous about turning on auth for all chrome requests this late in the game, there might be firefox issues with doing this too for all we know.

So instead this makes XHR manually grab the docshell and use the nsIAuthPromptProvider interface to get an nsIAuthPrompt. The neat thing about this is that nsIAuthPromptProvider has an aPromptReason argument that when set to PROMPT_PROXY makes us ignore the allowAuth flag.

But of course it is ugly to use that since we're not really doing a proxy authentication.

An alternative fix would be to grab the docshell. Get the current allowAuth value. Set allowAuth to TRUE. Use nsIAuthPromptProvider to get the nsIAuthPrompt. Reset the allowAuth value.

I don't really care much either way opinions welcome.

Comment 18

[Johnny Stenback (:jst)]

Comment on attachment 313742 [details] [diff] [review]
Patch to fix

I think now that I see this code I'd prefer to temporarily enable auth prompts on the docshell while we're requesting it from XHR, but I can't say that I feel that strongly about it. r+sr=jst either way.

Comment 19

[Christian :Biesinger (don't email me, ping me on IRC)]

fwiw I believe another issue was favicons - you probably don't want an auth prompt just for the favicon (which is loaded from chrome).

Comment 20

[Christian :Biesinger (don't email me, ping me on IRC)]

Wouldn't another option be to just call windowwatcher's GetPrompt with the right window argument?

Comment 21

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: Patch v2

That does indeed seem like the simplest solution.

Comment 22

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Fix checked in.

Comment 23

[Christian :Biesinger (don't email me, ping me on IRC)]

should this be done for nsXMLDocument as well?

Comment 24

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Well.. we have to make up our minds on if we want loads from chrome to create auth dialogs or not. I assume we have a general policy of not showing auth for chrome docshells for a reason. We made a spot-exception for XHR basically saying that that is the way to load external data. I guess we could add more exceptions, but I think there should be a reason for them.

Also, I personally think that document.load should have never been added. XHR is the way to go.