1855149 - Add authChallenges to network.responseStarted/responseCompleted

Status: RESOLVED FIXED

(Remote Protocol :: WebDriver BiDi, task, P2)

Description

[Julian Descottes [:jdescottes]]

For network interception, we now expect the auth challenges to be included in the response data.

This should be added in all events including response data: network.responseStarted, network.responseCompleted, network.authRequired.

https://w3c.github.io/webdriver-bidi/#extract-challenges

Comment 1

[Julian Descottes [:jdescottes]]

Hi Valentin,

I was looking at the parsing of challenges headers (WWW-Authenticate/Proxy-Authenticate). It's not completely trivial, because we can get multiple challenges, separated by a comma, but we can also have commas inside a challenge.

I think this is handled on the platform side at https://searchfox.org/mozilla-central/source/netwerk/protocol/http/nsHttpChannelAuthProvider.cpp#482 and I was wondering if we should try to expose this to JS somehow or if I should duplicate it?

Initially thought that nsIHttpAuthenticableChannel::WWWChallenges would be the parsed value, but I think it's just the raw header value? (and I couldn't QueryInterface any channel to nsIHttpAuthenticableChannel for some reason). Maybe this WWWChallenges getter at least concatenates all WWW-Authenticate headers in case there are more than 1?

Comment 2

[Julian Descottes [:jdescottes]]

Attachment: Bug 1855149 - [bidi] Add a helper to parse challenge headers

Parsing challenge headers is not trivial, given that there can be multiple challenges in a single header.
Implementing this properly most likely means implementing a proper tokenizer.
It would be better to reuse the implementation already used by necko but this is not exposed to JS yet.

For now, add a simple parser with xpcshell tests to be reused in the network module of WebDriver bidi.

Comment 3

[Julian Descottes [:jdescottes]]

Attachment: Bug 1855149 - [bidi] Add authChallenges to response data in network events

Depends on D190072

Comment 4

[Julian Descottes [:jdescottes]]

Attachment: Bug 1855149 - [wdspec] Check authChallenges in response payloads of network bidi tests

Depends on D190073

Comment 5

[Julian Descottes [:jdescottes]]

Attachment: Bug 1855149 - [wdspec] Add test for a non-empty authChallenges property in responseStarted

Depends on D190074

Comment 6

[Valentin Gosu [:valentin] (he/him)]

(In reply to Julian Descottes [:jdescottes] from comment #1)

Hi Valentin,

I was looking at the parsing of challenges headers (WWW-Authenticate/Proxy-Authenticate). It's not completely trivial, because we can get multiple challenges, separated by a comma, but we can also have commas inside a challenge.

I think this is handled on the platform side at https://searchfox.org/mozilla-central/source/netwerk/protocol/http/nsHttpChannelAuthProvider.cpp#482 and I was wondering if we should try to expose this to JS somehow or if I should duplicate it?

Up to you. I can add an XPCOM helper method to extract the challenges if you think that would be helpful.

Initially thought that nsIHttpAuthenticableChannel::WWWChallenges would be the parsed value, but I think it's just the raw header value? (and I couldn't QueryInterface any channel to nsIHttpAuthenticableChannel for some reason). Maybe this WWWChallenges getter at least concatenates all WWW-Authenticate headers in case there are more than 1?

Yes, WWWChallenges just returns the value of the header. Since the header is not a singleton, it also merges the headers if there are multiple of them.

Not sure why QI wouldn't work for nsIHttpAuthenticableChannel, I don't see anything wrong with the channel.

Comment 7

[Julian Descottes [:jdescottes]]

(In reply to Valentin Gosu [:valentin] (he/him) from comment #6)

(In reply to Julian Descottes [:jdescottes] from comment #1)

Hi Valentin,

I was looking at the parsing of challenges headers (WWW-Authenticate/Proxy-Authenticate). It's not completely trivial, because we can get multiple challenges, separated by a comma, but we can also have commas inside a challenge.

I think this is handled on the platform side at https://searchfox.org/mozilla-central/source/netwerk/protocol/http/nsHttpChannelAuthProvider.cpp#482 and I was wondering if we should try to expose this to JS somehow or if I should duplicate it?

Up to you. I can add an XPCOM helper method to extract the challenges if you think that would be helpful.

This would be really helpful, thanks! I filed Bug 1857847.

For nsIHttpAuthenticableChannel I will try again, maybe I was trying to do it on non-auth channels and that's why it was throwing? But I could succesfully use QI with nsIProxiedChannel on the same channels, so not sure. I'll try to have a reduced test case.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c85cf089e421
[bidi] Add a helper to parse challenge headers r=webdriver-reviewers,whimboo
https://hg.mozilla.org/integration/autoland/rev/ce2bba53b574
[bidi] Add authChallenges to response data in network events r=webdriver-reviewers,whimboo
https://hg.mozilla.org/integration/autoland/rev/c6b8f7f4a912
[wdspec] Check authChallenges in response payloads of network bidi tests r=webdriver-reviewers,whimboo
https://hg.mozilla.org/integration/autoland/rev/0c907b3752ed
[wdspec] Add test for a non-empty authChallenges property in responseStarted r=webdriver-reviewers,whimboo

Comment 9

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/42581 for changes under testing/web-platform/tests

Comment 10

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/c85cf089e421
https://hg.mozilla.org/mozilla-central/rev/ce2bba53b574
https://hg.mozilla.org/mozilla-central/rev/c6b8f7f4a912
https://hg.mozilla.org/mozilla-central/rev/0c907b3752ed

Comment 11

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot