Added POST capability to oauth Rietveld

rietveld.py

 # coding: utf-8
 # 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.
 """Defines class Rietveld to easily access a rietveld instance.
 
 Security implications:
 
 The following hypothesis are made:
 - Rietveld enforces:
@@ skipping 429 matching lines @@
     finally:
       upload.ErrorExit = old_error_exit
 
   # DEPRECATED.
   Send = get
 
 
 class OAuthRpcServer(object):
   def __init__(self,
                host,
-               client_id,
+               client_email,
                client_private_key,
                private_key_password='notasecret',
                user_agent=None,
                timeout=None,
                extra_headers=None):
     """Wrapper around httplib2.Http() that handles authentication.
 
-    client_id: client id for service account
+    client_email: email associated with the service account
     client_private_key: encrypted private key, as a string
     private_key_password: password used to decrypt the private key
     """
 
     # Enforce https
     host_parts = urlparse.urlparse(host)
 
     if host_parts.scheme == 'https':  # fine
       self.host = host
     elif host_parts.scheme == 'http':
       upload.logging.warning('Changing protocol to https')
       self.host = 'https' + host[4:]
     else:
       msg = 'Invalid url provided: %s' % host
       upload.logging.error(msg)
       raise ValueError(msg)
 
     self.host = self.host.rstrip('/')
 
     self.extra_headers = extra_headers or {}
 
     if not oa2client.HAS_OPENSSL:
-      logging.error("Support for OpenSSL hasn't been found, "
+      logging.error("No support for OpenSSL has been found, "
                     "OAuth2 support requires it.")
       logging.error("Installing pyopenssl will probably solve this issue.")
       raise RuntimeError('No OpenSSL support')
     creds = oa2client.SignedJwtAssertionCredentials(
-      client_id,
+      client_email,
       client_private_key,
       'https://www.googleapis.com/auth/userinfo.email',
       private_key_password=private_key_password,
       user_agent=user_agent)
 
     self._http = creds.authorize(httplib2.Http(timeout=timeout))
 
   def Send(self,
            request_path,
            payload=None,
@@ skipping 12 matching lines @@
     """
     # This method signature should match upload.py:AbstractRpcServer.Send()
     method = 'GET'
 
     headers = self.extra_headers.copy()
     headers.update(extra_headers or {})
 
     if payload is not None:
       method = 'POST'
       headers['Content-Type'] = content_type
-      raise NotImplementedError('POST requests are not yet supported.')
+#      raise NotImplementedError('POST requests are not yet supported.')

[Sergey Berezin, 2014/04/14 20:59:42]

Let's remove this comment.

[pgervais, 2014/04/14 21:28:05]

Done.

 
     prev_timeout = self._http.timeout
     try:
       if timeout:
         self._http.timeout = timeout
       # TODO(pgervais) implement some kind of retry mechanism (see upload.py).
       url = self.host + request_path
       if kwargs:
         url += "?" + urllib.urlencode(kwargs)
 
+      logging.info(url)

[Sergey Berezin, 2014/04/14 20:59:42]

Please add a meaningful message to the log.

[pgervais, 2014/04/14 21:28:05]

Removed it (was for testing, and forgotten)

       ret = self._http.request(url,
                                method=method,
                                body=payload,
                                headers=headers)
-      if not ret[0]['content-location'].startswith(self.host):
+
+      if (method == 'GET'
+          and not ret[0]['content-location'].startswith(self.host)):
         upload.logging.warning('Redirection to host %s detected: '
                                'login may have failed/expired.'
                                % urlparse.urlparse(
                                      ret[0]['content-location']).netloc)
       return ret[1]
 
     finally:
       self._http.timeout = prev_timeout
 
 
 class JwtOAuth2Rietveld(Rietveld):
   """Access to Rietveld using OAuth authentication.
 
   This class is supposed to be used only by bots, since this kind of
   access is restricted to service accounts.
   """
   # The parent__init__ is not called on purpose.
   # pylint: disable=W0231
   def __init__(self,
                url,
-               client_id,
+               client_email,
                client_private_key_file,
                private_key_password=None,
                extra_headers=None):
+
+    # These attributes are accessed by commit queue. Keep them.
+    self.email = client_email
+    self.private_key_file = client_private_key_file
+
     if private_key_password is None:  # '' means 'empty password'
       private_key_password = 'notasecret'
 
     self.url = url.rstrip('/')
+    bot_url = self.url
+    if not bot_url.endswith('/bots'):
+      bot_url += '/bots'
+
     with open(client_private_key_file, 'rb') as f:
       client_private_key = f.read()
-    self.rpc_server = OAuthRpcServer(url,
-                                     client_id,
+    logging.info('Using login: %s' % client_email)

[Sergey Berezin, 2014/04/14 20:59:42]

Would it make sense to say 'Using OAuth login: %s' to make logs more
self-explanatory?

[Sergey Berezin, 2014/04/14 22:49:46]

What about this comment?

[pgervais, 2014/04/14 23:10:43]

Sorry, I've overlooked it completely. I definitely makes sense. I'll take care
of that.

+    self.rpc_server = OAuthRpcServer(bot_url,
+                                     client_email,
                                      client_private_key,
                                      private_key_password=private_key_password,
                                      extra_headers=extra_headers or {})
     self._xsrf_token = None
     self._xsrf_token_time = None
 
 
 class CachingRietveld(Rietveld):
   """Caches the common queries.
 
@@ skipping 119 matching lines @@
   def trigger_try_jobs(  # pylint:disable=R0201
       self, issue, patchset, reason, clobber, revision, builders_and_tests,
       master=None):
     logging.info('ReadOnlyRietveld: triggering try jobs %r for issue %d' %
         (builders_and_tests, issue))
 
   def trigger_distributed_try_jobs(  # pylint:disable=R0201
       self, issue, patchset, reason, clobber, revision, masters):
     logging.info('ReadOnlyRietveld: triggering try jobs %r for issue %d' %
         (masters, issue))