Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(797)

Issue 8015004: webRequest.onAuthRequired listeners can provide authentication credentials. (Closed)

Created:
9 years, 3 months ago by cbentzel
Modified:
9 years, 2 months ago
CC:
chromium-reviews, cbentzel+watch_chromium.org, Erik does not do reviews, mihaip+watch_chromium.org, Aaron Boodman, darin-cc_chromium.org, Paweł Hajdan Jr., pam+watch_chromium.org
Visibility:
Public.

Description

webRequest.onAuthRequired listeners can provide authentication credentials. onAuthRequired listeners that specify "blocking" in the extraInfoSpec can return authentication credentials [username and password] in the BlockingResponse. If these are provided, Chrome will use these credentials rather than showing a login prompt for the user. If "blocking" is not specified, or an authCredentials object is not present in the BlockingResponse, then a login prompt will be displayed. Warning: If the authentication credentials are invalid, the extension may still present credentials for subsequent challenges. This could lead to infinite loops of bad credentials being entered without user intervention. BUG=32056 TEST=Write an extension which does a blocking onAuthRequired and provides correct credentials, validate that it works. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=104896

Patch Set 1 #

Patch Set 2 : Handle multiple blocking onAuthRequired #

Total comments: 1

Patch Set 3 : Rebase #

Total comments: 8

Patch Set 4 : Lots of revisions #

Patch Set 5 : Merge with trunk #

Patch Set 6 : Browser tests #

Patch Set 7 : API change, comment improvements #

Patch Set 8 : Merge with head #

Patch Set 9 : Fix tests #

Patch Set 10 : Small fixes #

Patch Set 11 : Fix tests #

Patch Set 12 : Fix indent #

Total comments: 18

Patch Set 13 : Small fixes from battre #

Patch Set 14 : Update description #

Patch Set 15 : Merge #

Unified diffs Side-by-side diffs Delta from patch set Stats (+772 lines, -111 lines) Patch
M chrome/browser/extensions/extension_webrequest_api.h View 1 2 3 4 5 6 7 6 chunks +26 lines, -3 lines 0 comments Download
M chrome/browser/extensions/extension_webrequest_api.cc View 1 2 3 4 5 6 7 8 9 12 chunks +102 lines, -14 lines 0 comments Download
M chrome/browser/extensions/extension_webrequest_api_constants.h View 1 2 3 4 5 6 7 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/browser/extensions/extension_webrequest_api_constants.cc View 1 2 3 4 5 6 7 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/browser/extensions/extension_webrequest_apitest.cc View 1 2 3 4 5 1 chunk +9 lines, -2 lines 0 comments Download
M chrome/browser/net/chrome_network_delegate.cc View 1 2 3 4 5 6 7 1 chunk +3 lines, -3 lines 0 comments Download
M chrome/common/extensions/api/extension_api.json View 1 2 3 4 5 6 7 8 9 10 11 12 13 3 chunks +17 lines, -3 lines 0 comments Download
M chrome/common/extensions/docs/experimental.webRequest.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 4 chunks +275 lines, -5 lines 0 comments Download
M chrome/test/data/extensions/api_test/webrequest/framework.js View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -1 line 0 comments Download
M chrome/test/data/extensions/api_test/webrequest/test_api.html View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +2 lines, -0 lines 0 comments Download
A chrome/test/data/extensions/api_test/webrequest/test_auth_required.html View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +323 lines, -0 lines 0 comments Download
M chrome/test/data/extensions/api_test/webrequest/test_complex.html View 1 2 3 4 5 6 7 2 chunks +0 lines, -80 lines 0 comments Download
M net/base/net_log_event_type_list.h View 1 2 3 1 chunk +8 lines, -0 lines 0 comments Download

Messages

Total messages: 36 (0 generated)
cbentzel
I need to add tests, but otherwise I think this approach is OK. mpcomplete: Please ...
9 years, 3 months ago (2011-09-23 21:04:39 UTC) #1
battre
thanks. willchan has reviewed most of my network related changes of the webrequest api. http://codereview.chromium.org/8015004/diff/4001/net/url_request/url_request.cc ...
9 years, 3 months ago (2011-09-24 13:17:54 UTC) #2
cbentzel
Thanks for the reviews. I'm going to split this into 2 CLs: One which adds ...
9 years, 2 months ago (2011-09-26 21:13:35 UTC) #3
Matt Perry
LGTM http://codereview.chromium.org/8015004/diff/4001/chrome/browser/extensions/extension_webrequest_api.h File chrome/browser/extensions/extension_webrequest_api.h (right): http://codereview.chromium.org/8015004/diff/4001/chrome/browser/extensions/extension_webrequest_api.h#newcode348 chrome/browser/extensions/extension_webrequest_api.h:348: // registered listener that supplies authentication credentials in ...
9 years, 2 months ago (2011-09-26 21:19:45 UTC) #4
Marius
Chris, couple of questions after eyeballing: - what happens on browser startup, when extension might ...
9 years, 2 months ago (2011-09-27 17:59:36 UTC) #5
Matt Perry
On 2011/09/27 17:59:36, Marius wrote: > Chris, > couple of questions after eyeballing: > - ...
9 years, 2 months ago (2011-09-27 18:11:43 UTC) #6
cbentzel
On Tue, Sep 27, 2011 at 2:11 PM, <mpcomplete@chromium.org> wrote: > On 2011/09/27 17:59:36, Marius ...
9 years, 2 months ago (2011-09-27 18:16:00 UTC) #7
cbentzel
On Tue, Sep 27, 2011 at 1:59 PM, <mschilder@google.com> wrote: > Chris, > couple of ...
9 years, 2 months ago (2011-09-27 18:18:47 UTC) #8
Marius
The extension's part of handling the auth request appears fairly synchronous. What if the extension ...
9 years, 2 months ago (2011-09-27 19:56:22 UTC) #9
cbentzel
On Tue, Sep 27, 2011 at 3:56 PM, <mschilder@google.com> wrote: > The extension's part of ...
9 years, 2 months ago (2011-09-27 20:03:18 UTC) #10
cbentzel
On Tue, Sep 27, 2011 at 4:03 PM, Chris Bentzel <cbentzel@chromium.org> wrote: > On Tue, ...
9 years, 2 months ago (2011-09-27 20:22:55 UTC) #11
Matt Perry
On 2011/09/27 20:22:55, cbentzel wrote: > On Tue, Sep 27, 2011 at 4:03 PM, Chris ...
9 years, 2 months ago (2011-09-27 20:25:00 UTC) #12
Marius
On Tue, Sep 27, 2011 at 1:03 PM, Chris Bentzel <cbentzel@chromium.org>wrote: > On Tue, Sep ...
9 years, 2 months ago (2011-09-27 22:21:14 UTC) #13
Matt Perry
On 2011/09/27 22:21:14, Marius wrote: > On Tue, Sep 27, 2011 at 1:03 PM, Chris ...
9 years, 2 months ago (2011-09-27 22:29:35 UTC) #14
Marius
On Tue, Sep 27, 2011 at 3:29 PM, <mpcomplete@chromium.org> wrote: > On 2011/09/27 22:21:14, Marius ...
9 years, 2 months ago (2011-09-27 22:33:59 UTC) #15
Matt Perry
On 2011/09/27 22:33:59, Marius wrote: > On Tue, Sep 27, 2011 at 3:29 PM, <mailto:mpcomplete@chromium.org> ...
9 years, 2 months ago (2011-09-28 00:02:30 UTC) #16
Matt Perry
On 2011/09/28 00:02:30, Matt Perry wrote: > On 2011/09/27 22:33:59, Marius wrote: > > On ...
9 years, 2 months ago (2011-09-28 00:03:00 UTC) #17
cbentzel
On Tue, Sep 27, 2011 at 8:02 PM, <mpcomplete@chromium.org> wrote: > Chris, what do you ...
9 years, 2 months ago (2011-09-28 00:50:36 UTC) #18
Matt Perry
On 2011/09/28 00:50:36, cbentzel wrote: > On Tue, Sep 27, 2011 at 8:02 PM, <mailto:mpcomplete@chromium.org> ...
9 years, 2 months ago (2011-09-28 18:45:15 UTC) #19
cbentzel
On Wed, Sep 28, 2011 at 2:45 PM, <mpcomplete@chromium.org> wrote: > On 2011/09/28 00:50:36, cbentzel ...
9 years, 2 months ago (2011-09-28 19:03:36 UTC) #20
Matt Perry
On 2011/09/28 19:03:36, cbentzel wrote: > On Wed, Sep 28, 2011 at 2:45 PM, <mailto:mpcomplete@chromium.org> ...
9 years, 2 months ago (2011-09-28 19:19:25 UTC) #21
Ryan Hamilton
On 2011/09/28 19:19:25, Matt Perry wrote: > On 2011/09/28 19:03:36, cbentzel wrote: > > On ...
9 years, 2 months ago (2011-10-06 19:37:16 UTC) #22
cbentzel
No update on async_blocking. Hoping to land blocking change soon.
9 years, 2 months ago (2011-10-07 14:22:05 UTC) #23
cbentzel
Please review again. A previous CL split the URLRequest/NetworkDelegate half of onAuthRequired out, and has ...
9 years, 2 months ago (2011-10-10 11:54:41 UTC) #24
cbentzel
http://codereview.chromium.org/8015004/diff/47020/chrome/common/extensions/docs/experimental.devtools.console.html File chrome/common/extensions/docs/experimental.devtools.console.html (right): http://codereview.chromium.org/8015004/diff/47020/chrome/common/extensions/docs/experimental.devtools.console.html#newcode19 chrome/common/extensions/docs/experimental.devtools.console.html:19: <title>chrome.experimental.devtools.console I'm not sure why these line breaks were ...
9 years, 2 months ago (2011-10-10 11:55:40 UTC) #25
battre
http://codereview.chromium.org/8015004/diff/47020/chrome/browser/extensions/extension_webrequest_api.cc File chrome/browser/extensions/extension_webrequest_api.cc (right): http://codereview.chromium.org/8015004/diff/47020/chrome/browser/extensions/extension_webrequest_api.cc#newcode1471 chrome/browser/extensions/extension_webrequest_api.cc:1471: if (canceled) { This is a change the semantics ...
9 years, 2 months ago (2011-10-10 13:47:03 UTC) #26
cbentzel
Thanks for the fast review. http://codereview.chromium.org/8015004/diff/47020/chrome/browser/extensions/extension_webrequest_api.cc File chrome/browser/extensions/extension_webrequest_api.cc (right): http://codereview.chromium.org/8015004/diff/47020/chrome/browser/extensions/extension_webrequest_api.cc#newcode1471 chrome/browser/extensions/extension_webrequest_api.cc:1471: if (canceled) { On ...
9 years, 2 months ago (2011-10-10 13:59:54 UTC) #27
battre
http://codereview.chromium.org/8015004/diff/47020/chrome/browser/extensions/extension_webrequest_api.cc File chrome/browser/extensions/extension_webrequest_api.cc (right): http://codereview.chromium.org/8015004/diff/47020/chrome/browser/extensions/extension_webrequest_api.cc#newcode1471 chrome/browser/extensions/extension_webrequest_api.cc:1471: if (canceled) { On 2011/10/10 13:59:54, cbentzel wrote: > ...
9 years, 2 months ago (2011-10-10 14:32:13 UTC) #28
cbentzel
The test for using an invalid password will require some changes to framework.js, since the ...
9 years, 2 months ago (2011-10-10 14:38:06 UTC) #29
battre
I think the test for a second onAuthRequired events is not essential. LGTM from my ...
9 years, 2 months ago (2011-10-10 14:47:10 UTC) #30
cbentzel
I'm still working on the bad-credentials test case, but will do it in a separate ...
9 years, 2 months ago (2011-10-10 15:09:19 UTC) #31
cbentzel
mpcomplete: Do you want to do another pass over this?
9 years, 2 months ago (2011-10-10 15:42:17 UTC) #32
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/cbentzel@chromium.org/8015004/51003
9 years, 2 months ago (2011-10-11 10:16:53 UTC) #33
commit-bot: I haz the power
Can't apply patch for file chrome/common/extensions/docs/experimental.devtools.console.html. While running patch -p1 --forward --force; patching file chrome/common/extensions/docs/experimental.devtools.console.html ...
9 years, 2 months ago (2011-10-11 10:17:00 UTC) #34
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/cbentzel@chromium.org/8015004/55002
9 years, 2 months ago (2011-10-11 12:58:31 UTC) #35
commit-bot: I haz the power
9 years, 2 months ago (2011-10-11 15:36:48 UTC) #36
Change committed as 104896

Powered by Google App Engine
This is Rietveld 408576698