--auth-schemes specifies which authentication schemes are supported on the co...

--auth-schemes specifies which authentication schemes are supported on the command line.

--auth-schemes should be a command separated list containing one or more of the following schemes: basic, digest, ntlm, or negotiate.

This will primarily be used to help triage user-reported bugs.

BUG=None
TEST=specify --auth-schemes and ensure that only the specified schemes are supported. Also, when unspecified all schemes should be supported.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=57073

[cbentzel, 2010-08-18 20:34:47]

On 2010/08/18 20:30:26, cbentzel wrote:
> 

Also, this removes --experimental-enable-negotiate-auth since that was on by
default and a user can control behavior with --auth-schemes.

[cbentzel, 2010-08-19 14:07:52]

Wan-Teh, sorry to add another CL to your queue but Andy won't be able to review
this today.

[wtc, 2010-08-19 17:11:41]

LGTM.  I'll be happy to review your code.

Just some nits below.  The most important issue is the name
of the new static method "CreateDefault".

http://codereview.chromium.org/3199002/diff/7001/8002
File chrome/common/chrome_switches.cc (right):

http://codereview.chromium.org/3199002/diff/7001/8002#newcode64
chrome/common/chrome_switches.cc:64: const char kAuthSchemes[]                  
= "auth-schemes";
I would prefer "http-auth-schemes" because these are *HTTP*
authentication schemes, but it's probably better to be
consistent with the related options that simply say "auth".

http://codereview.chromium.org/3199002/diff/7001/8003
File chrome/common/chrome_switches.h (left):

http://codereview.chromium.org/3199002/diff/7001/8003#oldcode156
chrome/common/chrome_switches.h:156: // Experimental features.
Keep the "Experimental features" and "End experimental features"
comments, because the kExperimentalSpellcheckerFeatures
option is still there.

http://codereview.chromium.org/3199002/diff/7001/8004
File net/http/http_auth_handler_factory.cc (right):

http://codereview.chromium.org/3199002/diff/7001/8004#newcode61
net/http/http_auth_handler_factory.cc:61: std::find(supported_schemes.begin(),
supported_schemes.end(), scheme) !=
Nit: the way you break this long return statement looks
uncommon.  Does std::vector not have a find method?

http://codereview.chromium.org/3199002/diff/7001/8005
File net/http/http_auth_handler_factory.h (right):

http://codereview.chromium.org/3199002/diff/7001/8005#newcode113
net/http/http_auth_handler_factory.h:113: static HttpAuthHandlerRegistryFactory*
CreateDefault(
A function that takes five parameters probably should not
be named Create*Default* :-)

[cbentzel, 2010-08-19 18:55:58]

Thanks for looking.

http://codereview.chromium.org/3199002/diff/7001/8002
File chrome/common/chrome_switches.cc (right):

http://codereview.chromium.org/3199002/diff/7001/8002#newcode64
chrome/common/chrome_switches.cc:64: const char kAuthSchemes[]                  
= "auth-schemes";
On 2010/08/19 17:11:41, wtc wrote:
> I would prefer "http-auth-schemes" because these are *HTTP*
> authentication schemes, but it's probably better to be
> consistent with the related options that simply say "auth".

Yes, I can do a pass in another CL to make all of the names http-auth. I'll have
to change the documentation as well.

http://codereview.chromium.org/3199002/diff/7001/8003
File chrome/common/chrome_switches.h (left):

http://codereview.chromium.org/3199002/diff/7001/8003#oldcode156
chrome/common/chrome_switches.h:156: // Experimental features.
On 2010/08/19 17:11:41, wtc wrote:
> Keep the "Experimental features" and "End experimental features"
> comments, because the kExperimentalSpellcheckerFeatures
> option is still there.

Andy added these comments when he added kExperimentalEnableNegotiateAuth, and it
isn't exactly correct as there are many command line flags for experimental
features which are not between these comments. 

However, I'll add the comments back in and make the change in another CL.

http://codereview.chromium.org/3199002/diff/7001/8004
File net/http/http_auth_handler_factory.cc (right):

http://codereview.chromium.org/3199002/diff/7001/8004#newcode61
net/http/http_auth_handler_factory.cc:61: std::find(supported_schemes.begin(),
supported_schemes.end(), scheme) !=
On 2010/08/19 17:11:41, wtc wrote:
> Nit: the way you break this long return statement looks
> uncommon.  Does std::vector not have a find method?

std::vector does not have find. I considered passing in a std::set but thought
that was overkill. I broke this up into a line to get the const_iterator and a
line to compare against end which looks better.

http://codereview.chromium.org/3199002/diff/7001/8005
File net/http/http_auth_handler_factory.h (right):

http://codereview.chromium.org/3199002/diff/7001/8005#newcode113
net/http/http_auth_handler_factory.h:113: static HttpAuthHandlerRegistryFactory*
CreateDefault(
On 2010/08/19 17:11:41, wtc wrote:
> A function that takes five parameters probably should not
> be named Create*Default* :-)

Good point. I changed this to HttpAuthHandlerRegistryFactory::Create. I'm not
sure if that's an improvement, but I couldn't come up with a better name. At
least I documented it this time around.

[wtc, 2010-08-19 21:33:20]

LGTM.