Add tests validating that Authorization headers get dropped on cross origin redirections#37145
Conversation
…origin redirections
wpt-pr-bot
requested review from
Manishearth,
annevk,
caitp,
jdm,
mnot,
wisniewskit and
yutakahirano
| }, "getAuthorizationHeaderValue - same origin redirection"); | ||
|
|
||
| promise_test(async (test) => { | ||
| const result = await getAuthorizationHeaderValue(get_host_info().HTTP_REMOTE_ORIGIN + "/fetch/api/resources/redirect.py?allow_headers=Authorization&location=" + encodeURIComponent(get_host_info().HTTP_ORIGIN + "/fetch/api/resources/dump-authorization-header.py")); |
There was a problem hiding this comment.
Thank you for writing this test.
I am implementing fix in Bug 1802086 for Gecko .
I tried these tests to verify my fix. Interestingly, we are not able to validate the test for service workers as we encounter an unhandled reject.
TypeError: NetworkError when attempting to fetch resource
Interestingly, the problem is not only limited to Gecko but also for other browsers . Interestingly, it fails even for Safari which has the fix for this issue. For Safari. the tests are passing for other sub-categories.
Hence, I suspect the issue might be related to the service worker related tests.
I would like to check with others who might be trying these tests.
There was a problem hiding this comment.
@MayyaSunil if you rename this file to authentication-redirection.https.any.js I suspect it'll start passing. Would you be willing to make that change as part of the Gecko fix? That is, I'm assuming the problem is that service workers require a secure context and we're not providing that.
There was a problem hiding this comment.
Hmm, although some subtests are passing. Not entirely sure what's up then. @youennf?
There was a problem hiding this comment.
@MayyaSunil if you rename this file to
authentication-redirection.https.any.jsI suspect it'll start passing. Would you be willing to make that change as part of the Gecko fix? That is, I'm assuming the problem is that service workers require a secure context and we're not providing that.
@annevk you might be right as I say security warnings during the service workers test only. I will try your fix and let you know how it goes.
btw, I dont see these tests in our official rep yet. I am not sure why the moz-wptsync-bot has added the blocker label.
There was a problem hiding this comment.
Renaming the file to authentication-redirection.https.any.js did not help. However, I do not see the security warnings.
There was a problem hiding this comment.
Okay, so the remaining problem is likely that it's using an HTTP_ORIGIN and not an HTTPS_ORIGIN. I suspect there is an HTTPS equivalent but that might be worth double checking in the get-host-info file. Then it won't get blocked due to Mixed Content.
That still doesn't explain it entirely to me, but maybe the service worker test is always running over HTTPS and the rename isn't needed and so we were just running into Mixed Content? (But that wouldn't explain the security warning you saw, but maybe that was something else.)
There was a problem hiding this comment.
Thanks @annevk, after changing HTTP_REMOTE_ORIGIN to HTTPS_REMOTE_ORIGIN and HTTP_ORIGIN to HTTPS_ORIGIN, the issue seems to have fixed. I will submit the fix once these tests are merged into our repo.
| @@ -0,0 +1,28 @@ | |||
| // META: global=window,worker | |||
There was a problem hiding this comment.
I think worker here should be replaced with sharedworked,dedicatedworker as we cant have XHR requests in the context of service workers?
This causes the following error:
PROMISE_REJECT(object "ReferenceError: XMLHttpRequest is not defined")
There was a problem hiding this comment.
Yeah that makes sense. Thanks for debugging!
6 participants
No description provided.