Extract authentication options handling into a separate function.

Extract authentication options handling into a separate function.

It is done in preparation for switching to OAuth2 as default (and only)
authentication method. Having all auth options handled by the same code makes it
easier to gradually add OAuth2 support.

As part of this, some options that would no longer work with OAuth2 (and that
are not being used from anywhere now, as far as I can tell) are removed:
  * Passing account password for authentication via command line.
  * Overriding 'Host' header when making requests to Rietveld (won't work with
    SSL anyway).
  * --account_type option (seems to be ClientLogin specific).

R=maruel@chromium.org
BUG=356813
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=294746

[Vadim Sh., 2015-04-09 01:04:39]

https://codereview.chromium.org/1075723002/diff/1/apply_issue.py
File apply_issue.py (right):

https://codereview.chromium.org/1075723002/diff/1/apply_issue.py#newcode150
apply_issue.py:150: if options.no_auth:
I do not fully understand this logic... I suspect I'll change this file later
when OAuth2 is added.

https://codereview.chromium.org/1075723002/diff/1/git_cherry_pick_upload.py
File git_cherry_pick_upload.py (right):

https://codereview.chromium.org/1075723002/diff/1/git_cherry_pick_upload.py#n...
git_cherry_pick_upload.py:130: parser = optparse.OptionParser(
downgraded from ArgumentParser to OptionParser, since all other script use
OptionParser (and auth.add_auth_options expecting it too).

https://codereview.chromium.org/1075723002/diff/1/git_cl.py
File git_cl.py (right):

https://codereview.chromium.org/1075723002/diff/1/git_cl.py#newcode1421
git_cl.py:1421: auth.add_auth_options(parser)
an alternative would be to add auth options to default parser class. But then
they would show up in --help for commands that do not do network calls.

https://codereview.chromium.org/1075723002/diff/1/my_activity.py
File my_activity.py (right):

https://codereview.chromium.org/1075723002/diff/1/my_activity.py#newcode16
my_activity.py:16: # TODO(vadimsh): This script knows too much about ClientLogin
and cookies. It
it enumerates cookies and uses ClientLogin to access code.google.com. Not sure
how "fresh" this script is. I didn't bother with it much.

https://codereview.chromium.org/1075723002/diff/1/presubmit_support.py
File presubmit_support.py (left):

https://codereview.chromium.org/1075723002/diff/1/presubmit_support.py#oldcod...
presubmit_support.py:1640: parser.add_option("--rietveld_password",
help=optparse.SUPPRESS_HELP)
I did code search for this (and other removed) options, they seem not being
used.

https://codereview.chromium.org/1075723002/diff/1/rietveld.py
File rietveld.py (left):

https://codereview.chromium.org/1075723002/diff/1/rietveld.py#oldcode49
rietveld.py:49: self.rpc_server = upload.HttpRpcServer(
this branch is removed, since password can't be passed from command line now

https://codereview.chromium.org/1075723002/diff/1/rietveld.py#oldcode452
rietveld.py:452: class OAuthRpcServer(object):
This will be eventually removed (or rather merged into something else).

https://codereview.chromium.org/1075723002/diff/1/rietveld.py#oldcode564
rietveld.py:564: class JwtOAuth2Rietveld(Rietveld):
This will be eventually removed. JWT stuff will be passed via auth_config to
regular Rietveld class.

[M-A Ruel, 2015-04-09 01:20:59]

lgtm, good luck.

https://codereview.chromium.org/1075723002/diff/1/apply_issue.py
File apply_issue.py (right):

https://codereview.chromium.org/1075723002/diff/1/apply_issue.py#newcode161
apply_issue.py:161: except rietveld.upload.ClientLoginError, e:
except rietveld.upload.ClientLoginError as e:

https://codereview.chromium.org/1075723002/diff/1/gcl.py
File gcl.py (right):

https://codereview.chromium.org/1075723002/diff/1/gcl.py#newcode1472
gcl.py:1472: if not argv:
I'd still add a:

sys.stderr.write('Warning: gcl is going away soon. Get off subversion!\n')
sys.stderr.write('See http://crbug.com/475321 for more details.\n')

https://codereview.chromium.org/1075723002/diff/1/my_activity.py
File my_activity.py (right):

https://codereview.chromium.org/1075723002/diff/1/my_activity.py#newcode16
my_activity.py:16: # TODO(vadimsh): This script knows too much about ClientLogin
and cookies. It
On 2015/04/09 01:04:39, Vadim Sh. wrote:
> it enumerates cookies and uses ClientLogin to access http://code.google.com.
Not sure
> how "fresh" this script is. I didn't bother with it much.

It's used, but it's lower priority.

[Vadim Sh., 2015-04-09 01:34:30]

https://codereview.chromium.org/1075723002/diff/1/apply_issue.py
File apply_issue.py (right):

https://codereview.chromium.org/1075723002/diff/1/apply_issue.py#newcode161
apply_issue.py:161: except rietveld.upload.ClientLoginError, e:
On 2015/04/09 01:20:59, M-A Ruel wrote:
> except rietveld.upload.ClientLoginError as e:

Done.

https://codereview.chromium.org/1075723002/diff/1/gcl.py
File gcl.py (right):

https://codereview.chromium.org/1075723002/diff/1/gcl.py#newcode1472
gcl.py:1472: if not argv:
On 2015/04/09 01:20:59, M-A Ruel wrote:
> I'd still add a:
> 
> sys.stderr.write('Warning: gcl is going away soon. Get off subversion!\n')
> sys.stderr.write('See http://crbug.com/475321 for more details.\n')

Done.

https://codereview.chromium.org/1075723002/diff/1/my_activity.py
File my_activity.py (right):

https://codereview.chromium.org/1075723002/diff/1/my_activity.py#newcode16
my_activity.py:16: # TODO(vadimsh): This script knows too much about ClientLogin
and cookies. It
On 2015/04/09 01:20:59, M-A Ruel wrote:
> On 2015/04/09 01:04:39, Vadim Sh. wrote:
> > it enumerates cookies and uses ClientLogin to access http://code.google.com.
> Not sure
> > how "fresh" this script is. I didn't bother with it much.
> 
> It's used, but it's lower priority.

Ack. Not sure what what to do with code.google.com. I think it doesn't support
OAuth2 (not sure).

[Vadim Sh., 2015-04-09 21:39:07]

The CQ bit was checked by vadimsh@chromium.org

[Vadim Sh., 2015-04-09 21:39:08]

The patchset sent to the CQ was uploaded after l-g-t-m from maruel@chromium.org
Link to the patchset: https://codereview.chromium.org/1075723002/#ps20001
(title: " ")

[commit-bot: I haz the power, 2015-04-09 21:39:29]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1075723002/20001

[commit-bot: I haz the power, 2015-04-09 21:39:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-04-09 21:39:38]

Failed to apply patch for depot_tools/auth.py:
While running svn add depot_tools/auth.py --force --config-dir
/b/infra_internal/commit_queue/subversion_config --non-interactive;
  patching file auth.py
  svn: warning: W155010:
'/b/infra_internal/commit_queue/workdir/tools/depot_tools/auth.py' not found
  svn: E200009: Could not add all targets because some targets don't exist
  svn: E200009: Illegal target for the requested operation

Patch:   N   depot_tools/auth.py
Index: auth.py
diff --git depot_tools/auth.py depot_tools/auth.py
new file mode 100644
index
0000000000000000000000000000000000000000..97520deca3cdf4f47300fa694194a7ae2dc2b031
--- /dev/null
+++ b/depot_tools/auth.py
@@ -0,0 +1,99 @@
+# Copyright (c) 2015 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+"""Authentication related functions."""
+
+import collections
+import optparse
+
+
+# Authentication configuration extracted from command line options.
+# See doc string for 'make_auth_config' for meaning of fields.
+AuthConfig = collections.namedtuple('AuthConfig', [
+    'use_oauth2', # deprecated, will be always True
+    'save_cookies', # deprecated, will be removed
+    'use_local_webserver',
+    'webserver_port',
+])
+
+
+def make_auth_config(
+    use_oauth2=None,
+    save_cookies=None,
+    use_local_webserver=None,
+    webserver_port=None):
+  """Returns new instance of AuthConfig.
+
+  If some config option is None, it will be set to a reasonable default value.
+  This function also acts as an authoritative place for default values of
+  corresponding command line options.
+  """
+  default = lambda val, d: val if val is not None else d
+  return AuthConfig(
+      default(use_oauth2, False),
+      default(save_cookies, True),
+      default(use_local_webserver, True),
+      default(webserver_port, 8090))
+
+
+def add_auth_options(parser):
+  """Appends OAuth related options to OptionParser."""
+  default_config = make_auth_config()
+  parser.auth_group = optparse.OptionGroup(parser, 'Auth options')
+  parser.auth_group.add_option(
+      '--oauth2',
+      action='store_true',
+      dest='use_oauth2',
+      default=default_config.use_oauth2,
+      help='Use OAuth 2.0 instead of a password.')
+  parser.auth_group.add_option(
+      '--no-oauth2',
+      action='store_false',
+      dest='use_oauth2',
+      default=default_config.use_oauth2,
+      help='Use password instead of OAuth 2.0.')
+  parser.auth_group.add_option(
+      '--no-cookies',
+      action='store_false',
+      dest='save_cookies',
+      default=default_config.save_cookies,
+      help='Do not save authentication cookies to local disk.')
+  parser.auth_group.add_option(
+      '--auth-no-local-webserver',
+      action='store_false',
+      dest='use_local_webserver',
+      default=default_config.use_local_webserver,
+      help='Do not run a local web server when performing OAuth2 login flow.')
+  parser.auth_group.add_option(
+      '--auth-host-port',
+      type=int,
+      default=default_config.webserver_port,
+      help='Port a local web server should listen on. Used only if '
+          '--auth-no-local-webserver is not set. [default: %default]')
+  parser.add_option_group(parser.auth_group)
+
+
+def extract_auth_config_from_options(options):
+  """Given OptionParser parsed options, extracts AuthConfig from it.
+
+  OptionParser should be populated with auth options by 'add_auth_options'.
+  """
+  return make_auth_config(
+      use_oauth2=options.use_oauth2,
+      save_cookies=False if options.use_oauth2 else options.save_cookies,
+      use_local_webserver=options.use_local_webserver,
+      webserver_port=options.auth_host_port)
+
+
+def auth_config_to_command_options(auth_config):
+  """AuthConfig -> list of strings with command line options."""
+  if not auth_config:
+    return []
+  opts = ['--oauth2' if auth_config.use_oauth2 else '--no-oauth2']
+  if not auth_config.save_cookies:
+    opts.append('--no-cookies')
+  if not auth_config.use_local_webserver:
+    opts.append('--auth-no-local-webserver')
+  opts.extend(['--auth-host-port', str(auth_config.webserver_port)])
+  return opts

[Vadim Sh., 2015-04-09 22:02:04]

Committed patchset #2 (id:20001) manually as 294746.