Bug 26510 – Add tests for authentication behavior with cross-origin XMLHttpRequest

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Alexey Proskuryakov]

We need to test how withCredentials attribute affects XMLHttpRequest.

[Alexey Proskuryakov]

Created attachment 31499 [details]
proposed patch

[Maciej Stachowiak]

Comment on attachment 31499 [details]
proposed patch

r=me

[David Levin]

It looks like mjs gave an r+ while i was looking at this so I'll just send along my comments as an fyi.

> Index: LayoutTests/http/tests/xmlhttprequest/cross-origin-authorization-expected.txt
> ===================================================================
> +Start
> +Trying different ways to access a password protected resource from another origin. The UA already has login and password for this protection space.

It would be nice to indicate the expected result here for anyone running it standalone.  Something like "You should see a several PASSes followed by a DONE."



> Index: LayoutTests/http/tests/xmlhttprequest/cross-origin-authorization.html
> ===================================================================
> +function test()
> +{
> +    log("Trying different ways to access a password protected resource from another origin. The UA already has login and password for this protection space.\n")
> +    log("SCRIPT SRC='...' Should succeed, since authorization is sent for cross-origin subresource loads.");
> +    var scriptElement = document.createElement("script");
> +    scriptElement.setAttribute("src", "http://localhost:8000/xmlhttprequest/resources/cross-origin-authorization.php");
> +    scriptElement.setAttribute("onload", "test_sync_auth_stored()");
> +    scriptElement.setAttribute("onerror", "test_sync_auth_stored()");

Why does it run the function for onload or onerror?


> +    document.body.appendChild(scriptElement);
> +}
> +
> +function test_sync_auth_stored()

testSyncAuthStored seems like it is more consistent with function names (even in js).
Same thing for the other functions in here.


> +
> +function test_async_cookies()
> +{
> +    log("Cross-origin XMLHttpRequest (async), testing cookies.");
> +
> +    var req = new XMLHttpRequest;
> +    req.open("GET", "http://localhost:8000/xmlhttprequest/resources/cross-origin-check-cookies.php", true);
> +    req.withCredentials = true;
> +    req.send();
> +    req.onload = function() {
> +	    log(req.responseText.match(/WK\-cross\-origin/) ? "PASS" : "FAIL");

There is a TAB here.


> Index: LayoutTests/http/tests/xmlhttprequest/cross-origin-no-authorization.html
> +function test_async_cookies()
> +{
> +    log("Cross-origin XMLHttpRequest (async), testing cookies.");
> +
> +    var req = new XMLHttpRequest;
> +    req.open("GET", "http://localhost:8000/xmlhttprequest/resources/cross-origin-check-cookies.php", true);
> +    req.send();
> +    req.onload = function() {
> +	    log(req.responseText.match(/WK\-cross\-origin/) ? "FAIL" : "PASS");
There is a TAB here.


> Index: LayoutTests/http/tests/xmlhttprequest/resources/cross-origin-authorization.php
> ===================================================================
> +<?php
> +header("Set-Cookie: WK-cross-origin=1");

It seems like it would be nice to only set the cookie after the authorization was checked (after "if (!isset($_SERVER['PHP_AUTH_USER']))").


> +if (!isset($_SERVER['PHP_AUTH_USER'])) {
Why not check the actual user name and password to verify that they were sent correctly?


> Index: LayoutTests/http/tests/xmlhttprequest/resources/cross-origin-check-cookies.php
> ===================================================================
> +if (isset($_COOKIE['WK-cross-origin'])) {
> +    echo 'WK-cross-origin: ' . $_COOKIE['WK-cross-origin'];
> +} else {
> +	echo 'Cross-origin cookie was not sent.';
TAB.


> Index: LayoutTests/http/tests/xmlhttprequest/resources/cross-origin-no-authorization.php
> ===================================================================

Same comments as above for the similar code.

[Alexey Proskuryakov]

Committed revision 44816.

Note that Firefox fails some of the tests - it doesn't send cookies even when withCredentials is true, and raises an exception when trying to specify cross-site credentials explicitly (which we silently ignore).

(In reply to comment #3)
> It would be nice to indicate the expected result here for anyone running it
> standalone.  Something like "You should see a several PASSes followed by a
> DONE."

Done.

> Why does it run the function for onload or onerror?

It's going to the next test regardless of whether the current one passed.

> testSyncAuthStored seems like it is more consistent with function names (even
> in js).
> Same thing for the other functions in here.

I like to be inconsistent in tests - what if someone accidentally modifies JS parser in a way that it no longer supports underscores?

> It seems like it would be nice to only set the cookie after the authorization
> was checked (after "if (!isset($_SERVER['PHP_AUTH_USER']))").

I'm not sure. The current code kind sends a signal that the cookie may be set before you expect it to - and that's the case because several tests set it anyway.

> > +if (!isset($_SERVER['PHP_AUTH_USER'])) {
> Why not check the actual user name and password to verify that they were sent
> correctly?

There are several user names used in the tests.

Thank you for your comments!