Closed Bug 282547 Opened 15 years ago Closed 7 years ago

[XHR] Login dialog with wrong XMLHttpRequest authentication

Categories

(Core :: XML, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla18
Tracking Status
firefox18 --- fixed

People

(Reporter: prokopiev, Assigned: baku)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

Attachments

(1 file, 4 obsolete files)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; ru-RU; rv:1.7.2) Gecko/20040808
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; ru-RU; rv:1.7.2) Gecko/20040808

I get standart login dialog instead of error in JavaScript if I use not existing
user or password in XMLHttpRequest

Reproducible: Always

Steps to Reproduce:
var endPoint = "http://localhost:8080/wsapp/services/SecureHelloWorldService";   
var httpRequest = new XMLHttpRequest();   
httpRequest.open('GET', endPoint, false, 'webdeveloper', 'webpassword');
httpRequest.send(null);
Actual Results:  
Standart login dialog

Expected Results:  
Exception in JavaScript
over to the XML component
Component: Networking: HTTP → XML
Summary: Login dialog with wrong XMLHttpRequest authentication → Login dialog with wrong XMLHttpRequest authentication
switch to default owner
Assignee: darin → xml
QA Contact: networking.http → ashshbhatt
Confirmed. Here we have behavior that is different from IE. IE only asks users
about login data if the script doesn't specify it. If the script specifies
username and password and 401 comes back, the send() call just returns without
success (no JavaScript exception). I think we should do it the same way.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Linux → All
Hardware: PC → All
Bug 221943 might be related.
darin, how would we be able to do this?  Do we have to do our own auth prompt
impl and set it on the channel before we open it if we have a username/password set?
bz, it seems XMLHttpRequest does that already... maybe it's enough to fail the
request for nsIAuthPrompt in its getInterface implementation; but the loadgroup
may have it, so maybe it should return an authprompt that will just return
failure when asked for a prompt.
Yeah, that last seems to be the simplest, if http deals well with it.

Doron, got time to do that?
or maybe nsIAuthPromptProvider is better (fewer functions)
there's also bug 277322 which should probably not be regressed...
Returning failure when asked for a prompt is no good.  HTTP will try the
loadgroup if it can't get a prompt from the channel (even if the channel has an
nsIAuthPromptProvider).

I suppose we could impl a shim nsIAuthPrompt that would just return errors for
everything...
Darin, will the suggestion in comment 10 work?
would be any progress here?
Sorry about the spam, wrong tab :-(
Does the spec say anything here?
Assignee: xml → nobody
QA Contact: ashshbhatt → xml
Duplicate of this bug: 520148
(In reply to comment #15)
> Does the spec say anything here?

Just in case, according to the current W3C working draft, a user agent must simply return the status 401 in this case without prompting the end user for a username and a password (as mentioned in bug 520148 comment #0).

http://www.w3.org/TR/2009/WD-XMLHttpRequest-20090820/#the-send-method
> [...]  If authentication fails, and request username and request password are
> provided, user agents must not prompt the end user for credentials.  [...]
> Note: End users are not prompted if credentials are provided through the
> open() API so that authors can implement their own user interface.
Summary: Login dialog with wrong XMLHttpRequest authentication → [XHR] Login dialog with wrong XMLHttpRequest authentication
Duplicate of this bug: 447935
I was wondering what the status of this bug is. It is marked as new but appears not so new.

- The same problem occurs in Firefox 4.0b9.

- The relevant text, noted in comment #17, has not changed in the current W3C candidate XMLHttpRequest document and is included in the W3C XMLHttpRequest2 working draft.

Note: Chrome 8.0.552.237 appears to have the same behaviour as Firefox.
Mark, "new" just means it's confirmed but no one is working on it yet.
Thanks. I understand. It is a minor issue.
It is just an annoying issue because a server side script is required to hide any authorisation errors from XHR.
Is there any news on thisone? I understand it's a minor issue but it's been like this for over 6 years now (the bug was opened in 2005). I do not have access to the serverside of the API I am using but would like to hide the authentication window when a user enters wrong credentials on my addon.
I had the same problem. I happened to be using JSONP. I worked around it by using a web worker and assuming a timeout means there's a hanging password prompt somewhere. Here's my code, paraphrased in CoffeeScript for brevity:

Worker:

this.onmessage = (event) =>
  request_id = event.data.request_id
  callback_name = "callback_#{request_id}"
  # "this" here is the global scope, because we used "=>" above
  this[callback_name] = (data) =>
    delete this[callback_name]
    this.postMessage({ request_id: request_id, data: data })

  username = encodeURIComponent(event.data.username)
  password = encodeURIComponent(event.data.password)
  importScripts("https://#{username}:#{password}@example.org/path/to/api/endpoint.json?callback=#{encodeURIComponent(callback_name)}")
  # For XMLHttpRequest, send "username" and "password" as normal

And in the main script, where you'd normally do a JSONP request:

worker_request_id = 0
worker_callbacks = {}
worker = new Worker("login-worker.js")

worker.onmessage = (event) ->
  request_id = event.data.request_id
  if worker_callbacks[request_id]?
    callback = worker_messages[request_id]
    delete worker_messages[request_id]
    callback(event.data.data)

prompt_for_credentials = (callback) ->
  ... some DOM code, with, for instance, a <form> submit calling callback.apply({})

until_credentials_valid = (callback) ->
  recurse = () -> prompt_for_credentials -> until_credentials_valid(callback)

  username = # get username
  password = # get password

  request_id = "#{worker_request_id += 1}"
  worker_callbacks[request_id] = callback
  worker.postMessage({ request_id: request_id, username: username, password: password })
  window.setTimeout(->
    if worker_callbacks[request_id]?
      # Request failed; prompt for password again
      delete worker_callbacks[request_id]
      recurse()
  , 20000) # 20s timeout

(Please excuse typos.)
I think this is now necessary for the Gaia email app, since we do form-based HTTP Auth, and B2G/Gaia now supports the HTTP auth dialog, meaning we get an unnecessary popup when typing in the wrong password for our email.

I believe the corresponding bug that added the auth dialog to B2G is: https://github.com/mozilla-b2g/gaia/issues/4732
blocking-basecamp: --- → ?
Josh: You have been all over XHR issues lately. Could you take this one?
Assignee: nobody → josh
Sorry to be changing flags back and forth, but I found a series of magical incantations that satisfies my needs for the Gaia email app. Specifically, if I manually add the Authorization header *and* set mozAnon: true in the XHR constructor, things work as expected. This would still be nice-to-have, but I think it would only directly benefit me if bug 776171 were also fixed.
blocking-basecamp: ? → ---
Actually, Andrea might be able to have a look at this since it seems similar to bug 761479.

Andrea: Sounds like this isn't a high priority bug right now, but if you have extra cycles, feel free to have a look.
Assignee: josh → amarchesini
Right. I don't *need* this for basecamp, but if we want the Gaia email app to be a proper "open web app" instead of an "open web app that requires some Firefox-specific extensions", we'll need to get this fixed eventually.
Attached patch patch 1 (obsolete) — Splinter Review
Attachment #667456 - Flags: review?(bzbarsky)
Comment on attachment 667456 [details] [diff] [review]
patch 1

What is the goal this patch is aiming to achieve in terms of desired behavior?

It would be good to put that into the checkin comment, in fact.
With this patch, XMLHttpRequest doesn't show any prompt asking for username/password.
I don't know if this is what we want or we prefer: if (URL has Credentials) { no_prompt }
Well, what does the spec say?
If authentication fails, XMLHttpRequest origin and the request URL are same origin, Authorization is not in the list of author request headers, request username is null, and request password is null, user agents should prompt the end user for their username and password.

Otherwise, if authentication fails, user agents must not prompt the end user for their username and password.
Attached patch patch 2 (obsolete) — Splinter Review
This new patch uses a dummy auth prompt object only if the URI contains username+password.
Attachment #667456 - Attachment is obsolete: true
Attachment #667456 - Flags: review?(bzbarsky)
Attachment #668028 - Flags: review?(bzbarsky)
Comment on attachment 668028 [details] [diff] [review]
patch 2

Please remove the else-after-return and outdent that else clause.

And shouldn't we be checking at least for the Authorization header?  And possibly for the same-origin thing....
Attached patch patch 2b (obsolete) — Splinter Review
I think the check for the Authorization header and some-origin should be done by the nsHttpChannel. If we have this issue, we should file a new bug.
Attachment #668028 - Attachment is obsolete: true
Attachment #668028 - Flags: review?(bzbarsky)
Attachment #668086 - Flags: review?(bzbarsky)
I don't think it makes any sense for nsHttpChannel to do any sort of origin checks.

I'm also not convinced it should be checking for Authorization, honestly, but I could maybe be argued into that one.  But any sort of same-origin checking needs to be in the XHR code.
Attached patch patch 3 (obsolete) — Splinter Review
3 checks are implemented by this patch:

1. CheckMayLoad
2. authorization
3. username + password in the URI
Attachment #668086 - Attachment is obsolete: true
Attachment #668086 - Flags: review?(bzbarsky)
Attachment #668458 - Flags: review?(bzbarsky)
Comment on attachment 668458 [details] [diff] [review]
patch 3

I'm not sure what the "on cascade" comment means.  Perhaps:

  // Verify that it's ok to prompt for credentials here, per spec

and maybe even with a spec pointer (link or section number).

Can we just use the XML_HTTP_REQUEST_USE_XSITE_AC for the same-origin bit?  Seems like we should be able to.  If we can't, please add a comment explaining why not.

s/LowerCaseEqualsASCII/LowerCaseEqualsLiteral/

We want to set showPrompt to false if either username or password is null.  So we want "||" there, not "&&".

r=me with those nits.  Thank you for sticking with this!
Attachment #668458 - Flags: review?(bzbarsky) → review+
In particular, I think for system XHR CheckMayLoad() would prevent prompts when we might want them here...
Attached patch patch 3bSplinter Review
Attachment #668458 - Attachment is obsolete: true
Attachment #668820 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/1c1cd655dfe5
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
Depends on: 799540
Comment on attachment 668820 [details] [diff] [review]
patch 3b

[Approval Request Comment]
This patch must be landed with 799540.
Attachment #668820 - Flags: approval-mozilla-aurora?
Attachment #668820 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.