Added OAuth2 authentication to apply_issue

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
@@ skipping 33 matching lines @@
   sys.stdout = Unbuffered(sys.stdout)
   parser = optparse.OptionParser(description=sys.modules[__name__].__doc__)
   parser.add_option(
       '-v', '--verbose', action='count', default=0,
       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(
-      '-w', '--password', default=None,
-      help='Password for email addressed.  Use - to read password from stdin.')
+      '-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. '
+           'Incompatible with -k')
+  parser.add_option(
+      '-k', '--private-key-file',
+      help='Path to file containing a private key in p12 format for OAuth2 '
+           'authentication. Incompatible with -w.')
   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='Base git ref to patch on top of, '
                     'used for verification.')
   options, args = parser.parse_args()
+
+  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.
-  # TODO(maruel): Use OAuth2 properly so we don't hit rate-limiting on login
-  # attempts.
-  # Bad except clauses order (HTTPError is an ancestor class of
-  # ClientLoginError)
-  # pylint: disable=E0701
-  obj = rietveld.Rietveld(options.server, '', None)
-  properties = None
-  try:
+  # Always try un-authenticated first, except for OAuth2
+  if options.private_key_file:
+    # OAuth2 authentication
+    obj = rietveld.JwtOAuth2Rietveld(options.server,
+                                 options.email,

[M-A Ruel, 2014/03/15 01:15:31]

alignment

That's why I prefer +4.

+                                 options.private_key_file,
+                                 private_key_password=options.password)

[M-A Ruel, 2014/03/15 01:15:31]

So password is the private key password in this case? It's not clear to the
user.

[pgervais, 2014/03/17 17:35:11]

I've fixed the help for the -w option.

     properties = obj.get_issue_properties(options.issue, False)
-  except urllib2.HTTPError, e:
-    if e.getcode() != 302:
-      raise
-    elif options.no_auth:
-      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:
+  else:
+    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, e:
+      if e.getcode() != 302:
+        raise
+      elif options.no_auth:
+        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:

[M-A Ruel, 2014/03/15 01:15:31]

stdout?

[pgervais, 2014/03/17 17:35:11]

It was used like that up to now. Might be wrong though :-)

+            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
-    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
 
   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 64 matching lines @@
               f, options.revision_mapping)
           annotated_gclient.emit_buildprops(revisions)
 
         return retcode
   return 0
 
 
 if __name__ == "__main__":
   fix_encoding.fix_encoding()
   sys.exit(main())