Reland "depot_tools: Stop using oauth2client"

This is a reland of 55e5853e5c

Original change's description:
> depot_tools: Stop using oauth2client
>
> Bug: 1001756
> Change-Id: I8a0ca2b0f44b20564a9d3192543a7a69788d8d87
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1854898
> Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
> Reviewed-by: Vadim Shtayura <vadimsh@chromium.org>

Bug: 1001756
Recipe-Nontrivial-Roll: chromiumos
Recipe-Nontrivial-Roll: skia
Change-Id: If2f584ce0b327324cfb67ce5f29d80986260bd61
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1867109
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Vadim Shtayura <vadimsh@chromium.org>

auth.py

@@ -21,7 +21,6 @@ import urlparse
 import subprocess2
 
 from third_party import httplib2
-from third_party.oauth2client import client
 
 
 # depot_tools/.
@@ -35,6 +34,11 @@ OAUTH_SCOPE_GERRIT = 'https://www.googleapis.com/auth/gerritcodereview'
 OAUTH_SCOPES = OAUTH_SCOPE_EMAIL
 
 
+# Mockable datetime.datetime.utcnow for testing.
+def datetime_now():
+  return datetime.datetime.utcnow()
+
+
 # Authentication configuration extracted from command line options.
 # See doc string for 'make_auth_config' for meaning of fields.
 AuthConfig = collections.namedtuple('AuthConfig', [
@@ -54,9 +58,9 @@ class AccessToken(collections.namedtuple('AccessToken', [
   def needs_refresh(self, now=None):
     """True if this AccessToken should be refreshed."""
     if self.expires_at is not None:
-      now = now or datetime.datetime.utcnow()
+      now = now or datetime_now()
-      # Allow 3 min of clock skew between client and backend.
+      # Allow 30s of clock skew between client and backend.
-      now += datetime.timedelta(seconds=180)
+      now += datetime.timedelta(seconds=30)
       return now >= self.expires_at
     # Token without expiration time never expires.
     return False
@@ -100,6 +104,8 @@ def has_luci_context_local_auth():
   return bool(params.default_account_id)
 
 
+# TODO(crbug.com/1001756): Remove. luci-auth uses local auth if available,
+# making this unnecessary.
 def get_luci_context_access_token(scopes=OAUTH_SCOPE_EMAIL):
   """Returns a valid AccessToken from the local LUCI context auth server.
 
@@ -291,18 +297,18 @@ def add_auth_options(parser, default_config=None):
       help='Do not save authentication cookies to local disk.')
 
   # OAuth2 related options.
+  # TODO(crbug.com/1001756): Remove. No longer supported.
   parser.auth_group.add_option(
       '--auth-no-local-webserver',
       action='store_false',
       dest='use_local_webserver',
       default=default_config.use_local_webserver,
-      help='Do not run a local web server when performing OAuth2 login flow.')
+      help='DEPRECATED. Do not use')
   parser.auth_group.add_option(
       '--auth-host-port',
       type=int,
       default=default_config.webserver_port,
-      help='Port a local web server should listen on. Used only if '
+      help='DEPRECATED. Do not use')
-          '--auth-no-local-webserver is not set. [default: %default]')
   parser.auth_group.add_option(
       '--auth-refresh-token-json',
       help='DEPRECATED. Do not use')
@@ -372,82 +378,49 @@ class Authenticator(object):
     logging.debug('Using auth config %r', config)
 
   def has_cached_credentials(self):
-    """Returns True if long term credentials (refresh token) are in cache.
+    """Returns True if credentials can be obtained.
 
-    Doesn't make network calls.
+    If returns False, get_access_token() later will probably ask for interactive
+    login by raising LoginRequiredError, unless local auth is configured.
 
-    If returns False, get_access_token() later will ask for interactive login by
+    If returns True, get_access_token() won't ask for interactive login.
-    raising LoginRequiredError.
-
-    If returns True, most probably get_access_token() won't ask for interactive
-    login, though it is not guaranteed, since cached token can be already
-    revoked and there's no way to figure this out without actually trying to use
-    it.
     """
     with self._lock:
-      return bool(self._get_cached_credentials())
+      return bool(self._get_luci_auth_token())
 
   def get_access_token(self, force_refresh=False, allow_user_interaction=False,
                        use_local_auth=True):
     """Returns AccessToken, refreshing it if necessary.
 
     Args:
-      force_refresh: forcefully refresh access token even if it is not expired.
+      TODO(crbug.com/1001756): Remove.
-      allow_user_interaction: True to enable blocking for user input if needed.
+      force_refresh: Ignored, luci-auth doesn't support force-refreshing tokens.
-      use_local_auth: default to local auth if needed.
+      allow_user_interaction: Ignored. allow_user_interaction is always False.
+      use_local_auth: Ignored. luci-auth already covers local_auth.
 
     Raises:
       AuthenticationError on error or if authentication flow was interrupted.
       LoginRequiredError if user interaction is required, but
           allow_user_interaction is False.
     """
-    def get_loc_auth_tkn():
-      exi = sys.exc_info()
-      if not use_local_auth:
-        logging.error('Failed to create access token')
-        raise
-      try:
-        self._access_token = get_luci_context_access_token()
-        if not self._access_token:
-          logging.error('Failed to create access token')
-          raise
-        return self._access_token
-      except LuciContextAuthError:
-        logging.exception('Failed to use local auth')
-        raise exi[0], exi[1], exi[2]
-
     with self._lock:
-      if force_refresh:
+      if self._access_token and not self._access_token.needs_refresh():
-        logging.debug('Forcing access token refresh')
-        try:
-          self._access_token = self._create_access_token(allow_user_interaction)
         return self._access_token
-        except LoginRequiredError:
-          return get_loc_auth_tkn()
 
-      # Load from on-disk cache on a first access.
+      # Token expired or missing. Maybe some other process already updated it,
-      if not self._access_token:
+      # reload from the cache.
-        self._access_token = self._load_access_token()
+      self._access_token = self._get_luci_auth_token()
+      if self._access_token and not self._access_token.needs_refresh():
+        return self._access_token
 
-      # Refresh if expired or missing.
-      if not self._access_token or self._access_token.needs_refresh():
-        # Maybe some other process already updated it, reload from the cache.
-        self._access_token = self._load_access_token()
       # Nope, still expired, need to run the refresh flow.
-        if not self._access_token or self._access_token.needs_refresh():
+      logging.error('Failed to create access token')
-          try:
+      raise LoginRequiredError(self._scopes)
-            self._access_token = self._create_access_token(
-                allow_user_interaction)
-          except LoginRequiredError:
-            get_loc_auth_tkn()
-
-      return self._access_token
 
   def authorize(self, http):
     """Monkey patches authentication logic of httplib2.Http instance.
 
     The modified http.request method will add authentication headers to each
-    request and will refresh access_tokens when a 401 is received on a
     request.
 
     Args:
@@ -457,7 +430,6 @@ class Authenticator(object):
        A modified instance of http that was passed in.
     """
     # Adapted from oauth2client.OAuth2Credentials.authorize.
-
     request_orig = http.request
 
     @functools.wraps(request_orig)
@@ -467,92 +439,37 @@ class Authenticator(object):
         connection_type=None):
       headers = (headers or {}).copy()
       headers['Authorization'] = 'Bearer %s' % self.get_access_token().token
-      resp, content = request_orig(
-          uri, method, body, headers, redirections, connection_type)
-      if resp.status in client.REFRESH_STATUS_CODES:
-        logging.info('Refreshing due to a %s', resp.status)
-        access_token = self.get_access_token(force_refresh=True)
-        headers['Authorization'] = 'Bearer %s' % access_token.token
       return request_orig(
           uri, method, body, headers, redirections, connection_type)
-      else:
-        return (resp, content)
 
     http.request = new_request
     return http
 
   ## Private methods.
 
-  def _get_cached_credentials(self):
+  def _run_luci_auth_login(self):
-    """Returns oauth2client.Credentials loaded from luci-auth."""
+    """Run luci-auth login.
-    credentials = _get_luci_auth_credentials(self._scopes)
-
-    if not credentials:
-      logging.debug('No cached token')
-    else:
-      _log_credentials_info('cached token', credentials)
-
-    return credentials if (credentials and not credentials.invalid) else None
-
-  def _load_access_token(self):
-    """Returns cached AccessToken if it is not expired yet."""
-    logging.debug('Reloading access token from cache')
-    creds = self._get_cached_credentials()
-    if not creds or not creds.access_token or creds.access_token_expired:
-      logging.debug('Access token is missing or expired')
-      return None
-    return AccessToken(str(creds.access_token), creds.token_expiry)
-
-  def _create_access_token(self, allow_user_interaction=False):
-    """Mints and caches a new access token, launching OAuth2 dance if necessary.
-
-    Uses cached refresh token, if present. In that case user interaction is not
-    required and function will finish quietly. Otherwise it will launch 3-legged
-    OAuth2 flow, that needs user interaction.
-
-    Args:
-      allow_user_interaction: if True, allow interaction with the user (e.g.
-          reading standard input, or launching a browser).
 
     Returns:
-      AccessToken.
+      AccessToken with credentials.
-
-    Raises:
-      AuthenticationError on error or if authentication flow was interrupted.
-      LoginRequiredError if user interaction is required, but
-          allow_user_interaction is False.
     """
-    logging.debug(
+    logging.debug('Running luci-auth login')
-        'Making new access token (allow_user_interaction=%r)',
+    subprocess2.check_call(['luci-auth', 'login', '-scopes', self._scopes])
-        allow_user_interaction)
+    return self._get_luci_auth_token()
-    credentials = self._get_cached_credentials()
 
-    # 3-legged flow with (perhaps cached) refresh token.
+  def _get_luci_auth_token(self):
-    refreshed = False
+    logging.debug('Running luci-auth token')
-    if credentials and not credentials.invalid:
     try:
-        logging.debug('Attempting to refresh access_token')
+      out, err = subprocess2.check_call_out(
-        credentials.refresh(httplib2.Http())
+          ['luci-auth', 'token', '-scopes', self._scopes, '-json-output', '-'],
-        _log_credentials_info('refreshed token', credentials)
+          stdout=subprocess2.PIPE, stderr=subprocess2.PIPE)
-        refreshed = True
+      logging.debug('luci-auth token stderr:\n%s', err)
-      except client.Error as err:
+      token_info = json.loads(out)
-        logging.warning(
+      return AccessToken(
-            'OAuth error during access token refresh (%s). '
+          token_info['token'],
-            'Attempting a full authentication flow.', err)
+          datetime.datetime.utcfromtimestamp(token_info['expiry']))
-
+    except subprocess2.CalledProcessError:
-    # Refresh token is missing or invalid, go through the full flow.
+      return None
-    if not refreshed:
-      if not allow_user_interaction:
-        logging.debug('Requesting user to login')
-        raise LoginRequiredError(self._scopes)
-      logging.debug('Launching OAuth browser flow')
-      credentials = _run_oauth_dance(self._scopes)
-      _log_credentials_info('new token', credentials)
-
-    logging.info(
-        'OAuth access_token refreshed. Expires in %s.',
-        credentials.token_expiry - datetime.datetime.utcnow())
-    return AccessToken(str(credentials.access_token), credentials.token_expiry)
 
 
 ## Private functions.
@@ -561,44 +478,3 @@ class Authenticator(object):
 def _is_headless():
   """True if machine doesn't seem to have a display."""
   return sys.platform == 'linux2' and not os.environ.get('DISPLAY')
-
-
-def _log_credentials_info(title, credentials):
-  """Dumps (non sensitive) part of client.Credentials object to debug log."""
-  if credentials:
-    logging.debug('%s info: %r', title, {
-        'access_token_expired': credentials.access_token_expired,
-        'has_access_token': bool(credentials.access_token),
-        'invalid': credentials.invalid,
-        'utcnow': datetime.datetime.utcnow(),
-        'token_expiry': credentials.token_expiry,
-    })
-
-
-def _get_luci_auth_credentials(scopes):
-  try:
-    token_info = json.loads(subprocess2.check_output(
-        ['luci-auth', 'token', '-scopes', scopes, '-json-output', '-'],
-        stderr=subprocess2.VOID))
-  except subprocess2.CalledProcessError:
-    return None
-
-  return client.OAuth2Credentials(
-      access_token=token_info['token'],
-      client_id=None,
-      client_secret=None,
-      refresh_token=None,
-      token_expiry=datetime.datetime.utcfromtimestamp(token_info['expiry']),
-      token_uri=None,
-      user_agent=None,
-      revoke_uri=None)
-
-
-def _run_oauth_dance(scopes):
-  """Perform full 3-legged OAuth2 flow with the browser.
-
-  Returns:
-    oauth2client.Credentials.
-  """
-  subprocess2.check_call(['luci-auth', 'login', '-scopes', scopes])
-  return _get_luci_auth_credentials(scopes)

tests/auth_test.py

@@ -5,104 +5,97 @@
 
 """Unit Tests for auth.py"""
 
-import __builtin__
+import calendar
 import datetime
 import json
-import logging
 import os
 import unittest
 import sys
-import time
 
 sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
 
-
-from testing_support import auto_stub
-from third_party import httplib2
 from third_party import mock
 
 import auth
+import subprocess2
+
 
+NOW = datetime.datetime(2019, 10, 17, 12, 30, 59, 0)
+VALID_EXPIRY = NOW + datetime.timedelta(seconds=31)
 
-class TestLuciContext(auto_stub.TestCase):
+
+class AuthenticatorTest(unittest.TestCase):
+  def setUp(self):
+    mock.patch('subprocess2.check_call').start()
+    mock.patch('subprocess2.check_call_out').start()
+    mock.patch('auth.datetime_now', return_value=NOW).start()
+    self.addCleanup(mock.patch.stopall)
+
+  def testHasCachedCredentials_NotLoggedIn(self):
+    subprocess2.check_call_out.side_effect = [
+        subprocess2.CalledProcessError(1, ['cmd'], 'cwd', 'stdout', 'stderr')]
+    authenticator = auth.get_authenticator(auth.make_auth_config())
+    self.assertFalse(authenticator.has_cached_credentials())
+
+  def testHasCachedCredentials_LoggedIn(self):
+    subprocess2.check_call_out.return_value = (
+        json.dumps({'token': 'token', 'expiry': 12345678}), '')
+    authenticator = auth.get_authenticator(auth.make_auth_config())
+    self.assertTrue(authenticator.has_cached_credentials())
+
+  def testGetAccessToken_NotLoggedIn(self):
+    subprocess2.check_call_out.side_effect = [
+        subprocess2.CalledProcessError(1, ['cmd'], 'cwd', 'stdout', 'stderr')]
+    authenticator = auth.get_authenticator(auth.make_auth_config())
+    self.assertRaises(auth.LoginRequiredError, authenticator.get_access_token)
+
+  def testGetAccessToken_CachedToken(self):
+    authenticator = auth.get_authenticator(auth.make_auth_config())
+    authenticator._access_token = auth.AccessToken('token', None)
+    self.assertEqual(
+        auth.AccessToken('token', None), authenticator.get_access_token())
+
+  def testGetAccesstoken_LoggedIn(self):
+    expiry = calendar.timegm(VALID_EXPIRY.timetuple())
+    subprocess2.check_call_out.return_value = (
+        json.dumps({'token': 'token', 'expiry': expiry}), '')
+    authenticator = auth.get_authenticator(auth.make_auth_config())
+    self.assertEqual(
+        auth.AccessToken('token', VALID_EXPIRY),
+        authenticator.get_access_token())
+
+  def testAuthorize(self):
+    http = mock.Mock()
+    http_request = http.request
+    http_request.__name__ = '__name__'
+
+    authenticator = auth.get_authenticator(auth.make_auth_config())
+    authenticator._access_token = auth.AccessToken('token', None)
+
+    authorized = authenticator.authorize(http)
+    authorized.request(
+        'https://example.com', method='POST', body='body',
+        headers={'header': 'value'})
+    http_request.assert_called_once_with(
+        'https://example.com', 'POST', 'body',
+        {'header': 'value', 'Authorization': 'Bearer token'}, mock.ANY,
+        mock.ANY)
+
+
+class AccessTokenTest(unittest.TestCase):
   def setUp(self):
-    auth._get_luci_context_local_auth_params.clear_cache()
+    mock.patch('auth.datetime_now', return_value=NOW).start()
-
+    self.addCleanup(mock.patch.stopall)
-  def _mock_local_auth(self, account_id, secret, rpc_port):
+
-    self.mock(os, 'environ', {'LUCI_CONTEXT': 'default/test/path'})
+  def testNeedsRefresh_NoExpiry(self):
-    self.mock(auth, '_load_luci_context', mock.Mock())
+    self.assertFalse(auth.AccessToken('token', None).needs_refresh())
-    auth._load_luci_context.return_value = {
+
-      'local_auth': {
+  def testNeedsRefresh_Expired(self):
-        'default_account_id': account_id,
+    expired = NOW + datetime.timedelta(seconds=30)
-        'secret': secret,
+    self.assertTrue(auth.AccessToken('token', expired).needs_refresh())
-        'rpc_port': rpc_port,
+
-      }
+  def testNeedsRefresh_Valid(self):
-    }
+    self.assertFalse(auth.AccessToken('token', VALID_EXPIRY).needs_refresh())
-
-  def _mock_loc_server_resp(self, status, content):
-    mock_resp = mock.Mock()
-    mock_resp.status = status
-    self.mock(httplib2.Http, 'request', mock.Mock())
-    httplib2.Http.request.return_value = (mock_resp, content)
-
-  def test_all_good(self):
-    self._mock_local_auth('account', 'secret', 8080)
-    self.assertTrue(auth.has_luci_context_local_auth())
-
-    expiry_time = datetime.datetime.min + datetime.timedelta(hours=1)
-    resp_content = {
-      'error_code': None,
-      'error_message': None,
-      'access_token': 'token',
-      'expiry': (expiry_time
-                 - datetime.datetime.utcfromtimestamp(0)).total_seconds(),
-    }
-    self._mock_loc_server_resp(200, json.dumps(resp_content))
-    params = auth._get_luci_context_local_auth_params()
-    token = auth._get_luci_context_access_token(params, datetime.datetime.min)
-    self.assertEqual(token.token, 'token')
-
-  def test_no_account_id(self):
-    self._mock_local_auth(None, 'secret', 8080)
-    self.assertFalse(auth.has_luci_context_local_auth())
-    self.assertIsNone(auth.get_luci_context_access_token())
-
-  def test_incorrect_port_format(self):
-    self._mock_local_auth('account', 'secret', 'port')
-    self.assertFalse(auth.has_luci_context_local_auth())
-    with self.assertRaises(auth.LuciContextAuthError):
-      auth.get_luci_context_access_token()
-
-  def test_expired_token(self):
-    params = auth._LuciContextLocalAuthParams('account', 'secret', 8080)
-    resp_content = {
-      'error_code': None,
-      'error_message': None,
-      'access_token': 'token',
-      'expiry': 1,
-    }
-    self._mock_loc_server_resp(200, json.dumps(resp_content))
-    with self.assertRaises(auth.LuciContextAuthError):
-      auth._get_luci_context_access_token(
-          params, datetime.datetime.utcfromtimestamp(1))
-
-  def test_incorrect_expiry_format(self):
-    params = auth._LuciContextLocalAuthParams('account', 'secret', 8080)
-    resp_content = {
-      'error_code': None,
-      'error_message': None,
-      'access_token': 'token',
-      'expiry': 'dead',
-    }
-    self._mock_loc_server_resp(200, json.dumps(resp_content))
-    with self.assertRaises(auth.LuciContextAuthError):
-      auth._get_luci_context_access_token(params, datetime.datetime.min)
-
-  def test_incorrect_response_content_format(self):
-    params = auth._LuciContextLocalAuthParams('account', 'secret', 8080)
-    self._mock_loc_server_resp(200, '5')
-    with self.assertRaises(auth.LuciContextAuthError):
-      auth._get_luci_context_access_token(params, datetime.datetime.min)
 
 
 if __name__ == '__main__':