Closed Bug 425078 Opened 17 years ago Closed 17 years ago

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

Categories

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

defect

Tracking

()

RESOLVED FIXED

People

(Reporter: rambousek, Assigned: sicking)

References

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

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
Attached file test extension (deleted) —
Flags: blocking-firefox3?
Keywords: regression
Version: unspecified → Trunk
Do you have a regression range? Was this caused by the changes in bug 422135/bug 422808, or some earlier change?
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.
Can you find a 1-day regression range, using -trunk builds from ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/ ?
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/
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
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?
Status: UNCONFIRMED → NEW
Component: Password Manager → DOM
Ever confirmed: true
Flags: blocking-firefox3?
Product: Firefox → Core
QA Contact: password.manager → general
Blocks: 422808
Flags: blocking1.9?
Hardware: PC → All
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.
Assignee: nobody → jonas
Flags: blocking1.9? → blocking1.9+
Fixing Priority. P2.
Priority: -- → P2
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...
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.
(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.
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.
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.
Attached patch Patch to fix (obsolete) (deleted) — Splinter Review
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.
Attachment #313742 - Flags: superreview?(jst)
Attachment #313742 - Flags: review?(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.
Attachment #313742 - Flags: superreview?(jst)
Attachment #313742 - Flags: superreview+
Attachment #313742 - Flags: review?(jst)
Attachment #313742 - Flags: review+
fwiw I believe another issue was favicons - you probably don't want an auth prompt just for the favicon (which is loaded from chrome).
Wouldn't another option be to just call windowwatcher's GetPrompt with the right window argument?
Attached patch Patch v2 (deleted) — Splinter Review
That does indeed seem like the simplest solution.
Attachment #313742 - Attachment is obsolete: true
Attachment #314302 - Flags: superreview?(jst)
Attachment #314302 - Flags: review?(jst)
Attachment #314302 - Attachment is patch: true
Attachment #314302 - Attachment mime type: application/octet-stream → text/plain
Attachment #314302 - Flags: superreview?(jst)
Attachment #314302 - Flags: superreview+
Attachment #314302 - Flags: review?(jst)
Attachment #314302 - Flags: review+
Fix checked in.
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
should this be done for nsXMLDocument as well?
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.
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: