From 733d4ec8e3364f9491291963475aee2a137bb2f1 Mon Sep 17 00:00:00 2001 From: Andrii Shyshkalov Date: Thu, 19 Apr 2018 11:48:58 -0700 Subject: [PATCH] [gerrit_util] learn about and use LUCI_CONTEXT when available. Unfortunately, w/o rewrite of gerrit_util, one can't take advantage of existing auth.Authenticator (which also doesn't know about ~/.netrc or .gitcookies, but that's fixable). This rewrite, however, will likely cause issues for nasty scripts outside of depot_tools which use gerrit_util as a Python library. So, I followed outdated way of gerrit_util :( Also contains refactoring of auth library, which was kept backwards compatible for the same reasons as above. R=vadimsh@chromium.org Test-with: led tool Test-artifact: https://ci.chromium.org/swarming/task/3cf4687dfd26ca10?server=chromium-swarm.appspot.com Bug: 834536 Change-Id: I6b84b8b5732aee5d345e2b2ba44f47aaecc0f6c5 Reviewed-on: https://chromium-review.googlesource.com/1018488 Reviewed-by: Vadim Shtayura Commit-Queue: Andrii Shyshkalov --- auth.py | 185 +++++++++++++++++------------- gerrit_util.py | 45 ++++++-- recipes/trigger_recipe_roller.txt | 2 +- tests/auth_test.py | 43 +++---- tests/git_cl_test.py | 2 + 5 files changed, 169 insertions(+), 108 deletions(-) diff --git a/auth.py b/auth.py index c415610163..e4f5e81b17 100644 --- a/auth.py +++ b/auth.py @@ -45,9 +45,12 @@ OAUTH_CLIENT_ID = ( '446450136466-2hr92jrq8e6i4tnsa56b52vacp7t3936.apps.googleusercontent.com') OAUTH_CLIENT_SECRET = 'uBfbay2KCy9t4QveJ-dOqHtp' -# List of space separated OAuth scopes for generated tokens. GAE apps usually -# use userinfo.email scope for authentication. -OAUTH_SCOPES = 'https://www.googleapis.com/auth/userinfo.email' +# This is what most GAE apps require for authentication. +OAUTH_SCOPE_EMAIL = 'https://www.googleapis.com/auth/userinfo.email' +# Gerrit and Git on *.googlesource.com require this scope. +OAUTH_SCOPE_GERRIT = 'https://www.googleapis.com/auth/gerritcodereview' +# Deprecated. Use OAUTH_SCOPE_EMAIL instead. +OAUTH_SCOPES = OAUTH_SCOPE_EMAIL # Additional OAuth scopes. ADDITIONAL_SCOPES = { @@ -74,10 +77,20 @@ AuthConfig = collections.namedtuple('AuthConfig', [ # OAuth access token with its expiration time (UTC datetime or None if unknown). -AccessToken = collections.namedtuple('AccessToken', [ +class AccessToken(collections.namedtuple('AccessToken', [ 'token', 'expires_at', -]) + ])): + + 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() + # Allow 5 min of clock skew between client and backend. + now += datetime.timedelta(seconds=300) + return now >= self.expires_at + # Token without expiration time never expires. + return False # Refresh token passed via --auth-refresh-token-json. @@ -106,8 +119,28 @@ class LoginRequiredError(AuthenticationError): class LuciContextAuthError(Exception): """Raised on errors related to unsuccessful attempts to load LUCI_CONTEXT""" + def __init__(self, msg, exc=None): + if exc is None: + logging.error(msg) + else: + logging.exception(msg) + msg = '%s: %s' % (msg, exc) + super(LuciContextAuthError, self).__init__(msg) + + +def has_luci_context_local_auth(): + """Returns whether LUCI_CONTEXT should be used for ambient authentication. + """ + try: + params = _get_luci_context_local_auth_params(os.environ) + except LuciContextAuthError: + return False + if params is None: + return False + return bool(params.default_account_id) + -def get_luci_context_access_token(): +def get_luci_context_access_token(scopes=OAUTH_SCOPE_EMAIL): """Returns a valid AccessToken from the local LUCI context auth server. Adapted from @@ -119,83 +152,97 @@ def get_luci_context_access_token(): None if LUCI_CONTEXT is absent. Raises: - LuciContextAuthError if the attempt to load LUCI_CONTEXT - and request its access token is unsuccessful. + LuciContextAuthError if LUCI_CONTEXT is present, but there was a failure + obtaining its access token. """ - return _get_luci_context_access_token(os.environ, datetime.datetime.utcnow()) + params = _get_luci_context_local_auth_params(os.environ) + if params is None: + return None + return _get_luci_context_access_token( + params, datetime.datetime.utcnow(), scopes) + + +_LuciContextLocalAuthParams = collections.namedtuple( + '_LuciContextLocalAuthParams', [ + 'default_account_id', + 'secret', + 'rpc_port', +]) -def _get_luci_context_access_token(env, now): +def _get_luci_context_local_auth_params(env): + """Returns local auth parameters if local auth is configured else None. + + Raises LuciContextAuthError on unexpected failures. + """ ctx_path = env.get('LUCI_CONTEXT') if not ctx_path: return None ctx_path = ctx_path.decode(sys.getfilesystemencoding()) - logging.debug('Loading LUCI_CONTEXT: %r', ctx_path) - - def authErr(msg, *args): - error_msg = msg % args - ex = sys.exc_info()[1] - if not ex: - logging.error(error_msg) - raise LuciContextAuthError(error_msg) - logging.exception(error_msg) - raise LuciContextAuthError('%s: %s' % (error_msg, ex)) - try: loaded = _load_luci_context(ctx_path) - except (OSError, IOError, ValueError): - authErr('Failed to open, read or decode LUCI_CONTEXT') + except (OSError, IOError, ValueError) as e: + raise LuciContextAuthError('Failed to open, read or decode LUCI_CONTEXT', e) try: local_auth = loaded.get('local_auth') - except AttributeError: - authErr('LUCI_CONTEXT not in proper format') - # failed to grab local_auth from LUCI context - if not local_auth: - logging.debug('local_auth: no local auth found') + except AttributeError as e: + raise LuciContextAuthError('LUCI_CONTEXT not in proper format', e) + if local_auth is None: + logging.debug('LUCI_CONTEXT configured w/o local auth') return None try: - account_id = local_auth.get('default_account_id') - secret = local_auth.get('secret') - rpc_port = int(local_auth.get('rpc_port')) - except (AttributeError, ValueError): - authErr('local_auth: unexpected local auth format') - - if not secret: - authErr('local_auth: no secret returned') - # if account_id not specified, LUCI_CONTEXT should not be picked up - if not account_id: + return _LuciContextLocalAuthParams( + default_account_id=local_auth.get('default_account_id'), + secret=local_auth.get('secret'), + rpc_port=int(local_auth.get('rpc_port'))) + except (AttributeError, ValueError) as e: + raise LuciContextAuthError('local_auth config malformed', e) + + +def _load_luci_context(ctx_path): + # Kept separate for test mocking. + with open(ctx_path) as f: + return json.load(f) + + +def _get_luci_context_access_token(params, now, scopes=OAUTH_SCOPE_EMAIL): + # No account, local_auth shouldn't be used. + if not params.default_account_id: return None + if not params.secret: + raise LuciContextAuthError('local_auth: no secret') logging.debug('local_auth: requesting an access token for account "%s"', - account_id) + params.default_account_id) http = httplib2.Http() - host = '127.0.0.1:%d' % rpc_port + host = '127.0.0.1:%d' % params.rpc_port resp, content = http.request( uri='http://%s/rpc/LuciLocalAuthService.GetOAuthToken' % host, method='POST', body=json.dumps({ - 'account_id': account_id, - 'scopes': OAUTH_SCOPES.split(' '), - 'secret': secret, + 'account_id': params.default_account_id, + 'scopes': scopes.split(' '), + 'secret': params.secret, }), headers={'Content-Type': 'application/json'}) if resp.status != 200: - err = ('local_auth: Failed to grab access token from ' - 'LUCI context server with status %d: %r') - authErr(err, resp.status, content) + raise LuciContextAuthError( + 'local_auth: Failed to grab access token from ' + 'LUCI context server with status %d: %r' % (resp.status, content)) try: token = json.loads(content) error_code = token.get('error_code') error_message = token.get('error_message') access_token = token.get('access_token') expiry = token.get('expiry') - except (AttributeError, ValueError): - authErr('local_auth: Unexpected access token response format') + except (AttributeError, ValueError) as e: + raise LuciContextAuthError('Unexpected access token response format', e) if error_code: - authErr('local_auth: Error %d in retrieving access token: %s', - error_code, error_message) + raise LuciContextAuthError( + 'Error %d in retrieving access token: %s', error_code, error_message) if not access_token: - authErr('local_auth: No access token returned from LUCI context server') + raise LuciContextAuthError( + 'No access token returned from LUCI context server') expiry_dt = None if expiry: try: @@ -203,25 +250,19 @@ def _get_luci_context_access_token(env, now): logging.debug( 'local_auth: got an access token for ' 'account "%s" that expires in %d sec', - account_id, (expiry_dt - now).total_seconds()) - except (TypeError, ValueError): - authErr('Invalid expiry in returned token') + params.default_account_id, (expiry_dt - now).total_seconds()) + except (TypeError, ValueError) as e: + raise LuciContextAuthError('Invalid expiry in returned token', e) else: logging.debug( - 'local auth: got an access token for ' - 'account "%s" that does not expire', - account_id) + 'local auth: got an access token for account "%s" that does not expire', + params.default_account_id) access_token = AccessToken(access_token, expiry_dt) - if _needs_refresh(access_token, now=now): - authErr('local_auth: the returned access token needs to be refreshed') + if access_token.needs_refresh(now=now): + raise LuciContextAuthError('Received access token is already expired') return access_token -def _load_luci_context(ctx_path): - with open(ctx_path) as f: - return json.load(f) - - def make_auth_config( use_oauth2=None, save_cookies=None, @@ -347,6 +388,7 @@ def get_authenticator_for_host(hostname, config): # Append some scheme, otherwise urlparse puts hostname into parsed.path. if '://' not in hostname: hostname = 'https://' + hostname + # TODO(tandrii): this is horrible. scopes = OAUTH_SCOPES parsed = urlparse.urlparse(hostname) if parsed.netloc in ADDITIONAL_SCOPES: @@ -469,11 +511,11 @@ class Authenticator(object): self._access_token = self._load_access_token() # Refresh if expired or missing. - if not self._access_token or _needs_refresh(self._access_token): + if not self._access_token or self._access_toke.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 _needs_refresh(self._access_token): + if not self._access_token or self._access_token.needs_refresh(): try: self._access_token = self._create_access_token( allow_user_interaction) @@ -695,17 +737,6 @@ def _read_refresh_token_json(path): 'Failed to read refresh token from %s: missing key %s' % (path, e)) -def _needs_refresh(access_token, now=None): - """True if AccessToken should be refreshed.""" - if access_token.expires_at is not None: - now = now or datetime.datetime.utcnow() - # Allow 5 min of clock skew between client and backend. - now += datetime.timedelta(seconds=300) - return now >= access_token.expires_at - # Token without expiration time never expires. - return False - - def _log_credentials_info(title, credentials): """Dumps (non sensitive) part of client.Credentials object to debug log.""" if credentials: diff --git a/gerrit_util.py b/gerrit_util.py index 14455645cc..9f8ccb783b 100644 --- a/gerrit_util.py +++ b/gerrit_util.py @@ -26,6 +26,7 @@ import urllib import urlparse from cStringIO import StringIO +import auth import gclient_utils import subprocess2 from third_party import httplib2 @@ -86,6 +87,10 @@ class Authenticator(object): Probes the local system and its environment and identifies the Authenticator instance to use. """ + # LUCI Context takes priority since it's normally present only on bots, + # which then must use it. + if LuciContextAuthenticator.is_luci(): + return LuciContextAuthenticator() if GceAuthenticator.is_gce(): return GceAuthenticator() return CookiesAuthenticator() @@ -207,17 +212,17 @@ class CookiesAuthenticator(Authenticator): return self.netrc.authenticators(host) def get_auth_header(self, host): - auth = self._get_auth_for_host(host) - if auth: - return 'Basic %s' % (base64.b64encode('%s:%s' % (auth[0], auth[2]))) + a = self._get_auth_for_host(host) + if a: + return 'Basic %s' % (base64.b64encode('%s:%s' % (a[0], a[2]))) return None def get_auth_email(self, host): """Best effort parsing of email to be used for auth for the given host.""" - auth = self._get_auth_for_host(host) - if not auth: + a = self._get_auth_for_host(host) + if not a: return None - login = auth[0] + login = a[0] # login typically looks like 'git-xxx.example.com' if not login.startswith('git-') or '.' not in login: return None @@ -303,14 +308,36 @@ class GceAuthenticator(Authenticator): return '%(token_type)s %(access_token)s' % token_dict +class LuciContextAuthenticator(Authenticator): + """Authenticator implementation that uses LUCI_CONTEXT ambient local auth. + """ + + @staticmethod + def is_luci(): + return auth.has_luci_context_local_auth() + + def __init__(self): + self._access_token = None + self._ensure_fresh() + + def _ensure_fresh(self): + if not self._access_token or self._access_token.needs_refresh(): + self._access_token = auth.get_luci_context_access_token( + scopes=' '.join([auth.OAUTH_SCOPE_EMAIL, auth.OAUTH_SCOPE_GERRIT])) + + def get_auth_header(self, _host): + self._ensure_fresh() + return 'Bearer %s' % self._access_token.token + + def CreateHttpConn(host, path, reqtype='GET', headers=None, body=None): """Opens an https connection to a gerrit service, and sends a request.""" headers = headers or {} bare_host = host.partition(':')[0] - auth = Authenticator.get().get_auth_header(bare_host) - if auth: - headers.setdefault('Authorization', auth) + a = Authenticator.get().get_auth_header(bare_host) + if a: + headers.setdefault('Authorization', a) else: LOGGER.debug('No authorization found for %s.' % bare_host) diff --git a/recipes/trigger_recipe_roller.txt b/recipes/trigger_recipe_roller.txt index 85019027fe..f13d8eea2f 100644 --- a/recipes/trigger_recipe_roller.txt +++ b/recipes/trigger_recipe_roller.txt @@ -7,4 +7,4 @@ the Build and Test Yeti. Then suddenly, and without delay, Batty the Build and Test Yeti -😒 +😒😒 diff --git a/tests/auth_test.py b/tests/auth_test.py index d34647f0e1..fd515316ba 100755 --- a/tests/auth_test.py +++ b/tests/auth_test.py @@ -24,10 +24,9 @@ from third_party import mock import auth -class TestGetLuciContextAccessToken(auto_stub.TestCase): - mock_env = {'LUCI_CONTEXT': 'default/test/path'} - +class TestLuciContext(auto_stub.TestCase): def _mock_local_auth(self, account_id, secret, rpc_port): + self.mock(os, 'environ', {'LUCI_CONTEXT': 'default/test/path'}) self.mock(auth, '_load_luci_context', mock.Mock()) auth._load_luci_context.return_value = { 'local_auth': { @@ -43,8 +42,10 @@ class TestGetLuciContextAccessToken(auto_stub.TestCase): self.mock(httplib2.Http, 'request', mock.Mock()) httplib2.Http.request.return_value = (mock_resp, content) - def test_correct_local_auth_format(self): - self._mock_local_auth('dead', 'beef', 10) + 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, @@ -54,23 +55,23 @@ class TestGetLuciContextAccessToken(auto_stub.TestCase): - datetime.datetime.utcfromtimestamp(0)).total_seconds(), } self._mock_loc_server_resp(200, json.dumps(resp_content)) - token = auth._get_luci_context_access_token( - self.mock_env, datetime.datetime.min) + params = auth._get_luci_context_local_auth_params(os.environ) + token = auth._get_luci_context_access_token(params, datetime.datetime.min) self.assertEquals(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('foo', 'bar', 'bar') + 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(self.mock_env, datetime.datetime.min) - - def test_no_account_id(self): - self._mock_local_auth(None, 'bar', 10) - token = auth._get_luci_context_access_token( - self.mock_env, datetime.datetime.min) - self.assertIsNone(token) + auth.get_luci_context_access_token() def test_expired_token(self): - self._mock_local_auth('dead', 'beef', 10) + params = auth._LuciContextLocalAuthParams('account', 'secret', 8080) resp_content = { 'error_code': None, 'error_message': None, @@ -80,10 +81,10 @@ class TestGetLuciContextAccessToken(auto_stub.TestCase): self._mock_loc_server_resp(200, json.dumps(resp_content)) with self.assertRaises(auth.LuciContextAuthError): auth._get_luci_context_access_token( - self.mock_env, datetime.datetime.utcfromtimestamp(1)) + params, datetime.datetime.utcfromtimestamp(1)) def test_incorrect_expiry_format(self): - self._mock_local_auth('dead', 'beef', 10) + params = auth._LuciContextLocalAuthParams('account', 'secret', 8080) resp_content = { 'error_code': None, 'error_message': None, @@ -92,13 +93,13 @@ class TestGetLuciContextAccessToken(auto_stub.TestCase): } self._mock_loc_server_resp(200, json.dumps(resp_content)) with self.assertRaises(auth.LuciContextAuthError): - auth._get_luci_context_access_token(self.mock_env, datetime.datetime.min) + auth._get_luci_context_access_token(params, datetime.datetime.min) def test_incorrect_response_content_format(self): - self._mock_local_auth('dead', 'beef', 10) + params = auth._LuciContextLocalAuthParams('account', 'secret', 8080) self._mock_loc_server_resp(200, '5') with self.assertRaises(auth.LuciContextAuthError): - auth._get_luci_context_access_token(self.mock_env, datetime.datetime.min) + auth._get_luci_context_access_token(params, datetime.datetime.min) if __name__ == '__main__': diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index b138c65d45..263a455aeb 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -663,6 +663,8 @@ class TestGitCl(TestCase): self.mock(git_cl.gerrit_util, 'SetReview', lambda h, i, msg=None, labels=None, notify=None: self._mocked_call('SetReview', h, i, msg, labels, notify)) + self.mock(git_cl.gerrit_util.LuciContextAuthenticator, 'is_luci', + staticmethod(lambda: False)) self.mock(git_cl.gerrit_util.GceAuthenticator, 'is_gce', classmethod(lambda _: False)) self.mock(git_cl, 'DieWithError',