Added OAuth2 authentication to apply_issue

apply_issue.py

Index: apply_issue.py
diff --git a/apply_issue.py b/apply_issue.py
index 690db0d323e4f545af0544288846548b0bb396a3..eab9b7b5987fa135c0094182fce7aab4eea5fc80 100755
--- a/apply_issue.py
+++ b/apply_issue.py
@@ -51,8 +51,17 @@ def main():
       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', default=None,

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

Remove this one while at it.

[pgervais, 2014/03/13 22:38:38]

I think we're still using it for the public rietveld. I'd like to double-check
before removing it, at least.

-      help='Password for email addressed.  Use - to read password from stdin.')
+      help='Password for email addressed.  Use - to read password from stdin. '
+      'Incompatible with -k')

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

prefer to keep alignment consistent.

[pgervais, 2014/03/13 22:38:38]

Done.

+  parser.add_option(
+      '-k', '--private-key-file', default=None,

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

default=None is the default, no need to specify it.

[pgervais, 2014/03/13 22:38:38]

alright, but explicit is better than implicit.
Done.

+      help='Path to file containing a private key in p12 format for OAuth2 '

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

You mean for a service account? That would be for the bots? I don't see this
being used by the users.

[pgervais, 2014/03/13 22:38:38]

It is indeed for the bots. No user will use that. 
I can use optparse.SUPPRESS_HELP if you think it makes more sense.

+      'authentication. Incompatible with -w.')
   parser.add_option(
       '-i', '--issue', type='int', help='Rietveld issue number')
   parser.add_option(
@@ -78,6 +87,12 @@ def main():
   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 '
@@ -101,44 +116,58 @@ def main():
     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):
+      print('file does not exist: %s' % str(options.email_file))

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

parser.error()
Then you don't need to return.

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

No need to str().

[pgervais, 2014/03/13 22:38:38]

Done.

+      return 1
+    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.OAuthRietveld(options.server,
+                                 options.email,
+                                 options.private_key_file,
+                                 private_key_password=options.password)
     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:
+            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]