Added OAuth2 authentication to apply_issue

rietveld.py

Index: rietveld.py
diff --git a/rietveld.py b/rietveld.py
index aad998e9c3e49d922ca38317c0e9c1381d88885a..a7566c103cd248fee6d4f81167fff76a856fbdc5 100644
--- a/rietveld.py
+++ b/rietveld.py
@@ -20,11 +20,16 @@ import logging
 import re
 import ssl
 import time
+import urllib
 import urllib2
+import urlparse
 
-from third_party import upload
 import patch
 
+from third_party import upload
+from third_party.oauth2client.client import SignedJwtAssertionCredentials
+from third_party import httplib2
+
 # Hack out upload logging.info()
 upload.logging = logging.getLogger('upload')
 # Mac pylint choke on this line.
@@ -39,8 +44,6 @@ class Rietveld(object):
     # It happens when the presubmit check is ran out of process, the cookie
     # needed to be recreated from the credentials. Instead, it should pass the
     # email and the cookie.
-    self.email = email
-    self.password = password
     if email and password:
       get_creds = lambda: (email, password)
       self.rpc_server = upload.HttpRpcServer(
@@ -438,6 +441,123 @@ class Rietveld(object):
   Send = get
 
 
+class OAuthRpcServer(object):
+  def __init__(self,
+               host,
+               client_id,
+               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_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 {}
+
+    creds = SignedJwtAssertionCredentials(
+      client_id,
+      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,
+           content_type='application/octet-stream',
+           timeout=None,
+           extra_headers=None,
+           **kwargs):
+    """Send a POST or GET request to the server.
+
+    Args:
+      request_path: path on the server to hit. This is concatenated with the
+        value of 'host' provided to the constructor.
+      payload: request is a POST if not None, GET otherwise
+      timeout: in seconds
+      extra_headers: (dict)
+    """
+    # 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.')
+
+    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)
+
+      ret = self._http.request(url,
+                               method=method,
+                               body=payload,
+                               headers=headers)
+      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.

[M-A Ruel, 2014/03/22 01:09:09]

Not a fan. Either split up the code into a third base class or make it work out.
I don't think it's a good design to skip calling the super class constructor.

[pgervais, 2014/03/24 20:27:36]

Well, this class is kind of a hack anyway, because the base class design is
-hum- bad. I would have fixed the base class if I ever knew what/who was using
it, and how (tests?) Since depot_tools does not define a clear boundary, I
expect some random code I'm not aware of to break if I change anything in the
existing classes/functions. I might be overthinking it, but I have been bitten
by that in the past.

If you have a solution to fix the base class while not breaking anything, I'd be
happy to implement it.

+  # pylint: disable=W0231
+  def __init__(self,
+               url,
+               client_id,
+               client_private_key_file,
+               private_key_password=None,
+               extra_headers=None):
+    if private_key_password is None:  # '' means 'empty password'
+      private_key_password = 'notasecret'
+
+    self.url = url.rstrip('/')
+    with open(client_private_key_file, 'rb') as f:
+      client_private_key = f.read()
+    self.rpc_server = OAuthRpcServer(url,
+                                     client_id,
+                                     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.