Extract authentication options handling into a separate function.

apply_issue.py

 #!/usr/bin/env python
 # Copyright (c) 2012 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.
 
 """Applies an issue from Rietveld.
 """
 
 import getpass
 import json
 import logging
 import optparse
 import os
 import subprocess
 import sys
 import urllib2
 
 import breakpad  # pylint: disable=W0611
 
 import annotated_gclient
+import auth
 import checkout
 import fix_encoding
 import gclient_utils
 import rietveld
 import scm
 
 BASE_DIR = os.path.dirname(os.path.abspath(__file__))
 
 
 class Unbuffered(object):
@@ skipping 18 matching lines @@
       help='Prints debugging infos')
   parser.add_option(
       '-e', '--email',
       help='Email address to access rietveld.  If not specified, anonymous '
            'access will be used.')
   parser.add_option(
       '-E', '--email-file',
       help='File containing the email address to access rietveld. '
            'If not specified, anonymous access will be used.')
   parser.add_option(
-      '-w', '--password',
-      help='Password for email addressed. Use - to read password from stdin. '
-           'if -k is provided, this is the private key file password.')
-  parser.add_option(
       '-k', '--private-key-file',
       help='Path to file containing a private key in p12 format for OAuth2 '
-           'authentication. Use -w to provide the decrypting password, if any.')
+           'authentication with "notasecret" password (as generated by Google '
+           'Cloud Console).')
   parser.add_option(
       '-i', '--issue', type='int', help='Rietveld issue number')
   parser.add_option(
       '-p', '--patchset', type='int', help='Rietveld issue\'s patchset number')
   parser.add_option(
       '-r',
       '--root_dir',
       default=os.getcwd(),
       help='Root directory to apply the patch')
   parser.add_option(
       '-s',
       '--server',
       default='http://codereview.chromium.org',
       help='Rietveld server')
   parser.add_option('--no-auth', action='store_true',
                     help='Do not attempt authenticated requests.')
   parser.add_option('--revision-mapping', default='{}',
                     help='When running gclient, annotate the got_revisions '
                          'using the revision-mapping.')
   parser.add_option('-f', '--force', action='store_true',
                     help='Really run apply_issue, even if .update.flag '
                          'is detected.')
   parser.add_option('-b', '--base_ref', help='DEPRECATED do not use.')
   parser.add_option('--whitelist', action='append', default=[],
                     help='Patch only specified file(s).')
   parser.add_option('--blacklist', action='append', default=[],
                     help='Don\'t patch specified file(s).')
   parser.add_option('-d', '--ignore_deps', action='store_true',
                     help='Don\'t run gclient sync on DEPS changes.')
+
+  auth.add_auth_options(parser)
   options, args = parser.parse_args()
+  auth_config = auth.extract_auth_config_from_options(options)
 
   if options.whitelist and options.blacklist:
     parser.error('Cannot specify both --whitelist and --blacklist')
 
-  if options.password and options.private_key_file:
-    parser.error('-k and -w options are incompatible')
   if options.email and options.email_file:
     parser.error('-e and -E options are incompatible')
 
   if (os.path.isfile(os.path.join(os.getcwd(), 'update.flag'))
       and not options.force):
     print 'update.flag file found: bot_update has run and checkout is already '
     print 'in a consistent state. No actions will be performed in this step.'
     return 0
   logging.basicConfig(
       format='%(levelname)5s %(module)11s(%(lineno)4d): %(message)s',
       level=[logging.WARNING, logging.INFO, logging.DEBUG][
           min(2, options.verbose)])
   if args:
     parser.error('Extra argument(s) "%s" not understood' % ' '.join(args))
   if not options.issue:
     parser.error('Require --issue')
   options.server = options.server.rstrip('/')
   if not options.server:
     parser.error('Require a valid server')
 
   options.revision_mapping = json.loads(options.revision_mapping)
 
-  if options.password == '-':
-    print('Reading password')
-    options.password = sys.stdin.readline().strip()
-
   # read email if needed
   if options.email_file:
     if not os.path.exists(options.email_file):
       parser.error('file does not exist: %s' % options.email_file)
     with open(options.email_file, 'rb') as f:
       options.email = f.read().strip()
 
   print('Connecting to %s' % options.server)
   # Always try un-authenticated first, except for OAuth2
   if options.private_key_file:
     # OAuth2 authentication
     obj = rietveld.JwtOAuth2Rietveld(options.server,
                                      options.email,
-                                     options.private_key_file,
-                                     private_key_password=options.password)
+                                     options.private_key_file)
     properties = obj.get_issue_properties(options.issue, False)
   else:
-    obj = rietveld.Rietveld(options.server, '', None)
+    # Passing None as auth_config disables authentication.
+    obj = rietveld.Rietveld(options.server, None)
     properties = None
     # Bad except clauses order (HTTPError is an ancestor class of
     # ClientLoginError)
     # pylint: disable=E0701
     try:
       properties = obj.get_issue_properties(options.issue, False)
     except urllib2.HTTPError as e:
       if e.getcode() != 302:
         raise
       if options.no_auth:

[Vadim Sh., 2015/04/09 01:04:38]

I do not fully understand this logic... I suspect I'll change this file later
when OAuth2 is added.

         exit('FAIL: Login detected -- is issue private?')
       # TODO(maruel): A few 'Invalid username or password.' are printed first,
       # we should get rid of those.
     except rietveld.upload.ClientLoginError, e:
       # Fine, we'll do proper authentication.
       pass
     if properties is None:
-      if options.email is not None:
-        obj = rietveld.Rietveld(options.server, options.email, options.password)
-        try:
-          properties = obj.get_issue_properties(options.issue, False)
-        except rietveld.upload.ClientLoginError, e:
-          if sys.stdout.closed:
-            print('Accessing the issue requires proper credentials.')
-            return 1
-      else:
-        print('Accessing the issue requires login.')
-        obj = rietveld.Rietveld(options.server, None, None)
-        try:
-          properties = obj.get_issue_properties(options.issue, False)
-        except rietveld.upload.ClientLoginError, e:
-          print('Accessing the issue requires proper credentials.')
-          return 1
+      obj = rietveld.Rietveld(options.server, auth_config, options.email)
+      try:
+        properties = obj.get_issue_properties(options.issue, False)
+      except rietveld.upload.ClientLoginError, e:

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

except rietveld.upload.ClientLoginError as e:

[Vadim Sh., 2015/04/09 01:34:29]

Done.

+        print('Accessing the issue requires proper credentials.')
+        return 1
 
   if not options.patchset:
     options.patchset = properties['patchsets'][-1]
     print('No patchset specified. Using patchset %d' % options.patchset)
 
   print('Downloading the patch.')
   try:
     patchset = obj.get_patch(options.issue, options.patchset)
   except urllib2.HTTPError, e:
     print(
@@ skipping 71 matching lines @@
   return 0
 
 
 if __name__ == "__main__":
   fix_encoding.fix_encoding()
   try:
     sys.exit(main())
   except KeyboardInterrupt:
     sys.stderr.write('interrupted\n')
     sys.exit(1)